Closed
Bug 766796
Opened 12 years ago
Closed 12 years ago
No particles in Spectrascade WebGL demo (regression)
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: epinal99-bugzilla2, Assigned: bzbarsky)
References
()
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
peterv
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Open Spectrascade WebGL demo: http://www.jeshua.me/spectrascade/sc
You should see a small gray 3D cube with a stream of particles around it.
In FF16, there are no particles but the rest of the demo is OK.
Mozregression range:
m-c
2012-05-24
2012-05-25
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f43e8d300f21&tochange=1dd0c5c6d9fd
m-i
2012-05-23
2012-05-24
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=59ec4eabd9ce&tochange=1865549541b7
Keywords: regression
Comment 2•12 years ago
|
||
I don't see any particles on FF 15 or 16 on Win7.
I do see what looks like five duplicate warnings in the Error Console per frame: "bufferSubData: negative offset." (at least, until it hits the 32-warning limit we added)
Comment 3•12 years ago
|
||
You also seem to be triggering a SetDimensions every frame. Are you setting the width and height to the same thing every frame, or something?
Comment 4•12 years ago
|
||
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/7ae630f43357
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120523083523
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5f14275bb276
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120523093244
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7ae630f43357&tochange=5f14275bb276
Inlocal build
Last Good: 7ffffcb45b94
First Bad: dd6c4f6a2448
Triggered by:
dd6c4f6a2448 Benoit Jacob — Bug 757526 - Use stdint instead of PRInt types in WebGL implementation - r=Ms2ger
Blocks: 757526
Updated•12 years ago
|
tracking-firefox15:
--- → ?
tracking-firefox16:
--- → ?
Updated•12 years ago
|
Version: 16 Branch → 15 Branch
Comment 5•12 years ago
|
||
Wow! Thanks a lot for this bisecting. Looking.
Comment 6•12 years ago
|
||
The demo generates a lot of these:
[11:23:36.160] Error: WebGL: bufferSubData: negative offset @ http://www.jeshua.me/spectrascade/js/spidergl.js:4748
Together with the fact that it's a regression from Bug 757526, it's quite clear what's happening there.
Comment 7•12 years ago
|
||
Here's the bufferSubData change made in bug 757526:
https://hg.mozilla.org/mozilla-central/rev/dd6c4f6a2448#l2.65
-WebGLContext::BufferSubData(PRInt32 target, PRInt32 offset, const JS::Value& data, JSContext *cx)
+WebGLContext::BufferSubData(WebGLenum target, WebGLintptr offset, const JS::Value& data, JSContext *cx)
This is indeed a functional change, but as far as I can see, it is fixing a conformance bug, not introducing one. Need further debugging, but at the moment it looks as if the demo is doing invalid bufferSubData calls. Not yet sure though that that is related to the rendering issue.
Comment 8•12 years ago
|
||
Can't easily debug on Windows at the moment due to bug 767006.
Depends on: 767006
Assignee | ||
Comment 9•12 years ago
|
||
So the second argument being passed in is an object. Specifically, a WebGLBuffer.
This lands in xpc::ValueToInt64, which converts it to a double. It gets back NaN.
Per spec this _should_ get turned into 0, but xpc::ValueToInt64 does static_cast<int64_t>(doubleval), which gets us 0x8000000000000000 (the bit-pattern of the NaN), but treated as a signed 64-bit int. I have no idea why it does that, bit-pattern thing, exactly.
In any case, that's a negative value, treated as a signed 64-bit int, and things break.
Now the page is of course totally broken in terms of what it passes to that second argument. But we need to fix the bug in xpc::ValueToInt64.
Component: Canvas: WebGL → XPConnect
QA Contact: canvas.webgl → xpconnect
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → bzbarsky
Assignee | ||
Comment 10•12 years ago
|
||
Benoit, what's a good way of testing this? Should be easy to reproduce by just passing NaN to any WebGL method that takes signed 64-bit ints and requires them to be nonnegative, without this patch
Attachment #635655 -
Flags: review?(peterv)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [need review]
Comment 11•12 years ago
|
||
We can simply take any WebGL method that takes a 64bit integer and allows to test, from JavaScript, whether that integer is exactly zero. bufferSubData is such a method and is probably the one that makes this easiest. Indeed, the WebGL spec says: "If the data would be written past the end of the buffer object an INVALID_VALUE error is generated." It also inherits the GL requirement to generate an error if the offset is negative. So it's quite simple: create a buffer of N bytes, and try to update N bytes at a certain offset, this will generate an error if the offset is nonzero. Like this:
assert(!gl.getError());
var b = gl.createBuffer();
gl.bindBuffer(gl.ARRAY_BUFFER, b);
var a = new Float32Array(1);
gl.bufferData(gl.ARRAY_BUFFER, a, gl.STATIC_DRAW);
var nan = 0/0;
gl.bufferSubData(gl.ARRAY_BUFFER, nan, a);
assert(!gl.getError()); // this is the test
But I'm not sure that we want to take this into the WebGL conformance test suite. The last time that I checked in such a general DOM/JS bindings test in the WebGL conformance test suite, I got a well-founded remark that this didn't really fit in there. Isn't there a "DOM/JS bindings conformance test suite"? Would fit better in such a place. If there isn't then maybe we could at least rationalize this effort by adding a dedicated test page for these things to the WebGL conformance tests.
Assignee | ||
Comment 12•12 years ago
|
||
Oh, I was just going to add this to our tree. There is no general "DOM/JS bindings test suite" yet, though I'm hoping there will be one at some point...
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #635862 -
Flags: review?(peterv)
Assignee | ||
Updated•12 years ago
|
Attachment #635655 -
Attachment is obsolete: true
Attachment #635655 -
Flags: review?(peterv)
Updated•12 years ago
|
Attachment #635862 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 14•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla16
Assignee | ||
Comment 15•12 years ago
|
||
Comment on attachment 635862 [details] [diff] [review]
Now with test
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 757526
User impact if declined: Some WebGL things apparently won't work correctly
Testing completed (on m-c, etc.): Page that shows this bug
Risk to taking this patch (and alternatives if risky): Low-risk: just converts
NaN to 0 when converting to int, as the spec requires, instead of converting
to garbage. Only affects conversion to 64-bit ints, which are very rare in
IDL.
String or UUID changes made by this patch: None.
Attachment #635862 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 16•12 years ago
|
||
And https://hg.mozilla.org/integration/mozilla-inbound/rev/421b653f33dc to deal with the Mac 10.5 bustage due to it not supporting WebGL.
Comment 17•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/82849fb54cc0
https://hg.mozilla.org/mozilla-central/rev/421b653f33dc
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Comment 18•12 years ago
|
||
Comment on attachment 635862 [details] [diff] [review]
Now with test
[Triage Comment]
Low risk fix that brings us closer to spec. Approved for Aurora 15.
Attachment #635862 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 19•12 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/c697cdebddc9 for the roll-up patch.
status-firefox15:
--- → fixed
Updated•12 years ago
|
status-firefox16:
--- → fixed
Comment 20•12 years ago
|
||
Able to see the issue on Nightly 2012-06-01. The particles are seen on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/20100101 Firefox/15.0b2. Verified fixed.
You need to log in
before you can comment on or make changes to this bug.
Description
•