From b7b9aaa9dda378ef384b5d0c3739e939d1da842a Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 24 Jul 2026 13:44:56 +0200 Subject: [PATCH] fix(render): eliminate EnvCell identity collisions --- .../retail-divergence-register.md | 2 +- docs/architecture/worldbuilder-inventory.md | 13 ++++ ...24-modern-runtime-slice-b-envcell-dedup.md | 36 +++++++---- .../Rendering/Wb/EnvCellLandblockBuild.cs | 4 +- .../Rendering/Wb/EnvCellRenderer.cs | 7 ++- .../Rendering/Wb/EnvCellGeometryIdentity.cs | 59 +++++++++++++++++-- .../Wb/EnvCellGeometryIdentityTests.cs | 31 ++++++++-- 7 files changed, 129 insertions(+), 23 deletions(-) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index cb555fc7..3d94b544 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -86,7 +86,7 @@ accepted-divergence entries (#96, #49, #50). | AD-21 | Null-clipRoot legacy outdoor safety path (no portal visibility, no punches/seals, no-clip terrain) for pre-spawn / login / legacy cameras; in-world retail always has a viewer_cell root | `src/AcDream.App/Rendering/WorldRenderFrameBuilder.cs` (`RuntimeWorldFrameRootSource`, `WorldRenderFrame.ClipRoot`); `src/AcDream.App/Rendering/WorldSceneRenderer.cs` (null-root safety draw) | Result is null ONLY when neither an interior root nor the synthetic outdoor node exists; kept so the login screen shows the live sky | If viewer-root resolution ever returns null in-world (membership bug, fly-camera edge), the frame silently degrades — interiors stop drawing through doorways; the old two-branch FLAP reappears for those frames | `SmartBox::RenderNormalMode` decomp:92635 | | AD-22 | Async streamed mesh loading with bounded CPU replay residency, per-frame upload budgets, and point-of-use self-heal (`EnsureLoaded` re-request in the dispatcher's mesh-missing path, **#128**); retail loads synchronously — geometry is never absent | `src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs`; `src/AcDream.App/Rendering/Wb/MeshUploadCaches.cs`; `src/AcDream.App/Rendering/Wb/MeshUploadFrameBudget.cs` | Immutable preparation descriptors and the bounded CPU cache can re-stage an evicted mesh; dispatcher self-heal makes absence transient while upload budgets prevent a portal arrival from monopolizing a frame | A future consumer that neither retains an owner nor reaches the self-heal/replay path can remain invisible; under heavy admission pressure a valid mesh can pop in later than retail's synchronous path | retail synchronous content load; `docs/architecture/worldbuilder-inventory.md` portal-readiness and bounded-residency seams | | AD-23 | Live entities with `ServerGuid != 0` and null `ParentCellId` are culled (ClipSlotCull) while indoor clip routing is active; retail objects are always cell-resident (synchronous add-to-cell at creation) | `src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs:484` | Phase U.4 policy: parentless = unresolved indoors, equivalent to retail's not-in-any-visible-cell ⇒ not drawn, *given membership resolves promptly* | An entity whose membership lags (late CreateObject hydration, resolver hiccup) blinks invisible while the player is indoors, even in plain sight | retail per-cell object lists in PView traversal | -| AD-24 | EnvCell shell geometry hash-deduplicated ((environmentId, structure, surface overrides) → 31-multiplier hash) and instanced; retail draws each CEnvCell's own structure directly | `src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs:276` | Verbatim WB EnvCellRenderManager port (Phase A8); dedup is what makes the single-VAO MDI cell pipeline cheap; intended visuals identical | A hash collision between distinct tuples renders the wrong interior shell in some room with NO diagnostic firing — wrong walls/floor in a dungeon room | retail `PView::DrawCells` → per-cell drawing_bsp (cited at :319) | +| AD-24 | EnvCell shell geometry content-deduplicated and instanced; retail draws each CEnvCell's own structure directly | `src/AcDream.Core/Rendering/Wb/EnvCellGeometryIdentity.cs` | Phase A8 retained WB's 31× hash; the 2026-07-24 full-DAT gate proved a real collision (`0x00030175`/`0x01BC0105`), so App+Bake now share a namespaced FNV-1a tuple identity and the bake rejects any full-tuple collision | A future collision outside the installed full-DAT gate could still merge different shells at runtime; the stronger 59-bit payload makes this extremely unlikely, and every bake fails loudly rather than publishing it | retail `PView::DrawCells` → per-cell drawing_bsp (cited at the former renderer `:319`) | | AD-25 | **REMOTE-DR sweep only** (the player half retired 2026-07-07 by the #182 verbatim rebuild): the remote dead-reckoning post-resolve still reflects velocity with the airborne-before-AND-after suppression; retail bounces unless grounded→grounded-and-not-sledding. The PLAYER path now runs the ported `handle_all_collisions` (`PhysicsObjUpdate`) with retail's `should_reflect` rule — the micro-bounce spiral it guarded is gone (contact is committed BEFORE the reflect and the small-velocity-zero is ungated) | `src/AcDream.App/Physics/RemotePhysicsUpdater.cs` (remote sweep post-resolve, #173 block) | The remote DR sweep hasn't been rebuilt yet (it has no fsf/SetPositionInternal chain); the old airborne-only suppression keeps remote landings from micro-bouncing on the remote landing-snap gate | Remote landing-reflection behavior (slope-landing momentum) won't reproduce; retire when the remote-DR sweep gets the same UpdateObjectInternal rebuild as the player | `handle_all_collisions` pc:282699-282715; ACE PhysicsObj.cs:2656-2721 | | AD-27 | Use/PickUp action fired on natural moveto completion via the `MoveToComplete` client-addition seam (retail's `CleanUpAndCallWeenie` contains no weenie call in this build and notifies nothing on arrival); retail sends the action once (server MoveToChain callback completes it). The companion `MoveToCancelled` seam only withdraws the waiting client presentation/action. | `src/AcDream.App/Interaction/SelectionInteractionController.cs` (`OnNaturalMoveToComplete`/`OnMoveToCancelled`); `src/AcDream.App/Rendering/GameWindow.cs` (player MoveTo seam binding); `src/AcDream.Core/Physics/Motion/MoveToManager.cs` (`MoveToComplete`/`MoveToCancelled`) | ACE's server-side chain may have timed out by the time our body arrives; the close-range deferred send hits ACE's WithinUseRadius fast-path. R4-V5 re-anchored from the deleted B.6 `AutoWalkArrived` event — same fires-on-arrival-only contract; cancellation/error only clears retained intent. | If the server's chain has NOT timed out, the action executes twice — door toggles open-then-closed, use-once interactions double-fire; protocol noise on non-ACE servers | ACE CreateMoveToChain / WithinUseRadius; `MoveToManager::CleanUpAndCallWeenie` 00529650 §7e (no weenie call) | | AD-28 | Chat transcript (`UiText`) and input (`UiChatInput`) are two separate widget classes placed inside their dat-authored container panels; retail's `ChatInterface` uses a single mode-flagged `UIElement_Text` (Type-12) that switches between read and edit mode | `src/AcDream.App/UI/Layout/ChatWindowController.cs:135` (transcript) + `:150` (input) | `UIElement_Text` is inside keystone.dll with no PDB/decomp; a two-widget split is functionally equivalent (read-only scroll, editable input) and is the structural adaptation required by our UiElement architecture | A future consumer expecting a single widget for both read/write (e.g. a plugin calling the chat API and getting one widget back) must be written to the two-widget contract | `UIElement_Text` (Type-12) @ keystone.dll; `gmMainChatUI::PostInit` @0x4ce130 | diff --git a/docs/architecture/worldbuilder-inventory.md b/docs/architecture/worldbuilder-inventory.md index 80652b41..4680da46 100644 --- a/docs/architecture/worldbuilder-inventory.md +++ b/docs/architecture/worldbuilder-inventory.md @@ -101,6 +101,19 @@ behavior change, no divergence-register row. running with no GL context, so baked pak output and live-client output stay byte-identical. +**MP1b EnvCell content identity correction (2026-07-24).** The extracted +WorldBuilder `EnvCellRenderManager.GetEnvCellGeomId` 31× polynomial is not a +safe unique resource key. A guarded full retail-DAT catalog found the concrete +collision `0x00030175` versus `0x01BC0105`: different environment/surface +tuples both map to `0x00000002020E8C13` and contain different polygons. +`AcDream.Core.Rendering.Wb.EnvCellGeometryIdentity` now owns one namespaced +FNV-1a identity shared by the App streaming build and `acdream-bake`; the +legacy calculation remains executable only for the conformance test that proves +the collision. The bake additionally compares the complete source tuple before +aliasing and fails on any collision. This is an acdream resource-ownership seam, +not a second DAT interpreter; `DatCollection` and `MeshExtractor` remain the +only reader/extractor path. + **Retail VFX hook compatibility seam (2026-07-14).** Chorizite.DatReaderWriter 2.1.7 models `CreateBlockingParticleHook` as the common hook header only, while retail inherits the complete `CreateParticleHook` payload. The narrow readers in diff --git a/docs/plans/2026-07-24-modern-runtime-slice-b-envcell-dedup.md b/docs/plans/2026-07-24-modern-runtime-slice-b-envcell-dedup.md index e7d43c14..04b2f1f3 100644 --- a/docs/plans/2026-07-24-modern-runtime-slice-b-envcell-dedup.md +++ b/docs/plans/2026-07-24-modern-runtime-slice-b-envcell-dedup.md @@ -21,11 +21,14 @@ bounded artifact whose size and time scale with unique geometry, not with all 1. `DatCollection` remains the only DAT reader. 2. `MeshExtractor` remains the only mesh/texture interpreter used by both the live path and the bake. -3. Geometry identity is the already-shipped WorldBuilder calculation: - `environmentId`, then `cellStructure`, then the ordered surface IDs through - the `hash = hash * 31 + value` chain, with bit 33 set. -4. The pure identity helper moves to `AcDream.Core.Rendering.Wb`; App and Bake - call the same implementation. No second or adapted hash is allowed. +3. Geometry identity is one deterministic hash of the complete typed tuple: + `environmentId`, `cellStructure`, surface count, then the ordered surface + IDs. App and Bake call the same Core implementation. +4. The pure identity helper lives in + `AcDream.Core.Rendering.Wb.EnvCellGeometryIdentity`. It uses FNV-1a in a + dedicated high-bit namespace and retains bit 33 for compatibility with + existing synthetic-geometry diagnostics. WorldBuilder's original + `hash = hash * 31 + value` calculation remains as a conformance helper only. 5. Every EnvCell TOC key is still `PakKey.Compose(PakAssetType.EnvCellMesh, fileId)`. 6. Aliases share `offset`, `length`, and `crc32`. The pak format and reader do @@ -42,16 +45,30 @@ bounded artifact whose size and time scale with unique geometry, not with all 11. The pak itself and raw bake logs are machine artifacts and are not committed. A compact, path-free bake report is committed. +**Full-DAT correction (2026-07-24):** the first guarded full-catalog run proved +that the original fixed contract was impossible. Installed retail DAT cells +`0x00030175` (`environment=0x277`, structure `0`, surfaces `0x013B,0x0034`) +and `0x01BC0105` (`environment=0x276`, structure `0`, surfaces +`0x04FC,0x0034`) both produce WorldBuilder ID `0x00000002020E8C13` despite +having visibly different geometry. The difference is algebraic: +one environment step contributes `31³`, exactly canceled by the first-surface +difference `0x04FC - 0x013B = 31²`. The collision guard stopped the bake before +writing an artifact. B1 was therefore corrected at the root: the old polynomial +is retained and tested as historical evidence, while runtime and bake now share +the stronger namespaced identity. The full-catalog collision gate remains +mandatory. + ## Implementation checkpoints ### B1 — One geometry identity seam -- Extract the pure WorldBuilder identity calculation from the App transaction - builder into `AcDream.Core.Rendering.Wb.EnvCellGeometryIdentity`. +- Extract one pure identity calculation from the App transaction builder into + `AcDream.Core.Rendering.Wb.EnvCellGeometryIdentity`. - Retain the existing App entry points as delegates so production behavior and callers do not change. -- Add Core conformance tests for empty surfaces, ordered surfaces, the bit-33 - namespace, and App/Bake shared values. +- Add Core conformance tests for empty surfaces, ordered surfaces, the + dedicated namespace, App/Bake shared values, and the installed-DAT collision + in the legacy WorldBuilder polynomial. **Gate:** existing App geometry-ID tests and new Core tests pass with the exact same values. @@ -133,4 +150,3 @@ dotnet test AcDream.slnx -c Release Then run the full bake from the Release binary, capture its compact report, and validate the output with `PakReader`. No visual gate is required because this slice changes no runtime path. - diff --git a/src/AcDream.App/Rendering/Wb/EnvCellLandblockBuild.cs b/src/AcDream.App/Rendering/Wb/EnvCellLandblockBuild.cs index 7ba5dde7..9a1c02a0 100644 --- a/src/AcDream.App/Rendering/Wb/EnvCellLandblockBuild.cs +++ b/src/AcDream.App/Rendering/Wb/EnvCellLandblockBuild.cs @@ -120,7 +120,9 @@ public sealed class EnvCellLandblockBuildBuilder } /// - /// Source: WB EnvCellRenderManager.cs:94-103 (verbatim arithmetic). + /// Collision-resistant successor to WB EnvCellRenderManager.cs:94-103. + /// The legacy polynomial and its installed-DAT collision remain covered by + /// Core conformance tests. /// public static ulong ComputeGeometryId( uint environmentId, diff --git a/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs b/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs index 36e7fd41..185c5001 100644 --- a/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs +++ b/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs @@ -4,7 +4,7 @@ // four reverted RR7 variants couldn't fix. // // Sources ported byte-for-byte: -// GetEnvCellGeomId <- WB EnvCellRenderManager.cs:94-103 +// GetEnvCellGeomId <- collision-resistant successor to WB's content key // PrepareRenderBatches <- WB EnvCellRenderManager.cs:247-373 // Render(filter:) <- WB EnvCellRenderManager.cs:395-511 // RenderModernMDIInternal <- WB BaseObjectRenderManager.cs:709-848 (single-slot variant) @@ -353,14 +353,15 @@ public sealed unsafe class EnvCellRenderer : IDisposable // --------------------------------------------------------------------------- // GetEnvCellGeomId - // Verbatim copy of WB EnvCellRenderManager.cs:94-103. + // Shared collision-resistant content identity. Core retains WB's original + // polynomial for conformance evidence and the installed-DAT collision test. // --------------------------------------------------------------------------- /// /// Returns a deduplicated geometry ID for an EnvCell based on its environment, /// cell structure index, and surface IDs. Bit 33 is set to distinguish from /// per-cell IDs (which use bit 32). - /// Source: WB EnvCellRenderManager.cs:94-103 (verbatim). + /// See . /// public static ulong GetEnvCellGeomId(uint environmentId, ushort cellStructure, List surfaces) => EnvCellLandblockBuildBuilder.ComputeGeometryId( diff --git a/src/AcDream.Core/Rendering/Wb/EnvCellGeometryIdentity.cs b/src/AcDream.Core/Rendering/Wb/EnvCellGeometryIdentity.cs index f03ba64d..3f20c6eb 100644 --- a/src/AcDream.Core/Rendering/Wb/EnvCellGeometryIdentity.cs +++ b/src/AcDream.Core/Rendering/Wb/EnvCellGeometryIdentity.cs @@ -3,21 +3,52 @@ using System.Collections.Generic; namespace AcDream.Core.Rendering.Wb; /// -/// Computes WorldBuilder's content identity for an EnvCell shell. +/// Computes acdream's collision-resistant content identity for an EnvCell shell. /// Equal environment, cell-structure, and ordered surface tuples share one /// prepared geometry payload even when they occur under different cell IDs. /// public static class EnvCellGeometryIdentity { + private const ulong FnvOffsetBasis = 14695981039346656037UL; + private const ulong FnvPrime = 1099511628211UL; + private const ulong PayloadMask = 0x0FFF_FFFD_FFFF_FFFFUL; + private const ulong Namespace = 0xE000_0002_0000_0000UL; + /// - /// Source: WorldBuilder EnvCellRenderManager.GetEnvCellGeomId, - /// lines 94-103. Bit 33 separates content identities from per-cell IDs, - /// whose synthetic geometry requests use bit 32. + /// Hashes the complete typed tuple with FNV-1a and places it in a dedicated + /// high-bit namespace. Bit 33 remains set for compatibility with existing + /// synthetic-geometry diagnostics; the top nibble prevents overlap with + /// DAT IDs and older low-bit synthetic IDs. + /// + /// WorldBuilder's original 31× polynomial is retained in + /// for conformance evidence, but it + /// is not a valid resource identity: installed retail DAT cells 0x00030175 + /// and 0x01BC0105 produce the same value for different geometry tuples. /// public static ulong Compute( uint environmentId, ushort cellStructure, IReadOnlyList surfaces) + { + ulong hash = FnvOffsetBasis; + AddUInt32(ref hash, environmentId); + AddUInt16(ref hash, cellStructure); + AddUInt32(ref hash, checked((uint)surfaces.Count)); + for (int i = 0; i < surfaces.Count; i++) + AddUInt16(ref hash, surfaces[i]); + + return (hash & PayloadMask) | Namespace; + } + + /// + /// Verbatim WorldBuilder EnvCellRenderManager.GetEnvCellGeomId + /// lines 94-103. Kept only to demonstrate and regression-test the real DAT + /// collision that required the stronger internal identity. + /// + public static ulong ComputeLegacyWorldBuilder( + uint environmentId, + ushort cellStructure, + IReadOnlyList surfaces) { long hash = 17; hash = unchecked(hash * 31 + environmentId); @@ -26,4 +57,24 @@ public static class EnvCellGeometryIdentity hash = unchecked(hash * 31 + surfaces[i]); return (ulong)hash | 0x2_0000_0000UL; } + + private static void AddUInt16(ref ulong hash, ushort value) + { + AddByte(ref hash, (byte)value); + AddByte(ref hash, (byte)(value >> 8)); + } + + private static void AddUInt32(ref ulong hash, uint value) + { + AddByte(ref hash, (byte)value); + AddByte(ref hash, (byte)(value >> 8)); + AddByte(ref hash, (byte)(value >> 16)); + AddByte(ref hash, (byte)(value >> 24)); + } + + private static void AddByte(ref ulong hash, byte value) + { + hash ^= value; + hash = unchecked(hash * FnvPrime); + } } diff --git a/tests/AcDream.Core.Tests/Rendering/Wb/EnvCellGeometryIdentityTests.cs b/tests/AcDream.Core.Tests/Rendering/Wb/EnvCellGeometryIdentityTests.cs index ebb4c10a..b75df724 100644 --- a/tests/AcDream.Core.Tests/Rendering/Wb/EnvCellGeometryIdentityTests.cs +++ b/tests/AcDream.Core.Tests/Rendering/Wb/EnvCellGeometryIdentityTests.cs @@ -5,18 +5,18 @@ namespace AcDream.Core.Tests.Rendering.Wb; public sealed class EnvCellGeometryIdentityTests { [Fact] - public void Compute_EmptySurfaces_MatchesExtractedWorldBuilderValue() + public void Compute_EmptySurfaces_IsStableGoldenValue() { Assert.Equal( - 0x2_0000_47D6UL, + 0xEC48_CD4F_7A03_5AD0UL, EnvCellGeometryIdentity.Compute(0x42, 7, Array.Empty())); } [Fact] - public void Compute_OrderedSurfaces_MatchesExtractedWorldBuilderValue() + public void Compute_OrderedSurfaces_IsStableGoldenValue() { Assert.Equal( - 0x2_20A7_A46CUL, + 0xEE36_FCB2_C067_E94BUL, EnvCellGeometryIdentity.Compute(0x42, 7, new ushort[] { 1, 2, 3 })); } @@ -38,5 +38,28 @@ public sealed class EnvCellGeometryIdentityTests new ushort[] { ushort.MaxValue, ushort.MaxValue }); Assert.NotEqual(0UL, id & 0x2_0000_0000UL); + Assert.Equal(0xE000_0000_0000_0000UL, id & 0xF000_0000_0000_0000UL); + } + + [Fact] + public void LegacyWorldBuilderCollision_InstalledDatTuples_GetDistinctModernIds() + { + ushort[] firstSurfaces = [0x013B, 0x0034]; + ushort[] secondSurfaces = [0x04FC, 0x0034]; + + ulong firstLegacy = EnvCellGeometryIdentity.ComputeLegacyWorldBuilder( + 0x0277, + 0, + firstSurfaces); + ulong secondLegacy = EnvCellGeometryIdentity.ComputeLegacyWorldBuilder( + 0x0276, + 0, + secondSurfaces); + + Assert.Equal(0x2_020E_8C13UL, firstLegacy); + Assert.Equal(firstLegacy, secondLegacy); + Assert.NotEqual( + EnvCellGeometryIdentity.Compute(0x0277, 0, firstSurfaces), + EnvCellGeometryIdentity.Compute(0x0276, 0, secondSurfaces)); } }