[SOLID] [M3] Split rhi.zig into interfaces/wrappers; decompose VulkanContext god-struct

[MichaelFisher1997]

Part of #776 — SOLID architecture debt umbrella.
Phase: P1 · Finding: M3

Problem

modules/engine-rhi/src/rhi.zig (1,384 LOC) mixes interface vtables + 4 wrapper structs + a legacy composite + ~50 deprecated passthroughs (self-admitted at line 1012). VulkanContext (rhi_context_types.zig:192-231) is a 24+-field god-struct spanning every rendering subsystem.

Evidence

rhi.zig:1012:

/// DEPRECATED: retained as a composition root during the RHI
/// modularization refactoring (issue #291 / #272).

rhi_context_types.zig:192-231 — 24+ fields: vulkan_device, resources, frames, swapchain, descriptors, pipeline_manager, render_pass_manager, shadow_system, ssao_system, bloom_system, water_system, taa_state, depth_pyramid, dynamic_resolution, timing, …

Fix

1. Split rhi.zig into interfaces.zig (vtables) + wrappers.zig (ResourceManager/RenderContext/UIRenderer/ShadowSystemWrapper).
2. Delete the ~50 deprecated passthrough methods (lines 1120-1383) once migration is confirmed complete.
3. Group VulkanContext fields into PostProcessBackend, ShadowBackend, HdrBackend, etc., then compose.

Verification

• nix develop --command zig build test (includes shader validation)
• If rendering-touching: capture headless screenshot baseline before/after (-Dskip-present)
• If behavior-preserving refactor: golden-output test must stay green

Constraints

• Small reviewable PR targeting dev; conventional commit (refactor: / feat:)
• Preserve current behavior unless this issue explicitly changes it
• Keep worker-thread RHI isolation intact (no RHI calls off main thread)

Notes

Coordinate with C2/C3 — those issues add new vtable entries; land this split first or agree on a merge order.

Tracking: #776

[github-actions]

Validation & Analysis (M3, P1)

Classification: Refactor (SRP / ISP). Part of #776 umbrella.

Evidence verified in tree:

• modules/engine-rhi/src/rhi.zig is 1,384 LOC; VulkanContext declared at modules/engine-graphics/src/vulkan/rhi_context_types.zig:192-231 with 24+ fields spanning device, frames, swapchain, descriptors, pipelines, shadow/ssao/bloom/water/post-process/taa/dynamic-res subsystems.
• Deprecated-passthrough block confirmed at rhi.zig:1120-1383 (header at 1121 explicitly marks it as migration scaffolding). The RHI composite header at line 1012 self-documents the same intent.
• Top-level declarations in rhi.zig already segregate 14 vtables + 4 wrappers (ResourceManager, RenderContext, UIRenderer, ShadowSystemWrapper) — the modularization is mostly structural already; the file is the barrier.

Duplicate / related PR scan: No open PRs target this issue (gh pr list --search 787 returns only an unrelated merged LPV PR). Safe to proceed.

Migration scope before passthrough deletion (must be tracked in the PR):
The lines 1120-1383 block is still the live API for several call sites — these must migrate to the focused wrappers as part of the same PR (or be marked in a follow-up):

• modules/game-core/src/settings/apply.zig:40-50 — rhi.setVSync/setWireframe/setTexturesEnabled/setAnisotropicFiltering/setMSAA/setFXAA/setTAABlendFactor/setTAAVelocityRejection/setDynamicResolution (~11 calls). Note: applyToRenderSettings at line 53-63 already uses IRenderSettings — the applyToRHI path is the deprecated-RHI variant.
• modules/game-ui/src/screens/world.zig:168-893 — setWireframe/setTexturesEnabled/setVSync/setDebugShadowView/setShadowDebugChannel/updateGlobalUniforms/getResolutionScale (~25+ calls). Highest concentration.
• modules/game-ui/src/screens/world_debug.zig:55-153, modules/game-ui/src/screens/environment.zig:160, modules/game-ui/src/screens/resource_packs.zig:130 — waitIdle/setDebugShadowView/setShadowDebugChannel.
• modules/engine-graphics/src/render_system.zig:129-338 — rhi.init/deinit, setFXAA/setBloom/setBloomIntensity plus self.rhi.renderContext() accessors (these are already wrapper-based).
• modules/world-lod/src/lod_mesh.zig:171, lod_renderer.zig:159,600 — rhi.waitIdle().
• modules/world-runtime/src/world.zig:296, modules/engine-graphics/src/lpv_system.zig:652 — getRenderResolution, updateTexture.
• modules/engine-rhi/src/render_settings.zig:201-246 (RenderSettingsAdapter) still routes through self.rhi.setBloomIntensity/setVignette* etc.

The passthroughs that remain truly unused (safe to delete without migration) are the resource lifecycle ones (createBuffer/createTexture/createShader/destroyBuffer/destroyTexture/destroyShader/uploadBuffer/updateBuffer/updateTexture) and render-pipeline ones (beginFrame/endFrame/setClearColor/beginMainPass/endMainPass/beginPostProcessPass/endPostProcessPass/draw/drawOffset/drawIndexed/bindTexture/bindShader/setModelMatrix/pushConstants/updateGlobalUniforms/updateShadowUniforms/bindBuffer/begin2DPass/end2DPass/drawRect2D/drawTexture2D/beginShadowPass/endShadowPass/beginGPass/endGPass/beginFXAAPass/endFXAAPass/computeBloom/computeTAA/computeDepthPyramid/setTextureUniforms/setViewport/bindUIPipeline) — most callers already route through wrappers. So the practical "delete ~50 deprecated passthroughs" target is realistic if the ~40 settings/render-options callers above are migrated in the same PR.

Architectural impact (concrete):

1. interfaces.zig — move 14 I* vtables and their helpers (IResourceFactory, IRenderContext, IPassOrchestrationContext, IPostProcessContext, INativeHandlesContext, ISSAOContext, IDebugOverlayContext, IShadowContext, IWaterContext, IUIContext, IGraphicsCommandEncoder, IRenderStateContext, IDeviceQuery, IDeviceTiming, IRenderOptionsContext). Approx 700-900 LOC.
2. wrappers.zig — move 4 concrete wrapper structs (ResourceManager, RenderContext, UIRenderer, ShadowSystemWrapper) plus WaterSystemWrapper. Approx 200-300 LOC.
3. rhi.zig keeps: re-exports (BufferHandle, RhiError, etc. via rhi_types) and the RHI composition root. Shrinks to ~250-350 LOC.
4. VulkanContext decomposition: group fields by responsibility. Suggested groupings (matches the existing field structure already):
   • DeviceBackend — vulkan_device, render_device, window, allocator, mutex
   • ResourceBackend — resources, frames, descriptors, pipeline_manager, render_pass_manager, legacy
   • SwapchainBackend — swapchain
   • HdrBackend — hdr, velocity, gpass
   • ShadowBackend — shadow_system, shadow_runtime, debug_shadow
   • PostProcessBackend — post_process, fxaa, taa, bloom, post_process_state, depth_pyramid
   • WaterBackend — water_system
   • UIBackend — ui
   • StateBackend — draw, options, runtime, timing, dynamic_resolution
   • VulkanContext then becomes a thin composition root (~9 fields). All existing ctx.shadow_system, ctx.swapchain, etc. accessors must be rewritten — expect a heavy diff in src/engine/graphics/vulkan/*.zig (pass_orchestration, draw_submission, ui_submission, frame_orchestration, resource_setup, state_control, shadow_bridge).

Scope assessment: This issue as written is actually two distinct refactors that should ship as separate PRs to stay reviewable:

1. PR-A (file split only): Move vtables to interfaces.zig and wrappers to wrappers.zig. No behavior change. Pure file-shuffle + import-path updates. (~150-300 LOC churn).
2. PR-B (passthrough deletion): Migrate the ~40 callers listed above to wrappers, then delete rhi.zig:1120-1383. (~10-15 files touched).
3. PR-C (VulkanContext decomposition): Group fields into 8-9 backend structs; rewrite all internal accessors in src/engine/graphics/vulkan/*.zig and modules/engine-graphics/src/vulkan/*.zig. Largest of the three, ~500+ LOC of churn across 25+ files.

The original issue bundles all three under "small reviewable PR" — recommend splitting into PR-A/B/C above and updating this issue (or creating #787a/#787b/#787c sub-issues).

Merge-order risk vs C2/C3:

• C2 ([SOLID] [C2] Eliminate INativeHandlesContext; move sky/water/shadow draws behind the RHI #784) modifies INativeHandlesContext callers in atmosphere_system.zig:39-42 and render_graph.zig:645-655. If PR-C lands first, those changes propagate against a decomposed VulkanContext — slightly safer.
• C3 ([SOLID] [C3] Add compute facet to RHI.VTable; rewrite gpu_mesher + lpv_backend against it #783) adds 5 new vtable entries to RHI.VTable (createComputePipeline/dispatch/pipelineBarrier/pushConstants/fillBuffer). PR-A's file split is independent of these. PR-B/C are independent too, as long as both PR-A and C3 don't race on rhi.zig layout.
• Recommendation: land PR-A first (no behavior change, no vtable changes), then either C3 (compute facet) or PR-C (decomposition) — but not both simultaneously against the same files.

Strengths to preserve (per #776 P2): worker-thread RHI isolation, the existing wrapper-vs-vtable split, the @hasDecl(Provider, "resourceManager") fallback pattern in lod_mesh.zig:60-90 (this proves backends can already be partial). Don't regress those.

Verification checklist for the implementer:

• nix develop --command zig build test (shader validation included)
• nix develop --command zig build -Doptimize=ReleaseFast
• Headless screenshot baseline before/after PR-C with -Dskip-present (the vulkan module touches every rendered frame)
• Golden-output test stays green (no behavior changes)
• Confirm modules/world-lod/src/lod_mesh.zig:60-90 @hasDecl adapter still resolves — any change to RHI shape could regress the LOD bridge

Not requesting more info — the issue is self-contained. Recommend the implementer split this into 3 sub-PRs as outlined and add a tracking checklist before starting.

[github-actions]

Comment posted on issue #787.

Summary of analysis:

• Classification: Refactor (SRP/ISP) — part of [Audit][Umbrella] SOLID architecture debt: RHI seam leakage, god-objects, and extension-safety gaps #776 umbrella, finding M3.
• Evidence verified: rhi.zig = 1,384 LOC; VulkanContext 24+ fields at rhi_context_types.zig:192-231; deprecated passthroughs confirmed at lines 1120-1383.
• No duplicate PRs open against [SOLID] [M3] Split rhi.zig into interfaces/wrappers; decompose VulkanContext god-struct #787.
• Key finding: Issue claims ~50 deprecated passthroughs can be deleted "once migration is confirmed complete," but ~40 call sites in modules/game-core/src/settings/apply.zig, modules/game-ui/src/screens/world.zig, modules/engine-graphics/src/render_system.zig, etc. still depend on them — those need to migrate to wrappers in the same PR.
• Scope: Recommended splitting the bundled work into 3 sub-PRs (file split only, passthrough deletion, VulkanContext decomposition) for reviewability.
• Merge order vs [SOLID] [C3] Add compute facet to RHI.VTable; rewrite gpu_mesher + lpv_backend against it #783/[SOLID] [C2] Eliminate INativeHandlesContext; move sky/water/shadow draws behind the RHI #784: PR-A (file split) is independent; PR-B/C should not race with C3's compute-facet additions to RHI.VTable.

opencode session  |  github run