Refactor rendering systems to use let-else (#9870)

# 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.

crates/bevy_pbr/src/material.rs

@@ -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>>,
@@ -418,78 +445,47 @@ 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 {
+                view_key |= tonemapping_pipeline_key(*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,
-                };
             }
             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)
-            {
+            else {
-                if let (Some(mesh), Some(material)) = (
+                continue;
-                    render_meshes.get(mesh_handle),
+            };
-                    render_materials.get(&material_handle.id()),
+            let Some(mesh) = render_meshes.get(mesh_handle) else {
-                ) {
+                continue;
-                    let mut mesh_key =
+            };
-                        MeshPipelineKey::from_primitive_topology(mesh.primitive_topology)
+            let Some(material) = render_materials.get(&material_handle.id()) else {
-                            | view_key;
+                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;
             }
-                    match material.properties.alpha_mode {
+            mesh_key |= alpha_mode_pipeline_key(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,
@@ -508,8 +504,7 @@ pub fn queue_material_meshes<M: Material>(
                 }
             };
 
-                    let distance = rangefinder
+            let distance = rangefinder.distance_translation(&mesh_transforms.transform.translation)
-                        .distance_translation(&mesh_transforms.transform.translation)
                 + material.properties.depth_bias;
             match material.properties.alpha_mode {
                 AlphaMode::Opaque => {
@@ -545,8 +540,6 @@ pub fn queue_material_meshes<M: Material>(
             }
         }
     }
-        }
-    }
 }
 
 /// Common [`Material`] properties, calculated for a specific material instance.

crates/bevy_pbr/src/render/light.rs

@@ -346,19 +346,18 @@ pub fn extract_lights(
 
     let mut point_lights_values = Vec::with_capacity(*previous_point_lights_len);
     for entity in global_point_lights.iter().copied() {
-        if let Ok((point_light, cubemap_visible_entities, transform, view_visibility)) =
+        let Ok((point_light, cubemap_visible_entities, transform, view_visibility)) =
             point_lights.get(entity)
-        {
+        else {
+            continue;
+        };
         if !view_visibility.get() {
             continue;
         }
         // TODO: This is very much not ideal. We should be able to re-use the vector memory.
         // However, since exclusive access to the main world in extract is ill-advised, we just clone here.
         let render_cubemap_visible_entities = cubemap_visible_entities.clone();
-            point_lights_values.push((
+        let extracted_point_light = ExtractedPointLight {
-                entity,
-                (
-                    ExtractedPointLight {
             color: point_light.color,
             // NOTE: Map from luminous power in lumens to luminous intensity in lumens per steradian
             // for a point light. See https://google.github.io/filament/Filament.html#mjx-eqn-pointLightLuminousPower
@@ -374,12 +373,12 @@ pub fn extract_lights(
                 * point_light_texel_size
                 * std::f32::consts::SQRT_2,
             spot_light_angles: None,
-                    },
+        };
-                    render_cubemap_visible_entities,
+        point_lights_values.push((
-                ),
+            entity,
+            (extracted_point_light, render_cubemap_visible_entities),
         ));
     }
-    }
     *previous_point_lights_len = point_lights_values.len();
     commands.insert_or_spawn_batch(point_lights_values);
 
@@ -1594,11 +1593,15 @@ pub fn queue_shadows<M: Material>(
             // NOTE: Lights with shadow mapping disabled will have no visible entities
             // so no meshes will be queued
             for entity in visible_entities.iter().copied() {
-                if let Ok((mesh_handle, material_handle)) = casting_meshes.get(entity) {
+                let Ok((mesh_handle, material_handle)) = casting_meshes.get(entity) else {
-                    if let (Some(mesh), Some(material)) = (
+                    continue;
-                        render_meshes.get(mesh_handle),
+                };
-                        render_materials.get(&material_handle.id()),
+                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 =
                     MeshPipelineKey::from_primitive_topology(mesh.primitive_topology)
                         | MeshPipelineKey::DEPTH_PREPASS;
@@ -1608,16 +1611,13 @@ pub fn queue_shadows<M: Material>(
                 if is_directional_light {
                     mesh_key |= MeshPipelineKey::DEPTH_CLAMP_ORTHO;
                 }
-                        let alpha_mode = material.properties.alpha_mode;
+                mesh_key |= match material.properties.alpha_mode {
-                        match alpha_mode {
                     AlphaMode::Mask(_)
                     | AlphaMode::Blend
                     | AlphaMode::Premultiplied
-                            | AlphaMode::Add => {
+                    | AlphaMode::Add => MeshPipelineKey::MAY_DISCARD,
-                                mesh_key |= MeshPipelineKey::MAY_DISCARD;
+                    _ => MeshPipelineKey::NONE,
-                            }
+                };
-                            _ => {}
-                        }
                 let pipeline_id = pipelines.specialize(
                     &pipeline_cache,
                     &prepass_pipeline,
@@ -1645,8 +1645,6 @@ pub fn queue_shadows<M: Material>(
             }
         }
     }
-        }
-    }
 }
 
 pub struct Shadow {

crates/bevy_pbr/src/wireframe.rs

@@ -127,20 +127,19 @@ fn queue_wireframes(
         let rangefinder = view.rangefinder3d();
 
         let view_key = msaa_key | MeshPipelineKey::from_hdr(view.hdr);
-        let add_render_phase =
+        let add_render_phase = |phase_item: (Entity, &Handle<Mesh>, &MeshTransforms)| {
-            |(entity, mesh_handle, mesh_transforms): (Entity, &Handle<Mesh>, &MeshTransforms)| {
+            let (entity, mesh_handle, mesh_transforms) = phase_item;
-                if let Some(mesh) = render_meshes.get(mesh_handle) {
+
-                    let mut key = view_key
+            let Some(mesh) = render_meshes.get(mesh_handle) else {
-                        | MeshPipelineKey::from_primitive_topology(mesh.primitive_topology);
+                return;
+            };
+            let mut key =
+                view_key | MeshPipelineKey::from_primitive_topology(mesh.primitive_topology);
             if mesh.morph_targets.is_some() {
                 key |= MeshPipelineKey::MORPH_TARGETS;
             }
-                    let pipeline_id = pipelines.specialize(
+            let pipeline_id =
-                        &pipeline_cache,
+                pipelines.specialize(&pipeline_cache, &wireframe_pipeline, key, &mesh.layout);
-                        &wireframe_pipeline,
-                        key,
-                        &mesh.layout,
-                    );
             let pipeline_id = match pipeline_id {
                 Ok(id) => id,
                 Err(err) => {
@@ -152,11 +151,9 @@ fn queue_wireframes(
                 entity,
                 pipeline: pipeline_id,
                 draw_function: draw_custom,
-                        distance: rangefinder
+                distance: rangefinder.distance_translation(&mesh_transforms.transform.translation),
-                            .distance_translation(&mesh_transforms.transform.translation),
                 batch_size: 1,
             });
-                }
         };
 
         if wireframe_config.global {

crates/bevy_sprite/src/mesh2d/material.rs

@@ -319,6 +319,21 @@ impl<P: PhaseItem, M: Material2d, const I: usize> RenderCommand<P>
     }
 }
 
+const fn tonemapping_pipeline_key(tonemapping: Tonemapping) -> Mesh2dPipelineKey {
+    match tonemapping {
+        Tonemapping::None => Mesh2dPipelineKey::TONEMAP_METHOD_NONE,
+        Tonemapping::Reinhard => Mesh2dPipelineKey::TONEMAP_METHOD_REINHARD,
+        Tonemapping::ReinhardLuminance => Mesh2dPipelineKey::TONEMAP_METHOD_REINHARD_LUMINANCE,
+        Tonemapping::AcesFitted => Mesh2dPipelineKey::TONEMAP_METHOD_ACES_FITTED,
+        Tonemapping::AgX => Mesh2dPipelineKey::TONEMAP_METHOD_AGX,
+        Tonemapping::SomewhatBoringDisplayTransform => {
+            Mesh2dPipelineKey::TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM
+        }
+        Tonemapping::TonyMcMapface => Mesh2dPipelineKey::TONEMAP_METHOD_TONY_MC_MAPFACE,
+        Tonemapping::BlenderFilmic => Mesh2dPipelineKey::TONEMAP_METHOD_BLENDER_FILMIC,
+    }
+}
+
 #[allow(clippy::too_many_arguments)]
 pub fn queue_material2d_meshes<M: Material2d>(
     transparent_draw_functions: Res<DrawFunctions<Transparent2d>>,
@@ -352,34 +367,26 @@ pub fn queue_material2d_meshes<M: Material2d>(
         if !view.hdr {
             if let Some(tonemapping) = tonemapping {
                 view_key |= Mesh2dPipelineKey::TONEMAP_IN_SHADER;
-                view_key |= match tonemapping {
+                view_key |= tonemapping_pipeline_key(*tonemapping);
-                    Tonemapping::None => Mesh2dPipelineKey::TONEMAP_METHOD_NONE,
-                    Tonemapping::Reinhard => Mesh2dPipelineKey::TONEMAP_METHOD_REINHARD,
-                    Tonemapping::ReinhardLuminance => {
-                        Mesh2dPipelineKey::TONEMAP_METHOD_REINHARD_LUMINANCE
-                    }
-                    Tonemapping::AcesFitted => Mesh2dPipelineKey::TONEMAP_METHOD_ACES_FITTED,
-                    Tonemapping::AgX => Mesh2dPipelineKey::TONEMAP_METHOD_AGX,
-                    Tonemapping::SomewhatBoringDisplayTransform => {
-                        Mesh2dPipelineKey::TONEMAP_METHOD_SOMEWHAT_BORING_DISPLAY_TRANSFORM
-                    }
-                    Tonemapping::TonyMcMapface => Mesh2dPipelineKey::TONEMAP_METHOD_TONY_MC_MAPFACE,
-                    Tonemapping::BlenderFilmic => Mesh2dPipelineKey::TONEMAP_METHOD_BLENDER_FILMIC,
-                };
             }
             if let Some(DebandDither::Enabled) = dither {
                 view_key |= Mesh2dPipelineKey::DEBAND_DITHER;
             }
         }
-
         for visible_entity in &visible_entities.entities {
-            if let Ok((material2d_handle, mesh2d_handle, mesh2d_uniform)) =
+            let Ok((material2d_handle, mesh2d_handle, mesh2d_uniform)) =
                 material2d_meshes.get(*visible_entity)
-            {
+            else {
-                if let Some(material2d) = render_materials.get(&material2d_handle.id()) {
+                continue;
-                    if let Some(mesh) = render_meshes.get(&mesh2d_handle.0) {
+            };
-                        let mesh_key = view_key
+            let Some(material2d) = render_materials.get(&material2d_handle.id()) else {
-                            | Mesh2dPipelineKey::from_primitive_topology(mesh.primitive_topology);
+                continue;
+            };
+            let Some(mesh) = render_meshes.get(&mesh2d_handle.0) else {
+                continue;
+            };
+            let mesh_key =
+                view_key | Mesh2dPipelineKey::from_primitive_topology(mesh.primitive_topology);
 
             let pipeline_id = pipelines.specialize(
                 &pipeline_cache,
@@ -414,9 +421,6 @@ pub fn queue_material2d_meshes<M: Material2d>(
             });
         }
     }
-            }
-        }
-    }
 }
 
 /// Data prepared for a [`Material2d`] instance.