fix(content): S1 review round - retail default sides shape, mask hoist, upload order
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 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
0840d5fb77
commit
e2543d0ef0
10 changed files with 265 additions and 73 deletions
|
|
@ -2041,6 +2041,34 @@ namespace AcDream.App.Rendering.Wb
|
|||
|
||||
#region Private: GPU Upload
|
||||
|
||||
/// <summary>
|
||||
/// 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 <see cref="TextureBatchData.IsCellShell"/>) is
|
||||
/// ordered by ascending <see cref="TextureBatchData.SourceSurfaceIndex"/>,
|
||||
/// retail's built-EnvCell subset draw order (contract §3.6:
|
||||
/// <c>D3DPolyRender::ConstructMesh</c> @0x0059DFA0 emits one attribute
|
||||
/// range per nonempty surface index ascending, and
|
||||
/// <c>DrawMesh</c> @0x0059D4A0 draws them in that order). Runs once per
|
||||
/// mesh upload; the allocation is not per-frame.
|
||||
/// </summary>
|
||||
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;
|
||||
|
||||
|
|
|
|||
|
|
@ -753,6 +753,25 @@ public sealed class MeshExtractor {
|
|||
public Surface Surface { get; }
|
||||
public uint SurfaceId { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Retail's built-EnvCell admission decided at slot creation
|
||||
/// (<c>RenderDeviceD3D::DrawEnvCell</c> @0x0059F170 →
|
||||
/// <c>D3DPolyRender::DrawMesh(..., arg4=1)</c> @0x0059D4A0 skips a
|
||||
/// subset unless <c>(Surface.Type & 6) != 0</c>, 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.
|
||||
/// </summary>
|
||||
public bool IsUntextured => RetailUntexturedSurfacePolicy.IsUntextured(Surface.Type);
|
||||
|
||||
/// <summary>
|
||||
/// Set the first time <c>ResolveSlotBatch</c> 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).
|
||||
/// </summary>
|
||||
public bool BatchResolutionAttempted;
|
||||
|
||||
/// <summary>
|
||||
/// Retail's <c>isStippledOrAlphaedMask</c> byte for this slot:
|
||||
/// <see cref="CellStructSideCandidates.InitialSurfaceMask"/> 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<TextureBatchData>>();
|
||||
var slots = new Dictionary<int, CellSurfaceSlot>();
|
||||
// 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<int, CellSurfaceSlot?>();
|
||||
|
||||
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<CellStructSideCandidate> 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<CellStructSideCandidate> 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<TextureBatchData>();
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -236,6 +236,11 @@ public static class CellSurfaceSubsets {
|
|||
/// was never emitted into <see cref="ObjectMeshData.TextureBatches"/>
|
||||
/// 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
|
||||
/// <c>ObjectMeshManager.OrderedUploadBatches</c>). A per-frame consumer
|
||||
/// must read a precomputed order (the uploaded batch list already is one),
|
||||
/// never call this per frame.
|
||||
/// </summary>
|
||||
public static IEnumerable<TextureBatchData> InAscendingSurfaceOrder(ObjectMeshData mesh) =>
|
||||
mesh.TextureBatches.Values
|
||||
|
|
|
|||
|
|
@ -30,12 +30,18 @@ public static class CellMesh
|
|||
/// ported from <c>D3DPolyRender::ConstructMesh</c> @0x0059DFA0, contract
|
||||
/// §3.4) targets a surface slot whose resolved <c>Surface.Type</c> is
|
||||
/// TEXTURED (<see cref="RetailUntexturedSurfacePolicy.IsUntextured"/>
|
||||
/// is false) — exactly the condition under which
|
||||
/// <c>MeshExtractor.PrepareCellStructMeshData</c> would emit at least
|
||||
/// one subset into the prepared package: retail's built-EnvCell draw
|
||||
/// admission is <c>(Surface.Type & (BASE1_IMAGE|BASE1_CLIPMAP)) != 0</c>
|
||||
/// is false) — retail's built-EnvCell draw admission
|
||||
/// <c>(Surface.Type & (BASE1_IMAGE|BASE1_CLIPMAP)) != 0</c>
|
||||
/// (<c>RenderDeviceD3D::DrawEnvCell</c> @0x0059F170 →
|
||||
/// <c>D3DPolyRender::DrawMesh(..., arg4=1)</c> @0x0059D4A0, contract §4).
|
||||
/// <c>D3DPolyRender::DrawMesh(..., arg4=1)</c> @0x0059D4A0, contract §4),
|
||||
/// evaluated without resolving the surface's texture dependency chain.
|
||||
/// It is therefore a conservative superset of what
|
||||
/// <c>MeshExtractor.PrepareCellStructMeshData</c> 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.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Side candidates come only from <c>CPolygon::sides_type</c>
|
||||
|
|
|
|||
|
|
@ -148,22 +148,33 @@ public static class CellStructSideCandidates
|
|||
/// branch returns a span over a static readonly array.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// 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: <c>ConstructMesh</c>
|
||||
/// initializes both loop bounds to 1 and widens the side bound only on
|
||||
/// <c>sides_type == 2</c> and the copy bound only on
|
||||
/// <c>sides_type == 1</c> (pseudo-C 426837-426842 and 427058-427066;
|
||||
/// Ghidra <c>iStack_44 = 1; local_5c = 1; if (*piVar6 == 2) ...;
|
||||
/// if (*piVar6 == 1) ...</c>). 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.
|
||||
/// </remarks>
|
||||
public static ReadOnlySpan<CellStructSideCandidate> GetCandidates(int rawSidesType) => rawSidesType switch
|
||||
{
|
||||
0 => SingleCandidates,
|
||||
1 => DoubleCandidates,
|
||||
2 => BothCandidates,
|
||||
_ => ReadOnlySpan<CellStructSideCandidate>.Empty,
|
||||
_ => SingleCandidates,
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// True for the three retail-defined <c>SidesType</c> values
|
||||
/// (<c>ST_SINGLE</c>=0, <c>ST_DOUBLE</c>=1, <c>ST_BOTH</c>=2,
|
||||
/// acclient.h:7372). Anything else is authored-data corruption that
|
||||
/// <see cref="GetCandidates"/> still renders as retail would (single
|
||||
/// side); callers use this only to report the anomaly.
|
||||
/// </summary>
|
||||
public static bool IsRetailDefinedSidesType(int rawSidesType) => rawSidesType is 0 or 1 or 2;
|
||||
|
||||
/// <summary>
|
||||
/// Retail's exact triangle-fan vertex index order for one triangle
|
||||
/// within a fan, per contract §3.4's "Fan index order" column: forward
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue