[R2-2][P1] Encapsulate LODMesh/LODChunk behind behavior methods

[MichaelFisher1997]

Part of #839 — [Audit][Round 2] A++ umbrella.
Phase: P1 · Finding: R2-2 · Depends on: nothing (unblocks R2-1)

Problem

LODMesh and LODChunk are treated as open structs by both lod_manager and lod_renderer. A field change to either propagates to two consumers — classic Law-of-Demeter / shotgun-surgery smell. This blocks clean extraction boundaries in R2-1.

Evidence

Both consumers reach into LODMesh bare fields:

• lod_renderer.zig:246, 248, 355, 425, 429, 592, 677-686 — reads buffer_handle, vertex_count, opaque_vertex_count, water_vertex_offset, water_vertex_count, vertex_offset, pooled, ready
• lod_manager.zig:1303, 1588-1589, 2515-2517, 2525, 2548-2553 — reads/writes ready, vertex_count, capacity, pending_vertices, opaque_vertex_count

Both consumers reach into LODChunk bare fields:

• lod_renderer.zig:360, 503-507, 666-670 — reads state, ready_children, transition_frames_remaining, lod_level
• lod_manager.zig — reads/writes state extensively across the worker pipeline

Manager hands raw &meshes and &regions HashMap pointers to the renderer — lod_manager.zig:1685. The renderer iterates these maps directly (lod_renderer.zig:347), exposing internal storage layout.

SchedulerContext (lod_scheduler.SchedulerContext) is a 14-field struct carrying raw pointers into the manager's regions, gen_queues, mutex, next_job_token, plus a callback coverage_ptr. The decoupling is structurally present but semantically hollow — "manager exposes its entire interior for one call."

Fix

Add behavior methods to LODMesh and LODChunk; collapse open-field access. Target methods (initial sketch — adjust during implementation):

LODMesh

pub fn drawRange(self: *LODMesh, layer: RenderLayer) ?MeshDrawRange
pub fn byteSize(self: *const LODMesh) usize
pub fn markUploaded(self: *LODMesh, vertex_count: u32, opaque_count: u32) void
pub fn isReady(self: *const LODMesh) bool
pub fn acquireWriteLock(self: *LODMesh) void  // wraps existing sync.Mutex

LODChunk

pub fn isRenderable(self: *const LODChunk) bool
pub fn transitionFadeProgress(self: *const LODChunk) f32
pub fn isCoveredByFinerLOD(self: *const LODChunk, config: CoverageConfig) bool
pub fn beginTransition(self: *LODChunk, target_level: LODLevel) void
pub fn lodLevel(self: *const LODChunk) LODLevel

LODMeshStorage iterator

Replace &meshes HashMap pointer exposure with: pub fn iterateRenderable(self: *LODMeshStorage, layer: RenderLayer) RenderableIter. The renderer asks for an iterator of (mesh_ref, draw_range) tuples instead of indexing the map.

Slim down SchedulerContext

Convert the 14-field struct to 3 fields: regions_view: RegionsView, queue_handle: QueueHandle, coverage: *const CoverageQuery. The view types are read-only facades.

Verification

• nix develop --command zig build test (includes shader validation)
• Headless screenshot baseline before/after (-Dskip-present, low/medium presets) — must be bit-exact
• nix develop --command zig build test -- --test-filter "LODMesh" and --test-filter "LODChunk"
• grep -rE "\.(vertex_count|buffer_handle|opaque_vertex_count|ready_children|transition_frames_remaining)" modules/world-lod/ should return only the new method bodies, no callers

Constraints

• Behavior-preserving. Render output unchanged.
• Small reviewable PR targeting dev; conventional commit (refactor(world-lod): encapsulate LODMesh/LODChunk).
• Per-method PRs are acceptable (one PR per struct is also fine if small).
• Keep worker-thread safety: encapsulation must not introduce new shared-mutable-state races. Existing per-mesh sync.Mutex moves into the methods.

Notes

• This is the prerequisite for clean R2-1 extraction boundaries. Land first.
• The duplicated coverage logic (areAllChunksLoaded vs isCoveredByChunks) is addressed separately in R2-1's bonus section.

Tracking: #839

[github-actions]

Classification

Refactor / Law-of-Demeter violation (labeled bug + enhancement).

Audit Validation ✅

I verified all evidence against the working tree. Findings are accurate:

LODMesh (modules/world-lod/src/lod_mesh.zig:89-115) — fields buffer_handle, vertex_count, opaque_vertex_count, water_vertex_offset, water_vertex_count, capacity, vertex_offset, pooled, pending_vertices, mutex, ready are all pub by virtue of struct visibility (default in this module).

LODChunk (modules/world-lod/src/lod_chunk.zig:129-175) — fields state, lod_level, ready_children, transition_frames_remaining are openly read in:

• lod_renderer.zig:503-507 — isCoveredByFinerLOD directly reads chunk.lod_level, chunk.ready_children, chunk.transition_frames_remaining
• lod_renderer.zig:670 — chunk.lod_level != .lod1 and chunk.ready_children >= 4
• lod_renderer.zig:360 — chunk.state != .renderable
• lod_renderer.zig:677-686 — mesh_draw_range reaches into mesh.ready, mesh.buffer_handle, mesh.opaque_vertex_count, mesh.water_vertex_count, mesh.vertex_count, mesh.water_vertex_offset

HashMap pointer exposure confirmed:

• lod_manager.zig:1685 — self.renderer.render(&self.meshes, &self.regions, ...) ✅
• lod_renderer.zig:182-183 — accepts *const [LODLevel.count]MeshMap, *const [LODLevel.count]RegionMap ✅
• lod_renderer.zig:347 — meshes.iterator() then regions.get(entry.key_ptr.*) ✅

SchedulerContext minor discrepancy: lod_scheduler.zig:20-41 actually has 16 fields (allocator, config, radii, active_lod_count, regions, gen_queues, mutex, player_cx, player_cz, next_job_token, cleanup_covered_regions, coverage_ptr, are_all_chunks_loaded, radius_reduction, defer_generation_dispatch, use_vertical_spans) — close to the reported 14, but worth using the actual count when sizing the facade.

Scope Assessment

This issue bundles four distinct refactors with separable blast radius:

1. LODMesh encapsulation (render-time reads)
2. LODChunk encapsulation (transition/coverage state)
3. LODMeshStorage iterator facade (breaks meshes/regions map exposure)
4. SchedulerContext slim-down (manager↔scheduler decoupling)

The issue text already permits per-struct PRs. Recommend splitting into 3 PRs against dev:

PR	Scope	Risk
PR-A	LODChunk methods only (isRenderable, transitionFadeProgress, isCoveredByFinerLOD, beginTransition, lodLevel)	Low — pure additive, state/ready_children writes remain internal during refactor
PR-B	LODMesh methods + mesh_draw_range move-into (drawRange, byteSize, markUploaded, isReady, acquireWriteLock)	Medium — mesh_draw_range is shared between diagnostic and main path
PR-C	LODMeshStorage::iterateRenderable + RegionsView/QueueHandle facade + SchedulerContext slim-down	Medium-High — touches lod_manager.zig:1685 call site; needs before/after headless screenshot for bit-exact verification

PR-C is the dependency edge into R2-1's decomposition; land it last to keep diff reviewable.

Implementation Notes

• Mutex handling: LODMesh.mutex is currently sync.Mutex = .{} at lod_mesh.zig:111 and locked directly in deinit/clearPendingVertices/pendingUploadBytes. acquireWriteLock should be paired with a releaseWriteLock (or return a guard struct) — single-method lock() without unlock is an API leak.
• Iterator pattern: RenderableIter should be a struct with next() ?struct { mesh: *LODMesh, draw_range: MeshDrawRange, key: LODRegionKey } — keep the key so the renderer can still resolve coverage against the chunk without re-iterating regions.
• Scheduler facade: RegionsView and QueueHandle should own no state beyond read-only references; the existing mutex: *sync.RwLock and next_job_token: *u32 are write-side and need a separate SchedulerWriter handle or stay on the context. Don't collapse them into a "view" — that loses the read/write split.
• SchedulerContext::CoverageFn callback: coverage_ptr: *anyopaque + are_all_chunks_loaded: CoverageFn is the cleanest signal-passing idiom in Zig — collapsing them into a *const CoverageQuery vtable is fine but not strictly necessary.

Verification (per issue, plus my additions)

• nix develop --command zig build test — must pass including shader validation
• Headless screenshot at low and medium presets, -Dskip-present — must be bit-exact pre/post
• nix develop --command zig build test -- --test-filter "LODMesh" and "LODChunk"
• grep -rE "\.(vertex_count|buffer_handle|opaque_vertex_count|ready_children|transition_frames_remaining)" modules/world-lod/ — only method bodies
• Additional: nix develop --command zig build -Doptimize=ReleaseFast && zig build test — ReleaseFast path exercises inlining and may surface any new perf regressions from the method indirection
• Thread safety: existing lod_mesh_tests.zig + add concurrency test that hammers acquireWriteLock while markUploaded is in flight

Ready to Implement

No blockers. The audit is well-grounded and the proposed method signatures are reasonable starting points. Splitting into PR-A/B/C (sequenced) will make review tractable and keep each diff <500 LOC.

[github-actions]

Comment posted to issue #841: #841 (comment)

opencode session  |  github run