Refactor batch_and_prepare_binned_render_phase in preparation for bin retention.
#16922
+293
−138
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
retention. This commit makes the following changes: * `IndirectParametersBuffer` has been changed from a `BufferVec` to a `RawBufferVec`. This won about 20us or so on Bistro by avoiding `encase` overhead. * The methods on the `GetFullBatchData` trait no longer have the `entity` parameter, as it was unused. * `PreprocessWorkItem`, which specifies a transform-and-cull operation, now supplies the mesh instance uniform output index directly instead of having the shader look it up from the indirect draw parameters. Accordingly, the responsibility of writing the output index to the indirect draw parameters has been moved from the CPU to the GPU. This is in preparation for retained indirect instance draw commands, where the mesh instance uniform output index may change from frame to frame, while the indirect instance draw commands will be cached. We won't want the CPU to have to upload the same indirect draw parameters again and again if a batch didn't change from frame to frame. * `batch_and_prepare_binned_render_phase` and `batch_and_prepare_sorted_render_phase` now allocate indirect draw commands for an entire batch set at a time when possible, instead of one batch at a time. This change will allow us to retain the indirect draw commands for whole batch sets. * `GetFullBatchData::get_batch_indirect_parameters_index` has been replaced with `GetFullBatchData::write_batch_indirect_parameters`, which takes an offset and writes into it instead of allocating. This is necessary in order to use the optimization mentioned in the previous point. * At the WGSL level, `IndirectParameters` has been factored out into `mesh_preprocess_types.wgsl`. This is because we'll need a new compute shader that zeroes out the instance counts in preparation for a new frame. That shader will need to access `IndirectParameters`, so it was moved to a separate file. * Bins are no longer raw vectors but are instances of a separate type, `RenderBin`. This is so that the bin can eventually contain its retained batches.
pcwalton
added
C-Code-Quality
IceSentry
approved these changes
Dec 22, 2024
kristoff3r
approved these changes
Dec 22, 2024
Comment on lines
+13
to
+14
| // `first_vertex` in both structures. | ||
| first_vertex: u32, |
There was a problem hiding this comment.
This comment does not agree with the name or comments on the Rust struct, there it's first_vertex_or_first_index.
Comment on lines
+15
to
+16
| // `first_instance` or `base_vertex`. | ||
| first_instance_or_base_vertex: u32, |
There was a problem hiding this comment.
In the Rust struct it's called base_vertex_or_first_instance, both are fine but they should agree I think.
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.
This commit makes the following changes:
IndirectParametersBufferhas been changed from aBufferVecto aRawBufferVec. This won about 20us or so on Bistro by avoidingencaseoverhead.The methods on the
GetFullBatchDatatrait no longer have theentityparameter, as it was unused.PreprocessWorkItem, which specifies a transform-and-cull operation, now supplies the mesh instance uniform output index directly instead of having the shader look it up from the indirect draw parameters. Accordingly, the responsibility of writing the output index to the indirect draw parameters has been moved from the CPU to the GPU. This is in preparation for retained indirect instance draw commands, where the mesh instance uniform output index may change from frame to frame, while the indirect instance draw commands will be cached. We won't want the CPU to have to upload the same indirect draw parameters again and again if a batch didn't change from frame to frame.batch_and_prepare_binned_render_phaseandbatch_and_prepare_sorted_render_phasenow allocate indirect draw commands for an entire batch set at a time when possible, instead of one batch at a time. This change will allow us to retain the indirect draw commands for whole batch sets.GetFullBatchData::get_batch_indirect_parameters_indexhas been replaced withGetFullBatchData::write_batch_indirect_parameters, which takes an offset and writes into it instead of allocating. This is necessary in order to use the optimization mentioned in the previous point.At the WGSL level,
IndirectParametershas been factored out intomesh_preprocess_types.wgsl. This is because we'll need a new compute shader that zeroes out the instance counts in preparation for a new frame. That shader will need to accessIndirectParameters, so it was moved to a separate file.Bins are no longer raw vectors but are instances of a separate type,
RenderBin. This is so that the bin can eventually contain its retained batches.