Support Index8 in SkBitmap.copyTo()

Allow copying an Index8 bitmap when srcConfig and dstConfig are both
Index8.

Also, change the logic of SkBitmap.copyTo() to do memcpy() if srcConfig
and dstConfig are the same.

[reed, 2009-05-01 18:04:59]

http://codereview.appspot.com/53082/diff/1/2
File include/core/SkBitmap.h (right):

http://codereview.appspot.com/53082/diff/1/2#newcode490
Line 490: SkColorTable(const SkColorTable& src);
Is the comment above this constructor correct? It should say: makes a deep-copy
of the src colortable.

http://codereview.appspot.com/53082/diff/1/3
File src/core/SkBitmap.cpp (right):

http://codereview.appspot.com/53082/diff/1/3#newcode699
Line 699: case kIndex8_Config:
indent the "if" block, as the return false is indented below

http://codereview.appspot.com/53082/diff/1/3#newcode708
Line 708: tmp.setConfig(dstConfig, this->width(), this->height());
Can we refactor this so there is only one shared call to allocPixels, perhaps
with a ctable ptr that is sometimes NULL?

Also, you can use SkAutoUnref au(ctable) after you (optionally) allocated it.
This guy will take care of calling unref() for you, so you can just return false
if allocPixels() fails.

http://codereview.appspot.com/53082/diff/1/3#newcode731
Line 731: 
don't exceed 80-columns in the comment

http://codereview.appspot.com/53082/diff/1/4
File src/core/SkColorTable.cpp (right):

http://codereview.appspot.com/53082/diff/1/4#newcode39
Line 39: SkColorTable::SkColorTable(const SkColorTable& src) {
Need to initialize f16Bitcache and fFlags

[weita, 2009-05-01 19:05:08]

http://codereview.appspot.com/53082/diff/1/2
File include/core/SkBitmap.h (right):

http://codereview.appspot.com/53082/diff/1/2#newcode490
Line 490: SkColorTable(const SkColorTable& src);
On 2009/05/01 18:04:59, reed wrote:
> Is the comment above this constructor correct? It should say: makes a
deep-copy
> of the src colortable.

Done.

The original comment should stick with SkColorTalbe(int count). I add a new
comment for the new constructor.

http://codereview.appspot.com/53082/diff/1/3
File src/core/SkBitmap.cpp (right):

http://codereview.appspot.com/53082/diff/1/3#newcode699
Line 699: case kIndex8_Config:
On 2009/05/01 18:04:59, reed wrote:
> indent the "if" block, as the return false is indented below

Done.

http://codereview.appspot.com/53082/diff/1/3#newcode708
Line 708: tmp.setConfig(dstConfig, this->width(), this->height());
On 2009/05/01 18:04:59, reed wrote:
> Can we refactor this so there is only one shared call to allocPixels, perhaps
> with a ctable ptr that is sometimes NULL?
> 
> Also, you can use SkAutoUnref au(ctable) after you (optionally) allocated it.
> This guy will take care of calling unref() for you, so you can just return
false
> if allocPixels() fails.

Done.

http://codereview.appspot.com/53082/diff/1/3#newcode731
Line 731: 
On 2009/05/01 18:04:59, reed wrote:
> don't exceed 80-columns in the comment

Done.

http://codereview.appspot.com/53082/diff/1/4
File src/core/SkColorTable.cpp (right):

http://codereview.appspot.com/53082/diff/1/4#newcode39
Line 39: SkColorTable::SkColorTable(const SkColorTable& src) {
On 2009/05/01 18:04:59, reed wrote:
> Need to initialize f16Bitcache and fFlags

Done.

[weita, 2009-05-01 19:22:41]

Add Patch Set 2

[reed, 2009-05-01 19:58:17]

much better!

http://codereview.appspot.com/53082/diff/9/1006
File include/core/SkBitmap.h (right):

http://codereview.appspot.com/53082/diff/9/1006#newcode488
Line 488: /**
Lets just say we make a deep copy of the colors. Mentioning fColors is a little
impl-specific (and won't make sense if read from some external doxygen
documentation.)

http://codereview.appspot.com/53082/diff/9/1006#newcode493
Line 493: /** Constructs an empty color table (zero colors).
I know this isn't your comment (its mine!), but it should really say something
like: preallocate the colortable to have 'count' colors, which are initially set
to 0

[weita, 2009-05-01 21:53:16]

Done with the pervious comments and pass tests

[weita, 2009-05-01 21:55:49]

http://codereview.appspot.com/53082/diff/9/1006
File include/core/SkBitmap.h (right):

http://codereview.appspot.com/53082/diff/9/1006#newcode488
Line 488: /**
On 2009/05/01 19:58:18, reed wrote:
> Lets just say we make a deep copy of the colors. Mentioning fColors is a
little
> impl-specific (and won't make sense if read from some external doxygen
> documentation.)

Done.

http://codereview.appspot.com/53082/diff/9/1006#newcode493
Line 493: /** Constructs an empty color table (zero colors).
On 2009/05/01 19:58:18, reed wrote:
> I know this isn't your comment (its mine!), but it should really say something
> like: preallocate the colortable to have 'count' colors, which are initially
set
> to 0

Done.

[reed, 2009-05-02 01:52:32]

LGTM