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.

Sign up for GitHub

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

Jump to bottom

Add a PrepareBatches render set that runs after PrepareResources. #10234

Closed
wants to merge 1 commit into from
Closed

Add a PrepareBatches render set that runs after PrepareResources. #10234

wants to merge 1 commit into from

Conversation

pcwalton
Copy link
Contributor

Objective

PR #10231 needs to add some logic that runs as part of PrepareResources but before batch building, because it creates data that batch building needs (the lightmap index, which is part of the Mesh structure).

Solution

This PR adds a new render set for the batching operations. It was split out of #10231 at reviewer request.

@pcwalton
Add a PrepareBatches render set that runs after PrepareResources.
2568518 
PR bevyengine#10231 needs to add some logic that runs as part of
`PrepareResources` but before batch building, because it creates data
that batch building needs. This commit adds a new render set for this.
It was split out of bevyengine#10231 at reviewer request.
@pcwalton pcwalton mentioned this pull request Oct 23, 2023
Merged
@superdump
Copy link
Contributor

Hmm, I’ll need to look more closely at this. It’s not obvious if it’s necessary / useful to add another render set at a glance. It isn’t harmful for sure and maybe it is indeed useful for specifying dependencies more simply for your case.

@superdump superdump requested a review from robtfm October 23, 2023 06:59
@pcwalton
Copy link
Contributor Author

@superdump Look at the interaction between https://github.com/bevyengine/bevy/pull/10231/files#diff-cbd1eecdb5d976e19535f4a9afc4d7cf545c4e4648c58eadecc1880f8304929aR160-R202 and https://github.com/bevyengine/bevy/pull/10231/files#diff-4bf3ed03d4129aad9f5678ba19f9b14ee8e3e61d6f6365e82197b01c74468b10R472-R484. MeshPipeline::get_batch_data depends on data populated by build_lightmap_texture_arrays, so build_lightmap_texture_arrays needs to run first. If there's another way you'd prefer me to do this, let me know.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Oct 23, 2023
@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Oct 23, 2023
@pcwalton
Copy link
Contributor Author

Closed as this is no longer blocking anything.

@pcwalton pcwalton closed this Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@superdump superdump Awaiting requested review from superdump

@nicopap nicopap Awaiting requested review from nicopap

@robtfm robtfm Awaiting requested review from robtfm

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Milestone
0.13
Development

Successfully merging this pull request may close these issues.
3 participants
@pcwalton @superdump @alice-i-cecile