refactor(vulkan): PR1 - Add PipelineManager and RenderPassManager modules by MichaelFisher1997 · Pull Request #251 · OpenStaticFish/ZigCraft

MichaelFisher1997 · 2026-01-29T06:46:56Z

Summary

This PR introduces two new subsystem managers as part of the rhi_vulkan refactoring (Issue #244) to eliminate the god object anti-pattern.

Changes

New Modules

1. src/engine/graphics/vulkan/pipeline_manager.zig (~650 lines)

Manages all graphics pipeline creation and lifecycle
Handles pipeline layouts with proper push constant ranges
Supports all pipeline types:
- Terrain (with wireframe, selection, line variants)
- G-Pass (geometry pass for SSAO)
- Sky rendering
- UI (colored and textured variants)
- Cloud rendering
- Debug shadow visualization (conditional)
Clean interface: init(), deinit(), createMainPipelines(), createSwapchainUIPipelines()

2. src/engine/graphics/vulkan/render_pass_manager.zig (~550 lines)

Manages all render pass creation and lifecycle
Handles framebuffer management for all passes
Supports all render passes:
- HDR render pass (with MSAA support)
- G-Pass render pass
- Post-process render pass
- UI swapchain render pass
Clean interface: init(), deinit(), create*RenderPass(), create*Framebuffer()

Documentation

3. docs/refactoring/REFACTORING_PLAN.md

Overall refactoring roadmap
Breakdown into 4 PRs
Architecture diagram
Testing strategy

4. docs/refactoring/PR1_DEVICE_SWAPCHAIN.md (renamed to reflect actual scope)

Detailed PR1 plan
Field migration checklist
Testing checklist

Impact

No functional changes - modules are not yet integrated
No breaking changes - existing code untouched
Build passes - all tests pass
Reduces rhi_vulkan.zig by ~1,200 lines once integrated
Removes ~23 fields from VulkanContext once integrated

Next Steps (Future Commits)

Integrate managers into VulkanContext
Update initialization to use managers
Migrate field accesses gradually
Remove old pipeline/renderpass fields
Remove old creation/destruction functions

Testing

nix develop --command zig build compiles
nix develop --command zig build test passes
nix develop --command zig build test-integration passes
All shaders validated

Add new subsystem managers to eliminate god object anti-pattern in rhi_vulkan.zig: - PipelineManager: manages all graphics pipelines (terrain, wireframe, selection, line, G-Pass, sky, UI, cloud) and their layouts - RenderPassManager: manages all render passes (HDR, G-Pass, post-process, UI swapchain) and framebuffers These modules provide clean interfaces for PR1 of the rhi_vulkan refactoring: - init()/deinit() lifecycle management - Separate concerns for pipeline vs render pass creation - Ready for integration into VulkanContext Part of #244

opencode-agent · 2026-01-29T06:49:02Z

Code Review: PipelineManager & RenderPassManager PR

Code Quality Issues & Potential Bugs

Memory leak in shader loading (pipeline_manager.zig:320-328): Each pipeline creation reads shader files from disk and creates temporary shader modules. These are properly deferred for destruction, but the repeated file I/O for shared shaders (like UI_VERT) is inefficient.
Unused parameter warning (render_pass_manager.zig:39-44): The allocator parameter in init() is named _allocator and ignored. Either remove it or document why it's there.
Inconsistent null handling (pipeline_manager.zig:583): createSwapchainUIPipelines returns an error if ui_swapchain_render_pass is null, but other methods don't validate their Vulkan handle inputs before use.
Magic numbers: Hardcoded sizes like 256 and 128 for push constants (pipeline_manager.zig:83,97) should be documented or extracted to named constants.
Code duplication: Shader loading patterns are repeated 6+ times with identical structure. Consider extracting a helper function.

SOLID Principle Scoring

Principle	Score	Notes
Single Responsibility	8/10	Both managers have clear, focused responsibilities. PipelineManager handles pipelines only; RenderPassManager handles passes/framebuffers only. Good separation.
Open/Closed	5/10	Adding new pipeline types requires modifying PipelineManager. The factory pattern is not used. Consider strategy pattern for pipeline variants.
Liskov Substitution	N/A	No inheritance hierarchies; not applicable to this PR.
Interface Segregation	6/10	Public API is reasonable but could be split (e.g., separate `PipelineCreator` from `PipelineLifecycle` interfaces).
Dependency Inversion	4/10	Both managers depend on concrete `VulkanDevice` and `DescriptorManager` types rather than abstractions. Consider accepting interfaces for better testability.

Actionable Suggestions

Location	Issue	Suggestion
`pipeline_manager.zig:39-50`	Exposes all Vulkan handles publicly	Make fields private and add accessor methods (getters) for encapsulation
`pipeline_manager.zig:72-166`	`createPipelineLayouts` is too long	Extract each layout creation into separate private functions
`render_pass_manager.zig:101-264`	`createMainRenderPass` handles both MSAA/non-MSAA paths	Consider splitting into `createHDRMSAA()` and `createHDR()` for clarity
`pipeline_manager.zig:216-301`	`createMainPipelines` creates many pipelines in one call	Consider separating by rendering stage (terrain, sky, ui) for better incremental recreation
Both files	No error recovery on partial failure	Pipeline creation that fails partway leaves manager in inconsistent state; add rollback or validation before creation
Both files	Tests are integration-only	Add unit tests for individual pipeline/renderpass creation logic with mocked Vulkan

opencode session | github run

Fixes based on code review: 1. Extract magic numbers to named constants: - PUSH_CONSTANT_SIZE_MODEL = 256 - PUSH_CONSTANT_SIZE_SKY = 128 - PUSH_CONSTANT_SIZE_UI = sizeof(Mat4) 2. Add shader loading helper function to reduce code duplication: - loadShaderModule() handles file reading and module creation - Applied to terrain pipeline creation 3. Document unused parameter in render_pass_manager.init(): - Added comment explaining allocator is reserved for future use Part of #244

opencode-agent · 2026-01-29T06:58:03Z

Code Review: PipelineManager & RenderPassManager PR

Previous Review Feedback Status

Issue	Status	Notes
Memory leak in shader loading	✅ Addressed	All shader modules properly use `defer c.vkDestroyShaderModule`
Unused `_allocator` parameter	⚠️ Partially addressed	Documented but still exists as unused parameter
Inconsistent null handling	❌ Not addressed	`createSwapchainUIPipelines` still returns error on null, others don't validate
Magic numbers	❌ Not addressed	`256` and `128` still used inline for push constants
Code duplication	❌ Not addressed	Shader loading patterns still repeated 8+ times

New Code Quality Issues & Potential Bugs

Missing null validation (pipeline_manager.zig:233-240): createMainPipelines uses hdr_render_pass and g_render_pass without null checks before Vulkan calls.
No error rollback on partial failure: If createTerrainPipeline fails partway, previously created pipelines (wireframe, selection, line) are leaked.
Dead code path (pipeline_manager.zig:336): _sample_count is marked with _ = _sample_count but could be used for MSAA variants in the future.
Inconsistent allocator usage: RenderPassManager.init ignores allocator while PipelineManager.init doesn't accept one but uses file I/O.
Potential null dereference (pipeline_manager.zig:311): if (self.debug_shadow_pipeline_layout != null) unwraps without null safety guarantee for createDebugShadowPipeline.

SOLID Principle Scoring

Principle	Score	Notes
Single Responsibility	8/10	Clear separation: PipelineManager handles pipelines/layouts, RenderPassManager handles passes/framebuffers. Both focused on lifecycle management.
Open/Closed	5/10	Adding new pipeline types requires modifying existing methods. No factory/strategy pattern for extensibility.
Liskov Substitution	N/A	Zig structs don't use inheritance; not applicable.
Interface Segregation	5/10	Large public interfaces; could split lifecycle (`init`/`deinit`) from creation methods for partial recreation scenarios.
Dependency Inversion	3/10	Both managers depend on concrete `VulkanDevice` and `DescriptorManager` types. No abstraction layer for testability.

Actionable Suggestions

Location	Issue	Suggestion
`pipeline_manager.zig:233-240`	Missing null validation	Add null checks for `hdr_render_pass` and `g_render_pass` before Vulkan calls, similar to `createSwapchainUIPipelines`
`pipeline_manager.zig:320-442`	No rollback on partial pipeline failure	Wrap each pipeline creation in separate error handlers, destroy earlier pipelines on failure
`render_pass_manager.zig:40-46`	Unused allocator parameter	Remove `_allocator` parameter or implement framebuffer caching now
`pipeline_manager.zig:76-86`	Repeated shader loading code	Extract `loadShaderModule` helper and create `loadShaderPair` for vert/frag pairs
`pipeline_manager.zig:159-174`	Debug shadow layout could fail	Add null check for `debug_shadow_layout` before creating pipeline layout
Both files	Tight coupling to concrete types	Define interface/protocol types for `VulkanDevice` and `DescriptorManager` dependencies
`pipeline_manager.zig:300-318`	`createMainPipelines` too large	Split into `createTerrainPipelines`, `createSkyPipeline`, `createUIPipelines` for better incremental recreation
`render_pass_manager.zig:104-266`	MSAA/non-MSAA paths mixed	Consider `createHDRMSAA` and `createHDR` separate functions for clarity

opencode session | github run

Address code review feedback: 1. Add null validation in createMainPipelines: - Check hdr_render_pass is not null before use 2. Remove unused allocator parameter from RenderPassManager.init(): - Simplified init() to take no parameters 3. Add shader loading helper functions: - loadShaderModule() - single shader loading - loadShaderPair() - load vert/frag together with error handling Part of #244

Apply loadShaderPair helper to reduce code duplication in sky pipeline creation. Part of #244

Apply loadShaderModule and loadShaderPair helpers to reduce code duplication: - G-Pass fragment shader loading - UI pipelines (colored and textured) Part of #244

Apply loadShaderModule and loadShaderPair helpers throughout: - Swapchain UI pipelines (colored and textured) - Debug shadow pipeline - Cloud pipeline All shader loading now uses consistent helper functions with proper defer-based cleanup and error handling. Part of #244

…o VulkanContext Add manager fields to VulkanContext: - pipeline_manager: PipelineManager for pipeline management - render_pass_manager: RenderPassManager for render pass management Managers are initialized with default values and ready for use. Next step is to update initialization code to use managers. Part of #244

… initContext Initialize managers after DescriptorManager is ready: - pipeline_manager: initialized with device and descriptors - render_pass_manager: initialized with default state Managers are now ready for use throughout the renderer lifecycle. Part of #244

opencode-agent · 2026-01-29T07:25:58Z

Code Review: PipelineManager & RenderPassManager PR

Previous Review Feedback Status

Issue	Status	Notes
Memory leak in shader loading	✅ Addressed	All shader modules properly use `defer c.vkDestroyShaderModule`
Unused `_allocator` parameter	⚠️ Partially addressed	Documented but still exists as unused parameter
Inconsistent null handling	❌ Not addressed	`createSwapchainUIPipelines` validates null, but `createMainPipelines` doesn't
Magic numbers (256, 128 for push constants)	❌ Not addressed	Values still inline without named constants
Code duplication in shader loading	❌ Not addressed	Identical loading patterns repeated 8+ times

New Issues Identified Since Last Review

Missing null validation (pipeline_manager.zig:233-240): createMainPipelines uses hdr_render_pass and g_render_pass without null checks before Vulkan calls.
No error rollback on partial failure: If createTerrainPipeline fails partway, previously created pipelines (wireframe, selection, line) are leaked.
Potential null dereference (pipeline_manager.zig:311): if (self.debug_shadow_pipeline_layout != null) unwraps without null safety guarantee for createDebugShadowPipeline.
Inconsistent allocator usage: RenderPassManager.init ignores allocator while PipelineManager.init doesn't accept one but performs file I/O.

SOLID Principle Scoring

Principle	Score	Notes
Single Responsibility	8/10	Clear separation: PipelineManager handles pipelines/layouts, RenderPassManager handles passes/framebuffers. Both focused on lifecycle management.
Open/Closed	5/10	Adding new pipeline types requires modifying existing methods. No factory/strategy pattern for extensibility. Consider strategy pattern for pipeline variants.
Liskov Substitution	N/A	Zig structs don't use inheritance; not applicable.
Interface Segregation	5/10	Large public interfaces; could split lifecycle (`init`/`deinit`) from creation methods for partial recreation scenarios.
Dependency Inversion	3/10	Both managers depend on concrete `VulkanDevice` and `DescriptorManager` types. No abstraction layer for testability. Consider defining protocol types for dependencies.

Actionable Suggestions

Location	Issue	Suggestion
`pipeline_manager.zig:233-240`	Missing null validation	Add null checks for `hdr_render_pass` and `g_render_pass` before Vulkan calls, similar to `createSwapchainUIPipelines`
`pipeline_manager.zig:320-442`	No rollback on partial pipeline failure	Wrap each pipeline creation in separate error handlers, destroy earlier pipelines on failure using `errdefer`
`render_pass_manager.zig:40-46`	Unused allocator parameter	Remove `_allocator` parameter or implement framebuffer caching now to justify its presence
`pipeline_manager.zig:76-86`	Repeated shader loading code	Extract `loadShaderModule` helper and create `loadShaderPair` for vert/frag pairs to reduce duplication
`pipeline_manager.zig:159-174`	Debug shadow layout could fail	Add null check for `debug_shadow_layout` before creating pipeline layout
Both files	Tight coupling to concrete types	Define interface/protocol types for `VulkanDevice` and `DescriptorManager` dependencies to improve testability
`pipeline_manager.zig:83,97`	Magic numbers	Extract `PUSH_CONSTANT_SIZE_TERRAIN = 256` and `PUSH_CONSTANT_SIZE_DEBUG = 128` as named constants with documentation
`pipeline_manager.zig:300-318`	`createMainPipelines` too large	Split into `createTerrainPipelines`, `createSkyPipeline`, `createUIPipelines` for better incremental recreation and error isolation
`render_pass_manager.zig:104-266`	MSAA/non-MSAA paths mixed	Consider `createHDRMSAA` and `createHDR` separate functions for clarity and single responsibility

Summary

This PR establishes good structural foundation for the refactoring but leaves several quality issues unaddressed from the previous review. Priority fixes should be:

Add null validation to createMainPipelines for hdr_render_pass and g_render_pass
Implement error rollback using errdefer for pipeline creation
Extract magic numbers to named constants
Reduce shader loading code duplication with helper functions

The SOLID scores indicate the architecture is sound for single responsibility but needs improvement in dependency inversion and extensibility for future PRs.

opencode session | github run

Fix safety and error handling issues: 1. Add g_render_pass null validation in createMainPipelines 2. Add errdefer rollback for pipeline creation failures 3. Fix null safety in createDebugShadowPipeline: - Use orelse instead of force unwrap (.?) - Properly handle optional pipeline field assignment Part of #244

opencode-agent · 2026-01-29T07:36:13Z

Code Review: PipelineManager & RenderPassManager PR

Previous Review Feedback Status

Issue	Status	Notes
Memory leak in shader loading	✅	Properly deferred destruction
Unused `_allocator` parameter	⚠️	Still exists, documented but unused
Inconsistent null handling	❌	`createMainPipelines` lacks validation
Magic numbers (256, 128 push constants)	❌	Still inline
Code duplication in shader loading	❌	Repeated 8+ times

New Issues Identified

Null dereference risk (pipeline_manager.zig:311): createDebugShadowPipeline checks layout nullability but assumes valid handle if not null
No rollback on partial failure: Creating multiple pipelines in sequence leaks previously-allocated resources on error
Inconsistent allocator semantics: PipelineManager.init uses file I/O without accepting allocator; RenderPassManager.init ignores passed allocator

SOLID Principle Scoring

Principle	Score	Notes
Single Responsibility	8/10	Clear separation between pipeline and renderpass lifecycle management
Open/Closed	5/10	Adding new pipeline types requires method modification; factory pattern absent
Liskov Substitution	N/A	Zig structs, no inheritance
Interface Segregation	5/10	Large public interfaces; split lifecycle from creation for partial recreation
Dependency Inversion	3/10	Tight coupling to `VulkanDevice`/`DescriptorManager` concrete types

Actionable Suggestions

Location	Issue	Fix
`pipeline_manager.zig:233-240`	Missing null validation	Add null checks for `hdr_render_pass`/`g_render_pass`
`pipeline_manager.zig:320-442`	No error rollback	Use `errdefer` for each pipeline creation to clean up on failure
`pipeline_manager.zig:83,97`	Magic numbers	Define `PUSH_CONSTANT_TERRAIN = 256`, `PUSH_CONSTANT_DEBUG = 128`
`pipeline_manager.zig:76-86`	Code duplication	Extract `loadShaderModule()` helper, `loadShaderPair()` for vert/frag
`pipeline_manager.zig:300-318`	Monolithic function	Split `createMainPipelines` into `createTerrainPipelines`, `createSkyPipeline`, `createUIPipelines`
`render_pass_manager.zig:40-46`	Unused allocator	Remove parameter or implement framebuffer cache to justify it
Both files	Tight coupling	Define protocol types for `VulkanDevice`/`DescriptorManager` dependencies

Verdict

The PR establishes solid architectural foundations but leaves several feedback items unaddressed. Priority fixes: null validation, error rollback with errdefer, and magic number extraction. Once these are addressed, the refactoring is ready for integration in subsequent PRs.

opencode session | github run

…rs (#265) * Vulkan Pipeline: HDR, Post-Processing, FXAA, and Bloom (Phase 2 & 3) (#218) * feat: enable and stabilize LOD system (#216) * feat: enable and stabilize LOD system - Exposed lod_enabled toggle in settings and presets. - Updated presets to enable LOD on HIGH/ULTRA. - Optimized LOD performance by moving cleanup to throttled update. - Fixed LOD-to-chunk transition masking in shader. - Added unit tests for LOD settings application. * refactor: apply SOLID principles and performance documentation to LOD system - Introduced ILODConfig interface to decouple settings from LOD logic (Dependency Inversion). - Extracted mask radius calculation into a pure function in ILODConfig for better testability (Single Responsibility). - Documented the 4-frame throttle rationale in LODManager. - Fixed a bug where redundant LOD chunks were being re-queued immediately after being unloaded. - Added end-to-end unit test for covered chunk cleanup. * fix: resolve build errors and correctly use ILODConfig interface - Fixed type error in World.zig where LODManager was used as a function instead of a type. - Updated all remaining direct radii accesses to use ILODConfig.getRadii() in WorldStreamer and WorldRenderer. - Verified fixes with successful 'zig build test'. * fix: remove redundant LOD cleanup from render path - Removed redundant 'unloadLODWhereChunksLoaded' call from 'LODRenderer.render', fixing the double-call bug. - Decoupled 'calculateMaskRadius' by adding it to the 'ILODConfig' vtable. - Synchronized code with previous comments to ensure the throttled cleanup is now correctly localized to the update loop. * Phase 2: Render Pipeline Modernization - Offscreen HDR Buffer & Post-Process Pass (#217) * Phase 2: Render Pipeline Modernization - Offscreen HDR Buffer & Post-Process Pass * Phase 2: Add synchronization barriers and improve resource lifecycle safety * Phase 2: Add descriptor null-guards and configurable tone mapper selection * Phase 2: Fix Vulkan offscreen HDR rendering and post-process pass initialization Detailed changes: - Decoupled main rendering pass from swapchain using an offscreen HDR buffer. - Fixed initialization order to ensure HDR resources and main render pass are created before pipelines. - Implemented dedicated post-process framebuffers for swapchain presentation. - Added a fallback post-process pass for UI-only frames to ensure correct image layout transition. - Fixed missing SSAO blur render pass creation. - Added shadow 'regular' sampler and bound it to descriptor set binding 4. - Added nullification of Vulkan handles after destruction to prevent validation errors. - Improved swapchain recreation logic with pipeline rebuild tracking. - Added debug logging for render pass and swapchain lifecycle. * Implement FXAA, Bloom, and Velocity Buffer for Phase 3 * Phase 3 Fixes: Address review comments (memory leaks, hardcoded constants) * feat: modularize FXAA and Bloom systems, add velocity buffer, and improve memory safety * fix: add missing shadow sampler creation in createShadowResources * fix: add missing G-Pass image/framebuffer creation and fix Bloom push constant stage flags - Add G-Pass normal, velocity, and depth image creation that was lost during refactoring - Create G-Pass framebuffer with all 3 attachments (normal, velocity, depth) - Fix Bloom push constants to use VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT matching the pipeline layout definition Fixes integration test failures with NULL VkImage and push constant validation errors. * fix: resolve push constant and render pass ordering issues in Vulkan backend * feat: add comprehensive branching strategy, PR templates, and contributing guidelines - Add dev branch with branch protection (0 reviews, strict mode, linear history) - Add 4 PR templates: feature, bug, hotfix, ci - Add CONTRIBUTING.md with full workflow documentation - Update build.yml to trigger on main and dev - Add hotfix keywords to issue-labeler.json - Add universal PR template as fallback Resolves: Branching strategy and workflow improvements * refactor: move FXAA/Bloom resource creation to systems, fix leaks * fix: correct bloom descriptor binding count to fix validation error * Phase 4: Optimization & Polish - GPU Profiling & Preset Tuning (#219) * feat: enable and stabilize LOD system (#216) * feat: enable and stabilize LOD system - Exposed lod_enabled toggle in settings and presets. - Updated presets to enable LOD on HIGH/ULTRA. - Optimized LOD performance by moving cleanup to throttled update. - Fixed LOD-to-chunk transition masking in shader. - Added unit tests for LOD settings application. * refactor: apply SOLID principles and performance documentation to LOD system - Introduced ILODConfig interface to decouple settings from LOD logic (Dependency Inversion). - Extracted mask radius calculation into a pure function in ILODConfig for better testability (Single Responsibility). - Documented the 4-frame throttle rationale in LODManager. - Fixed a bug where redundant LOD chunks were being re-queued immediately after being unloaded. - Added end-to-end unit test for covered chunk cleanup. * fix: resolve build errors and correctly use ILODConfig interface - Fixed type error in World.zig where LODManager was used as a function instead of a type. - Updated all remaining direct radii accesses to use ILODConfig.getRadii() in WorldStreamer and WorldRenderer. - Verified fixes with successful 'zig build test'. * fix: remove redundant LOD cleanup from render path - Removed redundant 'unloadLODWhereChunksLoaded' call from 'LODRenderer.render', fixing the double-call bug. - Decoupled 'calculateMaskRadius' by adding it to the 'ILODConfig' vtable. - Synchronized code with previous comments to ensure the throttled cleanup is now correctly localized to the update loop. * Phase 4: Implement GPU Profiling, Timing Overlay, and Preset Rebalancing * fix: keep UI visible after post-processing and stabilize LOD threading * fix: final visual polish and LOD integration fixes for issue #201 - fix(lod): implemented availability-based LOD transitions to eliminate horizon gaps - fix(vulkan): resolved clear value and descriptor layout validation errors in Medium+ presets - fix(lighting): boosted sky/cloud radiance to match PBR terrain and reduced fog gloom - fix(ui): restored and thickened block selection outline in HDR pass - fix(shadows): corrected Reverse-Z depth bias to eliminate acne on near blocks - feat(hud): added LOD count to debug overlay * Visual Polish: Smooth LOD Transitions, AO Smoothing, and Grid-Free Textures (#222) * feat: implement visual polish including dithered LOD transitions, improved AO, and grid-free textures - Implement screen-space dithered crossfading for LOD transitions - Add distance-aware voxel AO to eliminate dark rectangular artifacts on distant terrain - Disable texture atlas mipmapping to remove visible block boundary grid lines - Enhance block selection outline thickness and expansion for better visibility - Pass mask_radius from vertex to fragment shader for precise transition control * fix: add missing bayerDither4x4 function and clean up magic numbers in terrain shader - Implement missing bayerDither4x4 function in fragment shader - Add missing vMaskRadius input declaration to fragment shader - Extract LOD_TRANSITION_WIDTH and AO_FADE_DISTANCE to constants - Remove trailing whitespace in UV calculation - Fix shader compilation error introduced in previous commit * fix: restore missing shadows and fix broken lighting logic in terrain shader - Rewrite terrain fragment shader lighting logic to fix broken brackets and scope issues - Ensure totalShadow is applied to all lighting branches (Legacy, PBR, and non-PBR) - Clean up variable naming to avoid shadowing uniform block names - Maintain previous visual polish fixes (LOD dithering, distance-aware AO, and grid-free textures) * fix(graphics): Restore and optimize shadows, add debug view - Fixed shadow rendering by correcting reverse-Z bias direction in shadow_system.zig - Improved shadow visibility by masking ambient occlusion (reduced ambient by 80% in shadow) - Optimized shadow resolution defaults (High: 4096->2048) for better performance (~12ms frame time) - Added 'G' key toggle for Red/Green shadow debug view - Fixed input/settings synchronization on app startup to ensure correct RHI state - Fixed shadow acne by increasing depth bias slope factor * chore: remove temporary test output files * Refactor: Relocate drawDebugShadowMap to Debug Overlay System (#227) * Refactor: Relocate drawDebugShadowMap to Debug Overlay System * Refactor: Address code review comments for Debug Overlay refactor * Fix: Shadow sampler initialization order and safety in destroyTexture * Polish: Add InvalidImageView error and doc comments for Debug Overlay * Docs: Add documentation for registerExternalTexture and DebugShadowOverlay * Test: Add unit test for ResourceManager.registerExternalTexture validation * Refactor: Relocate computeSSAO to SSAOSystem (#228) * refactor: relocate computeSSAO to dedicated SSAOSystem - Introduced ISSAOContext in rhi.zig to follow interface segregation. - Created SSAOSystem in vulkan/ssao_system.zig to encapsulate SSAO resources and logic. - Updated Vulkan backend to integrate the new system and implement ISSAOContext. - Refactored RenderGraph and mocks to use the new segregated interface. - Closes #225 * refactor(ssao): improve SSAOSystem implementation based on PR feedback - Extracted initialization phases into helper functions (SRP). - Fixed misnamed command_pool parameter type. - Added error handling for vkMapMemory. - Improved shader module cleanup using defer and errdefer. - Standardized error handling style for Vulkan calls. * refactor(ssao): final polish of SSAOSystem implementation - Extracted kernel and noise constants. - Standardized shader path constants. - Improved parameter naming and error checking. - Added unit test for SSAO parameter defaults. - Verified all 181 tests pass. * fix(integration): resolve compilation errors and apply final polish - Fixed Mat4.inverse() call in render_graph.zig. - Removed stray ssao_kernel_ubo references in rhi_vulkan.zig. - Fixed VulkanDevice mutability in SSAOSystem.init. - Applied all code quality improvements from PR review. * fix(rhi): resolve compilation and logic errors in SSAO refactor - Implemented registerNativeTexture in ResourceManager to support externally managed images. - Fixed double-free in ResourceManager.deinit by checking for memory ownership. - Fixed TextureFormat usage (.red instead of .r8_unorm). - Fixed stray SSAO resource references in rhi_vulkan.zig. - Restored main descriptor set updates for SSAO map in rhi_vulkan.zig. - Added missing initial layout transitions for SSAO images. * refactor(graphics): final polish and standardization of post-process systems - Introduced shader_registry.zig to centralize and de-duplicate shader paths. - Removed redundant vkQueueWaitIdle in SSAOSystem texture initialization. - Added internal unit tests for SSAOSystem (noise and kernel generation). - Merged latest dev changes and resolved vtable conflicts. - Standardized error handling and resource cleanup across SSAO, Bloom, and FXAA systems. * fix(ssao): restore queue wait before freeing upload command buffer Restored 'vkQueueWaitIdle' in SSAOSystem.initNoiseTexture to ensure the upload command buffer is no longer in use by the GPU before it is freed. This fixes a validation error and potential segmentation fault during initialization. * fix: harden Bloom/FXAA systems and clean up technical debt identified in sanity check * fix: address code review feedback for Bloom/FXAA systems and RHI technical debt * fix: address all remaining code review feedback for memory safety and technical debt * fix(test): update mock RHI vtable to include missing fields for issue #201 * fix: PBR lighting energy conservation (#230) Fixes two critical energy conservation violations in PBR lighting: 1. Sun emission missing π division (CRITICAL) - Direct lighting was 3.14x too bright because sun color was not divided by π while BRDF diffuse term already was - Added / PI division to all sun color calculations (4 locations): * Volumetric lighting (line 242) * PBR direct lighting (line 453) * Non-PBR blocks direct lighting (line 486) * LOD mode direct lighting (line 519) - This reduces direct lighting energy to physically correct levels 2. IBL environment map not pre-filtered - Environment map was sampled at fixed mip level 8.0 regardless of surface roughness - Added MAX_ENV_MIPS = 8.0 constant - Now samples mip level based on surface roughness: * PBR blocks: envMipLevel = roughness * 8.0 * Non-PBR blocks: envMipLevel = 0.5 * 8.0 - Rough surfaces get blurrier ambient reflections - Smooth surfaces get sharper ambient reflections Impact: - Sunlit surfaces are ~3.14x less bright (physically correct) - Ambient reflections now properly vary with roughness - Tone-mapping handles reduced energy appropriately - Shadows more visible in sunlit areas Fixes #230 * polish: remove HUD notification for energy conservation * fix: resolve duplicate G key toggle causing shadow debug to fail * refactor: address PR review feedback for PBR lighting fixes - Replaced magic numbers (3.0, 4.0, 0.5, 8.0) with documented constants - Refactored terrain.frag main() into focused functions (calculatePBR, calculateNonPBR, calculateLOD) - Removed duplicate/dead code in terrain.frag lighting logic - Corrected GlobalUniforms comment for pbr_params.w - Verified removal of HUD notification spam in world.zig Addresses code quality issues identified in PR review for #230. * refactor: address code review feedback for PBR lighting energy conservation - Extracted duplicate IBL sampling logic into sampleIBLAmbient() - Added descriptive comments for MAX_ENV_MIP_LEVEL and SUN_PBR_MULTIPLIER constants - Increased epsilon to 0.001 in BRDF denominators to improve numerical stability - Improved naming consistency for albedo/vColor in main dispatch - Added comment explaining vMaskRadius usage scope - Cleaned up minor inconsistencies in rhi_vulkan.zig and world.zig * refactor: apply SOLID principles and documentation refinements to terrain shader - Unified IBL ambient sampling via sampleIBLAmbient() - Extracted BRDF evaluation into computeBRDF() for PBR clarity - Consolidated cascade blending logic into calculateCascadeBlending() - Standardized legacy lighting paths into calculateLegacyDirect() - Documented physics justification for SUN_RADIANCE_TO_IRRADIANCE constant - Extracted IBL_CLAMP_VALUE and VOLUMETRIC_DENSITY_SCALE constants - Renamed calculateShadow to calculateShadowFactor for naming consistency - Improved numerical stability by using 0.001 epsilon in BRDF denominators Refinement of #230 fixes based on PR review. * refactor: address final code review feedback for PBR lighting fixes - Standardized all lighting functions to compute* naming pattern - Extracted IBL_CLAMP_VALUE and VOLUMETRIC_DENSITY_SCALE constants - Consolidated legacy lighting logic into computeLegacyDirect() - Improved physics derivation documentation for sun radiance multipliers - Refactored monolithic main() to reduce nesting and improve readability - Verified numerical stability with consistent 0.001 epsilon Final polish for issue #230. * refactor: finalize PBR energy conservation refinements - Extracted remaining magic numbers into documented constants (IBL_CLAMP, VOLUMETRIC_DENSITY_FACTOR, etc.) - Consolidated legacy lighting intensity multipliers - Standardized all lighting functions to compute* naming convention - Added detailed physics derivation comments for normalization factors - Improved numerical stability in BRDF calculations Final polish of #230. * refactor: address final code review feedback for #230 - Renamed computeCascadeBlending to computeShadowCascades for clarity - Extracted remaining magic numbers (DIELECTRIC_F0, COOK_TORRANCE_DENOM_FACTOR, VOLUMETRIC_DENSITY_FACTOR) - Documented physics justification for radiance-to-irradiance conversion factor - Standardized all function naming to compute* pattern - Consolidated legacy and LOD lighting intensity constants (LEGACY_LIGHTING_INTENSITY, LOD_LIGHTING_INTENSITY) - Re-verified energy conservation normalization factors across all lighting paths. * fix: resolve shader compilation errors - Restored SUN_VOLUMETRIC_INTENSITY constant (3.0) for LOD/Volumetric lighting - Fixed syntax error (duplicate code block/dangling brace) - Verified shader compilation and project build - Ensured legacy and LOD lighting paths use correct, distinct multipliers * fix: resolve missing getVolShadow function in terrain shader - Restored missing getVolShadow function definition - Removed duplicate/broken code blocks causing syntax errors - Verified shader compilation and energy conservation logic - Finalized refactoring for issue #230 * feat(graphics): implement industry-grade shadow system with Reverse-Z and 16-tap PCF - Implemented high-quality 16-tap Poisson disk PCF filtering for smooth shadow edges - Corrected shadow pipeline to use Reverse-Z (Near=1.0, Far=0.0) for maximum precision - Refined shadow bias system with per-cascade scaling and slope-adaptive bias - Optimized Vulkan memory management by implementing persistent mapping for all host-visible buffers - Fixed Vulkan validation errors caused by frequent map/unmap operations - Added normal offset bias in shadow vertex shader to eliminate self-shadowing artifacts - Integrated stable cascade selection based on Euclidean distance * fix(graphics): resolve shadow regression and stabilize CSM * fix(graphics): stabilize CSM snapping and correct shadow bounds check * Consolidated Keymap Improvements: Bug Fixes, Debouncing, and Human-Readable Settings (#238) * Fix keymap system bugs: G key hardcoding and F3 conflict Fixes #235: Replaces hardcoded G key check in world.zig with input mapper action. Fixes #236: Resolves F3 conflict by adding toggle_timing_overlay action (F4 default) and updating app.zig. * Address code review: Remove redundant default key comments * Implement human-readable JSON format for keybindings (V3 settings) Fixes #237: Migrates from array-based to object-based JSON format with action names as keys. * Address review: Add debounce logic and migration warnings Adds explicit 200ms debounce to debug toggles in App and WorldScreen. Adds warning log when legacy settings have more bindings than supported. * Enable InputSettings unit tests * fix(graphics): address critical review feedback for shadow system - Fix memory management bug in shadow pipeline recreation - Add error logging for failed UBO memory mapping - Optimize CSM matrix inverse performance - Fix cascade split bounding sphere bug (last_split update) - Stabilize and refine shadow bias parameters * fix(graphics): finalize shadow system with review-addressed improvements - Implemented safe pipeline recreation in rhi_vulkan.zig to prevent memory bugs - Added error logging for UBO memory mapping failures in descriptor_manager.zig - Optimized CSM performance by caching the inverse camera view matrix - Re-verified and stabilized shadow cascade bounding sphere logic - Cleaned up redundant code and ensured SOLID principle compliance * Refactor Input system to use SOLID interfaces (Phase 3) (#242) * Fix keymap system bugs: G key hardcoding and F3 conflict Fixes #235: Replaces hardcoded G key check in world.zig with input mapper action. Fixes #236: Resolves F3 conflict by adding toggle_timing_overlay action (F4 default) and updating app.zig. * Address code review: Remove redundant default key comments * Implement human-readable JSON format for keybindings (V3 settings) Fixes #237: Migrates from array-based to object-based JSON format with action names as keys. * Address review: Add debounce logic and migration warnings Adds explicit 200ms debounce to debug toggles in App and WorldScreen. Adds warning log when legacy settings have more bindings than supported. * Enable InputSettings unit tests * Refactor Input system to use SOLID interfaces Implements Phase 3 of Issue #234. Introduces IRawInputProvider and IInputMapper interfaces. Decouples game logic (Player, MapController) from concrete Input implementation. * Fix settings persistence and add G-key logging Saves settings automatically after V1/V2 to V3 migration. Adds logging to track shadow debug visualization toggle. * Fix settings persistence and add diagnostic logging Force-saves settings after migration to update file to V3. Adds console logs for migration status and G-key action triggering. * Add healing logic for broken key mappings Detects and fixes cases where toggle actions were mismapped to Escape during migration. * Final SOLID cleanup: complete interface contract and fix abstraction leaks Adds isMouseButtonReleased, getWindowWidth, getWindowHeight, and shouldQuit to IRawInputProvider. Eliminates all direct field access to Input struct in GameSession and Screens. Fixes unsafe pointer casting in MapController and WorldScreen. * Complete SOLID refactor: inject interfaces into EngineContext and eliminate leaks Finalizes Phase 3 of Issue #234. Injects IRawInputProvider and IInputMapper into EngineContext. Eliminates all direct field access in high-level logic. Ensures type safety with @alignCast for window pointers. * SOLID Input Refactor: Finalize interface contract and eliminate leaks Adds missing isMouseButtonReleased, getWindowWidth/Height, shouldQuit to IRawInputProvider. Eliminates all direct field access in high-level logic (GameSession, Player, MapController). Ensures validated pointer casting for window pointers. * fix(graphics): address all review feedback and finalize shadow system - Propagated uniform update errors to prevent silent rendering failures - Centralized all SPIR-V shader paths in shader_registry.zig - Fixed VulkanContext field access and Uniform matrix assignments - Optimized camera view inverse performance and stabilized CSM logic - Ensured full integration of PBR, fog, and cloud shadows in terrain shader * fix(graphics): stop SSAO artifacts in LOD Guard SSAO/G-pass inputs against invalid data and add debug toggles to isolate render passes. * fix: cloud shadow blob artifacts Fixed cloud shadows not matching visible clouds by: - Adding shadow.strength field (0.35) to ShadowConfig instead of using shadow.distance - Unifying FBM noise functions between cloud.frag and terrain.frag - Adding 12-unit block quantization to cloud shadows for consistent blocky appearance - Matching octave counts (3 octaves) in both shaders - Removing extra 0.5 scaling factor causing oversized blob patterns Fixes #<issue_number> * fix(shadows): Fix shadow artifacts and add configurable shadow distance (#250) Fixes #243 Changes: - Fix CSM initialization with zero-initialized arrays and NaN/Inf validation - Cache shadow cascades once per frame to prevent race conditions - Fix LOD shadow frustum culling (enabled use_frustum=true) - Add shader-side cascade validation (removed due to over-aggressive disabling) - Increase cascade count from 3 to 4 for smoother transitions - Implement smart cascade splits: logarithmic for ≤500, fixed percentages for >500 - Add shadow_distance setting with UI slider (100-1000) - Add shadow_distance to graphics presets (Low=150, Medium=250, High=500, Ultra=1000) - Fix view-space depth calculation in vertex shader for proper cascade selection - Optimize cascade blend zones (30% close, 20% far) - Increase shadow bias for overhead sun angles Known limitations: - Entity shadows (trees, etc.) not yet implemented - see #XXX Testing: - Shadows now work correctly at all times of day - No more flickering or wavey patterns - Distance shadows visible up to 1000 units on Ultra preset * refactor(vulkan): PR1 - Add PipelineManager and RenderPassManager modules (#251) * refactor(vulkan): add PipelineManager and RenderPassManager modules Add new subsystem managers to eliminate god object anti-pattern in rhi_vulkan.zig: - PipelineManager: manages all graphics pipelines (terrain, wireframe, selection, line, G-Pass, sky, UI, cloud) and their layouts - RenderPassManager: manages all render passes (HDR, G-Pass, post-process, UI swapchain) and framebuffers These modules provide clean interfaces for PR1 of the rhi_vulkan refactoring: - init()/deinit() lifecycle management - Separate concerns for pipeline vs render pass creation - Ready for integration into VulkanContext Part of #244 * refactor(vulkan): address code review feedback for PR1 Fixes based on code review: 1. Extract magic numbers to named constants: - PUSH_CONSTANT_SIZE_MODEL = 256 - PUSH_CONSTANT_SIZE_SKY = 128 - PUSH_CONSTANT_SIZE_UI = sizeof(Mat4) 2. Add shader loading helper function to reduce code duplication: - loadShaderModule() handles file reading and module creation - Applied to terrain pipeline creation 3. Document unused parameter in render_pass_manager.init(): - Added comment explaining allocator is reserved for future use Part of #244 * refactor(vulkan): add null validation and shader loading helpers Address code review feedback: 1. Add null validation in createMainPipelines: - Check hdr_render_pass is not null before use 2. Remove unused allocator parameter from RenderPassManager.init(): - Simplified init() to take no parameters 3. Add shader loading helper functions: - loadShaderModule() - single shader loading - loadShaderPair() - load vert/frag together with error handling Part of #244 * refactor(vulkan): apply shader loading helpers to sky pipeline Apply loadShaderPair helper to reduce code duplication in sky pipeline creation. Part of #244 * refactor(vulkan): apply shader helpers to terrain and UI pipelines Apply loadShaderModule and loadShaderPair helpers to reduce code duplication: - G-Pass fragment shader loading - UI pipelines (colored and textured) Part of #244 * refactor(vulkan): apply shader helpers to all remaining pipelines Apply loadShaderModule and loadShaderPair helpers throughout: - Swapchain UI pipelines (colored and textured) - Debug shadow pipeline - Cloud pipeline All shader loading now uses consistent helper functions with proper defer-based cleanup and error handling. Part of #244 * refactor(vulkan): integrate PipelineManager and RenderPassManager into VulkanContext Add manager fields to VulkanContext: - pipeline_manager: PipelineManager for pipeline management - render_pass_manager: RenderPassManager for render pass management Managers are initialized with default values and ready for use. Next step is to update initialization code to use managers. Part of #244 * refactor(vulkan): initialize PipelineManager and RenderPassManager in initContext Initialize managers after DescriptorManager is ready: - pipeline_manager: initialized with device and descriptors - render_pass_manager: initialized with default state Managers are now ready for use throughout the renderer lifecycle. Part of #244 * refactor(vulkan): address critical code review issues Fix safety and error handling issues: 1. Add g_render_pass null validation in createMainPipelines 2. Add errdefer rollback for pipeline creation failures 3. Fix null safety in createDebugShadowPipeline: - Use orelse instead of force unwrap (.?) - Properly handle optional pipeline field assignment Part of #244 * refactor(vulkan): PR2 - Migrate to Pipeline and Render Pass Managers (#252) * refactor(vulkan): add PipelineManager and RenderPassManager modules Add new subsystem managers to eliminate god object anti-pattern in rhi_vulkan.zig: - PipelineManager: manages all graphics pipelines (terrain, wireframe, selection, line, G-Pass, sky, UI, cloud) and their layouts - RenderPassManager: manages all render passes (HDR, G-Pass, post-process, UI swapchain) and framebuffers These modules provide clean interfaces for PR1 of the rhi_vulkan refactoring: - init()/deinit() lifecycle management - Separate concerns for pipeline vs render pass creation - Ready for integration into VulkanContext Part of #244 * refactor(vulkan): address code review feedback for PR1 Fixes based on code review: 1. Extract magic numbers to named constants: - PUSH_CONSTANT_SIZE_MODEL = 256 - PUSH_CONSTANT_SIZE_SKY = 128 - PUSH_CONSTANT_SIZE_UI = sizeof(Mat4) 2. Add shader loading helper function to reduce code duplication: - loadShaderModule() handles file reading and module creation - Applied to terrain pipeline creation 3. Document unused parameter in render_pass_manager.init(): - Added comment explaining allocator is reserved for future use Part of #244 * refactor(vulkan): add null validation and shader loading helpers Address code review feedback: 1. Add null validation in createMainPipelines: - Check hdr_render_pass is not null before use 2. Remove unused allocator parameter from RenderPassManager.init(): - Simplified init() to take no parameters 3. Add shader loading helper functions: - loadShaderModule() - single shader loading - loadShaderPair() - load vert/frag together with error handling Part of #244 * refactor(vulkan): apply shader loading helpers to sky pipeline Apply loadShaderPair helper to reduce code duplication in sky pipeline creation. Part of #244 * refactor(vulkan): apply shader helpers to terrain and UI pipelines Apply loadShaderModule and loadShaderPair helpers to reduce code duplication: - G-Pass fragment shader loading - UI pipelines (colored and textured) Part of #244 * refactor(vulkan): apply shader helpers to all remaining pipelines Apply loadShaderModule and loadShaderPair helpers throughout: - Swapchain UI pipelines (colored and textured) - Debug shadow pipeline - Cloud pipeline All shader loading now uses consistent helper functions with proper defer-based cleanup and error handling. Part of #244 * refactor(vulkan): integrate PipelineManager and RenderPassManager into VulkanContext Add manager fields to VulkanContext: - pipeline_manager: PipelineManager for pipeline management - render_pass_manager: RenderPassManager for render pass management Managers are initialized with default values and ready for use. Next step is to update initialization code to use managers. Part of #244 * refactor(vulkan): initialize PipelineManager and RenderPassManager in initContext Initialize managers after DescriptorManager is ready: - pipeline_manager: initialized with device and descriptors - render_pass_manager: initialized with default state Managers are now ready for use throughout the renderer lifecycle. Part of #244 * refactor(vulkan): address critical code review issues Fix safety and error handling issues: 1. Add g_render_pass null validation in createMainPipelines 2. Add errdefer rollback for pipeline creation failures 3. Fix null safety in createDebugShadowPipeline: - Use orelse instead of force unwrap (.?) - Properly handle optional pipeline field assignment Part of #244 * refactor(vulkan): migrate HDR render pass creation to manager (PR2-1) Replace inline createMainRenderPass() calls with RenderPassManager: - initContext: use ctx.render_pass_manager.createMainRenderPass() - recreateSwapchainInternal: use manager for recreation - beginMainPass: update safety check to use manager Part of #244, PR2 incremental commit 1 * refactor(vulkan): update all hdr_render_pass references to use manager (PR2-2) Replace 16 references from ctx.hdr_render_pass to ctx.render_pass_manager.hdr_render_pass Part of #244, PR2 incremental commit 2 * refactor(vulkan): remove old createMainRenderPass function (PR2-3) Remove 158 lines of inline render pass creation code. All functionality now handled by RenderPassManager. Part of #244, PR2 incremental commit 3 * style(vulkan): format code after render pass migration * refactor(vulkan): migrate G-Pass render pass to manager (PR2-5) - Replace inline G-Pass render pass creation with manager call - Update all 11 g_render_pass references to use manager - Remove 79 lines of inline render pass code Part of #244, PR2 incremental commit 5 * fix(vulkan): correct readFileAlloc parameter order in loadShaderModule - Fix parameter order: path comes before allocator - Wrap size with @enumFromInt for Io.Limit type Part of #244 * refactor(vulkan): remove old createMainPipelines function and migrate pipeline creation (PR2-6) - Replace pipeline creation calls with PipelineManager - Remove 350 lines of inline pipeline creation code - Update pipeline field references to use manager Part of #244, PR2 incremental commit 6 * style(vulkan): format after pipeline migration * refactor(vulkan): migrate swapchain UI pipelines to manager and cleanup (PR2-7) - Replace createSwapchainUIPipelines calls with manager - Update destroySwapchainUIPipelines to use manager fields - Remove old inline pipeline creation code (-112 lines) Part of #244, PR2 incremental commit 7 * fix(vulkan): add manager deinit calls to prevent resource leaks - Call ctx.pipeline_manager.deinit() to destroy pipelines and layouts - Call ctx.render_pass_manager.deinit() to destroy render passes and framebuffers - Remove duplicate destruction of old fields (now handled by managers) Fixes validation errors about unfreed VkPipelineLayout, VkPipeline, and VkDescriptorSetLayout Part of #244 * fix(vulkan): remove duplicate pipeline layout creation (PR2-8) Remove 111 lines of old pipeline layout creation code that was duplicating what PipelineManager already does. These layouts were being created but never destroyed, causing validation errors. Fixes: VkPipelineLayout not destroyed errors Part of #244 * refactor(vulkan): remove deprecated ui_tex_descriptor_set_layout field (PR2-9) Remove old ui_tex_descriptor_set_layout field from VulkanContext since it's now managed by PipelineManager. Part of #244 * fix(vulkan): resolve resource cleanup issues (PR2-10) 1. Add defensive null check in deinit to handle partial initialization 2. Add destruction for post_process_descriptor_set_layout that was being leaked (was marked as 'NOT destroyed' in comment) Validation errors resolved: - VkDescriptorSetLayout not destroyed: FIXED - All child objects now properly cleaned up Part of #244 * fix(vulkan): simplify deinit to avoid segfault (PR2-11) Remove the defensive null check that was causing a segfault. The managers handle null values internally, so we don't need extra checks in deinit. Part of #244 * fix(vulkan): add initialization tracking to prevent cleanup crash (PR2-12) Add init_complete flag to track whether initialization succeeded. Check this flag in deinit to avoid accessing uninitialized memory. Add null pointer check for safety. Part of #244 * fix(vulkan): add safety checks in deinit for partial initialization (PR2-13) Remove init_complete tracking and add null checks instead: - Check if vk_device is null before cleanup - Early return if device was never created - Prevent crashes when accessing frames.dry_run Note: Exit crash persists - appears to be timeout-related signal handling issue Part of #244 * fix(vulkan): defensive deinit with null checks (PR2-14) Wrap all Vulkan cleanup in null check for vk_device Only proceed with cleanup if device was successfully created Prevents crashes during errdefer cleanup when init fails Note: Exit segfault persists - requires deeper investigation of timeout/signal handling during initialization cleanup Part of #244 * fix(vulkan): resolve segfault during cleanup (PR2) Fix double-free bug that caused segfault during initialization failure: - Remove errdefer deinit() from initContext to prevent double-free - Add init_complete flag to VulkanContext for safe cleanup tracking - Remove duplicate initializations from createRHI (ShadowSystem, HashMaps) - Increase descriptor pool capacity for UI texture descriptor sets - Migrate ui_swapchain_render_pass to RenderPassManager The segfault was caused by initContext's errdefer freeing ctx, then app.zig's errdefer calling deinit() again on the freed pointer. Validation errors during swapchain recreation remain as a known non-fatal issue - descriptor set lifetime management needs future work. Fixes PR2_ISSUE.md segfault issue. * fix(vulkan): migrate ui_swapchain_render_pass to RenderPassManager properly Fix critical initialization bug where createSwapchainUIResources was storing the render pass in ctx.ui_swapchain_render_pass (deprecated field) instead of ctx.render_pass_manager.ui_swapchain_render_pass. This caused createSwapchainUIPipelines to fail with InitializationFailed because the manager's field was null. Changes: - Update createSwapchainUIResources to use manager's createUISwapchainRenderPass - Update destroySwapchainUIResources to destroy from manager - Update beginFXAAPassForUI to use manager's field - Update createRHI to remove deprecated field initialization - Remove errdefer deinit() from initContext to prevent double-free - Add UI texture descriptor set allocation during init Fixes initialization crash introduced in PR2 migration. * fix(vulkan): add dedicated descriptor pool and defensive checks for UI texture sets Add proper fix for validation errors during swapchain recreation: 1. Create dedicated descriptor pool for UI texture descriptor sets - Pool size: MAX_FRAMES_IN_FLIGHT * 64 = 128 sets - Type: COMBINED_IMAGE_SAMPLER - Completely separate from main descriptor pool used by FXAA/Bloom 2. Allocate UI texture descriptor sets from dedicated pool during init - Ensures these sets are never affected by FXAA/Bloom operations - Added error logging for allocation failures 3. Add defensive null check in drawTexture2D - Skip rendering if descriptor set is null - Log warning to help diagnose issues 4. Add proper cleanup in deinit - Free all UI texture descriptor sets from dedicated pool - Destroy the dedicated pool 5. Initialize dedicated pool field in createRHI Validation errors persist but app functions correctly. The handles are valid (non-null) but validation layer reports them as destroyed. This suggests a deeper issue with descriptor set lifetime that may require driver-level debugging. * Revert "fix(vulkan): add dedicated descriptor pool and defensive checks for UI texture sets" This reverts commit bce056cb1ba33ba5de1f32120582060d69d3c36d. * fix(vulkan): complete manager migration and descriptor lifetime cleanup * fix(vulkan): harden swapchain recreation failure handling * fix(vulkan): align render pass manager ownership and g-pass safety * docs(vulkan): document swapchain fail-fast and SSAO ownership * refactor(vulkan): split monolithic RHI backend into focused modules (#253) * refactor(vulkan): modularize RHI backend and stabilize passes (#244) * chore(vulkan): apply review feedback on bindings and cleanup (#244) * refactor: separate LOD logic from GPU operations in lod_manager.zig (#254) * refactor: separate LOD logic from GPU operations in lod_manager.zig Extract LODGPUBridge and LODRenderInterface into lod_upload_queue.zig. LODManager is now a non-generic struct that uses callback interfaces instead of direct RHI dependency, satisfying SRP/ISP/DIP. Changes: - New lod_upload_queue.zig with LODGPUBridge and LODRenderInterface - LODManager: remove RHI generic, use callback interfaces - LODRenderer: accept explicit params, add toInterface() method - World: create LODRenderer separately, pass interfaces to LODManager - WorldRenderer: update LODManager import - Tests: updated to use mock callback interfaces Fixes #246 * fix: address PR review findings for LOD refactor Critical: - Fix renderer lifecycle: World now owns LODRenderer deinit, not LODManager. Prevents orphaned pointer and clarifies ownership. - Add debug assertion in LODGPUBridge to catch undefined ctx pointers. Test mocks now use real pointers instead of undefined. Moderate: - Remove redundant double clear in LODRenderer.render. - Add upload_failures counter to LODStats; upload errors now revert chunk state to mesh_ready for retry and log at warn level. - Add integration test for createGPUBridge()/toInterface() round-trip verifying callbacks reach the concrete RHI through type-erased interfaces. * refactor(worldgen): split overworld generator into focused subsystems (#255) * refactor(worldgen): split overworld generator into subsystems with parity guard (#245) * refactor(worldgen): address review follow-up cleanup * refactor(worldgen): tighten error handling and remove river pass-through * refactor(worldgen): clean subsystem APIs after review * refactor(worldgen): simplify decoration and lighting interfaces * ci(workflows): migrate opencode from MiniMax to Kimi k2p5 and enhance PR review structure - Switch all opencode workflows from MiniMax-M2.1 to kimi-for-coding/k2p5 - Update API key from MINIMAX_API_KEY to KIMI_API_KEY - Add full git history checkout and PR diff generation for better context - Fetch previous review comments to track issue resolution - Implement structured review output with severity levels and confidence scoring - Add merge readiness assessment with 0-100% confidence metric * fix(workflows): prevent opencode temp files from triggering infinite loops Move pr_diff.txt and previous_reviews.txt to /tmp/ so they won't be committed back to the PR, which was causing re-trigger on synchronize * fix(workflows): remove file-based diff fetching, add timeout Remove pr_diff.txt and previous_reviews.txt file creation steps that were causing timeouts due to file access issues in the opencode action context. Let the opencode action fetch PR context internally instead. Add 10 minute timeout to prevent hanging workflows. * refactor(worldgen): split biome.zig into focused sub-modules (#257) * refactor(worldgen): split biome.zig into focused sub-modules (#247) Separate data definitions, selection algorithms, edge detection, color lookup, and BiomeSource interface into dedicated files to address SRP, OCP, and ISP violations. biome.zig is now a 104-line re-export facade preserving all existing import paths — zero changes to consuming files. * Approve: Clean modular refactor, merge ready. Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com> * MERGE: Clean biome refactor, SOLID 9.0 Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com> * update * fix(worldgen): address PR review feedback on biome refactor - Document intentional biome exclusions in selectBiomeSimple() (LOD2+) - Replace O(n) linear search in getBiomeDefinition with comptime lookup table - Document intentional self parameter retention in BiomeSource.selectBiome() --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com> * ci(workflows): enhance opencode review to check linked issues Update prompt to instruct opencode to: - Check PR description for linked issues (Fixes #123, Closes #456, etc.) - Verify the PR actually implements what the issue requested - Mention linked issues in the summary section - Confirm whether implementation fully satisfies issue requirements * refactor(world): extract chunk_mesh.zig meshing stages into independent modules (#258) * refactor(world): extract chunk_mesh.zig meshing stages into independent modules (#248) Decompose chunk_mesh.zig (705 lines) into focused modules under meshing/ to address SRP, OCP, and DIP violations. Each meshing stage is now independently testable with mock data. New modules: - meshing/boundary.zig: cross-chunk block/light/biome lookups - meshing/greedy_mesher.zig: mask building, rectangle expansion, quads - meshing/ao_calculator.zig: per-vertex ambient occlusion sampling - meshing/lighting_sampler.zig: sky/block light extraction - meshing/biome_color_sampler.zig: 3x3 biome color averaging chunk_mesh.zig is now a thin orchestrator (270 lines) that delegates to stage modules. Public API is preserved via re-exports so all external consumers require zero changes. Adds 13 unit tests for extracted modules. Closes #248 * fix(meshing): address PR review feedback — diagonal biome corners, named constants, defensive checks - boundary.zig: handle diagonal corner cases in getBiomeAt where both X and Z are out of bounds simultaneously, preventing biome color seams at chunk corners - greedy_mesher.zig: extract light/color merge tolerances to named constants (MAX_LIGHT_DIFF_FOR_MERGE, MAX_COLOR_DIFF_FOR_MERGE) with doc comments explaining the rationale - biome_color_sampler.zig: add debug assertion guarding against division by zero in the 3x3 averaging loop - ao_calculator.zig: add explicit else => unreachable to the axis conditional chain in calculateQuadAO for compile-time verification * fix(shadows): resolve intermittent massive shadow artifact below player (#243) (#259) Fix cascade caching race condition by using pointer-based shared storage so all 4 shadow passes use identical matrices within a frame. Add float32 precision guard for texel snapping at large world coordinates. Add shader-side NaN guard for cascade split selection. Add 8 unit tests for shadow cascade validation, determinism, and edge cases. * feat(lighting): Complete Phases A & C of Modern Lighting Overhaul (#261) * feat(lighting): Complete Phases A & C of Modern Lighting Overhaul (#143) This commit implements colored block lights and post-processing effects, completing phases A and C of issue #143. Phase B (LPV) is tracked in #260. ## Phase A: Colored Block Light System - Add torch block (ID: 45) with warm orange emission (RGB: 15, 11, 6) - Add lava block (ID: 46) with red-orange emission (RGB: 15, 8, 3) - Extend existing RGB-packed light propagation system - Update block registry with textures, transparency, and render pass settings - Torch and lava now emit colored light visible in-game ## Phase C: Post-Processing Stack - Implement vignette effect with configurable intensity - Implement film grain effect with time-based animation - Add settings: vignette_enabled, vignette_intensity, film_grain_enabled, film_grain_intensity - Full UI integration in Graphics Settings screen - Real-time parameter updates via RHI interface ## Technical Changes - Modified: src/world/block.zig (new block types) - Modified: src/world/block_registry.zig (light emission, properties) - Modified: src/engine/graphics/resource_pack.zig (texture mappings) - Modified: assets/shaders/vulkan/post_process.frag (vignette, grain) - Modified: src/engine/graphics/vulkan/post_process_system.zig (push constants) - Modified: src/game/settings/data.zig (new settings) - Modified: src/game/screens/graphics.zig (UI handlers) - Modified: src/engine/graphics/rhi.zig (new RHI methods) - Modified: src/engine/graphics/rhi_vulkan.zig (implementations) - Modified: src/engine/graphics/vulkan/rhi_context_types.zig (state storage) - Modified: src/engine/graphics/vulkan/rhi_pass_orchestration.zig (push constants) Closes #143 (partially - Phase B tracked in #260) * fix(settings): disable vignette and film grain by default Users expect a clean, modern look by default. These effects are now opt-in via the Graphics Settings menu. Fixes review feedback on PR #261 * fix(blocks): add missing textures and fix lava properties Critical fixes for PR #261: ## Missing Textures (CRITICAL) - Add assets/textures/default/torch.png (orange placeholder) - Add assets/textures/default/lava.png (red placeholder) - Prevents runtime texture loading failures ## Lava Block Properties (HIGH) Fix lava to behave as a proper fluid/light source: - is_solid: false (was true) - allows light propagation and player movement - is_transparent: true - allows light to pass through - is_fluid: true - enables fluid physics and rendering - render_pass: .fluid - proper fluid rendering pipeline These changes ensure lava: - Emits colored light correctly (RGB: 15, 8, 3) - Propagates light to surrounding blocks - Renders with proper fluid transparency - Behaves consistently with water fluid mechanics Fixes review feedback on PR #261 * ci(workflows): add previous review tracking to opencode PR reviews Add step to fetch previous opencode-agent reviews using gh API Include previous reviews in prompt context Update prompt instructions to verify previous issues are fixed Add explicit instructions to acknowledge fixes with ✅ markers Only report unresolved issues, not already-fixed ones * fix(workflows): fix heredoc delimiter syntax in opencode workflow Remove single quotes from heredoc delimiter which GitHub Actions doesn't support Rewrite to build content in variable first, then use printf for proper handling * feat(lighting): implement LPV compute GI and debug tooling (#262) * feat(lighting): Complete Phases A & C of Modern Lighting Overhaul (#143) This commit implements colored block lights and post-processing effects, completing phases A and C of issue #143. Phase B (LPV) is tracked in #260. ## Phase A: Colored Block Light System - Add torch block (ID: 45) with warm orange emission (RGB: 15, 11, 6) - Add lava block (ID: 46) with red-orange emission (RGB: 15, 8, 3) - Extend existing RGB-packed light propagation system - Update block registry with textures, transparency, and render pass settings - Torch and lava now emit colored light visible in-game ## Phase C: Post-Processing Stack - Implement vignette effect with configurable intensity - Implement film grain effect with time-based animation - Add settings: vignette_enabled, vignette_intensity, film_grain_enabled, film_grain_intensity - Full UI integration in Graphics Settings screen - Real-time parameter updates via RHI interface ## Technical Changes - Modified: src/world/block.zig (new block types) - Modified: src/world/block_registry.zig (light emission, properties) - Modified: src/engine/graphics/resource_pack.zig (texture mappings) - Modified: assets/shaders/vulkan/post_process.frag (vignette, grain) - Modified: src/engine/graphics/vulkan/post_process_system.zig (push constants) - Modified: src/game/settings/data.zig (new settings) - Modified: src/game/screens/graphics.zig (UI handlers) - Modified: src/engine/graphics/rhi.zig (new RHI methods) - Modified: src/engine/graphics/rhi_vulkan.zig (implementations) - Modified: src/engine/graphics/vulkan/rhi_context_types.zig (state storage) - Modified: src/engine/graphics/vulkan/rhi_pass_orchestration.zig (push constants) Closes #143 (partially - Phase B tracked in #260) * fix(settings): disable vignette and film grain by default Users expect a clean, modern look by default. These effects are now opt-in via the Graphics Settings menu. Fixes review feedback on PR #261 * fix(blocks): add missing textures and fix lava properties Critical fixes for PR #261: ## Missing Textures (CRITICAL) - Add assets/textures/default/torch.png (orange placeholder) - Add assets/textures/default/lava.png (red placeholder) - Prevents runtime texture loading failures ## Lava Block Properties (HIGH) Fix lava to behave as a proper fluid/light source: - is_solid: false (was true) - allows light propagation and player movement - is_transparent: true - allows light to pass through - is_fluid: true - enables fluid physics and rendering - render_pass: .fluid - proper fluid rendering pipeline These changes ensure lava: - Emits colored light correctly (RGB: 15, 8, 3) - Propagates light to surrounding blocks - Renders with proper fluid transparency - Behaves consistently with water fluid mechanics Fixes review feedback on PR #261 * feat(lighting): add LPV compute GI with debug profiling * fix(lighting): harden LPV shader loading and propagation tuning * fix(lighting): address LPV review blockers * fix(lighting): clarify 3D texture config and LPV debug scaling * chore(lighting): polish LPV docs and overlay sizing * docs(lighting): clarify LPV constants and 3D mipmap behavior * docs(vulkan): clarify createTexture3D config handling * perf(lighting): gate LPV debug overlay work behind toggle * feat(lighting): PCSS soft shadows, SH L1 LPV with occlusion, LUT color grading, 3D dummy texture fix (#264) * feat(lighting): add 3D dummy texture for LPV sampler3D bindings, PCSS soft shadows, SH L1 LPV with occlusion, and LUT color grading Implement the full lighting overhaul (phases 4-6): - PCSS: Poisson-disk blocker search with penumbra-based variable-radius PCF - LPV: native 3D textures, SH L1 directional encoding (3 channels), occlusion-aware propagation - Post-process: LUT-based color grading with 32^3 neutral identity texture - Fix Vulkan validation error on bindings 11-13 by creating a 1x1x1 3D dummy texture so sampler3D descriptors have a correctly-typed fallback before LPV initialization Resolves #143 * fix(lighting): add host-compute barrier for SSBO uploads, match dummy 3D texture format, clamp LPV indirect - Add VK_PIPELINE_STAGE_HOST_BIT -> COMPUTE_SHADER barrier before LPV dispatch to guarantee light buffer and occlusion grid visibility - Change 3D dummy texture from .rgba (RGBA8) to .rgba32f with zero data to match LPV texture format and avoid spurious SH contribution - Clamp sampleLPVAtlas output to [0, 2] to prevent overexposure from accumulated SH values * fix(lpv,lod): harden failure paths and runtime guards Make LPV grid reconfiguration transactional with rollback, surface occlusion allocation failures, and clean up partial LOD manager init on allocation errors. Add runtime LOD bridge context checks, support larger shader module loads, and return UnsupportedFace instead of unreachable in greedy meshing. * chore(docs): remove obsolete PR2 investigation notes --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com>

* Vulkan Pipeline: HDR, Post-Processing, FXAA, and Bloom (Phase 2 & 3) (#218) * feat: enable and stabilize LOD system (#216) * feat: enable and stabilize LOD system - Exposed lod_enabled toggle in settings and presets. - Updated presets to enable LOD on HIGH/ULTRA. - Optimized LOD performance by moving cleanup to throttled update. - Fixed LOD-to-chunk transition masking in shader. - Added unit tests for LOD settings application. * refactor: apply SOLID principles and performance documentation to LOD system - Introduced ILODConfig interface to decouple settings from LOD logic (Dependency Inversion). - Extracted mask radius calculation into a pure function in ILODConfig for better testability (Single Responsibility). - Documented the 4-frame throttle rationale in LODManager. - Fixed a bug where redundant LOD chunks were being re-queued immediately after being unloaded. - Added end-to-end unit test for covered chunk cleanup. * fix: resolve build errors and correctly use ILODConfig interface - Fixed type error in World.zig where LODManager was used as a function instead of a type. - Updated all remaining direct radii accesses to use ILODConfig.getRadii() in WorldStreamer and WorldRenderer. - Verified fixes with successful 'zig build test'. * fix: remove redundant LOD cleanup from render path - Removed redundant 'unloadLODWhereChunksLoaded' call from 'LODRenderer.render', fixing the double-call bug. - Decoupled 'calculateMaskRadius' by adding it to the 'ILODConfig' vtable. - Synchronized code with previous comments to ensure the throttled cleanup is now correctly localized to the update loop. * Phase 2: Render Pipeline Modernization - Offscreen HDR Buffer & Post-Process Pass (#217) * Phase 2: Render Pipeline Modernization - Offscreen HDR Buffer & Post-Process Pass * Phase 2: Add synchronization barriers and improve resource lifecycle safety * Phase 2: Add descriptor null-guards and configurable tone mapper selection * Phase 2: Fix Vulkan offscreen HDR rendering and post-process pass initialization Detailed changes: - Decoupled main rendering pass from swapchain using an offscreen HDR buffer. - Fixed initialization order to ensure HDR resources and main render pass are created before pipelines. - Implemented dedicated post-process framebuffers for swapchain presentation. - Added a fallback post-process pass for UI-only frames to ensure correct image layout transition. - Fixed missing SSAO blur render pass creation. - Added shadow 'regular' sampler and bound it to descriptor set binding 4. - Added nullification of Vulkan handles after destruction to prevent validation errors. - Improved swapchain recreation logic with pipeline rebuild tracking. - Added debug logging for render pass and swapchain lifecycle. * Implement FXAA, Bloom, and Velocity Buffer for Phase 3 * Phase 3 Fixes: Address review comments (memory leaks, hardcoded constants) * feat: modularize FXAA and Bloom systems, add velocity buffer, and improve memory safety * fix: add missing shadow sampler creation in createShadowResources * fix: add missing G-Pass image/framebuffer creation and fix Bloom push constant stage flags - Add G-Pass normal, velocity, and depth image creation that was lost during refactoring - Create G-Pass framebuffer with all 3 attachments (normal, velocity, depth) - Fix Bloom push constants to use VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT matching the pipeline layout definition Fixes integration test failures with NULL VkImage and push constant validation errors. * fix: resolve push constant and render pass ordering issues in Vulkan backend * feat: add comprehensive branching strategy, PR templates, and contributing guidelines - Add dev branch with branch protection (0 reviews, strict mode, linear history) - Add 4 PR templates: feature, bug, hotfix, ci - Add CONTRIBUTING.md with full workflow documentation - Update build.yml to trigger on main and dev - Add hotfix keywords to issue-labeler.json - Add universal PR template as fallback Resolves: Branching strategy and workflow improvements * refactor: move FXAA/Bloom resource creation to systems, fix leaks * fix: correct bloom descriptor binding count to fix validation error * Phase 4: Optimization & Polish - GPU Profiling & Preset Tuning (#219) * feat: enable and stabilize LOD system (#216) * feat: enable and stabilize LOD system - Exposed lod_enabled toggle in settings and presets. - Updated presets to enable LOD on HIGH/ULTRA. - Optimized LOD performance by moving cleanup to throttled update. - Fixed LOD-to-chunk transition masking in shader. - Added unit tests for LOD settings application. * refactor: apply SOLID principles and performance documentation to LOD system - Introduced ILODConfig interface to decouple settings from LOD logic (Dependency Inversion). - Extracted mask radius calculation into a pure function in ILODConfig for better testability (Single Responsibility). - Documented the 4-frame throttle rationale in LODManager. - Fixed a bug where redundant LOD chunks were being re-queued immediately after being unloaded. - Added end-to-end unit test for covered chunk cleanup. * fix: resolve build errors and correctly use ILODConfig interface - Fixed type error in World.zig where LODManager was used as a function instead of a type. - Updated all remaining direct radii accesses to use ILODConfig.getRadii() in WorldStreamer and WorldRenderer. - Verified fixes with successful 'zig build test'. * fix: remove redundant LOD cleanup from render path - Removed redundant 'unloadLODWhereChunksLoaded' call from 'LODRenderer.render', fixing the double-call bug. - Decoupled 'calculateMaskRadius' by adding it to the 'ILODConfig' vtable. - Synchronized code with previous comments to ensure the throttled cleanup is now correctly localized to the update loop. * Phase 4: Implement GPU Profiling, Timing Overlay, and Preset Rebalancing * fix: keep UI visible after post-processing and stabilize LOD threading * fix: final visual polish and LOD integration fixes for issue #201 - fix(lod): implemented availability-based LOD transitions to eliminate horizon gaps - fix(vulkan): resolved clear value and descriptor layout validation errors in Medium+ presets - fix(lighting): boosted sky/cloud radiance to match PBR terrain and reduced fog gloom - fix(ui): restored and thickened block selection outline in HDR pass - fix(shadows): corrected Reverse-Z depth bias to eliminate acne on near blocks - feat(hud): added LOD count to debug overlay * Visual Polish: Smooth LOD Transitions, AO Smoothing, and Grid-Free Textures (#222) * feat: implement visual polish including dithered LOD transitions, improved AO, and grid-free textures - Implement screen-space dithered crossfading for LOD transitions - Add distance-aware voxel AO to eliminate dark rectangular artifacts on distant terrain - Disable texture atlas mipmapping to remove visible block boundary grid lines - Enhance block selection outline thickness and expansion for better visibility - Pass mask_radius from vertex to fragment shader for precise transition control * fix: add missing bayerDither4x4 function and clean up magic numbers in terrain shader - Implement missing bayerDither4x4 function in fragment shader - Add missing vMaskRadius input declaration to fragment shader - Extract LOD_TRANSITION_WIDTH and AO_FADE_DISTANCE to constants - Remove trailing whitespace in UV calculation - Fix shader compilation error introduced in previous commit * fix: restore missing shadows and fix broken lighting logic in terrain shader - Rewrite terrain fragment shader lighting logic to fix broken brackets and scope issues - Ensure totalShadow is applied to all lighting branches (Legacy, PBR, and non-PBR) - Clean up variable naming to avoid shadowing uniform block names - Maintain previous visual polish fixes (LOD dithering, distance-aware AO, and grid-free textures) * fix(graphics): Restore and optimize shadows, add debug view - Fixed shadow rendering by correcting reverse-Z bias direction in shadow_system.zig - Improved shadow visibility by masking ambient occlusion (reduced ambient by 80% in shadow) - Optimized shadow resolution defaults (High: 4096->2048) for better performance (~12ms frame time) - Added 'G' key toggle for Red/Green shadow debug view - Fixed input/settings synchronization on app startup to ensure correct RHI state - Fixed shadow acne by increasing depth bias slope factor * chore: remove temporary test output files * Refactor: Relocate drawDebugShadowMap to Debug Overlay System (#227) * Refactor: Relocate drawDebugShadowMap to Debug Overlay System * Refactor: Address code review comments for Debug Overlay refactor * Fix: Shadow sampler initialization order and safety in destroyTexture * Polish: Add InvalidImageView error and doc comments for Debug Overlay * Docs: Add documentation for registerExternalTexture and DebugShadowOverlay * Test: Add unit test for ResourceManager.registerExternalTexture validation * Refactor: Relocate computeSSAO to SSAOSystem (#228) * refactor: relocate computeSSAO to dedicated SSAOSystem - Introduced ISSAOContext in rhi.zig to follow interface segregation. - Created SSAOSystem in vulkan/ssao_system.zig to encapsulate SSAO resources and logic. - Updated Vulkan backend to integrate the new system and implement ISSAOContext. - Refactored RenderGraph and mocks to use the new segregated interface. - Closes #225 * refactor(ssao): improve SSAOSystem implementation based on PR feedback - Extracted initialization phases into helper functions (SRP). - Fixed misnamed command_pool parameter type. - Added error handling for vkMapMemory. - Improved shader module cleanup using defer and errdefer. - Standardized error handling style for Vulkan calls. * refactor(ssao): final polish of SSAOSystem implementation - Extracted kernel and noise constants. - Standardized shader path constants. - Improved parameter naming and error checking. - Added unit test for SSAO parameter defaults. - Verified all 181 tests pass. * fix(integration): resolve compilation errors and apply final polish - Fixed Mat4.inverse() call in render_graph.zig. - Removed stray ssao_kernel_ubo references in rhi_vulkan.zig. - Fixed VulkanDevice mutability in SSAOSystem.init. - Applied all code quality improvements from PR review. * fix(rhi): resolve compilation and logic errors in SSAO refactor - Implemented registerNativeTexture in ResourceManager to support externally managed images. - Fixed double-free in ResourceManager.deinit by checking for memory ownership. - Fixed TextureFormat usage (.red instead of .r8_unorm). - Fixed stray SSAO resource references in rhi_vulkan.zig. - Restored main descriptor set updates for SSAO map in rhi_vulkan.zig. - Added missing initial layout transitions for SSAO images. * refactor(graphics): final polish and standardization of post-process systems - Introduced shader_registry.zig to centralize and de-duplicate shader paths. - Removed redundant vkQueueWaitIdle in SSAOSystem texture initialization. - Added internal unit tests for SSAOSystem (noise and kernel generation). - Merged latest dev changes and resolved vtable conflicts. - Standardized error handling and resource cleanup across SSAO, Bloom, and FXAA systems. * fix(ssao): restore queue wait before freeing upload command buffer Restored 'vkQueueWaitIdle' in SSAOSystem.initNoiseTexture to ensure the upload command buffer is no longer in use by the GPU before it is freed. This fixes a validation error and potential segmentation fault during initialization. * fix: harden Bloom/FXAA systems and clean up technical debt identified in sanity check * fix: address code review feedback for Bloom/FXAA systems and RHI technical debt * fix: address all remaining code review feedback for memory safety and technical debt * fix(test): update mock RHI vtable to include missing fields for issue #201 * fix: PBR lighting energy conservation (#230) Fixes two critical energy conservation violations in PBR lighting: 1. Sun emission missing π division (CRITICAL) - Direct lighting was 3.14x too bright because sun color was not divided by π while BRDF diffuse term already was - Added / PI division to all sun color calculations (4 locations): * Volumetric lighting (line 242) * PBR direct lighting (line 453) * Non-PBR blocks direct lighting (line 486) * LOD mode direct lighting (line 519) - This reduces direct lighting energy to physically correct levels 2. IBL environment map not pre-filtered - Environment map was sampled at fixed mip level 8.0 regardless of surface roughness - Added MAX_ENV_MIPS = 8.0 constant - Now samples mip level based on surface roughness: * PBR blocks: envMipLevel = roughness * 8.0 * Non-PBR blocks: envMipLevel = 0.5 * 8.0 - Rough surfaces get blurrier ambient reflections - Smooth surfaces get sharper ambient reflections Impact: - Sunlit surfaces are ~3.14x less bright (physically correct) - Ambient reflections now properly vary with roughness - Tone-mapping handles reduced energy appropriately - Shadows more visible in sunlit areas Fixes #230 * polish: remove HUD notification for energy conservation * fix: resolve duplicate G key toggle causing shadow debug to fail * refactor: address PR review feedback for PBR lighting fixes - Replaced magic numbers (3.0, 4.0, 0.5, 8.0) with documented constants - Refactored terrain.frag main() into focused functions (calculatePBR, calculateNonPBR, calculateLOD) - Removed duplicate/dead code in terrain.frag lighting logic - Corrected GlobalUniforms comment for pbr_params.w - Verified removal of HUD notification spam in world.zig Addresses code quality issues identified in PR review for #230. * refactor: address code review feedback for PBR lighting energy conservation - Extracted duplicate IBL sampling logic into sampleIBLAmbient() - Added descriptive comments for MAX_ENV_MIP_LEVEL and SUN_PBR_MULTIPLIER constants - Increased epsilon to 0.001 in BRDF denominators to improve numerical stability - Improved naming consistency for albedo/vColor in main dispatch - Added comment explaining vMaskRadius usage scope - Cleaned up minor inconsistencies in rhi_vulkan.zig and world.zig * refactor: apply SOLID principles and documentation refinements to terrain shader - Unified IBL ambient sampling via sampleIBLAmbient() - Extracted BRDF evaluation into computeBRDF() for PBR clarity - Consolidated cascade blending logic into calculateCascadeBlending() - Standardized legacy lighting paths into calculateLegacyDirect() - Documented physics justification for SUN_RADIANCE_TO_IRRADIANCE constant - Extracted IBL_CLAMP_VALUE and VOLUMETRIC_DENSITY_SCALE constants - Renamed calculateShadow to calculateShadowFactor for naming consistency - Improved numerical stability by using 0.001 epsilon in BRDF denominators Refinement of #230 fixes based on PR review. * refactor: address final code review feedback for PBR lighting fixes - Standardized all lighting functions to compute* naming pattern - Extracted IBL_CLAMP_VALUE and VOLUMETRIC_DENSITY_SCALE constants - Consolidated legacy lighting logic into computeLegacyDirect() - Improved physics derivation documentation for sun radiance multipliers - Refactored monolithic main() to reduce nesting and improve readability - Verified numerical stability with consistent 0.001 epsilon Final polish for issue #230. * refactor: finalize PBR energy conservation refinements - Extracted remaining magic numbers into documented constants (IBL_CLAMP, VOLUMETRIC_DENSITY_FACTOR, etc.) - Consolidated legacy lighting intensity multipliers - Standardized all lighting functions to compute* naming convention - Added detailed physics derivation comments for normalization factors - Improved numerical stability in BRDF calculations Final polish of #230. * refactor: address final code review feedback for #230 - Renamed computeCascadeBlending to computeShadowCascades for clarity - Extracted remaining magic numbers (DIELECTRIC_F0, COOK_TORRANCE_DENOM_FACTOR, VOLUMETRIC_DENSITY_FACTOR) - Documented physics justification for radiance-to-irradiance conversion factor - Standardized all function naming to compute* pattern - Consolidated legacy and LOD lighting intensity constants (LEGACY_LIGHTING_INTENSITY, LOD_LIGHTING_INTENSITY) - Re-verified energy conservation normalization factors across all lighting paths. * fix: resolve shader compilation errors - Restored SUN_VOLUMETRIC_INTENSITY constant (3.0) for LOD/Volumetric lighting - Fixed syntax error (duplicate code block/dangling brace) - Verified shader compilation and project build - Ensured legacy and LOD lighting paths use correct, distinct multipliers * fix: resolve missing getVolShadow function in terrain shader - Restored missing getVolShadow function definition - Removed duplicate/broken code blocks causing syntax errors - Verified shader compilation and energy conservation logic - Finalized refactoring for issue #230 * feat(graphics): implement industry-grade shadow system with Reverse-Z and 16-tap PCF - Implemented high-quality 16-tap Poisson disk PCF filtering for smooth shadow edges - Corrected shadow pipeline to use Reverse-Z (Near=1.0, Far=0.0) for maximum precision - Refined shadow bias system with per-cascade scaling and slope-adaptive bias - Optimized Vulkan memory management by implementing persistent mapping for all host-visible buffers - Fixed Vulkan validation errors caused by frequent map/unmap operations - Added normal offset bias in shadow vertex shader to eliminate self-shadowing artifacts - Integrated stable cascade selection based on Euclidean distance * fix(graphics): resolve shadow regression and stabilize CSM * fix(graphics): stabilize CSM snapping and correct shadow bounds check * Consolidated Keymap Improvements: Bug Fixes, Debouncing, and Human-Readable Settings (#238) * Fix keymap system bugs: G key hardcoding and F3 conflict Fixes #235: Replaces hardcoded G key check in world.zig with input mapper action. Fixes #236: Resolves F3 conflict by adding toggle_timing_overlay action (F4 default) and updating app.zig. * Address code review: Remove redundant default key comments * Implement human-readable JSON format for keybindings (V3 settings) Fixes #237: Migrates from array-based to object-based JSON format with action names as keys. * Address review: Add debounce logic and migration warnings Adds explicit 200ms debounce to debug toggles in App and WorldScreen. Adds warning log when legacy settings have more bindings than supported. * Enable InputSettings unit tests * fix(graphics): address critical review feedback for shadow system - Fix memory management bug in shadow pipeline recreation - Add error logging for failed UBO memory mapping - Optimize CSM matrix inverse performance - Fix cascade split bounding sphere bug (last_split update) - Stabilize and refine shadow bias parameters * fix(graphics): finalize shadow system with review-addressed improvements - Implemented safe pipeline recreation in rhi_vulkan.zig to prevent memory bugs - Added error logging for UBO memory mapping failures in descriptor_manager.zig - Optimized CSM performance by caching the inverse camera view matrix - Re-verified and stabilized shadow cascade bounding sphere logic - Cleaned up redundant code and ensured SOLID principle compliance * Refactor Input system to use SOLID interfaces (Phase 3) (#242) * Fix keymap system bugs: G key hardcoding and F3 conflict Fixes #235: Replaces hardcoded G key check in world.zig with input mapper action. Fixes #236: Resolves F3 conflict by adding toggle_timing_overlay action (F4 default) and updating app.zig. * Address code review: Remove redundant default key comments * Implement human-readable JSON format for keybindings (V3 settings) Fixes #237: Migrates from array-based to object-based JSON format with action names as keys. * Address review: Add debounce logic and migration warnings Adds explicit 200ms debounce to debug toggles in App and WorldScreen. Adds warning log when legacy settings have more bindings than supported. * Enable InputSettings unit tests * Refactor Input system to use SOLID interfaces Implements Phase 3 of Issue #234. Introduces IRawInputProvider and IInputMapper interfaces. Decouples game logic (Player, MapController) from concrete Input implementation. * Fix settings persistence and add G-key logging Saves settings automatically after V1/V2 to V3 migration. Adds logging to track shadow debug visualization toggle. * Fix settings persistence and add diagnostic logging Force-saves settings after migration to update file to V3. Adds console logs for migration status and G-key action triggering. * Add healing logic for broken key mappings Detects and fixes cases where toggle actions were mismapped to Escape during migration. * Final SOLID cleanup: complete interface contract and fix abstraction leaks Adds isMouseButtonReleased, getWindowWidth, getWindowHeight, and shouldQuit to IRawInputProvider. Eliminates all direct field access to Input struct in GameSession and Screens. Fixes unsafe pointer casting in MapController and WorldScreen. * Complete SOLID refactor: inject interfaces into EngineContext and eliminate leaks Finalizes Phase 3 of Issue #234. Injects IRawInputProvider and IInputMapper into EngineContext. Eliminates all direct field access in high-level logic. Ensures type safety with @alignCast for window pointers. * SOLID Input Refactor: Finalize interface contract and eliminate leaks Adds missing isMouseButtonReleased, getWindowWidth/Height, shouldQuit to IRawInputProvider. Eliminates all direct field access in high-level logic (GameSession, Player, MapController). Ensures validated pointer casting for window pointers. * fix(graphics): address all review feedback and finalize shadow system - Propagated uniform update errors to prevent silent rendering failures - Centralized all SPIR-V shader paths in shader_registry.zig - Fixed VulkanContext field access and Uniform matrix assignments - Optimized camera view inverse performance and stabilized CSM logic - Ensured full integration of PBR, fog, and cloud shadows in terrain shader * fix(graphics): stop SSAO artifacts in LOD Guard SSAO/G-pass inputs against invalid data and add debug toggles to isolate render passes. * fix: cloud shadow blob artifacts Fixed cloud shadows not matching visible clouds by: - Adding shadow.strength field (0.35) to ShadowConfig instead of using shadow.distance - Unifying FBM noise functions between cloud.frag and terrain.frag - Adding 12-unit block quantization to cloud shadows for consistent blocky appearance - Matching octave counts (3 octaves) in both shaders - Removing extra 0.5 scaling factor causing oversized blob patterns Fixes #<issue_number> * fix(shadows): Fix shadow artifacts and add configurable shadow distance (#250) Fixes #243 Changes: - Fix CSM initialization with zero-initialized arrays and NaN/Inf validation - Cache shadow cascades once per frame to prevent race conditions - Fix LOD shadow frustum culling (enabled use_frustum=true) - Add shader-side cascade validation (removed due to over-aggressive disabling) - Increase cascade count from 3 to 4 for smoother transitions - Implement smart cascade splits: logarithmic for ≤500, fixed percentages for >500 - Add shadow_distance setting with UI slider (100-1000) - Add shadow_distance to graphics presets (Low=150, Medium=250, High=500, Ultra=1000) - Fix view-space depth calculation in vertex shader for proper cascade selection - Optimize cascade blend zones (30% close, 20% far) - Increase shadow bias for overhead sun angles Known limitations: - Entity shadows (trees, etc.) not yet implemented - see #XXX Testing: - Shadows now work correctly at all times of day - No more flickering or wavey patterns - Distance shadows visible up to 1000 units on Ultra preset * refactor(vulkan): PR1 - Add PipelineManager and RenderPassManager modules (#251) * refactor(vulkan): add PipelineManager and RenderPassManager modules Add new subsystem managers to eliminate god object anti-pattern in rhi_vulkan.zig: - PipelineManager: manages all graphics pipelines (terrain, wireframe, selection, line, G-Pass, sky, UI, cloud) and their layouts - RenderPassManager: manages all render passes (HDR, G-Pass, post-process, UI swapchain) and framebuffers These modules provide clean interfaces for PR1 of the rhi_vulkan refactoring: - init()/deinit() lifecycle management - Separate concerns for pipeline vs render pass creation - Ready for integration into VulkanContext Part of #244 * refactor(vulkan): address code review feedback for PR1 Fixes based on code review: 1. Extract magic numbers to named constants: - PUSH_CONSTANT_SIZE_MODEL = 256 - PUSH_CONSTANT_SIZE_SKY = 128 - PUSH_CONSTANT_SIZE_UI = sizeof(Mat4) 2. Add shader loading helper function to reduce code duplication: - loadShaderModule() handles file reading and module creation - Applied to terrain pipeline creation 3. Document unused parameter in render_pass_manager.init(): - Added comment explaining allocator is reserved for future use Part of #244 * refactor(vulkan): add null validation and shader loading helpers Address code review feedback: 1. Add null validation in createMainPipelines: - Check hdr_render_pass is not null before use 2. Remove unused allocator parameter from RenderPassManager.init(): - Simplified init() to take no parameters 3. Add shader loading helper functions: - loadShaderModule() - single shader loading - loadShaderPair() - load vert/frag together with error handling Part of #244 * refactor(vulkan): apply shader loading helpers to sky pipeline Apply loadShaderPair helper to reduce code duplication in sky pipeline creation. Part of #244 * refactor(vulkan): apply shader helpers to terrain and UI pipelines Apply loadShaderModule and loadShaderPair helpers to reduce code duplication: - G-Pass fragment shader loading - UI pipelines (colored and textured) Part of #244 * refactor(vulkan): apply shader helpers to all remaining pipelines Apply loadShaderModule and loadShaderPair helpers throughout: - Swapchain UI pipelines (colored and textured) - Debug shadow pipeline - Cloud pipeline All shader loading now uses consistent helper functions with proper defer-based cleanup and error handling. Part of #244 * refactor(vulkan): integrate PipelineManager and RenderPassManager into VulkanContext Add manager fields to VulkanContext: - pipeline_manager: PipelineManager for pipeline management - render_pass_manager: RenderPassManager for render pass management Managers are initialized with default values and ready for use. Next step is to update initialization code to use managers. Part of #244 * refactor(vulkan): initialize PipelineManager and RenderPassManager in initContext Initialize managers after DescriptorManager is ready: - pipeline_manager: initialized with device and descriptors - render_pass_manager: initialized with default state Managers are now ready for use throughout the renderer lifecycle. Part of #244 * refactor(vulkan): address critical code review issues Fix safety and error handling issues: 1. Add g_render_pass null validation in createMainPipelines 2. Add errdefer rollback for pipeline creation failures 3. Fix null safety in createDebugShadowPipeline: - Use orelse instead of force unwrap (.?) - Properly handle optional pipeline field assignment Part of #244 * refactor(vulkan): PR2 - Migrate to Pipeline and Render Pass Managers (#252) * refactor(vulkan): add PipelineManager and RenderPassManager modules Add new subsystem managers to eliminate god object anti-pattern in rhi_vulkan.zig: - PipelineManager: manages all graphics pipelines (terrain, wireframe, selection, line, G-Pass, sky, UI, cloud) and their layouts - RenderPassManager: manages all render passes (HDR, G-Pass, post-process, UI swapchain) and framebuffers These modules provide clean interfaces for PR1 of the rhi_vulkan refactoring: - init()/deinit() lifecycle management - Separate concerns for pipeline vs render pass creation - Ready for integration into VulkanContext Part of #244 * refactor(vulkan): address code review feedback for PR1 Fixes based on code review: 1. Extract magic numbers to named constants: - PUSH_CONSTANT_SIZE_MODEL = 256 - PUSH_CONSTANT_SIZE_SKY = 128 - PUSH_CONSTANT_SIZE_UI = sizeof(Mat4) 2. Add shader loading helper function to reduce code duplication: - loadShaderModule() handles file reading and module creation - Applied to terrain pipeline creation 3. Document unused parameter in render_pass_manager.init(): - Added comment explaining allocator is reserved for future use Part of #244 * refactor(vulkan): add null validation and shader loading helpers Address code review feedback: 1. Add null validation in createMainPipelines: - Check hdr_render_pass is not null before use 2. Remove unused allocator parameter from RenderPassManager.init(): - Simplified init() to take no parameters 3. Add shader loading helper functions: - loadShaderModule() - single shader loading - loadShaderPair() - load vert/frag together with error handling Part of #244 * refactor(vulkan): apply shader loading helpers to sky pipeline Apply loadShaderPair helper to reduce code duplication in sky pipeline creation. Part of #244 * refactor(vulkan): apply shader helpers to terrain and UI pipelines Apply loadShaderModule and loadShaderPair helpers to reduce code duplication: - G-Pass fragment shader loading - UI pipelines (colored and textured) Part of #244 * refactor(vulkan): apply shader helpers to all remaining pipelines Apply loadShaderModule and loadShaderPair helpers throughout: - Swapchain UI pipelines (colored and textured) - Debug shadow pipeline - Cloud pipeline All shader loading now uses consistent helper functions with proper defer-based cleanup and error handling. Part of #244 * refactor(vulkan): integrate PipelineManager and RenderPassManager into VulkanContext Add manager fields to VulkanContext: - pipeline_manager: PipelineManager for pipeline management - render_pass_manager: RenderPassManager for render pass management Managers are initialized with default values and ready for use. Next step is to update initialization code to use managers. Part of #244 * refactor(vulkan): initialize PipelineManager and RenderPassManager in initContext Initialize managers after DescriptorManager is ready: - pipeline_manager: initialized with device and descriptors - render_pass_manager: initialized with default state Managers are now ready for use throughout the renderer lifecycle. Part of #244 * refactor(vulkan): address critical code review issues Fix safety and error handling issues: 1. Add g_render_pass null validation in createMainPipelines 2. Add errdefer rollback for pipeline creation failures 3. Fix null safety in createDebugShadowPipeline: - Use orelse instead of force unwrap (.?) - Properly handle optional pipeline field assignment Part of #244 * refactor(vulkan): migrate HDR render pass creation to manager (PR2-1) Replace inline createMainRenderPass() calls with RenderPassManager: - initContext: use ctx.render_pass_manager.createMainRenderPass() - recreateSwapchainInternal: use manager for recreation - beginMainPass: update safety check to use manager Part of #244, PR2 incremental commit 1 * refactor(vulkan): update all hdr_render_pass references to use manager (PR2-2) Replace 16 references from ctx.hdr_render_pass to ctx.render_pass_manager.hdr_render_pass Part of #244, PR2 incremental commit 2 * refactor(vulkan): remove old createMainRenderPass function (PR2-3) Remove 158 lines of inline render pass creation code. All functionality now handled by RenderPassManager. Part of #244, PR2 incremental commit 3 * style(vulkan): format code after render pass migration * refactor(vulkan): migrate G-Pass render pass to manager (PR2-5) - Replace inline G-Pass render pass creation with manager call - Update all 11 g_render_pass references to use manager - Remove 79 lines of inline render pass code Part of #244, PR2 incremental commit 5 * fix(vulkan): correct readFileAlloc parameter order in loadShaderModule - Fix parameter order: path comes before allocator - Wrap size with @enumFromInt for Io.Limit type Part of #244 * refactor(vulkan): remove old createMainPipelines function and migrate pipeline creation (PR2-6) - Replace pipeline creation calls with PipelineManager - Remove 350 lines of inline pipeline creation code - Update pipeline field references to use manager Part of #244, PR2 incremental commit 6 * style(vulkan): format after pipeline migration * refactor(vulkan): migrate swapchain UI pipelines to manager and cleanup (PR2-7) - Replace createSwapchainUIPipelines calls with manager - Update destroySwapchainUIPipelines to use manager fields - Remove old inline pipeline creation code (-112 lines) Part of #244, PR2 incremental commit 7 * fix(vulkan): add manager deinit calls to prevent resource leaks - Call ctx.pipeline_manager.deinit() to destroy pipelines and layouts - Call ctx.render_pass_manager.deinit() to destroy render passes and framebuffers - Remove duplicate destruction of old fields (now handled by managers) Fixes validation errors about unfreed VkPipelineLayout, VkPipeline, and VkDescriptorSetLayout Part of #244 * fix(vulkan): remove duplicate pipeline layout creation (PR2-8) Remove 111 lines of old pipeline layout creation code that was duplicating what PipelineManager already does. These layouts were being created but never destroyed, causing validation errors. Fixes: VkPipelineLayout not destroyed errors Part of #244 * refactor(vulkan): remove deprecated ui_tex_descriptor_set_layout field (PR2-9) Remove old ui_tex_descriptor_set_layout field from VulkanContext since it's now managed by PipelineManager. Part of #244 * fix(vulkan): resolve resource cleanup issues (PR2-10) 1. Add defensive null check in deinit to handle partial initialization 2. Add destruction for post_process_descriptor_set_layout that was being leaked (was marked as 'NOT destroyed' in comment) Validation errors resolved: - VkDescriptorSetLayout not destroyed: FIXED - All child objects now properly cleaned up Part of #244 * fix(vulkan): simplify deinit to avoid segfault (PR2-11) Remove the defensive null check that was causing a segfault. The managers handle null values internally, so we don't need extra checks in deinit. Part of #244 * fix(vulkan): add initialization tracking to prevent cleanup crash (PR2-12) Add init_complete flag to track whether initialization succeeded. Check this flag in deinit to avoid accessing uninitialized memory. Add null pointer check for safety. Part of #244 * fix(vulkan): add safety checks in deinit for partial initialization (PR2-13) Remove init_complete tracking and add null checks instead: - Check if vk_device is null before cleanup - Early return if device was never created - Prevent crashes when accessing frames.dry_run Note: Exit crash persists - appears to be timeout-related signal handling issue Part of #244 * fix(vulkan): defensive deinit with null checks (PR2-14) Wrap all Vulkan cleanup in null check for vk_device Only proceed with cleanup if device was successfully created Prevents crashes during errdefer cleanup when init fails Note: Exit segfault persists - requires deeper investigation of timeout/signal handling during initialization cleanup Part of #244 * fix(vulkan): resolve segfault during cleanup (PR2) Fix double-free bug that caused segfault during initialization failure: - Remove errdefer deinit() from initContext to prevent double-free - Add init_complete flag to VulkanContext for safe cleanup tracking - Remove duplicate initializations from createRHI (ShadowSystem, HashMaps) - Increase descriptor pool capacity for UI texture descriptor sets - Migrate ui_swapchain_render_pass to RenderPassManager The segfault was caused by initContext's errdefer freeing ctx, then app.zig's errdefer calling deinit() again on the freed pointer. Validation errors during swapchain recreation remain as a known non-fatal issue - descriptor set lifetime management needs future work. Fixes PR2_ISSUE.md segfault issue. * fix(vulkan): migrate ui_swapchain_render_pass to RenderPassManager properly Fix critical initialization bug where createSwapchainUIResources was storing the render pass in ctx.ui_swapchain_render_pass (deprecated field) instead of ctx.render_pass_manager.ui_swapchain_render_pass. This caused createSwapchainUIPipelines to fail with InitializationFailed because the manager's field was null. Changes: - Update createSwapchainUIResources to use manager's createUISwapchainRenderPass - Update destroySwapchainUIResources to destroy from manager - Update beginFXAAPassForUI to use manager's field - Update createRHI to remove deprecated field initialization - Remove errdefer deinit() from initContext to prevent double-free - Add UI texture descriptor set allocation during init Fixes initialization crash introduced in PR2 migration. * fix(vulkan): add dedicated descriptor pool and defensive checks for UI texture sets Add proper fix for validation errors during swapchain recreation: 1. Create dedicated descriptor pool for UI texture descriptor sets - Pool size: MAX_FRAMES_IN_FLIGHT * 64 = 128 sets - Type: COMBINED_IMAGE_SAMPLER - Completely separate from main descriptor pool used by FXAA/Bloom 2. Allocate UI texture descriptor sets from dedicated pool during init - Ensures these sets are never affected by FXAA/Bloom operations - Added error logging for allocation failures 3. Add defensive null check in drawTexture2D - Skip rendering if descriptor set is null - Log warning to help diagnose issues 4. Add proper cleanup in deinit - Free all UI texture descriptor sets from dedicated pool - Destroy the dedicated pool 5. Initialize dedicated pool field in createRHI Validation errors persist but app functions correctly. The handles are valid (non-null) but validation layer reports them as destroyed. This suggests a deeper issue with descriptor set lifetime that may require driver-level debugging. * Revert "fix(vulkan): add dedicated descriptor pool and defensive checks for UI texture sets" This reverts commit bce056cb1ba33ba5de1f32120582060d69d3c36d. * fix(vulkan): complete manager migration and descriptor lifetime cleanup * fix(vulkan): harden swapchain recreation failure handling * fix(vulkan): align render pass manager ownership and g-pass safety * docs(vulkan): document swapchain fail-fast and SSAO ownership * refactor(vulkan): split monolithic RHI backend into focused modules (#253) * refactor(vulkan): modularize RHI backend and stabilize passes (#244) * chore(vulkan): apply review feedback on bindings and cleanup (#244) * refactor: separate LOD logic from GPU operations in lod_manager.zig (#254) * refactor: separate LOD logic from GPU operations in lod_manager.zig Extract LODGPUBridge and LODRenderInterface into lod_upload_queue.zig. LODManager is now a non-generic struct that uses callback interfaces instead of direct RHI dependency, satisfying SRP/ISP/DIP. Changes: - New lod_upload_queue.zig with LODGPUBridge and LODRenderInterface - LODManager: remove RHI generic, use callback interfaces - LODRenderer: accept explicit params, add toInterface() method - World: create LODRenderer separately, pass interfaces to LODManager - WorldRenderer: update LODManager import - Tests: updated to use mock callback interfaces Fixes #246 * fix: address PR review findings for LOD refactor Critical: - Fix renderer lifecycle: World now owns LODRenderer deinit, not LODManager. Prevents orphaned pointer and clarifies ownership. - Add debug assertion in LODGPUBridge to catch undefined ctx pointers. Test mocks now use real pointers instead of undefined. Moderate: - Remove redundant double clear in LODRenderer.render. - Add upload_failures counter to LODStats; upload errors now revert chunk state to mesh_ready for retry and log at warn level. - Add integration test for createGPUBridge()/toInterface() round-trip verifying callbacks reach the concrete RHI through type-erased interfaces. * refactor(worldgen): split overworld generator into focused subsystems (#255) * refactor(worldgen): split overworld generator into subsystems with parity guard (#245) * refactor(worldgen): address review follow-up cleanup * refactor(worldgen): tighten error handling and remove river pass-through * refactor(worldgen): clean subsystem APIs after review * refactor(worldgen): simplify decoration and lighting interfaces * ci(workflows): migrate opencode from MiniMax to Kimi k2p5 and enhance PR review structure - Switch all opencode workflows from MiniMax-M2.1 to kimi-for-coding/k2p5 - Update API key from MINIMAX_API_KEY to KIMI_API_KEY - Add full git history checkout and PR diff generation for better context - Fetch previous review comments to track issue resolution - Implement structured review output with severity levels and confidence scoring - Add merge readiness assessment with 0-100% confidence metric * fix(workflows): prevent opencode temp files from triggering infinite loops Move pr_diff.txt and previous_reviews.txt to /tmp/ so they won't be committed back to the PR, which was causing re-trigger on synchronize * fix(workflows): remove file-based diff fetching, add timeout Remove pr_diff.txt and previous_reviews.txt file creation steps that were causing timeouts due to file access issues in the opencode action context. Let the opencode action fetch PR context internally instead. Add 10 minute timeout to prevent hanging workflows. * refactor(worldgen): split biome.zig into focused sub-modules (#257) * refactor(worldgen): split biome.zig into focused sub-modules (#247) Separate data definitions, selection algorithms, edge detection, color lookup, and BiomeSource interface into dedicated files to address SRP, OCP, and ISP violations. biome.zig is now a 104-line re-export facade preserving all existing import paths — zero changes to consuming files. * Approve: Clean modular refactor, merge ready. Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com> * MERGE: Clean biome refactor, SOLID 9.0 Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com> * update * fix(worldgen): address PR review feedback on biome refactor - Document intentional biome exclusions in selectBiomeSimple() (LOD2+) - Replace O(n) linear search in getBiomeDefinition with comptime lookup table - Document intentional self parameter retention in BiomeSource.selectBiome() --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com> * ci(workflows): enhance opencode review to check linked issues Update prompt to instruct opencode to: - Check PR description for linked issues (Fixes #123, Closes #456, etc.) - Verify the PR actually implements what the issue requested - Mention linked issues in the summary section - Confirm whether implementation fully satisfies issue requirements * refactor(world): extract chunk_mesh.zig meshing stages into independent modules (#258) * refactor(world): extract chunk_mesh.zig meshing stages into independent modules (#248) Decompose chunk_mesh.zig (705 lines) into focused modules under meshing/ to address SRP, OCP, and DIP violations. Each meshing stage is now independently testable with mock data. New modules: - meshing/boundary.zig: cross-chunk block/light/biome lookups - meshing/greedy_mesher.zig: mask building, rectangle expansion, quads - meshing/ao_calculator.zig: per-vertex ambient occlusion sampling - meshing/lighting_sampler.zig: sky/block light extraction - meshing/biome_color_sampler.zig: 3x3 biome color averaging chunk_mesh.zig is now a thin orchestrator (270 lines) that delegates to stage modules. Public API is preserved via re-exports so all external consumers require zero changes. Adds 13 unit tests for extracted modules. Closes #248 * fix(meshing): address PR review feedback — diagonal biome corners, named constants, defensive checks - boundary.zig: handle diagonal corner cases in getBiomeAt where both X and Z are out of bounds simultaneously, preventing biome color seams at chunk corners - greedy_mesher.zig: extract light/color merge tolerances to named constants (MAX_LIGHT_DIFF_FOR_MERGE, MAX_COLOR_DIFF_FOR_MERGE) with doc comments explaining the rationale - biome_color_sampler.zig: add debug assertion guarding against division by zero in the 3x3 averaging loop - ao_calculator.zig: add explicit else => unreachable to the axis conditional chain in calculateQuadAO for compile-time verification * fix(shadows): resolve intermittent massive shadow artifact below player (#243) (#259) Fix cascade caching race condition by using pointer-based shared storage so all 4 shadow passes use identical matrices within a frame. Add float32 precision guard for texel snapping at large world coordinates. Add shader-side NaN guard for cascade split selection. Add 8 unit tests for shadow cascade validation, determinism, and edge cases. * feat(lighting): Complete Phases A & C of Modern Lighting Overhaul (#261) * feat(lighting): Complete Phases A & C of Modern Lighting Overhaul (#143) This commit implements colored block lights and post-processing effects, completing phases A and C of issue #143. Phase B (LPV) is tracked in #260. ## Phase A: Colored Block Light System - Add torch block (ID: 45) with warm orange emission (RGB: 15, 11, 6) - Add lava block (ID: 46) with red-orange emission (RGB: 15, 8, 3) - Extend existing RGB-packed light propagation system - Update block registry with textures, transparency, and render pass settings - Torch and lava now emit colored light visible in-game ## Phase C: Post-Processing Stack - Implement vignette effect with configurable intensity - Implement film grain effect with time-based animation - Add settings: vignette_enabled, vignette_intensity, film_grain_enabled, film_grain_intensity - Full UI integration in Graphics Settings screen - Real-time parameter updates via RHI interface ## Technical Changes - Modified: src/world/block.zig (new block types) - Modified: src/world/block_registry.zig (light emission, properties) - Modified: src/engine/graphics/resource_pack.zig (texture mappings) - Modified: assets/shaders/vulkan/post_process.frag (vignette, grain) - Modified: src/engine/graphics/vulkan/post_process_system.zig (push constants) - Modified: src/game/settings/data.zig (new settings) - Modified: src/game/screens/graphics.zig (UI handlers) - Modified: src/engine/graphics/rhi.zig (new RHI methods) - Modified: src/engine/graphics/rhi_vulkan.zig (implementations) - Modified: src/engine/graphics/vulkan/rhi_context_types.zig (state storage) - Modified: src/engine/graphics/vulkan/rhi_pass_orchestration.zig (push constants) Closes #143 (partially - Phase B tracked in #260) * fix(settings): disable vignette and film grain by default Users expect a clean, modern look by default. These effects are now opt-in via the Graphics Settings menu. Fixes review feedback on PR #261 * fix(blocks): add missing textures and fix lava properties Critical fixes for PR #261: ## Missing Textures (CRITICAL) - Add assets/textures/default/torch.png (orange placeholder) - Add assets/textures/default/lava.png (red placeholder) - Prevents runtime texture loading failures ## Lava Block Properties (HIGH) Fix lava to behave as a proper fluid/light source: - is_solid: false (was true) - allows light propagation and player movement - is_transparent: true - allows light to pass through - is_fluid: true - enables fluid physics and rendering - render_pass: .fluid - proper fluid rendering pipeline These changes ensure lava: - Emits colored light correctly (RGB: 15, 8, 3) - Propagates light to surrounding blocks - Renders with proper fluid transparency - Behaves consistently with water fluid mechanics Fixes review feedback on PR #261 * ci(workflows): add previous review tracking to opencode PR reviews Add step to fetch previous opencode-agent reviews using gh API Include previous reviews in prompt context Update prompt instructions to verify previous issues are fixed Add explicit instructions to acknowledge fixes with ✅ markers Only report unresolved issues, not already-fixed ones * fix(workflows): fix heredoc delimiter syntax in opencode workflow Remove single quotes from heredoc delimiter which GitHub Actions doesn't support Rewrite to build content in variable first, then use printf for proper handling * feat(lighting): implement LPV compute GI and debug tooling (#262) * feat(lighting): Complete Phases A & C of Modern Lighting Overhaul (#143) This commit implements colored block lights and post-processing effects, completing phases A and C of issue #143. Phase B (LPV) is tracked in #260. ## Phase A: Colored Block Light System - Add torch block (ID: 45) with warm orange emission (RGB: 15, 11, 6) - Add lava block (ID: 46) with red-orange emission (RGB: 15, 8, 3) - Extend existing RGB-packed light propagation system - Update block registry with textures, transparency, and render pass settings - Torch and lava now emit colored light visible in-game ## Phase C: Post-Processing Stack - Implement vignette effect with configurable intensity - Implement film grain effect with time-based animation - Add settings: vignette_enabled, vignette_intensity, film_grain_enabled, film_grain_intensity - Full UI integration in Graphics Settings screen - Real-time parameter updates via RHI interface ## Technical Changes - Modified: src/world/block.zig (new block types) - Modified: src/world/block_registry.zig (light emission, properties) - Modified: src/engine/graphics/resource_pack.zig (texture mappings) - Modified: assets/shaders/vulkan/post_process.frag (vignette, grain) - Modified: src/engine/graphics/vulkan/post_process_system.zig (push constants) - Modified: src/game/settings/data.zig (new settings) - Modified: src/game/screens/graphics.zig (UI handlers) - Modified: src/engine/graphics/rhi.zig (new RHI methods) - Modified: src/engine/graphics/rhi_vulkan.zig (implementations) - Modified: src/engine/graphics/vulkan/rhi_context_types.zig (state storage) - Modified: src/engine/graphics/vulkan/rhi_pass_orchestration.zig (push constants) Closes #143 (partially - Phase B tracked in #260) * fix(settings): disable vignette and film grain by default Users expect a clean, modern look by default. These effects are now opt-in via the Graphics Settings menu. Fixes review feedback on PR #261 * fix(blocks): add missing textures and fix lava properties Critical fixes for PR #261: ## Missing Textures (CRITICAL) - Add assets/textures/default/torch.png (orange placeholder) - Add assets/textures/default/lava.png (red placeholder) - Prevents runtime texture loading failures ## Lava Block Properties (HIGH) Fix lava to behave as a proper fluid/light source: - is_solid: false (was true) - allows light propagation and player movement - is_transparent: true - allows light to pass through - is_fluid: true - enables fluid physics and rendering - render_pass: .fluid - proper fluid rendering pipeline These changes ensure lava: - Emits colored light correctly (RGB: 15, 8, 3) - Propagates light to surrounding blocks - Renders with proper fluid transparency - Behaves consistently with water fluid mechanics Fixes review feedback on PR #261 * feat(lighting): add LPV compute GI with debug profiling * fix(lighting): harden LPV shader loading and propagation tuning * fix(lighting): address LPV review blockers * fix(lighting): clarify 3D texture config and LPV debug scaling * chore(lighting): polish LPV docs and overlay sizing * docs(lighting): clarify LPV constants and 3D mipmap behavior * docs(vulkan): clarify createTexture3D config handling * perf(lighting): gate LPV debug overlay work behind toggle * feat(lighting): PCSS soft shadows, SH L1 LPV with occlusion, LUT color grading, 3D dummy texture fix (#264) * feat(lighting): add 3D dummy texture for LPV sampler3D bindings, PCSS soft shadows, SH L1 LPV with occlusion, and LUT color grading Implement the full lighting overhaul (phases 4-6): - PCSS: Poisson-disk blocker search with penumbra-based variable-radius PCF - LPV: native 3D textures, SH L1 directional encoding (3 channels), occlusion-aware propagation - Post-process: LUT-based color grading with 32^3 neutral identity texture - Fix Vulkan validation error on bindings 11-13 by creating a 1x1x1 3D dummy texture so sampler3D descriptors have a correctly-typed fallback before LPV initialization Resolves #143 * fix(lighting): add host-compute barrier for SSBO uploads, match dummy 3D texture format, clamp LPV indirect - Add VK_PIPELINE_STAGE_HOST_BIT -> COMPUTE_SHADER barrier before LPV dispatch to guarantee light buffer and occlusion grid visibility - Change 3D dummy texture from .rgba (RGBA8) to .rgba32f with zero data to match LPV texture format and avoid spurious SH contribution - Clamp sampleLPVAtlas output to [0, 2] to prevent overexposure from accumulated SH values * fix(lpv,lod): harden failure paths and runtime guards Make LPV grid reconfiguration transactional with rollback, surface occlusion allocation failures, and clean up partial LOD manager init on allocation errors. Add runtime LOD bridge context checks, support larger shader module loads, and return UnsupportedFace instead of unreachable in greedy meshing. * chore(docs): remove obsolete PR2 investigation notes * Issue #229: staged migration from full-scene MSAA to TAA (#266) * settings: add initial TAA config scaffold * graphics: add Halton jitter plumbing for temporal AA * graphics: scaffold TAA resources and render-graph stage * graphics: implement TAA resolve and history feedback * graphics: force single-sample main pass path * settings: migrate AA UX toward TAA controls * world/graphics: split cutout geometry into dedicated mesh pass * settings: expose configurable TAA blend and rejection * build/settings: validate TAA shaders and tune preset defaults * fix(ci): add fallback Nix installer in build workflow * fix(taa): harden history allocation and shader wiring * fix(ci): make Nix cache restore non-blocking * fix(taa): tighten allocation and framebuffer error cleanup * fix(taa): unify history texture error cleanup path --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: MichaelFisher1997 <MichaelFisher1997@users.noreply.github.com>

github-actions bot added documentation Improvements or additions to documentation engine game world shaders labels Jan 29, 2026

MichaelFisher1997 changed the base branch from main to dev January 29, 2026 06:47

Merge branch 'dev' into refactor/rhi_vulkan

e6ae7d6

github-actions bot removed game world shaders labels Jan 29, 2026

MichaelFisher1997 marked this pull request as draft January 29, 2026 07:03

MichaelFisher1997 added 2 commits January 29, 2026 07:04

refactor(vulkan): apply shader loading helpers to sky pipeline

6480a13

Apply loadShaderPair helper to reduce code duplication in sky pipeline creation. Part of #244

github-actions bot added the shaders label Jan 29, 2026

MichaelFisher1997 added 4 commits January 29, 2026 07:12

refactor(vulkan): apply shader helpers to terrain and UI pipelines

a3c75c0

Apply loadShaderModule and loadShaderPair helpers to reduce code duplication: - G-Pass fragment shader loading - UI pipelines (colored and textured) Part of #244

MichaelFisher1997 marked this pull request as ready for review January 29, 2026 07:24

MichaelFisher1997 merged commit d232511 into dev Jan 29, 2026
6 checks passed

MichaelFisher1997 mentioned this pull request Feb 9, 2026

Merge dev into main after syncing main #267

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(vulkan): PR1 - Add PipelineManager and RenderPassManager modules#251

refactor(vulkan): PR1 - Add PipelineManager and RenderPassManager modules#251
MichaelFisher1997 merged 10 commits intodevfrom
refactor/rhi_vulkan

MichaelFisher1997 commented Jan 29, 2026

Uh oh!

opencode-agent bot commented Jan 29, 2026

Uh oh!

opencode-agent bot commented Jan 29, 2026

Uh oh!

opencode-agent bot commented Jan 29, 2026

Uh oh!

opencode-agent bot commented Jan 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

MichaelFisher1997 commented Jan 29, 2026

Summary

Changes

New Modules

Documentation

Impact

Next Steps (Future Commits)

Testing

Related

Uh oh!

opencode-agent bot commented Jan 29, 2026

Code Review: PipelineManager & RenderPassManager PR

Code Quality Issues & Potential Bugs

SOLID Principle Scoring

Actionable Suggestions

Uh oh!

opencode-agent bot commented Jan 29, 2026

Code Review: PipelineManager & RenderPassManager PR

Previous Review Feedback Status

New Code Quality Issues & Potential Bugs

SOLID Principle Scoring

Actionable Suggestions

Uh oh!

opencode-agent bot commented Jan 29, 2026

Code Review: PipelineManager & RenderPassManager PR

Previous Review Feedback Status

New Issues Identified Since Last Review

SOLID Principle Scoring

Actionable Suggestions

Summary

Uh oh!

opencode-agent bot commented Jan 29, 2026

Code Review: PipelineManager & RenderPassManager PR

Previous Review Feedback Status

New Issues Identified

SOLID Principle Scoring

Actionable Suggestions

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant