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

Improve tilemap performance by using quadrants only for rendering #74603

Closed
wants to merge 1 commit into from

Conversation

groud
Copy link
Member

@groud groud commented Mar 8, 2023

This PR is a huge refactor of the TileMap internals. It makes it so any update does not have to update a full quadrant for all systems (physics, navigation, etc...) but only for the rendering part. It should help significantly reduce the slowdowns in the editor.

This PR does not do it right now, but we could probably revert what does: #67330. This would simplify the code a bit, and since a cell would only be modified if it was actually changed, the original issue would still be solved.

To be tested, but it should close #72405

@smix8
Copy link
Contributor

smix8 commented Mar 8, 2023

If I read this pr correctly you are removing the quadrant entirely for everything that is not rendering.

I am not convinced that this is a good idea.

While this may mitigate some performance cost for updating single cells it also basically voiding chances of fixing physics collision and navigation mesh performance and quality issues. Those issues are not the result of quadrant update costs but from having so many splintered and small cells in the first place. For merging collision and navigation polygons an in-between update unit like quadrants is still needed.

@groud
Copy link
Member Author

groud commented Mar 8, 2023

While this may mitigate some performance cost for updating single cells it also basically voiding chances of fixing physics collision and navigation mesh performance and quality issues. Those issues are not the result of quadrant update costs but from having so many splintered and small cells in the first place. For merging collision and navigation polygons an in-between update unit like quadrants is still needed.

The idea would be to reintroduce per-system quadrants where needed. We can easily add some PhysicsQuadrant and NavigationQuadrant that can have different sizes from the RenderingQuadrant size. This needed for games with Y-sorting for example, as, right now, enabling Y-sorting forces the quadrant size to 1. Having different quadrant types and different associated size would greatly improve performances in that case.

@smix8
Copy link
Contributor

smix8 commented Mar 8, 2023

Guess that is fine too.

Does not have to be this pr but could you add those navigation "quadrants" as you see fit in a draft / pr.

I added the navigation polygon merging for TileMap quadrants in the NavigationMeshGenerator rework expecting the quadrants to be available, now I dont know how to remake that part with them removed.

@groud
Copy link
Member Author

groud commented Mar 8, 2023

Does not have to be this pr but could you add those navigation "quadrants" as you see fit in a draft / pr.

Yeah, sounds good! Once this one gets merged, I'll open a branch adding those other quadrant types, I guess you may rebase your work on top of it or something like that.

@groud groud force-pushed the improve_tilemap_performances branch 2 times, most recently from 2fe1cee to 341c62b Compare March 10, 2023 15:33
@grusad
Copy link

grusad commented Apr 25, 2023

Whats the status on this one? Will it be added on the next patch? Im the one opened this #72405 and im struggling bad with crashes/freezes all the time.

@groud
Copy link
Member Author

groud commented Apr 25, 2023

Whats the status on this one? Will it be added on the next patch? Im the one opened this #72405 and im struggling bad with crashes/freezes all the time.

I don't think so, it needs a lot of work and I lack time to do so. Maybe someone could help but that's a difficult problem to solve.

@grusad
Copy link

grusad commented Jun 6, 2023

thats unfortunate. Tiled just got an exporter for godot 4, i'll have to check that one out.

@groud groud force-pushed the improve_tilemap_performances branch from 341c62b to c67e477 Compare August 28, 2023 11:32
@groud groud requested a review from a team as a code owner August 28, 2023 11:32
@groud
Copy link
Member Author

groud commented Aug 28, 2023

I'll close this PR and open a new one, the changes are a bit different now and need more explanations.

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.

TileMap editor is slow with large tilemaps
4 participants