Skip to content

Commit

Permalink
Refactor rendering systems to use let-else (#9870)
Browse files Browse the repository at this point in the history 
# Objective

Some rendering system did heavy use of `if let`, and could be improved
by using `let else`.

## Solution

- Reduce rightward drift by using let-else over if-let
- Extract value-to-key mappings to their own functions so that the
system is less bloated, easier to understand
- Use a `let` binding instead of untupling in closure argument to reduce
indentation

## Note to reviewers

Enable the "no white space diff" for easier viewing.
In the "Files changed" view, click on the little cog right of the "Jump
to" text, on the row where the "Review changes" button is. then enable
the "Hide whitespace" checkbox and click reload.
  • Loading branch information
@nicopap
nicopap authored Sep 20, 2023
1 parent 9873c97 commit 47d87e4
Show file tree
Hide file tree
Showing 4 changed files with 260 additions and 268 deletions.
201 changes: 97 additions & 104 deletions crates/bevy_pbr/src/material.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,33 @@ impl<P: PhaseItem, M: Material, const I: usize> RenderCommand<P> for SetMaterial
}
}

const fn alpha_mode_pipeline_key(alpha_mode: AlphaMode) -> MeshPipelineKey {
match alpha_mode {
// Premultiplied and Add share the same pipeline key
// They're made distinct in the PBR shader, via `premultiply_alpha()`
AlphaMode::Premultiplied | AlphaMode::Add => MeshPipelineKey::BLEND_PREMULTIPLIED_ALPHA,
AlphaMode::Blend => MeshPipelineKey::BLEND_ALPHA,
AlphaMode::Multiply => MeshPipelineKey::BLEND_MULTIPLY,
AlphaMode::Mask(_) => MeshPipelineKey::MAY_DISCARD,
_ => MeshPipelineKey::NONE,
}
}

const fn tonemapping_pipeline_key(tonemapping: Tonemapping) -> MeshPipelineKey {
match tonemapping {
Tonemapping::None => MeshPipelineKey::TONEMAP_METHOD_NONE,
Tonemapping::Reinhard => MeshPipelineKey::TONEMAP_METHOD_REINHARD,
Tonemapping::ReinhardLuminance => MeshPipelineKey::TONEMAP_METHOD_REINHARD_LUMINANCE,
Tonemapping::AcesFitted => MeshPipelineKey::TONEMAP_METHOD_ACES_FITTED,
Tonemapping::AgX => MeshPipelineKey::TONEMAP_METHOD_AGX,
Tonemapping::SomewhatBoringDisplayTransform => {
MeshPipelineKey::TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM
}
Tonemapping::TonyMcMapface => MeshPipelineKey::TONEMAP_METHOD_TONY_MC_MAPFACE,
Tonemapping::BlenderFilmic => MeshPipelineKey::TONEMAP_METHOD_BLENDER_FILMIC,
}
}

#[allow(clippy::too_many_arguments)]
pub fn queue_material_meshes<M: Material>(
opaque_draw_functions: Res<DrawFunctions<Opaque3d>>,
Expand Down Expand Up @@ -418,131 +445,97 @@ pub fn queue_material_meshes<M: Material>(
if normal_prepass.is_some() {
view_key |= MeshPipelineKey::NORMAL_PREPASS;
}

if taa_settings.is_some() {
view_key |= MeshPipelineKey::TAA;
}
let environment_map_loaded = environment_map.is_some_and(|map| map.is_loaded(&images));

let environment_map_loaded = match environment_map {
Some(environment_map) => environment_map.is_loaded(&images),
None => false,
};
if environment_map_loaded {
view_key |= MeshPipelineKey::ENVIRONMENT_MAP;
}

if !view.hdr {
if let Some(tonemapping) = tonemapping {
view_key |= MeshPipelineKey::TONEMAP_IN_SHADER;
view_key |= match tonemapping {
Tonemapping::None => MeshPipelineKey::TONEMAP_METHOD_NONE,
Tonemapping::Reinhard => MeshPipelineKey::TONEMAP_METHOD_REINHARD,
Tonemapping::ReinhardLuminance => {
MeshPipelineKey::TONEMAP_METHOD_REINHARD_LUMINANCE
}
Tonemapping::AcesFitted => MeshPipelineKey::TONEMAP_METHOD_ACES_FITTED,
Tonemapping::AgX => MeshPipelineKey::TONEMAP_METHOD_AGX,
Tonemapping::SomewhatBoringDisplayTransform => {
MeshPipelineKey::TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM
}
Tonemapping::TonyMcMapface => MeshPipelineKey::TONEMAP_METHOD_TONY_MC_MAPFACE,
Tonemapping::BlenderFilmic => MeshPipelineKey::TONEMAP_METHOD_BLENDER_FILMIC,
};
view_key |= tonemapping_pipeline_key(*tonemapping);
}
if let Some(DebandDither::Enabled) = dither {
view_key |= MeshPipelineKey::DEBAND_DITHER;
}
}

if ssao.is_some() {
view_key |= MeshPipelineKey::SCREEN_SPACE_AMBIENT_OCCLUSION;
}

let rangefinder = view.rangefinder3d();
for visible_entity in &visible_entities.entities {
if let Ok((material_handle, mesh_handle, mesh_transforms)) =
let Ok((material_handle, mesh_handle, mesh_transforms)) =
material_meshes.get(*visible_entity)
{
if let (Some(mesh), Some(material)) = (
render_meshes.get(mesh_handle),
render_materials.get(&material_handle.id()),
) {
let mut mesh_key =
MeshPipelineKey::from_primitive_topology(mesh.primitive_topology)
| view_key;
if mesh.morph_targets.is_some() {
mesh_key |= MeshPipelineKey::MORPH_TARGETS;
}
match material.properties.alpha_mode {
AlphaMode::Blend => {
mesh_key |= MeshPipelineKey::BLEND_ALPHA;
}
AlphaMode::Premultiplied | AlphaMode::Add => {
// Premultiplied and Add share the same pipeline key
// They're made distinct in the PBR shader, via `premultiply_alpha()`
mesh_key |= MeshPipelineKey::BLEND_PREMULTIPLIED_ALPHA;
}
AlphaMode::Multiply => {
mesh_key |= MeshPipelineKey::BLEND_MULTIPLY;
}
AlphaMode::Mask(_) => {
mesh_key |= MeshPipelineKey::MAY_DISCARD;
}
_ => (),
}

let pipeline_id = pipelines.specialize(
&pipeline_cache,
&material_pipeline,
MaterialPipelineKey {
mesh_key,
bind_group_data: material.key.clone(),
},
&mesh.layout,
);
let pipeline_id = match pipeline_id {
Ok(id) => id,
Err(err) => {
error!("{}", err);
continue;
}
};

let distance = rangefinder
.distance_translation(&mesh_transforms.transform.translation)
+ material.properties.depth_bias;
match material.properties.alpha_mode {
AlphaMode::Opaque => {
opaque_phase.add(Opaque3d {
entity: *visible_entity,
draw_function: draw_opaque_pbr,
pipeline: pipeline_id,
distance,
batch_size: 1,
});
}
AlphaMode::Mask(_) => {
alpha_mask_phase.add(AlphaMask3d {
entity: *visible_entity,
draw_function: draw_alpha_mask_pbr,
pipeline: pipeline_id,
distance,
batch_size: 1,
});
}
AlphaMode::Blend
| AlphaMode::Premultiplied
| AlphaMode::Add
| AlphaMode::Multiply => {
transparent_phase.add(Transparent3d {
entity: *visible_entity,
draw_function: draw_transparent_pbr,
pipeline: pipeline_id,
distance,
batch_size: 1,
});
}
}
else {
continue;
};
let Some(mesh) = render_meshes.get(mesh_handle) else {
continue;
};
let Some(material) = render_materials.get(&material_handle.id()) else {
continue;
};
let mut mesh_key = view_key;

mesh_key |= MeshPipelineKey::from_primitive_topology(mesh.primitive_topology);

if mesh.morph_targets.is_some() {
mesh_key |= MeshPipelineKey::MORPH_TARGETS;
}
mesh_key |= alpha_mode_pipeline_key(material.properties.alpha_mode);

let pipeline_id = pipelines.specialize(
&pipeline_cache,
&material_pipeline,
MaterialPipelineKey {
mesh_key,
bind_group_data: material.key.clone(),
},
&mesh.layout,
);
let pipeline_id = match pipeline_id {
Ok(id) => id,
Err(err) => {
error!("{}", err);
continue;
}
};

let distance = rangefinder.distance_translation(&mesh_transforms.transform.translation)
+ material.properties.depth_bias;
match material.properties.alpha_mode {
AlphaMode::Opaque => {
opaque_phase.add(Opaque3d {
entity: *visible_entity,
draw_function: draw_opaque_pbr,
pipeline: pipeline_id,
distance,
batch_size: 1,
});
}
AlphaMode::Mask(_) => {
alpha_mask_phase.add(AlphaMask3d {
entity: *visible_entity,
draw_function: draw_alpha_mask_pbr,
pipeline: pipeline_id,
distance,
batch_size: 1,
});
}
AlphaMode::Blend
| AlphaMode::Premultiplied
| AlphaMode::Add
| AlphaMode::Multiply => {
transparent_phase.add(Transparent3d {
entity: *visible_entity,
draw_function: draw_transparent_pbr,
pipeline: pipeline_id,
distance,
batch_size: 1,
});
}
}
}
Expand Down
Loading

0 comments on commit 47d87e4

Please sign in to comment.