Shadow per view discrete lod - #125
Closed
nnewson wants to merge 2 commits into
Closed
Conversation
… to name the same thing next frame as it did last frame. Nothing in the engine did. Spot and point slots are handed out per frame in whatever order the active light set produces, so keying on a physical slot applies a departed light's dead band to whichever light takes its place — shadows behaving oddly near thresholds, with nothing else to see. Node is the identity authority: it is the non-copyable scene instance, while a Light is copyable parameter data that cannot say which instance in the scene it belongs to. NodeId is a process-unique monotonic id, allocated atomically and carried into Lighting so the shadow pass has something stable to key punctual views on. The two cheaper handles both alias: a gathered-array index is traversal order, so adding or reordering a node re-points every key past it, and a node pointer can be recycled by the allocator so a deleted node's address may reappear as a different node and inherit its history. The id lives in a move-aware owner. A plain scalar member is copied by a defaulted move, leaving the moved-from and moved-to objects both alive holding one "process-unique" id; NodeIdentity gives the moved-from object a fresh id so two live objects can never claim the same identity. The tests assert all three properties — destination inherits, source stays valid, and the two differ — because checking only the destination is exactly how that bug survives. The primitive lives in core/ rather than scene/. Node remains the authority; only the physical location changed, so the graphics seam can carry an id without a graphics header including a scene header. Worth noting cmake/check_layering.cmake only detects DIRECT forbidden includes, so that transitive breach would have passed CI. ShadowCasterId identifies a single geometry BINDING. Object::objectId is per Object, so a multi-geometry mesh emits several shadow commands sharing one id and they would overwrite each other's history, the finer caster dragging the coarser one's level around. ShadowCasterGeneration advances when a binding's shadowGeometry is replaced, through a checked helper that aborts rather than wrapping to First — where a caster could find history keyed against a chain replaced 2^64 generations ago. ShadowLogicalViewId is encapsulated with validating factories rather than a public aggregate. As an aggregate it could be assembled with a point face of 9, a nonzero face on a cascade, a zero light id, or left default-constructed silently meaning cascade 0 — each producing a key that looks valid while naming the wrong view. The default is Invalid. Spot and Point are separate kinds: a light is only ever one or the other, so a shared "punctual" kind's collision would be unreachable today, but that invariant is held elsewhere and a key type should not lean on it. The all-pairs test caught this by contradicting a line I had written asserting the collision was harmless. World-only deliberately SHARES its cascade's identity. The two passes must make the same choice for a given rigid caster, and sharing the key guarantees it rather than hoping two independent computations agree. The persistent key is (ShadowCasterId, ShadowCasterGeneration, ShadowLogicalViewId) — never a physical slot. The generation is part of the key rather than a field compared at each lookup, so a replaced chain simply finds nothing instead of relying on every future call site to remember a comparison. NodeId, ShadowCasterId and ShadowCasterGeneration are strong enum class types, not uint64_t aliases: as aliases the identity domains are freely interchangeable and ShadowLogicalViewId::spot(someCasterId) compiles while naming nothing. Separately, kCubeFaceCount becomes the single authority for cube topology, in graphics/gpu_limits.hpp. The count participates in logical-view key validation, shadow matrix indexing, image layer indexing and the flat point-view slot arithmetic, and it was duplicated as a literal across all of them: shadow_diagnostics point capacity and flattening, renderer frustum ranges, the shadow face loop, point-map layers and face views, the overlay's slot decode, kShadowTotalMatrixCount, and the generic cubemap upload/view paths. Drift would have been quiet and destructive — every index still in range, just pointing at the wrong face. Note the two flattenings differ: cubemap face views are mip-major, point shadow face views are cube-major, same stride. The test now pins the topology once with STATIC_REQUIRE(kCubeFaceCount == 6) and expresses every flattening expectation through the constant. Nothing consumes these identities yet — no shadow code reads them and selectShadowLod is still only called from tests. They are wired in slice 3, where the command seam moves and LOD is resolved per shadow view.
…they meant the same fit The renderer kept every shadow matrix in one flat array and the description of the view that produced it nowhere at all. Culling built a frustum from a matrix, the LOD choice was made against the camera, and the shadow pass rasterised with a third read of the same slot — three consumers with no way to tell whether they were talking about the same fit. Nothing checked, because there was nothing to check against. ShadowRenderViewSet holds each view's matrix, its ShadowView projection descriptor and its stable logical identity in one read-only entry, addressed by the physical (group, slot) SH-01's diagnostics already use. Writers are per family — setCascade, setSelf, setSpot, setPoint — each deriving the identity from the slot it is writing and rejecting a descriptor of the wrong kind, so a cascade identity in a spot slot or an orthographic projection on a point face is unrepresentable rather than merely detectable. World-only stores nothing at all: enableWorldOnly sets a bit and the lookup aliases the cascade entry, so a cascade re-fitted afterwards moves both passes. A copy would have been equal only at the instant it was taken. Every fit now produces the matrix and the descriptor from the same intermediates. fitCascade returns the texel size the cascade snapped to rather than having it recomputed elsewhere; the self-shadow fit derives its texel size from the same radius as its ortho box; each point face gets a descriptor built from the very direction its matrix looks down. The renderer's shadowViewProjs_ is gone, and every consumer is a projection of the set: the ShadowUBO array, the LightUBO cascade/spot/self arrays, the coarse cull frustums, and the shadow pass. Two states are kept apart deliberately. An invalid projection descriptor is engaged and reported — the view rasterises and selection says InvalidView — while a non-finite render matrix is refused, because it is what culling tests against and what the GPU transforms with: a NaN plane makes the cull permissive rather than empty, and NaN clip coordinates draw nothing. A rejected write clears the slot it addressed instead of leaving the previous frame's fit in place; an out-of-range address touches nothing, and the point writer validates its light slot before flattening, since the flat index can wrap back into range. Refusal is terminal. Every writer returns [[nodiscard]] bool, and no counter — activeSpotCasters_, activePointCasters_, the self-shadow slot map — advances until the set has accepted the view, so a pass can never be driven by a count the set does not back. A point light is counted only once all six faces are in. In a Dev build the set's assertion fires first, inside the writer; under NDEBUG the returned false reaches rejectedShadowView, which throws for a named Fatal: message and a non-zero exit. Whether the world-only pass runs is now the set's answer, read back in recordShadowPass; anySkinned is only the request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.