Closed Bug 748112 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Canvas: WebGL, defect)

All
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla15
Tracking Status
firefox12 --- unaffected
firefox13 --- unaffected
firefox14 --- verified
firefox15 --- verified

People

(Reporter: bjacob, Assigned: bjacob)

References

Details

(Whiteboard: webgl-conformance[qa!])

Attachments

(1 file, 1 obsolete file)

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.
Attachment #617644 - Flags: review?(jgilbert)
Whiteboard: webgl-conformance webgl-test-needed
Does this need to land on any branches?
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+
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+
Attachment #617644 - Attachment is obsolete: true
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?
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?
http://hg.mozilla.org/integration/mozilla-inbound/rev/8977089ce890
Assignee: nobody → bjacob
Target Milestone: --- → mozilla15
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
Closed: 7 years ago
Resolution: --- → FIXED
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+
(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
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
Whiteboard: webgl-conformance[qa+] → webgl-conformance[qa!]
You need to log in before you can comment on or make changes to this bug.