mirror of
https://github.com/bevyengine/bevy.git
synced 2026-05-06 06:06:42 -04:00
Fix stale bind group race for Material2d (#23457)
# Objective
If a `GpuImage` was modified (`Assets<Image>::insert`) in the same frame
as a Material2d that used it, it could create a race, potentially
leading to the old `TextureView` reference being read when creating the
`Material2d` bind group, leading to the material bind group never
getting the updated `GpuImage` reference.
## Solution
Register `GpuImage` as a dependency of `PreparedMaterial2d`.
Not all `Material2d` implementations might use `Image` handles, so I
guess this potentially has some cost in terms of performance. Ideally
we'd add some sort of mechanism so this dependency is only added when
needed, but this fix focuses on the minimal change to make the current
implementation correct.
## Testing
I ran into this problem when trying to get bevy_magic_light_2d working
on bevy 0.18 together with bevy_ecs_tilemap.
Minimal(ish) test case:
For some reason it only triggers on 0.18.1 though, not on current main.
```
//! Test: Material2d reading a render target image that gets replaced on resize.
//!
//! Without the fix (PreparedMaterial2d depending on GpuImage), the red spinner
//! rendered through the Material2d quad stops updating after a window resize.
use bevy::{
asset::uuid_handle,
camera::{visibility::RenderLayers, RenderTarget},
ecs::message::MessageReader,
prelude::*,
render::{
render_asset::RenderAssets,
render_resource::*,
texture::GpuImage,
Render, RenderApp, RenderSystems,
},
shader::ShaderRef,
sprite_render::{Material2d, Material2dPlugin, PreparedMaterial2d},
window::WindowResized,
};
const RENDER_TARGET: Handle<Image> = uuid_handle!("8bdbc7e2-ddc0-4264-9701-0a40476a4de6");
const SHOW_MATERIAL: Handle<ShowTextureMaterial> =
uuid_handle!("9a2cb207-fe74-45ed-b2c8-7ad4fc504c57");
#[derive(AsBindGroup, Clone, TypePath, Asset)]
struct ShowTextureMaterial {
#[texture(0)]
#[sampler(1)]
image: Handle<Image>,
}
impl Material2d for ShowTextureMaterial {
fn fragment_shader() -> ShaderRef {
"examples/show_texture.wgsl".into()
}
}
#[derive(Component)]
struct Spinner;
fn main() {
let mut app = App::new();
app.add_plugins((
DefaultPlugins,
Material2dPlugin::<ShowTextureMaterial>::default(),
));
// Adding a system with Commands to PrepareAssets perturbs the system
// ordering enough to trigger the bug.
app.sub_app_mut(RenderApp).add_systems(
Render,
(|_commands: Commands| {}).in_set(RenderSystems::PrepareAssets),
);
app
.add_systems(Startup, setup)
.add_systems(PreUpdate, handle_resize)
.add_systems(Update, spin)
.run();
}
fn setup(
mut commands: Commands,
mut images: ResMut<Assets<Image>>,
mut materials: ResMut<Assets<ShowTextureMaterial>>,
mut meshes: ResMut<Assets<Mesh>>,
) {
images
.insert(RENDER_TARGET.id(), create_render_target_image(1280, 720))
.expect("insert");
// Camera rendering to image target
commands.spawn((
Camera2d,
Camera::default(),
RenderTarget::Image(RENDER_TARGET.clone().into()),
Name::new("target_camera"),
));
// Camera rendering to screen
commands.spawn((
Camera2d,
Camera { order: 1, ..default() },
RenderLayers::layer(1),
Name::new("screen_camera"),
));
// Red spinner rendered to image target
commands.spawn((
Sprite {
color: Color::srgb(1.0, 0.0, 0.0),
custom_size: Some(Vec2::splat(100.0)),
..default()
},
Transform::default(),
Spinner,
));
// Blue spinner rendered to screen directly
commands.spawn((
Sprite {
color: Color::srgb(0.0, 0.0, 1.0),
custom_size: Some(Vec2::splat(100.0)),
..default()
},
Transform::from_xyz(300.0, 0.0, 0.0),
RenderLayers::layer(1),
Spinner,
));
// Material2d quad showing the render target on screen
materials
.insert(SHOW_MATERIAL.id(), ShowTextureMaterial {
image: RENDER_TARGET.clone(),
})
.expect("insert material");
commands.spawn((
Mesh2d(meshes.add(Mesh::from(Rectangle::new(400.0, 300.0)))),
MeshMaterial2d(SHOW_MATERIAL.clone()),
Transform::from_xyz(-200.0, 0.0, 1.0),
RenderLayers::layer(1),
));
}
fn spin(mut q: Query<&mut Transform, With<Spinner>>, time: Res<Time<Real>>) {
for mut t in &mut q {
t.rotation = Quat::from_rotation_z(time.elapsed_secs());
}
}
fn handle_resize(
mut images: ResMut<Assets<Image>>,
mut materials: ResMut<Assets<ShowTextureMaterial>>,
mut resize_events: MessageReader<WindowResized>,
mut last_size: Local<UVec2>,
) {
for event in resize_events.read() {
let w = event.width as u32;
let h = event.height as u32;
if w == 0 || h == 0 {
continue;
}
let new_size = UVec2::new(w, h);
if new_size == *last_size {
continue;
}
*last_size = new_size;
images
.insert(RENDER_TARGET.id(), create_render_target_image(w, h))
.expect("insert image");
// Re-insert material so its bind group gets recreated with the new GpuImage
materials
.insert(SHOW_MATERIAL.id(), ShowTextureMaterial {
image: RENDER_TARGET.clone(),
})
.expect("insert material");
}
}
fn create_render_target_image(w: u32, h: u32) -> Image {
let size = Extent3d { width: w, height: h, ..default() };
let mut image = Image {
texture_descriptor: TextureDescriptor {
label: Some("render_target"),
size,
dimension: TextureDimension::D2,
format: TextureFormat::bevy_default(),
mip_level_count: 1,
sample_count: 1,
usage: TextureUsages::TEXTURE_BINDING
| TextureUsages::COPY_DST
| TextureUsages::RENDER_ATTACHMENT,
view_formats: &[],
},
..default()
};
image.resize(size);
image
}
```
Without the fix the red rectangle will stop spinning after resizing the
window.
This commit is contained in:
committed by
GitHub
parent
45e454a83b
commit
dc2cd99082
@@ -47,6 +47,7 @@ use bevy_render::{
|
||||
},
|
||||
renderer::RenderDevice,
|
||||
sync_world::{MainEntity, MainEntityHashMap},
|
||||
texture::GpuImage,
|
||||
view::ExtractedView,
|
||||
Extract, ExtractSchedule, GpuResourceAppExt, Render, RenderApp, RenderStartup, RenderSystems,
|
||||
};
|
||||
@@ -280,7 +281,7 @@ where
|
||||
app.init_asset::<M>()
|
||||
.init_resource::<EntitiesNeedingSpecialization<M>>()
|
||||
.register_type::<MeshMaterial2d<M>>()
|
||||
.add_plugins(RenderAssetPlugin::<PreparedMaterial2d<M>>::default())
|
||||
.add_plugins(RenderAssetPlugin::<PreparedMaterial2d<M>, GpuImage>::default())
|
||||
.add_systems(
|
||||
PostUpdate,
|
||||
check_entities_needing_specialization::<M>.after(AssetEventSystems),
|
||||
|
||||
@@ -50,7 +50,7 @@ where
|
||||
|
||||
app.init_asset::<M>()
|
||||
.register_type::<MaterialNode<M>>()
|
||||
.add_plugins(RenderAssetPlugin::<PreparedUiMaterial<M>>::default());
|
||||
.add_plugins(RenderAssetPlugin::<PreparedUiMaterial<M>, GpuImage>::default());
|
||||
|
||||
if let Some(render_app) = app.get_sub_app_mut(RenderApp) {
|
||||
render_app
|
||||
|
||||
Reference in New Issue
Block a user