Fix issue in image decoders where the bitmap config was not properly set.

Fix issue in image decoders where the bitmap config was not properly set.

R=scroggo@google.com

Committed: https://code.google.com/p/skia/source/detail?r=13503

[scroggo, 2014-02-19 20:05:43]

lgtm

https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp
File src/images/SkImageRef.cpp (right):

https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp#ne...
src/images/SkImageRef.cpp:34: // can get communicated down to the codec.
nit: I'd find this easier to follow if it stated that it is passed down as the
preferred config (it's not intuitive to me that the codec would need to know the
wrapping bitmap's colortype/alphatype).

https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp#ne...
src/images/SkImageRef.cpp:129: SkASSERT(this->info().fHeight ==
fBitmap.height());
Also the color type?

[reed1, 2014-02-19 20:08:07]

+1 for both of leon's comments

[djsollen, 2014-02-19 20:12:41]

https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp
File src/images/SkImageRef.cpp (right):

https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp#ne...
src/images/SkImageRef.cpp:129: SkASSERT(this->info().fHeight ==
fBitmap.height());
On 2014/02/19 20:05:43, scroggo wrote:
> Also the color type?

Didn't put the colortype here is there is no easy accessor on the bitmap.  I
guess I could do the following...

SkImageInfo info;
SkASSERT(!fBitmap.asImageInfo(&info) || this->info().fColorType ==
info.fColorType);

[reed1, 2014-02-19 20:13:33]

On 2014/02/19 20:12:41, djsollen wrote:
> https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp
> File src/images/SkImageRef.cpp (right):
> 
>
https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp#ne...
> src/images/SkImageRef.cpp:129: SkASSERT(this->info().fHeight ==
> fBitmap.height());
> On 2014/02/19 20:05:43, scroggo wrote:
> > Also the color type?
> 
> Didn't put the colortype here is there is no easy accessor on the bitmap.  I
> guess I could do the following...
> 
> SkImageInfo info;
> SkASSERT(!fBitmap.asImageInfo(&info) || this->info().fColorType ==
> info.fColorType);

Good point. Newer skia has exactly this sort of helper (bm.colorType()) but I'm
fine if you leave it off for now.

[djsollen, 2014-02-19 20:25:33]

https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp
File src/images/SkImageRef.cpp (right):

https://codereview.chromium.org/167763003/diff/1/src/images/SkImageRef.cpp#ne...
src/images/SkImageRef.cpp:34: // can get communicated down to the codec.
On 2014/02/19 20:05:43, scroggo wrote:
> nit: I'd find this easier to follow if it stated that it is passed down as the
> preferred config (it's not intuitive to me that the codec would need to know
the
> wrapping bitmap's colortype/alphatype).

We actually need it to be the actual config and not just the preferred one. If
the codec gives us something other than that then we have a problem.

[djsollen, 2014-02-19 20:28:30]

The CQ bit was checked by djsollen@google.com

[commit-bot: I haz the power, 2014-02-19 20:28:41]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/djsollen@google.com/167763003/100001

[djsollen, 2014-02-19 21:45:40]

Committed patchset #2 manually as r13503 (presubmit successful).