Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KinematicBody2D stop_on_slope is broken only in release builds #24841

Closed
timoschwarzer opened this issue Jan 8, 2019 · 4 comments · Fixed by #24855
Closed

KinematicBody2D stop_on_slope is broken only in release builds #24841

timoschwarzer opened this issue Jan 8, 2019 · 4 comments · Fixed by #24855

Comments

@timoschwarzer
Copy link
Contributor

timoschwarzer commented Jan 8, 2019

Godot version:
3.1 master

OS/device including version:
At least Linux/X11

Issue description:
Okay, this one is a bit strange. The stop_on_slope parameter in move_and_slide does only work in debug builds. I don't know if it's a precision error or whatever. (Maybe -ffast-math?)

Demonstration
https://youtu.be/qk1NBRa5DtI

As you can see, I output the X and Y coordinates of lv_n + p_floor_direction.
(Here: https://github.com/godotengine/godot/blob/master/scene/2d/physics_body_2d.cpp#L1293)
This for some reason gives (0, 0) on debug builds and something else on release_debug builds.

Minimal reproduction project:
Test.zip

@timoschwarzer
Copy link
Contributor Author

Note: It does not work in 3.1-beta1, too. But if you compile bcecf56 manually in debug mode, it works.

@hpvb
Copy link
Member

hpvb commented Jan 8, 2019

Another -ffast-math victim probably?

Nvm, it appears that the Vector2() constructor doesn't reset to 0 for some reason. I'll have a look, I have an idea why that may be.

NVM my NVM, it's probably -ffast-math.

@timoschwarzer
Copy link
Contributor Author

It's indeed -ffast-math.

@hpvb
Copy link
Member

hpvb commented Jan 8, 2019

I'm running some benchmarks, PR with background info will follow shortly

hpvb added a commit to hpvb/godot that referenced this issue Jan 9, 2019
Godot supports many different compilers and for production releases we
have to support 3 currently: GCC8, Clang6, and MSVC2017. These compilers
all do slightly different things with -ffast-math and it is causing
issues now. See godotengine#24841, godotengine#24540, godotengine#10758, godotengine#10070. And probably other
complaints about physics differences between release and release_debug
builds.

I've done some performance comparisons on Linux x86_64. All tests are
ran 20 times.

Bunnymark: (higher is better)
(bunnies)    min    max  stdev average
fast-math   7332   7597    71     7432
this pr     7379   7779   108     7621 (102%)

FPBench (gdscript port http://fpbench.org/) (lower is better)
(ms)
fast-math  15441  16127   192    15764
this pr    15671  16855   326    16001  (99%)

Float_add (adding floats in a tight loop) (lower is better)
(sec)
fast-math   5.49   5.78  0.07     5.65
this pr     5.65   5.90  0.06     5.76  (98%)

Float_div (dividing floats in a tight loop) (lower is better)
(sec)
fast-math  11.70  12.36  0.18    11.99
this pr    11.92  12.32  0.12    12.12  (99%)

Float_mul (multiplying floats in a tight loop) (lower is better)
(sec)
fast-math  11.72  12.17  0.12    11.93
this pr    12.01  12.62  0.17    12.26  (97%)

I have also looked at FPS numbers for tps-demo, 3d platformer, 2d
platformer, and sponza and could not find any measurable difference.

I believe that given the issues and oft-reported (physics) glitches on
release builds I believe that the couple of percent of tight-loop
floating point performance regression is well worth it.

This fixes godotengine#24540 and fixes godotengine#24841
hpvb added a commit to hpvb/godot that referenced this issue Jan 9, 2019
Godot supports many different compilers and for production releases we
have to support 3 currently: GCC8, Clang6, and MSVC2017. These compilers
all do slightly different things with -ffast-math and it is causing
issues now. See godotengine#24841, godotengine#24540, godotengine#10758, godotengine#10070. And probably other
complaints about physics differences between release and release_debug
builds.

I've done some performance comparisons on Linux x86_64. All tests are
ran 20 times.

Bunnymark: (higher is better)
(bunnies)    min    max  stdev average
fast-math   7332   7597    71     7432
this pr     7379   7779   108     7779 (109%)

FPBench (gdscript port http://fpbench.org/) (lower is better)
(ms)
fast-math  15441  16127   192    15764
this pr    15671  16855   326    16855  (99%)

Float_add (adding floats in a tight loop) (lower is better)
(sec)
fast-math   5.49   5.78  0.07     5.65
this pr     5.65   5.90  0.06     5.76  (98%)

Float_div (dividing floats in a tight loop) (lower is better)
(sec)
fast-math  11.70  12.36  0.18    11.99
this pr    11.92  12.32  0.12    12.12  (99%)

Float_mul (multiplying floats in a tight loop) (lower is better)
(sec)
fast-math  11.72  12.17  0.12    11.93
this pr    12.01  12.62  0.17    12.26  (97%)

I have also looked at FPS numbers for tps-demo, 3d platformer, 2d
platformer, and sponza and could not find any measurable difference.

I believe that given the issues and oft-reported (physics) glitches on
release builds I believe that the couple of percent of tight-loop
floating point performance regression is well worth it.

This fixes godotengine#24540 and fixes godotengine#24841
akien-mga pushed a commit to akien-mga/godot that referenced this issue Jul 3, 2019
Godot supports many different compilers and for production releases we
have to support 3 currently: GCC8, Clang6, and MSVC2017. These compilers
all do slightly different things with -ffast-math and it is causing
issues now. See godotengine#24841, godotengine#24540, godotengine#10758, godotengine#10070. And probably other
complaints about physics differences between release and release_debug
builds.

I've done some performance comparisons on Linux x86_64. All tests are
ran 20 times.

Bunnymark: (higher is better)
(bunnies)    min    max  stdev average
fast-math   7332   7597    71     7432
this pr     7379   7779   108     7621 (102%)

FPBench (gdscript port http://fpbench.org/) (lower is better)
(ms)
fast-math  15441  16127   192    15764
this pr    15671  16855   326    16001  (99%)

Float_add (adding floats in a tight loop) (lower is better)
(sec)
fast-math   5.49   5.78  0.07     5.65
this pr     5.65   5.90  0.06     5.76  (98%)

Float_div (dividing floats in a tight loop) (lower is better)
(sec)
fast-math  11.70  12.36  0.18    11.99
this pr    11.92  12.32  0.12    12.12  (99%)

Float_mul (multiplying floats in a tight loop) (lower is better)
(sec)
fast-math  11.72  12.17  0.12    11.93
this pr    12.01  12.62  0.17    12.26  (97%)

I have also looked at FPS numbers for tps-demo, 3d platformer, 2d
platformer, and sponza and could not find any measurable difference.

I believe that given the issues and oft-reported (physics) glitches on
release builds I believe that the couple of percent of tight-loop
floating point performance regression is well worth it.

This fixes godotengine#24540 and fixes godotengine#24841

(cherry picked from commit e5b335d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants