9 years ago
8 years ago


(Reporter: bjacob, Assigned: bjacob)



Firefox Tracking Flags

(Not tracked)



(2 attachments)

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[0]".
    // 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 "[0]".
    // There is no such requirement in the OpenGL (non-ES) spec and indeed we have OpenGL implementations returning
    // "foo" instead of "foo[0]". So, when implementing WebGL on top of desktop OpenGL, we must check if the
    // returned name ends in [0], and if it doesn't, append that.

This allows us to get a bit farther in this test:

The test still fails at line 263,

    var value = gl.getUniform(program, elemLoc);

as getUniform returns NS_ERROR_FAILURE. Need to investigate that separately.
Attachment #457435 - Attachment is patch: true
Attachment #457435 - Attachment mime type: application/octet-stream → text/plain
Attachment #457435 - Flags: review?(vladimir)
Comment on attachment 457435 [details] [diff] [review]
Fix uniform arrays

Won't this fail if, say, "foo[1]" 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[0] so it should be enough to check for the foo case and append [0] in that case.
This new patch
 - now does the [0] 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,

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.
Last Resolved: 9 years ago
Resolution: --- → FIXED
Assignee: nobody → bjacob
You need to log in before you can comment on or make changes to this bug.