From 5cc3352f5b2d5467e4d765ece48b10ae8626cc6a Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Fri, 3 Nov 2023 01:09:14 +0000 Subject: [PATCH] allow DeferredPrepass to work without other prepass markers (#10223) # Objective fix crash / misbehaviour when `DeferredPrepass` is used without `DepthPrepass`. - Deferred lighting requires the depth prepass texture to be present, so that the depth texture is available for binding. without it the deferred lighting pass will use 0 for depth of all meshes. - When `DeferredPrepass` is used without other prepass markers, and with any materials that use `OpaqueRenderMode::Forward`, those entities will try to queue to the `Opaque3dPrepass` render phase, which doesn't exist, causing a crash. ## Solution - check if the prepass phases exist before queueing - generate prepass textures if `Opaque3dDeferred` is present - add a note to the DeferredPrepass marker to note that DepthPrepass is also required by the default deferred lighting pass - also changed some `With.is_some()`s to `Has`s --- crates/bevy_core_pipeline/src/core_3d/mod.rs | 47 ++++++++++---------- crates/bevy_core_pipeline/src/prepass/mod.rs | 1 + crates/bevy_pbr/src/prepass/mod.rs | 15 +++---- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/crates/bevy_core_pipeline/src/core_3d/mod.rs b/crates/bevy_core_pipeline/src/core_3d/mod.rs index f7404efbd7654..f003ed6191f2b 100644 --- a/crates/bevy_core_pipeline/src/core_3d/mod.rs +++ b/crates/bevy_core_pipeline/src/core_3d/mod.rs @@ -450,10 +450,10 @@ pub fn extract_camera_prepass_phase( ( Entity, &Camera, - Option<&DepthPrepass>, - Option<&NormalPrepass>, - Option<&MotionVectorPrepass>, - Option<&DeferredPrepass>, + Has, + Has, + Has, + Has, ), With, >, @@ -465,33 +465,30 @@ pub fn extract_camera_prepass_phase( if camera.is_active { let mut entity = commands.get_or_spawn(entity); - if depth_prepass.is_some() - || normal_prepass.is_some() - || motion_vector_prepass.is_some() - { + if depth_prepass || normal_prepass || motion_vector_prepass { entity.insert(( RenderPhase::::default(), RenderPhase::::default(), )); } - if deferred_prepass.is_some() { + if deferred_prepass { entity.insert(( RenderPhase::::default(), RenderPhase::::default(), )); } - if depth_prepass.is_some() { + if depth_prepass { entity.insert(DepthPrepass); } - if normal_prepass.is_some() { + if normal_prepass { entity.insert(NormalPrepass); } - if motion_vector_prepass.is_some() { + if motion_vector_prepass { entity.insert(MotionVectorPrepass); } - if deferred_prepass.is_some() { + if deferred_prepass { entity.insert(DeferredPrepass); } } @@ -685,15 +682,17 @@ pub fn prepare_prepass_textures( ( Entity, &ExtractedCamera, - Option<&DepthPrepass>, - Option<&NormalPrepass>, - Option<&MotionVectorPrepass>, - Option<&DeferredPrepass>, + Has, + Has, + Has, + Has, ), - ( + Or<( With>, With>, - ), + With>, + With>, + )>, >, ) { let mut depth_textures = HashMap::default(); @@ -714,7 +713,7 @@ pub fn prepare_prepass_textures( height: physical_target_size.y, }; - let cached_depth_texture = depth_prepass.is_some().then(|| { + let cached_depth_texture = depth_prepass.then(|| { depth_textures .entry(camera.target.clone()) .or_insert_with(|| { @@ -735,7 +734,7 @@ pub fn prepare_prepass_textures( .clone() }); - let cached_normals_texture = normal_prepass.is_some().then(|| { + let cached_normals_texture = normal_prepass.then(|| { normal_textures .entry(camera.target.clone()) .or_insert_with(|| { @@ -757,7 +756,7 @@ pub fn prepare_prepass_textures( .clone() }); - let cached_motion_vectors_texture = motion_vector_prepass.is_some().then(|| { + let cached_motion_vectors_texture = motion_vector_prepass.then(|| { motion_vectors_textures .entry(camera.target.clone()) .or_insert_with(|| { @@ -779,7 +778,7 @@ pub fn prepare_prepass_textures( .clone() }); - let cached_deferred_texture = deferred_prepass.is_some().then(|| { + let cached_deferred_texture = deferred_prepass.then(|| { deferred_textures .entry(camera.target.clone()) .or_insert_with(|| { @@ -801,7 +800,7 @@ pub fn prepare_prepass_textures( .clone() }); - let deferred_lighting_pass_id_texture = deferred_prepass.is_some().then(|| { + let deferred_lighting_pass_id_texture = deferred_prepass.then(|| { deferred_lighting_id_textures .entry(camera.target.clone()) .or_insert_with(|| { diff --git a/crates/bevy_core_pipeline/src/prepass/mod.rs b/crates/bevy_core_pipeline/src/prepass/mod.rs index ba4de1952c82e..63b8c764af5ff 100644 --- a/crates/bevy_core_pipeline/src/prepass/mod.rs +++ b/crates/bevy_core_pipeline/src/prepass/mod.rs @@ -55,6 +55,7 @@ pub struct NormalPrepass; pub struct MotionVectorPrepass; /// If added to a [`crate::prelude::Camera3d`] then deferred materials will be rendered to the deferred gbuffer texture and will be available to subsequent passes. +/// Note the default deferred lighting plugin also requires `DepthPrepass` to work correctly. #[derive(Component, Default, Reflect)] pub struct DeferredPrepass; diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 2235457d5164e..6f2dfed69e12d 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -784,9 +784,6 @@ pub fn queue_prepass_material_meshes( view_key |= MeshPipelineKey::MOTION_VECTOR_PREPASS; } - let mut opaque_phase_deferred = opaque_deferred_phase.as_mut(); - let mut alpha_mask_phase_deferred = alpha_mask_deferred_phase.as_mut(); - let rangefinder = view.rangefinder3d(); for visible_entity in &visible_entities.entities { @@ -859,7 +856,7 @@ pub fn queue_prepass_material_meshes( match alpha_mode { AlphaMode::Opaque => { if deferred { - opaque_phase_deferred + opaque_deferred_phase .as_mut() .unwrap() .add(Opaque3dDeferred { @@ -870,8 +867,8 @@ pub fn queue_prepass_material_meshes( batch_range: 0..1, dynamic_offset: None, }); - } else { - opaque_phase.as_mut().unwrap().add(Opaque3dPrepass { + } else if let Some(opaque_phase) = opaque_phase.as_mut() { + opaque_phase.add(Opaque3dPrepass { entity: *visible_entity, draw_function: opaque_draw_prepass, pipeline_id, @@ -883,7 +880,7 @@ pub fn queue_prepass_material_meshes( } AlphaMode::Mask(_) => { if deferred { - alpha_mask_phase_deferred + alpha_mask_deferred_phase .as_mut() .unwrap() .add(AlphaMask3dDeferred { @@ -894,8 +891,8 @@ pub fn queue_prepass_material_meshes( batch_range: 0..1, dynamic_offset: None, }); - } else { - alpha_mask_phase.as_mut().unwrap().add(AlphaMask3dPrepass { + } else if let Some(alpha_mask_phase) = alpha_mask_phase.as_mut() { + alpha_mask_phase.add(AlphaMask3dPrepass { entity: *visible_entity, draw_function: alpha_mask_draw_prepass, pipeline_id,