[webkit-reviews] review granted: [Bug 105821] Optimize the texture packing for texImage2D() and texSubImage2D() in WebGL : [Attachment 182953] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Jun Jiang
<jun.a.jiang at intel.com>'s request for review:
Bug 105821: Optimize the texture packing for texImage2D() and texSubImage2D()
in WebGL
https://bugs.webkit.org/show_bug.cgi?id=105821

Attachment 182953: Patch
https://bugs.webkit.org/attachment.cgi?id=182953&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=182953&action=review


Thanks for persisting with this patch. It gets much closer to unifying this
code again with Mozilla's, which will make it easier to share code in the
future. Two tiny nits, but neither is worth continuing to delay your patch.
r=me

> Source/WebCore/platform/graphics/GraphicsContext3D.cpp:1239
> +	   || format == GraphicsContext3D::DataFormatA16Little

Please see the question in the previous review regarding these 16-bit per
channel formats. Since they are exposed here only for the Core Graphics port,
and they don't seem to be useful to represent any existing high dynamic range
file format, I think we should hide their use inside CG-specific code and
normalize incoming images to 8 bits per channel if necessary. Do you agree? If
so please file another bug.

> Source/WebCore/platform/graphics/GraphicsContext3D.cpp:1434
> +    OwnArrayPtr<uint8_t> unpackedIntermediateSrcData;

Should have been m_unpackedIntermediateSrcData. Not a huge deal.