Do not convert non gray PNG to A8.

Do not convert non gray PNG to A8.

If the user requested A8, but the source is not gray, switch to
ARGB8888.

BUG=https://b.corp.google.com/issue?id=9189955
R=reed@google.com

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

[reed1, 2013-09-27 14:20:53]

I find this aspect of the API still the most complicated : how the caller, and
the impl, decide what output-config to use, and given that, what transformation
happens as we write into that output-config (e.g. gray -> alpha).

Going forward, Skia will probably continue to handle decoding, but I'd like us
to consider a clean slate approach to a newer API if possible, to make this sort
of feature clearer.

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libpng.cpp (right):

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libpng.cpp:534: if (*configp !=
SkBitmap::kARGB_4444_Config) {
? If configp is set to kA8_Config and *hasAlphap, why don't we keep the configp
at kA8? Why change it to kARGB?

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libpng.cpp:539: if (PNG_COLOR_TYPE_GRAY != colorType)
{
Is this the same as comparing against srcDepth? If so, I think that might read
better, since earlier (#525) we seem to be converting from png-specific
colorType to skia-specific srcDepth.

[scroggo, 2013-09-27 21:21:35]

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libpng.cpp (right):

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libpng.cpp:534: if (*configp !=
SkBitmap::kARGB_4444_Config) {
On 2013/09/27 14:20:54, reed1 wrote:
> ? If configp is set to kA8_Config and *hasAlphap, why don't we keep the
configp
> at kA8? Why change it to kARGB?

Because we do a pure convert from grayscale to A-8 : one value that meant degree
of white/black now means degree of alpha. I don't think it's obvious how to
handle something that had two dimensions (alpha and gray to gray). It could be
just take the alpha, but Android is not interested in this transformation.

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libpng.cpp:539: if (PNG_COLOR_TYPE_GRAY != colorType)
{
On 2013/09/27 14:20:54, reed1 wrote:
> Is this the same as comparing against srcDepth? If so, I think that might read
> better, since earlier (#525) we seem to be converting from png-specific
> colorType to skia-specific srcDepth.

Yes, that would be equivalent.

[scroggo, 2013-09-30 20:26:56]

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libpng.cpp (right):

https://codereview.chromium.org/24882002/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libpng.cpp:539: if (PNG_COLOR_TYPE_GRAY != colorType)
{
On 2013/09/27 21:21:35, scroggo wrote:
> On 2013/09/27 14:20:54, reed1 wrote:
> > Is this the same as comparing against srcDepth? If so, I think that might
read
> > better, since earlier (#525) we seem to be converting from png-specific
> > colorType to skia-specific srcDepth.
> 
> Yes, that would be equivalent.

Done.

[reed1, 2013-09-30 20:43:28]

lgtm

[scroggo, 2013-10-02 15:50:31]

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