[SOLID] [C3] Add compute facet to RHI.VTable; rewrite gpu_mesher + lpv_backend against it

[MichaelFisher1997]

Part of #776 — SOLID architecture debt umbrella.
Phase: P0 · Finding: C3

Problem

world-runtime (highest tier) imports the concrete Vulkan backend and performs an unsafe downcast of the opaque RHI pointer, then issues 27 raw c.vk* calls (vkCmdDispatch, vkCmdPipelineBarrier, vkWaitForFences, …). The RHI abstraction is bypassed entirely for GPU meshing. Same pattern in lpv_backend.zig ( candidly documented as intentional).

Evidence

modules/world-runtime/src/gpu_mesher.zig:9-11,89:

const graphics = @import("engine-graphics");
const VulkanContext = graphics.VulkanContext;
...
const vk_ctx: *VulkanContext = @ptrCast(@alignCast(rhi.ptr));
// then: c.vkCmdDispatch(cmd, CHUNK_Y, 1, 1); etc.

modules/engine-graphics/src/lpv_backend.zig:6-15 carries the same @ptrCast(@alignCast(rhi.ptr)) downcast.

Fix

1. Add a compute facet to RHI.VTable: createComputePipeline, bindComputePipeline, dispatch, pushConstants, fillBuffer, pipelineBarrier.
2. Implement the facet in the Vulkan backend (port the existing raw calls).
3. Rewrite gpu_mesher.zig and lpv_backend.zig to consume the abstraction.
4. Remove both @ptrCast(@alignCast(rhi.ptr)) downcasts and the engine-graphics import from world-runtime.

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

Must land before C2 — C2's drawSky/drawWater will share this compute/dispatch infrastructure. Capture a headless screenshot baseline before and after (per #776 Key risks). Keep worker-thread RHI isolation intact: compute dispatch stays on the main thread.

Tracking: #776

[github-actions]

I've validated the issue against the codebase and confirmed the findings. Here's my comment:

---

Classification: Refactor / SOLID architecture debt (linked to #776 umbrella). Scope is well-contained — appropriate for a single PR.

Validation:

Confirmed the evidence is accurate:

• modules/world-runtime/src/gpu_mesher.zig:9-10,89 does @import("engine-graphics") + @ptrCast(@alignCast(rhi.ptr)) to reach VulkanContext, then issues raw c.vkCmdDispatch / c.vkCmdPipelineBarrier / c.vkCmdBindDescriptorSets etc. The world-runtime module currently depends upward on engine-graphics, which inverts the tiering.
• modules/engine-graphics/src/lpv_backend.zig does the same downcast — agreed this is "candidly documented" but should still be reworked.

No duplicate PR exists (checked git log --all, only #781 landed recently). Branch is clean.

Scope check: ✅ Single PR is appropriate. Touches four files: engine-rhi/src/rhi.zig, engine-graphics/src/rhi_vulkan.zig, world-runtime/src/gpu_mesher.zig, engine-graphics/src/lpv_backend.zig. No shader or data-layout changes needed — MeshPushConstants / MeshBuildResult extern structs and mesh.comp.spv are unaffected.

Implementation gotchas worth surfacing in the PR description:

1. pushConstants already exists on IGraphicsCommandEncoder (rhi.zig:561). vkCmdPushConstants is valid in compute command buffers, so the compute facet should delegate to the encoder's pushConstants rather than re-add a duplicate VTable entry — otherwise two paths diverge over time.
2. createComputePipeline needs more than the proposed signature to actually replace gpu_mesher lines 89+: Vulkan also requires a pipeline layout and a descriptor set layout. Suggest splitting into createComputePipelineLayout(descriptor_set_layout) + createComputePipeline(comp_spv, layout), plus a bindDescriptorSet(set, pipeline_bind_point) method. Without these, the consumer still needs raw vkCmdBindDescriptorSets.
3. createShader is currently graphics-only (vertex_src + fragment_src). For compute you'll need either a new createComputeShader(comp_src) or extend the existing one with a stage enum. The latter is cleaner and avoids a second ShaderHandle type.
4. pipelineBarrier is the trickiest to abstract. Vulkan's VkMemoryBarrier/VkBufferMemoryBarrier/VkImageMemoryBarrier split, dependency flags, and access masks are very API-specific. Recommend a small enum-driven struct ({ stages_src, stages_dst, access_src, access_dst }) rather than leaking Vk* types.
5. Threading: the note about keeping compute dispatch main-thread-only is correct and already satisfied — gpu_mesher.dispatch() is called from the main render loop. Just make sure the new facet doesn't accidentally enable worker threads.

Suggested sequencing (per the "must land before C2" constraint):

1. Add IComputeContext + ComputeSystemWrapper to rhi.zig, register in RHI.VTable (around line 1045), add rhi.computeSystem() accessor mirroring rhi.shadowSystem() / rhi.uiRenderer().
2. Implement in rhi_vulkan.zig by porting the existing raw calls (use a typed ComputeContext struct holding pipeline, pipeline_layout, descriptor_sets like gpu_mesher already does internally).
3. Rewrite gpu_mesher.zig first (smaller blast radius — drop the engine-graphics import), then lpv_backend.zig second.
4. Verification: nix develop --command zig build test, plus use the headless-graphics-verification skill to capture a baseline screenshot before and after (per [Audit][Umbrella] SOLID architecture debt: RHI seam leakage, god-objects, and extension-safety gaps #776 Key risks). Behavior-preserving → golden output must stay green.

Acceptance criteria worth adding:

• No @ptrCast(@alignCast(rhi.ptr)) in world-runtime or engine-graphics/src/lpv_backend.zig.
• world-runtime/build.zig no longer depends on engine-graphics.
• Headless screenshot diff before/after = 0.
• zig build test green; zig fmt src/ clean.

One open question for the author: should IComputeContext be one combined facet or split into IComputeDispatch (dispatch/barrier/fillBuffer) and IComputePipelineFactory (create/bind pipeline)? Splitting matches the existing segregation pattern (IResourceFactory vs IGraphicsCommandEncoder) and would let, e.g., a hypothetical WebGPU backend implement pipeline creation without exposing compute command encoding. Worth deciding before the PR to avoid a v2 churn.

opencode session  |  github run