Skip to content

Make ClearColorConfig::None work correctly (esp in the presence of MSAA) #23844

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

Merged
alice-i-cecile merged 7 commits into from
Apr 17, 2026
+85 −56
Merged

Make ClearColorConfig::None work correctly (esp in the presence of MSAA) #23844

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8630cca
Cache ping pong atomics for view targets.
tychedelia Apr 17, 2026
554eeb1
Writeback when loading.
tychedelia Apr 17, 2026
c2593a4
Ci.
tychedelia Apr 17, 2026
ac26219
Ci.
tychedelia Apr 17, 2026
c9a15f7
Clean-up old entries.
tychedelia Apr 17, 2026
71bc8b6
Use entry API.
tychedelia Apr 17, 2026
e770b3c
Merge branch 'main' into view-target-fixes
tychedelia Apr 17, 2026
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
12 changes: 10 additions & 2 deletions crates/bevy_post_process/src/msaa_writeback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bevy_app::{App, Plugin};
use bevy_camera::MsaaWriteback;
use bevy_camera::{ClearColorConfig, MsaaWriteback};
use bevy_color::LinearRgba;
use bevy_core_pipeline::{
blit::{BlitPipeline, BlitPipelineKey},
Expand Down Expand Up @@ -108,7 +108,15 @@ fn prepare_msaa_writeback_pipelines(
// Determine if we should do MSAA writeback based on the camera's setting
let should_writeback = match camera.msaa_writeback {
MsaaWriteback::Off => false,
MsaaWriteback::Auto => camera.sorted_camera_index_for_target > 0,
// writeback is needed when the main pass must load existing content
// from the main texture, either because another camera already
// rendered to this target or because this camera preserves content across
// frames via load op load. otherwise we'd read from an ephemeral sampled
// texture that doesn't have the real content
MsaaWriteback::Auto => {
camera.sorted_camera_index_for_target > 0
|| matches!(camera.clear_color, ClearColorConfig::None)
}
MsaaWriteback::Always => true,
};

Expand Down
124 changes: 70 additions & 54 deletions crates/bevy_render/src/view/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::{
},
GpuResourceAppExt, Render, RenderApp, RenderSystems,
};
use alloc::sync::Arc;
use alloc::sync::{Arc, Weak};
use bevy_app::{App, Plugin};
use bevy_color::{LinearRgba, Oklaba};
use bevy_derive::{Deref, DerefMut};
Expand Down Expand Up @@ -1127,6 +1127,13 @@ pub fn cleanup_view_targets_for_resize(
}
}

type MainTextureKey = (
Option<NormalizedRenderTarget>,
TextureUsages,
TextureFormat,
Msaa,
);

pub fn prepare_view_targets(
mut commands: Commands,
clear_color_global: Res<ClearColor>,
Expand All @@ -1140,6 +1147,7 @@ pub fn prepare_view_targets(
&Msaa,
)>,
view_target_attachments: Res<ViewTargetAttachments>,
mut main_texture_atomics: Local<HashMap<MainTextureKey, Weak<AtomicUsize>>>,
) {
let mut textures = <HashMap<_, _>>::default();
for (entity, camera, view, texture_usage, msaa) in cameras.iter() {
Expand Down Expand Up @@ -1181,63 +1189,71 @@ pub fn prepare_view_targets(
}
});

let (a, b, sampled, main_texture) = textures
.entry((
camera.target.clone(),
texture_usage.0,
main_texture_format,
msaa,
))
.or_insert_with(|| {
let descriptor = TextureDescriptor {
label: None,
size: target_size.to_extents(),
mip_level_count: 1,
sample_count: 1,
dimension: TextureDimension::D2,
format: main_texture_format,
usage: texture_usage.0,
view_formats: match main_texture_format {
TextureFormat::Bgra8Unorm => &[TextureFormat::Bgra8UnormSrgb],
TextureFormat::Rgba8Unorm => &[TextureFormat::Rgba8UnormSrgb],
_ => &[],
},
};
let a = texture_cache.get(
&render_device,
TextureDescriptor {
label: Some("main_texture_a"),
..descriptor
},
);
let b = texture_cache.get(
let key: MainTextureKey = (
camera.target.clone(),
texture_usage.0,
main_texture_format,
*msaa,
);
let (a, b, sampled, main_texture) = textures.entry(key.clone()).or_insert_with(|| {
let descriptor = TextureDescriptor {
label: None,
size: target_size.to_extents(),
mip_level_count: 1,
sample_count: 1,
dimension: TextureDimension::D2,
format: main_texture_format,
usage: texture_usage.0,
view_formats: match main_texture_format {
TextureFormat::Bgra8Unorm => &[TextureFormat::Bgra8UnormSrgb],
TextureFormat::Rgba8Unorm => &[TextureFormat::Rgba8UnormSrgb],
_ => &[],
},
};
let a = texture_cache.get(
&render_device,
TextureDescriptor {
label: Some("main_texture_a"),
..descriptor
},
);
let b = texture_cache.get(
&render_device,
TextureDescriptor {
label: Some("main_texture_b"),
..descriptor
},
);
let sampled = if msaa.samples() > 1 {
let sampled = texture_cache.get(
&render_device,
TextureDescriptor {
label: Some("main_texture_b"),
..descriptor
label: Some("main_texture_sampled"),
size: target_size.to_extents(),
mip_level_count: 1,
sample_count: msaa.samples(),
dimension: TextureDimension::D2,
format: main_texture_format,
usage: TextureUsages::RENDER_ATTACHMENT,
view_formats: descriptor.view_formats,
},
);
let sampled = if msaa.samples() > 1 {
let sampled = texture_cache.get(
&render_device,
TextureDescriptor {
label: Some("main_texture_sampled"),
size: target_size.to_extents(),
mip_level_count: 1,
sample_count: msaa.samples(),
dimension: TextureDimension::D2,
format: main_texture_format,
usage: TextureUsages::RENDER_ATTACHMENT,
view_formats: descriptor.view_formats,
},
);
Some(sampled)
} else {
None
};
let main_texture = Arc::new(AtomicUsize::new(0));
(a, b, sampled, main_texture)
});
Some(sampled)
} else {
None
};
// re-use the same atomics frame to frame for views with the same main texture
// to ensure post process writes persist through msaa writeback
let main_texture = main_texture_atomics
.get(&key)
.and_then(Weak::upgrade)
.unwrap_or_else(|| {
let arc = Arc::new(AtomicUsize::new(0));
main_texture_atomics.insert(key.clone(), Arc::downgrade(&arc));
arc
});
Copy link
Copy Markdown
Member

@beicause beicause Apr 17, 2026

Choose a reason for hiding this comment

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

Can use HashMap.entry

Is it correct to use Weak<AtomicUsize>, won’t it be dropped at the start of each frame?

Copy link
Copy Markdown
Member Author

@tychedelia tychedelia Apr 17, 2026

Choose a reason for hiding this comment

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

Basically, we re-insert the ViewTarget every frame. As long as the configuration doesn't change, this is just replacing the effective same component over and over again. So there is an existing strong handle on the component, which we will replace here. I forgot to add the clean-up at the top of the system, but it's basically a cheap way to see if an entry is actually being used. I also just switched to using the actual entry api here, thanks!

(a, b, sampled, main_texture)
});

let main_textures = MainTargetTextures {
a: ColorAttachment::new(a.clone(), sampled.clone(), None, converted_clear_color),
Expand Down