Bug 38709 – getActiveUniform must ensure names of arrays end in "[0]"

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

In order to match OpenGL ES 2.0 semantics, getActiveUniform must ensure that if the name of an array uniform is fetched, the suffix "[0]" is present. Desktop GL does not enforce this rule. See section 2.10.4 "Shader Variables" in the ES spec version 2.0.24 (p. 34, GetActiveUniform), and compare to section 2.15.3 "Shader Variables" in the OpenGL 2.1 spec version dated 2006-12-01 (p. 80).

[Zhenyao Mo]

Created attachment 58414 [details]
patch

[Kenneth Russell]

Comment on attachment 58414 [details]
patch

The changes generally look good, in particular the strengthening of the test case. We should consider using bindAttribLocation to avoid some of the swapping logic, but that isn't necessary to do now. A couple of comments:

WebCore/html/canvas/WebGLRenderingContext.cpp:966
 +          }
I know this is how Chromium's command buffer version is written, but here it would be much more concisely expressed as:

if (info.size > 1 && !info.name.endsWith("[0]"))
    info.name.append("[0]");


LayoutTests/fast/canvas/webgl/resources/intArrayUniformShader.vert:7
 +                  + ival2[0] + ival2[1];
The formatting is unnecessarily weird here.

[Zhenyao Mo]

Created attachment 58429 [details]
revised patch: responding to kbr's review

[Kenneth Russell]

Comment on attachment 58429 [details]
revised patch: responding to kbr's review

Looks good. Assuming you re-ran the test after the change to the string manipulation code.

[Zhenyao Mo]

(In reply to comment #4)
> (From update of attachment 58429 [details])
> Looks good. Assuming you re-ran the test after the change to the string manipulation code.

Yes I did.  However, my Mac GL driver returns with "[0]", so the code path is not covered by the test on my Mac.

[Dimitri Glazkov (Google)]

Comment on attachment 58429 [details]
revised patch: responding to kbr's review

Weeeeird. But ok.

[WebKit Commit Bot]

Comment on attachment 58429 [details]
revised patch: responding to kbr's review

Clearing flags on attachment: 58429

Committed r61020: <http://trac.webkit.org/changeset/61020>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.