From e2543d0ef043e242068eaa2b495d9edf9582ba0f Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 2 Sep 2026 19:37:25 +0200 Subject: [PATCH] fix(content): S1 review round - retail default sides shape, mask hoist, upload order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Campaign OVERHAUL S1 review fixes (two Opus lens reviews, findings verified by the lead against the decomp): - a raw sides_type outside 0/1/2 constructs retail's default single-side shape (ConstructMesh @0x0059DFA0 loop bounds default to 1) instead of dropping the polygon; still counted as a data anomaly (corpus has none); - the positive-surface stippling mask OR runs once per polygon before the degenerate-fan guard, as retail's count loop does (pseudo-C 426859-426866); - untextured slots keep their mask accounting but bake no texture and no vertices (contract §9 item 3); the dev pak shrinks by 114 KB; - failed surface-override / Surface / texture-dependency lookups are attempted and logged once per slot, not once per candidate; - cell-shell batches upload in ascending source surface index, retail's built-EnvCell subset draw order (ConstructMesh attribute-range scan, DrawMesh @0x0059D4A0); ordinary GfxObj meshes keep storage order; - CellMesh.HasDrawableGeometry documented as the admission rule without texture-dependency resolution (a conservative superset of emission); - the stippling/surface equivalence sweep's cell half is pinned at zero again; InAscendingSurfaceOrder is marked bake/upload/test-only; - plan §5: reviewer findings are verified by the lead, one skeptic at most for a blocking finding, never more than five agents per step. Three new Content tests pin the mask hoist, the vertex-free untextured slot, and the single-side fallback. Content 213/213, Core Meshing and Conformance green, App hermetic 6,757/6,757, Release build 0/0. Co-Authored-By: Claude Fable 5.1 --- ...-09-01-campaign-overhaul-world-solidity.md | 9 +- .../Rendering/Wb/ObjectMeshManager.cs | 52 ++++++-- src/AcDream.Content/MeshExtractor.cs | 112 ++++++++++++------ src/AcDream.Content/ObjectMeshData.cs | 5 + src/AcDream.Core/Meshing/CellMesh.cs | 16 ++- .../Meshing/CellStructSideCandidates.cs | 29 +++-- ...uctSurfaceConstructionInstalledDatTests.cs | 10 +- .../MeshExtractorSolidFaceExtractionTests.cs | 85 +++++++++++++ .../StipplingSurfaceEquivalenceTests.cs | 8 +- .../Meshing/CellStructSideCandidatesTests.cs | 12 +- 10 files changed, 265 insertions(+), 73 deletions(-) diff --git a/docs/plans/2026-09-01-campaign-overhaul-world-solidity.md b/docs/plans/2026-09-01-campaign-overhaul-world-solidity.md index d4762275..61af5276 100644 --- a/docs/plans/2026-09-01-campaign-overhaul-world-solidity.md +++ b/docs/plans/2026-09-01-campaign-overhaul-world-solidity.md @@ -167,7 +167,12 @@ keeps the lead in the loop at every chunk. | Implementer | Sonnet, one dispatch per chunk | one file cluster + its tests; build and test green is the return condition; returns a diff summary and open questions | commits; adds evidence infrastructure; starts a third fix round; touches files outside its contract | | Retail reviewer | Opus, one pass per slice on the production port | checks each ported branch against the named decomp with addresses; findings as file:line + decomp citation | reviews tooling | | Architecture reviewer | Opus, only at the two cutover slices (S2, S5) | one owner per fact; layering; lifetime; performance shape | | -| Adversarial verify | Workflow fan-out, review stage only | three skeptics per finding try to refute it; a finding survives with two of three | | +| Finding verification | the lead, against the source | each reviewer finding is checked by the lead against the decomp/contract/code before it is acted on; at most one Opus skeptic, and only for a blocking finding the lead cannot settle | spawns a skeptic per finding (the S1 review fanned out to ~30 agents this way on 2026-09-02; never again) | + +**Agent budget per slice:** implementers as needed (one per chunk, serial for +coupled files); reviewers two (retail lens, and the architecture lens only at +S2/S5); skeptics at most one per blocking finding. Anything that would spawn +more than five agents in one step is wrong and stops. Chunk contracts state explicit stop conditions: build red after two attempts → return; contract ambiguity → return with the question; any need for a third @@ -620,7 +625,7 @@ Update immediately when a slice changes state. Chat is not the ledger. |---|---|---|---|---|---|---|---| | v1 OH0 | CLOSED | `oh0-baseline.md` | `b3b7d922` | read-only classification | Release build green; 319 focused / 1 skip | n/a | historical; candidates it committed are S1/S2 seeds | | v1 OH1 | SUPERSEDED | research contracts committed at `5cd4fd2c`/`5d907ae9` (kept, binding) | T0–T3 evidence grammar parked on `quarantine/oh1-evidence-grammar-2026-09-02` | reviews found the grammar a false oracle three times | n/a | n/a | see §15 | -| S1 Geometry | IN PROGRESS | `oh2-cellstruct-surface-contract.md` (§3.5 arbitrated on the binary 2026-09-02) | chunk A landed `acf17246` (descriptor, extraction, record, serializer, recipe 8); chunk B (CellMesh unification, installed-DAT scan, canonical pins, AP-234, docs) in progress | lead verified chunk A against the named pseudo-C and the paired binary; one inexact absent-UV rule found and corrected before landing; retail review of the whole slice pending | chunk A: Core 32 descriptor tests, Content 170/170, Bake 18/18, Launcher.Core 365/365 (Lane!=Linux), Release solution build 0/0 | G1 pending | recipe 7 → 8; dev pak rebake queued | +| S1 Geometry | IMPLEMENTED + REVIEWED — G1 PENDING | `oh2-cellstruct-surface-contract.md` (§3.5 arbitrated on the binary 2026-09-02) | chunk A `acf17246`; chunk B `b681717c`; hygiene `0840d5fb`; review-fix round (see notes) | lead verified both chunks against the named pseudo-C and the paired binary (absent-UV rule corrected before landing); two Opus lens reviews (retail, data/architecture) returned 8 distinct anchored findings, all verified by the lead and fixed: anomalous sides value now takes retail's default single-side shape instead of dropping; per-polygon mask OR hoisted before the degenerate guard; untextured slots no longer bake vertices; failed slot/texture lookups memoized once per slot; cell-shell upload order is ascending source surface index; HasDrawableGeometry doc corrected; equivalence sweep cell half pinned at zero; InAscendingSurfaceOrder marked bake/upload/test-only | build 0/0; Content 213/213 (Lane!=Timing incl. PreparedPackage on the recipe-8 pak); Core 4,947/4,948 with the one failure `CollisionShadowVerifierTests.DisabledGraphTraversal_AllocatesZeroBytes` (allocation measurement outside S1 files; passed in the previous full run, rerun in isolation below); Bake 21/21; Launcher.Core 365/365 (Lane!=Linux); Runtime 1,884/1,884; App hermetic 6,757/6,757 | G1 pending | dev pak rebaked twice to recipe 8 (final 597,369,112 B, 0 failures, `acdream.recipe7.pak` kept); TOC 2,237,866 → 2,237,865 explained (one unreferenced solid-color texture payload; all 729,888 cell mesh keys present); corpus scan 5,346 landblocks / 729,888 cells / 8,601,560 polygons, old-only 3,197 (all untextured), new-only 0, unexplained 0, anomalous sides 0 | | Capture | READY | §7 + `tools/walk-oracle/oh/README.md` | scripts committed `ebaa41df` + `14d8fe64`; lead resolved DBObj::m_DID=+0x28 and the cdecl alpha-function conventions; recon attach re-confirms | — | recon dt/x/uf dumps at session start | owner session ~1 h | before S3 | | S2 World graph | PLANNED | `oh1-construction-landscape-contract.md` | — | — | — | G2 | seeds in `b3b7d922` | | S3 Walk | PLANNED | built-mesh/world contracts + captures | — | — | — | folded into G3 | | diff --git a/src/AcDream.App/Rendering/Wb/ObjectMeshManager.cs b/src/AcDream.App/Rendering/Wb/ObjectMeshManager.cs index 2d4b3edb..c3a0e74a 100644 --- a/src/AcDream.App/Rendering/Wb/ObjectMeshManager.cs +++ b/src/AcDream.App/Rendering/Wb/ObjectMeshManager.cs @@ -2041,6 +2041,34 @@ namespace AcDream.App.Rendering.Wb #region Private: GPU Upload + /// + /// Upload-time batch order for one prepared mesh. Ordinary GfxObj + /// meshes keep the (Width,Height,Format) storage grouping. A cell-shell + /// mesh (any batch with ) is + /// ordered by ascending , + /// retail's built-EnvCell subset draw order (contract §3.6: + /// D3DPolyRender::ConstructMesh @0x0059DFA0 emits one attribute + /// range per nonempty surface index ascending, and + /// DrawMesh @0x0059D4A0 draws them in that order). Runs once per + /// mesh upload; the allocation is not per-frame. + /// + private static IEnumerable<((int Width, int Height, TextureFormat Format) Format, TextureBatchData Batch)> OrderedUploadBatches(ObjectMeshData meshData) + { + var pairs = new List<((int Width, int Height, TextureFormat Format), TextureBatchData)>(); + bool cellShell = false; + foreach (var (format, batches) in meshData.TextureBatches) + { + foreach (var batch in batches) + { + pairs.Add((format, batch)); + cellShell |= batch.IsCellShell; + } + } + if (cellShell) + return pairs.OrderBy(p => p.Item2.SourceSurfaceIndex); // stable: OrderBy preserves storage order on ties + return pairs; + } + private ObjectRenderData? UploadGfxObjMeshData(ObjectMeshData meshData) { if (meshData.Vertices.Length == 0) return null; @@ -2111,20 +2139,18 @@ namespace AcDream.App.Rendering.Wb } // OH2/S1 (docs/research/2026-09-01-overhaul/oh2-cellstruct-surface-contract.md - // §3.6 point 5): for a cell-shell mesh (batch.IsCellShell true), - // this dictionary walk groups by (Width,Height,Format) for - // atlas/storage packing only — it is NOT retail's ascending - // source-surface-index subset draw order. The resulting - // `renderBatches` list order is what EnvCellRenderer later - // replays as draw order. Recovering the exact retail order - // requires walking AcDream.Content.CellSurfaceSubsets. - // InAscendingSurfaceOrder(meshData) instead; no consumer does - // that yet (OH7's ordered draw stream is the intended owner) so - // this upload path still does not preserve retail's subset - // order for cell shells. - foreach (var (format, batches) in meshData.TextureBatches) + // §3.6): TextureBatches groups by (Width,Height,Format) for + // atlas/storage packing only. For a cell-shell mesh the + // `renderBatches` list order is what EnvCellRenderer replays + // as subset draw order, and retail draws built-EnvCell + // subsets in ascending source surface-array index + // (ConstructMesh @0x0059DFA0 attribute-range scan, DrawMesh + // @0x0059D4A0 ascending subset loop). So cell-shell batches + // are uploaded in that order; ordinary GfxObj meshes keep the + // storage order. This runs once per mesh at upload time, not + // per frame. + foreach (var (format, batch) in OrderedUploadBatches(meshData)) { - foreach (var batch in batches) { if (batch.Indices.Count == 0) continue; diff --git a/src/AcDream.Content/MeshExtractor.cs b/src/AcDream.Content/MeshExtractor.cs index e7cdc9b5..59f7e8d1 100644 --- a/src/AcDream.Content/MeshExtractor.cs +++ b/src/AcDream.Content/MeshExtractor.cs @@ -753,6 +753,25 @@ public sealed class MeshExtractor { public Surface Surface { get; } public uint SurfaceId { get; } + /// + /// Retail's built-EnvCell admission decided at slot creation + /// (RenderDeviceD3D::DrawEnvCell @0x0059F170 → + /// D3DPolyRender::DrawMesh(..., arg4=1) @0x0059D4A0 skips a + /// subset unless (Surface.Type & 6) != 0, contract §4). + /// An untextured slot still receives its mask accounting but no + /// texture decode and no vertices: retail constructs geometry it + /// never draws, and the contract (§9 item 3) allows production not + /// to serialize vertices that can never draw. + /// + public bool IsUntextured => RetailUntexturedSurfacePolicy.IsUntextured(Surface.Type); + + /// + /// Set the first time ResolveSlotBatch runs for this slot, so a + /// failed texture-dependency lookup is attempted and logged exactly + /// once per slot rather than once per candidate (S1 review finding). + /// + public bool BatchResolutionAttempted; + /// /// Retail's isStippledOrAlphaedMask byte for this slot: /// once, @@ -807,7 +826,23 @@ public sealed class MeshExtractor { // "negativeLane") is a sufficient, exact lane key. var vertexLookup = new Dictionary<(ushort vertId, ushort uvIdx, bool negativeLane), ushort>(); var batchesByFormat = new Dictionary<(int Width, int Height, TextureFormat Format), List>(); - var slots = new Dictionary(); + // null value = the slot's surface override or Surface record failed + // to resolve; cached so the lookup and its diagnostic happen once + // per slot, not once per candidate (S1 review finding). + var slots = new Dictionary(); + + CellSurfaceSlot? GetOrCreateSlot(int slot) { + if (slots.TryGetValue(slot, out var existing)) + return existing; + CellSurfaceSlot? created = null; + if (TryResolveSlot(slot, out var surface, out var surfaceId)) { + created = new CellSurfaceSlot(surface, surfaceId) { + Mask = CellStructSideCandidates.InitialSurfaceMask(surface.Type), + }; + } + slots[slot] = created; + return created; + } var min = new Vector3(float.MaxValue); var max = new Vector3(float.MinValue); @@ -848,6 +883,7 @@ public sealed class MeshExtractor { // re-implementing the retail SIDE/SUBSET algorithm twice; the DAT // texture-decode plumbing itself is unrelated to that rule). void ResolveSlotBatch(CellSurfaceSlot state) { + state.BatchResolutionAttempted = true; Surface surface = state.Surface; uint surfaceId = state.SurfaceId; @@ -1044,6 +1080,18 @@ public sealed class MeshExtractor { foreach (var poly in cellStruct.Polygons.Values) { ct.ThrowIfCancellationRequested(); + + // Contract §3.2 (ConstructMesh count loop, pseudo-C 426859-426866, + // 0x0059E1B3-0x0059E1CD): the positive-surface stippling OR runs + // ONCE PER POLYGON, before and independent of the num_pts and + // sides_type branches. It therefore runs here even for a + // degenerate fan or an anomalous sides value (S1 review finding). + if (poly.PosSurface >= 0 + && GetOrCreateSlot(poly.PosSurface) is { } positiveSlot) { + positiveSlot.Mask = CellStructSideCandidates.ApplyStipplingMaskBit( + positiveSlot.Mask, CellStructPolygonSurfaceSide.Positive, poly.Stippling); + } + if (poly.VertexIds.Count < 3) continue; // OH2/S1 chunk-1: side candidates come ONLY from sides_type @@ -1052,43 +1100,34 @@ public sealed class MeshExtractor { // sides_type integer (0/1/2), not a GPU cull enum — see // CellStructSideCandidates' own remarks. NoPos/NoNeg (below, // IsUvAbsent) govern UV-array absence only (§3.5) and no - // longer suppress a candidate. - ReadOnlySpan candidates = - CellStructSideCandidates.GetCandidates((int)poly.SidesType); - if (candidates.Length == 0) { - // Contract §3.4 closing paragraph: only 0/1/2 are - // retail-defined. Quarantine/report, don't invent a shape. + // longer suppress a candidate. A raw value outside 0/1/2 takes + // retail's default single-side shape (the loop bounds default to + // 1) and is only counted as a data anomaly. + int rawSidesType = (int)poly.SidesType; + if (!CellStructSideCandidates.IsRetailDefinedSidesType(rawSidesType)) unknownSidesTypePolygons++; - continue; - } + ReadOnlySpan candidates = + CellStructSideCandidates.GetCandidates(rawSidesType); foreach (var candidate in candidates) { short surfaceIdxRaw = candidate.SurfaceSlot == CellStructPolygonSurfaceSide.Positive ? poly.PosSurface : poly.NegSurface; if (surfaceIdxRaw < 0) continue; - int slot = surfaceIdxRaw; - if (!slots.TryGetValue(slot, out var slotState)) { - if (!TryResolveSlot(slot, out var surface, out var surfaceId)) continue; - slotState = new CellSurfaceSlot(surface, surfaceId) { - Mask = CellStructSideCandidates.InitialSurfaceMask(surface.Type), - }; - slots[slot] = slotState; - } + var slotState = GetOrCreateSlot(surfaceIdxRaw); + if (slotState is null) continue; // override/Surface lookup failed; logged once per slot. - // Contract §3.2: the per-polygon mask OR always targets the - // POSITIVE surface slot regardless of which specific - // stippling bits are set — ApplyStipplingMaskBit already - // encodes that via candidate.SurfaceSlot (a no-op when this - // candidate's SurfaceSlot is Negative). - slotState.Mask = CellStructSideCandidates.ApplyStipplingMaskBit( - slotState.Mask, candidate.SurfaceSlot, poly.Stippling); + // Contract §4 admission is a per-slot fact known here: an + // untextured slot keeps its mask accounting (above) but gets + // no texture decode and no vertices, since retail never + // draws it and the prepared package need not carry it. + if (slotState.IsUntextured) continue; - if (slotState.Batch is null) { + if (!slotState.BatchResolutionAttempted) { ResolveSlotBatch(slotState); } - if (slotState.Batch is null) continue; // texture resolution failed a dependency lookup; already logged. + if (slotState.Batch is null) continue; // texture resolution failed a dependency lookup; logged once per slot. bool useNegUv = candidate.UvSlot == CellStructPolygonSurfaceSide.Negative; bool invertNormal = candidate.NormalSign < 0; @@ -1114,6 +1153,15 @@ public sealed class MeshExtractor { int skippedUntexturedSlots = 0; foreach (var slot in slots.Keys.OrderBy(s => s)) { var state = slots[slot]; + if (state is null) continue; + + // Contract §4: built-EnvCell DrawMesh(arg4=1) admits a subset + // iff (Surface.Type & (BASE1_IMAGE|BASE1_CLIPMAP)) != 0. The + // slot's mask/ownership facts exist; nothing else was built. + if (state.IsUntextured) { + skippedUntexturedSlots++; + continue; + } if (state.Batch is null) continue; var batch = state.Batch; @@ -1130,16 +1178,6 @@ public sealed class MeshExtractor { // sides_type is NOT stored here any more. batch.CullMode = CullMode.Clockwise; - // Contract §4: built-EnvCell DrawMesh(arg4=1) admits a subset - // iff (Surface.Type & (BASE1_IMAGE|BASE1_CLIPMAP)) != 0. An - // untextured slot is fully constructed above (its mask/ - // triangle-ownership facts exist) but is not emitted into the - // prepared output. - if (RetailUntexturedSurfacePolicy.IsUntextured(state.Surface.Type)) { - skippedUntexturedSlots++; - continue; - } - if (!batchesByFormat.TryGetValue(state.Format, out var list)) { list = new List(); batchesByFormat[state.Format] = list; @@ -1149,7 +1187,7 @@ public sealed class MeshExtractor { if (unknownSidesTypePolygons > 0) { _logger.LogWarning( - "CellStruct id=0x{Id:X16}: {Count} polygon(s) had an unrecognized raw sides_type (only 0/1/2 are retail-defined per OH2 contract §3.4); zero geometry candidates were constructed for them.", + "CellStruct id=0x{Id:X16}: {Count} polygon(s) had a raw sides_type outside 0/1/2 (OH2 contract §3.4); they were constructed as retail's default single-side shape.", id, unknownSidesTypePolygons); } if (skippedUntexturedSlots > 0) { diff --git a/src/AcDream.Content/ObjectMeshData.cs b/src/AcDream.Content/ObjectMeshData.cs index b3d79a39..afa0ef71 100644 --- a/src/AcDream.Content/ObjectMeshData.cs +++ b/src/AcDream.Content/ObjectMeshData.cs @@ -236,6 +236,11 @@ public static class CellSurfaceSubsets { /// was never emitted into /// in the first place, so it is absent here too — this enumerates /// DRAWABLE subsets, not every constructed slot. + /// This LINQ form allocates and is for bake, upload, and test time only + /// (App uploads consume it once per mesh in + /// ObjectMeshManager.OrderedUploadBatches). A per-frame consumer + /// must read a precomputed order (the uploaded batch list already is one), + /// never call this per frame. /// public static IEnumerable InAscendingSurfaceOrder(ObjectMeshData mesh) => mesh.TextureBatches.Values diff --git a/src/AcDream.Core/Meshing/CellMesh.cs b/src/AcDream.Core/Meshing/CellMesh.cs index 2c5d78f5..aeca4a9c 100644 --- a/src/AcDream.Core/Meshing/CellMesh.cs +++ b/src/AcDream.Core/Meshing/CellMesh.cs @@ -30,12 +30,18 @@ public static class CellMesh /// ported from D3DPolyRender::ConstructMesh @0x0059DFA0, contract /// §3.4) targets a surface slot whose resolved Surface.Type is /// TEXTURED ( - /// is false) — exactly the condition under which - /// MeshExtractor.PrepareCellStructMeshData would emit at least - /// one subset into the prepared package: retail's built-EnvCell draw - /// admission is (Surface.Type & (BASE1_IMAGE|BASE1_CLIPMAP)) != 0 + /// is false) — retail's built-EnvCell draw admission + /// (Surface.Type & (BASE1_IMAGE|BASE1_CLIPMAP)) != 0 /// (RenderDeviceD3D::DrawEnvCell @0x0059F170 → - /// D3DPolyRender::DrawMesh(..., arg4=1) @0x0059D4A0, contract §4). + /// D3DPolyRender::DrawMesh(..., arg4=1) @0x0059D4A0, contract §4), + /// evaluated without resolving the surface's texture dependency chain. + /// It is therefore a conservative superset of what + /// MeshExtractor.PrepareCellStructMeshData actually emits: the + /// extractor additionally drops a textured slot whose SurfaceTexture, + /// RenderSurface, palette, or pixel format cannot be resolved (an + /// extractor-side quarantine outcome, logged there), which this predicate + /// still reports as drawable. Missing texture data is a DAT defect, not a + /// retail admission decision, so the predicate follows the admission rule. /// /// /// Side candidates come only from CPolygon::sides_type diff --git a/src/AcDream.Core/Meshing/CellStructSideCandidates.cs b/src/AcDream.Core/Meshing/CellStructSideCandidates.cs index 3039f703..60fcfa33 100644 --- a/src/AcDream.Core/Meshing/CellStructSideCandidates.cs +++ b/src/AcDream.Core/Meshing/CellStructSideCandidates.cs @@ -148,22 +148,33 @@ public static class CellStructSideCandidates /// branch returns a span over a static readonly array. /// /// - /// Unknown/out-of-range values (anything other than 0, 1, or 2) yield - /// zero candidates. The installed DAT corpus and the retail header - /// define only 0/1/2 (contract §3.4 closing paragraph); retail's own - /// branching would fall through to the single-side shape for other - /// values, but the port deliberately does not invent that fourth public - /// semantic — an unrecognized raw value stays data for the caller to - /// quarantine/report, not a silently-guessed geometry shape. + /// Any value other than 1 or 2 takes the single-side shape. That is + /// retail's literal branching, not a guess: ConstructMesh + /// initializes both loop bounds to 1 and widens the side bound only on + /// sides_type == 2 and the copy bound only on + /// sides_type == 1 (pseudo-C 426837-426842 and 427058-427066; + /// Ghidra iStack_44 = 1; local_5c = 1; if (*piVar6 == 2) ...; + /// if (*piVar6 == 1) ...). The installed corpus contains only 0/1/2 + /// (pinned by the S1 installed-DAT scan); callers should still count a + /// raw value outside that set as a data anomaly for diagnostics, but the + /// geometry it produces is retail's, so no divergence row is needed. /// public static ReadOnlySpan GetCandidates(int rawSidesType) => rawSidesType switch { - 0 => SingleCandidates, 1 => DoubleCandidates, 2 => BothCandidates, - _ => ReadOnlySpan.Empty, + _ => SingleCandidates, }; + /// + /// True for the three retail-defined SidesType values + /// (ST_SINGLE=0, ST_DOUBLE=1, ST_BOTH=2, + /// acclient.h:7372). Anything else is authored-data corruption that + /// still renders as retail would (single + /// side); callers use this only to report the anomaly. + /// + public static bool IsRetailDefinedSidesType(int rawSidesType) => rawSidesType is 0 or 1 or 2; + /// /// Retail's exact triangle-fan vertex index order for one triangle /// within a fan, per contract §3.4's "Fan index order" column: forward diff --git a/tests/AcDream.Content.Tests/CellStructSurfaceConstructionInstalledDatTests.cs b/tests/AcDream.Content.Tests/CellStructSurfaceConstructionInstalledDatTests.cs index db675eb5..f079cf73 100644 --- a/tests/AcDream.Content.Tests/CellStructSurfaceConstructionInstalledDatTests.cs +++ b/tests/AcDream.Content.Tests/CellStructSurfaceConstructionInstalledDatTests.cs @@ -150,12 +150,14 @@ public sealed class CellStructSurfaceConstructionInstalledDatTests { if (poly.VertexIds.Count < 3) continue; polygons++; + // A raw sides value outside 0/1/2 is an authored-data + // anomaly, counted here; retail (and the port) still + // construct its single-side shape (ConstructMesh + // @0x0059DFA0 loop bounds default to 1). + if (!CellStructSideCandidates.IsRetailDefinedSidesType((int)poly.SidesType)) + unknownSidesType++; ReadOnlySpan polyCandidates = CellStructSideCandidates.GetCandidates((int)poly.SidesType); - if (polyCandidates.Length == 0) { - unknownSidesType++; - continue; - } foreach (var candidate in polyCandidates) { candidates++; diff --git a/tests/AcDream.Content.Tests/MeshExtractorSolidFaceExtractionTests.cs b/tests/AcDream.Content.Tests/MeshExtractorSolidFaceExtractionTests.cs index 1e046349..d45ea7d1 100644 --- a/tests/AcDream.Content.Tests/MeshExtractorSolidFaceExtractionTests.cs +++ b/tests/AcDream.Content.Tests/MeshExtractorSolidFaceExtractionTests.cs @@ -497,6 +497,91 @@ public sealed class MeshExtractorSolidFaceExtractionTests Assert.All(mesh.Vertices, vertex => Assert.Equal(Vector2.Zero, vertex.UV)); } + [Fact] + public void PrepareCellStructMeshData_DegeneratePolygon_StillOrsPositiveSurfaceMask() + { + // ConstructMesh @0x0059DFA0 count loop (pseudo-C 426859-426866): the + // positive-surface stippling OR runs once per polygon BEFORE the + // num_pts branch, so a two-vertex polygon that builds no fan still + // stamps bit 1 on its positive slot. Slot 0 gets its geometry from + // the textured quad (stippling 0, so no bit from it) and its mask bit + // only from the degenerate polygon. + var dats = new FakeMeshExtractorDats(); + RegisterCellTexturedSurface(dats); + var cellStruct = BuildQuadCellStruct(RetailCullMode.Landblock); + cellStruct.Polygons[1] = new Polygon + { + SidesType = RetailCullMode.Landblock, + Stippling = StipplingType.NoPos, + PosSurface = 0, + NegSurface = -1, + VertexIds = [0, 1], + }; + var extractor = new MeshExtractor(dats, NullLogger.Instance, sideStagedSink: null); + + ObjectMeshData mesh = Assert.IsType( + extractor.PrepareCellStructMeshData( + id: 1, cellStruct, surfaceOverrides: [2], Matrix4x4.Identity, CancellationToken.None)); + + TextureBatchData batch = Assert.Single(Assert.Single(mesh.TextureBatches).Value); + Assert.Equal(6, batch.Indices.Count); + Assert.Equal(1, batch.RetailSurfaceMask); + } + + [Fact] + public void PrepareCellStructMeshData_UntexturedSlot_EmitsNoVertices() + { + // An untextured slot fails built-EnvCell admission (contract §4). + // Retail constructs its geometry and never draws it; the prepared + // package does not carry vertices that can never draw (contract §9 + // item 3). Slot 0 is textured, slot 1 is Base1Solid: only the four + // vertices of the textured quad may reach the output. + var dats = new FakeMeshExtractorDats(); + RegisterCellTexturedSurface(dats); + dats.Register(SolidSurfaceId, new Surface + { + Type = SurfaceType.Base1Solid, + ColorValue = new ColorARGB { Alpha = 255, Red = 64, Green = 96, Blue = 128 }, + }); + var cellStruct = BuildQuadCellStruct(RetailCullMode.Landblock); + cellStruct.Polygons[1] = new Polygon + { + SidesType = RetailCullMode.Landblock, + PosSurface = 1, + NegSurface = -1, + VertexIds = [0, 1, 2, 3], + }; + var extractor = new MeshExtractor(dats, NullLogger.Instance, sideStagedSink: null); + + ObjectMeshData mesh = Assert.IsType( + extractor.PrepareCellStructMeshData( + id: 1, cellStruct, surfaceOverrides: [2, 1], Matrix4x4.Identity, CancellationToken.None)); + + TextureBatchData batch = Assert.Single(Assert.Single(mesh.TextureBatches).Value); + Assert.Equal(0, batch.SourceSurfaceIndex); + Assert.Equal(4, mesh.Vertices.Length); + } + + [Fact] + public void PrepareCellStructMeshData_SidesValueOutsideRetailSet_ConstructsSingleSideFan() + { + // ConstructMesh @0x0059DFA0 initializes both loop bounds to 1 and + // widens them only on an exact 1 / 2, so an anomalous raw value + // draws as ST_SINGLE rather than vanishing (S1 review finding). + var dats = new FakeMeshExtractorDats(); + RegisterCellTexturedSurface(dats); + var cellStruct = BuildQuadCellStruct((RetailCullMode)7); + var extractor = new MeshExtractor(dats, NullLogger.Instance, sideStagedSink: null); + + ObjectMeshData mesh = Assert.IsType( + extractor.PrepareCellStructMeshData( + id: 1, cellStruct, surfaceOverrides: [2], Matrix4x4.Identity, CancellationToken.None)); + + TextureBatchData batch = Assert.Single(Assert.Single(mesh.TextureBatches).Value); + Assert.Equal(6, batch.Indices.Count); + Assert.Equal(4, mesh.Vertices.Length); + } + [Fact] public void PrepareCellStructMeshData_TwoSlotsSameSurfaceDid_RemainTwoDistinctSubsets() { diff --git a/tests/AcDream.Core.Tests/Conformance/StipplingSurfaceEquivalenceTests.cs b/tests/AcDream.Core.Tests/Conformance/StipplingSurfaceEquivalenceTests.cs index 310b3b3d..27a5cd5b 100644 --- a/tests/AcDream.Core.Tests/Conformance/StipplingSurfaceEquivalenceTests.cs +++ b/tests/AcDream.Core.Tests/Conformance/StipplingSurfaceEquivalenceTests.cs @@ -176,8 +176,14 @@ public sealed class StipplingSurfaceEquivalenceTests foreach (var v in aViolations.Take(20)) _out.WriteLine($" {v}"); _out.WriteLine($"(b) building untextured-but-not-NoPos (load-bearing): {bViolations.Count}"); foreach (var v in bViolations.Take(20)) _out.WriteLine($" {v}"); - _out.WriteLine($"(c) cell NoPos-but-not-portal-poly (REPORT ONLY, not asserted): {cellPortalPolyMismatches.Count}"); + _out.WriteLine($"(c) cell NoPos-but-not-portal-poly (content-shape pin, not a draw rule): {cellPortalPolyMismatches.Count}"); foreach (var v in cellPortalPolyMismatches.Take(20)) _out.WriteLine($" {v}"); + // Pinned as a content-shape golden (S1 review, 2026-09-02): zero on + // the installed DATs recorded in the OH2 contract §1 table. It is not + // a draw rule any more (cells resolve the Surface's own Type), but a + // change here means the authored content or this sweep changed and + // deserves a look rather than a silent console line. + Assert.Empty(cellPortalPolyMismatches); Assert.Empty(aViolations); Assert.Empty(bViolations); diff --git a/tests/AcDream.Core.Tests/Meshing/CellStructSideCandidatesTests.cs b/tests/AcDream.Core.Tests/Meshing/CellStructSideCandidatesTests.cs index 73496744..80900e2c 100644 --- a/tests/AcDream.Core.Tests/Meshing/CellStructSideCandidatesTests.cs +++ b/tests/AcDream.Core.Tests/Meshing/CellStructSideCandidatesTests.cs @@ -79,11 +79,19 @@ public class CellStructSideCandidatesTests [InlineData(3)] [InlineData(-1)] [InlineData(99)] - public void UnknownSidesValue_YieldsNoCandidates(int rawSidesType) + public void UnknownSidesValue_FallsBackToRetailSingleSideShape(int rawSidesType) { + // ConstructMesh @0x0059DFA0 initializes both loop bounds to 1 and + // widens them only on an exact 1 / 2 match, so any other raw value + // constructs the single positive fan (S1 review finding, 2026-09-02). var candidates = CellStructSideCandidates.GetCandidates(rawSidesType); - Assert.True(candidates.IsEmpty); + var single = Assert.Single(candidates.ToArray()); + Assert.Equal(CellStructPolygonSurfaceSide.Positive, single.SurfaceSlot); + Assert.Equal(0, single.CopyOrdinal); + Assert.Equal(1, single.NormalSign); + Assert.False(single.ReverseWinding); + Assert.False(CellStructSideCandidates.IsRetailDefinedSidesType(rawSidesType)); } // ---- exact fan index order (§3.4 "Fan index order" column) ----