Hi, this is mostly a heads up for now. The best way to fix is still open and debated. Summary: * libwebp tree was updated in chromium to 0.2.0 with alpha support: http://codereview.chromium.org/10832153/ * WEBPImageDecoder.cpp now needs an update to correct handle compressed image with alpha we're setting "buffer.setHasAlpha(false);" always, at line 122 Rendering of webp-with-alpha is 'weird', as if values rgb values were not pre-multiplied. * But even when setting "setHasAlpha(true)" (when there's alpha channel only), we need to address the pre-multiplication of rgb values (cf m_premultiplyAlpha and line 79). * libwebp can now emit premultiplied values, using MODE_bgrA instead of MODE_BGRA as outputMode() i'll poke around the code further and try to get a patch, but any insight from Noel or others is welcome!
Created attachment 157103 [details] example webp file Lossless-coded original WebP file.
Created attachment 157106 [details] expected decoded result expected decoded result (decoded with 'dwebp' tool)
Created attachment 157108 [details] badly decoded result screen capture from chrome 22.0.1229.0 dev Looks like rgb values are not alpha-multiplied
Created attachment 157474 [details] Patch
Comment on attachment 157474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157474&action=review > Source/WebCore/platform/image-decoders/webp/WEBPImageDecoder.cpp:59 > + , m_decoder(0), m_hasAlpha(false) This should be split on two lines.
Created attachment 157491 [details] Patch
Created attachment 157678 [details] Patch
New patch with Layout test update and new files for decoding tests
Comment on attachment 157678 [details] Patch Attachment 157678 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13462770 New failing tests: fast/images/webp-image-decoding.html
Created attachment 157692 [details] Archive of layout-test-results from gce-cr-linux-06 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-06 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
chromium-ews was unable to decode the new images and displayed the broken-image icon instead (with size 20x20 instead of expected 64x64). I'm not sure why the decoding is broken...
(In reply to comment #11) > chromium-ews was unable to decode the new images and displayed the broken-image icon instead (with size 20x20 instead of expected 64x64). I'm not sure why the decoding is broken... This same behavior will be seen if the resources (test2.webp, test3.webp) are missing. This works in a local build for me, not sure if it has something to do with how the chromium build applies the patch and whether those files are missing there, though. Is there a more detailed build log that shows the patch being applied?
(In reply to comment #12) Needed a chromium DEPS roll.
(In reply to comment #0) > i'll poke around the code further and try to get a patch, but any insight from Noel or others is welcome! Noted this part ... > * But even when setting "setHasAlpha(true)" (when there's alpha channel only), we need to > address the pre-multiplication of rgb values (cf m_premultiplyAlpha and line 79). and wondered if you intend to handle m_premultiplyAlpha for the alpha supporting formats? If so, file a bug about it and move on with this patch which otherwise looks fine to me.
Created attachment 158901 [details] Patch
(In reply to comment #15) > Created an attachment (id=158901) [details] > Patch re-uploaded the patch. It's likely that the updated DEPS will make the chrome-ews test pass this time.
(In reply to comment #14) > (In reply to comment #0) > > > i'll poke around the code further and try to get a patch, but any insight from Noel or others is welcome! > > Noted this part ... > > > * But even when setting "setHasAlpha(true)" (when there's alpha channel only), we need to > > address the pre-multiplication of rgb values (cf m_premultiplyAlpha and line 79). > > and wondered if you intend to handle m_premultiplyAlpha for the alpha supporting formats? If so, file a bug about it and move on with this patch which otherwise looks fine to me. I should mention that browsers (moz, webkit, chrome) in general want a decoded row callback. This to control the decoded frame buffer color correction, alpha pre-multiplication (m_premultiplyAlpha = true|false), and the numerical stability when the frame buffer is used with other web content (the <canvas> element in particular). Perhaps a row callback is planned for a future version of libwebp.
Comment on attachment 158901 [details] Patch Clearing flags on attachment: 158901 Committed r125869: <http://trac.webkit.org/changeset/125869>
All reviewed patches have been landed. Closing bug.