Skip to content

[R2-5][P2] Convert updateGlobalUniforms 13-param signature to GlobalUniforms struct #846

Description

@MichaelFisher1997

Part of #839 — [Audit][Round 2] A++ umbrella.
Phase: P2 · Finding: R2-5

Problem

IRenderStateContext.updateGlobalUniforms takes 13 positional parameters. Adding a single uniform (e.g. SSAO intensity, time-of-day weight) means editing the signature across every caller, the wrapper, the vtable field, and the backend implementation. This is textbook shotgun surgery and an AGENTS.md-style readability violation.

Evidence

modules/engine-rhi/src/rhi.zig:354 — wrapper signature (one of 4 sites):

pub fn updateGlobalUniforms(self: RenderContext, view_proj: Mat4, cam_pos: Vec3,
    sun_dir: Vec3, sun_color: Vec3, time: f32, fog_color: Vec3, fog_density: f32,
    fog_enabled: bool, sun_intensity: f32, ambient: f32, use_texture: bool,
    frame_params: FrameRenderParams) !void

4 mirrored call sites for the same 13 params:

  • rhi.zig:354RenderContext.updateGlobalUniforms (wrapper)
  • rhi.zig:355 — delegates to self.state.updateGlobalUniforms(...) (same 13 args)
  • rhi.zig:596IRenderStateContext.VTable.updateGlobalUniforms (function pointer type)
  • rhi.zig:615-616IRenderStateContext.updateGlobalUniforms (interface method)

Plus every backend implementation (rhi_vulkan.zig and the corresponding vulkan/rhi_render_state.zig) and every caller:

  • engine-graphics/src/render_graph.zig (the dominant caller)
  • src/game/app.zig:397
  • modules/game-ui/src/screens/world.zig:397

Adding a uniform today = 4 signature edits in rhi.zig + 1 in backend + N in callers.

Fix

Introduce GlobalUniforms struct (mirrors the existing FrameRenderParams pattern at rhi_types.zig:298):

// modules/engine-rhi/src/rhi_types.zig
pub const GlobalUniforms = struct {
    view_proj: Mat4,
    cam_pos: Vec3,
    sun_dir: Vec3,
    sun_color: Vec3,
    time: f32,
    fog_color: Vec3,
    fog_density: f32,
    fog_enabled: bool,
    sun_intensity: f32,
    ambient: f32,
    use_texture: bool,
    // Future uniforms land here — no signature change required.
};

pub const FrameRenderParams = struct { ... }; // unchanged

New signatures:

// Wrapper
pub fn updateGlobalUniforms(self: RenderContext, uniforms: GlobalUniforms, frame_params: FrameRenderParams) !void

// VTable
updateGlobalUniforms: *const fn (ptr: *anyopaque, uniforms: GlobalUniforms, frame_params: FrameRenderParams) anyerror!void,

Update 4 sites in rhi.zig + 1 backend site + N callers. After this, adding a uniform = 1 struct field edit + N callers that opt in (others can use default .{ ... } literal).

Verification

  • nix develop --command zig build test (includes shader validation — uniform layout must match shader uniform blocks)
  • nix develop --command zig build test -- --test-filter "RenderGraph"
  • Required: headless screenshot baseline before/after (-Dskip-present, low/medium presets) — visual parity must be bit-exact (uniform byte layout must not change)
  • Grep verification: grep -rE "updateGlobalUniforms\(" modules/ src/ should show only 2-arg calls

Constraints

  • Behavior-preserving. Shader uniform block layout must not change.
  • Single PR — straightforward mechanical refactor.
  • Conventional commit: refactor(engine-rhi): bundle updateGlobalUniforms args into GlobalUniforms struct
  • While here, consider migrating the anyerror! return at rhi.zig:596 to RhiError! (small bonus).

Notes

  • Pattern is already established by FrameRenderParams at rhi_types.zig:298 — this is a consistency fix, not a new idiom.
  • Stretch: consider GlobalUniforms as extern struct if shader binding wants a stable byte layout (check current staging code first).

Tracking: #839

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingengineenhancementNew feature or requestquestionFurther information is requested

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions