Bug 132430 – Fix handling of attributes prior to compiling shader

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Brent Fulgham]

Some WebGL sites broke after ANGLE symbol name hashing was activated under Bug 121849. This is because we were not properly honoring the expected behavior of bound attributes (from the Khronos documentation): "glBindAttribLocation can be called before any vertex shader objects are bound to the specified program object. It is also permissible to bind a generic attribute index to an attribute variable name that is never used in a vertex shader."

WebGL programs that called bindAttribLocations prior to compiling shader sources would perform the bind using the non-hashed symbol name, but would later create the attributes as hashed names. Consequently, the program would refer to attributes that were never actually part of any shader, resulting in some amazing display artifacts.

This patch adds a dictionary of hashed symbol names so that we can tell the WebGL program the proper name that will be used when the shader is eventually compiled, allowing the WebGL program to link against the proper symbol after compiling and linking completes.

[Brent Fulgham]

<rdar://problem/16394921>

[Brent Fulgham]

It might be more correct to partition the hash by program, but since the hash is deterministic (the same input name will always hash to the same output, as long as the hash table is not cleared), it's probably not worth the extra complexity. This will also reduce the number of hashes we have to compute, since each symbol is only hashed once, instead of once for each program.

[Brent Fulgham]

Created attachment 230588 [details]
Patch

[Brent Fulgham]

Created attachment 230590 [details]
Patch

[Dean Jackson]

Comment on attachment 230590 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230590&action=review

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:807
> -    Platform3DObject shaders[2];
> +    Platform3DObject shaders[2] = { 0 };

I guess this could be just { } right?

[Brent Fulgham]

Created attachment 230600 [details]
Patch

[Dean Jackson]

Comment on attachment 230600 [details]
Patch

Nit. You're using two space indent in the JS, but I expect you simply copied an existing test.

[Brent Fulgham]

Committed r168112: <http://trac.webkit.org/changeset/168112>