From 40ed2851a04f8a0468718fb07d13825a183d9e98 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Sat, 27 Nov 2021 17:40:07 +0000 Subject: [PATCH 1/4] Remove render assets, even if they have been changed --- pipelined/bevy_render2/src/render_asset.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pipelined/bevy_render2/src/render_asset.rs b/pipelined/bevy_render2/src/render_asset.rs index 6b259efd7180c..9d9cd099fd43e 100644 --- a/pipelined/bevy_render2/src/render_asset.rs +++ b/pipelined/bevy_render2/src/render_asset.rs @@ -102,9 +102,8 @@ fn extract_render_asset( changed_assets.insert(handle); } AssetEvent::Removed { handle } => { - if !changed_assets.remove(handle) { - removed.push(handle.clone_weak()); - } + changed_assets.remove(handle); + removed.push(handle.clone_weak()); } } } From 32204c19d966886400c6b17f0dc0478f4a04c3be Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Sat, 27 Nov 2021 17:54:25 +0000 Subject: [PATCH 2/4] Properly support changing images --- pipelined/bevy_sprite2/src/render/mod.rs | 74 ++++++++++++++++-------- 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/pipelined/bevy_sprite2/src/render/mod.rs b/pipelined/bevy_sprite2/src/render/mod.rs index eb330900034af..c08eb55c698ce 100644 --- a/pipelined/bevy_sprite2/src/render/mod.rs +++ b/pipelined/bevy_sprite2/src/render/mod.rs @@ -1,4 +1,4 @@ -use std::{cmp::Ordering, ops::Range}; +use std::{cmp::Ordering, collections::hash_map::Entry, ops::Range}; use crate::{ texture_atlas::{TextureAtlas, TextureAtlasSprite}, @@ -18,7 +18,7 @@ use bevy_render2::{ render_phase::{Draw, DrawFunctions, RenderPhase, TrackedRenderPass}, render_resource::*, renderer::{RenderDevice, RenderQueue}, - texture::{BevyDefault, Image}, + texture::{BevyDefault, GpuImage, Image}, view::{ComputedVisibility, ViewUniform, ViewUniformOffset, ViewUniforms}, RenderWorld, }; @@ -437,7 +437,8 @@ pub fn prepare_sprites( #[derive(Default)] pub struct ImageBindGroups { - values: HashMap, BindGroup>, + // We don't support change detection on RenderAssets, so we have to cache the GpuImage here + values: HashMap, (BindGroup, GpuImage)>, } #[allow(clippy::too_many_arguments)] @@ -476,26 +477,26 @@ pub fn queue_sprites( ); for mut transparent_phase in views.iter_mut() { for (entity, batch) in sprite_batches.iter_mut() { - image_bind_groups - .values - .entry(batch.handle.clone_weak()) - .or_insert_with(|| { - let gpu_image = gpu_images.get(&batch.handle).unwrap(); - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(&gpu_image.texture_view), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&gpu_image.sampler), - }, - ], - label: Some("sprite_material_bind_group"), - layout: &sprite_pipeline.material_layout, - }) - }); + let gpu_image = gpu_images.get(&batch.handle).unwrap().clone(); + match image_bind_groups.values.entry(batch.handle.clone_weak()) { + Entry::Occupied(mut already_existing) => { + let (group, image) = already_existing.get_mut(); + // Can't compare GPUImages directly, this has to be good enough + if image.texture_view.id() != gpu_image.texture_view.id() + || image.sampler.id() != gpu_image.sampler.id() + { + *group = + create_bind_group(&render_device, &gpu_image, &sprite_pipeline); + *image = gpu_image; + } + } + Entry::Vacant(vacant) => { + vacant.insert(( + create_bind_group(&render_device, &gpu_image, &sprite_pipeline), + gpu_image, + )); + } + }; transparent_phase.add(Transparent2d { draw_function: draw_sprite_function, pipeline: if batch.colored { @@ -511,6 +512,27 @@ pub fn queue_sprites( } } +fn create_bind_group( + render_device: &RenderDevice, + gpu_image: &GpuImage, + sprite_pipeline: &SpritePipeline, +) -> BindGroup { + render_device.create_bind_group(&BindGroupDescriptor { + entries: &[ + BindGroupEntry { + binding: 0, + resource: BindingResource::TextureView(&gpu_image.texture_view), + }, + BindGroupEntry { + binding: 1, + resource: BindingResource::Sampler(&gpu_image.sampler), + }, + ], + label: Some("sprite_material_bind_group"), + layout: &sprite_pipeline.material_layout, + }) +} + pub struct DrawSprite { params: SystemState<( SRes, @@ -556,7 +578,11 @@ impl Draw for DrawSprite { ); pass.set_bind_group( 1, - image_bind_groups.values.get(&sprite_batch.handle).unwrap(), + &image_bind_groups + .values + .get(&sprite_batch.handle) + .unwrap() + .0, &[], ); From e76124bba46458aff47ab8eac7c46ab6cca3a1a9 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Mon, 29 Nov 2021 20:10:10 +0000 Subject: [PATCH 3/4] Extract the evemts instead --- pipelined/bevy_sprite2/src/lib.rs | 1 + pipelined/bevy_sprite2/src/render/mod.rs | 80 +++++++++++++++--------- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/pipelined/bevy_sprite2/src/lib.rs b/pipelined/bevy_sprite2/src/lib.rs index 27d36548c8ba3..2545763d84d5c 100644 --- a/pipelined/bevy_sprite2/src/lib.rs +++ b/pipelined/bevy_sprite2/src/lib.rs @@ -44,6 +44,7 @@ impl Plugin for SpritePlugin { .init_resource::() .init_resource::() .add_system_to_stage(RenderStage::Extract, render::extract_sprites) + .add_system_to_stage(RenderStage::Extract, render::extract_events) .add_system_to_stage(RenderStage::Prepare, render::prepare_sprites) .add_system_to_stage(RenderStage::Queue, queue_sprites); diff --git a/pipelined/bevy_sprite2/src/render/mod.rs b/pipelined/bevy_sprite2/src/render/mod.rs index c08eb55c698ce..f1da18e6c0e09 100644 --- a/pipelined/bevy_sprite2/src/render/mod.rs +++ b/pipelined/bevy_sprite2/src/render/mod.rs @@ -1,10 +1,10 @@ -use std::{cmp::Ordering, collections::hash_map::Entry, ops::Range}; +use std::{cmp::Ordering, ops::Range}; use crate::{ texture_atlas::{TextureAtlas, TextureAtlasSprite}, Rect, Sprite, SPRITE_SHADER_HANDLE, }; -use bevy_asset::{Assets, Handle}; +use bevy_asset::{AssetEvent, Assets, Handle}; use bevy_core::FloatOrd; use bevy_core_pipeline::Transparent2d; use bevy_ecs::{ @@ -172,6 +172,37 @@ pub struct ExtractedSprites { sprites: Vec, } +#[derive(Default)] +pub struct SpriteAssetEvents { + images: Vec>, +} + +pub fn extract_events( + mut render_world: ResMut, + mut image_events: EventReader>, +) { + let mut events = render_world + .get_resource_mut::() + .unwrap(); + let SpriteAssetEvents { ref mut images } = *events; + images.clear(); + + for image in image_events.iter() { + // AssetEvent: !Clone + images.push(match image { + AssetEvent::Created { handle } => AssetEvent::Created { + handle: handle.clone_weak(), + }, + AssetEvent::Modified { handle } => AssetEvent::Modified { + handle: handle.clone_weak(), + }, + AssetEvent::Removed { handle } => AssetEvent::Removed { + handle: handle.clone_weak(), + }, + }); + } +} + pub fn extract_sprites( mut render_world: ResMut, images: Res>, @@ -438,7 +469,7 @@ pub fn prepare_sprites( #[derive(Default)] pub struct ImageBindGroups { // We don't support change detection on RenderAssets, so we have to cache the GpuImage here - values: HashMap, (BindGroup, GpuImage)>, + values: HashMap, BindGroup>, } #[allow(clippy::too_many_arguments)] @@ -454,7 +485,17 @@ pub fn queue_sprites( gpu_images: Res>, mut sprite_batches: Query<(Entity, &SpriteBatch)>, mut views: Query<&mut RenderPhase>, + events: Res, ) { + // If an image has changed, the GpuImage has (probably) changed + for event in &events.images { + match event { + AssetEvent::Created { .. } => None, + AssetEvent::Modified { handle } => image_bind_groups.values.remove(handle), + AssetEvent::Removed { handle } => image_bind_groups.values.remove(handle), + }; + } + if let Some(view_binding) = view_uniforms.uniforms.binding() { sprite_meta.view_bind_group = Some(render_device.create_bind_group(&BindGroupDescriptor { entries: &[BindGroupEntry { @@ -477,26 +518,13 @@ pub fn queue_sprites( ); for mut transparent_phase in views.iter_mut() { for (entity, batch) in sprite_batches.iter_mut() { - let gpu_image = gpu_images.get(&batch.handle).unwrap().clone(); - match image_bind_groups.values.entry(batch.handle.clone_weak()) { - Entry::Occupied(mut already_existing) => { - let (group, image) = already_existing.get_mut(); - // Can't compare GPUImages directly, this has to be good enough - if image.texture_view.id() != gpu_image.texture_view.id() - || image.sampler.id() != gpu_image.sampler.id() - { - *group = - create_bind_group(&render_device, &gpu_image, &sprite_pipeline); - *image = gpu_image; - } - } - Entry::Vacant(vacant) => { - vacant.insert(( - create_bind_group(&render_device, &gpu_image, &sprite_pipeline), - gpu_image, - )); - } - }; + image_bind_groups + .values + .entry(batch.handle.clone_weak()) + .or_insert_with(|| { + let gpu_image = gpu_images.get(&batch.handle).unwrap().clone(); + create_bind_group(&render_device, &gpu_image, &sprite_pipeline) + }); transparent_phase.add(Transparent2d { draw_function: draw_sprite_function, pipeline: if batch.colored { @@ -578,11 +606,7 @@ impl Draw for DrawSprite { ); pass.set_bind_group( 1, - &image_bind_groups - .values - .get(&sprite_batch.handle) - .unwrap() - .0, + image_bind_groups.values.get(&sprite_batch.handle).unwrap(), &[], ); From dfe5927085c044461d2261540d1e936f68c600df Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 1 Dec 2021 13:59:47 -0800 Subject: [PATCH 4/4] Add missing resource, remove unnecessary clone, remove old comment, inline bind group creation for simplicity --- pipelined/bevy_sprite2/src/lib.rs | 3 +- pipelined/bevy_sprite2/src/render/mod.rs | 43 ++++++++++-------------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/pipelined/bevy_sprite2/src/lib.rs b/pipelined/bevy_sprite2/src/lib.rs index 2545763d84d5c..cd3bb5babac83 100644 --- a/pipelined/bevy_sprite2/src/lib.rs +++ b/pipelined/bevy_sprite2/src/lib.rs @@ -43,8 +43,9 @@ impl Plugin for SpritePlugin { .init_resource::>() .init_resource::() .init_resource::() + .init_resource::() .add_system_to_stage(RenderStage::Extract, render::extract_sprites) - .add_system_to_stage(RenderStage::Extract, render::extract_events) + .add_system_to_stage(RenderStage::Extract, render::extract_sprite_events) .add_system_to_stage(RenderStage::Prepare, render::prepare_sprites) .add_system_to_stage(RenderStage::Queue, queue_sprites); diff --git a/pipelined/bevy_sprite2/src/render/mod.rs b/pipelined/bevy_sprite2/src/render/mod.rs index f1da18e6c0e09..41fa90b419be4 100644 --- a/pipelined/bevy_sprite2/src/render/mod.rs +++ b/pipelined/bevy_sprite2/src/render/mod.rs @@ -18,7 +18,7 @@ use bevy_render2::{ render_phase::{Draw, DrawFunctions, RenderPhase, TrackedRenderPass}, render_resource::*, renderer::{RenderDevice, RenderQueue}, - texture::{BevyDefault, GpuImage, Image}, + texture::{BevyDefault, Image}, view::{ComputedVisibility, ViewUniform, ViewUniformOffset, ViewUniforms}, RenderWorld, }; @@ -177,7 +177,7 @@ pub struct SpriteAssetEvents { images: Vec>, } -pub fn extract_events( +pub fn extract_sprite_events( mut render_world: ResMut, mut image_events: EventReader>, ) { @@ -468,7 +468,6 @@ pub fn prepare_sprites( #[derive(Default)] pub struct ImageBindGroups { - // We don't support change detection on RenderAssets, so we have to cache the GpuImage here values: HashMap, BindGroup>, } @@ -522,8 +521,21 @@ pub fn queue_sprites( .values .entry(batch.handle.clone_weak()) .or_insert_with(|| { - let gpu_image = gpu_images.get(&batch.handle).unwrap().clone(); - create_bind_group(&render_device, &gpu_image, &sprite_pipeline) + let gpu_image = gpu_images.get(&batch.handle).unwrap(); + render_device.create_bind_group(&BindGroupDescriptor { + entries: &[ + BindGroupEntry { + binding: 0, + resource: BindingResource::TextureView(&gpu_image.texture_view), + }, + BindGroupEntry { + binding: 1, + resource: BindingResource::Sampler(&gpu_image.sampler), + }, + ], + label: Some("sprite_material_bind_group"), + layout: &sprite_pipeline.material_layout, + }) }); transparent_phase.add(Transparent2d { draw_function: draw_sprite_function, @@ -540,27 +552,6 @@ pub fn queue_sprites( } } -fn create_bind_group( - render_device: &RenderDevice, - gpu_image: &GpuImage, - sprite_pipeline: &SpritePipeline, -) -> BindGroup { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(&gpu_image.texture_view), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&gpu_image.sampler), - }, - ], - label: Some("sprite_material_bind_group"), - layout: &sprite_pipeline.material_layout, - }) -} - pub struct DrawSprite { params: SystemState<( SRes,