From 04aedf12fad07f9f5b20246b7009501e0a0f4fa5 Mon Sep 17 00:00:00 2001 From: re0312 <45868716+re0312@users.noreply.github.com> Date: Sat, 20 Jan 2024 17:30:44 +0800 Subject: [PATCH] optimize batch_and_prepare_render_phase (#11323) # Objective - since #9685 ,bevy introduce automatic batching of draw commands, - `batch_and_prepare_render_phase` take the responsibility for batching `phaseItem`, - `GetBatchData` trait is used for indentify each phaseitem how to batch. it defines a associated type `Data `used for Query to fetch data from world. - however,the impl of `GetBatchData ` in bevy always set ` type Data=Entity` then we acually get following code `let entity:Entity =query.get(item.entity())` that cause unnecessary overhead . ## Solution - remove associated type `Data ` and `Filter` from `GetBatchData `, - change the type of the `query_item ` parameter in get_batch_data from` Self::Data` to `Entity`. - `batch_and_prepare_render_phase ` no longer takes a query using `F::Data, F::Filter` - `get_batch_data `now returns `Option<(Self::BufferData, Option)>` --- ## Performance based in main merged with #11290 Window 11 ,Intel 13400kf, NV 4070Ti ![image](https://github.com/bevyengine/bevy/assets/45868716/f63b9d98-6aee-4057-a2c7-a2162b2db765) frame time from 3.34ms to 3 ms, ~ 10% ![image](https://github.com/bevyengine/bevy/assets/45868716/a06eea9c-f79e-4324-8392-8d321560c5ba) `batch_and_prepare_render_phase` from 800us ~ 400 us ## Migration Guide trait `GetBatchData` no longer hold associated type `Data `and `Filter` `get_batch_data` `query_item `type from `Self::Data` to `Entity` and return `Option<(Self::BufferData, Option)>` `batch_and_prepare_render_phase` should not have a query --- crates/bevy_pbr/src/render/mesh.rs | 19 +++++++------------ crates/bevy_render/src/batching/mod.rs | 13 ++++--------- crates/bevy_sprite/src/mesh2d/mesh.rs | 16 ++++++---------- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 2831f6158a..03094f3b56 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/crates/bevy_pbr/src/render/mesh.rs @@ -12,7 +12,7 @@ use bevy_core_pipeline::{ use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ prelude::*, - query::{QueryItem, ROQueryItem}, + query::ROQueryItem, system::{lifetimeless::*, SystemParamItem, SystemState}, }; use bevy_math::{Affine3, Rect, UVec2, Vec4}; @@ -476,9 +476,6 @@ impl MeshPipeline { impl GetBatchData for MeshPipeline { type Param = (SRes, SRes); - type Data = Entity; - type Filter = With; - // The material bind group ID, the mesh ID, and the lightmap ID, // respectively. type CompareData = (MaterialBindGroupId, AssetId, Option>); @@ -487,14 +484,12 @@ impl GetBatchData for MeshPipeline { fn get_batch_data( (mesh_instances, lightmaps): &SystemParamItem, - entity: &QueryItem, - ) -> (Self::BufferData, Option) { - let mesh_instance = mesh_instances - .get(entity) - .expect("Failed to find render mesh instance"); - let maybe_lightmap = lightmaps.render_lightmaps.get(entity); + entity: Entity, + ) -> Option<(Self::BufferData, Option)> { + let mesh_instance = mesh_instances.get(&entity)?; + let maybe_lightmap = lightmaps.render_lightmaps.get(&entity); - ( + Some(( MeshUniform::new( &mesh_instance.transforms, maybe_lightmap.map(|lightmap| lightmap.uv_rect), @@ -504,7 +499,7 @@ impl GetBatchData for MeshPipeline { mesh_instance.mesh_asset_id, maybe_lightmap.map(|lightmap| lightmap.image), )), - ) + )) } } diff --git a/crates/bevy_render/src/batching/mod.rs b/crates/bevy_render/src/batching/mod.rs index 5f7d5f69d2..859d56e98d 100644 --- a/crates/bevy_render/src/batching/mod.rs +++ b/crates/bevy_render/src/batching/mod.rs @@ -1,7 +1,7 @@ use bevy_ecs::{ component::Component, + entity::Entity, prelude::Res, - query::{QueryFilter, QueryItem, ReadOnlyQueryData}, system::{Query, ResMut, StaticSystemParam, SystemParam, SystemParamItem}, }; use bevy_utils::nonmax::NonMaxU32; @@ -57,8 +57,6 @@ impl BatchMeta { /// items. pub trait GetBatchData { type Param: SystemParam + 'static; - type Data: ReadOnlyQueryData; - type Filter: QueryFilter; /// Data used for comparison between phase items. If the pipeline id, draw /// function id, per-instance data buffer dynamic offset and this data /// matches, the draws can be batched. @@ -72,8 +70,8 @@ pub trait GetBatchData { /// for the `CompareData`. fn get_batch_data( param: &SystemParamItem, - query_item: &QueryItem, - ) -> (Self::BufferData, Option); + query_item: Entity, + ) -> Option<(Self::BufferData, Option)>; } /// Batch the items in a render phase. This means comparing metadata needed to draw each phase item @@ -81,16 +79,13 @@ pub trait GetBatchData { pub fn batch_and_prepare_render_phase( gpu_array_buffer: ResMut>, mut views: Query<&mut RenderPhase>, - query: Query, param: StaticSystemParam, ) { let gpu_array_buffer = gpu_array_buffer.into_inner(); let system_param_item = param.into_inner(); let mut process_item = |item: &mut I| { - let batch_query_item = query.get(item.entity()).ok()?; - - let (buffer_data, compare_data) = F::get_batch_data(&system_param_item, &batch_query_item); + let (buffer_data, compare_data) = F::get_batch_data(&system_param_item, item.entity())?; let buffer_index = gpu_array_buffer.push(buffer_data); let index = buffer_index.index.get(); diff --git a/crates/bevy_sprite/src/mesh2d/mesh.rs b/crates/bevy_sprite/src/mesh2d/mesh.rs index d1a9b41907..9817e67f91 100644 --- a/crates/bevy_sprite/src/mesh2d/mesh.rs +++ b/crates/bevy_sprite/src/mesh2d/mesh.rs @@ -5,7 +5,7 @@ use bevy_core_pipeline::core_2d::Transparent2d; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ prelude::*, - query::{QueryItem, ROQueryItem}, + query::ROQueryItem, system::{lifetimeless::*, SystemParamItem, SystemState}, }; use bevy_math::{Affine3, Vec4}; @@ -339,25 +339,21 @@ impl Mesh2dPipeline { impl GetBatchData for Mesh2dPipeline { type Param = SRes; - type Data = Entity; - type Filter = With; type CompareData = (Material2dBindGroupId, AssetId); type BufferData = Mesh2dUniform; fn get_batch_data( mesh_instances: &SystemParamItem, - entity: &QueryItem, - ) -> (Self::BufferData, Option) { - let mesh_instance = mesh_instances - .get(entity) - .expect("Failed to find render mesh2d instance"); - ( + entity: Entity, + ) -> Option<(Self::BufferData, Option)> { + let mesh_instance = mesh_instances.get(&entity)?; + Some(( (&mesh_instance.transforms).into(), mesh_instance.automatic_batching.then_some(( mesh_instance.material_bind_group_id, mesh_instance.mesh_asset_id, )), - ) + )) } }