diff --git a/OPSTACK-PROGRESS.md b/OPSTACK-PROGRESS.md index 681bf9d..9dda041 100644 --- a/OPSTACK-PROGRESS.md +++ b/OPSTACK-PROGRESS.md @@ -2679,3 +2679,86 @@ line"**, because the next person's guess would be as good as mine was. tile is *wrong*. 3. Everything else should be unchanged. The worm change cannot alter density: `EffectOverBox` and `MaxCarveOverBox` are box-verdict methods, and `Eval` is untouched. + +## 2026-07-28 — the worm fix WORKED. `AllSolid killed by: RoomGraphSource x40` — and that is not an answer. + +The attribution line did its job on its first run: + +``` +AllSolid killed by: RoomGraphSource x40 +``` + +`WormFieldSource` is gone from that list, so the previous entry's fix landed exactly as reasoned. The +blocker moved one operator upstream, to the room source itself. + +**And my own warning is now the thing to distrust.** It said: *"If it names RoomGraphSource, the tiles +genuinely straddle cave."* That is a **hypothesis wearing the costume of a conclusion** — the same +mistake as the previous warning, one level down. `RoomGraphSource` has FOUR primitive classes behind +it (rooms, tunnels, pits, chimneys) whose bounds differ enormously in quality, and "the tiles straddle +cave" is only one of the things a `Both` from it can mean. So: no third guess. Two changes, one of +them proved, and an instrument for the rest. + +### Proved, not guessed: the columns test is gone + +The first version treated columns as **infinite cylinders in Z** (the cache gives them no vertical +bound), so a box hundreds of voxels below the owning room answered `Both` because it shared an XY +circle with a column. That was the loosest test in the function — and it was **redundant**, not +conservative: + +```cpp +void FRoomColumnMod::Eval(...) const +{ + if (!VF_NearCaveSurface(InOut.Sdf, P.SDFBlendRadius)) { return; } // <- the only consumer +``` + +Columns are read by exactly one operator, and it gates on `Sdf` being near a cave surface. In a box +no room, tunnel, pit or chimney reaches, `Sdf` stays `FLT_MAX` at every voxel, so **no column can +execute regardless of where it sits in XY**. Removing the test tightens the verdict without touching +its correctness. That is a proof, not a relaxation. + +### The instrument: which primitive class actually reaches the box + +`EffectOverBox` no longer early-outs on the first hit. It **counts all four classes**, because +stopping at the first gives the right verdict and no information — which is precisely why +`RoomGraphSource x40` told us nothing actionable. The cost is nil at the scale that matters: we have +just run `BuildChunkCache`, which dwarfs a walk over ~100 structs, and the verdict is memoised so the +walk happens once per box rather than thirteen times. + +`VoxelDensityOps::GetLastRoomBoxDiagnostic()` exposes it. **Deliberately a read-back of what the +operator computed, not a re-derivation in the test** — the test has everything needed to replay the +criterion, and replaying it would create a second definition that drifts from the real one and lies +on the day it is believed. Same reason `VF_BuildOpStackForChunk` exists. + +The report now prints, per killed tile, how many rooms and tunnels of those in the cache actually +reach the box. **The hypothesis it is built to kill or confirm:** a tunnel is culled per voxel by its +**bounding sphere**, and for a long thin capsule that sphere is an enormous over-estimate, while a +room's cull sphere is a fair fit for a roughly spherical room. If tunnels ≫ rooms, the box test is +losing to capsule bounding spheres rather than to real cave, and the fix is a segment-vs-box distance +— nothing to do with the sampler or with cave density. + +### What was deliberately NOT done, and why + +Tightening tunnels to a real capsule test is **not** free correctness: the per-voxel cull *is* the +bounding sphere, so a capsule test would be tighter than the cull and would break the stated criterion +("no primitive survives its cull"). Making it sound needs the stronger criterion — *no primitive can +bring `Sdf` below `max(Blend, SDFBlendRadius·3, WormNetworkRange)`* — which in turn needs a bound on +how far `SmoothMin` of N primitives can dip below `min`. That is real §0.2 design work, and doing it +blind, in the same build, before knowing whether tunnels are even the problem, is the §C10 mistake +verbatim. **Measure, then tighten what the numbers name.** + +### Ready to build. Likely compile-error spots + +1. `VoxelDensityOps::FRoomBoxDiagnostic` + `GetLastRoomBoxDiagnostic()` — new declaration in the + header, defined in the .cpp *after* the anonymous namespace closes (it reads + `FRoomGraphSource::BoxState()`, whose type lives in that namespace; legal, since the type does not + appear in the function's signature). +2. `FBoxState` gained eight `int32` counters. +3. The test accumulates into new locals and calls `VoxelDensityOps::GetLastRoomBoxDiagnostic()`. + +### What to read + +1. **`...and when RoomGraphSource is the killer`** — the new second line. `tunnels 40 / rooms 3` says + capsule bounding spheres; `rooms 40` says the tiles really are near rooms and the sampler is what + to look at; pits or chimneys leading would be a surprise worth stopping on. +2. Whether removing the columns test alone moved `proved` off zero. If it did, that number is the + first real T1.d saving in the plugin. diff --git a/Source/VoxelForge/Private/Tests/VoxelForgeOpStackTunnelTest.cpp b/Source/VoxelForge/Private/Tests/VoxelForgeOpStackTunnelTest.cpp index 182213e..3e0a4ca 100644 --- a/Source/VoxelForge/Private/Tests/VoxelForgeOpStackTunnelTest.cpp +++ b/Source/VoxelForge/Private/Tests/VoxelForgeOpStackTunnelTest.cpp @@ -1107,6 +1107,9 @@ bool FVoxelForgeOpStackTunnelTest::RunTest(const FString& Parameters) int32 NumBruteSamples = 0, NumViolations = 0; float WorstViolation = 0.0f; TMap SolidKillerCounts; + int32 NumRoomKilled = 0; + int32 TilesHitByRooms = 0, TilesHitByTunnels = 0, TilesHitByPits = 0, TilesHitByChimneys = 0; + int32 SumHitRooms = 0, SumNumRooms = 0, SumHitTunnels = 0, SumNumTunnels = 0; FRandomStream Rng(97531); for (int32 t = 0; t < 40; ++t) @@ -1131,7 +1134,26 @@ bool FVoxelForgeOpStackTunnelTest::RunTest(const FString& Parameters) if (SolidKiller != INDEX_NONE) { - SolidKillerCounts.FindOrAdd(Stack.GetOpDebugName(SolidKiller))++; + const FString KillerName = Stack.GetOpDebugName(SolidKiller); + SolidKillerCounts.FindOrAdd(KillerName)++; + + // VENTILATION PAR CLASSE DE PRIMITIVE. Quand c'est la source de salles qui tue, + // « les tuiles traversent une grotte » n'est pas une réponse : les salles, les + // tunnels, les pits et les cheminées ont chacun leur borne, de finesse très + // différente (une sphère englobante de capsule est un très mauvais tunnel). On lit + // ce que l'opérateur a RÉELLEMENT calculé plutôt que de rejouer le critère ici. + if (KillerName == TEXT("RoomGraphSource")) + { + const VoxelDensityOps::FRoomBoxDiagnostic D = + VoxelDensityOps::GetLastRoomBoxDiagnostic(); + ++NumRoomKilled; + if (D.HitRooms > 0) { ++TilesHitByRooms; } + if (D.HitTunnels > 0) { ++TilesHitByTunnels; } + if (D.HitPits > 0) { ++TilesHitByPits; } + if (D.HitChimneys > 0) { ++TilesHitByChimneys; } + SumHitRooms += D.HitRooms; SumNumRooms += D.NumRooms; + SumHitTunnels += D.HitTunnels; SumNumTunnels += D.NumTunnels; + } } if (Verdict == EVoxelTileClass::Mixed) { ++NumMixed; continue; } @@ -1191,6 +1213,23 @@ bool FVoxelForgeOpStackTunnelTest::RunTest(const FString& Parameters) TEXT("alone drove SolidMargin negative on every tile in the world. Attribution is ") TEXT("cheap; a second wrong guess is not."), *Breakdown)); + + if (NumRoomKilled > 0) + { + AddInfo(FString::Printf( + TEXT("...and when RoomGraphSource is the killer (%d tiles), WHICH primitive class ") + TEXT("reaches the box: rooms %d, tunnels %d, pits %d, chimneys %d (tiles, not ") + TEXT("primitives -- a tile can be hit by several). Averages per killed tile: ") + TEXT("%.1f of %.1f rooms reach, %.1f of %.1f tunnels reach. THIS is the line that ") + TEXT("says what to tighten. A tunnel is culled per voxel by its BOUNDING SPHERE, ") + TEXT("which for a long thin capsule is an enormous over-estimate; a room's cull ") + TEXT("sphere is a fair fit. So tunnels >> rooms here would mean the box test is ") + TEXT("losing to capsule bounding spheres, not to real cave -- and the fix would ") + TEXT("be a segment-vs-box distance, not anything about the sampler."), + NumRoomKilled, TilesHitByRooms, TilesHitByTunnels, TilesHitByPits, TilesHitByChimneys, + (float)SumHitRooms / (float)NumRoomKilled, (float)SumNumRooms / (float)NumRoomKilled, + (float)SumHitTunnels / (float)NumRoomKilled, (float)SumNumTunnels / (float)NumRoomKilled)); + } } if (NumProved == 0) diff --git a/Source/VoxelForge/Private/VoxelDensityOpStack.cpp b/Source/VoxelForge/Private/VoxelDensityOpStack.cpp index 70bd349..1d2500a 100644 --- a/Source/VoxelForge/Private/VoxelDensityOpStack.cpp +++ b/Source/VoxelForge/Private/VoxelDensityOpStack.cpp @@ -2162,6 +2162,12 @@ namespace uint32 KeyLayout = 0xFFFFFFFFu; bool bValid = false; EVoxelOpEffect Verdict = EVoxelOpEffect::Both; + + /** DIAGNOSTIC — combien de primitives de chaque classe atteignent la dernière boîte + * interrogée, et combien le cache en contenait. Lu par les tests via + * `VoxelDensityOps::GetLastRoomBoxDiagnostic`. N'entre dans aucune décision. */ + int32 HitRooms = 0, HitTunnels = 0, HitPits = 0, HitChimneys = 0; + int32 NumRooms = 0, NumTunnels = 0, NumPits = 0, NumChimneys = 0; }; static FBoxState& BoxState() @@ -2338,61 +2344,70 @@ namespace return (dx * dx + dy * dy) <= RSq; }; - bool bReached = false; + //----------------------------------------------------------------- + // ⚠️ PAS D'EARLY-OUT : ON COMPTE PAR CLASSE, ET C'EST DÉLIBÉRÉ + //----------------------------------------------------------------- + // La version d'origine s'arrêtait à la première primitive atteinte. Elle donnait le bon + // verdict et AUCUNE information : quand `AllSolid killed by: RoomGraphSource x40` est + // tombé, il n'y avait aucun moyen de dire si le coupable était les salles, les tunnels + // ou les pits — donc aucun moyen de savoir quoi resserrer. Compter les cinq classes + // sépare les causes, et c'est la règle que ce projet a payée plusieurs fois : quand un + // zéro a plusieurs causes possibles, chacune a son propre nombre. + // + // Le coût est nul à l'échelle qui compte : on vient d'appeler `BuildChunkCache`, qui + // est de plusieurs ordres de grandeur au-dessus d'un parcours de ~100 structs, et le + // verdict est mémoïsé donc ce parcours arrive UNE fois par boîte, pas treize. + // + // No early-out on purpose: stopping at the first hit gives the right verdict and no + // information. When a zero has several possible causes, each gets its own number. + B.NumRooms = B.Cache.Rooms.Num(); + B.NumTunnels = B.Cache.Tunnels.Num(); + B.NumPits = B.Cache.Pits.Num(); + B.NumChimneys = B.Cache.Chimneys.Num(); + B.HitRooms = B.HitTunnels = B.HitPits = B.HitChimneys = 0; for (const FCachedRoom& R : B.Cache.Rooms) { - if (SphereHitsBox(R.Center, R.CullRadiusSq, QMin, QMax)) { bReached = true; break; } + if (SphereHitsBox(R.Center, R.CullRadiusSq, QMin, QMax)) { ++B.HitRooms; } } - if (!bReached) + for (const FCachedTunnel& T : B.Cache.Tunnels) { - for (const FCachedTunnel& T : B.Cache.Tunnels) - { - if (SphereHitsBox(T.BoundCenter, T.BoundRadiusSq, QMin, QMax)) { bReached = true; break; } - } + if (SphereHitsBox(T.BoundCenter, T.BoundRadiusSq, QMin, QMax)) { ++B.HitTunnels; } } - if (!bReached) + // Miroir exact des deux `continue` de `Eval` : actif si `Z < TopZ + BlendK` ET + // `Z >= TopZ - Depth - BlendK`. + for (const FCachedPit& Pit : B.Cache.Pits) { - // Miroir exact des deux `continue` de `Eval` : actif si `Z < TopZ + BlendK` ET - // `Z >= TopZ - Depth - BlendK`. - for (const FCachedPit& Pit : B.Cache.Pits) - { - if (!(RMinZ < Pit.TopZ + Pit.BlendK)) { continue; } - if (!(RMaxZ >= Pit.TopZ - Pit.Depth - Pit.BlendK)) { continue; } - if (CircleHitsBoxXY(Pit.CenterX, Pit.CenterY, Pit.BoundXYRadiusSq)) - { - bReached = true; break; - } - } + if (!(RMinZ < Pit.TopZ + Pit.BlendK)) { continue; } + if (!(RMaxZ >= Pit.TopZ - Pit.Depth - Pit.BlendK)) { continue; } + if (CircleHitsBoxXY(Pit.CenterX, Pit.CenterY, Pit.BoundXYRadiusSq)) { ++B.HitPits; } } - if (!bReached) + // Miroir exact : actif si `Z > BottomZ - BlendK` ET `Z <= BottomZ + Height + BlendK`. + for (const FCachedChimney& Ch : B.Cache.Chimneys) { - // Miroir exact : actif si `Z > BottomZ - BlendK` ET `Z <= BottomZ + Height + BlendK`. - for (const FCachedChimney& Ch : B.Cache.Chimneys) - { - if (!(RMaxZ > Ch.BottomZ - Ch.BlendK)) { continue; } - if (!(RMinZ <= Ch.BottomZ + Ch.Height + Ch.BlendK)) { continue; } - if (CircleHitsBoxXY(Ch.CenterX, Ch.CenterY, Ch.BoundXYRadiusSq)) - { - bReached = true; break; - } - } - } - if (!bReached) - { - // Les colonnes ne sont pas lues par CETTE source (c'est `FRoomColumnMod`, STEP 4d, - // qui parcourt `GetCache()`), mais elles héritent de ce verdict. Elles n'ont aucune - // borne en Z dans le cache : on les traite donc comme des cylindres infinis, ce qui - // est le test le plus prudent qu'on puisse écrire à partir de ce qui est stocké. - for (const FCachedColumn& Col : B.Cache.Columns) - { - if (CircleHitsBoxXY(Col.CenterX, Col.CenterY, Col.BoundXYRadiusSq)) - { - bReached = true; break; - } - } + if (!(RMaxZ > Ch.BottomZ - Ch.BlendK)) { continue; } + if (!(RMinZ <= Ch.BottomZ + Ch.Height + Ch.BlendK)) { continue; } + if (CircleHitsBoxXY(Ch.CenterX, Ch.CenterY, Ch.BoundXYRadiusSq)) { ++B.HitChimneys; } } + //----------------------------------------------------------------- + // ✅ LES COLONNES NE SONT PLUS TESTÉES — ET C'EST PROUVÉ, PAS RELÂCHÉ + //----------------------------------------------------------------- + // La première version les traitait en cylindres INFINIS en Z (le cache ne leur donne + // aucune borne verticale), ce qui rendait `Both` pour une boîte située des centaines de + // voxels sous la salle propriétaire. Inutile : le seul consommateur des colonnes est + // `FRoomColumnMod`, dont l'`Eval` commence par + // `if (!VF_NearCaveSurface(InOut.Sdf, P.SDFBlendRadius)) { return; }` + // Si aucune salle, aucun tunnel, aucun pit et aucune cheminée n'atteint la boîte, `Sdf` + // y reste `FLT_MAX`, le gate est faux à chaque voxel, et **aucune colonne ne peut + // s'exécuter** — quelle que soit sa position XY. Le test était donc REDONDANT, pas + // prudent. Le retirer resserre le verdict sans toucher à sa correction. + // + // Columns are not tested: their only consumer gates on Sdf being near a cave surface, + // which cannot happen in a box no room/tunnel/pit/chimney reaches. The test was + // redundant rather than conservative, and it was the loosest one here. + const bool bReached = (B.HitRooms + B.HitTunnels + B.HitPits + B.HitChimneys) > 0; + B.Verdict = bReached ? EVoxelOpEffect::Both : EVoxelOpEffect::Identity; B.KeyBox = VoxelBox; B.KeyStrate = StrateIdx; @@ -3687,6 +3702,26 @@ namespace // ajoutée avec elle ne ferme rien → C2059. // END OF THE ANONYMOUS NAMESPACE — new operators go ABOVE this line. +//============================================================================= +// DIAGNOSTIC — voir la déclaration dans VoxelDensityOpStack.h +//============================================================================= + +VoxelDensityOps::FRoomBoxDiagnostic VoxelDensityOps::GetLastRoomBoxDiagnostic() +{ + const FRoomGraphSource::FBoxState& B = FRoomGraphSource::BoxState(); + + FRoomBoxDiagnostic D; + D.HitRooms = B.HitRooms; + D.HitTunnels = B.HitTunnels; + D.HitPits = B.HitPits; + D.HitChimneys = B.HitChimneys; + D.NumRooms = B.NumRooms; + D.NumTunnels = B.NumTunnels; + D.NumPits = B.NumPits; + D.NumChimneys = B.NumChimneys; + return D; +} + //============================================================================= // FVoxelOpStack //============================================================================= diff --git a/Source/VoxelForge/Public/VoxelDensityOpStack.h b/Source/VoxelForge/Public/VoxelDensityOpStack.h index ced5887..4dc73a0 100644 --- a/Source/VoxelForge/Public/VoxelDensityOpStack.h +++ b/Source/VoxelForge/Public/VoxelDensityOpStack.h @@ -330,6 +330,31 @@ namespace VoxelDensityOps int32 Seed, float SpineRadius, const UVoxelStrateManager* StrateManager); + /** + * DIAGNOSTIC — la ventilation par CLASSE DE PRIMITIVE du dernier `FRoomGraphSource::EffectOverBox` + * évalué sur ce thread. **Tests uniquement. N'entre dans aucune décision de génération.** + * + * ⚠️ POURQUOI ÇA EXISTE PLUTÔT QUE D'ÊTRE REFAIT DANS LE TEST. Le test a déjà tout ce qu'il faut + * pour rejouer le critère — il appelle `BuildChunkCache` ailleurs. Le rejouer serait une + * DEUXIÈME définition du critère, qui dériverait de la vraie et mentirait exactement le jour où + * on la croirait. C'est la même raison qui a fait exister `VF_BuildOpStackForChunk`. On expose + * donc ce que l'opérateur a réellement calculé. + * + * `Hit*` = combien de primitives de cette classe atteignent la boîte (0 partout ⇒ `Identity`). + * `Num*` = combien le cache en contenait, ce qui distingue « aucune n'atteint » de « il n'y en + * avait aucune » — deux zéros de sens opposé. + * + * Reads back what the operator actually computed, rather than letting the test re-derive the + * criterion: a second copy would drift and would lie on the day it was believed. + */ + struct FRoomBoxDiagnostic + { + int32 HitRooms = 0, HitTunnels = 0, HitPits = 0, HitChimneys = 0; + int32 NumRooms = 0, NumTunnels = 0, NumPits = 0, NumChimneys = 0; + }; + + VOXELFORGE_API FRoomBoxDiagnostic GetLastRoomBoxDiagnostic(); + /** * FloatingIslands — 7 ops, et **la pile tourne à l'ENVERS** : * ConstantVoid → IslandBlob → SdfRoughness → SdfFill → [structural post ×3]