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

KinematicBody performance and quality improvements #27415

Merged
merged 1 commit into from
May 2, 2019

Conversation

aqnuep
Copy link
Contributor

@aqnuep aqnuep commented Mar 25, 2019

KinematicBody performance and quality improvements

With this change finally one can use compound collisions (like those created
by Gridmaps) without serious performance issues. The previous KinematicBody
code for Bullet was practically doing a whole bunch of unnecessary
calculations. Gridmaps with fairly large octant sizes (in my case 32) can get
up to 10000x speedup with this change (literally!). I expect the FPS demo to
get a fair speedup as well.

List of fixes and improvements:

  • Fixed a general bug in move_and_slide that affects both GodotPhysics and
    Bullet, where ray shapes would be ignored unless the stop_on_slope parameter
    is disabled. Not sure where that came from, but looking at the 2D physics
    code it was obvious there's a difference.
  • Enabled the dynamic AABB tree that Bullet uses to allow broadphase collision
    tests against individual shapes of compound shapes. This is crucial to get
    good performance with Gridmaps and in general improves the performance
    whenever a KinematicBody collides with compound collision shapes.
  • Added code to the broadphase collision detection code used by the Bullet
    module for KinematicBodies to also do broadphase on the sub-shapes of
    compound collision shapes. This is possible thanks to the dynamic AABB
    tree that was previously disabled and it's the change that provides the
    biggest performance boost.
  • Now broadphase test is only done once per KinematicBody in Bullet instead of
    once per each of its shapes which was completely unnecessary.
  • Fixed the way how the ray separation results are populated in Bullet which
    was completely broken previously, overwriting previous results and similar
    non-sense.
  • Fixed ray shapes for good now. Previously the margin set in the editor was
    not respected at all, and the KinematicBody code for ray separation was
    complete bogus, thus all previous attempts to fix it were mislead.
  • Fixed an obvious bug also in GodotPhysics where an out-of-bounds index was
    used in the ray result array.

There are a whole set of other problems with the KinematicBody code of Bullet
which cost performance and may cause unexpected behavior, but those are not
addressed in this change (need to keep it "simple").

Not sure whether this fixes any outstanding Github issues but I wouldn't be
surprised.

@aqnuep aqnuep force-pushed the kinematicbody_fixes branch 2 times, most recently from 10a6a41 to 7a1de7c Compare March 25, 2019 22:47
@Chaosus Chaosus added this to the 3.2 milestone Mar 26, 2019
@aqnuep
Copy link
Contributor Author

aqnuep commented Mar 26, 2019

I've checked the FPS tutorial and indeed it runs much faster and smoother.

However, you won't be able to climp stairs without jumping (not sure you could before), but it's no surprise when you consider this:

  1. The stairs in the FPS tutorial are represented by a slope shape, except for the last step which for some reason is a true step.
  2. The tutorial uses a box shape as the "foot", but that's just not the appropriate way to make a character be able to climb actual steps, at least not without additional logic.

Pinging @TwistedTwigleg who is the maintainer of the FPS tutorial project.

@TwistedTwigleg
Copy link
Contributor

This is great work @aqnuep!
I haven’t yet, but once I’m able I’ll give it a test run with the FPS tutorial, though I have no doubt that it indeed improves performance (especially for grid maps).

As far as the FPS tutorial goes, if I remember correctly you were able to climb up stairs previously with no special code, in fact I think if you went up the stairs too quickly you would actually launch into the air a bit.

  1. The stairs are supposed to be slopes all the way. Likely the last step you are seeing is actually a floor tile. I had a few issues lining everything up initially. This will probably need to be left as is for now, though if I can change it to a slope I will.
  2. The reason the tutorial uses a box shape as a foot is because it was too slippery and would start sliding if there is a slight angle. This made the player slide down slopes pretty dramatically and going up slopes took a lot longer because of it, which is why there is the box, though if I recall this was in Godot 3 beta. This has likely since been fixed and the box should probably be removed.

I can update the FPS tutorial on Github at least with some changes and optimizations. I remember someone mentioning changing some settings in one of the materials speeds things up, but now I cannot find the post. I can also see about updating the documentation afterwards.

Anyway, later I’ll check the FPS tutorial with this PR just to double check. It would be great if performance is improved, and it will also be a good opportunity to see if the project works with Godot 3.1.

Thanks for the ping, and great work on the PR!

@muiroc
Copy link
Contributor

muiroc commented Mar 26, 2019

I've found an issue when the CollisionShape has some transform:
Check this scene stage_modified.zip in the Platformer 3D demo, enable visualize collision shapes

the middle box collider has no extra transform, it works,
now try walking on one of the other two boxes
the collision shapes of those two got some local offsets and causes that weird collision result

Also kinematic bodies are pushing rigidbodies even if infinite_inertia is false, (but this also happened before, only more rarely)

@aqnuep
Copy link
Contributor Author

aqnuep commented Mar 26, 2019

As far as the FPS tutorial goes, if I remember correctly you were able to climb up stairs previously with no special code, in fact I think if you went up the stairs too quickly you would actually launch into the air a bit.

That might have been just the result of the bugs in the code. You know, sometimes two wrongs happen to make a right :)

  1. The stairs are supposed to be slopes all the way. Likely the last step you are seeing is actually a floor tile. I had a few issues lining everything up initially. This will probably need to be left as is for now, though if I can change it to a slope I will.

They are not. All the stairs have a step like on the screenshot below:

godot 2019-03-26 16-29-48-86

You can check it yourself by enabling Debug->Visible Collision Shapes before running the project.

  1. The reason the tutorial uses a box shape as a foot is because it was too slippery and would start sliding if there is a slight angle. This made the player slide down slopes pretty dramatically and going up slopes took a lot longer because of it, which is why there is the box, though if I recall this was in Godot 3 beta. This has likely since been fixed and the box should probably be removed.

That's what the stop_on_slope parameter of move_and_slide is for, which for some reason is set to the value of 0.05 in the FPS tutorial while it should actually be a boolean value (I guess left-over from the interface of a previous version of Godot).

I've found an issue when the CollisionShape has some transform:
Check this scene stage_modified.zip in the Platformer 3D demo, enable visualize collision shapes

the middle box collider has no extra transform, it works,
now try walking on one of the other two boxes
the collision shapes of those two got some local offsets and causes that weird collision result

I'll check that.

Also kinematic bodies are pushing rigidbodies even if infinite_inertia is false, (but this also happened before, only more rarely)

Yeah, I did not change the code for infinite_inertia, thus behavior should be the same as before barring any side effects of the bugs in the original code.

@TwistedTwigleg
Copy link
Contributor

That might have been just the result of the bugs in the code. You know, sometimes two wrongs happen to make a right :)

That is likely. I remember being surprised that I didn't need to write any code to handle slopes and the like.

They are not. All the stairs have a step like on the screenshot below:

I could have sworn the collision shapes were slopes... My apologizes 😅
Why I did it that way, I cannot imagine.

Thinking about it, they might have that step because I used a convex hull for the collusion shape, though I don't remember for sure. Regardless of the reason, the stairs shouldn't have a single random step. Stairs in games are primarily either a continuous slope, or every step has collision (mainly for IK).

I wonder if I still have the blender file somewhere. If I do, I might be able to make a collision mesh to use.

Thanks for bringing this up! I'll see what I can do to fix this once I have a chance to get back to my development machine.

That's what the stop_on_slope parameter of move_and_slide is for, which for some reason is set to the value of 0.05 in the FPS tutorial while it should actually be a boolean value (I guess left-over from the interface of a previous version of Godot).

I think the old API allowed you to set the tolerance or something. I'm surprised it worked at all to be honest. I guess 0.05 gets converted to false?

Regardless, I'll change this as well, since 0.05 doesn't make sense when the value is supposed to be a boolean.

@aqnuep
Copy link
Contributor Author

aqnuep commented Mar 26, 2019

@muiroc, thanks for the repro case. I know what's going on.

For shapes with a non-zero transform the Bullet module uses a compound shape. Previously on each transform change all shapes were re-created. As I change that, but apparently didn't fix some other issues (in particular properly updating the transform in such cases), it causes this problem.

I guess I'll just revert to the old behavior of recreating all shapes when a shape's transform changes for now, as fixing this will require a bit more twiddling with the code of Bullet CollisionBodies.

EDIT: Actually I take it back. There seems to be some other issue with the leaf node detection code, but I have a guess what's it.

@muiroc
Copy link
Contributor

muiroc commented Mar 26, 2019

@aqnuep Cool! Thank you for approaching this!

Lately physics performance are what I'm worried about since in some cases (generally when moving into a corner), with a trimesh and/or about 6 moving kinematic bodies, things can gets quite slow...
I still need to setup a proper example project and submit an issue

btw trimeshes seems unaffected by that local transform thing

@TwistedTwigleg
Copy link
Contributor

TwistedTwigleg commented Mar 26, 2019

This is off topic, but since the issue came up here, I thought I'd mention it quickly:

The foot box, the move_and_slide functional call, and stairs in the Godot FPS tutorial Github repository have been updated.

The collision box on the player's feet seems to not be needed when move_and_slide is called with the proper arguments. Everything seems to be good there, and indeed it appears that the issue was the move_and_slide API changed a bit since when the tutorial was written.

As for the stairs, the issue wasn't actually the collision shape per say. The collision shape was/is a slope, which you can check by looking at the .dae file and the scene the mesh library was created from.

However, the issue was the slop wasn't quite tall enough, so when two stairs are put together there is a lip when they connect. This lip was also present when stairs connect to a floor tile, but the effect is much less noticeable.

Unfortunately, without redoing the stairs completely I can't easily completely fix the issue, especially since I no longer seem to have the .blend files anymore.

However, I removed the lip almost entirely by pulling each stair a bit and adjusting the collision shape mesh. The lip is still there and slows the player down just a hair, but it is way better than it was before. Now the player can go up and down the stairs with no special code needed, and without having to jump when the stair tiles meet.

At some point I'll update the documentation. It will probably have to wait until the weekend, especially because of all the zip files (maybe we should drop the zip files for every part? I dunno, something to bring up in the Godot Documentation repository I suppose.)


As for testing the PR, I haven't just yet. I need to setup my computer to compile Godot from source first.

I'll update this post with the before and after performance gains once possible.


Edit: So I tested both Godot 3.1 release and this PR built without debug symbols. Here are the results:

  • This PR (desert level): 111-117 FPS on average
  • Godot 3.1 (desert level): 110-113 FPS on average

For testing I disabled V-Sync and used the Window/Monitor section of the Godot debug menu to track the frame rate. I stood more or less in the center of the level and fired the pistol until I needed to reload.

As far as I can tell, there is only a minor speed bump for the FPS tutorial. Not what I would have expected, but maybe I do not have something setup correctly.

With this change finally one can use compound collisions (like those created
by Gridmaps) without serious performance issues. The previous KinematicBody
code for Bullet was practically doing a whole bunch of unnecessary
calculations. Gridmaps with fairly large octant sizes (in my case 32) can get
up to 10000x speedup with this change (literally!). I expect the FPS demo to
get a fair speedup as well.

List of fixes and improvements:

- Fixed a general bug in move_and_slide that affects both GodotPhysics and
  Bullet, where ray shapes would be ignored unless the stop_on_slope parameter
  is disabled. Not sure where that came from, but looking at the 2D physics
  code it was obvious there's a difference.
- Enabled the dynamic AABB tree that Bullet uses to allow broadphase collision
  tests against individual shapes of compound shapes. This is crucial to get
  good performance with Gridmaps and in general improves the performance
  whenever a KinematicBody collides with compound collision shapes.
- Added code to the broadphase collision detection code used by the Bullet
  module for KinematicBodies to also do broadphase on the sub-shapes of
  compound collision shapes. This is possible thanks to the dynamic AABB
  tree that was previously disabled and it's the change that provides the
  biggest performance boost.
- Now broadphase test is only done once per KinematicBody in Bullet instead of
  once per each of its shapes which was completely unnecessary.
- Fixed the way how the ray separation results are populated in Bullet which
  was completely broken previously, overwriting previous results and similar
  non-sense.
- Fixed ray shapes for good now. Previously the margin set in the editor was
  not respected at all, and the KinematicBody code for ray separation was
  complete bogus, thus all previous attempts to fix it were mislead.
- Fixed an obvious bug also in GodotPhysics where an out-of-bounds index was
  used in the ray result array.

There are a whole set of other problems with the KinematicBody code of Bullet
which cost performance and may cause unexpected behavior, but those are not
addressed in this change (need to keep it "simple").

Not sure whether this fixes any outstanding Github issues but I wouldn't be
surprised.
@aqnuep
Copy link
Contributor Author

aqnuep commented Mar 26, 2019

@muiroc, it's fixed now. The problem was that I forgot to transform the broadphase search bounds into the local space of the compound shape.

@rogeriodec
Copy link

I'm developing a game (initially for Android) that has 400 active KinematicBody on the screen.
This proved to be impractical on Android (the game runs at 3 FPS)...
This way I'm forced to completely change my strategy: either abandon the project for Android and focus on Windows or change the whole idea of the game on Android.
In both cases, there will be a huge rework.
I say this, as this issue seems like it can solve this problem without my having to abandon my project.
So, I ask you to approve this update, as soon as possible.

@reduz
Copy link
Member

reduz commented Apr 4, 2019

This looks good from the non-bullet side, up to @AndreaCatania to review it.

@reduz
Copy link
Member

reduz commented Apr 4, 2019

I'm developing a game (initially for Android) that has 400 active KinematicBody on the screen.
This proved to be impractical on Android (the game runs at 3 FPS)...

KinematicBody is expensive, it requires a lot more computation than a single rigid body. If possible, try using a dynamic controller (simple rigid body with integration override) instead of a kinematic one for large numbers.

@realkotob
Copy link
Contributor

realkotob commented Apr 5, 2019

KinematicBody is expensive, it requires a lot more computation than a single rigid body. If possible, try using a dynamic controller (simple rigid body with integration override) instead of a kinematic one for large numbers.

@reduz Ah thank you, I was wondering why the hell my scene was lagging with a few dozen boids, I guess I'll try using rigidbody and see what happens. I was suspecting the limitation was coming from the physics engine since the calculations themselves happen on a separate thread and never lock or hang the main process.

Is there a docs section for using the profiler or debugger? I tried using it to identify hangups but I couldn't even tell what was taking the most time
e.g. it only says _physics_process so I can't tell what in the physics exactly is lagging.

@akien-mga
Copy link
Member

@AndreaCatania Could you review the Bullet changes?

@AndreaCatania
Copy link
Contributor

Sorry for the delay I'll check it today

@AndreaCatania
Copy link
Contributor

@aqnuep Nice work with broad phase improvement, keep improving it please!
@akien-mga merge it

@akien-mga
Copy link
Member

Thanks for the review. I'll wait a couple of days to merge this one as I merged over 50 PRs the last two days, with potential for some regressions, so I'd prefer to hold off on big PRs for a bit to make it easier to spot and bisect potential issues.

@akien-mga akien-mga merged commit 2629242 into godotengine:master May 2, 2019
@akien-mga
Copy link
Member

Thanks a lot!

@piratesephiroth
Copy link
Contributor

piratesephiroth commented Dec 1, 2019

KinematicBody is expensive, it requires a lot more computation than a single rigid body. If possible, try using a dynamic controller (simple rigid body with integration override) instead of a kinematic one for large numbers.

What about a RigidBody on Kinematic mode? Is it as expensive as a KinematicBody?

@AndreaCatania
Copy link
Contributor

A RB in KM == KB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.