RESOLVED FIXED Bug 75999
[chromium] Color profiles are incorrect for images without premultiplied alpha
https://bugs.webkit.org/show_bug.cgi?id=75999
Summary [chromium] Color profiles are incorrect for images without premultiplied alpha
Kenneth Russell
Reported 2012-01-10 15:33:35 PST
The test case for http://crbug.com/108237 demonstrates that when a PNG containing an alpha channel is loaded via the Skia backend on Mac OS X with the WebGL setting premultiplyAlpha=false, the resulting colors are incorrect. The problem is in the application of the color space in resolveColorSpace(), ImageDecoderSkia.cpp via CoreGraphics. There are a couple of basic assumptions that the alpha channel is premultiplied, one in SkCreateCGImageRefWithColorspace, and one in the creation of the CGBitmapContext. It doesn't look like there is any way to apply the color space conversion without a premultiplied alpha channel; creation of a CGBitmapContext fails if the flag kCGImageAlphaFirst is passed instead of kCGImageAlphaPremultipliedFirst. Minimally, the code should be updated to skip the application of the color space if the caller has specified separate alpha.
Attachments
Patch (11.16 KB, patch)
2012-01-10 18:17 PST, Kenneth Russell
senorblanco: review+
Kenneth Russell
Comment 1 2012-01-10 18:17:06 PST
Cary Clark
Comment 2 2012-01-11 05:05:19 PST
lgtm
Stephen White
Comment 3 2012-01-11 07:04:52 PST
Comment on attachment 121955 [details] Patch I'll trust Cary's ok on this one. r=me
Cary Clark
Comment 4 2012-01-11 07:26:35 PST
reed brought up a concern when I mentioned this to him this morning. Does this only ignore color profiles when the texture is used by WebGL or does this cause color profiles to additionally be ignored in normal 2D drawing?
Mike Reed
Comment 5 2012-01-11 08:02:04 PST
we ask, because it looks like from the original test page, that the 2D path is drawing correctly as is (i.e. w/ the colorprofile applied).
Kenneth Russell
Comment 6 2012-01-11 11:22:16 PST
(In reply to comment #5) > we ask, because it looks like from the original test page, that the 2D path is drawing correctly as is (i.e. w/ the colorprofile applied). WebGL is the only API that causes the premultiplyAlpha flag to be set to false while decoding images. The browser normally premultiplies the alpha channel. Therefore this change only affects images uploaded as WebGL textures.
Kenneth Russell
Comment 7 2012-01-11 12:17:26 PST
noel gordon
Comment 8 2012-01-16 19:12:41 PST
(In reply to comment #7) > Committed r104732: <http://trac.webkit.org/changeset/104732> This changed behavior for JPEG images too, was that intentional?
Kenneth Russell
Comment 9 2012-01-17 14:12:48 PST
(In reply to comment #8) > (In reply to comment #7) > > Committed r104732: <http://trac.webkit.org/changeset/104732> > > This changed behavior for JPEG images too, was that intentional? Not 100% intentional, but the code path I added also skips the colorspace conversion for PNGs that don't have an alpha channel when WebGL's premultiplyAlpha setting is false. If you have a better suggestion on how to correctly perform the colorspace conversions in more situations, please let me know. Are you seeing behavior changes outside of WebGL usage? Any such change is definitely unintentional; please file a new bug if so.
John Bauman
Comment 10 2012-01-17 14:24:00 PST
You could check ImageFrame::hasAlpha as well in ImageFrame::setStatus, which should work with jpegs and opaque pngs.
Kenneth Russell
Comment 11 2012-01-17 17:11:38 PST
(In reply to comment #10) > You could check ImageFrame::hasAlpha as well in ImageFrame::setStatus, which should work with jpegs and opaque pngs. OK, filed Bug 76498 to fix this.
noel gordon
Comment 12 2012-01-17 22:09:13 PST
(In reply to comment #9) > Are you seeing behavior changes outside of WebGL usage? Any such change is definitely unintentional; please file a new bug if so. Nope. Seems this change is restricted to WebGL per comment #5 (and thank you for adding that comment btw). Outside of WebGL, we have some test coverage for color correction on OS(DARWIN) to keep us honest.
noel gordon
Comment 13 2012-01-17 22:38:06 PST
(In reply to comment #11) > (In reply to comment #10) > > You could check ImageFrame::hasAlpha as well in ImageFrame::setStatus, which should work with jpegs and opaque pngs. > > OK, filed Bug 76498 to fix this. Thanks, John's idea is good. I mentioned JPEG because their premultiplied colors are identical to their unpremultiplied colors because their alpha is 255 everywhere. So I suspected that color correction would still "just work" for JPEG in WebGL even with the issues mentioned in your opening comments.
Note You need to log in before you can comment on or make changes to this bug.