Fixing libpng transform use

This change:
- supports kGray correctly
- avoid extra call to png_get_IHDR by storing the bit depth
- call transforms as needed
- checks for tRNS alpha value in RGB and GRAY color types


BUG=skia:

Committed: https://skia.googlesource.com/skia/+/9693037fd41b7ce545b44beaa3489dcfd915018c

[emmaleer, 2015-07-01 20:13:44]

emmaleer@google.com changed reviewers:
+ msarett@google.com, scroggo@google.com

[emmaleer, 2015-07-01 20:13:45]

These are the initial changes regarding the libpng transforms set in
SkCoded_libpng.
This change only sets the necessary transforms, and the swizzler is used as
before.
Regardless of if we decide to use libpng's transforms to implement swizzling or
not, this code is needed.

[scroggo, 2015-07-01 20:26:01]

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
File src/codec/SkCodec_libpng.cpp (right):

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:418: case kRGBA_8888_SkColorType:
Don't we need to update the swizzler in order to support both?

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:438: fNumberPasses =
png_set_interlace_handling(fPng_ptr);
I think you mentioned this to me before: png_set_interlace_handling returns 1
for a non-interlaced image?

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:467: //would have excited before now if the
colorType was supported by png
exited*

[emmaleer, 2015-07-01 20:56:58]

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
File src/codec/SkCodec_libpng.cpp (right):

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:418: case kRGBA_8888_SkColorType:
On 2015/07/01 20:26:01, scroggo wrote:
> Don't we need to update the swizzler in order to support both?

Yes, I changed this to only support N32, and will update the swizzler to support
both in a later CL

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:438: fNumberPasses =
png_set_interlace_handling(fPng_ptr);
On 2015/07/01 20:26:01, scroggo wrote:
> I think you mentioned this to me before: png_set_interlace_handling returns 1
> for a non-interlaced image?

Yes, if the image is not interlaced it returns 1, and doesn't turn on the
interlaced flag.

https://codereview.chromium.org/1214203005/diff/20001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:467: //would have excited before now if the
colorType was supported by png
On 2015/07/01 20:26:01, scroggo wrote:
> exited*

Acknowledged.

[scroggo, 2015-07-01 21:08:09]

> Calling minimal transforms in libpng without using transforms for swizzling

> This is the initial changes which fix where the transforms are being set,
> and call any transforms which are necessary.
> The swizzler is still being used as before.

nit: This description says a lot of stuff about what this change is not
(relative to https://codereview.chromium.org/1212373006/). I think it would be
more interesting to state what is accomplished by this CL:

- support kGray correctly
- avoid extra call to png_get_IHDR by storing the bit depth
- call transforms as needed
- fix other bugs around color conversions?

Otherwise, LGTM

[emmaleer, 2015-07-01 21:19:26]

The CQ bit was checked by emmaleer@google.com

[commit-bot: I haz the power, 2015-07-01 21:19:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1214203005/40001

[commit-bot: I haz the power, 2015-07-01 21:20:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-01 21:20:22]

Exceeded global retry quota

[emmaleer, 2015-07-01 21:24:02]

The CQ bit was checked by emmaleer@google.com

[emmaleer, 2015-07-01 21:24:03]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1214203005/#ps60001
(title: "Rebased code to fix merge issue")

[commit-bot: I haz the power, 2015-07-01 21:24:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1214203005/60001

[commit-bot: I haz the power, 2015-07-01 21:24:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-01 21:24:44]

Exceeded global retry quota

[emmaleer, 2015-07-01 21:37:03]

The CQ bit was checked by emmaleer@google.com

[emmaleer, 2015-07-01 21:37:03]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1214203005/#ps80001
(title: "fixing merge conflict")

[commit-bot: I haz the power, 2015-07-01 21:37:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1214203005/80001

[commit-bot: I haz the power, 2015-07-01 21:44:35]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/9693037fd41b7ce545b44beaa3489dcfd915018c

[jvanverth1, 2015-07-02 17:35:14]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1213743004/ by jvanverth@google.com.

The reason for reverting is: DEPS roll failing.

[scroggo, 2015-07-07 13:06:58]

On 2015/07/02 17:35:14, jvanverth1 wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
> https://codereview.chromium.org/1213743004/ by mailto:jvanverth@google.com.
> 
> The reason for reverting is: DEPS roll failing.

Sheriffs, in the future, please provide a link to the failing DEPS roll (or did
I miss the link somewhere?).

It appears that the failure is here: https://codereview.chromium.org/1214943004/

That does not look related to this change. Chrome is not using SkCodec, so I do
not see how it could be related.