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.

Sign up for GitHub

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

Move sprite batches to resource #17636

Merged
merged 2 commits into from
Feb 2, 2025
Merged

Move sprite batches to resource #17636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d428ab
Move sprite batches to resource
tychedelia Feb 1, 2025
fd15e6c
Store reference to entry.
tychedelia Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/bevy_sprite/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ impl Plugin for SpritePlugin {

fn finish(&self, app: &mut App) {
if let Some(render_app) = app.get_sub_app_mut(RenderApp) {
render_app.init_resource::<SpritePipeline>();
render_app
.init_resource::<SpriteBatches>()
.init_resource::<SpritePipeline>();
}
}
}
Expand Down
56 changes: 28 additions & 28 deletions crates/bevy_sprite/src/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use bevy_core_pipeline::{
TonemappingLuts,
},
};
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{
prelude::*,
query::ROQueryItem,
Expand All @@ -19,7 +20,7 @@ use bevy_image::{BevyDefault, Image, ImageSampler, TextureAtlasLayout, TextureFo
use bevy_math::{Affine3A, FloatOrd, Quat, Rect, Vec2, Vec4};
use bevy_platform_support::collections::HashMap;
use bevy_render::sync_world::MainEntity;
use bevy_render::view::RenderVisibleEntities;
use bevy_render::view::{RenderVisibleEntities, RetainedViewEntity};
use bevy_render::{
render_asset::RenderAssets,
render_phase::{
Expand Down Expand Up @@ -483,7 +484,10 @@ pub struct SpriteViewBindGroup {
pub value: BindGroup,
}

#[derive(Component, PartialEq, Eq, Clone)]
#[derive(Resource, Deref, DerefMut, Default)]
pub struct SpriteBatches(HashMap<(RetainedViewEntity, MainEntity), SpriteBatch>);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have an optimised hash of a pair of entities yet? If not then perhaps we need one? In the degenerate case with one batch per sprite I suppose this would hurt. But, sprites should use sprite sheets so that should not happen for large numbers of sprites in practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think using EntityHash should work no matter how many entities you use in the key. I'll include that instead of the default hasher and confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This didn't seem to matter in my testing.


#[derive(PartialEq, Eq, Clone)]
pub struct SpriteBatch {
image_handle_id: AssetId<Image>,
range: Range<u32>,
Expand Down Expand Up @@ -616,8 +620,6 @@ pub fn prepare_sprite_view_bind_groups(
}

pub fn prepare_sprite_image_bind_groups(
mut commands: Commands,
mut previous_len: Local<usize>,
render_device: Res<RenderDevice>,
render_queue: Res<RenderQueue>,
mut sprite_meta: ResMut<SpriteMeta>,
Expand All @@ -627,6 +629,7 @@ pub fn prepare_sprite_image_bind_groups(
extracted_sprites: Res<ExtractedSprites>,
mut phases: ResMut<ViewSortedRenderPhases<Transparent2d>>,
events: Res<SpriteAssetEvents>,
mut batches: ResMut<SpriteBatches>,
) {
// If an image has changed, the GpuImage has (probably) changed
for event in &events.images {
Expand All @@ -640,7 +643,7 @@ pub fn prepare_sprite_image_bind_groups(
};
}

let mut batches: Vec<(Entity, SpriteBatch)> = Vec::with_capacity(*previous_len);
batches.clear();

// Clear the sprite instances
sprite_meta.sprite_instance_buffer.clear();
Expand All @@ -650,7 +653,8 @@ pub fn prepare_sprite_image_bind_groups(

let image_bind_groups = &mut *image_bind_groups;

for transparent_phase in phases.values_mut() {
for (retained_view, transparent_phase) in phases.iter_mut() {
let mut current_batch = None;
let mut batch_item_index = 0;
let mut batch_image_size = Vec2::ZERO;
let mut batch_image_handle = AssetId::invalid();
Expand Down Expand Up @@ -690,8 +694,7 @@ pub fn prepare_sprite_image_bind_groups(
});

batch_item_index = item_index;
batches.push((
item.entity(),
current_batch = Some(batches.entry((*retained_view, item.main_entity())).insert(
tychedelia marked this conversation as resolved.
Show resolved Hide resolved
SpriteBatch {
image_handle_id: batch_image_handle,
range: index..index,
Expand Down Expand Up @@ -771,7 +774,7 @@ pub fn prepare_sprite_image_bind_groups(
transparent_phase.items[batch_item_index]
.batch_range_mut()
.end += 1;
batches.last_mut().unwrap().1.range.end += 1;
current_batch.as_mut().unwrap().get_mut().range.end += 1;
index += 1;
}
}
Expand Down Expand Up @@ -802,9 +805,6 @@ pub fn prepare_sprite_image_bind_groups(
.sprite_index_buffer
.write_buffer(&render_device, &render_queue);
}

*previous_len = batches.len();
commands.insert_or_spawn_batch(batches);
}

/// [`RenderCommand`] for sprite rendering.
Expand Down Expand Up @@ -834,19 +834,19 @@ impl<P: PhaseItem, const I: usize> RenderCommand<P> for SetSpriteViewBindGroup<I
}
pub struct SetSpriteTextureBindGroup<const I: usize>;
impl<P: PhaseItem, const I: usize> RenderCommand<P> for SetSpriteTextureBindGroup<I> {
type Param = SRes<ImageBindGroups>;
type ViewQuery = ();
type ItemQuery = Read<SpriteBatch>;
type Param = (SRes<ImageBindGroups>, SRes<SpriteBatches>);
type ViewQuery = Read<ExtractedView>;
type ItemQuery = ();

fn render<'w>(
_item: &P,
_view: (),
batch: Option<&'_ SpriteBatch>,
image_bind_groups: SystemParamItem<'w, '_, Self::Param>,
item: &P,
view: ROQueryItem<'w, Self::ViewQuery>,
_entity: Option<()>,
(image_bind_groups, batches): SystemParamItem<'w, '_, Self::Param>,
pass: &mut TrackedRenderPass<'w>,
) -> RenderCommandResult {
let image_bind_groups = image_bind_groups.into_inner();
let Some(batch) = batch else {
let Some(batch) = batches.get(&(view.retained_view_entity, item.main_entity())) else {
return RenderCommandResult::Skip;
};

Expand All @@ -864,19 +864,19 @@ impl<P: PhaseItem, const I: usize> RenderCommand<P> for SetSpriteTextureBindGrou

pub struct DrawSpriteBatch;
impl<P: PhaseItem> RenderCommand<P> for DrawSpriteBatch {
type Param = SRes<SpriteMeta>;
type ViewQuery = ();
type ItemQuery = Read<SpriteBatch>;
type Param = (SRes<SpriteMeta>, SRes<SpriteBatches>);
type ViewQuery = Read<ExtractedView>;
type ItemQuery = ();

fn render<'w>(
_item: &P,
_view: (),
batch: Option<&'_ SpriteBatch>,
sprite_meta: SystemParamItem<'w, '_, Self::Param>,
item: &P,
view: ROQueryItem<'w, Self::ViewQuery>,
_entity: Option<()>,
(sprite_meta, batches): SystemParamItem<'w, '_, Self::Param>,
pass: &mut TrackedRenderPass<'w>,
) -> RenderCommandResult {
let sprite_meta = sprite_meta.into_inner();
let Some(batch) = batch else {
let Some(batch) = batches.get(&(view.retained_view_entity, item.main_entity())) else {
return RenderCommandResult::Skip;
};

Expand Down