Skip to content

Commit

Permalink
Fix dependency of shadow mapping on the optional PrepassPlugin (#7878)
Browse files Browse the repository at this point in the history
# Objective

Unfortunately, there are three issues with my changes introduced by #7784.

1.  The changes left some dead code. This is already taken care of here: #7875.
2. Disabling prepass causes failures because the shadow mapping relies on the `PrepassPlugin` now.
3. Custom materials use the `prepass.wgsl` shader, but this does not always define a fragment entry point.

This PR fixes 2. and 3. and resolves #7879.

## Solution

- Add a regression test with disabled prepass.
- Split `PrepassPlugin` into two plugins:
  - `PrepassPipelinePlugin` contains the part that is required for the shadow mapping to work and is unconditionally added.
  - `PrepassPlugin` now only adds the systems and resources required for the "real" prepasses.
- Add a noop fragment entry point to `prepass.wgsl`, used if `NORMAL_PASS` is not defined.


Co-authored-by: Edgar Geier <geieredgar@gmail.com>
  • Loading branch information
geieredgar and geieredgar committed Mar 3, 2023
1 parent 85c8fb9 commit cb0db07
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 13 deletions.
7 changes: 7 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,13 @@ description = "Showcases wireframe rendering"
category = "3D Rendering"
wasm = true

[[example]]
name = "no_prepass"
path = "tests/3d/no_prepass.rs"

[package.metadata.example.no_prepass]
hidden = true

# Animation
[[example]]
name = "animated_fox"
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
render, AlphaMode, DrawMesh, DrawPrepass, EnvironmentMapLight, MeshPipeline, MeshPipelineKey,
MeshUniform, PrepassPlugin, RenderLightSystems, SetMeshBindGroup, SetMeshViewBindGroup, Shadow,
MeshUniform, PrepassPipelinePlugin, PrepassPlugin, RenderLightSystems, SetMeshBindGroup,
SetMeshViewBindGroup, Shadow,
};
use bevy_app::{App, IntoSystemAppConfig, Plugin};
use bevy_asset::{AddAsset, AssetEvent, AssetServer, Assets, Handle};
Expand Down Expand Up @@ -207,6 +208,9 @@ where
.add_system(queue_material_meshes::<M>.in_set(RenderSet::Queue));
}

// PrepassPipelinePlugin is required for shadow mapping and the optional PrepassPlugin
app.add_plugin(PrepassPipelinePlugin::<M>::default());

if self.prepass_enabled {
app.add_plugin(PrepassPlugin::<M>::default());
}
Expand Down
52 changes: 40 additions & 12 deletions crates/bevy_pbr/src/prepass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,18 @@ pub const PREPASS_BINDINGS_SHADER_HANDLE: HandleUntyped =
pub const PREPASS_UTILS_SHADER_HANDLE: HandleUntyped =
HandleUntyped::weak_from_u64(Shader::TYPE_UUID, 4603948296044544);

pub struct PrepassPlugin<M: Material>(PhantomData<M>);
/// Sets up everything required to use the prepass pipeline.
///
/// This does not add the actual prepasses, see [`PrepassPlugin`] for that.
pub struct PrepassPipelinePlugin<M: Material>(PhantomData<M>);

impl<M: Material> Default for PrepassPlugin<M> {
impl<M: Material> Default for PrepassPipelinePlugin<M> {
fn default() -> Self {
Self(Default::default())
}
}

impl<M: Material> Plugin for PrepassPlugin<M>
impl<M: Material> Plugin for PrepassPipelinePlugin<M>
where
M::Data: PartialEq + Eq + Hash + Clone,
{
Expand All @@ -93,6 +96,34 @@ where
Shader::from_wgsl
);

let Ok(render_app) = app.get_sub_app_mut(RenderApp) else {
return;
};

render_app
.add_system(queue_prepass_view_bind_group::<M>.in_set(RenderSet::Queue))
.init_resource::<PrepassPipeline<M>>()
.init_resource::<PrepassViewBindGroup>()
.init_resource::<SpecializedMeshPipelines<PrepassPipeline<M>>>();
}
}

/// Sets up the prepasses for a [`Material`].
///
/// This depends on the [`PrepassPipelinePlugin`].
pub struct PrepassPlugin<M: Material>(PhantomData<M>);

impl<M: Material> Default for PrepassPlugin<M> {
fn default() -> Self {
Self(Default::default())
}
}

impl<M: Material> Plugin for PrepassPlugin<M>
where
M::Data: PartialEq + Eq + Hash + Clone,
{
fn build(&self, app: &mut bevy_app::App) {
let Ok(render_app) = app.get_sub_app_mut(RenderApp) else {
return;
};
Expand All @@ -104,15 +135,11 @@ where
.in_set(RenderSet::Prepare)
.after(bevy_render::view::prepare_windows),
)
.add_system(queue_prepass_view_bind_group::<M>.in_set(RenderSet::Queue))
.add_system(queue_prepass_material_meshes::<M>.in_set(RenderSet::Queue))
.add_system(sort_phase_system::<Opaque3dPrepass>.in_set(RenderSet::PhaseSort))
.add_system(sort_phase_system::<AlphaMask3dPrepass>.in_set(RenderSet::PhaseSort))
.init_resource::<PrepassPipeline<M>>()
.init_resource::<DrawFunctions<Opaque3dPrepass>>()
.init_resource::<DrawFunctions<AlphaMask3dPrepass>>()
.init_resource::<PrepassViewBindGroup>()
.init_resource::<SpecializedMeshPipelines<PrepassPipeline<M>>>()
.add_render_command::<Opaque3dPrepass, DrawPrepass<M>>()
.add_render_command::<AlphaMask3dPrepass, DrawPrepass<M>>();
}
Expand Down Expand Up @@ -254,11 +281,12 @@ where

let vertex_buffer_layout = layout.get_layout(&vertex_attributes)?;

// The fragment shader is only used when the normal prepass is enabled or the material uses alpha cutoff values
let fragment = if key
.mesh_key
.intersects(MeshPipelineKey::NORMAL_PREPASS | MeshPipelineKey::ALPHA_MASK)
|| blend_key == MeshPipelineKey::BLEND_PREMULTIPLIED_ALPHA
// The fragment shader is only used when the normal prepass is enabled
// or the material uses alpha cutoff values and doesn't rely on the standard prepass shader
let fragment = if key.mesh_key.contains(MeshPipelineKey::NORMAL_PREPASS)
|| ((key.mesh_key.contains(MeshPipelineKey::ALPHA_MASK)
|| blend_key == MeshPipelineKey::BLEND_PREMULTIPLIED_ALPHA)
&& self.material_fragment_shader.is_some())
{
// Use the fragment shader from the material if present
let frag_shader_handle = if let Some(handle) = &self.material_fragment_shader {
Expand Down
11 changes: 11 additions & 0 deletions tests/3d/no_prepass.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//! A test to confirm that `bevy` allows disabling the prepass of the standard material.
//! This is run in CI to ensure that this doesn't regress again.
use bevy::{pbr::PbrPlugin, prelude::*};

fn main() {
App::new()
.add_plugins(DefaultPlugins.set(PbrPlugin {
prepass_enabled: false,
}))
.run();
}

0 comments on commit cb0db07

Please sign in to comment.