From 93c87fc9b1a199a06f4ed5a49e589057afb2fd78 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 25 Jun 2026 22:15:40 +0200 Subject: [PATCH] refactor: bundle render args into RenderParams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group the five shared pose/shading args (azimuth, altitude, zoom, light_dir, fg_override) into a RenderParams struct passed by reference. render_frame goes 7 args -> 3, render_frame_gpu 8 -> 4, which drops render_frame_gpu's #[allow(clippy::too_many_arguments)] (rasterize_triangle's per-triangle one stays). Behavior-preserving: GPU/CPU output stays byte-identical (gpu_compare 0.00%), full suite green (41 tests). Addresses the RenderParams judgment call in #3 — opened as a draft so the API shape can be reviewed before committing to it pre-1.0. --- src/lib.rs | 61 ++++++++++++++++++++++++++++-------------- src/main.rs | 15 +++++++---- tests/empty_mesh.rs | 16 ++++++++--- tests/gen_snapshots.rs | 11 ++++++-- tests/gpu_compare.rs | 13 ++++++--- tests/gpu_resize.rs | 11 ++++++-- tests/regression.rs | 11 ++++++-- 7 files changed, 101 insertions(+), 37 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 18767c9..2b1127c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,12 +13,18 @@ //! use glam::Vec3; //! use a3d::model::load_model; //! use a3d::render::Framebuffer; -//! use a3d::{render_frame, framebuffer_to_string}; +//! use a3d::{render_frame, framebuffer_to_string, RenderParams}; //! //! let mesh = load_model(Path::new("models/dog.stl")).expect("load model"); //! let mut fb = Framebuffer::new(80, 24); -//! let light = Vec3::new(1.0, -1.0, 0.0).normalize(); -//! render_frame(&mut fb, &mesh, 0.0, 0.0, 1.0, light, None); +//! let params = RenderParams { +//! azimuth: 0.0, +//! altitude: 0.0, +//! zoom: 1.0, +//! light_dir: Vec3::new(1.0, -1.0, 0.0).normalize(), +//! fg_override: None, +//! }; +//! render_frame(&mut fb, &mesh, ¶ms); //! print!("{}", framebuffer_to_string(&fb)); //! ``` //! @@ -57,17 +63,32 @@ pub fn rotate_x(v: Vec3, cos_a: f32, sin_a: f32) -> Vec3 { Vec3::new(v.x, v.y * cos_a - v.z * sin_a, v.y * sin_a + v.z * cos_a) } +/// Camera pose and shading parameters shared by the CPU and GPU renderers. +#[derive(Clone, Copy, Debug)] +pub struct RenderParams { + /// Orbit azimuth in radians. + pub azimuth: f32, + /// Orbit altitude in radians. + pub altitude: f32, + /// Zoom factor applied to projected X/Y (depth is left unscaled). + pub zoom: f32, + /// Direction the scene light points toward (expected normalized). + pub light_dir: Vec3, + /// Optional foreground color override in linear RGB `[0, 1]`; `None` keeps + /// each triangle's own material/vertex color. + pub fg_override: Option<[f32; 3]>, +} + /// Render a single frame into the framebuffer. /// This is the core rendering function, usable without a terminal for testing. -pub fn render_frame( - fb: &mut Framebuffer, - mesh: &model::Mesh, - azimuth: f32, - altitude: f32, - zoom: f32, - light_dir: Vec3, - fg_override: Option<[f32; 3]>, -) { +pub fn render_frame(fb: &mut Framebuffer, mesh: &model::Mesh, params: &RenderParams) { + let RenderParams { + azimuth, + altitude, + zoom, + light_dir, + fg_override, + } = *params; let w = fb.width; let h = fb.height; @@ -127,19 +148,19 @@ pub fn render_frame( } /// Render a single frame using the GPU compute pipeline. -// Mirrors `render_frame`'s parameter set; a `RenderParams` struct is tracked as -// follow-up cleanup (see issue #3). -#[allow(clippy::too_many_arguments)] pub fn render_frame_gpu( fb: &mut Framebuffer, pipeline: &RasterPipeline, ctx: &GpuContext, - azimuth: f32, - altitude: f32, - zoom: f32, - light_dir: Vec3, - fg_override: Option<[f32; 3]>, + params: &RenderParams, ) { + let RenderParams { + azimuth, + altitude, + zoom, + light_dir, + fg_override, + } = *params; let w = fb.width; let h = fb.height; diff --git a/src/main.rs b/src/main.rs index 3ebfd3f..6414821 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,7 +11,7 @@ use a3d::gpu::{GpuContext, RasterPipeline}; use a3d::model::load_model; use a3d::render::Framebuffer; use a3d::terminal::{InputEvent, TerminalDisplay}; -use a3d::{AL_SPEED, AZ_SPEED, render_frame, render_frame_gpu}; +use a3d::{AL_SPEED, AZ_SPEED, RenderParams, render_frame, render_frame_gpu}; #[derive(Parser)] #[command(name = "a3d", about = "GPU-accelerated ASCII 3D renderer")] @@ -184,12 +184,17 @@ fn main() { } // Render + let params = RenderParams { + azimuth, + altitude, + zoom, + light_dir, + fg_override: fg_color, + }; if let (Some(pipeline), Some(ctx)) = (&gpu_pipeline, &gpu_ctx) { - render_frame_gpu( - &mut fb, pipeline, ctx, azimuth, altitude, zoom, light_dir, fg_color, - ); + render_frame_gpu(&mut fb, pipeline, ctx, ¶ms); } else { - render_frame(&mut fb, &mesh, azimuth, altitude, zoom, light_dir, fg_color); + render_frame(&mut fb, &mesh, ¶ms); } let fps_label = if current_fps > 0.0 { diff --git a/tests/empty_mesh.rs b/tests/empty_mesh.rs index 8b2b16f..b1a545e 100644 --- a/tests/empty_mesh.rs +++ b/tests/empty_mesh.rs @@ -6,7 +6,7 @@ use glam::Vec3; use a3d::gpu::{GpuContext, RasterPipeline}; use a3d::model::Mesh; use a3d::render::Framebuffer; -use a3d::{framebuffer_to_string, render_frame, render_frame_gpu}; +use a3d::{RenderParams, framebuffer_to_string, render_frame, render_frame_gpu}; const LIGHT_DIR: Vec3 = Vec3::new(0.70710677, -0.70710677, 0.0); const W: usize = 80; @@ -23,10 +23,20 @@ fn is_blank(s: &str) -> bool { s.chars().all(|c| c == ' ' || c == '\n') } +fn front_view() -> RenderParams { + RenderParams { + azimuth: 0.0, + altitude: 0.0, + zoom: 1.0, + light_dir: LIGHT_DIR, + fg_override: None, + } +} + #[test] fn cpu_renders_empty_mesh_blank() { let mut fb = Framebuffer::new(W, H); - render_frame(&mut fb, &empty_mesh(), 0.0, 0.0, 1.0, LIGHT_DIR, None); + render_frame(&mut fb, &empty_mesh(), &front_view()); assert!( is_blank(&framebuffer_to_string(&fb)), "empty mesh should render blank on the CPU" @@ -42,7 +52,7 @@ fn gpu_builds_and_renders_empty_mesh_without_crashing() { // Constructing the pipeline used to panic here on a zero-sized buffer. let pipeline = RasterPipeline::new(&ctx, &empty_mesh(), W as u32, H as u32); let mut fb = Framebuffer::new(W, H); - render_frame_gpu(&mut fb, &pipeline, &ctx, 0.0, 0.0, 1.0, LIGHT_DIR, None); + render_frame_gpu(&mut fb, &pipeline, &ctx, &front_view()); assert!( is_blank(&framebuffer_to_string(&fb)), "empty mesh should render blank on the GPU" diff --git a/tests/gen_snapshots.rs b/tests/gen_snapshots.rs index 261cf65..e925971 100644 --- a/tests/gen_snapshots.rs +++ b/tests/gen_snapshots.rs @@ -6,7 +6,7 @@ use glam::Vec3; use a3d::model::load_model; use a3d::render::Framebuffer; -use a3d::{framebuffer_to_string, render_frame}; +use a3d::{RenderParams, framebuffer_to_string, render_frame}; const LIGHT_DIR: Vec3 = Vec3::new(0.70710677, -0.70710677, 0.0); @@ -20,7 +20,14 @@ fn render_snapshot( ) -> String { let mesh = load_model(Path::new(model_path)).expect("model should load"); let mut fb = Framebuffer::new(width, height); - render_frame(&mut fb, &mesh, azimuth, altitude, zoom, LIGHT_DIR, None); + let params = RenderParams { + azimuth, + altitude, + zoom, + light_dir: LIGHT_DIR, + fg_override: None, + }; + render_frame(&mut fb, &mesh, ¶ms); framebuffer_to_string(&fb) } diff --git a/tests/gpu_compare.rs b/tests/gpu_compare.rs index da8b67c..4728f59 100644 --- a/tests/gpu_compare.rs +++ b/tests/gpu_compare.rs @@ -8,7 +8,7 @@ use a3d::gpu::GpuContext; use a3d::gpu::RasterPipeline; use a3d::model::load_model; use a3d::render::Framebuffer; -use a3d::{framebuffer_to_string, render_frame, render_frame_gpu}; +use a3d::{RenderParams, framebuffer_to_string, render_frame, render_frame_gpu}; const LIGHT_DIR: Vec3 = Vec3::new(0.70710677, -0.70710677, 0.0); const W: usize = 80; @@ -19,15 +19,22 @@ const H: usize = 24; /// gracefully on GPU-less CI runners instead of failing the suite. fn compare(model: &str, az: f32, al: f32, zoom: f32) -> Option<(String, String, f64)> { let mesh = load_model(Path::new(model)).expect("model should load"); + let params = RenderParams { + azimuth: az, + altitude: al, + zoom, + light_dir: LIGHT_DIR, + fg_override: None, + }; let mut cpu_fb = Framebuffer::new(W, H); - render_frame(&mut cpu_fb, &mesh, az, al, zoom, LIGHT_DIR, None); + render_frame(&mut cpu_fb, &mesh, ¶ms); let cpu_str = framebuffer_to_string(&cpu_fb); let ctx = pollster::block_on(GpuContext::new())?; let pipeline = RasterPipeline::new(&ctx, &mesh, W as u32, H as u32); let mut gpu_fb = Framebuffer::new(W, H); - render_frame_gpu(&mut gpu_fb, &pipeline, &ctx, az, al, zoom, LIGHT_DIR, None); + render_frame_gpu(&mut gpu_fb, &pipeline, &ctx, ¶ms); let gpu_str = framebuffer_to_string(&gpu_fb); let total = cpu_str.chars().count(); diff --git a/tests/gpu_resize.rs b/tests/gpu_resize.rs index a210dd1..a71893f 100644 --- a/tests/gpu_resize.rs +++ b/tests/gpu_resize.rs @@ -9,13 +9,20 @@ use glam::Vec3; use a3d::gpu::{GpuContext, RasterPipeline}; use a3d::model::load_model; use a3d::render::Framebuffer; -use a3d::{framebuffer_to_string, render_frame_gpu}; +use a3d::{RenderParams, framebuffer_to_string, render_frame_gpu}; const LIGHT_DIR: Vec3 = Vec3::new(0.70710677, -0.70710677, 0.0); fn render(pipeline: &RasterPipeline, ctx: &GpuContext, w: usize, h: usize) -> String { let mut fb = Framebuffer::new(w, h); - render_frame_gpu(&mut fb, pipeline, ctx, 0.0, 0.0, 1.0, LIGHT_DIR, None); + let params = RenderParams { + azimuth: 0.0, + altitude: 0.0, + zoom: 1.0, + light_dir: LIGHT_DIR, + fg_override: None, + }; + render_frame_gpu(&mut fb, pipeline, ctx, ¶ms); framebuffer_to_string(&fb) } diff --git a/tests/regression.rs b/tests/regression.rs index 67a1e15..e57e4d2 100644 --- a/tests/regression.rs +++ b/tests/regression.rs @@ -4,7 +4,7 @@ use glam::Vec3; use a3d::model::load_model; use a3d::render::Framebuffer; -use a3d::{framebuffer_to_string, render_frame}; +use a3d::{RenderParams, framebuffer_to_string, render_frame}; const LIGHT_DIR: Vec3 = Vec3::new(0.70710677, -0.70710677, 0.0); @@ -19,7 +19,14 @@ fn render_snapshot( ) -> String { let mesh = load_model(Path::new(model_path)).expect("model should load"); let mut fb = Framebuffer::new(width, height); - render_frame(&mut fb, &mesh, azimuth, altitude, zoom, LIGHT_DIR, None); + let params = RenderParams { + azimuth, + altitude, + zoom, + light_dir: LIGHT_DIR, + fg_override: None, + }; + render_frame(&mut fb, &mesh, ¶ms); framebuffer_to_string(&fb) }