Fixes and improvments from codereview

Fixed not normalizing normal in fragment shader

Many docstring and comment improvements

Author: sweepline

crates/bevy_pbr/src/material.rs

@@ -42,7 +42,11 @@ pub struct StandardMaterial {
     #[render_resources(ignore)]
     #[shader_def]
     pub unlit: bool,
-    /// This allows for flat shading even with indiced meshes.
+    /// Flat shading makes the shader generate normals per face instead of per vertex.
+    /// This means that you get a uniform color over the whole triangle (think Virtua Racing).
+    /// The method uses fragment shader derivatives which can cause some artifacts in some cases.
+    /// An alternative is using `Mesh::duplicate_vertices()` and `Mesh::compute_flat_normals()`
+    /// and setting this setting to false, this will make sure no artifacts occur.
     #[render_resources(ignore)]
     #[shader_def]
     pub flat_shading: bool,

crates/bevy_pbr/src/render_graph/pbr_pipeline/pbr.frag

@@ -373,9 +373,8 @@ void main() {
 
 #    ifdef STANDARDMATERIAL_FLAT_SHADING
     vec3 N = normalize(cross(dFdy(v_WorldPosition), dFdx(v_WorldPosition)));
-#    endif
-#    ifndef STANDARDMATERIAL_FLAT_SHADING
-    vec3 N = v_WorldNormal;
+#    else
+    vec3 N = normalize(v_WorldNormal);
 #    endif
 
 #    ifdef STANDARDMATERIAL_NORMAL_MAP

examples/3d/flat_shading.rs

@@ -1,5 +1,8 @@
 use bevy::prelude::*;
 
+/// This example shows the difference between flat shading
+/// and smooth shading (default) in `StandardMaterial`.
+/// Flat shading gives a much more "Polygonal" or "Retro" look to meshes.
 fn main() {
     App::new()
         .insert_resource(Msaa { samples: 4 })
@@ -14,30 +17,31 @@ fn setup(
     mut meshes: ResMut<Assets<Mesh>>,
     mut materials: ResMut<Assets<StandardMaterial>>,
 ) {
-    // flat
+    // Flat shaded icosphere (ORANGE)
     commands.spawn_bundle(PbrBundle {
         mesh: meshes.add(Mesh::from(shape::Icosphere {
             radius: 0.5,
             subdivisions: 4,
         })),
         material: materials.add(StandardMaterial {
-            base_color: Color::rgb(0.8, 0.7, 0.6),
+            base_color: Color::ORANGE,
             flat_shading: true,
             ..Default::default()
         }),
         transform: Transform::from_xyz(-0.55, 0.5, 0.0),
         ..Default::default()
     });
-    // smooth
+    // Smooth shaded icosphere (BLUE)
     commands.spawn_bundle(PbrBundle {
         mesh: meshes.add(Mesh::from(shape::Icosphere {
             radius: 0.5,
             subdivisions: 4,
         })),
-        material: materials.add(Color::rgb(0.8, 0.7, 0.6).into()),
+        material: materials.add(Color::BLUE.into()),
         transform: Transform::from_xyz(0.55, 0.5, 0.0),
         ..Default::default()
     });
+
     // plane
     commands.spawn_bundle(PbrBundle {
         mesh: meshes.add(Mesh::from(shape::Plane { size: 5.0 })),

examples/README.md

@@ -107,7 +107,7 @@ Example | File | Description
 `update_gltf_scene` | [`3d/update_gltf_scene.rs`](./3d/update_gltf_scene.rs) | Update a scene from a gltf file, either by spawning the scene as a child of another entity, or by accessing the entities of the scene
 `wireframe` | [`3d/wireframe.rs`](./3d/wireframe.rs) | Showcases wireframe rendering
 `z_sort_debug` | [`3d/z_sort_debug.rs`](./3d/z_sort_debug.rs) | Visualizes camera Z-ordering
-`flat_shading` | [`3d/flat_shading.rs`](./3d/flat_shading.rs) | Simple 3D scene showing flat and normal (smooth) shading
+`flat_shading` | [`3d/flat_shading.rs`](./3d/flat_shading.rs) | Simple 3D scene showing flat and smooth (default) shading
 
 ## Application