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

Chunk tilemap physics #102662

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

groud
Copy link
Member

@groud groud commented Feb 10, 2025

Right now, TileMapLayer physics is implemented using one collision body for each cell, which is likely lowering runtime performance (to be verified, but I doubt having thousands of colliders on a map has no impact). This PR reworks the TileMapLayer physics so that cells shapes get merged into bigger collision shapes, whenever possible. Aphysics_quadrant_size param allow changing the size of each collision chunk.

This video illustrate the processing done:

2025-02-10.16-16-07.mp4

Note this PR breaks compatibility for get_coords_for_body_rid, as with the PR, a single body can cover multiple cells.

Bugsquad edit: Fixes #84163 Fixes #89458

@Calinou
Copy link
Member

Calinou commented Feb 10, 2025

@groud
Copy link
Member Author

groud commented Feb 10, 2025

(this is essentially the physics counterpart of the existing TileMap navmesh baking).

More or less though. This PR makes the baking as built-in to the TileMaplayer itself, while navmesh baking is made on the navigation region side (like, you to define a region and bake it).

@KoBeWi
Copy link
Member

KoBeWi commented Feb 10, 2025

I could confirm it fixes performance issues (#84163). Could not confirm fixing seams (#89458) because it's too unreliable, but it's safe to assume it's fixed too.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 10, 2025

From further testing:

  • It would be nice to have a way to visualize chunks. If the chunk size is too small, it could still cause seams and visualization would make it easier to pick a better size. Although seems like there is no size limit, so you can just make it super big to cover whole map.
  • The chunks are regenerated every time you modify tiles. While it's expected, it causes problems in the editor, because too big chunks make editing super slow. Is there no way to avoid re-creating shapes in the editor? Maybe only do that when they are visible? Though it's easily solvable with a script that makes chunks bigger at runtime, so not much a problem.

@groud
Copy link
Member Author

groud commented Feb 10, 2025

It would be nice to have a way to visualize chunks. If the chunk size is too small, it could still cause seams and visualization would make it easier to pick a better size. Although seems like there is no size limit, so you can just make it super big to cover whole map.

Ah well, the colors are a bit randomized (to distinguish shapes a bit), but it is done per-chunk. So well, the first shape always use the same color so you cannot distinguish chunks. I guess making the seams visible should be doable, I'll give it a try.

The chunks are regenerated every time you modify tiles. While it's expected, it causes problems in the editor, because too big chunks make editing super slow. Is there no way to avoid re-creating shapes in the editor? Maybe only do that when they are visible? Though it's easily solvable with a script that makes chunks bigger at runtime, so not much a problem.

Hmm, I don't think it would be easily solvable. Modifying the TileSet kind of has to re-generate the whole TileMap. I guess it could be optimized but that's sadly a toooon of work. So for now I don't really see a good solution for the problem.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 10, 2025

Modifying the TileSet kind of has to re-generate the whole TileMap.

I meant modifying the TileMap itself. When you paint or erase tiles, it seems to re-create modified physics chunks for no reason.

@groud
Copy link
Member Author

groud commented Feb 10, 2025

I meant modifying the TileMap itself. When you paint or erase tiles, it seems to re-create physics chunks for no reason.

Oh ok. Well yeah, it updates chunk by chunk, like for rendering. So indeed changing a tile in a given chunk re-renders the whole chunk. Not sure it could be optimized more though.

Maybe I could limit the chunk size in the editor, but It might hurt debugging I guess.

In any case, the polygon merging can maybe be optimized a bit, I need to check.

@djrain
Copy link

djrain commented Feb 10, 2025

This would be great to have for light occluders also. I can say that merging tiles greatly reduced the impact of shadows in our game in 4.3. Clay has made some shadow performance improvements in 4.4 that I've yet to test, but I'm guessing the chunking will still help.

@groud
Copy link
Member Author

groud commented Feb 10, 2025

This would be great to have for light occluders also. I can say that merging tiles greatly reduced the impact of shadows in our game in 4.3. Clay has made some shadow performance improvements in 4.4 that I've yet to test, but I'm guessing the chunking will still help.

That's a good point. It should be doable.

@smix8
Copy link
Contributor

smix8 commented Feb 10, 2025

It is definitely an improvement over the status quo.

Could not confirm fixing seams (#89458) because it's too unreliable, but it's safe to assume it's fixed too.

No full fix to physics ghost collision. This technically can not fix physics seam issues as when you bake it to chunks you still have chunk seams. For physics to really remove all those internal edge ghost collision issues it would need to merge all cells together that create a combined shape instead of slicing them to chunk size.

I don't think we can keep the debug lines, as it has a performance impact.

Change from rendering the debug with the very inefficient canvas item polyline function to bake a chunk mesh to render, it is dirt cheap in comparison.

@groud
Copy link
Member Author

groud commented Feb 10, 2025

Change from rendering the debug with the very inefficient canvas item polyline function to bake a chunk mesh to render, it is dirt cheap in comparison.

Hmm, interesting idea. IIRC, I had issues with 2D meshes back in the days, but maybe it got better? I'll try it out.

@smix8
Copy link
Contributor

smix8 commented Feb 10, 2025

Maybe there were issues but by now the 2d navmesh, csg and other places with a lot of edge debug use it already with no reported issues so I think it should not be a problem for TileMap to adopt it as well to improve performance.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 10, 2025

This technically can not fix physics seam issues as when you bake it to chunks you still have chunk seams.

You can set chunk size to cover the whole TileMap. I checked my biggest one, chunk size 512 and works without problems. It does not impact loading time, but modifying the tiles is slower (luckily I don't need to do it).

@smix8
Copy link
Contributor

smix8 commented Feb 10, 2025

You can set chunk size to cover the whole TileMap.

Ok I did not thought about that option, just assumed there would be a limit of quadrant size of sorts, yeah that could work.

@groud
Copy link
Member Author

groud commented Feb 10, 2025

I'm thinking about something. Maybe I could try implementing some sort of an automatic "edge smoothing" ?
edgesmoothing

Like, a 45° on the edge of each chunk's polygon might help avoiding characters being stuck when sliding? I guess we gotta try by making this change in tile first maybe. By like, adding a 1px "chamfer" and if there still are issues.

@Calinou
Copy link
Member

Calinou commented Feb 10, 2025

You can set chunk size to cover the whole TileMap. I checked my biggest one, chunk size 512 and works without problems. It does not impact loading time, but modifying the tiles is slower (luckily I don't need to do it).

I'm wondering if we should improve the usability of huge chunk sizes by limiting their size in the editor, so that you can use one in the project and not suffer from slow editing speeds.

On the other hand, I know it's been discouraged in the past to have huge collision shapes spanning the whole level, as each dynamic object will have to test collisions against the whole shape regardless of its position. This is particularly the case in 3D where trimesh collision is generally much more complex than tile collision in 2D.

I'm thinking about something. Maybe I could try implementing some sort of an automatic "edge smoothing" ?

I think this is better solved at a character controller level by implementing skimming, so that you don't need to make the collision shapes more complex.

At a core level, skimming refers to preserving the character's velocity when hitting something for a short amount of time. This means that you will keep sliding until you no longer hit the surface, and as soon as you are no longer colliding, you will keep the velocity you've had before hitting the surface. You could see it as a reverse coyote time of sorts.

Skimming as a concept has been used in both 2D and 3D games, with varying implementations (some being more generous than others).

Note that regardless of this, I don't think skimming (or adding chamfers to collision shapes) this resolves the issue of characters bumping when sliding on the ground. One way to minimize the issue is to disable gravity while on a flat surface.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

Testing project: test_tilemap_large.zip

Physics seams issues

Holding left arrow in the testing project's main scene.

Before

rolling_ball_before.mp4

After

Physics seams issues still occur occasionally, but are much rarer and less severe.

rolling_ball_after.mp4

After (physics quadrant size set to 2^31 - 1)

This fully avoids the issue, but makes editing slower in the editor and may have adverse performance issues in large scenes (due to each object having to check collision against the entire level's shape).

rolling_ball_after_max_quadrant.mp4

Performance

PC specifications
  • CPU: Intel Core i9-13900K
  • GPU: NVIDIA GeForce RTX 4090
  • RAM: 64 GB (2×32 GB DDR5-5800 C30)
  • SSD: Solidigm P44 Pro 2 TB
  • OS: Linux (Fedora 41)

Command line used for testing (we render in a tiny window to avoid GPU bottlenecks1):

godot --path /path/to/project lots_of_tiles.tscn --resolution 64x64 --print-fps

1000 RigidBodies enclosed in a complex TileMap.

Before

We encounter a "physics spiral of death" phenomenon, hence the sudden decrease in FPS at some point.

Project FPS: 391 (2.55 mspf)
Project FPS: 577 (1.73 mspf)
Project FPS: 11 (90.90 mspf)
Project FPS: 8 (125.00 mspf)
Project FPS: 60 (16.66 mspf)
Project FPS: 9 (111.11 mspf)
Project FPS: 20 (50.00 mspf)
Project FPS: 590 (1.69 mspf)
Project FPS: 328 (3.04 mspf)
Project FPS: 399 (2.50 mspf)
Project FPS: 442 (2.26 mspf)

After

No more physics spiral of death, and overall performance is greatly improved.

Project FPS: 837 (1.19 mspf)
Project FPS: 488 (2.04 mspf)
Project FPS: 698 (1.43 mspf)
Project FPS: 876 (1.14 mspf)
Project FPS: 900 (1.11 mspf)
Project FPS: 872 (1.14 mspf)
Project FPS: 880 (1.13 mspf)
Project FPS: 649 (1.54 mspf)

Feedback

  • The property should be documented in the class reference, and it should have a property hint added to set a suitable range. I'd suggest something like 1,1024,1 as sizes above 1024 will likely run into the aforementioned performance issue, on top of being very slow for editing.
  • We could sidestep the editing performance problem at large quadrant sizes by clamping the size of the physics quadrant while in the editor to a value like 16 or 32.

Footnotes

  1. --disable-render-loop breaks --print-fps reporting, so we can't use it to test physics performance.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 10, 2025

it should have a property hint added to set a suitable range. I'd suggest something like 1,1024,1 as sizes above 1024 will likely run into the aforementioned performance issue

That depends. The chunk size is square, while the TileMap can be wide. My widest level is 128x1664, which is much smaller than 1024x1024, but with size limit it couldn't be covered by single chunk. Though with or_greater it would be fine.

The slow editor editing should be fixed not circumvented. As I said, physics chunks don't need to be created in the editor if the shapes are not visible.

@smix8
Copy link
Contributor

smix8 commented Feb 10, 2025

For editing performance could just do as the navmesh baking does. The navmesh editing has a timer that triggers and sets the time on any editing. So only when the user stops editing the timer fires and the navmesh gets rebaked. Could work the same for this collision bake.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 10, 2025

Ah well, the colors are a bit randomized (to distinguish shapes a bit), but it is done per-chunk. So well, the first shape always use the same color so you cannot distinguish chunks.

From what I see, you randomize colors once per chunk, so each chunk should have unique color. But:

  • rand.random returns the same value for whatever reason.
  • The color stays unchanged anyway, even if randomizing the same number should change it (because you are adding to previous color).
  • You are using (-1, 1) range for color randomization, which can eventually go out of bounds. Not sure if set_hsv() can handle it.

This is what I got by changing rand.random() to Matf::randf() (and tweaking some values):
image
You could distinguish both chunks and shapes by randomizing hue for chunks and value for shapes.

Comment on lines +3154 to +3196
if (physics_quadrant_size == p_size) {
return;
}
ERR_FAIL_COND_MSG(p_size < 1, "Physics quandrant size cannot be smaller than 1.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually I've seen error checks before guards, but not sure if it's consistent.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 10, 2025

Note this PR breaks compatibility for get_coords_for_body_rid, as with the PR, a single body can cover multiple cells.

This can be worked around with (

var pos := tile_map_layer.to_local(col.get_position())
var coords := tile_map_layer.local_to_map(pos)

but I just remembered that it wasn't very reliable. The position would some times report outside any tile. (I didn't test if it's still the case)

@groud
Copy link
Member Author

groud commented Feb 11, 2025

rand.random returns the same value for whatever reason.

Yeah. Because rand is initialialized every time, and with the same seed, within _physics_draw_quadrant_debug (ran for each quadrant).
I'll generate the seed from the quadrant coords, that should be enough.

The color stays unchanged anyway, even if randomizing the same number should change it (because you are adding to previous color).

I'm not sure, it worked for me within a quadrant.

You are using (-1, 1) range for color randomization, which can eventually go out of bounds. Not sure if set_hsv() can handle it.

Well, the returned value is multiplied by 0.05 or 0.1, so well, it's not that wide of a range. The hue parameter goes through fmod so it should be fine. There's only the value one that might go below 0 but I'm not sure if that's possible.

This is what I got by changing rand.random() to Matf::randf() (and tweaking some values):

I don't want something that spread, it should stay around the selected debug color.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 11, 2025

rand is initialialized every time, and with the same seed, within _physics_draw_quadrant_debug (ran for each quadrant).

Ok I misunderstood what the method does. Makes sense.

@groud groud force-pushed the chunk_tilemap_physics branch from 57daef3 to 0bce0c8 Compare February 11, 2025 14:38
@groud groud marked this pull request as ready for review February 11, 2025 14:43
@groud groud requested review from a team as code owners February 11, 2025 14:43
@groud groud force-pushed the chunk_tilemap_physics branch from 0bce0c8 to 3327d93 Compare February 11, 2025 14:44
@KoBeWi
Copy link
Member

KoBeWi commented Feb 11, 2025

You need to run doctool.

@groud groud force-pushed the chunk_tilemap_physics branch from 3327d93 to 287344c Compare February 11, 2025 15:07
@groud groud requested a review from a team as a code owner February 11, 2025 15:07
@groud groud force-pushed the chunk_tilemap_physics branch from 287344c to ac91ada Compare February 11, 2025 15:16
@groud groud force-pushed the chunk_tilemap_physics branch from ac91ada to 4765bc8 Compare February 11, 2025 15:29
@groud
Copy link
Member Author

groud commented Feb 11, 2025

Alright, thanks. It should be ready now. Hope I didn't mess anything else 😅

@KoBeWi
Copy link
Member

KoBeWi commented Feb 11, 2025

Some generated shapes don't look optimal 🤔
image
The slope could be a single triangle, but it got heavily segmented (assuming the outlines are accurate). Though it's probably limitation of the algorithm. Either case, it has no visible effect on performance.

@groud
Copy link
Member Author

groud commented Feb 11, 2025

I am not super sure. Maybe there are some precision errors too. Is there any visible gap if you zoom in a lot ?

But yeah, the concave to convex algorithm might have some limitations too. Though I would expect it to be smart enough to merge the triangles there. Weird...

@KoBeWi
Copy link
Member

KoBeWi commented Feb 11, 2025

Ok I guess that's the reason
image

EDIT:
I fixed the shape and it looks good now:
image

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quadrant preview looks great now.
Editing with big quadrants is still slow, but it's easy to workaround, so whateves. Also I only tested dev build, in optimized build it's probably not noticeable.

@KoBeWi KoBeWi modified the milestones: 4.x, 4.5 Feb 11, 2025
@groud
Copy link
Member Author

groud commented Feb 11, 2025

Hmm, if you want to try it out there's a solution = SimplifyPaths(solution, 0.01); line in the geometry_2d.cpp. You may try to increase the 0.01 to something higher, maybe it can help too. If your shape isn't, like, wrongly configured I mean.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 11, 2025

Yeah the tile shape was wrong. I made it before grid snap was a thing and it wasn't aligned properly (slopes spanning over 4 tiles are tricky).

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.

Physics bodies can collide with tile seams Big TileMap with collisions is extremely slow
6 participants