Skip to content

Commit

Permalink
Revert "Fix sprite performance regression since retained render world (
Browse files Browse the repository at this point in the history
…#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](#17078) that broke
"many-to-one main->render world entities" for 2d.

## Testing

`cargo run --example text2d`
`cargo run --example sprite_slice`
  • Loading branch information
rparrett authored Jan 4, 2025
1 parent 39f38a1 commit 859c2d7
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 37 deletions.
37 changes: 18 additions & 19 deletions crates/bevy_sprite/src/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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<Entity>,
pub render_entity: Entity,
}

#[derive(Resource, Default)]
pub struct ExtractedSprites {
pub sprites: MainEntityHashMap<ExtractedSprite>,
pub sprites: HashMap<(Entity, MainEntity), ExtractedSprite>,
}

#[derive(Resource, Default)]
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
},
);
}
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions crates/bevy_sprite/src/texture_slice/computed_slices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ impl ComputedTextureSlices {
&'a self,
transform: &'a GlobalTransform,
original_entity: Entity,
render_entity: Entity,
sprite: &'a Sprite,
) -> impl ExactSizeIterator<Item = ExtractedSprite> + 'a {
let mut flip = Vec2::ONE;
Expand All @@ -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,
}
})
}
Expand Down
6 changes: 4 additions & 2 deletions crates/bevy_text/src/text2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
},
);
}
Expand Down
23 changes: 9 additions & 14 deletions examples/stress_tests/bevymark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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(),
))
Expand Down

0 comments on commit 859c2d7

Please sign in to comment.