Divide the single VisibleEntities list into separate lists for 2D meshes, 3D meshes, lights, and UI elements, for performance.
#12582
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
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.
alice-i-cecile
added
A-Rendering
|
How do UI entities fit into this categorization? |
alice-i-cecile
added
the
M-Needs-Migration-Guide
|
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.) |
|
I added UI entities to the hash map. |
pcwalton
changed the title
VisibleEntities list into separate lists for 2D meshes, 3D meshes, and lights, for performance.VisibleEntities list into separate lists for 2D meshes, 3D meshes, lights, and UI elements, for performance.
NthTensor
reviewed
Apr 1, 2024
NthTensor
approved these changes
Apr 1, 2024
|
Addressed review comments and merge conflicts. |
kristoff3r
approved these changes
Apr 11, 2024
alice-i-cecile
added
the
S-Ready-For-Final-Review
|
@pcwalton once merge conflicts are resolved I'll merge this. The doc improvements mentioned by @kristoff3r would be great too. |
|
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
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`.
|
The migration guide here was a little confusing -- at first, it seemed that I would have to implement my own .add_systems(PostUpdate, (
check_visibility::<With<MyCustomRenderable>>,
)
.in_set(VisibilitySystems::CheckVisibility))Maybe this could be added to the migration guide? |
ChristopherBiscardi
added a commit
to ChristopherBiscardi/bevy_ecs_tilemap
that referenced
this pull request
Jun 7, 2024
[12582](bevyengine/bevy#12582) divided `VisibleEntities` into separate lists. So now we have to specify which kind of entity we want. I think we want the Mesh here, and I think we can get rid of the `.index` calls on Entity since Entity [already compares bits](https://docs.rs/bevy_ecs/0.14.0-rc.2/src/bevy_ecs/entity/mod.rs.html#173) for optimized codegen purposes. Waiting to do that until the other changes are in though so as to not change functionality until post-upgrade.
ChristopherBiscardi
added a commit
to ChristopherBiscardi/bevy_ecs_tilemap
that referenced
this pull request
Jun 7, 2024
As a result of [12582](bevyengine/bevy#12582) `check_visibility` must be implemented for the "renderable" tilemap entities. Doing this is trivial by taking advantage of the existing `check_visibility` type arguments, which accept a [`QF: QueryFilter + 'static`](https://docs.rs/bevy/0.14.0-rc.2/bevy/render/view/fn.check_visibility.html). The same `QueryFilter`` is used when checking `VisibleEntities`. I've chosen `With<TilemapRenderSettings` because presumably if the entity doesn't have a `TilemapRenderSettings` then it will not be rendering, but this could be as sophisticated or simple as we want. For example `WithLight` is currently implemented as ```rust pub type WithLight = Or<(With<PointLight>, With<SpotLight>, With<DirectionalLight>)>; ```
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 17, 2024
# Objective - After #12582 , Bevy split visibleEntities into a TypeIdMap for different types of entities, but the behavior in `check_light_mesh_visibility `simply calls HashMap::clear(), which will reallocate memory every frame. ## Testing cargo run --release --example many_cubes --features bevy/trace_tracy -- --shadows ~10% win in `check_light_mesh_visibilty` 
rparrett
added a commit
to StarArawn/bevy_ecs_tilemap
that referenced
this pull request
Jul 5, 2024
* Update to 0.14.0-rc.2 * [12997](bevyengine/bevy#12997): rename `multi-threaded` to `multi_threaded` * RenderAssets<Image> is now RenderAssets<GpuImage> Implemented in [12827](bevyengine/bevy#12827) * FloatOrd is now in bevy_math implemented in [12732](bevyengine/bevy#12732) * convert Transparent2d::dynamic_offset to extra_index [12889](bevyengine/bevy#12889) Gpu Frustum Culling removed the dynamic_offset of Transparent2d and it became `extra_index` with the special value `PhaseItemExtraIndex::NONE`, which indicates the `None` that was here previously * RenderPhase<Transparent2d> -> ViewSortedRenderPhases<Transparent2d> [12453](https://github.com/StarArawn/bevy_ecs_tilemap/pull/bevyengine/bevy#12453): Render phases are now binned or sorted. Following the changes in the `mesh2d_manual` [example](https://github.com/bevyengine/bevy/blob/ecdd1624f302c5f71aaed95b0984cbbecf8880b7/examples/2d/mesh2d_manual.rs#L357-L358): use the `ViewSortedRenderPhases` resource. * get_sub_app_mut is now an Option in [9202](https://github.com/StarArawn/bevy_ecs_tilemap/pull/bevyengine/bevy/pull/9202) SubApp access has changed * GpuImage::size f32 -> u32 via UVec2 [11698](bevyengine/bevy#11698) changed `GpuImage::size` to `UVec2`. Right above this, `Extent3d` does the same thing, so I'm taking a small leap and assuming can `as`. * GpuMesh::primitive_topology -> key_bits/BaseMeshPipeline [12791](bevyengine/bevy#12791) the `primitive_topology` field on `GpuMesh` was removed in favor of `key_bits` which can be constructed using `BaseMeshPipeline::from_primitive_topology` * RenderChunk2d::prepare requires &mut MeshVertexBufferLayouts now [12216](bevyengine/bevy#12216) introduced an argument `&mut MeshVertexBufferLayouts` to `get_mesh_vertex_buffer_layout`, which bevy_ecs_tilemap calls in `RenderChunk2d::prepare` * into_linear_f32 -> color.0.linear().to_f32_array(), [12163](bevyengine/bevy#12163) bevy_color was created and Color handling has changed. Specifically Color::as_linear_rgba_f32 has been removed. LinearRgba is now its own type that can be accessed via [`linear()`](https://docs.rs/bevy/0.14.0-rc.2/bevy/color/enum.Color.html#method.linear) and then converted. * Must specify type of VisibleEntities when accessing [12582](bevyengine/bevy#12582) divided `VisibleEntities` into separate lists. So now we have to specify which kind of entity we want. I think we want the Mesh here, and I think we can get rid of the `.index` calls on Entity since Entity [already compares bits](https://docs.rs/bevy_ecs/0.14.0-rc.2/src/bevy_ecs/entity/mod.rs.html#173) for optimized codegen purposes. Waiting to do that until the other changes are in though so as to not change functionality until post-upgrade. * app.world access is functions now - [9202](bevyengine/bevy#9202) changed world access to functions. [relevent line](https://github.com/bevyengine/bevy/pull/9202/files#diff-b2fba3a0c86e496085ce7f0e3f1de5960cb754c7d215ed0f087aa556e529f97fR640) - This also surfaced [12655](bevyengine/bevy#12655) which removed `Into<AssetId<T>>` for `Handle<T>`. using a reference or .id() is the solution here. * We don't need `World::cell`, and it doesn't exist anymore In [12551](bevyengine/bevy#12551) `WorldCell` was removed. ...but it turns out we don't need it or its replacement anyway. * examples error out unless this bevy bug is addressed with these features being added bevyengine/bevy#13728 * check_visibility is required for the entity that is renderable As a result of [12582](bevyengine/bevy#12582) `check_visibility` must be implemented for the "renderable" tilemap entities. Doing this is trivial by taking advantage of the existing `check_visibility` type arguments, which accept a [`QF: QueryFilter + 'static`](https://docs.rs/bevy/0.14.0-rc.2/bevy/render/view/fn.check_visibility.html). The same `QueryFilter`` is used when checking `VisibleEntities`. I've chosen `With<TilemapRenderSettings` because presumably if the entity doesn't have a `TilemapRenderSettings` then it will not be rendering, but this could be as sophisticated or simple as we want. For example `WithLight` is currently implemented as ```rust pub type WithLight = Or<(With<PointLight>, With<SpotLight>, With<DirectionalLight>)>; ``` * view.view_proj -> view.clip_from_world [13289](bevyengine/bevy#13489) introduced matrix naming changes, including `view_proj` which becomes `clip_from_world` * color changes to make tests runnable * clippy fix * Update Cargo.toml Co-authored-by: Rob Parrett <robparrett@gmail.com> * Update Cargo.toml Co-authored-by: Rob Parrett <robparrett@gmail.com> * final clippy fixes * Update Cargo.toml Co-authored-by: Rob Parrett <robparrett@gmail.com> * Simplify async loading in ldtk/tiled helpers See Bevy #12550 * remove second allow lint * rc.3 bump * bump version for major release * remove unused features --------- Co-authored-by: Rob Parrett <robparrett@gmail.com>
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 splits
VisibleEntities::entitiesinto four separate lists: one for lights, one for 2D meshes, one for 3D meshes, and one for UI elements. This allowsqueue_material_meshesand 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::entitiesis aHashMapfrom theTypeIdrepresenting aQueryFilterto the appropriateEntitylist. I had to do this becauseVisibleEntitiesis 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 specializedcheck_visibilityto the schedule.This provides a 16.23% end-to-end speedup on
many_foxeswith 10,000 foxes (24.06 ms/frame to 20.70 ms/frame).Migration guide
check_visibilityandVisibleEntitiesnow store the four types of renderable entities--2D meshes, 3D meshes, lights, and UI elements--separately. If your custom rendering code examinesVisibleEntities, it will now need to specify which type of entity it's interested in using theWithMesh2d,WithMesh,WithLight, andWithNodetypes respectively. If your app introduces a new type of renderable entity, you'll need to add an explicit call tocheck_visibilityto the schedule to accommodate your new component or components.Analysis
many_foxes, 10,000 foxes:main:many_foxes, 10,000 foxes, this branch:queue_material_meshes(yellow = this branch, red =main):queue_shadows(yellow = this branch, red =main):