Bug 52068 – Need to consider UNPACK_ALIGNMENT in GraphicsContext3D::texImage2DResourceSafe

• Component: WebGL
• Status: RESOLVED FIXED

[Zhenyao Mo]

Need to UNPACK_ALIGNMENT to 1 before calling texImage2D, then restore the original value.

[Zhenyao Mo]

Created attachment 78250 [details]
Patch

[Kenneth Russell]

Comment on attachment 78250 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78250&action=review

> WebCore/platform/graphics/GraphicsContext3D.cpp:138
> +        getIntegerv(GraphicsContext3D::UNPACK_ALIGNMENT, &unpackAlignment);

This query will kill performance of texture uploads. Instead, change the function signature and all callers to pass in the current unpack alignment, or track the unpack alignment in the GraphicsContext3D implementation.

[Zhenyao Mo]

Created attachment 78443 [details]
Patch

refactor the code, adding a new function computeImageSizeInBytes and use it whenever we can

[Kenneth Russell]

Comment on attachment 78443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78443&action=review

The code changes look good; however, there's a bug in the test case pulled down from Khronos that needs to be fixed.

> LayoutTests/fast/canvas/webgl/read-pixels-test-expected.txt:1
> +CONSOLE MESSAGE: line 114: ReferenceError: Can't find variable: INVALID_OPERATION

There's a bug in the test case; the initialization of the various bad tests isn't picking up the enum values through the WebGLRenderingContext object. An exception is being thrown, and the test isn't completing successfully.

[Zhenyao Mo]

Created attachment 78551 [details]
Patch

[Zhenyao Mo]

(In reply to comment #5)
> Created an attachment (id=78551) [details]
> Patch

I fixed the test on the khronos side.  This patch is in sync with khronos.

Sorry Adrienne, I copied finishTest() to webgl-test.html for this patch.  It might add to the mess you try to clean up.

[Adrienne Walker]

(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=78551) [details] [details]
> > Patch
> 
> I fixed the test on the khronos side.  This patch is in sync with khronos.
> 
> Sorry Adrienne, I copied finishTest() to webgl-test.html for this patch.  It might add to the mess you try to clean up.

Thanks for the heads up.  I needed that function for the video test too, which is why I originally started looking into the script cleanup.  It's a good short term solution; we can clean it up with the other changes later.

[Kenneth Russell]

Comment on attachment 78551 [details]
Patch

Looks good. Nice work.

[Zhenyao Mo]

Committed r75524: <http://trac.webkit.org/changeset/75524>

[WebKit Review Bot]

http://trac.webkit.org/changeset/75524 might have broken SnowLeopard Intel Release (Tests)
The following tests are not passing:
fast/canvas/webgl/read-pixels-test.html