Refactor lod_manager.zig - Separate LOD Logic from GPU Operations

[MichaelFisher1997]

Description

src/world/lod_manager.zig (1,198 lines) mixes high-level LOD region management with low-level GPU operations through a generic RHI parameter. This violates Single Responsibility Principle and creates tight coupling.

Current Problems

• SRP Violation: Manages both logical LOD state AND rendering/GPU concerns
• ISP Violation: LODManager(RHI) uses only subset of RHI but has full interface access
• DIP Violation: Direct dependency on concrete RHI type through generics

Technical Details

Current generic pattern:

pub fn LODManager(comptime RHI: type) type {
    return struct {
        rhi: RHI,  // Direct RHI dependency
        
        // LOD logic (should remain)
        regions: [LODLevel.count]std.HashMap(...),
        gen_queues: [LODLevel.count]std.ArrayList(...),
        
        // GPU operations (should be extracted)
        fn processUploads() { mesh.upload(self.rhi); }
        fn unloadDistantRegions() { mesh.deinit(self.rhi); }
        fn render() { LODRenderer.render(self.rhi); }
    };
}

Mixed responsibilities:

• LOD Logic: Region tracking, generation queuing, state transitions, distance-based selection
• GPU Operations: Buffer uploads (processUploads), deferred deletion (unloadDistantRegions), rendering coordination

Proposed Solution

1. Extract LODRenderer - Already partially exists, ensure complete separation
2. Create LODUploadQueue - Handles all GPU upload/deletion operations
3. Define callback interface - LODManager uses callbacks instead of direct RHI access

New architecture:

// LODManager - pure logic, no RHI
pub const LODManager = struct {
    regions: [...],
    upload_queue: LODUploadQueue,  // Callback-based
    renderer: LODRenderer,         // Already separate
    
    pub fn update(self: *Self, player_pos: Vec3) void {
        // Pure LOD logic
        self.queueUploads();
    }
};

// LODUploadQueue - handles GPU operations
pub const LODUploadQueue = struct {
    on_upload: *const fn(mesh: LODMesh) void,
    on_delete: *const fn(mesh: LODMesh) void,
};

Pass Criteria

• LODManager has no rhi field (removed from struct)
• LODManager uses callback interface for GPU operations
• LODRenderer fully separated with clean interface
• LODUploadQueue extracted as standalone module
• All existing tests pass: nix develop --command zig build test
• No performance regression (upload queuing still efficient)

[opencode-agent]

This is a well-documented refactoring request that identifies genuine architectural issues in src/world/lod_manager.zig. The Single Responsibility Principle violations and tight coupling to RHI through generics are valid concerns that would benefit from this refactoring.

Classification: Feature Request / Refactoring

Analysis:

The proposed solution follows the existing architectural patterns in this codebase:

• The RHI abstraction pattern already used in src/engine/graphics/rhi.zig
• Callback-based interfaces for decoupling (similar to job system patterns)
• Separation of logical state from rendering concerns

Considerations for implementation:

1. Callback design: The callback interface (on_upload, on_delete) should handle ownership semantics clearly - who owns the LODMesh during the callback, and how errors are propagated if GPU operations fail.

2. Thread safety: The existing lod_manager.zig likely uses mutexes for thread-safe region access. This pattern should be preserved or explicitly documented in the new architecture.

3. Error handling: GPU operations can fail (out of memory, device lost). The callback interface should support error propagation, not just fire-and-forget semantics.

4. Performance: The queuing mechanism should maintain the same batching behavior for uploads to avoid individual buffer operations.

Suggested approach:

• Implement LODUploadQueue first with the callback interface
• Refactor LODManager to hold the queue instead of RHI
• Extract/refine LODRenderer to use the queue
• Update call sites in src/world/world.zig where LODManager is instantiated

The pass criteria are clear and testable. This is a focused refactoring that can be implemented in one PR without breaking into sub-issues.

opencode session  |  github run