Bug 40319 – Implement format conversions in texImage2D and texSubImage2D taking HTML data

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

Per recent updates to the WebGL spec, texImage2D and texSubImage2D now take format and type arguments which imply that the source data is converted to the format specified by these arguments before transferring to the GL. These format conversions need to be implemented.

This bug depends on 40318.

[Kenneth Russell]

Created attachment 59448 [details]
Patch

From the ChangeLog:

Generalized code supporting premultiplication of alpha and vertical flip to pack texture data into requested format and type. Handled incoming image data of various formats, RGBA and BGRA in particular, both to reduce the number of temporary copies during texture upload and to support premultiplying alpha for the texImage2D and texSubImage2D entry points taking ArrayBufferView in a subsequent bug. Added test case exercising all combinations of format/type combinations, premultiplication of alpha, and Image/ImageData upload. (Incorporated pnglib.js under fast/canvas/webgl/resources/ to be able to generate Image elements programmatically.) Tested in Safari on Mac OS X and in Chromium on Mac OS X, Windows and Linux.

Note to reviewers:

This is a large patch, but it is the smallest it can possibly be and still implement a self-contained unit of functionality. The test case is exhaustive and should exercise all of the new code paths, some of which have also been verified manually by modifiying some existing WebGL demos.

[WebKit Review Bot]

Attachment 59448 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/html/canvas/WebGLRenderingContext.cpp:3403:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/html/canvas/WebGLRenderingContext.cpp:3407:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kenneth Russell]

Created attachment 59449 [details]
Revised patch

Revised patch fixing style errors

[Dimitri Glazkov (Google)]

Comment on attachment 59449 [details]
Revised patch

Some comments from over-the-shoulder review:

WebCore/platform/graphics/GraphicsContext3D.cpp:459
 +          if (destinationFormat == RGBA && alphaOp == kAlphaDoNothing) {
must check sourceDataFormat == kSourceFormatRGBA8!

WebCore/platform/graphics/GraphicsContext3D.cpp:479
 +              switch (alphaOp) {
add case for kAlphaDoNothing in case sourceDataFormat is not RGBA8

LayoutTests/fast/canvas/webgl/tex-image-with-format-and-type.html:97
 +                  useImageData: (useImageData ? true : false),
Can just do !!useImageData

[Kenneth Russell]

Created attachment 59571 [details]
Revised patch

Addressed code review feedback.

Updated pnglib.js to latest version, which is now BSD licensed.

[Dimitri Glazkov (Google)]

Comment on attachment 59571 [details]
Revised patch

r=me.

[Kenneth Russell]

Committed r61715: <http://trac.webkit.org/changeset/61715>