fix: stabilize distant LOD and Vulkan screen transitions

[MichaelFisher1997]

Summary

• keep the complete radial distant-LOD hierarchy active through the qualified 512-chunk user horizon, with bounded scheduling, memory-pressure fallback, and edited-source persistence
• repair compact topology upload readiness, direct/indirect draw selection, shader sentinels, frustum culling, and distant terrain/water blending
• harden graphics and transfer command abort paths, defer RmlUi screen/world construction to GPU-idle transition boundaries, and avoid retaining two worlds during replacement
• add LOD, screen-factory, quit-to-title, transfer-abort, persistence, and robustness regression coverage

Validation

• nix develop --command zig fmt --check src/ modules/
• nix develop --command zig build test
• nix develop --command zig build -Doptimize=ReleaseFast
• nix develop --command zig build test-integration -Dskip-present=true
• nix develop --command zig build test-robustness

Manual verification

• confirmed the RmlUi world → Pause → Quit to Title flow no longer reports VK_ERROR_DEVICE_LOST on RADV

[github-actions]

kcov coverage

Line coverage ran for this PR and uploaded a non-blocking report artifact named kcov-report. Codecov upload is configured as non-blocking while the project captures a stable baseline.

[github-actions]

📋 Summary

No linked issues found in the PR description.

This PR stabilizes distant LOD streaming and Vulkan screen transitions through four major areas of work: (1) keeping the complete radial LOD hierarchy active with bounded scheduling, memory-pressure fallback, and edited-source persistence; (2) repairing compact topology upload readiness, direct/indirect draw selection, shader sentinels, and frustum culling; (3) hardening graphics/transfer command abort paths and deferring screen/world construction to GPU-idle boundaries; (4) adding comprehensive regression coverage. The implementation is high-quality with thorough integer overflow protection (i64→i128 widening), correct lock ordering, and consistent shader/RHI sentinel changes.

📌 Review Metadata

• Reviewed Commit SHA: 1c270154ffa7a071aabeffa1de747f19cb192ffb
• Reviewed PR: fix: stabilize distant LOD and Vulkan screen transitions #948

🔴 Critical Issues (Must Fix - Blocks Merge)

✅ All previously reported critical issues have been resolved.

None identified.

⚠️ High Priority Issues (Should Fix)

✅ All previously reported high-priority issues have been resolved.

None identified.

💡 Medium Priority Issues (Nice to Fix)

None identified.

ℹ️ Low Priority Suggestions (Optional)

[LOW] modules/world-lod/src/lod_renderer.zig:1344 - Magic number in mask radius conversion
Confidence: Low
Description: conservativeChunkDiskMaskRadius converts positive mask radii to negative values (-mask_radius) to encode the "ready disk" semantics. The interplay between LOD_UNMASKED_SENTINEL (0.5), positive legacy masks, and negative ready-disk masks is subtle. The shader-side shouldDiscardLODFragment must exactly mirror this encoding. While the test coverage validates the current behavior, the encoding scheme would benefit from a brief documented invariant comment near the constant definitions.
Impact: Low — future shader/RHI changes could silently break the mask handshake.
Suggested Fix: Add a 2-3 line comment block near LOD_UNMASKED_SENTINEL documenting: "positive = legacy overlap mask, negative = contiguous ready-detail disk radius in blocks, 0.5 = unmasked sentinel."

[LOW] modules/game-ui/src/screen.zig:1217 - replace_factory clears stack before construction, leaving empty stack on construction failure
Confidence: Low
Description: In applyPendingTransitions, the replace_factory path calls self.clearStack() before factory.construct(). If construction fails (e.g., OOM), the stack is left empty with no recovery path. This is explicitly justified in the comment ("release the old stack first instead of temporarily retaining two worlds"), and matches the prior replace behavior's failure semantics, but the failure surface is now wider since construction (not just append) can fail.
Impact: Low — only affects OOM-during-quit-to-title, which is an extreme edge case. The deliberate tradeoff avoids doubling memory.
Suggested Fix: Acceptable as-is. If desired, could construct first, then destroy old stack — but this contradicts the stated memory goal.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	9	Clear separation: ScreenFactory handles construction deferral, LODScanState handles cursor state, ChunkQueueCoordinator handles scan logic
Open/Closed	8	Screen factory pattern is extensible via comptime generics without modifying ScreenManager; LODConfig helpers are pure functions
Liskov Substitution	9	IScreen vtable additions (drawBackground) are optional with null defaults; new ChunkChecker function is a drop-in replacement
Interface Segregation	8	LODRenderInterface gained one parameter (detail_render_radius) threaded cleanly through; no fat interfaces
Dependency Inversion	8	Scheduler depends on abstract LODScanState; ingestion provenance resolved through well-typed enum rather than flags
Average	8.4

🎯 Final Assessment

Overall Confidence Score: 90%

Confidence Breakdown:

• Code Quality: 92% (clean, well-commented, consistent conventions, comprehensive overflow protection)
• Completeness: 90% (all four stated goals implemented with extensive regression tests)
• Risk Level: 88% (large surface area touching GPU lifecycle, but careful lock ordering and abort paths; manual RADV verification noted)
• Test Coverage: 90% (new tests for scheduler ring cursor, ingestion races, persistence invalidation, screen factory lifecycle, frustum planes, compact index upload gating, quit-event handling)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• Tests present and passing (per PR validation steps)

Verdict:

MERGE — Well-executed stabilization PR with thorough testing, correct GPU resource lifecycle management, comprehensive integer overflow protection, and no identified blocking issues.

{
  "reviewed_sha": "1c270154ffa7a071aabeffa1de747f19cb192ffb",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 90,
  "recommendation": "MERGE"
}

opencode session  |  github run