The default bug view has changed. See this FAQ.

WebGL Water demo broken on Windows by long identifier mapping, because of double underscores

VERIFIED FIXED in Firefox 14

Status

()

Core
Canvas: WebGL
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

unspecified
mozilla15
All
Windows 7
Points:
---

Firefox Tracking Flags

(firefox12 unaffected, firefox13 unaffected, firefox14 verified, firefox15 verified)

Details

(Whiteboard: webgl-conformance[qa!])

Attachments

(1 attachment, 1 obsolete attachment)

Test case: enable Web Console with JS warnings, and go to http://madebyevan.com/webgl-water/

Expected results: works

Actual: [15:13:12.398] uncaught exception: compile error: ERROR: 0:7: 'webgl__gl_Model_bbba1bfc05b59690' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords 

The problem is that long identifier mapping prepends identifiers with "webgl_" so "_foo" becomes "webgl__foo".

I believed that this actually regressed with the latest ANGLE update, but the real bug here comes from the long identifier mapping, bug 676071.
(Assignee)

Comment 1

5 years ago
Created attachment 617644 [details] [diff] [review]
fix double underscore bug in long id mapping
Attachment #617644 - Flags: review?(jgilbert)
(Assignee)

Updated

5 years ago
Whiteboard: webgl-conformance webgl-test-needed
Does this need to land on any branches?
(Assignee)

Comment 3

5 years ago
This needs to land wherever the WebGL Water demo is broken. My guess is that it's only been broken since the last ANGLE update, which means it'll have to be backported to Aurora 14. But it could also be needed on Beta 13 if it's been regressed ever since long id mapping landed.
Attachment #617644 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 617855 [details] [diff] [review]
better: also handle the case of underscore at position 8

The problem is that we take the 9 first chars of the identifier, and build a shortened identifier like

webgl_$(nine_first_chars)_$(hash_value)

so when $(nine_first_chars) ends with an underscore, we have the same problem.
Attachment #617855 - Flags: review?(jgilbert)
Attachment #617855 - Flags: review?(jgilbert) → review+
(Assignee)

Updated

5 years ago
Attachment #617644 - Attachment is obsolete: true
(Assignee)

Comment 5

5 years ago
Comment on attachment 617855 [details] [diff] [review]
better: also handle the case of underscore at position 8

[Approval Request Comment]
Please see https://wiki.mozilla.org/Tree_Rules for information on mozilla-central landings that do not require approval.

Possible risk to Fennec Native 14 (if any): None. This part of ANGLE is not even used at all yet on Android
Attachment #617855 - Flags: approval-mozilla-central?
(Assignee)

Comment 6

5 years ago
man, I will hate toys^Wmobile even more if this fails to land in time due to m-c approval :-/
Comment on attachment 617855 [details] [diff] [review]
better: also handle the case of underscore at position 8

[Triage Comment]
Merge of 14 to Aurora has completed.  Please renominate for mozilla-aurora.
Attachment #617855 - Flags: approval-mozilla-central?
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/8977089ce890
Assignee: nobody → bjacob
Target Milestone: --- → mozilla15
(Assignee)

Comment 9

5 years ago
Interestingly, i can only reproduce the problem on Windows. I guess that only the D3DCompiler rejects double underscores.
OS: All → Windows 7
Summary: WebGL Water demo broken by long identifier mapping → WebGL Water demo broken on Windows by long identifier mapping, because of double underscores
https://hg.mozilla.org/mozilla-central/rev/8977089ce890
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 11

5 years ago
I updated this test
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/glsl/misc/shader-with-256-character-identifier.frag.html

I think that would show this bug if it still existed.
Comment on attachment 617855 [details] [diff] [review]
better: also handle the case of underscore at position 8

[Approval Request Comment]
Regression caused by (bug #): 734657
User impact if declined: some webgl apps don't work
Testing completed (on m-c, etc.): On m-c for a while
Risk to taking this patch (and alternatives if risky): very low
String changes made by this patch: none
Attachment #617855 - Flags: approval-mozilla-aurora?
(In reply to Gregg Tavares from comment #11)
> I updated this test
> https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/
> conformance/glsl/misc/shader-with-256-character-identifier.frag.html
> 
> I think that would show this bug if it still existed.

Yup, definitely would. Thanks!
Whiteboard: webgl-conformance webgl-test-needed → webgl-conformance
Comment on attachment 617855 [details] [diff] [review]
better: also handle the case of underscore at position 8

[Triage Comment]
Broken WebGL apps, and we're very early in the cycle. Approving for Aurora 14.
Attachment #617855 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/742565069606
status-firefox12: --- → unaffected
status-firefox13: --- → unaffected
status-firefox14: --- → fixed
status-firefox15: --- → fixed
(In reply to Benoit Jacob [:bjacob] from comment #0)
> Test case: enable Web Console with JS warnings, and go to
> http://madebyevan.com/webgl-water/
> 
> Expected results: works
> 
> Actual: [15:13:12.398] uncaught exception: compile error: ERROR: 0:7:
> 'webgl__gl_Model_bbba1bfc05b59690' : identifiers containing two consecutive
> underscores (__) are reserved as possible future keywords

Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20100101 Firefox/14.0 beta 7
status-firefox14: fixed → verified
Whiteboard: webgl-conformance → webgl-conformance[qa+]
Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/20100101 Firefox/15.0 beta 1
Status: RESOLVED → VERIFIED
status-firefox15: fixed → verified
Whiteboard: webgl-conformance[qa+] → webgl-conformance[qa!]
You need to log in before you can comment on or make changes to this bug.