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

Divide the single VisibleEntities list into separate lists for 2D meshes, 3D meshes, lights, and UI elements, for performance. #12582

Merged
merged 16 commits into from Apr 11, 2024

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Mar 19, 2024

This commit splits VisibleEntities::entities into four separate lists: one for lights, one for 2D meshes, one for 3D meshes, and one for UI elements. This allows queue_material_meshes and similar methods to avoid examining entities that are obviously irrelevant. In particular, this separation helps scenes with many skinned meshes, as the individual bones are considered visible entities but have no rendered appearance.

Internally, VisibleEntities::entities is a HashMap from the TypeId representing a QueryFilter to the appropriate Entity list. I had to do this because VisibleEntities is located within an upstream crate from the crates that provide lights (bevy_pbr) and 2D meshes (bevy_sprite). As an added benefit, this setup allows apps to provide their own types of renderable components, by simply adding a specialized check_visibility to the schedule.

This provides a 16.23% end-to-end speedup on many_foxes with 10,000 foxes (24.06 ms/frame to 20.70 ms/frame).

Migration guide

  • check_visibility and VisibleEntities now store the four types of renderable entities--2D meshes, 3D meshes, lights, and UI elements--separately. If your custom rendering code examines VisibleEntities, it will now need to specify which type of entity it's interested in using the WithMesh2d, WithMesh, WithLight, and WithNode types respectively. If your app introduces a new type of renderable entity, you'll need to add an explicit call to check_visibility to the schedule to accommodate your new component or components.

Analysis

many_foxes, 10,000 foxes: main:
Screenshot 2024-03-31 114444

many_foxes, 10,000 foxes, this branch:
Screenshot 2024-03-31 114256

queue_material_meshes (yellow = this branch, red = main):
Screenshot 2024-03-31 114637

queue_shadows (yellow = this branch, red = main):
Screenshot 2024-03-31 114607

meshes, 3D meshes, and lights, for performance.

This commit splits `VisibleEntities::entities` into three separate
lists: one for lights, one for 2D meshes, and one for 3D meshes. This
allows `queue_material_meshes` and similar methods to avoid examining
entities that are obviously irrelevant. In particular, this separation
helps scenes with many skinned meshes, as the individual bones are
considered visible entities but have no rendered appearance.

Internally, `VisibleEntities::entities` is a `HashMap` from the `TypeId`
representing a `QueryFilter` to the appropriate `Entity` list. I had to
do this because `VisibleEntities` is located within an upstream crate
from the crates that provide lights (`bevy_pbr`) and 2D meshes
(`bevy_sprite`). As an added benefit, this setup allows apps to provide
their own types of renderable components, by simply adding a specialized
`check_visibility` to the schedule.

Currently, this provides minimal performance benefit in `many_foxes`,
but on top of binning (bevyengine#12453) it provides a 20% end-to-end speedup on
that benchmark.

* `check_visibility` and `VisibleEntities` now store the three types of
  renderable entities--2D meshes, 3D meshes, and lights--separately. If
  your custom rendering code examines `VisibleEntities`, it will now
  need to specify which type of entity it's interested in using the
  `WithMesh2d`, `WithMesh`, and `WithLight` types respectively. If your
  app introduces a new type of renderable entity, you'll need to add an
  explicit call to `check_visibility` to the schedule to accommodate
  your new component or components.
@pcwalton pcwalton requested a review from superdump March 19, 2024 21:00
@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 labels Mar 19, 2024
@alice-i-cecile
Copy link
Member

How do UI entities fit into this categorization?

@alice-i-cecile alice-i-cecile added the C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide label Mar 19, 2024
@pcwalton
Copy link
Contributor Author

They should be 2D meshes, but I'll check. If they aren't handled then I can just add them to the HashMap. (This is why I made it extensible.)

@pcwalton
Copy link
Contributor Author

I added UI entities to the hash map.

@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Mar 31, 2024
@pcwalton pcwalton requested a review from james7132 March 31, 2024 18:36
@pcwalton pcwalton changed the title Divide the single VisibleEntities list into separate lists for 2D meshes, 3D meshes, and lights, for performance. Divide the single VisibleEntities list into separate lists for 2D meshes, 3D meshes, lights, and UI elements, for performance. Mar 31, 2024
Copy link
Contributor

@NthTensor NthTensor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love how extensible this is. Profiled on my machine and found a similar speedup.

@pcwalton
Copy link
Contributor Author

pcwalton commented Apr 1, 2024

Addressed review comments and merge conflicts.

Copy link
Contributor

@kristoff3r kristoff3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, it makes a ton of sense to split these up per type, and I'm a big fan of generic systems like this.

crates/bevy_render/src/view/visibility/mod.rs Outdated Show resolved Hide resolved
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Apr 11, 2024
@alice-i-cecile
Copy link
Member

@pcwalton once merge conflicts are resolved I'll merge this. The doc improvements mentioned by @kristoff3r would be great too.

@pcwalton
Copy link
Contributor Author

pcwalton commented Apr 11, 2024

Done. @alice-i-cecile

auto-merge was automatically disabled April 11, 2024 19:57

Head branch was pushed to by a user without write access

@superdump superdump added this pull request to the merge queue Apr 11, 2024
Merged via the queue into bevyengine:main with commit 5caf085 Apr 11, 2024
27 checks passed
pcwalton added a commit to pcwalton/bevy that referenced this pull request Apr 12, 2024
`Sprite`, `Text`, and `Handle<MeshletMesh>` were types of renderable
entities that the new segregated visible entity system didn't handle, so
they didn't appear.

Because `bevy_text` depends on `bevy_sprite`, and the visibility
computation of text happens in the latter crate, I had to introduce a
new marker component, `SpriteSource`. `SpriteSource` marks entities that
aren't themselves sprites but become sprites during rendering. I added
this component to `Text2dBundle`. Unfortunately, this is technically a
breaking change, although I suspect it won't break anybody in practice
except perhaps editors.
github-merge-queue bot pushed a commit that referenced this pull request Apr 13, 2024
`Sprite`, `Text`, and `Handle<MeshletMesh>` were types of renderable
entities that the new segregated visible entity system didn't handle, so
they didn't appear.

Because `bevy_text` depends on `bevy_sprite`, and the visibility
computation of text happens in the latter crate, I had to introduce a
new marker component, `SpriteSource`. `SpriteSource` marks entities that
aren't themselves sprites but become sprites during rendering. I added
this component to `Text2dBundle`. Unfortunately, this is technically a
breaking change, although I suspect it won't break anybody in practice
except perhaps editors.

Fixes #12935.

## Changelog

### Changed

* `Text2dBundle` now includes a new marker component, `SpriteSource`.
Bevy uses this internally to optimize visibility calculation.

## Migration Guide

* `Text` now requires a `SpriteSource` marker component in order to
appear. This component has been added to `Text2dBundle`.
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-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants