Allow mix of hdr and non-hdr cameras to same render target (#13419)

Changes:
- Track whether an output texture has been written to yet and only clear
it on the first write.
- Use `ClearColorConfig` on `CameraOutputMode` instead of a raw
`LoadOp`.
- Track whether a output texture has been seen when specializing the
upscaling pipeline and use alpha blending for extra cameras rendering to
that texture that do not specify an explicit blend mode.

Fixes #6754

## Testing

Tested against provided test case in issue:

![image](https://github.com/bevyengine/bevy/assets/10366310/d066f069-87fb-4249-a4d9-b6cb1751971b)

---

## Changelog

- Allow cameras rendering to the same output texture with mixed hdr to
work correctly.

## Migration Guide

- - Change `CameraOutputMode` to use `ClearColorConfig` instead of
`LoadOp`.

crates/bevy_core_pipeline/src/upscaling/mod.rs

@@ -4,6 +4,7 @@ use bevy_ecs::prelude::*;
 use bevy_render::camera::{CameraOutputMode, ExtractedCamera};
 use bevy_render::view::ViewTarget;
 use bevy_render::{render_resource::*, Render, RenderApp, RenderSet};
+use bevy_utils::HashSet;
 
 mod node;
 
@@ -32,16 +33,32 @@ fn prepare_view_upscaling_pipelines(
     blit_pipeline: Res<BlitPipeline>,
     view_targets: Query<(Entity, &ViewTarget, Option<&ExtractedCamera>)>,
 ) {
+    let mut output_textures = HashSet::new();
     for (entity, view_target, camera) in view_targets.iter() {
+        let out_texture_id = view_target.out_texture().id();
         let blend_state = if let Some(ExtractedCamera {
             output_mode: CameraOutputMode::Write { blend_state, .. },
             ..
         }) = camera
         {
-            *blend_state
+            match *blend_state {
+                None => {
+                    // If we've already seen this output for a camera and it doesn't have a output blend
+                    // mode configured, default to alpha blend so that we don't accidentally overwrite
+                    // the output texture
+                    if output_textures.contains(&out_texture_id) {
+                        Some(BlendState::ALPHA_BLENDING)
+                    } else {
+                        None
+                    }
+                }
+                _ => *blend_state,
+            }
         } else {
             None
         };
+        output_textures.insert(out_texture_id);
+
         let key = BlitPipelineKey {
             texture_format: view_target.out_texture_format(),
             blend_state,

crates/bevy_core_pipeline/src/upscaling/node.rs

@@ -1,11 +1,11 @@
 use crate::{blit::BlitPipeline, upscaling::ViewUpscalingPipeline};
 use bevy_ecs::{prelude::*, query::QueryItem};
+use bevy_render::camera::{ClearColor, ClearColorConfig};
 use bevy_render::{
     camera::{CameraOutputMode, ExtractedCamera},
     render_graph::{NodeRunError, RenderGraphContext, ViewNode},
     render_resource::{
-        BindGroup, BindGroupEntries, LoadOp, Operations, PipelineCache, RenderPassColorAttachment,
-        RenderPassDescriptor, StoreOp, TextureViewId,
+        BindGroup, BindGroupEntries, PipelineCache, RenderPassDescriptor, TextureViewId,
     },
     renderer::RenderContext,
     view::ViewTarget,
@@ -33,19 +33,22 @@ impl ViewNode for UpscalingNode {
     ) -> Result<(), NodeRunError> {
         let pipeline_cache = world.get_resource::<PipelineCache>().unwrap();
         let blit_pipeline = world.get_resource::<BlitPipeline>().unwrap();
+        let clear_color_global = world.get_resource::<ClearColor>().unwrap();
 
-        let color_attachment_load_op = if let Some(camera) = camera {
+        let clear_color = if let Some(camera) = camera {
             match camera.output_mode {
-                CameraOutputMode::Write {
-                    color_attachment_load_op,
-                    ..
-                } => color_attachment_load_op,
+                CameraOutputMode::Write { clear_color, .. } => clear_color,
                 CameraOutputMode::Skip => return Ok(()),
             }
         } else {
-            LoadOp::Clear(Default::default())
+            ClearColorConfig::Default
         };
-
+        let clear_color = match clear_color {
+            ClearColorConfig::Default => Some(clear_color_global.0),
+            ClearColorConfig::Custom(color) => Some(color),
+            ClearColorConfig::None => None,
+        };
+        let converted_clear_color = clear_color.map(|color| color.into());
         let upscaled_texture = target.main_texture_view();
 
         let mut cached_bind_group = self.cached_texture_bind_group.lock().unwrap();
@@ -69,14 +72,9 @@ impl ViewNode for UpscalingNode {
 
         let pass_descriptor = RenderPassDescriptor {
             label: Some("upscaling_pass"),
-            color_attachments: &[Some(RenderPassColorAttachment {
-                view: target.out_texture(),
-                resolve_target: None,
-                ops: Operations {
-                    load: color_attachment_load_op,
-                    store: StoreOp::Store,
-                },
-            })],
+            color_attachments: &[Some(
+                target.out_texture_color_attachment(converted_clear_color),
+            )],
             depth_stencil_attachment: None,
             timestamp_writes: None,
             occlusion_query_set: None,

crates/bevy_render/src/camera/camera.rs

@@ -35,7 +35,7 @@ use bevy_window::{
     WindowScaleFactorChanged,
 };
 use std::ops::Range;
-use wgpu::{BlendState, LoadOp, TextureFormat, TextureUsages};
+use wgpu::{BlendState, TextureFormat, TextureUsages};
 
 use super::{ClearColorConfig, Projection};
 
@@ -488,9 +488,8 @@ pub enum CameraOutputMode {
     Write {
         /// The blend state that will be used by the pipeline that writes the intermediate render textures to the final render target texture.
         blend_state: Option<BlendState>,
-        /// The color attachment load operation that will be used by the pipeline that writes the intermediate render textures to the final render
-        /// target texture.
-        color_attachment_load_op: LoadOp<wgpu::Color>,
+        /// The clear color operation to perform on the final render target texture.
+        clear_color: ClearColorConfig,
     },
     /// Skips writing the camera output to the configured render target. The output will remain in the
     /// Render Target's "intermediate" textures, which a camera with a higher order should write to the render target
@@ -505,7 +504,7 @@ impl Default for CameraOutputMode {
     fn default() -> Self {
         CameraOutputMode::Write {
             blend_state: None,
-            color_attachment_load_op: LoadOp::Clear(Default::default()),
+            clear_color: ClearColorConfig::Default,
         }
     }
 }
@@ -824,6 +823,7 @@ pub struct ExtractedCamera {
     pub clear_color: ClearColorConfig,
     pub sorted_camera_index_for_target: usize,
     pub exposure: f32,
+    pub hdr: bool,
 }
 
 pub fn extract_cameras(
@@ -897,12 +897,13 @@ pub fn extract_cameras(
                     order: camera.order,
                     output_mode: camera.output_mode,
                     msaa_writeback: camera.msaa_writeback,
-                    clear_color: camera.clear_color.clone(),
+                    clear_color: camera.clear_color,
                     // this will be set in sort_cameras
                     sorted_camera_index_for_target: 0,
                     exposure: exposure
                         .map(|e| e.exposure())
                         .unwrap_or_else(|| Exposure::default().exposure()),
+                    hdr: camera.hdr,
                 },
                 ExtractedView {
                     clip_from_view: camera.clip_from_view(),
@@ -954,6 +955,7 @@ pub struct SortedCamera {
     pub entity: Entity,
     pub order: isize,
     pub target: Option<NormalizedRenderTarget>,
+    pub hdr: bool,
 }
 
 pub fn sort_cameras(
@@ -966,6 +968,7 @@ pub fn sort_cameras(
             entity,
             order: camera.order,
             target: camera.target.clone(),
+            hdr: camera.hdr,
         });
     }
     // sort by order and ensure within an order, RenderTargets of the same type are packed together
@@ -986,7 +989,9 @@ pub fn sort_cameras(
             }
         }
         if let Some(target) = &sorted_camera.target {
-            let count = target_counts.entry(target.clone()).or_insert(0usize);
+            let count = target_counts
+                .entry((target.clone(), sorted_camera.hdr))
+                .or_insert(0usize);
             let (_, mut camera) = cameras.get_mut(sorted_camera.entity).unwrap();
             camera.sorted_camera_index_for_target = *count;
             *count += 1;

crates/bevy_render/src/camera/clear_color.rs

@@ -6,7 +6,7 @@ use bevy_reflect::prelude::*;
 use serde::{Deserialize, Serialize};
 
 /// For a camera, specifies the color used to clear the viewport before rendering.
-#[derive(Reflect, Serialize, Deserialize, Clone, Debug, Default)]
+#[derive(Reflect, Serialize, Deserialize, Copy, Clone, Debug, Default)]
 #[reflect(Serialize, Deserialize, Default)]
 pub enum ClearColorConfig {
     /// The clear color is taken from the world's [`ClearColor`] resource.

crates/bevy_render/src/texture/texture_attachment.rs

@@ -1,5 +1,5 @@
 use super::CachedTexture;
-use crate::render_resource::TextureView;
+use crate::render_resource::{TextureFormat, TextureView};
 use bevy_color::LinearRgba;
 use std::sync::{
     atomic::{AtomicBool, Ordering},
@@ -120,3 +120,41 @@ impl DepthAttachment {
         }
     }
 }
+
+/// A wrapper for a [`TextureView`] that is used as a [`RenderPassColorAttachment`] for a view
+/// target's final output texture.
+#[derive(Clone)]
+pub struct OutputColorAttachment {
+    pub view: TextureView,
+    pub format: TextureFormat,
+    is_first_call: Arc<AtomicBool>,
+}
+
+impl OutputColorAttachment {
+    pub fn new(view: TextureView, format: TextureFormat) -> Self {
+        Self {
+            view,
+            format,
+            is_first_call: Arc::new(AtomicBool::new(true)),
+        }
+    }
+
+    /// Get this texture view as an attachment. The attachment will be cleared with a value of
+    /// the provided `clear_color` if this is the first time calling this function, otherwise it
+    /// will be loaded.
+    pub fn get_attachment(&self, clear_color: Option<LinearRgba>) -> RenderPassColorAttachment {
+        let first_call = self.is_first_call.fetch_and(false, Ordering::SeqCst);
+
+        RenderPassColorAttachment {
+            view: &self.view,
+            resolve_target: None,
+            ops: Operations {
+                load: match (clear_color, first_call) {
+                    (Some(clear_color), true) => LoadOp::Clear(clear_color.into()),
+                    (None, _) | (Some(_), false) => LoadOp::Load,
+                },
+                store: StoreOp::Store,
+            },
+        }
+    }
+}