Allocate light data UBO from per-frame allocator, misc. fixes and cleanup #5970
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Instead of maintaining a separate uniform buffer for light data, allocate it from the per-frame temp allocator. - Light data is typically updated one to several times per frame, making it a good candidate for temp buffer backing. - This avoids an early serialization point, where we were attempting to modify the single light data buffer while it may have been in-use by the previously-submitted frame.
4 tasks
fluffyfreak
approved these changes
Nov 17, 2024
|
Also as tested on Windows this all works 👍 |
- The HashTable implementation expected that its memory would be externally zero-initialized before the constructor is called - Since HashTable stores a raw pointer to the backing memory and allocates on construction, making a temporary copy of ZoneDumper in dumpzones() lead to a double-delete
|
Significant performance improvement on the RPi5, at least as good as #5972 so I'll close that one 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another general collection of "fix" commits.
This PR adds additional profiling information to the renderer, and resolves an "early serialization" point which required the previous frame's rendering to be ~mostly complete before starting to record commands for the current frame.
I don't expect any significant performance increase from this PR, except in the case of extremely GPU-bound devices where you might see an improvement of up to a few milliseconds in frame-time, as more CPU work is able to be overlapped with the previous frame's GPU execution time.
This change does fully enforce the existing invariant that you must call
Renderer::SetLights()before attempting to record a draw command using a lit material. An assertion will be thrown in debug mode if this invariant is broken.It also fixes #5940 - turned out to be quite a simple one-liner once I realized what the problem was.