refactor: update

Signed-off-by: Niu Zhihong <zhihong@nzhnb.com>

Author: MRNIU

Cargo.lock

@@ -5,11 +5,17 @@ edition = "2021"
 
 [dependencies]
 glam = "0.29"
-tobj = { version = "4.0", features = ["async"] }
+tobj = "4.0"
 image = { version = "0.25", default-features = false, features = ["png", "jpeg", "bmp", "tga"] }
 rayon = "1.10"
 log = "0.4"
 thiserror = "2"
 
 [dev-dependencies]
 env_logger = "0.11"
+proptest = "1"
+criterion = { version = "0.5", features = ["html_reports"] }
+
+[[bench]]
+name = "render_bench"
+harness = false

simple_renderer/Cargo.toml

@@ -0,0 +1,76 @@
+use criterion::{black_box, criterion_group, criterion_main, Criterion};
+use glam::{Mat4, Vec3};
+use simple_renderer::{Light, Model, RenderingMode, Shader, SimpleRenderer};
+
+const BENCH_W: usize = 200;
+const BENCH_H: usize = 150;
+const TEAPOT_PATH: &str = "../obj/utah-teapot-texture/teapot.obj";
+
+fn setup_shader() -> Shader {
+    let model_matrix = Mat4::from_scale(Vec3::splat(0.02))
+        * Mat4::from_translation(Vec3::new(0.0, -5.0, 0.0))
+        * Mat4::from_rotation_x((-105.0_f32).to_radians());
+
+    let mut shader = Shader::new();
+    shader.set_uniform("modelMatrix", model_matrix);
+    shader.set_uniform(
+        "viewMatrix",
+        Mat4::look_at_rh(Vec3::new(0.0, 0.0, 1.0), Vec3::ZERO, Vec3::Y),
+    );
+    shader.set_uniform(
+        "projectionMatrix",
+        Mat4::perspective_rh_gl(
+            60.0_f32.to_radians(),
+            BENCH_W as f32 / BENCH_H as f32,
+            0.1,
+            100.0,
+        ),
+    );
+    shader.set_uniform("cameraPos", Vec3::new(0.0, 0.0, 1.0));
+    shader.set_lights(&[
+        Light {
+            direction: Vec3::new(1.0, 5.0, 1.0),
+            ..Light::default()
+        },
+        Light {
+            direction: Vec3::new(-3.0, -2.0, 2.0),
+            ..Light::default()
+        },
+    ]);
+    shader
+}
+
+fn bench_render_mode(c: &mut Criterion, name: &str, mode: RenderingMode) {
+    let model = Model::load(TEAPOT_PATH).expect("load model");
+    let mut shader = setup_shader();
+    let mut renderer = SimpleRenderer::new(BENCH_W, BENCH_H);
+    renderer.set_rendering_mode(mode);
+    let mut buffer = vec![0u32; BENCH_W * BENCH_H];
+
+    c.bench_function(name, |b| {
+        b.iter(|| {
+            buffer.fill(0);
+            renderer
+                .draw_model(
+                    black_box(&model),
+                    black_box(&mut shader),
+                    black_box(&mut buffer),
+                )
+                .unwrap();
+        })
+    });
+}
+
+fn render_benchmarks(c: &mut Criterion) {
+    bench_render_mode(c, "per_triangle_200x150", RenderingMode::PerTriangle);
+    bench_render_mode(c, "tile_based_200x150", RenderingMode::TileBased);
+    bench_render_mode(c, "deferred_200x150", RenderingMode::Deferred);
+    bench_render_mode(
+        c,
+        "tile_based_deferred_200x150",
+        RenderingMode::TileBasedDeferred,
+    );
+}
+
+criterion_group!(benches, render_benchmarks);
+criterion_main!(benches);

simple_renderer/benches/render_bench.rs

@@ -35,30 +35,32 @@ impl Color {
     /// Create a color from floats in the `[0.0, 255.0]` range.
     ///
     /// Values are rounded via `+0.5` then truncated to `u8`.
+    /// Out-of-range values are clamped to `[0.0, 255.0]` before casting.
     /// Note: these are raw `[0, 255]` floats, NOT normalized `[0, 1]`.
     #[inline]
     pub fn from_f32(r: f32, g: f32, b: f32, a: f32) -> Self {
         Self {
             channels: [
-                (r + 0.5) as u8,
-                (g + 0.5) as u8,
-                (b + 0.5) as u8,
-                (a + 0.5) as u8,
+                ((r + 0.5).clamp(0.0, 255.0)) as u8,
+                ((g + 0.5).clamp(0.0, 255.0)) as u8,
+                ((b + 0.5).clamp(0.0, 255.0)) as u8,
+                ((a + 0.5).clamp(0.0, 255.0)) as u8,
             ],
         }
     }
 
     /// Create a color from normalized floats in the `[0.0, 1.0]` range.
     ///
     /// Each component is multiplied by 255.0, then truncated to `u8`.
+    /// Out-of-range values are clamped to `[0.0, 255.0]` before casting.
     #[inline]
     pub fn from_normalized(r: f32, g: f32, b: f32, a: f32) -> Self {
         Self {
             channels: [
-                (r * 255.0) as u8,
-                (g * 255.0) as u8,
-                (b * 255.0) as u8,
-                (a * 255.0) as u8,
+                ((r * 255.0).clamp(0.0, 255.0)) as u8,
+                ((g * 255.0).clamp(0.0, 255.0)) as u8,
+                ((b * 255.0).clamp(0.0, 255.0)) as u8,
+                ((a * 255.0).clamp(0.0, 255.0)) as u8,
             ],
         }
     }
@@ -253,6 +255,33 @@ mod tests {
         assert_eq!(c, Color::new(255, 255, 255, 255));
     }
 
+    #[test]
+    fn from_f32_clamps_negative() {
+        let c = Color::from_f32(-10.0, -5.0, 0.0, 255.0);
+        assert_eq!(c.r(), 0);
+        assert_eq!(c.g(), 0);
+        assert_eq!(c.b(), 0);
+        assert_eq!(c.a(), 255);
+    }
+
+    #[test]
+    fn from_f32_clamps_overflow() {
+        let c = Color::from_f32(300.0, 256.0, 255.0, 255.0);
+        assert_eq!(c.r(), 255);
+        assert_eq!(c.g(), 255);
+        assert_eq!(c.b(), 255);
+        assert_eq!(c.a(), 255);
+    }
+
+    #[test]
+    fn from_normalized_clamps_overflow() {
+        let c = Color::from_normalized(1.5, -0.5, 0.5, 1.0);
+        assert_eq!(c.r(), 255);
+        assert_eq!(c.g(), 0);
+        assert_eq!(c.b(), 127);
+        assert_eq!(c.a(), 255);
+    }
+
     // ── Constant tests ─────────────────────────────────────────────────
 
     #[test]

simple_renderer/src/color.rs

@@ -1,13 +1,22 @@
 use thiserror::Error;
 
 /// Errors that can occur in the renderer.
+#[non_exhaustive]
 #[derive(Debug, Error)]
 pub enum RendererError {
-    #[error("Model loading failed: {0}")]
-    ModelLoad(String),
+    #[error("Model loading failed: {path}")]
+    ModelLoad {
+        path: String,
+        #[source]
+        source: Box<dyn std::error::Error + Send + Sync>,
+    },
 
-    #[error("Texture loading failed: {0}")]
-    TextureLoad(String),
+    #[error("Texture loading failed: {path}")]
+    TextureLoad {
+        path: String,
+        #[source]
+        source: Box<dyn std::error::Error + Send + Sync>,
+    },
 
     #[error("Rendering failed: {0}")]
     RenderFailed(String),
@@ -25,8 +34,11 @@ mod tests {
 
     #[test]
     fn error_display() {
-        let e = RendererError::ModelLoad("file not found".into());
-        assert_eq!(e.to_string(), "Model loading failed: file not found");
+        let e = RendererError::ModelLoad {
+            path: "test.obj".into(),
+            source: "file not found".into(),
+        };
+        assert!(e.to_string().contains("test.obj"));
     }
 
     #[test]
@@ -45,4 +57,14 @@ mod tests {
         let err: Result<i32> = Err(RendererError::RenderFailed("oops".into()));
         assert!(err.is_err());
     }
+
+    #[test]
+    fn error_source_chain() {
+        use std::error::Error;
+        let e = RendererError::ModelLoad {
+            path: "model.obj".into(),
+            source: Box::new(std::io::Error::new(std::io::ErrorKind::NotFound, "missing")),
+        };
+        assert!(e.source().is_some());
+    }
 }

simple_renderer/src/error.rs

@@ -1,4 +1,5 @@
 use std::path::Path;
+use std::sync::Arc;
 
 use crate::color::Color;
 use crate::error::{RendererError, Result};
@@ -17,8 +18,10 @@ impl Texture {
     /// Load a texture from an image file (PNG, JPEG, BMP, TGA).
     pub fn load_from_file<P: AsRef<Path>>(path: P) -> Result<Texture> {
         let path = path.as_ref();
-        let img = image::open(path)
-            .map_err(|e| RendererError::TextureLoad(format!("{}: {}", path.display(), e)))?;
+        let img = image::open(path).map_err(|e| RendererError::TextureLoad {
+            path: path.display().to_string(),
+            source: Box::new(e),
+        })?;
 
         let rgba = img.to_rgba8();
         let (width, height) = rgba.dimensions();
@@ -64,9 +67,9 @@ pub struct Material {
     pub ambient: Vec3,
     pub diffuse: Vec3,
     pub specular: Vec3,
-    pub ambient_texture: Option<Texture>,
-    pub diffuse_texture: Option<Texture>,
-    pub specular_texture: Option<Texture>,
+    pub ambient_texture: Option<Arc<Texture>>,
+    pub diffuse_texture: Option<Arc<Texture>>,
+    pub specular_texture: Option<Arc<Texture>>,
 }
 
 impl Default for Material {

simple_renderer/src/material.rs

@@ -41,8 +41,11 @@ impl Model {
             ..Default::default()
         };
 
-        let (models, materials_result) = tobj::load_obj(obj_path, &load_options)
-            .map_err(|e| RendererError::ModelLoad(format!("{}: {}", path, e)))?;
+        let (models, materials_result) =
+            tobj::load_obj(obj_path, &load_options).map_err(|e| RendererError::ModelLoad {
+                path: path.to_string(),
+                source: Box::new(e),
+            })?;
 
         // Load materials — warn on failure, fall back to empty vec.
         let materials = match materials_result {
@@ -55,7 +58,7 @@ impl Model {
 
         let mut vertices: Vec<Vertex> = Vec::new();
         let mut faces: Vec<Face> = Vec::new();
-        let mut texture_cache: HashMap<PathBuf, Texture> = HashMap::new();
+        let mut texture_cache: HashMap<PathBuf, Arc<Texture>> = HashMap::new();
 
         for model in &models {
             let mesh = &model.mesh;
@@ -134,7 +137,7 @@ impl Model {
         material_id: Option<usize>,
         materials: &[tobj::Material],
         directory: &str,
-        texture_cache: &mut HashMap<PathBuf, Texture>,
+        texture_cache: &mut HashMap<PathBuf, Arc<Texture>>,
     ) -> Material {
         let mat_idx = match material_id {
             Some(idx) if idx < materials.len() => idx,
@@ -193,22 +196,23 @@ impl Model {
     fn load_texture_cached(
         texture_name: Option<&str>,
         directory: &str,
-        cache: &mut HashMap<PathBuf, Texture>,
-    ) -> Option<Texture> {
+        cache: &mut HashMap<PathBuf, Arc<Texture>>,
+    ) -> Option<Arc<Texture>> {
         let name = texture_name?;
         if name.is_empty() {
             return None;
         }
 
         let full_path = PathBuf::from(directory).join(name);
 
-        if let Some(cached) = cache.get(&full_path) {
-            return Some(cached.clone());
+        if cache.contains_key(&full_path) {
+            return Some(Arc::clone(cache.get(&full_path).unwrap()));
         }
 
         match Texture::load_from_file(&full_path) {
             Ok(tex) => {
-                cache.insert(full_path, tex.clone());
+                let tex = Arc::new(tex);
+                cache.insert(full_path, Arc::clone(&tex));
                 Some(tex)
             }
             Err(e) => {
@@ -244,8 +248,8 @@ mod tests {
         let result = Model::load("/nonexistent/path/model.obj");
         assert!(result.is_err());
         match result.unwrap_err() {
-            RendererError::ModelLoad(msg) => {
-                assert!(!msg.is_empty());
+            RendererError::ModelLoad { path, .. } => {
+                assert!(!path.is_empty());
             }
             other => panic!("Expected ModelLoad error, got: {:?}", other),
         }

simple_renderer/src/model.rs

@@ -7,6 +7,7 @@ use crate::renderers::Renderer;
 use crate::shader::Shader;
 use log::info;
 
+#[non_exhaustive]
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[repr(u8)]
 pub enum RenderingMode {
@@ -27,6 +28,20 @@ impl std::fmt::Display for RenderingMode {
     }
 }
 
+impl TryFrom<u8> for RenderingMode {
+    type Error = u8;
+
+    fn try_from(value: u8) -> std::result::Result<Self, Self::Error> {
+        match value {
+            0 => Ok(Self::PerTriangle),
+            1 => Ok(Self::TileBased),
+            2 => Ok(Self::Deferred),
+            3 => Ok(Self::TileBasedDeferred),
+            other => Err(other),
+        }
+    }
+}
+
 pub struct SimpleRenderer {
     width: usize,
     height: usize,
@@ -167,4 +182,21 @@ mod tests {
             assert_eq!(r.rendering_mode(), *mode);
         }
     }
+
+    #[test]
+    fn try_from_u8_valid() {
+        assert_eq!(RenderingMode::try_from(0), Ok(RenderingMode::PerTriangle));
+        assert_eq!(RenderingMode::try_from(1), Ok(RenderingMode::TileBased));
+        assert_eq!(RenderingMode::try_from(2), Ok(RenderingMode::Deferred));
+        assert_eq!(
+            RenderingMode::try_from(3),
+            Ok(RenderingMode::TileBasedDeferred)
+        );
+    }
+
+    #[test]
+    fn try_from_u8_invalid() {
+        assert_eq!(RenderingMode::try_from(4), Err(4));
+        assert_eq!(RenderingMode::try_from(255), Err(255));
+    }
 }