[webkit-reviews] review granted: [Bug 38282] Passing premultiplyAlpha=false to tex{Sub}Image2D loses information (skia) : [Attachment 65433] revised patch: an attempt to fix the Qt port

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Zhenyao Mo <zmo at google.com>'s
request for review:
Bug 38282: Passing premultiplyAlpha=false to tex{Sub}Image2D loses information
(skia)
https://bugs.webkit.org/show_bug.cgi?id=38282

Attachment 65433: revised patch: an attempt to fix the Qt port
https://bugs.webkit.org/attachment.cgi?id=65433&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
Generally looks good. A few comments; please address before committing.

WebCore/platform/graphics/cg/ImageSourceCG.cpp:68
 +	, m_premultiplyAlpha(premultiplyAlpha)
Should add a FIXME indicating this isn't obeyed yet.

WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp: 
 +	if (skiaConfig != SkBitmap::kARGB_8888_Config)
I think it would be a good idea to retain some sort of this test of the
configuration rather than just the assertion about pixels->rowBytes() below.

WebCore/platform/image-decoders/ImageDecoder.h:158
 +		if (!a && m_premultiplyAlpha)
It may be clearer here and below if m_premultiplyAlpha is tested first; i.e.,
"if (m_premultiplyAlpha && !a)".

WebCore/platform/image-decoders/ImageDecoder.h:197
 +				  // Whether to premultiply alpha to R, G, B
channels;
alpha to -> alpha into

LayoutTests/fast/canvas/webgl/gl-teximage-expected.txt:1
 +  CONSOLE MESSAGE: line 260: ReferenceError: Can't find variable:
notifyFinishedToHarness
This looks bad. Please investigate why this is being reported and fix.


LayoutTests/platform/mac/Skipped:297
 +  fast/canvas/webgl/gl-teximage.html
We should consider either adding this to mac-snowleopard or moving the skipped
entries from that directory to this one. Doesn't need to be fixed right now.