Skip to content

refactor: bundle render args into RenderParams - #30

Draft
marksverdhei wants to merge 1 commit into
mainfrom
refactor/render-params
Draft

refactor: bundle render args into RenderParams#30
marksverdhei wants to merge 1 commit into
mainfrom
refactor/render-params

Conversation

@marksverdhei

Copy link
Copy Markdown
Contributor

Draft — for your decision on the RenderParams judgment call (#3). Opened as a draft so you can see the concrete diff before committing to the API shape; not meant to merge without your go-ahead.

Groups the five shared pose/shading arguments (azimuth, altitude, zoom, light_dir, fg_override) into a RenderParams struct passed by reference:

pub fn render_frame(fb: &mut Framebuffer, mesh: &Mesh, params: &RenderParams)
pub fn render_frame_gpu(fb: &mut Framebuffer, pipeline: &RasterPipeline, ctx: &GpuContext, params: &RenderParams)

Pros

  • render_frame 7 args → 3; render_frame_gpu 8 → 4. Drops render_frame_gpu's #[allow(clippy::too_many_arguments)] (the per-triangle rasterize_triangle allow stays — its args don't bundle).
  • A named struct reads better at call sites than five positional floats.
  • Pre-1.0 / pre-public is the cheapest time to change this signature.

Cons / why it's a judgment call

  • API churn across ~6 call sites (done here).
  • Only removes 1 of the 2 lint suppressions.
  • My standing recommendation in Hivemind Maintenance Tasks Epoch 1 #3 was to defer this to M3 (when materials/lighting grow the param set) — this draft exists so you can weigh that against the concrete diff.

Verification: behavior-preserving — GPU/CPU output stays byte-identical (gpu_compare 0.00%), full suite green (41 tests), fmt + clippy -D warnings clean.

Merge if you like the shape; otherwise close and I'll keep the deferral note in #3.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant