Bug 41383 – Need to validate the size of the incoming arrays for uniform* functions

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

[Zhenyao Mo]

either Float32Arrays or (float*, int size) argument pairs.

[Zhenyao Mo]

Created attachment 60191 [details]
patch

[Kenneth Russell]

Gregg and I talked about this at length. The GL ES spec has this to say if you pass in too much data when setting elements in a uniform array (section 2.10, "Vertex Shaders", pp.36-37):

"When loading N elements starting at an arbitrary position k in a uniform declared as an array, elements k through k + N − 1 in the array will be replaced with the new values. Values for any array element that exceeds the highest array element index used, as reported by GetActiveUniform, will be ignored by the GL."

I think the only requirement should be that the incoming data (int*, Int32Array, float*, Float32Array) have at least one element of the given size (1, 2, 3, 4 for uniform[1234][fi]v, 1, 4, 9, 16 for uniformMatrix[1234]fv). I think the requirement that the number of elements in the array be a multiple of the size of the uniform should be dropped. The tests should then verify that passing in too little data raises INVALID_VALUE.

Sorry about the confusion.

[Zhenyao Mo]

(In reply to comment #2)
> Gregg and I talked about this at length. The GL ES spec has this to say if you pass in too much data when setting elements in a uniform array (section 2.10, "Vertex Shaders", pp.36-37):
> 
> "When loading N elements starting at an arbitrary position k in a uniform declared as an array, elements k through k + N − 1 in the array will be replaced with the new values. Values for any array element that exceeds the highest array element index used, as reported by GetActiveUniform, will be ignored by the GL."
> 
> I think the only requirement should be that the incoming data (int*, Int32Array, float*, Float32Array) have at least one element of the given size (1, 2, 3, 4 for uniform[1234][fi]v, 1, 4, 9, 16 for uniformMatrix[1234]fv). I think the requirement that the number of elements in the array be a multiple of the size of the uniform should be dropped. The tests should then verify that passing in too little data raises INVALID_VALUE.
> 
> Sorry about the confusion.

No problem.  I'll revise.

[Zhenyao Mo]

Created attachment 60305 [details]
revised patch after kbr's clarification

[Kenneth Russell]

Comment on attachment 60305 [details]
revised patch after kbr's clarification

Code looks good. A couple of small typos in the test, not really necessary to fix.


LayoutTests/fast/canvas/webgl/gl-uniform-arrays.html:165
 +      srcValues: [16, 15, 14, 13, 12, 11, 10, 11, 9],
11, 10, 11, 9 : did you mean to write 11, 10, 9, 8?


LayoutTests/fast/canvas/webgl/gl-uniform-arrays.html:198
 +      srcValues: [16, 15, 14, 13, 12, 11, 10, 11, 9, 8, 7, 6],
11, 10, 11, 9, 8, 7, 6: did you mean 11, 10, 9, 8, 7, 6, 5?

[Zhenyao Mo]

Created attachment 60317 [details]
revised patch: fixed the typo in the test.

[Kenneth Russell]

Comment on attachment 60317 [details]
revised patch: fixed the typo in the test.

Looks good.

[Dimitri Glazkov (Google)]

Comment on attachment 60317 [details]
revised patch: fixed the typo in the test.

r=me.

[WebKit Commit Bot]

Comment on attachment 60317 [details]
revised patch: fixed the typo in the test.

Clearing flags on attachment: 60317

Committed r62384: <http://trac.webkit.org/changeset/62384>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.