1255106 - Add a SurfaceFilter for color space conversion

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P3)

Description

[Seth Fowler [:seth] [:s2h]]

The PNG and JPEG decoders transform data in place to implement color space conversion with QCMS. This is exactly the type of thing that should be implemented as a SurfaceFilter.

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

We actually do color management on the native sized image. It would be good if we write such a filter, and place it to the end of the filter chain, so that we color manage fewer pixels. I've put together patches which does this, as well as brings nsJPEGDecoder into the surface pipe fold. Theoretically this brings us closer to color managing everything on non-mobile, if it really does lower the decode time. I still need to assess the performance delta (especially since we have seen regressions with SurfacePipe before with the GIF decoder). My current work probably does regress grayscale JPEGs (surface pipe works with BGRAm not grayscale), but I'm not convinced I should care about them too much relative to ones with color.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 1. Add qcms support for converting gray to RBGA.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 1. Move color transform state to image decoder base class.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 2. Implement color management filter for SurfacePipe.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 3. Use color management filter with decoders using SurfacePipe.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 4. Ensure we don't apply CMS transforms in cases we missed.

GIF images should not apply CMS transforms if disabled by caller via
SurfaceFlags::NO_COLORSPACE_CONVERSION. WebP images should reject any
non-RGB ICC profiles, not just grayscale profiles.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbc7bc0a46511ecdbba2a34cdcf4235c71862253

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Whoops, I inverted nsPNGDecoder's mPipeUseTransform.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=470fe9fae2045a368e5ae217c1095fbe164de90f

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

I made some small changes to part 4 to use the color management filter even for interlaced PNG images. This seems to fix most/all of the failing reftests -- the interlacing filter is a somewhat complicated filter, so it would be best to color manage afterwards.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=06a5fc5bf49032345539d846f31292f0b3181895

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 1. Move color transform state to image decoder base class.

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 2. Implement color management filter for SurfacePipe.

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 3. Use color management filter with decoders using SurfacePipe.

Comment 13

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1255106 - Part 4. Ensure we don't apply CMS transforms in cases we missed.

GIF images should not apply CMS transforms if disabled by caller via
SurfaceFlags::NO_COLORSPACE_CONVERSION. WebP images should reject any
non-RGB ICC profiles, not just grayscale profiles.

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

prelanding try since it has been a while: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfae2c0d43bb59e1a87f261f86f0a5954c54720c

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/524f61bca438
Part 1. Move color transform state to image decoder base class. r=tnikkel
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a7b422645b7
Part 2. Implement color management filter for SurfacePipe. r=tnikkel
https://hg.mozilla.org/integration/mozilla-inbound/rev/85eb83a6f313
Part 3. Use color management filter with decoders using SurfacePipe. r=tnikkel
https://hg.mozilla.org/integration/mozilla-inbound/rev/aed0c5da0ce0
Part 4. Ensure we don't apply CMS transforms in cases we missed. r=tnikkel

Comment 16

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/524f61bca438
https://hg.mozilla.org/mozilla-central/rev/3a7b422645b7
https://hg.mozilla.org/mozilla-central/rev/85eb83a6f313
https://hg.mozilla.org/mozilla-central/rev/aed0c5da0ce0