Bug 98470 – [WebGL] [Mac] queried attributes and uniforms need to return the original variable name (not the mapped name)

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

[Dean Jackson]

conformance/glsl/misc/glsl-long-variable-names.html is still failing on Mac, even after r130417. It seems that I didn't update the queried attribute name when recording the mapping.

[Radar WebKit Bug Importer]

<rdar://problem/12439435>

[Roger Fong]

Created attachment 167851 [details]
Patch

[Roger Fong]

Alternative I can add another hash map going the other way instead of searching through the current one (assuming that the mappings are one to one)

[Dean Jackson]

They are 1:1, but I don't think should worry about a reverse map unless this turns out to be a hot path.

[Dean Jackson]

Comment on attachment 167851 [details]
Patch

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

> Source/WebCore/ChangeLog:7
> +        Reviewed by NOBODY (OOPS!).
> +

I usually write a paragraph here saying what I did (although the title is pretty obvious in this case). For example "When a variable name in a shader is long, we translate it into a short form and keep a mapping between the new and old names. However, we were accidentally telling the client-side code the translated name. It should always use the original names"

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:635
> +    String origName = originalSymbolName(program, SHADER_SYMBOL_TYPE_ATTRIBUTE, String(name.get(), nameLength));
> +    
> +    info.name = origName;

Use long names in variables: originalName

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:662
> +    String origName = originalSymbolName(program, SHADER_SYMBOL_TYPE_UNIFORM, String(name.get(), nameLength));
> +    
> +    info.name = origName;

Ditto.

[Tim Horton]

Comment on attachment 167851 [details]
Patch

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

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:712
> +            if (symbolEntry->second.mappedName == name)
> +                return symbolEntry->first;

Didn't we (first, second) -> (key, value)?

[kov's GTK+ EWS bot]

Comment on attachment 167851 [details]
Patch

Attachment 167851 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14250063

[Roger Fong]

Created attachment 167855 [details]
Patch

[Early Warning System Bot]

Comment on attachment 167855 [details]
Patch

Attachment 167855 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14250079

[Tim Horton]

Comment on attachment 167855 [details]
Patch

r- so the bots stop trying

[Roger Fong]

Created attachment 167875 [details]
Patch

[Tim Horton]

Comment on attachment 167875 [details]
Patch

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

Carrying over Dino's r+ from before, assuming it builds now.

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:-660
> -

?

[Roger Fong]

resolved: http://trac.webkit.org/changeset/130985