Shadow hygiene: one shared limits authority, and recording gated by e…

CLAUDE.md

@@ -129,6 +129,8 @@ assets/               glTF samples + HDR skyboxes
 - **render/constants.hpp** — single source of truth for scalar render tunables (shadow biases, IBL strengths, shadow/IBL extents, camera FOV, bloom config). GPU data-layout limits (frames-in-flight, joint/morph/light counts, shadow caster caps + matrix layout) live in **graphics/gpu_limits.hpp** so the Vulkan-free graphics layer can size its arrays; constants.hpp includes it, so all constants stay reachable through one include.
 - **Layering guard** — `graphics/` and `scene/` headers must not include `render/`, and `render/` headers must not include `scene/` (`render`↔`scene` meet only through the Vulkan-free `graphics/renderable_scene.hpp` seam). Enforced by the `layering_guards` CTest case (`cmake/check_layering.cmake`). Graphics `.cpp` files may include `render/resources.hpp` — that's the documented GPU-allocation bridge.
 - **GPU resource model** — three orthogonal layers; never conflate them. **(1) Identity:** opaque handles (`graphics/gpu_handle.hpp`) into `Resources`' tables — an index packed with a generation so a stale handle to a reused slot is detectably invalid. **(2) Lifetime:** off-the-shelf `vk::raii` owns every Vulkan object whose lifetime is *independent of a device-memory allocation* (image views, samplers, pipelines, descriptor layouts, command pools). `vk::raii` is a lifetime tool, not a memory allocator — it imposes nothing on allocation strategy. **(3) Memory / sub-allocation:** VMA is the arena, used the idiomatic way (`vmaCreateBuffer`/`vmaCreateImage`) so it owns the resource + its sub-allocation and frees them together. Because `vmaDestroy*` couples the two, buffers and images are the *one* exception to layer 2: their `VkBuffer`/`VkImage` lifetime lives inside a small custom move-only VMA RAII wrapper (`UniqueVmaBuffer`/`UniqueVmaImage`) — still RAII, just VMA-backed. This is **not** a migration away from `vk::raii`; revisit layer 2 only if runtime streaming (mid-frame create/destroy needing a deletion queue) ever arrives.
+- **One declaration of every shared GPU data-layout limit** — the sizes and indices that a C++ block and a shader block must agree on (caster/light/joint/morph/emitter/kernel counts, the shadow matrix bases, the map-validity bits). Purely GLSL-side algorithm constants are NOT in scope and this mechanism does not own them: a compute workgroup size, a scan radix, a tap count with no C++ counterpart stays where it is used. `shaders/gpu_limits.glsl` is written in the subset that is valid GLSL *and* valid C++; shaders `#include` it and `graphics/gpu_limits.hpp` includes it inside a `shader_limits` namespace, re-exporting each value under its `k`-name. Add a shader-visible limit **there**, never as a literal on either side, and keep the file inside the common subset (no `constexpr`, `inline`, `namespace`, `static_cast`, unsigned suffixes — each breaks the *other* language, in files that never mention this one). The `gpu_limits_guard` CTest case sweeps `shaders/` for a re-declaration, requires each consumer to use the name rather than a literal, and requires each `k`-constant to be defined *as* the shared declaration, so C++ cannot drift back to hard-coded values behind green shader checks.
+- **A shadow family's recording and its uploaded validity are one value** — `ShadowMapValidity` (`graphics/shadow_map_validity.hpp`) is derived once per frame in `Renderer::uploadFrameLighting`, from the COMPLETED view set, and used twice: it gates the families in `Shadows::recordPass` and it is uploaded as `LightUBO::shadowMapValidMask` for every sampling path in `shader.frag`. Never skip a family's recording without routing the decision through it — a skipped family's depth image holds an earlier frame's content, and sampling it produces no error, no crash, and shadows from a frame that is gone.
 - **GPU data-layout discipline** — every CPU struct shared with a shader (UBO/SSBO) lives in `render/ubo.hpp` with `alignas` + `static_assert`s pinning its std140/std430 offsets and size. Preserve this: when you change a shader-visible struct, update both sides and keep the static_asserts — they are the only thing catching a silent host↔GPU layout mismatch. Mapped host-visible writes go through `graphics/mapped_buffer.hpp` `writeMapped` (a bounds-checked `std::span<std::byte>`), never a raw `void*`. **And a block bound by more than one shader is declared ONCE, in a shared `shaders/*.glsl` include** (`light_ubo.glsl` for `LightUBO`, `material.glsl` for the bindless `Materials` SSBO + `MaterialData`, `shadow_push.glsl` for the `ShadowPushConstants` push block), never hand-copied per shader: field offsets depend on every field before them, so a copy missing an inserted field misreads everything after it, with no validation error and no crash. That is how the sky came to be multiplied by a shadow matrix — `selfShadowViewProj` was added to the struct and `shader.frag`, not to `skybox.frag`, and the wrong value read 1.0 until a scene had two skinned self-shadow casters. The `shader_block_guards` CTest case (`cmake/check_shader_blocks.cmake`) fails on a re-declared block *and* on a shared include that stops declaring it.
 
 ## Code Style
@@ -167,8 +169,9 @@ Reference class: `include/fire_engine/graphics/image.hpp`.
 
 Catch2 (v3, `Catch2::Catch2WithMain`). Single binary `test_fire_engine`. CTest registers
 `test_fire_engine` as the fast `~[slow]` entry, so plain `ctest` and `ctest --preset fast`
-stay fast. The `tests-full` target runs the all-tags Catch2 binary plus the graphics-layer
-include guard and the shared-shader-block guard; from the source root use `cmake --build --preset full`. Test files mirror
+stay fast. The `tests-full` target runs the all-tags Catch2 binary plus the four build-time guards —
+graphics-layer includes, shared shader blocks, the shadow bias law, and the shared GPU limits; from
+the source root use `cmake --build --preset full`. Test files mirror
 source paths. Shared helpers/traits live in `tests/support/`. Test assets in `tests/assets/`
 → copied to `build/test_assets/`.
 

CMakeLists.txt

@@ -104,6 +104,7 @@ add_library(fireengine SHARED
     src/graphics/shadow_lod_resolver.cpp
     src/graphics/shadow_caster_alpha.cpp
     src/graphics/shadow_bias.cpp
+    src/graphics/shadow_map_validity.cpp
     src/graphics/shadow_caster_deformation.cpp
     src/graphics/shadow_render_view.cpp
     src/graphics/shadow_diagnostics.cpp
@@ -178,7 +179,14 @@ add_library(fireengine SHARED
     src/scene/transform.cpp
 )
 
-target_include_directories(fireengine PUBLIC ${PROJECT_SOURCE_DIR}/include ${Stb_INCLUDE_DIR})
+# `shaders/` is on the C++ include path because a PUBLIC header needs it: graphics/gpu_limits.hpp
+# includes shaders/gpu_limits.glsl, the one file where the limits shared with GLSL are written.
+# BUILD_INTERFACE-only — nothing is installed, and a consumer of an installed tree would have no
+# shaders/ to point at.
+target_include_directories(fireengine PUBLIC
+    ${PROJECT_SOURCE_DIR}/include
+    $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/shaders>
+    ${Stb_INCLUDE_DIR})
 target_compile_features(fireengine PUBLIC cxx_std_23)
 target_compile_options(fireengine PRIVATE -Wall -Wextra -Wpedantic)
 if(FIRE_ENGINE_WARNINGS_AS_ERRORS)
@@ -291,6 +299,9 @@ endif()
 # any of them rebuilds every shader — includes are rare + shared, so this over-approximation is cheap
 # and keeps the SPV outputs correct.
 set(SHADER_INCLUDES
+    # Read by BOTH languages: the shaders include it directly, and the public C++ header
+    # graphics/gpu_limits.hpp includes it to re-export the same values under the engine's k-names.
+    ${PROJECT_SOURCE_DIR}/shaders/gpu_limits.glsl
     ${PROJECT_SOURCE_DIR}/shaders/vdpm_repair_classify.glsl
     ${PROJECT_SOURCE_DIR}/shaders/vdpm_apply_classify.glsl
     ${PROJECT_SOURCE_DIR}/shaders/light_ubo.glsl
@@ -435,6 +446,7 @@ add_executable(test_fire_engine
     tests/graphics/test_shadow_caster_deformation.cpp
     tests/graphics/test_shadow_caster_alpha.cpp
     tests/graphics/test_shadow_bias.cpp
+    tests/graphics/test_shadow_map_validity.cpp
     tests/graphics/test_shadow_render_view.cpp
     tests/graphics/test_texture.cpp
     tests/graphics/test_sampler_settings.cpp
@@ -509,6 +521,7 @@ add_custom_target(tests-full
   COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R layering_guards
   COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R shader_block_guards
     COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R shadow_bias_guard
+  COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R gpu_limits_guard
   DEPENDS test_fire_engine
   COMMENT "Running the full test suite, including [slow] tests"
 )
@@ -543,6 +556,17 @@ add_test(
     -P ${PROJECT_SOURCE_DIR}/cmake/check_shadow_bias.cmake
 )
 
+# The limits shared by C++ and GLSL are declared once and read by both sides. A one-sided edit
+# compiles cleanly and then indexes the wrong region of a bound buffer — in range, so no validation
+# error — which is why this is a build-time check rather than a runtime one.
+add_test(
+  NAME gpu_limits_guard
+  COMMAND ${CMAKE_COMMAND}
+    -DSHADER_DIR=${PROJECT_SOURCE_DIR}/shaders
+    -DINCLUDE_DIR=${PROJECT_SOURCE_DIR}/include
+    -P ${PROJECT_SOURCE_DIR}/cmake/check_gpu_limits.cmake
+)
+
 find_program(CLANG_TIDY_EXE NAMES clang-tidy)
 if(CLANG_TIDY_EXE)
     get_target_property(FIRE_ENGINE_TIDY_SOURCES fireengine SOURCES)

README.md

@@ -189,7 +189,7 @@ The transient pipelines are destroyed once the bake completes; only the resultin
 4. `PhysicsWorld::step(1.0f / 60.0f)` runs zero or more fixed substeps from the frame accumulator.
 5. `scene_.applyPhysics(physics_, alpha)` pulls dynamic and corrected kinematic transforms back into scene nodes — interpolated by `alpha = accumulator / fixedDt` between the last two simulated poses for smooth motion above 60 Hz — and resolves composed-world matrices.
 6. `Renderer::drawFrame()` acquires a swapchain image and records the frame passes:
-   - **Shadow passes** — directional cascades render both the full CSM and a world-only CSM that excludes skinned casters. Each skinned self-shadow slot renders two tightly-fit passes: the first captures the nearest light-facing surface, and the second samples that first depth and discards it so the forward shader can sample the next useful self-occluder. Spot and point shadow passes replay the same compatible shadow draw commands through their per-layer/per-face depth attachment views. Skin and morph still apply in the shadow vertex shader
+   - **Shadow passes** — directional cascades render both the full CSM and a world-only CSM that excludes skinned casters. Each skinned self-shadow slot renders two tightly-fit passes: the first captures the nearest light-facing surface, and the second samples that first depth and discards it so the forward shader can sample the next useful self-occluder. Spot and point shadow passes replay the same compatible shadow draw commands through their per-layer/per-face depth attachment views. Skin and morph still apply in the shadow vertex shader. **Which families record is one decision per frame** (`ShadowMapValidity`): the slot-addressed families disappear when nothing assigns them — no self-shadow caster, no spot or point caster, no skinned draw for the world-only CSM — while the main cascades still record (and clear) whenever there is a sun to fit them to, casters or not. The directional families additionally require a primary directional light, and `--no-shadows` clears everything. A skipped family costs nothing at all — no draws, no clears, no timestamps — and the *same* value is uploaded as a bit mask the forward shader reads, so its sampling path answers fully lit instead of reading depth left over from an earlier frame
    - **Forward pass** — begin the HDR offscreen pass, draw the skybox (LEQUAL depth, no write), then call `scene.buildDrawCommands(frameInfo, frustums, out)` through the Vulkan-free `RenderableScene` interface (the scene culls internally and emits draws); Mesh/Object emit `DrawCommand`s that the Renderer buckets into opaque, transmissive, and blend lists, sorts the blend bucket back-to-front by `sortDepth`, and replays through the same bind/draw loop resolving handles via `Resources`
    - **Transmission pass** — when transmissive draws are present, capture the opaque scene colour mip chain and replay transmissive draws so the shader can sample scene-behind-glass data
    - **TAA resolve** — the forward/transmission passes also write a screen-space velocity (motion-vector) attachment; the resolve reprojects the previous frame's accumulated history along that buffer, neighbourhood-clamps it against the current 3×3 to kill ghosting, blends, and blits the result back into the HDR target. Sub-pixel projection jitter (Halton(2,3)) drives the accumulation; particles render afterwards with the un-jittered projection so they stay out of history. Skipped under `--no-taa`
@@ -396,6 +396,11 @@ FE_LOG=render:debug ./fireEngineApp
 
 Current categories are `app`, `general`, `gltf`, `physics`, `ragdoll`, and `render`.
 
+`render:debug` also prints a periodic **shadow recording** line — per family, whether it was recorded
+or skipped, its raster passes and its GPU milliseconds — which is how `--no-shadows` is checked: it
+suppresses the *recording*, not only the sampling, so every family must read `skipped passes=0
+0.000ms`. A frame that still rendered into maps nobody samples would look identical on screen.
+
 ## Dependencies
 
 Managed via the vcpkg manifest (`vcpkg.json`):