Closed
Bug 748112
Opened 12 years ago
Closed 12 years ago
WebGL Water demo broken on Windows by long identifier mapping, because of double underscores
Categories
(Core :: Graphics: CanvasWebGL, defect)
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)
1.33 KB,
patch
|
jgilbert
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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•12 years ago
|
||
Attachment #617644 -
Flags: review?(jgilbert)
Assignee | ||
Updated•12 years ago
|
Whiteboard: webgl-conformance webgl-test-needed
Comment 2•12 years ago
|
||
Does this need to land on any branches?
Assignee | ||
Comment 3•12 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.
Updated•12 years ago
|
Attachment #617644 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 4•12 years ago
|
||
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)
Updated•12 years ago
|
Attachment #617855 -
Flags: review?(jgilbert) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #617644 -
Attachment is obsolete: true
Assignee | ||
Comment 5•12 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•12 years ago
|
||
man, I will hate toys^Wmobile even more if this fails to land in time due to m-c approval :-/
Comment 7•12 years ago
|
||
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•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/8977089ce890
Assignee: nobody → bjacob
Target Milestone: --- → mozilla15
Assignee | ||
Comment 9•12 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
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8977089ce890
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 11•12 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.
Assignee | ||
Comment 12•12 years ago
|
||
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?
Assignee | ||
Comment 13•12 years ago
|
||
(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 14•12 years ago
|
||
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+
Assignee | ||
Comment 15•12 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/742565069606
status-firefox12:
--- → unaffected
status-firefox13:
--- → unaffected
status-firefox14:
--- → fixed
status-firefox15:
--- → fixed
Comment 16•12 years ago
|
||
(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
Updated•12 years ago
|
Whiteboard: webgl-conformance → webgl-conformance[qa+]
Comment 17•12 years ago
|
||
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.
Description
•