The optimization made in Bug 122607 introduced a bug when dealing with array types. As documented in the ANGLEWebKitBridge.cpp file, ANGLE identifies arrays by affixing "[0]" to the name. The new logic incorrectly assumed arrays would always have sizes > 1, and deleted the "[0]" characters at the end of the name. However, there are a number of cases where single-element arrays are used, and the new logic prevented WebKit from finding them in its symbol store. The attached patch corrects the problem so that the following three.js examples work properly: http://threejs.org/examples/#webgl_lights_hemisphere http://threejs.org/examples/#webgl_materials2
<rdar://problem/15394564>
Created attachment 220266 [details] Patch
Attachment 220266 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/html/canvas/WebGLRenderingContext.cpp', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/ChangeLog:9: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 220266 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220266&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3179 > + String arrayBrackets = "[" + String::number(index) + "]"; > + String uniformName = info.name + arrayBrackets; I guess we could do this in one line, although I always forget the optimisation rules for string building.
I think this'll fail if we have a size 1 array, say soo int foo[1] or something, but then try to getUniformLocation of foo[0].
Committed r161242: <http://trac.webkit.org/changeset/161242>
Roger pointed out that we might do the wrong thing now if we try to access the zeroeth element of an array, because we always strip off the "[0]" bracket and only test for "[x]" when we have an array size greater than 1.
Comment on attachment 220266 [details] Patch Attachment 220266 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4513015000989696 New failing tests: webgl/1.0.2/conformance/glsl/misc/shader-with-array-of-structs-containing-arrays.html webgl/1.0.2/conformance/glsl/misc/shader-with-similar-uniform-array-names.html
Created attachment 220270 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 220271 [details] Patch
(In reply to comment #8) > (From update of attachment 220266 [details]) > Attachment 220266 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/4513015000989696 > > New failing tests: > webgl/1.0.2/conformance/glsl/misc/shader-with-array-of-structs-containing-arrays.html > webgl/1.0.2/conformance/glsl/misc/shader-with-similar-uniform-array-names.html This may have been caused by the 0-th element case Roger pointed out.
Created attachment 220274 [details] Patch (with Test Case)
Committed r161247: <http://trac.webkit.org/changeset/161247>
Comment on attachment 220266 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220266&action=review >> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3179 >> + String arrayBrackets = "[" + String::number(index) + "]"; >> + String uniformName = info.name + arrayBrackets; > > I guess we could do this in one line, although I always forget the optimisation rules for string building. This needs to be done in one line. It will do less memory allocation. String uniformName = info.name + "[" + String::number(index) + "]";
Reopen to incorporate Darin's suggestion.
Created attachment 220460 [details] Patch
Committed r161378: <http://trac.webkit.org/changeset/161378>