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

[3.x] Faster editor grid #92725

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Conversation

lawnjelly
Copy link
Member

Use static LocalVectors instead of PoolVectors for temporaries.

Notes

  • I noticed _init_grid() taking a long time in some editor profiles when moving the 3D viewport (40% CPU).
  • _init_grid() (aside from the renderer churn) is currently bottlenecked by PoolVector allocations and thread safety during push_back().
  • This makes _init_grid() approx 8x faster (not very rigorous, based on profiles, but certainly a lot faster), based on a pretty simple change, so why not.

Use static `LocalVectors` instead of `PoolVectors` for temporaries.
@Calinou
Copy link
Member

Calinou commented Jun 3, 2024

Sounds great – this should help reduce CPU bottlenecks and improve battery life when working in 3D. Is a similar optimization possible in 4.x?

@lawnjelly
Copy link
Member Author

Is a similar optimization possible in 4.x?

It may not be as bad there, as it is using Vector as opposed to PoolVector, but it should also be faster with static LocalVectors. I'll see if I can do a PR for 4.x.

The other factor with the grid is the rendering churn (which I'm sure we've mentioned as an inefficiency before), it should be using dynamic VBs for this kind of thing and I have a feeling it's recreating VBs each frame, but it's a while since I looked at that.

@lawnjelly lawnjelly merged commit 72aac81 into godotengine:3.x Jun 13, 2024
14 checks passed
@lawnjelly lawnjelly deleted the faster_editor_grid branch June 13, 2024 07:49
@lawnjelly
Copy link
Member Author

Thanks!

@akien-mga akien-mga modified the milestones: 3.x, 3.6 Jun 13, 2024
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.

4 participants