Bug 132430 - Fix handling of attributes prior to compiling shader
Summary: Fix handling of attributes prior to compiling shader
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brent Fulgham
Keywords: InRadar
Depends on: 121849
  Show dependency treegraph
Reported: 2014-05-01 10:08 PDT by Brent Fulgham
Modified: 2014-05-01 12:03 PDT (History)
10 users (show)

See Also:

Patch (5.52 KB, patch)
2014-05-01 10:16 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (7.66 KB, patch)
2014-05-01 10:30 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (15.92 KB, patch)
2014-05-01 11:57 PDT, Brent Fulgham
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-05-01 10:08:02 PDT
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.
Comment 1 Brent Fulgham 2014-05-01 10:08:43 PDT
Comment 2 Brent Fulgham 2014-05-01 10:10:36 PDT
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.
Comment 3 Brent Fulgham 2014-05-01 10:16:58 PDT
Created attachment 230588 [details]
Comment 4 Brent Fulgham 2014-05-01 10:30:49 PDT
Created attachment 230590 [details]
Comment 5 Dean Jackson 2014-05-01 10:39:46 PDT
Comment on attachment 230590 [details]

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?
Comment 6 Brent Fulgham 2014-05-01 11:57:33 PDT
Created attachment 230600 [details]
Comment 7 Dean Jackson 2014-05-01 12:01:20 PDT
Comment on attachment 230600 [details]

Nit. You're using two space indent in the JS, but I expect you simply copied an existing test.
Comment 8 Brent Fulgham 2014-05-01 12:03:22 PDT
Committed r168112: <http://trac.webkit.org/changeset/168112>