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.
Created attachment 121955 [details] Patch
lgtm
Comment on attachment 121955 [details] Patch I'll trust Cary's ok on this one. r=me
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?
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).
(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.
Committed r104732: <http://trac.webkit.org/changeset/104732>
(In reply to comment #7) > Committed r104732: <http://trac.webkit.org/changeset/104732> This changed behavior for JPEG images too, was that intentional?
(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.
You could check ImageFrame::hasAlpha as well in ImageFrame::setStatus, which should work with jpegs and opaque pngs.
(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.
(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.
(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.