Fix Equals and serialization for rare pngs

Fix Equals and serialization for rare pngs

PNGs may contain a gAMA chunk that specifies gamma
values as floats.  If so, we will use these floats
to create an SkColorSpace.

This CL fixes Equals(), serialize(), and
Deserialize() to correctly handle SkColorSpaces
with strange gammas, where we are unable to fall
back on the profile data.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221983002

Committed: https://skia.googlesource.com/skia/+/d9015a43cf6c6e7c372c804ee3e1021b153d505f

[msarett, 2016-08-08 15:52:08]

Description was changed from

==========
Fix Equals and serialization for rare pngs

BUG=skia:
==========

to

==========
Fix Equals and serialization for rare pngs

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221983002
==========

[msarett, 2016-08-08 15:55:39]

Description was changed from

==========
Fix Equals and serialization for rare pngs

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221983002
==========

to

==========
Fix Equals and serialization for rare pngs

PNGs may contain a gAMA chunk that specifies gamma
values as floats.  If so, we will use these floats
to create an SkColorSpace.

This CL fixes Equals(), serialize(), and
Deserialize() to correctly handle SkColorSpaces
with strange gammas, where we are unable to fall
back on the profile data.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221983002
==========

[msarett, 2016-08-08 15:56:17]

msarett@google.com changed reviewers:
+ mtklein@google.com

[msarett, 2016-08-22 13:46:38]

msarett@google.com changed reviewers:
+ brianosman@google.com, reed@google.com

[msarett, 2016-08-22 13:46:38]

Please take a look.

[Brian Osman, 2016-08-22 14:00:03]

https://codereview.chromium.org/2221983002/diff/1/src/core/SkColorSpace.cpp
File src/core/SkColorSpace.cpp (right):

https://codereview.chromium.org/2221983002/diff/1/src/core/SkColorSpace.cpp#n...
src/core/SkColorSpace.cpp:347: if (ColorSpaceHeader::kFloatGamma_Flag !=
header.fFlags || length < 15 * sizeof(float))
The first condition on the 'if' is strange - we're in a case block that just
ensured that header.fFlags will always be kFloatGamma.

https://codereview.chromium.org/2221983002/diff/1/tests/ColorSpaceTest.cpp
File tests/ColorSpaceTest.cpp (right):

https://codereview.chromium.org/2221983002/diff/1/tests/ColorSpaceTest.cpp#ne...
tests/ColorSpaceTest.cpp:190: SkColorSpace::Equals(space, newSpace1.get());
Do you want to be checking/asserting the return values here?

[msarett, 2016-08-22 14:10:24]

https://codereview.chromium.org/2221983002/diff/1/src/core/SkColorSpace.cpp
File src/core/SkColorSpace.cpp (right):

https://codereview.chromium.org/2221983002/diff/1/src/core/SkColorSpace.cpp#n...
src/core/SkColorSpace.cpp:347: if (ColorSpaceHeader::kFloatGamma_Flag !=
header.fFlags || length < 15 * sizeof(float))
On 2016/08/22 14:00:02, Brian Osman wrote:
> The first condition on the 'if' is strange - we're in a case block that just
> ensured that header.fFlags will always be kFloatGamma.

Yeah thanks...  Removing that condition.

https://codereview.chromium.org/2221983002/diff/1/tests/ColorSpaceTest.cpp
File tests/ColorSpaceTest.cpp (right):

https://codereview.chromium.org/2221983002/diff/1/tests/ColorSpaceTest.cpp#ne...
tests/ColorSpaceTest.cpp:190: SkColorSpace::Equals(space, newSpace1.get());
On 2016/08/22 14:00:02, Brian Osman wrote:
> Do you want to be checking/asserting the return values here?

Yes good catch.

[Brian Osman, 2016-08-22 18:32:49]

lgtm

[msarett, 2016-08-22 18:33:15]

The CQ bit was checked by msarett@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-22 18:33:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-22 19:12:26]

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

[commit-bot: I haz the power, 2016-08-22 19:12:27]

Dry run: This issue passed the CQ dry run.

[msarett, 2016-08-22 19:28:32]

The CQ bit was checked by msarett@google.com

[commit-bot: I haz the power, 2016-08-22 19:28:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-22 19:29:38]

Description was changed from

==========
Fix Equals and serialization for rare pngs

PNGs may contain a gAMA chunk that specifies gamma
values as floats.  If so, we will use these floats
to create an SkColorSpace.

This CL fixes Equals(), serialize(), and
Deserialize() to correctly handle SkColorSpaces
with strange gammas, where we are unable to fall
back on the profile data.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221983002
==========

to

==========
Fix Equals and serialization for rare pngs

PNGs may contain a gAMA chunk that specifies gamma
values as floats.  If so, we will use these floats
to create an SkColorSpace.

This CL fixes Equals(), serialize(), and
Deserialize() to correctly handle SkColorSpaces
with strange gammas, where we are unable to fall
back on the profile data.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221983002

Committed:
https://skia.googlesource.com/skia/+/d9015a43cf6c6e7c372c804ee3e1021b153d505f
==========

[commit-bot: I haz the power, 2016-08-22 19:29:40]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/d9015a43cf6c6e7c372c804ee3e1021b153d505f