diff --git a/CMakeLists.txt b/CMakeLists.txt index 5ec32e3..d89e018 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -295,6 +295,7 @@ set(SHADER_INCLUDES ${PROJECT_SOURCE_DIR}/shaders/light_ubo.glsl ${PROJECT_SOURCE_DIR}/shaders/material.glsl ${PROJECT_SOURCE_DIR}/shaders/shadow_push.glsl + ${PROJECT_SOURCE_DIR}/shaders/forward_push.glsl ${PROJECT_SOURCE_DIR}/shaders/shadow_depth.glsl ${PROJECT_SOURCE_DIR}/shaders/self_shadow_second.glsl ) diff --git a/README.md b/README.md index 006a1f2..8b5cacc 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ I've no doubt these are all solved problems nowadays with the Unreal engine et a - **Authored-camera adoption** — `GltfLoader::findFirstCamera` walks the default scene's node tree DFS and returns the first camera-bearing node's view; FireEngine reframes its runtime camera to match before the first frame - **GPU particle system (compute-driven)** — `ParticleEmitter` is a scene component, gathered each frame like `Light` into a Vulkan-free `EmitterState`. A renderer-owned `ParticleSystem` simulates a pooled particle SSBO with a **compute shader** (spawn dead slots at the emitter up to a per-frame budget via an atomic spawn-claim; integrate the rest under gravity), then renders the pool as **instanced camera-facing billboards** (`cmd.draw(6, poolCount)`, per-instance data read from the SSBO by `gl_InstanceIndex`) blended **additively into the HDR target** so bloom catches the glow. **Soft particles**: the fragment shader fades against sampled scene depth so particles dissolve smoothly into geometry with no hard clip edge. Built on the compute-pipeline + synchronization2 buffer-barrier path - **HDR offscreen forward pass + bloom + ACES post-process** — forward writes into an R16G16B16A16 target. **Dual-filter bloom** (6-mip RGBA16F chain at half-screen res, 13-tap CoD downsample with Karis-average on the first pass to suppress fireflies, 9-tap tent upsample with additive blend) produces a low-pass HDR contribution. Post-process mixes the HDR target with bloom mip 0 (`bloomStrength = 0.04` default; `0` is bit-identical to a no-bloom path), then ACES tonemap + gamma 2.2 before presenting -- **SSAO + contact shadows** — a **depth prepass** (reusing the forward vertex shader with `invariant gl_Position`; the forward pass loads it with `LESS_OR_EQUAL`) fills the shared depth buffer before lighting. A renderer-owned `Ssao` subsystem then reconstructs view-space position + normal from depth alone (no normal G-buffer — analytic unprojection from the projection matrix) and writes an **R8G8** target: R = hemisphere-kernel ambient occlusion, G = a sun-direction screen-space **contact-shadow** ray-march. The forward shader samples it to multiply SSAO into the IBL/ambient terms and the contact term into the **direct sun** (ambient stays on pure CSM). A **depth-aware bilateral blur** (5×5, view-space-Z edge-stop) smooths the per-pixel sampling/march noise without bleeding across silhouettes, with TAA carrying the temporal denoise. SSAO and contact shadows are on by default; contact shadows fill the CSM's short-range contact gap and use an N·L gate plus view-Z-scaled depth window and silhouette edge guard to avoid screen-space streaks. Live overlay sliders (radius / bias / intensity / power, contact length) and a `--debug-ssao` view +- **SSAO + contact shadows** — a **depth prepass** (reusing the forward vertex shader with `invariant gl_Position`; the forward pass loads it with `LESS_OR_EQUAL`) fills the shared depth buffer before lighting. It applies the material's **alpha cutout** through the same shared test the forward and shadow passes use, so a MASK material writes depth only where it is actually opaque — otherwise it occludes across its own holes, hiding geometry behind a leaf card and making the AO below treat it as a solid sheet. A renderer-owned `Ssao` subsystem then reconstructs view-space position + normal from depth alone (no normal G-buffer — analytic unprojection from the projection matrix) and writes an **R8G8** target: R = hemisphere-kernel ambient occlusion, G = a sun-direction screen-space **contact-shadow** ray-march. The forward shader samples it to multiply SSAO into the IBL/ambient terms and the contact term into the **direct sun** (ambient stays on pure CSM). A **depth-aware bilateral blur** (5×5, view-space-Z edge-stop) smooths the per-pixel sampling/march noise without bleeding across silhouettes, with TAA carrying the temporal denoise. SSAO and contact shadows are on by default; contact shadows fill the CSM's short-range contact gap and use an N·L gate plus view-Z-scaled depth window and silhouette edge guard to avoid screen-space streaks. Live overlay sliders (radius / bias / intensity / power, contact length) and a `--debug-ssao` view - **GPU soft-body / cloth (XPBD)** — `-c` drops a cloth that simulates entirely on the GPU, or author one on any glTF mesh with `extras.Cloth` (samples: `assets/ClothSheet/ClothSheet.gltf`, `assets/ClothBanner/ClothBanner.gltf`). A renderer-owned `SoftBodySystem` runs an XPBD compute solver each substep: `cloth_predict` integrates gravity + wind, `cloth_solve` projects distance constraints **graph-coloured** into race-free batches (Gauss-Seidel by colour), `cloth_collide` pushes particles out of world colliders, and `cloth_finalize` writes solved positions + normals (recomputed from a per-vertex→triangle **CSR adjacency**, so arbitrary meshes work, not just grids) into a storage **vertex buffer** the forward/shadow passes read — so the cloth renders, lights, and casts shadows through the normal forward path (double-sided), no new render shaders. The solver is **descriptor-free**: every buffer reaches the shaders as a `bufferDeviceAddress` pointer. Collision primitives (plane / sphere / box / capsule) are gathered each frame from `PhysicsWorld` (`gatherColliders`) plus a ground plane. Constraint stiffness is authored per type (structural/shear stiff, bend soft); substeps, a global **compliance multiplier**, damping, gravity, and wind are **live overlay sliders**. Built on the same compute + buffer-barrier path as particles - **Temporal anti-aliasing (TAA)** — sub-pixel Halton(2,3) projection jitter plus velocity-buffer history accumulation anti-aliases geometry edges *and* specular/shading shimmer (unlike MSAA, which only covers geometry edges). The forward + transmission passes write a screen-space motion-vector attachment; the resolve reprojects the previous frame's history along it (`historyUV = uv − velocity`), neighbourhood-clamps to the current 3×3 to suppress ghosting/disocclusion, and blends. Motion vectors are jitter-free so the jitter cancels in accumulation. Per-node previous-world-matrix tracking feeds rigid + animated motion (skinned deformation is camera-motion-only in v1); particles render after the resolve, kept out of history. `--no-taa` reverts to the raw image, `--debug-velocity` visualises the buffer - **Frustum culling (camera + shadow casters)** — built on a reusable fat-AABB BVH (`AabbBvh`, the same core the physics broadphase and static-mesh triangle index use). Two stages: a **persistent scene BVH** (`SceneCuller`, an `AabbBvh` over rigid renderables) pre-culls each frame against the union of the camera frustum and every shadow caster's frustum, so off-screen nodes skip draw-building entirely (no UBO writes, no per-vertex bounds) — `O(log N + visible)` instead of `O(N)`; then a **precise per-pass cull** drops the survivors that fall outside a given pass's frustum (`buildDrawBuckets` for the camera, per-cascade/spot/point-face in the shadow pass). Frustums are 6 Gribb–Hartmann planes (Vulkan `[0,1]` depth) with a conservative positive-vertex AABB test (no false negatives). Deformable (skinned/morph) meshes skip the coarse BVH (bind-pose bounds under-cover the animated pose) and rely on the precise stage's exact per-frame world bounds. The coarse union is a strict superset of what any pass keeps, so culling never drops a visible draw. Live overlay toggle + tracked/visible/culled counts; off submits everything (A/B + regression escape hatch) diff --git a/cmake/check_shader_blocks.cmake b/cmake/check_shader_blocks.cmake index 902cfea..b9941e0 100644 --- a/cmake/check_shader_blocks.cmake +++ b/cmake/check_shader_blocks.cmake @@ -18,16 +18,19 @@ endif() # | # -# SH-05 added the last three. `ShadowPushConstants` had been hand-copied into three shadow stages -# (and would have been four the moment the masked path arrived), and push constants are a raw byte -# range with no reflection at all — strictly worse than a UBO, which at least has a declared size. -# `Materials` and `MaterialData` became shared the moment a shadow fragment path had to apply the -# VISIBLE material's alpha cutout: a second copy of that struct is a second cutoff, a second UV-set -# choice and a second transform, and a shadow disagreeing with its own surface reads as a bias bug. +# SH-05 added `Materials` and `ShadowPushConstants`; the cutout-aware depth prepass added +# `ForwardPushConstants`. The two push blocks are the worst case of the kinds guarded here: a raw byte +# range with no reflection at all — strictly worse than a UBO, which at least has a declared size — +# and `ShadowPushConstants` had already been hand-copied into three shadow stages before a fourth +# needed it. `Materials` and `MaterialData` became shared the moment a depth-only pass had to apply +# the VISIBLE material's alpha cutout: a second copy of that struct is a second cutoff, a second +# UV-set choice and a second transform, and a pass disagreeing with its own surface reads as a bias +# bug. set(guarded_blocks "LightUBO|light_ubo.glsl" "Materials|material.glsl" "ShadowPushConstants|shadow_push.glsl" + "ForwardPushConstants|forward_push.glsl" ) set(offenders "") diff --git a/docs/acceptance-testing.md b/docs/acceptance-testing.md index 1cdc6ab..e293b93 100644 --- a/docs/acceptance-testing.md +++ b/docs/acceptance-testing.md @@ -421,7 +421,11 @@ this order: - The cutout must cast a **perforated** shadow on the floor around x [−4.1, −0.3], z [−10.8, −6.9] — the checker pattern of its own base-colour alpha, matching the perforation you see on the surface. A solid rectangle there means the masked shadow path has stopped applying - (`shadow_masked.frag`, or the caster's `ShadowCasterAlpha` classification). It is authored + (`shadow_masked.frag`, or the caster's `ShadowCasterAlpha` classification). + - You must also **see through** the perforation: its holes show the white floor low down and the + dark sky higher up. Holes that read uniformly dark mean the DEPTH PREPASS has stopped applying + the cutout (`depth_prepass.frag`), so the forward pass is depth-rejecting everything behind + them — a separate fix from the shadow one, and easy to mistake for a material bug. It is authored double-sided, and now for a milder reason than before: a single-sided caster no longer has to put the sun on its back face to cast at all, but keeping it double-sided is what exercises the masked *and* two-sided mode together. diff --git a/docs/codereview.md b/docs/codereview.md index 5318999..431c63b 100644 --- a/docs/codereview.md +++ b/docs/codereview.md @@ -659,10 +659,29 @@ with the chosen no-alias contract rather than retaining that behaviour as an acc --- -## Out-of-tier finding — the depth prepass ignores the alpha cutout (2026-08-03) +## Out-of-tier finding — the depth prepass ignores the alpha cutout (2026-08-03) — ✅ CLEARED Found while landing **SH-05** (material-aware shadow casters), in the same class of defect and in a -different pass, so it is recorded here rather than folded into that item. +different pass, so it was recorded here rather than folded into that item. **Fixed on branch +`depth-prepass-alpha-cutout`** (2026-08-04): the prepass pipeline opts into the bindless set, carries +`ForwardPushConstants`, and its fragment stage applies the same `material.glsl` cutout the forward and +shadow passes use — gated on the material's own `alphaCutoff`, which is exactly equivalent (a +non-MASK material publishes 0, and at 0 the test can never discard) and costs an opaque draw one +scalar SSBO read instead of a texture fetch. No CPU-side classification was added: the prepass needs +none, unlike the shadow path, whose LOD pin is a CPU decision. + +Both decisions the finding flagged were settled rather than assumed. **BLEND** stays out of the +prepass by virtue of its bucket, and would be inert anyway (cutoff 0). **Depth equality** is +preserved: a fragment discard does not perturb `gl_Position`, and the two passes evaluate one +implementation on the same UVs from the same vertex path, so neither can keep what the other drops. + +Measured effect, for the record: the ShadowLodDemo capture's cutout holes now show the floor and sky +behind them instead of reading uniformly dark, and the SH-03 sweep's measured shadowed area moved +12.07% -> 11.68% — the change localised (by differencing the two runs' area masks) to the cutout +quad's own footprint, with the absolute differing-pixel counts unchanged at every printed digit. See +`render/constants.hpp`. + +The finding as originally written follows. ### High: `depth_prepass.frag` writes depth through a MASK material's holes diff --git a/docs/images/shadow-lod-full-detail.png b/docs/images/shadow-lod-full-detail.png index 5961e9d..5bed765 100644 Binary files a/docs/images/shadow-lod-full-detail.png and b/docs/images/shadow-lod-full-detail.png differ diff --git a/docs/images/shadow-lod-selected.png b/docs/images/shadow-lod-selected.png index 1754a69..caf8727 100644 Binary files a/docs/images/shadow-lod-selected.png and b/docs/images/shadow-lod-selected.png differ diff --git a/docs/onboarding.md b/docs/onboarding.md index a32ae4d..87d276a 100644 --- a/docs/onboarding.md +++ b/docs/onboarding.md @@ -929,10 +929,29 @@ the same change — most have a test or guard that will catch you, but not all. is visible in the panel, while the optimistic one silently restores a cutout casting its quad. BLEND classifies as `Opaque` deliberately — its shadow semantics are an open design decision, and the material authority publishes `alphaCutoff` 0 for it anyway. -- **The alpha-cutout test has ONE implementation, and the shadow pass uses the forward one** - (SH-05). `materialAlphaCutoutFails` / `materialBaseColourTexel` / `materialSlotUv` live in - [`shaders/material.glsl`](../shaders/material.glsl); `shader.frag`, `shadow_masked.frag` and - `self_shadow_second_masked.frag` all call them. A shadow that tested a different cutoff, UV set or +- **The GPU material's alpha and alphaCutoff ranges are enforced at the packing seam**, and one + optimisation depends on it. `toMaterialUBO` (`src/graphics/material_binding.cpp`) clamps the packed + alpha into glTF's [0,1] and the packed cutoff to >= 0, warning when it has to — `Material` is a + plain value type that accepts any float, so the guarantee is made where the value becomes GPU + truth. The two clamps differ in kind, and the difference is worth stating: the CUTOFF clamp is + behaviour-preserving (a negative cutoff already discarded nothing), while the ALPHA clamp + deliberately CHANGES what an invalid value does — a negative alpha used to discard, and clamped to 0 + the fragment is kept. That is glTF-spec normalisation of nonsense input, not preservation of it. The + warning is emitted once per material from `Resources::registerMaterial`, never from the packing path, + which is reachable from `noexcept` variant queries. What breaks without the invariant: + `shader.frag` applies `alpha < alphaCutoff` to EVERY material, since a non-MASK one packs cutoff 0, + so a negative alpha would discard an OPAQUE surface — while `depth_prepass.frag`, which SKIPS that + test when the packed cutoff is 0, would keep the fragment and leave a depth-only occluder. Pinned by + `MaterialBinding.PackingEnforcesTheAlphaRangeInvariant`. If you add a packed material field that a + shader branches on, ask what the branch assumes about its range. +- **EVERY pass that writes depth applies the alpha cutout, and they all use ONE implementation** + (SH-05, extended to the depth prepass). `materialAlphaCutoutFails` / `materialBaseColourTexel` / + `materialSlotUv` live in [`shaders/material.glsl`](../shaders/material.glsl); `shader.frag`, + `shadow_masked.frag`, `self_shadow_second_masked.frag` and `depth_prepass.frag` all call them. + A depth-only pass that skips the test does not merely look wrong where the cutout is — it writes + occlusion across the holes, so the DEPTH PREPASS rejected everything behind a leaf card in the + forward pass and made SSAO treat it as a solid sheet, while the SHADOW pass cast the quad instead + of the leaf. If you add a pass that writes depth, that is the question to ask of it. A shadow that tested a different cutoff, UV set or `KHR_texture_transform` from its own surface would cast a silhouette the surface does not have, and the symptom reads as a shadow-bias artefact rather than as a mask bug. There is deliberately no shadow-only material format: the shadow pass reaches the same bindless `materials[]` entry through @@ -1015,7 +1034,9 @@ the same change — most have a test or guard that will catch you, but not all. ([`shaders/light_ubo.glsl`](../shaders/light_ubo.glsl)), the bindless `Materials` SSBO + `MaterialData` struct ([`shaders/material.glsl`](../shaders/material.glsl), shared by `shader.frag` and the SH-05 masked shadow paths) and the `ShadowPushConstants` push block - ([`shaders/shadow_push.glsl`](../shaders/shadow_push.glsl), shared by four shadow stages). A PUSH + ([`shaders/shadow_push.glsl`](../shaders/shadow_push.glsl), shared by four shadow stages) and + `ForwardPushConstants` ([`shaders/forward_push.glsl`](../shaders/forward_push.glsl), shared by + `shader.frag` and `depth_prepass.frag`). A PUSH block is the worst case of the three: it is a raw byte range with no driver-side reflection at all, so a member added to one copy silently reinterprets every field after it in the others — the C++ side is pinned by `offsetof` static_asserts on `ShadowPushConstants` in diff --git a/docs/review-order.md b/docs/review-order.md index 9908394..dd5007f 100644 --- a/docs/review-order.md +++ b/docs/review-order.md @@ -226,6 +226,8 @@ Read paired with `ubo.hpp`, `descriptor_bindings.hpp`, `gpu_limits.hpp`. | `taa.frag` | TAA resolve: `historyUV = uv − velocity`, 3×3 neighbourhood clamp, `mix(current, history, historyBlend)`; falls back to current when history is invalid or the reprojected UV is off-screen. Fullscreen triangle (`postprocess.vert`). | | `shadow.vert`/`.frag`, `shadow_masked.frag`, `self_shadow_second.frag`, `self_shadow_second_masked.frag` | Skinning+morph still run here so animated geo casts matching shadows; second pass = back-faces only. **SH-05 split each fragment stage into an opaque and a masked path** — the masked ones apply the visible material's alpha cutout from `material.glsl` (bindless set 2, indexed by `ShadowPushConstants::materialIndex`), so a cutout casts its silhouette rather than its quad; the second-depth masked path tests the cutout FIRST, since a masked-out fragment is not a surface and cannot be anybody's second depth. One vertex shader serves all four: it emits both UV sets unconditionally (skinning moves positions, never UVs) rather than duplicating the skin/morph maths in a second stage. | | `shadow_push.glsl`, `shadow_depth.glsl`, `self_shadow_second.glsl` | The shadow stages' shared includes (SH-05). `shadow_push.glsl` is **the single declaration of `ShadowPushConstants`** — guarded, because a push block is a raw byte range with no reflection, and it had been hand-copied into three stages before a fourth arrived. `shadow_depth.glsl` holds the point-face linear distance/range `gl_FragDepth` write, so the opaque and masked paths cannot record different depths for the same face. `self_shadow_second.glsl` holds the dual-depth rejection test, which IS the pass — a drifted copy would show as acne on cutout characters only, looking like a bias problem. Each still hand-copies a shadow limit (`SHADOW_TOTAL_MATRIX_COUNT`, `SHADOW_POINT_MATRIX_BASE`, the self-slot count) from `gpu_limits.hpp`; generating them is the open hygiene item in the roadmap. | +| `depth_prepass.frag` | Depth-only, and **no longer empty**: it applies the material's ALPHA CUTOUT through the shared `material.glsl` test, gated on the material's own `alphaCutoff` so an opaque draw pays one scalar SSBO read and no texture fetch (the gate is exactly equivalent — `toMaterialUBO` publishes a cutoff only for MASK, and at cutoff 0 the test can never discard, in this stage or the forward one). Before that it wrote depth across a cutout's holes, so the forward pass' `LESS_OR_EQUAL` test rejected whatever stood behind them and SSAO — which reconstructs position and normal from this depth alone — treated the cutout as a solid sheet. The prepass and the forward pass MUST discard the same fragments: one keeping what the other drops leaves either a depth-only occluder or a shaded fragment whose depth nobody wrote, which is why both call one implementation on the same UVs from the same vertex path. | +| `forward_push.glsl` | **The single declaration of `ForwardPushConstants`**, included by `shader.frag` and `depth_prepass.frag`. Guarded (`shader_block_guards`), for the reason a push block always is: no driver-side reflection, so a member added to one copy silently reinterprets every field after it — and a shifted `materialIndex` is a wrong bindless material rather than an error. Note the pipeline layout must declare the WHOLE block even for a stage that reads one field of it. | | `material.glsl` | **The single declaration of the bindless material authority** — `MaterialData`, the `Materials` SSBO and the `textures[]` array at `set = 2`, plus `matTex`, `materialSlotUv`, `materialBaseColourTexel`, `materialAlpha` and `materialAlphaCutoutFails`. Guarded (`shader_block_guards`), and shared since SH-05 gave the shadow pass a path that must apply the SAME cutout as `shader.frag`: a second copy of this struct is a second cutoff, a second UV-set choice and a second transform. An including shader must declare its own push block named `pc` with a `uint materialIndex` FIRST — forward stages use `ForwardPushConstants`, shadow stages `shadow_push.glsl` — which is what lets one file serve both. | | `light_ubo.glsl` | **The single declaration of the `LightUBO` block**, `#include`d by `shader.frag` and `skybox.frag` (each `#define`s `LIGHT_UBO_SET`/`LIGHT_UBO_BINDING` first — the buffer sits at a different descriptor address in each). It exists because the block used to be hand-copied: `selfShadowViewProj` was added to the C++ struct and `shader.frag` but not `skybox.frag`, which then read `environmentParams` 256 bytes early — inside `selfShadowViewProj[1]` — so the sky was multiplied by a shadow-matrix element. Silent for months (0 VUIDs, no crash) because unused self slots are IDENTITY, making the misread exactly 1.0 until a scene supplied a *second* skinned self-shadow caster. Any shader binding this buffer must include this file, not restate it — a partial block is not a smaller mistake, it shifts every field after the omission. `shader_block_guards` (CTest) enforces it; `LightUBO`'s `offsetof` asserts in `ubo.hpp` pin the C++ side. | | `skybox.*`, `postprocess.*` | Cubemap sample; ACES/gamma. `skybox.frag` reads ONE field of `LightUBO` (`environmentParams.x`, the sky intensity) but must declare the whole block — hence the shared include above. | diff --git a/docs/roadmap.md b/docs/roadmap.md index 9f68ddf..cc2f2e6 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -218,14 +218,6 @@ between broadphases; handle packing silently aliases invalid inputs), 5 medium, counts + compile-time render-default relationships, enum logger categories with an indexed immutable config, parser/output state into a `.cpp` with precedence tests. -**Out-of-tier finding (2026-08-03).** `depth_prepass.frag` ignores an `alphaMode: MASK` material's -cutout, so it writes depth through the holes: anything behind a cutout is depth-rejected by the -forward pass, and SSAO/contact shadows treat the cutout as a solid sheet. Long-standing (visible in -the pre-SH-05 reference capture), found while landing SH-05, and now a small fix that should reuse -SH-05's shared `shaders/material.glsl` cutout rather than repeat it — see -[`codereview.md`](codereview.md) § Out-of-tier finding for the evidence and the two decisions it -carries. - Further tiers of this review are expected to follow the [`review-order.md`](review-order.md) tiers. --- diff --git a/include/fire_engine/graphics/material_binding.hpp b/include/fire_engine/graphics/material_binding.hpp index 573c955..77a03ef 100644 --- a/include/fire_engine/graphics/material_binding.hpp +++ b/include/fire_engine/graphics/material_binding.hpp @@ -15,6 +15,36 @@ using MaterialTextureHandles = std::array= 0 — because +// `depth_prepass.frag` skips its cutout test when the packed cutoff is 0, which is equivalent to +// running it only while alpha >= 0 (see material_binding.cpp for the full argument). +// +// PURE and noexcept, and separate from the warning below on purpose: `toMaterialUBO` is reachable +// through `materialsEquivalent` from `Object::wouldChangeVariant` and +// `Mesh::isSelectableVariantState`, both `noexcept`, so nothing on that path may throw — and a +// formatting or allocation failure inside a log call would terminate the process rather than report +// anything. +struct MaterialAlphaRangeIssues +{ + bool alpha{false}; + bool cutoff{false}; + + [[nodiscard]] bool any() const noexcept + { + return alpha || cutoff; + } +}; + +[[nodiscard]] +MaterialAlphaRangeIssues materialAlphaRangeIssues(const Material& material) noexcept; + +// Reports the above. NOT noexcept, and deliberately NOT called from the packing path: call it from +// a non-noexcept site that runs ONCE per material — `Resources::registerMaterial`'s first-sight +// branch — so a per-frame variant-selection query can neither terminate the process nor re-emit the +// same warning every frame. +void warnOnMaterialAlphaRangeIssues(const Material& material); + [[nodiscard]] bool materialsEquivalent(const Material& lhs, const Material& rhs); diff --git a/include/fire_engine/render/constants.hpp b/include/fire_engine/render/constants.hpp index 75227a9..e6f1994 100644 --- a/include/fire_engine/render/constants.hpp +++ b/include/fire_engine/render/constants.hpp @@ -123,22 +123,22 @@ inline constexpr float kPointShadowInfiniteRangeFallback = 100.0f; // an image-wide average dilutes it away. // // The shadowed area is MEASURED, not assumed: the pixels that differ between the reference and the -// same view with `--no-shadows`. It came out at 12.07% of the frame — 10.4% before SH-05, which is -// most of why this table was re-measured (see the notes below). (A first pass called "darker than -// half" shadowed, which counted the night skybox and every dark material — 39.8% — and flattered -// every percentage by ~3.8x.) The reference captured twice gives a noise floor of exactly zero, so -// every number below is signal — and a NON-ZERO noise floor invalidates the run (see the SH-06 -// note). +// same view with `--no-shadows`. It came out at 11.68% of the frame — 10.4% before SH-05 — and the +// notes below track every move it has made, because every relative error in this table is divided +// by it. (A first pass called "darker than half" shadowed, which counted the night skybox and every +// dark material — 39.8% — and flattered every percentage by ~3.8x.) The reference captured twice +// gives a noise floor of exactly zero, so every number below is signal — and a NON-ZERO noise floor +// invalidates the run (see the SH-06 note). // -// MEASURED ON MERGED `main` (SH-05 + SH-06 together), 2026-08-04: +// MEASURED with SH-05 + SH-06 + the cutout-aware depth prepass, 2026-08-04: // // budget differing shadow px worst px cascade tris (of 43472 at full detail) // 0.5 0.000% 0/255 23272 (53.5%) identical error to the reference // 1 0.003% 60/255 22120 (50.9%) -// 2 0.249% 121/255 20968 (48.2%) -// 4 0.322% 121/255 19624 (45.1%) -// 8 1.426% 161/255 16200 (37.3%) -// 16 5.103% 172/255 13064 (30.1%) +// 2 0.257% 121/255 20968 (48.2%) +// 4 0.333% 121/255 19624 (45.1%) +// 8 1.472% 161/255 16200 (37.3%) +// 16 5.271% 172/255 13064 (30.1%) // // ACCEPTANCE THRESHOLD, registered before the numbers were corrected: at most 0.1% of the shadowed // pixels may differ from full detail, AND the differences must sit on silhouette edges rather than @@ -240,6 +240,27 @@ inline constexpr float kPointShadowInfiniteRangeFallback = 100.0f; // The 0.1% threshold was re-applied unchanged and STILL selects budget 1. It has now survived four // independent re-derivations — SH-04, SH-06, SH-05, and this merged run — without being moved, // which is the point of registering a criterion before seeing the data. +// +// RE-MEASURED AGAIN after the CUTOUT-AWARE DEPTH PREPASS (2026-08-04). `depth_prepass.frag` had +// been writing depth through a MASK material's holes, so the forward pass depth-rejected everything +// behind them; fixing it changes what the metric can see, so the table was re-taken. What moved, +// and what did not: +// +// * the ABSOLUTE differing-pixel counts did not move at all — 0.0300% / 0.0389% / 0.1720% / +// 0.6158% of the frame at budgets 2 / 4 / 8 / 16, identical to the previous run at every +// printed digit. Shadow-LOD error is what it was; +// * the DENOMINATOR fell, 12.07% -> 11.68%, so every relative error rose in proportion (2: 0.249% +// -> 0.257%, 4: 0.322% -> 0.333%, 8: 1.426% -> 1.472%, 16: 5.103% -> 5.271%). Budget 1 stays at +// 0.003%; +// * the change is LOCALISED, not diffuse, and that was checked rather than assumed: differencing +// the two runs' shadowed-area masks lights up the cutout quad's own footprint (plus sub-pixel +// slivers on two silhouettes elsewhere) and nothing else. Pixels inside the quad's holes used +// to count as shadowed and no longer do — which is the fix, not a regression in the +// measurement; +// * the triangle column and the dead band are untouched (22120/43472 and 30/52 draws at budget 1; +// 3 / 1 / 1 transitions, ZERO reversals). The prepass has nothing to do with shadow selection. +// +// The 0.1% threshold still selects budget 1: 0.257% at budget 2 is 2.5x over it. inline constexpr float kShadowLodPixelBudget = 1.0f; // Coarsening must project within `budget * ratio`, while refining triggers at `budget` — the gap is // the dead band, and 1.0 disables it. diff --git a/shaders/depth_prepass.frag b/shaders/depth_prepass.frag index 1986e95..568d0ff 100644 --- a/shaders/depth_prepass.frag +++ b/shaders/depth_prepass.frag @@ -1,8 +1,48 @@ #version 450 -// Depth prepass: depth-only, no colour attachment. The fixed-function depth test -// writes gl_FragCoord.z; the fragment shader itself produces nothing. Reuses -// shader.vert so the written depth matches the forward pass exactly. +// Depth prepass: depth-only, no colour attachment. The fixed-function depth test writes +// gl_FragCoord.z; this stage produces no colour. It reuses shader.vert so the written depth matches +// the forward pass exactly. +// +// It is NOT empty any more, and the reason is the whole point of the pass: an `alphaMode: MASK` +// material's coverage is not its triangles. The forward shader discards fragments whose base-colour +// alpha falls below the material's cutoff, and a prepass that did not would write depth across the +// holes — so anything BEHIND a cutout failed the forward pass' LESS_OR_EQUAL test and never shaded +// (a leaf card's gaps read as background-coloured nothing), and SSAO, which reconstructs position +// and normal from this depth alone, occluded as if the cutout were a solid sheet. +// +// The test is the SHARED one (material.glsl), on the same UVs from the same vertex path and the same +// bindless material entry, because prepass and forward must discard the same fragments: a fragment +// the prepass keeps and the forward discards leaves a depth-only occluder, and the reverse leaves a +// shaded fragment whose depth nobody wrote. +#include "forward_push.glsl" +#include "material.glsl" + +// Only what this stage reads. shader.vert writes more (normals, TBN, clip positions); a fragment +// stage need not declare outputs it ignores. +layout(location = 3) in vec2 fragTexCoord; +layout(location = 8) in vec2 fragTexCoord1; + void main() { + // GATED on the material's own cutoff, so an opaque draw pays one scalar SSBO read and NO texture + // fetch — the prepass covers the whole screen, and this runs per fragment of it. + // + // The gate is exactly equivalent to running the test unconditionally, which is what makes it a + // cost decision rather than a behavioural one — and the equivalence rests on an invariant that is + // ENFORCED, not assumed. `toMaterialUBO` (graphics/material_binding.cpp) publishes a cutoff only + // for MASK and 0 for every other mode, AND clamps the packed alpha into glTF's [0,1] and the + // packed cutoff to >= 0. With alpha >= 0 the skipped test (`alpha < 0`) can never discard, here + // or in the forward stage that shares this implementation. Without that clamp this gate would BE + // a bug: a negative alpha discards in the forward pass while this stage keeps the fragment, + // leaving a depth-only occluder. A MASK material authored with alphaCutoff 0 discards nothing in + // EITHER pass, which is that value's meaning in the spec. + if (material.materialParams.z > 0.0) + { + if (materialAlphaCutoutFails(materialAlpha(materialBaseColourTexel(fragTexCoord, + fragTexCoord1)))) + { + discard; + } + } } diff --git a/shaders/forward_push.glsl b/shaders/forward_push.glsl new file mode 100644 index 0000000..9a977df --- /dev/null +++ b/shaders/forward_push.glsl @@ -0,0 +1,18 @@ +// The FORWARD-family push-constant block — declared ONCE, here, and included by every stage that +// reads it: `shader.frag` and, since the depth prepass learned to apply the alpha cutout, +// `depth_prepass.frag`. +// +// Push constants are a raw byte range with no driver-side reflection, so a member added to one copy +// and not another silently reinterprets every field after it — a shifted `materialIndex` indexes a +// different bindless material, which is a wrong texture rather than an error. The C++ side is +// ForwardPushConstants in render/ubo.hpp, whose static_asserts pin these offsets; the range the two +// recorders push and the range the pipeline layout declares must cover the whole block, even in a +// stage that reads only one field of it. +layout(push_constant) uniform ForwardPushConstants { + int selfShadowSlot; + uint materialIndex; // index into the global materials[] SSBO for this draw + uint lodLevel; // selected discrete LOD level (read only for the LOD debug tint) + // Level this mesh's shadow draw selected, or 0xFFFFFFFF when it casts no shadow (kNoShadowLod + // in graphics/shadow_diagnostics.hpp). Read only for the Shadow-LOD debug tint. + uint shadowLodLevel; +} pc; diff --git a/shaders/shader.frag b/shaders/shader.frag index 23555c6..c74ea18 100644 --- a/shaders/shader.frag +++ b/shaders/shader.frag @@ -25,16 +25,8 @@ layout(binding = 29) uniform CameraUBO { // The material authority this draw reads lives in material.glsl (bindless set 2), which indexes // materials[] through `pc.materialIndex` — so the push block is declared FIRST and the include -// follows it. -layout(push_constant) uniform ForwardPushConstants { - int selfShadowSlot; - uint materialIndex; // index into the global materials[] SSBO for this draw - uint lodLevel; // selected discrete LOD level (read only for the LOD debug tint) - // Level this mesh's shadow draw selected, or 0xFFFFFFFF when it casts no shadow (kNoShadowLod - // in graphics/shadow_diagnostics.hpp). Read only for the Shadow-LOD debug tint. - uint shadowLodLevel; -} pc; - +// follows it. Both are shared declarations; neither may be restated here. +#include "forward_push.glsl" #include "material.glsl" // Shared palette for the two LOD debug views, so a level always means the same colour in both. diff --git a/src/graphics/material_binding.cpp b/src/graphics/material_binding.cpp index f9b914a..b486c09 100644 --- a/src/graphics/material_binding.cpp +++ b/src/graphics/material_binding.cpp @@ -1,9 +1,12 @@ #include +#include #include #include #include +#include + #include #include #include @@ -36,6 +39,57 @@ bool sameTextureSlot(const TextureSlot& a, const TextureSlot& b) noexcept return a.texture->handle() == b.texture->handle(); } +// The ALPHA-RANGE INVARIANT of the GPU material, enforced here because this is the one seam every +// producer (the glTF loader, procedural materials, variant materials) passes through on its way to +// the shaders. `Material` accepts any float — it is a plain value type — so the guarantee has to be +// made where the value becomes GPU truth, not hoped for at each call site. +// +// glTF pins both ranges: `baseColorFactor.a` is [0,1], and `alphaCutoff` has a spec minimum of 0. +// Out-of-range values are normalised to the spec's reading of them rather than rejected — this is +// authored colour data, not a metric. The two clamps are NOT the same kind of change: +// +// * the CUTOFF clamp is behaviour-preserving. A negative cutoff already discarded nothing in +// `shader.frag` (any alpha >= 0 clears it), so mapping it to 0 keeps exactly that; +// * the ALPHA clamp deliberately CHANGES behaviour for invalid input, and that is the point. A +// negative alpha used to discard in `shader.frag` — an OPAQUE surface silently vanishing on a +// value nobody meant, since a non-MASK material packs cutoff 0 and the test still runs. Clamped +// to 0 the fragment is kept. That is glTF-spec normalisation of nonsense, not preservation of +// it. +// +// Two passes depend on the invariant holding: +// +// * `shader.frag` applies `alpha < alphaCutoff` to EVERY material, per the above; +// * `depth_prepass.frag` SKIPS that test when the packed cutoff is 0, which is only equivalent to +// running it while alpha >= 0. Without this invariant the prepass would keep a fragment the +// forward pass discards, leaving a depth-only occluder — the exact class of divergence the +// cutout-aware prepass exists to remove. +// +// These two are PURE and noexcept, and deliberately do not log. `toMaterialUBO` is reachable from +// `materialsEquivalent`, which `Object::wouldChangeVariant` and `Mesh::isSelectableVariantState` +// call from `noexcept` query functions — a throw out of formatting or allocation inside a warning +// would terminate the process. The diagnostic lives in `warnOnMaterialAlphaRangeIssues` below, +// which a non-noexcept caller invokes ONCE per material (see Resources::registerMaterial); that +// also stops a variant-selection query from re-emitting the same warning every frame. +[[nodiscard]] +float packedAlpha(float alpha) noexcept +{ + if (!std::isfinite(alpha)) + { + return 1.0f; // opaque: the safe reading of a value that names no coverage at all + } + return std::clamp(alpha, 0.0f, 1.0f); +} + +[[nodiscard]] +float packedAlphaCutoff(float cutoff) noexcept +{ + if (!std::isfinite(cutoff)) + { + return 0.0f; // discards nothing, which is what a non-finite threshold cannot ask for + } + return std::max(cutoff, 0.0f); +} + void writeUv(UvXform& dst, const UvTransform& transform) noexcept { dst.offsetScale[0] = transform.offsetX; @@ -47,20 +101,53 @@ void writeUv(UvXform& dst, const UvTransform& transform) noexcept } // namespace +MaterialAlphaRangeIssues materialAlphaRangeIssues(const Material& mat) noexcept +{ + // The cutoff is only consulted for MASK — every other mode packs 0 regardless of what was + // authored, so an out-of-range cutoff on an OPAQUE material is not an issue with anything. + const bool cutoffMatters = mat.alphaMode() == AlphaMode::Mask; + return MaterialAlphaRangeIssues{ + .alpha = packedAlpha(mat.alpha()) != mat.alpha(), + .cutoff = cutoffMatters && packedAlphaCutoff(mat.alphaCutoff()) != mat.alphaCutoff(), + }; +} + +void warnOnMaterialAlphaRangeIssues(const Material& mat) +{ + const MaterialAlphaRangeIssues issues = materialAlphaRangeIssues(mat); + if (issues.alpha) + { + log::warn( + log::category::general, + "material base-colour alpha {} is outside glTF's [0,1] (or not finite); packing {}", + mat.alpha(), packedAlpha(mat.alpha())); + } + if (issues.cutoff) + { + log::warn( + log::category::general, + "material alphaCutoff {} is negative or not finite; packing {} (discards nothing)", + mat.alphaCutoff(), packedAlphaCutoff(mat.alphaCutoff())); + } +} + MaterialUBO toMaterialUBO(const Material& mat) { MaterialUBO ubo{}; ubo.diffuseAlpha[0] = mat.baseColor().r(); ubo.diffuseAlpha[1] = mat.baseColor().g(); ubo.diffuseAlpha[2] = mat.baseColor().b(); - ubo.diffuseAlpha[3] = mat.alpha(); + ubo.diffuseAlpha[3] = packedAlpha(mat.alpha()); ubo.emissiveRoughness[0] = mat.emissive().r(); ubo.emissiveRoughness[1] = mat.emissive().g(); ubo.emissiveRoughness[2] = mat.emissive().b(); ubo.emissiveRoughness[3] = mat.roughness(); ubo.materialParams[0] = mat.metallic(); ubo.materialParams[1] = mat.normalScale(); - ubo.materialParams[2] = mat.alphaMode() == AlphaMode::Mask ? mat.alphaCutoff() : 0.0f; + // Cutoff reaches the GPU only for MASK — every other mode packs 0, which is what makes the + // shared cutout test inert rather than wrong for them. + ubo.materialParams[2] = + mat.alphaMode() == AlphaMode::Mask ? packedAlphaCutoff(mat.alphaCutoff()) : 0.0f; ubo.materialParams[3] = mat.occlusionStrength(); using Slot = MaterialTextureSlot; ubo.textureFlags[0] = mat.texture(Slot::BaseColour).has() ? 1 : 0; diff --git a/src/render/pipeline.cpp b/src/render/pipeline.cpp index dc2e3c9..d3f439f 100644 --- a/src/render/pipeline.cpp +++ b/src/render/pipeline.cpp @@ -235,7 +235,8 @@ PipelineConfig Pipeline::depthPrepassConfig() config.vertShaderPath = "shader.vert.spv"; config.fragShaderPath = "depth_prepass.frag.spv"; // Same per-object push-descriptor set 0 as forward (frame/skin/morph + morph - // SSBO) so pushForwardObjectDescriptors works unchanged; no globals/bindless. + // SSBO) so pushForwardObjectDescriptors works unchanged. No forward GLOBALS (set 1 exists here + // but is empty) — but it DOES take the bindless material set, see below. config.bindings = perObjectSet0Bindings(); config.pushDescriptorSet0 = true; // Depth-only: no colour attachments. depthFormat matches the shared D32. @@ -245,6 +246,13 @@ PipelineConfig Pipeline::depthPrepassConfig() // Cull per draw like the forward opaque pipeline (double-sided => no cull) so // double-sided geometry writes the same depth the forward pass will test. config.dynamicCullMode = true; + // The prepass applies the material's ALPHA CUTOUT, so it reads the same bindless material + // authority the forward shader does (set 2 — an empty set-1 layout keeps that index identical + // across pipelines, see the constructor) and takes the same push block to index it with. + // Without this the prepass wrote depth through a cutout's holes and the forward pass then + // depth-rejected whatever stood behind them. + config.bindlessSet = true; + addFragmentPushConstant(config, static_cast(sizeof(ForwardPushConstants))); return config; } diff --git a/src/render/renderer.cpp b/src/render/renderer.cpp index 3cd82a9..2613421 100644 --- a/src/render/renderer.cpp +++ b/src/render/renderer.cpp @@ -934,6 +934,7 @@ void Renderer::recordDepthPrepass(vk::CommandBuffer cmd, const DrawBuckets& buck const vk::PipelineLayout layout = resources_.vulkanPipelineLayout(depthPrepassHandle_); cmd.bindPipeline(vk::PipelineBindPoint::eGraphics, resources_.vulkanPipeline(depthPrepassHandle_)); + bool bindlessBound = false; for (const auto& dc : buckets.opaque) { // buckets.opaque also carries the skybox (fullscreen triangle, no depth / @@ -951,6 +952,21 @@ void Renderer::recordDepthPrepass(vk::CommandBuffer cmd, const DrawBuckets& buck dc.indexType == DrawIndexType::UInt32 ? vk::IndexType::eUint32 : vk::IndexType::eUint16; cmd.bindIndexBuffer(resources_.vulkanBuffer(dc.indexBuffer), 0, indexType); pushForwardObjectDescriptors(cmd, resources_, layout, dc); + if (!bindlessBound) + { + // Bindless materials (set 2) for the prepass' alpha-cutout test. Bound AFTER the first + // push-descriptor write to set 0, the same ordering the forward recorder documents: + // layout compatibility preserves set 0, and this order avoids a Vulkan Validation + // Layers 1.4.350 first-use push-state defect. Once per pass — one pipeline, one set. + cmd.bindDescriptorSets(vk::PipelineBindPoint::eGraphics, layout, 2, + resources_.bindlessDescriptorSet(), {}); + bindlessBound = true; + } + // Per draw, because materialIndex is per draw. Built by the SAME helper the forward pass + // uses, so the prepass cannot end up testing a different material than the surface it is + // writing depth for. + cmd.pushConstants(layout, vk::ShaderStageFlagBits::eFragment, 0, + makeForwardPushConstants(dc)); recordIndexedDraw(cmd, dc, resources_); } cmd.endRendering(); diff --git a/src/render/resources.cpp b/src/render/resources.cpp index dfc0490..8ce5000 100644 --- a/src/render/resources.cpp +++ b/src/render/resources.cpp @@ -611,6 +611,12 @@ uint32_t Resources::registerMaterial(const Material& material) } const uint32_t index = materialCount_++; + // FIRST SIGHT of this material, and the only place its authored values become GPU state — so + // this is where an out-of-range alpha or cutoff is reported. Deliberately not inside + // `toMaterialUBO`: that runs on the `noexcept` variant-comparison path + // (Object::wouldChangeVariant), where a throw out of log formatting would terminate, and it + // would repeat the warning every frame. + warnOnMaterialAlphaRangeIssues(material); const MaterialUBO ubo = toMaterialUBO(material); writeMapped(materialMapped_.subspan(static_cast(index) * sizeof(MaterialUBO)), ubo); diff --git a/tests/graphics/test_material.cpp b/tests/graphics/test_material.cpp index 459ee63..1afb383 100644 --- a/tests/graphics/test_material.cpp +++ b/tests/graphics/test_material.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -138,6 +139,129 @@ TEST_CASE("MaterialBinding.ToMaterialUboPacksCoreFields", "[MaterialBinding]") CHECK(ubo.materialParams[3] == Catch::Approx(0.7f).margin(1e-5f)); } +TEST_CASE("MaterialBinding.PackingEnforcesTheAlphaRangeInvariant", "[MaterialBinding]") +{ + // The invariant two depth-writing passes rest on. `shader.frag` applies `alpha < alphaCutoff` + // to EVERY material (a non-MASK one packs cutoff 0), while `depth_prepass.frag` SKIPS that test + // when the packed cutoff is 0 — equivalent only while alpha >= 0. If an out-of-range value + // reached the GPU the two would disagree, and a fragment the forward pass discards but the + // prepass keeps leaves a depth-only occluder: invisible geometry that still hides what is + // behind it. + // + // `Material` is a plain value type that accepts any float, so this seam is where the guarantee + // is made. The two clamps differ in kind: the CUTOFF clamp is behaviour-preserving (a negative + // cutoff already discarded nothing in the forward pass), while the ALPHA clamp deliberately + // CHANGES behaviour for invalid input — a negative alpha used to discard, and clamped to 0 the + // fragment is kept. That is glTF-spec normalisation of a value nobody meant, not preservation. + SECTION("base-colour alpha is clamped into glTF's [0,1]") + { + Material mat; + mat.alpha(-0.25f); + CHECK(toMaterialUBO(mat).diffuseAlpha[3] == Catch::Approx(0.0f).margin(1e-5f)); + mat.alpha(4.0f); + CHECK(toMaterialUBO(mat).diffuseAlpha[3] == Catch::Approx(1.0f).margin(1e-5f)); + } + SECTION("a non-finite alpha packs opaque") + { + // Every non-finite value, not just NaN: negative infinity would otherwise fall through the + // range path to 0 and read as fully transparent, which is a different answer from "this + // value names no coverage at all". + Material mat; + for (const float bad : + {std::numeric_limits::quiet_NaN(), std::numeric_limits::infinity(), + -std::numeric_limits::infinity()}) + { + mat.alpha(bad); + CHECK(toMaterialUBO(mat).diffuseAlpha[3] == Catch::Approx(1.0f).margin(1e-5f)); + } + } + SECTION("a negative cutoff packs 0, which is what it already meant") + { + Material mat; + mat.alphaMode(AlphaMode::Mask); + mat.alphaCutoff(-0.5f); + CHECK(toMaterialUBO(mat).materialParams[2] == Catch::Approx(0.0f).margin(1e-5f)); + } + SECTION("a non-finite cutoff packs 0") + { + Material mat; + mat.alphaMode(AlphaMode::Mask); + for (const float bad : + {std::numeric_limits::quiet_NaN(), std::numeric_limits::infinity(), + -std::numeric_limits::infinity()}) + { + mat.alphaCutoff(bad); + CHECK(toMaterialUBO(mat).materialParams[2] == Catch::Approx(0.0f).margin(1e-5f)); + } + } + SECTION("the specific divergence that motivated this: MASK, cutoff 0, negative alpha") + { + // Pre-enforcement this pair made the forward pass discard (-0.25 < 0) and the prepass keep + // (its gate saw cutoff 0 and skipped the test). Packed, the pair can no longer express it: + // alpha is 0 and the cutoff is 0, so NEITHER pass discards. + Material mat; + mat.alphaMode(AlphaMode::Mask); + mat.alphaCutoff(0.0f); + mat.alpha(-0.25f); + const MaterialUBO ubo = toMaterialUBO(mat); + CHECK(ubo.diffuseAlpha[3] == Catch::Approx(0.0f).margin(1e-5f)); + CHECK(ubo.materialParams[2] == Catch::Approx(0.0f).margin(1e-5f)); + CHECK_FALSE(ubo.diffuseAlpha[3] < ubo.materialParams[2]); + } + SECTION("in-range values are packed untouched") + { + Material mat; + mat.alphaMode(AlphaMode::Mask); + mat.alpha(0.25f); + mat.alphaCutoff(0.75f); + const MaterialUBO ubo = toMaterialUBO(mat); + CHECK(ubo.diffuseAlpha[3] == Catch::Approx(0.25f).margin(1e-5f)); + CHECK(ubo.materialParams[2] == Catch::Approx(0.75f).margin(1e-5f)); + } +} + +TEST_CASE("MaterialBinding.AlphaRangeIssuesAreReportableWithoutPacking", "[MaterialBinding]") +{ + // The diagnostic's DECISION, pinned pure. The warning itself sits behind this and is emitted + // once per material from Resources::registerMaterial, because `toMaterialUBO` is reachable from + // `Object::wouldChangeVariant` — a noexcept query — where a throw out of log formatting would + // terminate the process. Keeping the decision separate from the reporting is what makes it + // testable at all. + SECTION("a well-formed material reports nothing") + { + Material mat; + mat.alphaMode(AlphaMode::Mask); + mat.alpha(0.5f); + mat.alphaCutoff(0.5f); + const MaterialAlphaRangeIssues issues = materialAlphaRangeIssues(mat); + CHECK_FALSE(issues.alpha); + CHECK_FALSE(issues.cutoff); + CHECK_FALSE(issues.any()); + } + SECTION("each field is reported independently") + { + Material alphaBad; + alphaBad.alpha(2.0f); + CHECK(materialAlphaRangeIssues(alphaBad).alpha); + CHECK_FALSE(materialAlphaRangeIssues(alphaBad).cutoff); + + Material cutoffBad; + cutoffBad.alphaMode(AlphaMode::Mask); + cutoffBad.alphaCutoff(-1.0f); + CHECK(materialAlphaRangeIssues(cutoffBad).cutoff); + CHECK_FALSE(materialAlphaRangeIssues(cutoffBad).alpha); + } + SECTION("a cutoff nobody consults is not an issue") + { + // Only MASK packs a cutoff at all; on any other mode the authored value is discarded, so + // reporting it would send someone hunting for a problem that cannot reach a shader. + Material mat; + mat.alphaMode(AlphaMode::Opaque); + mat.alphaCutoff(-1.0f); + CHECK_FALSE(materialAlphaRangeIssues(mat).any()); + } +} + TEST_CASE("MaterialBinding.ToMaterialUboUsesExtensionDefaultsWhenAbsent", "[MaterialBinding]") { // Absent optional blocks must pack the same defaults the old always-present diff --git a/tests/render/test_pipeline_config.cpp b/tests/render/test_pipeline_config.cpp index 6da07ae..4009eec 100644 --- a/tests/render/test_pipeline_config.cpp +++ b/tests/render/test_pipeline_config.cpp @@ -9,6 +9,7 @@ using fire_engine::bindingIndex; using fire_engine::ForwardBinding; +using fire_engine::ForwardPushConstants; using fire_engine::Pipeline; using fire_engine::ShadowBinding; @@ -110,6 +111,29 @@ TEST_CASE("PipelineConfig.ForwardConfigPushesSet0", "[PipelineConfig]") CHECK_FALSE(Pipeline::skyboxConfig().pushDescriptorSet0); } +TEST_CASE("PipelineConfig.DepthPrepassReadsTheMaterialAuthority", "[PipelineConfig]") +{ + // The prepass applies the alpha cutout, so it needs the same bindless material set (2) and a + // push block to index it with. Without either, a MASK material writes depth through its holes + // and the forward pass depth-rejects whatever stands behind them — the defect this pins. + const auto prepass = Pipeline::depthPrepassConfig(); + const auto forward = Pipeline::forwardConfig(); + + CHECK(prepass.bindlessSet); + REQUIRE(prepass.pushConstantRanges.size() == 1u); + CHECK(prepass.pushConstantRanges[0].stageFlags == vk::ShaderStageFlagBits::eFragment); + CHECK(prepass.pushConstantRanges[0].offset == 0u); + // The WHOLE block, even though the stage reads one field of it: the range must cover what the + // shader declares, and both stages share one declaration (shaders/forward_push.glsl). + CHECK(prepass.pushConstantRanges[0].size == sizeof(ForwardPushConstants)); + // Same vertex path as the forward pass, so the depth it writes and the UVs it tests are the + // forward pass' own — that identity is what makes the two discard the same fragments. + CHECK(prepass.vertShaderPath == forward.vertShaderPath); + CHECK(prepass.fragShaderPath == "depth_prepass.frag.spv"); + CHECK(prepass.dynamicCullMode); + CHECK(prepass.depthWrite); +} + TEST_CASE("PipelineConfig.ShadowConfigLeavesCullingToTheRecorder", "[PipelineConfig]") { auto config = Pipeline::shadowConfig();