Bug 132430

Summary: Fix handling of attributes prior to compiling shader
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebGLAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, dino, fpizlo, kondapallykalyan, mmaxfield, noam, oliver, roger_fong
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 121849    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch dino: review+

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
<rdar://problem/16394921>
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]
Patch
Comment 4 Brent Fulgham 2014-05-01 10:30:49 PDT
Created attachment 230590 [details]
Patch
Comment 5 Dean Jackson 2014-05-01 10:39:46 PDT
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?
Comment 6 Brent Fulgham 2014-05-01 11:57:33 PDT
Created attachment 230600 [details]
Patch
Comment 7 Dean Jackson 2014-05-01 12:01:20 PDT
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.
Comment 8 Brent Fulgham 2014-05-01 12:03:22 PDT
Committed r168112: <http://trac.webkit.org/changeset/168112>