From 859c2d77f9659c229fa8bd76f0c554ba4f73d243 Mon Sep 17 00:00:00 2001 From: Rob Parrett Date: Fri, 3 Jan 2025 16:22:18 -0800 Subject: [PATCH] Revert "Fix sprite performance regression since retained render world (#17078)" (#17123) # Objective Fixes #17098 It seems that it's not totally obvious how to fix this, but that reverting might be part of the solution anyway. Let's get the repo back into a working state. ## Solution Revert the [recent optimization](https://github.com/bevyengine/bevy/pull/17078) that broke "many-to-one main->render world entities" for 2d. ## Testing `cargo run --example text2d` `cargo run --example sprite_slice` --- crates/bevy_sprite/src/render/mod.rs | 37 +++++++++---------- .../src/texture_slice/computed_slices.rs | 2 - crates/bevy_text/src/text2d.rs | 6 ++- examples/stress_tests/bevymark.rs | 23 +++++------- 4 files changed, 31 insertions(+), 37 deletions(-) diff --git a/crates/bevy_sprite/src/render/mod.rs b/crates/bevy_sprite/src/render/mod.rs index f509a5750384a..3adbfc9417dde 100644 --- a/crates/bevy_sprite/src/render/mod.rs +++ b/crates/bevy_sprite/src/render/mod.rs @@ -19,6 +19,7 @@ use bevy_ecs::{ }; use bevy_image::{BevyDefault, Image, ImageSampler, TextureFormatPixelInfo}; use bevy_math::{Affine3A, FloatOrd, Quat, Rect, Vec2, Vec4}; +use bevy_render::sync_world::MainEntity; use bevy_render::view::RenderVisibleEntities; use bevy_render::{ render_asset::RenderAssets, @@ -31,7 +32,7 @@ use bevy_render::{ *, }, renderer::{RenderDevice, RenderQueue}, - sync_world::{MainEntityHashMap, RenderEntity, TemporaryRenderEntity}, + sync_world::{RenderEntity, TemporaryRenderEntity}, texture::{DefaultImageSampler, FallbackImage, GpuImage}, view::{ ExtractedView, Msaa, ViewTarget, ViewUniform, ViewUniformOffset, ViewUniforms, @@ -340,12 +341,11 @@ pub struct ExtractedSprite { /// For cases where additional [`ExtractedSprites`] are created during extraction, this stores the /// entity that caused that creation for use in determining visibility. pub original_entity: Option, - pub render_entity: Entity, } #[derive(Resource, Default)] pub struct ExtractedSprites { - pub sprites: MainEntityHashMap, + pub sprites: HashMap<(Entity, MainEntity), ExtractedSprite>, } #[derive(Resource, Default)] @@ -390,13 +390,16 @@ pub fn extract_sprites( if let Some(slices) = slices { extracted_sprites.sprites.extend( slices - .extract_sprites( - transform, - original_entity, - commands.spawn(TemporaryRenderEntity).id(), - sprite, - ) - .map(|e| (original_entity.into(), e)), + .extract_sprites(transform, original_entity, sprite) + .map(|e| { + ( + ( + commands.spawn(TemporaryRenderEntity).id(), + original_entity.into(), + ), + e, + ) + }), ); } else { let atlas_rect = sprite @@ -417,7 +420,7 @@ pub fn extract_sprites( // PERF: we don't check in this function that the `Image` asset is ready, since it should be in most cases and hashing the handle is expensive extracted_sprites.sprites.insert( - original_entity.into(), + (entity, original_entity.into()), ExtractedSprite { color: sprite.color.into(), transform: *transform, @@ -429,7 +432,6 @@ pub fn extract_sprites( image_handle_id: sprite.image.id(), anchor: sprite.anchor.as_vec(), original_entity: Some(original_entity), - render_entity: entity, }, ); } @@ -556,11 +558,8 @@ pub fn queue_sprites( .items .reserve(extracted_sprites.sprites.len()); - for (main_entity, extracted_sprite) in extracted_sprites.sprites.iter() { - let index = extracted_sprite - .original_entity - .unwrap_or(extracted_sprite.render_entity) - .index(); + for ((entity, main_entity), extracted_sprite) in extracted_sprites.sprites.iter() { + let index = extracted_sprite.original_entity.unwrap_or(*entity).index(); if !view_entities.contains(index as usize) { continue; @@ -573,7 +572,7 @@ pub fn queue_sprites( transparent_phase.add(Transparent2d { draw_function: draw_sprite_function, pipeline, - entity: (extracted_sprite.render_entity, *main_entity), + entity: (*entity, *main_entity), sort_key, // batch_range and dynamic_offset will be calculated in prepare_sprites batch_range: 0..0, @@ -663,7 +662,7 @@ pub fn prepare_sprite_image_bind_groups( // Compatible items share the same entity. for item_index in 0..transparent_phase.items.len() { let item = &transparent_phase.items[item_index]; - let Some(extracted_sprite) = extracted_sprites.sprites.get(&item.entity.1) else { + let Some(extracted_sprite) = extracted_sprites.sprites.get(&item.entity) else { // If there is a phase item that is not a sprite, then we must start a new // batch to draw the other phase item(s) and to respect draw order. This can be // done by invalidating the batch_image_handle diff --git a/crates/bevy_sprite/src/texture_slice/computed_slices.rs b/crates/bevy_sprite/src/texture_slice/computed_slices.rs index eedf2c849e400..490071a6005ed 100644 --- a/crates/bevy_sprite/src/texture_slice/computed_slices.rs +++ b/crates/bevy_sprite/src/texture_slice/computed_slices.rs @@ -28,7 +28,6 @@ impl ComputedTextureSlices { &'a self, transform: &'a GlobalTransform, original_entity: Entity, - render_entity: Entity, sprite: &'a Sprite, ) -> impl ExactSizeIterator + 'a { let mut flip = Vec2::ONE; @@ -54,7 +53,6 @@ impl ComputedTextureSlices { flip_y, image_handle_id: sprite.image.id(), anchor: Self::redepend_anchor_from_sprite_to_slice(sprite, slice), - render_entity, } }) } diff --git a/crates/bevy_text/src/text2d.rs b/crates/bevy_text/src/text2d.rs index 2c90fc8c1f622..e3f65b43115db 100644 --- a/crates/bevy_text/src/text2d.rs +++ b/crates/bevy_text/src/text2d.rs @@ -206,7 +206,10 @@ pub fn extract_text2d_sprite( let atlas = texture_atlases.get(&atlas_info.texture_atlas).unwrap(); extracted_sprites.sprites.insert( - original_entity.into(), + ( + commands.spawn(TemporaryRenderEntity).id(), + original_entity.into(), + ), ExtractedSprite { transform: transform * GlobalTransform::from_translation(position.extend(0.)), color, @@ -217,7 +220,6 @@ pub fn extract_text2d_sprite( flip_y: false, anchor: Anchor::Center.as_vec(), original_entity: Some(original_entity), - render_entity: commands.spawn(TemporaryRenderEntity).id(), }, ); } diff --git a/examples/stress_tests/bevymark.rs b/examples/stress_tests/bevymark.rs index 26efbf164ffa1..761b997344cfb 100644 --- a/examples/stress_tests/bevymark.rs +++ b/examples/stress_tests/bevymark.rs @@ -13,7 +13,7 @@ use bevy::{ render_asset::RenderAssetUsages, render_resource::{Extent3d, TextureDimension, TextureFormat}, }, - sprite::{AlphaMode2d, SpritePlugin}, + sprite::AlphaMode2d, utils::Duration, window::{PresentMode, WindowResolution}, winit::{UpdateMode, WinitSettings}, @@ -132,21 +132,16 @@ fn main() { App::new() .add_plugins(( - DefaultPlugins - .set(WindowPlugin { - primary_window: Some(Window { - title: "BevyMark".into(), - resolution: WindowResolution::new(1920.0, 1080.0) - .with_scale_factor_override(1.0), - present_mode: PresentMode::AutoNoVsync, - ..default() - }), + DefaultPlugins.set(WindowPlugin { + primary_window: Some(Window { + title: "BevyMark".into(), + resolution: WindowResolution::new(1920.0, 1080.0) + .with_scale_factor_override(1.0), + present_mode: PresentMode::AutoNoVsync, ..default() - }) - .set(SpritePlugin { - #[cfg(feature = "bevy_sprite_picking_backend")] - add_picking: false, }), + ..default() + }), FrameTimeDiagnosticsPlugin, LogDiagnosticsPlugin::default(), ))