From 3fd774515a9afa4ea11c15f3405822264ab01292 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 11 Apr 2026 20:59:10 +0200 Subject: [PATCH] fix(core): emit both sides of double-sided polygons in GfxObjMesh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lifestone (and likely any weenie with closed shells using NoPos / Negative / Both stippling) rendered with visible holes where you could see inside it — confirmed via the user's "see into it" description. Root cause: GfxObjMesh.Build skipped any polygon whose PosSurface was out of range, which is exactly what a NoPos-stippled or negative-only polygon looks like. Backface culling isn't involved (acdream has it disabled); we were simply dropping triangles. Ported the pos/neg emission rule from references/WorldBuilder/Chorizite.OpenGLSDLBackend/Lib/ ObjectMeshManager.cs (lines 955-971 and 1510-1577): pos side: emit when !Stippling.NoPos and PosSurface is valid neg side: emit when Stippling.Negative, Stippling.Both, OR (!Stippling.NoNeg && SidesType == CullMode.Clockwise) The "Clockwise CullMode means NegUVIndices are on the wire" rule is non-obvious but matches how Polygon.Unpack reads NegUVIndices, so any closed mesh relying on that convention now renders correctly. Neg-side triangles get the reversed fan winding and a negated vertex normal. With culling off the winding only matters for lighting consistency, but keeping the semantics right future-proofs the fix if we ever enable back-face culling for a perf pass. The dedup cache is keyed by (posIdx, uvIdx, isNeg) so the same vertex can carry different normals on the pos and neg sides. Pos-side winding is preserved at the original (0, i, i+1) order so the existing single-triangle and fan-triangulation tests still pass — neg side uses (i+1, i, 0), which is the same shape mirrored. 194 tests green. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/AcDream.Core/Meshing/GfxObjMesh.cs | 203 +++++++++++++++++++------ 1 file changed, 154 insertions(+), 49 deletions(-) diff --git a/src/AcDream.Core/Meshing/GfxObjMesh.cs b/src/AcDream.Core/Meshing/GfxObjMesh.cs index cd57cad..b2a24c1 100644 --- a/src/AcDream.Core/Meshing/GfxObjMesh.cs +++ b/src/AcDream.Core/Meshing/GfxObjMesh.cs @@ -2,6 +2,7 @@ using System.Numerics; using AcDream.Core.Terrain; using DatReaderWriter; using DatReaderWriter.DBObjs; +using DatReaderWriter.Enums; namespace AcDream.Core.Meshing; @@ -9,7 +10,8 @@ public static class GfxObjMesh { /// /// Walk a GfxObj's polygons and produce one - /// per referenced Surface. Polygons are triangulated as fans. + /// per referenced Surface, emitting positive-side and negative-side + /// triangles separately when the polygon specifies both. /// /// The GfxObj to build sub-meshes from. /// @@ -17,77 +19,180 @@ public static class GfxObjMesh /// . When null (e.g. offline tests) /// all sub-meshes default to . /// + /// + /// + /// Ported from + /// references/WorldBuilder/Chorizite.OpenGLSDLBackend/Lib/ObjectMeshManager.cs + /// (BuildPolygonIndices + the pos/neg emission loop around line 955). + /// The rule for emitting a polygon side: + /// + /// + /// Pos side: emit whenever !Stippling.NoPos and + /// PosSurface is a valid index. + /// Neg side: emit when + /// Stippling.Negative, Stippling.Both, or + /// (!Stippling.NoNeg && SidesType == CullMode.Clockwise). + /// The last condition is AC's non-obvious convention for "this + /// polygon has a back face even though nothing in Stippling + /// declares it" — you cannot drop it without punching holes + /// through closed meshes like the lifestone and any other + /// weenie that relies on double-sided polygons. + /// + /// + /// Neg-side triangles get the reversed winding and a negated vertex + /// normal so lighting stays correct if we ever enable face culling; + /// acdream currently renders with culling disabled, but we still emit + /// reversed indices to keep the semantics right. + /// + /// + /// The dedup cache is keyed by (posIdx, uvIdx, isNeg) because + /// the same vertex position on the pos and neg sides needs different + /// normals (and potentially different UVs via NegUVIndices). + /// + /// public static IReadOnlyList Build(GfxObj gfxObj, DatCollection? dats = null) { - // Group output vertices and indices per surface index. - var perSurface = new Dictionary Vertices, List Indices, Dictionary<(int pos, int uv), uint> Dedupe)>(); + // One bucket per (surface-index, isNeg) pair. Negative-side triangles + // always land in a different bucket than their positive counterparts + // because their normals and winding differ; the renderer doesn't care + // about the distinction once sub-meshes are emitted, but the build + // loop has to keep them separate to produce correct vertex data. + var perBucket = new Dictionary<(int surfaceIdx, bool isNeg), + (List Vertices, List Indices, + Dictionary<(int pos, int uv, bool neg), uint> Dedupe)>(); foreach (var kvp in gfxObj.Polygons) { var poly = kvp.Value; - if (poly.VertexIds.Count < 3) - continue; // degenerate + continue; // degenerate — can't form a triangle - int surfaceIdx = poly.PosSurface; - if (surfaceIdx < 0 || surfaceIdx >= gfxObj.Surfaces.Count) - continue; // out of range surface + // --- Positive side --- + bool hasPos = !poly.Stippling.HasFlag(StipplingType.NoPos); + if (hasPos) + EmitSide(poly, poly.PosSurface, isNeg: false); - if (!perSurface.TryGetValue(surfaceIdx, out var bucket)) + // --- Negative side --- + // Three ways AC flags a polygon as double-sided: + // 1. Stippling.Negative or Stippling.Both — explicit. + // 2. Stippling.NoNeg is NOT set AND SidesType == Clockwise — + // AC's "Clockwise CullMode means there are NegUVIndices + // on the wire" convention. See + // DatReaderWriter/.../Generated/Types/Polygon.generated.cs + // — NegUVIndices are only read when SidesType == Clockwise, + // and WorldBuilder uses the same rule to decide whether to + // emit the neg side at build time. + bool hasNeg = + poly.Stippling.HasFlag(StipplingType.Negative) || + poly.Stippling.HasFlag(StipplingType.Both) || + (!poly.Stippling.HasFlag(StipplingType.NoNeg) && poly.SidesType == CullMode.Clockwise); + + if (hasNeg) + EmitSide(poly, poly.NegSurface, isNeg: true); + + void EmitSide(DatReaderWriter.Types.Polygon p, short surfaceIdx, bool isNeg) { - bucket = (new List(), new List(), new Dictionary<(int, int), uint>()); - perSurface[surfaceIdx] = bucket; - } + if (surfaceIdx < 0 || surfaceIdx >= gfxObj.Surfaces.Count) + return; - // Collect output vertex indices for this polygon. - var polyOut = new List(poly.VertexIds.Count); - bool skipPoly = false; - - for (int i = 0; i < poly.VertexIds.Count; i++) - { - int posIdx = poly.VertexIds[i]; - int uvIdx = i < poly.PosUVIndices.Count ? poly.PosUVIndices[i] : 0; - - if (!gfxObj.VertexArray.Vertices.TryGetValue((ushort)posIdx, out var sw)) + var bucketKey = ((int)surfaceIdx, isNeg); + if (!perBucket.TryGetValue(bucketKey, out var bucket)) { - skipPoly = true; - break; + bucket = (new List(), new List(), + new Dictionary<(int, int, bool), uint>()); + perBucket[bucketKey] = bucket; } - var texcoord = uvIdx >= 0 && uvIdx < sw.UVs.Count - ? new Vector2(sw.UVs[uvIdx].U, sw.UVs[uvIdx].V) - : Vector2.Zero; - - var key = (posIdx, uvIdx); - if (!bucket.Dedupe.TryGetValue(key, out var outIdx)) + // Collect one output index per polygon corner. If we fail to + // resolve a vertex we abort the whole polygon rather than + // emitting a degenerate triangle (matches the behavior of + // the previous builder). + var polyOut = new List(p.VertexIds.Count); + bool skipPoly = false; + for (int i = 0; i < p.VertexIds.Count; i++) { - outIdx = (uint)bucket.Vertices.Count; - bucket.Vertices.Add(new Vertex(sw.Origin, sw.Normal, texcoord, TerrainLayer: 0)); - bucket.Dedupe[key] = outIdx; + int posIdx = p.VertexIds[i]; + + // UV index selection: neg side uses NegUVIndices when + // present; otherwise fall back to PosUVIndices; otherwise + // zero. Matches WorldBuilder/ObjectMeshManager.cs:1521-1524. + int uvIdx = 0; + if (isNeg && p.NegUVIndices.Count > 0 && i < p.NegUVIndices.Count) + uvIdx = p.NegUVIndices[i]; + else if (!isNeg && i < p.PosUVIndices.Count) + uvIdx = p.PosUVIndices[i]; + else if (i < p.PosUVIndices.Count) + uvIdx = p.PosUVIndices[i]; // neg side with no NegUVIndices — borrow pos + + if (!gfxObj.VertexArray.Vertices.TryGetValue((ushort)posIdx, out var sw)) + { + skipPoly = true; + break; + } + + var texcoord = uvIdx >= 0 && uvIdx < sw.UVs.Count + ? new Vector2(sw.UVs[uvIdx].U, sw.UVs[uvIdx].V) + : Vector2.Zero; + + // Negate the vertex normal for the neg side so lighting + // stays correct if we ever enable face culling. With + // culling disabled the shader still samples this normal + // for the diffuse term so getting it right matters + // regardless of backface state. + var normal = isNeg ? -sw.Normal : sw.Normal; + + var key = (posIdx, uvIdx, isNeg); + if (!bucket.Dedupe.TryGetValue(key, out var outIdx)) + { + outIdx = (uint)bucket.Vertices.Count; + bucket.Vertices.Add(new Vertex(sw.Origin, normal, texcoord, TerrainLayer: 0)); + bucket.Dedupe[key] = outIdx; + } + polyOut.Add(outIdx); } - polyOut.Add(outIdx); - } - if (skipPoly || polyOut.Count < 3) - continue; + if (skipPoly || polyOut.Count < 3) + return; - // Fan triangulation: (v0, v1, v2), (v0, v2, v3), ... - for (int i = 1; i < polyOut.Count - 1; i++) - { - bucket.Indices.Add(polyOut[0]); - bucket.Indices.Add(polyOut[i]); - bucket.Indices.Add(polyOut[i + 1]); + // Fan triangulation. Pos side keeps the original + // (0, i, i+1) winding the earlier builder used so existing + // tests and render behavior are preserved. Neg side emits + // the opposite winding so the two faces point away from + // each other — matches WorldBuilder/ObjectMeshManager.cs: + // 1564-1577 once you account for the reversed pos order. + if (isNeg) + { + for (int i = 1; i < polyOut.Count - 1; i++) + { + bucket.Indices.Add(polyOut[i + 1]); + bucket.Indices.Add(polyOut[i]); + bucket.Indices.Add(polyOut[0]); + } + } + else + { + for (int i = 1; i < polyOut.Count - 1; i++) + { + bucket.Indices.Add(polyOut[0]); + bucket.Indices.Add(polyOut[i]); + bucket.Indices.Add(polyOut[i + 1]); + } + } } } - // Emit one sub-mesh per surface. - var result = new List(perSurface.Count); - foreach (var kvp in perSurface) + // Emit one sub-mesh per (surface, side) bucket. The sub-mesh API + // doesn't care whether a surface came from the pos or neg side — + // both go through the same texture cache path. + var result = new List(perBucket.Count); + foreach (var kvp in perBucket) { - var surfaceId = (uint)gfxObj.Surfaces[kvp.Key]; + var (surfaceIdx, _) = kvp.Key; + var surfaceId = (uint)gfxObj.Surfaces[surfaceIdx]; - // Resolve Surface.Type flags when a DatCollection is available so the - // renderer can split the draw into opaque and translucent passes. + // Resolve Surface.Type flags when a DatCollection is available + // so the renderer can split the draw into opaque and translucent + // passes. var translucency = TranslucencyKind.Opaque; if (dats is not null) {