Skip to content

Conversation

@ManevilleF
Copy link
Contributor

@ManevilleF ManevilleF commented Jun 16, 2022

Objective

Following #4489 by @aevyrie I think we can improve the batching:

  • Making it optional* but enabled by default
  • Making the batch size customizable
  • Propagating the batching to other bevy_render systems

Solution

I'm opening this Draft showing how I think we can apply the target improvements:

  • The batch size can be enabled or disabled directly through the RenderPlugin where I added an optional field, set by default to 1024 like in [Merged by Bors] - Parallel Frustum Culling #4489 .
  • The systems retrieve the batch size through an optional resource, meaning we can add, edit or remove this resource at any time, making it customizable and dynamic.

Notes

  • The code is not very pretty but I think there are ways to improve it or maybe make the whole conditional parallelization generic and therefore easily appliable to other systems in bevy_render or maybe other crates.
  • I didn't test or benchmark the new code yet, since I don't think this MR should be merged before the design is discussed

@ManevilleF ManevilleF marked this pull request as ready for review June 16, 2022 09:41
@ManevilleF
Copy link
Contributor Author

An other design choice could be that RenderBatchSize is always present and set through a App::init_resource() but storing an Option<usize> for the batch_size.
this would mean that it could be retrieved through Res<> instead of Option<Res<>> and maybe add clarity

@james7132
Copy link
Member

Wouldn't #4777 largely remove the need for this?

@ManevilleF
Copy link
Contributor Author

ManevilleF commented Jun 16, 2022

Wouldn't #4777 largely remove the need for this?

Absolutely, I didn't know it existed but if the batch size can be dynamically estimated I don't think there is need for a config resource. Or maybe the resource can have like a Auto value, using the estimate, and an Override value allowing more customization

Edit: Maybe I should base this PR off yours
Edit 2: Maybe the batch size estimation could be overriden using a global resource, defining the global paralelization behaviour ?

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 20, 2022
@ManevilleF
Copy link
Contributor Author

#4777 and #4663 make this obsolete

@ManevilleF ManevilleF closed this Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants