For example: Given the GLSL: uniform float array[10]; In JS, the following fails: gl.getUniformLocation(program, "array"); The following succeeds: gl.getUniformLocation(program, "array[0]"); gl.getUniformLocation(program, "array[1]"); ... gl.getUniformLocation(program, "array[9]"); According to the glGetUniformLocation man page [1], the former should succeed: """ Except if the last part of name indicates a uniform variable array, the location of the first element of an array can be retrieved by using the name of the array, or by using the name appended by "[0]". """ [1]: http://www.khronos.org/opengles/sdk/docs/man/xhtml/glGetUniformLocation.xml
Created attachment 169650 [details] Patch For this patch's test, I've made to following change to Khronos WebGL conformance test suite: https://github.com/KhronosGroup/WebGL/commit/6451bfd138b2f603b667cbf7839885f665607c7a Do we need a WebKit test for this or is the WebGL conformance test sufficient?
Hmm based on this patch it looks like just using the array name with no subscripts was probably broken before I landed r131105. Sorry I missed that :/ but at least I didn't break it! woo? Also I think that the single size array does work properly, if it's size 1 then it'll just hit the 'else' path which behaves properly (although that probably wasn't the clearest way of doing things...). Hmm, o wait, I guess that in order to fix the first problem, the distinction between 'array' and 'not array' has to be made regardless. Anyways, looks good to me, although I don't have review so we'll have to wait for dino. Also hi!
(In reply to comment #1) > Do we need a WebKit test for this or is the WebGL conformance test sufficient? I wanna say just the conformance test, but Dean will know better.
(In reply to comment #2) > Hmm based on this patch it looks like just using the array name with no subscripts was probably broken before I landed r131105. Sorry I missed that :/ but at least I didn't break it! woo? No worries- this was broken before you touched it :) > > Also I think that the single size array does work properly, if it's size 1 then it'll just hit the 'else' path which behaves properly (although that probably wasn't the clearest way of doing things...). Yup, that worked properly before. I think it's just more explicit now. > > Hmm, o wait, I guess that in order to fix the first problem, the distinction between 'array' and 'not array' has to be made regardless. > > Anyways, looks good to me, although I don't have review so we'll have to wait for dino. Thanks for looking over it. I'll ask Dino for a review. > > Also hi! Hi! Nice to see you working on WebKit :) (In reply to comment #3) > (In reply to comment #1) > > Do we need a WebKit test for this or is the WebGL conformance test sufficient? > > I wanna say just the conformance test, but Dean will know better. Ok, will do.
(In reply to comment #1) > Created an attachment (id=169650) [details] > Patch > > For this patch's test, I've made to following change to Khronos WebGL conformance test suite: https://github.com/KhronosGroup/WebGL/commit/6451bfd138b2f603b667cbf7839885f665607c7a > > Do we need a WebKit test for this or is the WebGL conformance test sufficient? I have been submitting patches to WebKit that reference the Khronos suite. There is an open bug to merge that into WebKit. Once that's done we'll be in a great position for testing regressions.
Thanks for the review! (In reply to comment #5) > (In reply to comment #1) > > Created an attachment (id=169650) [details] [details] > > Patch > > > > For this patch's test, I've made to following change to Khronos WebGL conformance test suite: https://github.com/KhronosGroup/WebGL/commit/6451bfd138b2f603b667cbf7839885f665607c7a > > > > Do we need a WebKit test for this or is the WebGL conformance test sufficient? > > I have been submitting patches to WebKit that reference the Khronos suite. There is an open bug to merge that into WebKit. Once that's done we'll be in a great position for testing regressions. Ok- I'm looking forward to it!
Comment on attachment 169650 [details] Patch Clearing flags on attachment: 169650 Committed r131952: <http://trac.webkit.org/changeset/131952>
All reviewed patches have been landed. Closing bug.