From f7ed9407bf76c0f2c9aab77bc7a1f9b38a0469b3 Mon Sep 17 00:00:00 2001 From: Fr0zka Date: Mon, 27 Jul 2026 16:27:26 +0200 Subject: [PATCH] feat: wire SurfaceWorld (biome-less) + fix a column-memo perf trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All six checks green, 9399 samples inside the overhang window. The single-entry column memo was correct only if the caller walks a Z column before changing XY, which the mesher does not promise. Iterating X first within a Z slice would miss on every voxel and re-run the whole height stack per voxel, cliff resamples included — an order of magnitude on the plugin's most expensive archetype. The tests could not have caught it: they sample random XY, where a one-entry memo and a 256-entry one behave identically. Only reading the access pattern finds this. Replaced with a direct-mapped 256-entry thread_local table hashed on the XY bit patterns, full key compared on hit, so a collision costs a recompute and never returns the wrong column. Wiring: UsesOperatorStackForChunk returns true for SurfaceWorld only when the strate has no biomes. The original blends heights toward the neighbouring biome across the border band; the stack evaluates one param set, so a biome strate would get a hard seam at every border rather than a subtle shift. The guard sits beside the archetype list so "can this strate take the stack?" stays one question in one place, and GetDensityAt keeps a defensive CP_BiomeCtx check that falls back if the two disagree. UNVERIFIED: not compiled. Co-Authored-By: Claude Opus 5 --- OPSTACK-PROGRESS.md | 47 +++++++++++++++++++ .../Private/VoxelDensityOpStack.cpp | 30 +++++++++--- Source/VoxelForge/Private/VoxelGenerator.cpp | 20 ++++++++ .../VoxelForge/Private/VoxelStrateManager.cpp | 16 +++++++ 4 files changed, 107 insertions(+), 6 deletions(-) diff --git a/OPSTACK-PROGRESS.md b/OPSTACK-PROGRESS.md index e935771..e2a4f5a 100644 --- a/OPSTACK-PROGRESS.md +++ b/OPSTACK-PROGRESS.md @@ -1382,3 +1382,50 @@ keeping a raw pointer; and the two newly-public generator methods. `GSurfColCache`. `§C1` still open. --- + +## 2026-07-27 — SurfaceWorld WIRED (no biomes yet). And a perf trap caught before it shipped. + +All six checks green, **9399 samples deliberately inside the overhang window** — the op was genuinely +exercised rather than skipped. + +### ⚠️ The one-entry column memo would have been a disaster, not a slowdown + +`FSurfaceColumnSource`'s memo held a single entry. That is correct **only if the caller walks a whole +Z column before changing XY** — and the mesher promises nothing of the sort. If it iterates X first +within a Z slice, *every* voxel misses and the full height stack re-runs per voxel, **including the +cliff's four structural resamples**. On the most expensive archetype in the plugin that is an order +of magnitude, not a few percent. + +It would also have been invisible in the tests: they sample random XY, where a one-entry memo and a +256-entry one behave identically. **The tests could not have caught this; only reading the access +pattern could.** + +Replaced with a **direct-mapped 256-entry table**, `thread_local`, hashed on the XY bit patterns, +with the **full key compared on hit** — a collision can only cost a recompute, never return the +wrong column. Robust to any iteration order the mesher chooses. + +### Wired, with the biome guard in one place + +`UsesOperatorStackForChunk` now returns true for `SurfaceWorld` **only when the strate has no +biomes**. The original evaluates the dominant biome and interpolates the *heights* toward the +neighbour across the border band; the stack evaluates one param set. Without the `Mask` combiner a +biome strate would not shift subtly — it would get **a hard seam at every biome border**. + +The guard lives in `UsesOperatorStackForChunk`, beside the archetype list, so "can this strate take +the stack?" stays one question asked in one place. `GetDensityAt` carries a second, defensive check +on `CP_BiomeCtx.IsValid()`: if the two ever disagree it falls back to the `switch`, because an +unported world is recoverable and a wrong one is not. + +**UNVERIFIED:** not compiled. Likely spots: the `FSlot` struct + `thread_local` array inside a const +method; `return S.C` (the previous `return C` referred to a name now scoped inside the `if`); the +new `SurfaceWorld` case in `GetDensityAt`'s op-stack switch. + +**What to try in the editor after the build:** tick `bUseOperatorStack` on a **biome-less** +SurfaceWorld strate and compare. Both paths compute the same function, so this is a wiring check, +not a look change — expect it identical. **A biome strate will silently ignore the flag**, by design. + +**Next single action:** build, then the visual A/B. After that the remaining SurfaceWorld work is +the `Mask` combiner (biome blending, §5's Phase 3 prototype) and integrating `GSurfColCache` so the +stack path reuses the existing box cache rather than only its own table. `§C1` still open. + +--- diff --git a/Source/VoxelForge/Private/VoxelDensityOpStack.cpp b/Source/VoxelForge/Private/VoxelDensityOpStack.cpp index 77b26f4..22627db 100644 --- a/Source/VoxelForge/Private/VoxelDensityOpStack.cpp +++ b/Source/VoxelForge/Private/VoxelDensityOpStack.cpp @@ -449,13 +449,31 @@ namespace const FColumn& GetColumn(float WorldX, float WorldY) const { - thread_local FColumn C{}; - thread_local uint64 CachedId = 0; - thread_local float CachedX = FLT_MAX, CachedY = FLT_MAX; + // ⚠️ POURQUOI UNE TABLE ET PAS UNE SEULE ENTRÉE. Un mémo à une entrée n'est correct que + // si l'appelant descend une colonne Z avant de changer de XY. Le mesher n'en promet + // RIEN — s'il itère X en premier dans une tranche Z, chaque voxel raterait et on + // relancerait toute la pile de hauteur par voxel, cliff compris (4 resamples + // structurels). Ce n'est pas « un peu plus lent », c'est un ordre de grandeur sur + // l'archétype le plus cher du plugin. + // + // Table à correspondance directe, 256 entrées, clé COMPLÈTE comparée sur touche : une + // collision ne peut que coûter un recalcul, jamais rendre une mauvaise colonne. + // + // A single-entry memo is only correct-by-luck: it assumes the caller walks a Z column + // before changing XY, which the mesher does not promise. Direct-mapped 256-entry table + // with the FULL key compared on hit — a collision costs a recompute, never a wrong column. + struct FSlot { uint64 Id; float X, Y; FColumn C; }; + thread_local FSlot Slots[256] = {}; - if (CachedId != InstanceId || CachedX != WorldX || CachedY != WorldY) + const uint32 HX = *reinterpret_cast(&WorldX); + const uint32 HY = *reinterpret_cast(&WorldY); + const uint32 Idx = (HX * 0x9E3779B9u ^ HY * 0x85EBCA6Bu) >> 24; // [0,255] + + FSlot& S = Slots[Idx]; + if (S.Id != InstanceId || S.X != WorldX || S.Y != WorldY) { - CachedId = InstanceId; CachedX = WorldX; CachedY = WorldY; + S.Id = InstanceId; S.X = WorldX; S.Y = WorldY; + FColumn& C = S.C; C.TerrainZ = TerrainStack.EvalHeight(WorldX, WorldY); C.CeilSurf = CeilingStack.EvalHeight(WorldX, WorldY); @@ -482,7 +500,7 @@ namespace if (Slope > KINDA_SMALL_NUMBER) { C.DirX = GX / Slope; C.DirY = GY / Slope; } } } - return C; + return S.C; } /** Le champ structurel nu — l'overhang s'en sert pour emprunter la roche amont. */ diff --git a/Source/VoxelForge/Private/VoxelGenerator.cpp b/Source/VoxelForge/Private/VoxelGenerator.cpp index 0dcec96..09ddacd 100644 --- a/Source/VoxelForge/Private/VoxelGenerator.cpp +++ b/Source/VoxelForge/Private/VoxelGenerator.cpp @@ -578,6 +578,26 @@ float UVoxelGenerator::GetDensityAt(float WorldX, float WorldY, float WorldZ) co VoxelDensityOps::BuildSlabStack(CP_OpStack, CP_Slab, Seed, OriginSpineRadius, StrateManager); break; + + case ECaveGeneratorType::SurfaceWorld: + // Même garde dégénérée que les autres, et une SECONDE garde : la pile ne sait + // pas encore mélanger les biomes. `UsesOperatorStackForChunk` refuse déjà les + // strates à biomes, mais un `CP_BiomeCtx` valide ici voudrait dire que les deux + // sources d'information se contredisent — auquel cas on retombe sur le `switch`, + // parce qu'un monde non porté est récupérable et un monde faux ne l'est pas. + // A valid biome context here would mean the two sources of truth disagree; fall + // back rather than generate a world with seams at every biome border. + if (CP_Surface.StrateTopWorldZ - CP_Surface.StrateBottomWorldZ <= 0.0f + || CP_BiomeCtx.IsValid()) + { + CP_UseOpStack = false; + break; + } + OpCtx.StrateTopWorldZ = CP_Surface.StrateTopWorldZ; + OpCtx.StrateBottomWorldZ = CP_Surface.StrateBottomWorldZ; + VoxelDensityOps::BuildSurfaceStack(CP_OpStack, CP_Surface, Seed, + OriginSpineRadius, StrateManager); + break; default: // UsesOperatorStackForChunk ne rend true que pour les archétypes portés, donc // on ne devrait jamais arriver ici. Si ça arrive, retomber sur le `switch` diff --git a/Source/VoxelForge/Private/VoxelStrateManager.cpp b/Source/VoxelForge/Private/VoxelStrateManager.cpp index 8faadb0..774a9bd 100644 --- a/Source/VoxelForge/Private/VoxelStrateManager.cpp +++ b/Source/VoxelForge/Private/VoxelStrateManager.cpp @@ -574,6 +574,22 @@ bool UVoxelStrateManager::UsesOperatorStackForChunk(const FIntVector& ChunkCoord case ECaveGeneratorType::Maze: return true; // Phase 1 case ECaveGeneratorType::FlatPlain: // Phase 2 — les deux partagent case ECaveGeneratorType::CrystalChamber: return true; // UNE seule pile (BuildSlabStack) + + case ECaveGeneratorType::SurfaceWorld: + // ⚠️ PORTÉ, MAIS PAS AVEC LES BIOMES. `BuildSurfaceStack` évalue UN jeu de params ; le + // chemin d'origine évalue le biome dominant puis interpole les HAUTEURS vers le voisin + // dans la bande de frontière (le combiner `Mask`, prototype de la Phase 3 — §5). Sans lui, + // une strate à biomes perdrait ses transitions : pas un décalage subtil, une couture nette + // à chaque frontière de biome. + // + // Donc la garde est ici, dans la MÊME fonction que la liste des archétypes portés, plutôt + // que dispersée dans `GetDensityAt` : « cette strate peut-elle prendre la pile ? » reste + // une seule question, posée à un seul endroit. + // Ported, but NOT with biomes: the stack evaluates ONE param set, while the original blends + // heights toward the neighbouring biome. Without the Mask combiner a biome strate would lose + // its transitions — a hard seam at every biome border, not a subtle shift. + return Def->Biomes.Num() == 0; + default: return false; } }