Closed Bug 968566 Opened 10 years ago Closed 10 years ago

Math.fround() Bug

Categories

(Core :: JavaScript Engine, defect)

30 Branch
x86_64
Windows 8.1
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla30
Tracking Status
firefox29 --- verified
firefox30 --- verified

People

(Reporter: marc.rochel, Assigned: luke)

References

()

Details

Attachments

(1 file)

What did I do:
I have code in c++ which I compile to js with emscripten. emscripten has a setting called PRECISE_F32 which lets me specify whether it approximates float 32 calculations
with the js number or if it should put Math.fround() around all calculations to get precisely the same results. Since FF27, there is an optimization in place
that detects this and instead of rounding all the time just uses the 32 bit floating hardware available for calculations.
The attached file contains the compiled js code. In float64 is the code compiled without Math.fround() and in float32 all C-float calculations are surrounded with Math.fround().
I run these codes on FF26 and FF30.0a1 and Chrome. Note: The game uses a Newton Game Dynamics as a physics engine and therefore has a lot of C-float32 calculations in it.
(Besides attaching the both version I also uploaded them here: http://www.transmogrifier.de/ffbugreport/ )


What happened:
On FF26 and Chrome both versions run correctly. You can move around and play the game as expected. The version with Math.fround() runs slower than the one without though.
On FF27, the version without Math.fround() runs correctly as well. However, the version with Math.fround() in it show wired bugs. Objects jsut disappear and you can see the message "Something left the world" in the
output box below the game. This message appears if a physical body leaves the game world, which never happens with the other version/browser combinations. I assume that's because some calculations result in wrong result.


What should have happened:
The Math.fround() version of the game should work in FF27, too.
Oh, I meant:

What should have happened:
The Math.fround() version of the game should work in FF30.0a1, too. (I tried both and both have this issue.)
I can also reproduce, but only on FF27.  The game runs fine starting in FF28 which is in Beta and will be released in March.  Looks like we'll just have to wait 6 weeks before a fix.  I guess I'll leave this bug open for the next 6 weeks and then close WORKSFORME.  Thanks for reporting, let us know if you see any other problems.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I just tried this Version: ftp://ftp.mozilla.org/pub/mozilla.org/firefox/releases/28.0b1/win32/en-US/Firefox%20Setup%2028.0b1.exe I can confirm it works correctly with that version.

Still, it does not work in FF30.0a1 for me.
Thanks for the report! I've tried the game for a few minutes and didn't see anything wrong in the latest nightly (at least no differences between the f64 and the f32 versions - there are teleports (or jumps?) sometimes but I guess it's a part of the game, otherwise it'd mean that both versions are affected).

If you have some time and are interested to help us solve this issue, could you
- either bake a smaller test case, that doesn't use the canvas but that shows the error (by logging in the console some values and show that some of them are wrong, for instance)? Or create a C file that compiles with emscripten into a JS file that doesn't run correctly in the latest nightly?
- or use a tool like mozregression [0] to find the first nightly that misbehaves?

Any of these would dramatically help us track the underlying bug!

[0] http://harthur.github.io/mozregression/
Flags: needinfo?(marc.rochel)
Ok this is really odd. I ust updated to latest nightly and it still doesn't work for me. The about box tells me 30.0a1 (2014-02-06).

Just to clearify what I mean with works correctly: Yes the games switches the map every 10 seconds. That's normal. Apart from that, for example the screen directly after pressing a key to close the title Screen should look like this http://www.transmogrifier.de/ffbugreport/right.png . On the latest nightly, it Looks like this: http://www.transmogrifier.de/ffbugreport/wrong.png . Clearly the player character to the left and the portal to the right are missing.

I know the whole game is an ugly code to start debugging on. I'm sorry for that. I can try to create a smaller code that has similar issues - but probably not before the weekend.

What really makes me wonder is why this works for you on the latest nightly while it doesn't for me. What's different here?
Flags: needinfo?(marc.rochel)
I just used mozregression on this issue. I pinpointed two points in times when the behaviour realted to this issue changed.

First, there is this
Last good revision: 1ad9af3a2ab8 (2013-12-12)
First bad revision: 8b5875dc7e31 (2013-12-13)
The good Revision works as it should. The bad one just crashes Firefox altogether after loading the code.

This changed again here
Last good revision: 14ac61461f2a (2014-01-06)
First bad revision: e7a366c1036c (2014-01-07)
The good Revision crashes Firefox after loading the code. The bad one behaves like the latest nightly does for me right now.

Apart from that, this was the first time I used mozregression. I'm on win64 so I added --bits=32 to the commandline. After it pinpointed those intervals, it tried to continue - I assume hourly? Anyhow, it took some time, then downloaded another version, but then just exited with "No builds available for 64 bit Windows (try specifying --bits=32)". Somehow it seems to forget that I added the --bits=32 option at this point. So right now, I can only tell the day when the behaviour changed. Should I report this as a seperate mozregression bug?
Thanks for bisecting!

The first range is http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1ad9af3a2ab8&tochange=8b5875dc7e31 and includes landing of Float32 in Odin.

The second range is http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=14ac61461f2a&tochange=e7a366c1036c and the only patch related to odin is from bug 956402, which was a refactoring, so it doesn't clearly makes sense.

I'll try to investigate more deeply later. Can you observe the bug only on a specific platform or OS?

Regarding the mozregression bug, it should be filed separately, over there: https://github.com/mozilla/mozregression/issues
I tested on Win 8.1 64 bit. I only have this available right now.

OT: I reported the mozgeression issue here https://github.com/mozilla/mozregression/issues/81
Bug 969203 may have some relation to this problem.
Hi Marc, it's been a while and the next version of Firefox landed. We've fixed a few issues related to Float32 in Odin, is there any way you could retry to reproduce locally with the latest Nightly? Thanks a lot!
Flags: needinfo?(marc.rochel)
Hi Benjamin!

Ok I just tried the code from above (http://www.transmogrifier.de/ffbugreport/) on the following Versions on Windows:

FF29 on beta update channel, without Math.fround	works
FF29 on beta update channel, with Math.fround		broken
FF31.0a1 (2014-04-03), without Math.fround		works
FF31.0a1 (2014-04-03), with Math.fround			works

So beta update channel is broken for me, but the newest nightly works.
Thanks for working on this :)
Flags: needinfo?(marc.rochel)
(In reply to Marc from comment #11)
> 
> So beta update channel is broken for me, but the newest nightly works.
> Thanks for working on this :)

Thanks a lot for trying it out and reporting the issue initially! If you are willing to help us find out what's the underlying bug that helped solving the issue, feel free to use mozregression again, that will be greatly appreciated. Otherwise, that's fine and thanks again for helping here!
Flags: needinfo?(marc.rochel)
Ok. I did mozgression. "good" means "broken", "bad" means "works" - as I'm searching for the first Version that works again instead of the first Version where it breaks.

Got as far as we can go bisecting nightlies...
Ensuring we have enough metadata to get a pushlog...
Last good revision: f073b3d6db1f (2014-03-14)
First bad revision: 82c90c17fc95 (2014-03-15)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f073b3d6db1f&tochange=82c90c17fc95

I still have that mozregression issue I described earlier, so I can't go any further. However, looking at the changes made, maybe this one was the cause: https://bugzilla.mozilla.org/show_bug.cgi?id=983448 ?
Flags: needinfo?(marc.rochel)
Thanks a lot! This is indeed certainly the cause of the bug, we'll ask for an uplift in mozilla beta.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee: nobody → luke
Depends on: 983448
Target Milestone: --- → mozilla30
Just for completeness: I just updated FF29 beta channel and tried it again with Math.fround and now that one works as well :)
Updating flags based on comment 15. Could you also verify this on Firefox 30 (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/)?
Flags: needinfo?(marc.rochel)
I downloaded and tested http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/firefox-30.0a2.en-US.win32.zip and it works with and without Math.fround.
Flags: needinfo?(marc.rochel)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: