Created attachment 457435 [details] [diff] [review] Fix uniform arrays This patch does two things. It replaces some INVALID_VALUE by INVALID_OPERATION and does as explained in this comment: // OpenGL ES 2.0 specifies that if foo is a uniform array, GetActiveUniform returns its name as "foo". // See section 2.10 page 35 in the OpenGL ES 2.0.24 specification: // // > If the active uniform is an array, the uniform name returned in name will always // > be the name of the uniform array appended with "". // // There is no such requirement in the OpenGL (non-ES) spec and indeed we have OpenGL implementations returning // "foo" instead of "foo". So, when implementing WebGL on top of desktop OpenGL, we must check if the // returned name ends in , and if it doesn't, append that. This allows us to get a bit farther in this test: https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/gl-uniform-arrays.html The test still fails at line 263, var value = gl.getUniform(program, elemLoc); as getUniform returns NS_ERROR_FAILURE. Need to investigate that separately.
Comment on attachment 457435 [details] [diff] [review] Fix uniform arrays Won't this fail if, say, "foo" is returned? Though maybe that can't happen, I see "Only one active uniform variable will be reported for a uniform array." in the sdk man page at least. Let's see what happens... Also, any reason to not just unconditionally do this, instead of just on desktop GL?
Attachment #457435 - Flags: review?(vladimir) → review+
No very big reason not to do this on GLES, it's just that if the GLES implementation is correct, this is not needed. The cost of doing this is a little conditional branching, a little more executable code, 3 more bytes allocated per name. But, I can agree that's still worth it as this tricky difference between the GL and GLES spec could easily be overlooked by GLES implementers. So I'll do that. As far as I understand, in case of an array, the only things that GL can return are foo or foo so it should be enough to check for the foo case and append  in that case.
Created attachment 457645 [details] [diff] [review] Fix uniform arrays, improved This new patch - now does the  check unconditionally, which will help with buggy GLES implementatoins as discussed above - fixes the above-mentioned failure in GetUniform. It was quite tricky. Remember how we loop over active uniforms in GetUniform. The problem was that in the case of a uniform array, we needed to loop over the individual uniform components of that array.
Attachment #457645 - Flags: review?(vladimir)
note: with this new patch, we now almost completely pass the above-mentioned test, https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/gl-uniform-arrays.html The remaining failure is something I'll fix in a separate bug, basically we forget to validate the size of buffers passed to UniformXxx() functions.
Attachment #457645 - Flags: review?(vladimir) → review+
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.