Bug 41884 – Implement bufferData and bufferSubData with ArrayBuffer as input

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Zhenyao Mo]

They are in the spec, but not implemented.

[Zhenyao Mo]

Created attachment 61173 [details]
patch

[Kenneth Russell]

Comment on attachment 61173 [details]
patch

The logic looks good. The test is incorrectly named -- it should be buffer-data-array-buffer.html. A few specific comments.

WebCore/html/canvas/WebGLBuffer.cpp:104
 +  }
While reviewing this, I realized there's a preexisting bug in associateBufferData(ArrayBufferView*). Let's commit this change and fix the other bug under https://bugs.webkit.org/show_bug.cgi?id=42124 , and refactor this code under the other bug.


WebCore/html/canvas/WebGLBuffer.cpp:167
 +  
Let's refactor this code under https://bugs.webkit.org/show_bug.cgi?id=42124 .


WebCore/html/canvas/WebGLRenderingContext.cpp:391
 +  }
While reviewing this I realized there's a preexisting bug in bufferData and bufferSubData taking ArrayBufferView. Let's check this code in as is to maintain parity between the implementations, and fix both under https://bugs.webkit.org/show_bug.cgi?id=42125 .


LayoutTests/fast/canvas/webgl/buffer-data-array-view.html:12
 +  description("Test bufferData/bufferSubData with ArrayView input");
ArrayView -> ArrayBuffer

[Zhenyao Mo]

Created attachment 61315 [details]
revised patch

Changed the test name.

Also, add usage enum check (since the patch for checking this in other bufferData() entries has already landed).

[Kenneth Russell]

Comment on attachment 61315 [details]
revised patch

Looks good to me. One additional comment.


WebCore/html/canvas/WebGLRenderingContext.cpp:379
 +          if (!validateBufferDataUsage(usage))
This needs to be done in all cases. It's inappropriate to do the associateBufferData call if the later bufferData call will fail because of an INVALID_ENUM error. This fix can be done in a subsequent bug for this entry point as well as the one taking ArrayBufferView*.

[Zhenyao Mo]

Committed r63224: <http://trac.webkit.org/changeset/63224>