Opened 12 years ago
Closed 10 years ago
#1193 closed defect (worksforme)
Replace SpiderMonkey's Math functions with platform consistent versions
Reported by: | historic_bruno | Owned by: | |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 16 |
Component: | Core engine | Keywords: | spidermonkey |
Cc: | Yves | Patch: |
Description (last modified by )
As Philip reported on Bugzilla, there are differences in SpiderMonkey's floating point Math functions on different platforms. Even a small discrepancy can cause OOS errors in multiplayer games, if those functions are used in e.g. simulation, map generation, or AI code. We already use fixed point approximations for Currently no scripts appear to use sin
/cos
and atan2
in the engine, but not yet for scripts.atan
, acos
, or asin
.
(There may also be differences in pow
and exp
which we do use rarely, but I wasn't able to reproduce them with the above test cases.)
Attachments (1)
Change History (26)
comment:1 by , 12 years ago
Owner: | changed from | to
---|
comment:2 by , 12 years ago
Description: | modified (diff) |
---|---|
Summary: | Replace SpiderMonkey's trig functions with safe fixed-point versions → Replace SpiderMonkey's trig functions with platform consistent versions |
Updated title and description since fixed point is no longer the focus.
Also [11220] is relevant, it introduced a new rmgen function GetAngle
, which calls Math.acos
. I don't know if acos
is even a problem, I simply mentioned all the trig functions :)
comment:4 by , 12 years ago
This ticket is probably solved well enough for Alpha 9. Would be nice to add some tests eventually.
comment:5 by , 12 years ago
Milestone: | Alpha 9 → Alpha 10 |
---|
by , 12 years ago
These changes should fix the ticket, will wait till after A9 to commit them
comment:8 by , 12 years ago
This is not closed because we were wanting to add some tests. I should get round to this sometime soon.
comment:9 by , 12 years ago
Milestone: | Alpha 10 → Backlog |
---|
comment:10 by , 12 years ago
follow-up: 13 comment:12 by , 11 years ago
Summary: | Replace SpiderMonkey's trig functions with platform consistent versions → Replace SpiderMonkey's Math functions with platform consistent versions |
---|
Should also replace Math.pow
and Math.exp
at some point.
comment:13 by , 11 years ago
Milestone: | Backlog → Alpha 14 |
---|---|
Priority: | Should Have → Release Blocker |
Replying to historic_bruno:
Should also replace
Math.pow
andMath.exp
at some point.
This is in fact rather important, see #1990.
comment:14 by , 11 years ago
There are some implementations of Math.pow, Math.exp and Math.log in #1990 now.
comment:15 by , 11 years ago
Cc: | added |
---|
I found this in Spidermonkey's configure script:
--enable-more-deterministic Enable changes that make the shell more deterministic"
I don't know what it does and the word "more" looks suspicious.
comment:17 by , 11 years ago
As a reference, link to a sqrt algorithm: http://en.wikipedia.org/wiki/Methods_of_computing_square_roots#Binary_numeral_system_.28base_2.29
Of course, there is also a very simple way to just define sqrt(x)=pow(x,0.5). But a native sqrt algorithm would be more optimal.
comment:18 by , 11 years ago
comment:19 by , 11 years ago
I hope Math.round/ceil/floor don't give problems, as I use them for pow and exp calculation. And I don't know how we can replace those functions, as we can't read the internal values.
For Math.sqrt, you could try defining it as pow(x,0.5) and see if those problems still occur (although they're hard to test).
comment:20 by , 11 years ago
Owner: | removed |
---|
comment:21 by , 11 years ago
Current plan: test before A14 release, if there are no OOS errors with Aegis, push this to backlog. Otherwise experiment with replacing Math.pow
, but note it's much slower than the native Math.sqrt
(by 600x or so)
comment:22 by , 11 years ago
Milestone: | Alpha 14 → Alpha 15 |
---|---|
Priority: | Release Blocker → Should Have |
No recent OOS reports with Aegis, so I'm pushing this back to A15 and lowering priority. All that remains is deciding if we want to replace Math.sqrt
and adding some runtime tests like #433.
comment:24 by , 10 years ago
Milestone: | Alpha 15 → Alpha 16 |
---|
comment:25 by , 10 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
It has been working a long time now, so I don't think there are issues with the remaining functions.
The sin/cos approximations which are accurate to about 10-9 have been added with commit http://trac.wildfiregames.com/changeset/11226 . I will look at writing a good approximation for atan2.