refactor: #100 — remove hiddenTerrainCells / BuildingTerrainCells plumbing
Retired in favour of Task 1's retail-faithful terrain shader Z nudge.
Pure removal — ~50 LOC of dead surface area across:
- src/AcDream.Core/Terrain/LandblockMesh.cs (drop parameter +
cell-collapse block)
- src/AcDream.Core/World/LoadedLandblock.cs (drop field)
- src/AcDream.Core/World/LandblockLoader.cs (drop method + call)
- src/AcDream.App/Rendering/GameWindow.cs (3 sites)
- src/AcDream.App/Streaming/GpuWorldState.cs (6 ctor sites)
- src/AcDream.App/Streaming/LandblockStreamer.cs (1 ctor site)
- tests/AcDream.Core.Tests/World/LandblockLoaderTests.cs (drop test)
- tests/AcDream.Core.Tests/Terrain/LandblockMeshTests.cs (drop test)
No retail anchor — the deleted mechanism never had one; this commit
rolls our code back to the actual retail behaviour established in
the prior commit's shader nudge.
ISSUES.md #100 moved to Recently closed.
Cross-ref:
docs/research/2026-05-25-issue-100-terrain-cutout-handoff.md
docs/superpowers/plans/2026-05-25-issue-100-terrain-cutout.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
f48c74aa8b
commit
a64e6f20da
9 changed files with 52 additions and 165 deletions
|
|
@ -761,56 +761,6 @@ family (sling-out — also likely).
|
|||
|
||||
---
|
||||
|
||||
## #100 — Transparent rectangular patches around every house (terrain rendering)
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** MEDIUM (visual regression; affects every Holtburg house)
|
||||
**Filed:** 2026-05-24
|
||||
**Component:** rendering, terrain
|
||||
|
||||
**Description:** Standing outside any Holtburg house, the ground in a
|
||||
rectangular footprint around the building appears as a flat dark patch
|
||||
instead of cobblestone / grass terrain. Visible as a sharp-edged
|
||||
rectangle the size of the house's outdoor footprint. Same shape on
|
||||
every house observed.
|
||||
|
||||
User report 2026-05-24 (with screenshot): "around every house now I
|
||||
missing the ground texture, it is transparent. I can see through the
|
||||
ground."
|
||||
|
||||
**Root cause / status:** **Bisect 2026-05-24 — commit `35b37df`** is the introducer (the only commit on this worktree branch that touches `src/AcDream.Core/Terrain/`). It added a `hiddenTerrainCells` parameter to `LandblockMesh.Build` that collapses terrain triangles owned by buildings to zero-area degenerates, intended so the building's own ground-level mesh visually fills the gap (avoids Z-fighting between terrain and building floor).
|
||||
|
||||
The hide mechanism works at **outdoor-cell granularity** — 24 m × 24 m cells indexed by `cy * 8 + cx` from `LandBlockInfo.Buildings`. A cottage building only fills ~half of one outdoor cell (cottage footprint ~12 m × 12 m vs cell 24 m × 24 m), so the entire 24 × 24 cell terrain gets hidden but the cottage geometry only covers a smaller area inside it. The visible result: a dark rectangle (sky / framebuffer clear bleeding through) around every house where terrain was hidden but no building mesh fills the gap.
|
||||
|
||||
Confirmed in [`src/AcDream.Core/Terrain/LandblockMesh.cs:178`](src/AcDream.Core/Terrain/LandblockMesh.cs:178):
|
||||
```csharp
|
||||
if (hiddenTerrainCells is not null && hiddenTerrainCells.Contains(cellIdx))
|
||||
{
|
||||
indices[i] = (uint)(cellIdx * VerticesPerCell); // collapse to vertex 0 → degenerate
|
||||
continue;
|
||||
}
|
||||
```
|
||||
|
||||
The cells flagged hidden come from `LandblockLoader.BuildBuildingTerrainCells` (also kept by 35b37df), which reads `LandBlockInfo.Buildings` and emits one cellIdx per listed building's `cy*8+cx`.
|
||||
|
||||
The b3ce505 issue-#98 fix did NOT cause or interact with this — it only touched physics collision code.
|
||||
|
||||
**Fix paths** (need design decision):
|
||||
1. **Polygon-level terrain occlusion** instead of cell-level. Build per-poly cutouts from each building's ground-footprint convex hull / bounding box, modify the terrain mesh to actually have a hole the building exactly fits. Retail-faithful for this case but a real engineering change to `LandblockMesh.Build`.
|
||||
2. **Drop the hiddenTerrainCells mechanism entirely** and accept Z-fighting on the building floor vs terrain seam (or solve Z-fighting via a tiny render-only Z lift on the building floor mesh, the same trick we already use for env cell floors at line 5363 `+ new Vector3(0f, 0f, 0.02f)`).
|
||||
3. **Render the building's "yard" mesh** if buildings have such a thing in retail. (Need to check — Holtburg cottages may have stone foundation polys around them that retail renders.)
|
||||
|
||||
Option 2 is the smallest change; option 1 is the most faithful. Option 3 needs retail visual research.
|
||||
|
||||
**Files:**
|
||||
- `src/AcDream.Core/Terrain/LandblockMesh.cs:178` — the collapse code
|
||||
- `src/AcDream.Core/World/LandblockLoader.cs` — `BuildBuildingTerrainCells`
|
||||
- `src/AcDream.App/Rendering/GameWindow.cs:1808, 5366, 8761` — sites calling `LandblockMesh.Build` with `hiddenTerrainCells`
|
||||
|
||||
**Acceptance:** Standing outside a Holtburg house, the ground around it
|
||||
renders with the same cobblestone / grass texture as the surrounding
|
||||
terrain — no dark rectangular patches.
|
||||
|
||||
---
|
||||
|
||||
## #98-old-context-preserved-for-reference
|
||||
|
|
@ -3474,6 +3424,44 @@ Unverified. The likely culprits, ranked by suspected probability:
|
|||
|
||||
# Recently closed
|
||||
|
||||
## #100 — [DONE 2026-05-25 · f48c74aa + 64518d59] Transparent rectangular patches around every house (terrain rendering)
|
||||
|
||||
**Status:** DONE
|
||||
**Closed:** 2026-05-25
|
||||
**Commits:** `f48c74aa`, `64518d59`
|
||||
**Component:** rendering, terrain
|
||||
|
||||
**Resolution (2026-05-25 · #100):** Replaced the cell-level
|
||||
`hiddenTerrainCells` mechanism with retail's per-vertex Z nudge
|
||||
(`zFightTerrainAdjust = 0.00999999978`) applied inside the modern
|
||||
terrain vertex shader. Render terrain everywhere; coplanar building
|
||||
floors win the depth test by being 1 cm higher than the rendered
|
||||
terrain. Physics path untouched. ~50 LOC of `BuildingTerrainCells`
|
||||
plumbing removed across LandblockMesh / LoadedLandblock /
|
||||
LandblockLoader / GameWindow / GpuWorldState / LandblockStreamer
|
||||
plus the corresponding unit test. Retail anchors:
|
||||
acclient_2013_pseudo_c.txt:1120769 + :702254.
|
||||
|
||||
**Description:** Standing outside any Holtburg house, the ground in a
|
||||
rectangular footprint around the building appears as a flat dark patch
|
||||
instead of cobblestone / grass terrain. Visible as a sharp-edged
|
||||
rectangle the size of the house's outdoor footprint. Same shape on
|
||||
every house observed.
|
||||
|
||||
User report 2026-05-24 (with screenshot): "around every house now I
|
||||
missing the ground texture, it is transparent. I can see through the
|
||||
ground."
|
||||
|
||||
**Root cause:** Bisect 2026-05-24 — commit `35b37df` is the introducer. It
|
||||
added a `hiddenTerrainCells` parameter to `LandblockMesh.Build` that collapses
|
||||
terrain triangles owned by buildings to zero-area degenerates. The hide
|
||||
mechanism works at outdoor-cell granularity (24 m × 24 m cells), so the entire
|
||||
cell terrain was hidden but the cottage geometry only covers a smaller area inside
|
||||
it — leaving a dark transparent rectangle. The fix renders terrain everywhere and
|
||||
uses retail's Z nudge to ensure building floors win the depth test.
|
||||
|
||||
---
|
||||
|
||||
## #101 — [DONE 2026-05-25 · 5240d65 + 6ca872f] Stair-step cylinder phantom blocks player on multi-part EnvCell entity
|
||||
|
||||
**Closed:** 2026-05-25
|
||||
|
|
|
|||
|
|
@ -1806,7 +1806,7 @@ public sealed class GameWindow : IDisposable
|
|||
// _heightTable and _blendCtx are read-only after initialization.
|
||||
// lb.Heightmap is the pre-loaded LandBlock; no dat read needed here.
|
||||
return AcDream.Core.Terrain.LandblockMesh.Build(
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache, lb.BuildingTerrainCells);
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache);
|
||||
});
|
||||
_streamer.Start();
|
||||
|
||||
|
|
@ -5145,8 +5145,7 @@ public sealed class GameWindow : IDisposable
|
|||
return new AcDream.Core.World.LoadedLandblock(
|
||||
baseLoaded.LandblockId,
|
||||
baseLoaded.Heightmap,
|
||||
merged,
|
||||
baseLoaded.BuildingTerrainCells);
|
||||
merged);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -8803,7 +8802,7 @@ public sealed class GameWindow : IDisposable
|
|||
uint lbX = (id >> 24) & 0xFFu;
|
||||
uint lbY = (id >> 16) & 0xFFu;
|
||||
return AcDream.Core.Terrain.LandblockMesh.Build(
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache, lb.BuildingTerrainCells);
|
||||
lb.Heightmap, lbX, lbY, _heightTable, _blendCtx, _surfaceCache);
|
||||
});
|
||||
_streamer.Start();
|
||||
|
||||
|
|
|
|||
|
|
@ -176,8 +176,7 @@ public sealed class GpuWorldState
|
|||
landblock = new LoadedLandblock(
|
||||
landblock.LandblockId,
|
||||
landblock.Heightmap,
|
||||
merged,
|
||||
landblock.BuildingTerrainCells);
|
||||
merged);
|
||||
_pendingByLandblock.Remove(landblock.LandblockId);
|
||||
}
|
||||
|
||||
|
|
@ -239,8 +238,7 @@ public sealed class GpuWorldState
|
|||
_loaded[kvp.Key] = new LoadedLandblock(
|
||||
kvp.Value.LandblockId,
|
||||
kvp.Value.Heightmap,
|
||||
newList,
|
||||
kvp.Value.BuildingTerrainCells);
|
||||
newList);
|
||||
|
||||
// Add to new (via AppendLiveEntity which handles pending)
|
||||
AppendLiveEntity(newCanonicalLb, entity);
|
||||
|
|
@ -341,7 +339,7 @@ public sealed class GpuWorldState
|
|||
foreach (var e in lb.Entities)
|
||||
if (e.ServerGuid != serverGuid) newList.Add(e);
|
||||
|
||||
_loaded[kvp.Key] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, newList, lb.BuildingTerrainCells);
|
||||
_loaded[kvp.Key] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, newList);
|
||||
rebuiltLoaded = true;
|
||||
}
|
||||
|
||||
|
|
@ -398,8 +396,7 @@ public sealed class GpuWorldState
|
|||
_loaded[canonicalLandblockId] = new LoadedLandblock(
|
||||
lb.LandblockId,
|
||||
lb.Heightmap,
|
||||
newEntities,
|
||||
lb.BuildingTerrainCells);
|
||||
newEntities);
|
||||
RebuildFlatView();
|
||||
return;
|
||||
}
|
||||
|
|
@ -463,8 +460,7 @@ public sealed class GpuWorldState
|
|||
_loaded[canonical] = new LoadedLandblock(
|
||||
lb.LandblockId,
|
||||
lb.Heightmap,
|
||||
System.Array.Empty<WorldEntity>(),
|
||||
lb.BuildingTerrainCells);
|
||||
System.Array.Empty<WorldEntity>());
|
||||
_pendingByLandblock.Remove(canonical);
|
||||
RebuildFlatView();
|
||||
}
|
||||
|
|
@ -500,7 +496,7 @@ public sealed class GpuWorldState
|
|||
var merged = new List<WorldEntity>(lb.Entities.Count + entities.Count);
|
||||
merged.AddRange(lb.Entities);
|
||||
merged.AddRange(entities);
|
||||
_loaded[canonical] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, merged, lb.BuildingTerrainCells);
|
||||
_loaded[canonical] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, merged);
|
||||
if (_wbSpawnAdapter is not null)
|
||||
_wbSpawnAdapter.OnLandblockLoaded(_loaded[canonical]);
|
||||
|
||||
|
|
|
|||
|
|
@ -231,8 +231,7 @@ public sealed class LandblockStreamer : IDisposable
|
|||
lb = new LoadedLandblock(
|
||||
lb.LandblockId,
|
||||
lb.Heightmap,
|
||||
System.Array.Empty<AcDream.Core.World.WorldEntity>(),
|
||||
lb.BuildingTerrainCells);
|
||||
System.Array.Empty<AcDream.Core.World.WorldEntity>());
|
||||
}
|
||||
_outbox.Writer.TryWrite(new LandblockStreamResult.Loaded(
|
||||
load.LandblockId, tier, lb, mesh));
|
||||
|
|
|
|||
|
|
@ -40,15 +40,13 @@ public static class LandblockMesh
|
|||
/// <param name="heightTable">Region.LandDefs.LandHeightTable — 256 float heights.</param>
|
||||
/// <param name="ctx">TerrainAtlas-derived blending inputs.</param>
|
||||
/// <param name="surfaceCache">Shared SurfaceInfo cache keyed by palette code.</param>
|
||||
/// <param name="hiddenTerrainCells">Optional cell indices (cy * 8 + cx) to draw as zero-area triangles.</param>
|
||||
public static LandblockMeshData Build(
|
||||
LandBlock block,
|
||||
uint landblockX,
|
||||
uint landblockY,
|
||||
float[] heightTable,
|
||||
TerrainBlendingContext ctx,
|
||||
System.Collections.Generic.IDictionary<uint, SurfaceInfo> surfaceCache,
|
||||
System.Collections.Generic.IReadOnlySet<int>? hiddenTerrainCells = null)
|
||||
System.Collections.Generic.IDictionary<uint, SurfaceInfo> surfaceCache)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(block);
|
||||
ArgumentNullException.ThrowIfNull(heightTable);
|
||||
|
|
@ -168,21 +166,9 @@ public static class LandblockMesh
|
|||
}
|
||||
}
|
||||
|
||||
// Indices are trivial 0..383 since we don't deduplicate verts. When
|
||||
// a building owns an outdoor terrain cell, keep the fixed 384-index
|
||||
// contract but collapse its two triangles so the building/stair mesh
|
||||
// can visually own the hole.
|
||||
// Indices are trivial 0..383 since we don't deduplicate verts.
|
||||
for (uint i = 0; i < VerticesPerLandblock; i++)
|
||||
{
|
||||
int cellIdx = (int)i / VerticesPerCell;
|
||||
if (hiddenTerrainCells is not null && hiddenTerrainCells.Contains(cellIdx))
|
||||
{
|
||||
indices[i] = (uint)(cellIdx * VerticesPerCell);
|
||||
continue;
|
||||
}
|
||||
|
||||
indices[i] = i;
|
||||
}
|
||||
|
||||
return new LandblockMeshData(vertices, indices);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,30 +23,8 @@ public static class LandblockLoader
|
|||
var entities = info is null
|
||||
? Array.Empty<WorldEntity>()
|
||||
: BuildEntitiesFromInfo(info, landblockId);
|
||||
var buildingTerrainCells = info is null
|
||||
? null
|
||||
: BuildBuildingTerrainCells(info);
|
||||
|
||||
return new LoadedLandblock(landblockId, block, entities, buildingTerrainCells);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Map LandBlockInfo.Buildings to 8x8 terrain mesh cells (cy * 8 + cx).
|
||||
/// Retail attaches each CBuildingObj to its outside landcell during
|
||||
/// CLandBlock::init_buildings; keep this signal separate from stabs so
|
||||
/// ordinary static props do not punch holes in terrain.
|
||||
/// </summary>
|
||||
public static IReadOnlySet<int> BuildBuildingTerrainCells(LandBlockInfo info)
|
||||
{
|
||||
var result = new HashSet<int>();
|
||||
foreach (var building in info.Buildings)
|
||||
{
|
||||
int cx = Math.Clamp((int)(building.Frame.Origin.X / 24f), 0, 7);
|
||||
int cy = Math.Clamp((int)(building.Frame.Origin.Y / 24f), 0, 7);
|
||||
result.Add(cy * 8 + cx);
|
||||
}
|
||||
|
||||
return result;
|
||||
return new LoadedLandblock(landblockId, block, entities);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
|
|||
|
|
@ -5,5 +5,4 @@ namespace AcDream.Core.World;
|
|||
public sealed record LoadedLandblock(
|
||||
uint LandblockId,
|
||||
LandBlock Heightmap,
|
||||
IReadOnlyList<WorldEntity> Entities,
|
||||
IReadOnlySet<int>? BuildingTerrainCells = null);
|
||||
IReadOnlyList<WorldEntity> Entities);
|
||||
|
|
|
|||
|
|
@ -54,35 +54,6 @@ public class LandblockMeshTests
|
|||
Assert.Equal(128 * 3, mesh.Indices.Length);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_HiddenTerrainCell_PreservesCountsAndDegeneratesOnlyThatCell()
|
||||
{
|
||||
var block = BuildFlatLandBlock();
|
||||
var cache = new Dictionary<uint, SurfaceInfo>();
|
||||
int hiddenCell = (3 * LandblockMesh.CellsPerSide) + 5;
|
||||
|
||||
var mesh = LandblockMesh.Build(
|
||||
block,
|
||||
0,
|
||||
0,
|
||||
IdentityHeightTable,
|
||||
MakeContext(),
|
||||
cache,
|
||||
new HashSet<int> { hiddenCell });
|
||||
|
||||
Assert.Equal(LandblockMesh.VerticesPerLandblock, mesh.Vertices.Length);
|
||||
Assert.Equal(LandblockMesh.VerticesPerLandblock, mesh.Indices.Length);
|
||||
|
||||
int hiddenBase = hiddenCell * LandblockMesh.VerticesPerCell;
|
||||
for (int i = 0; i < LandblockMesh.VerticesPerCell; i++)
|
||||
Assert.Equal((uint)hiddenBase, mesh.Indices[hiddenBase + i]);
|
||||
|
||||
int visibleCell = hiddenCell + 1;
|
||||
int visibleBase = visibleCell * LandblockMesh.VerticesPerCell;
|
||||
for (int i = 0; i < LandblockMesh.VerticesPerCell; i++)
|
||||
Assert.Equal((uint)(visibleBase + i), mesh.Indices[visibleBase + i]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_Vertices_CoverExactly192x192WorldUnits()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -117,35 +117,6 @@ public class LandblockLoaderTests
|
|||
Assert.Empty(entities);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildBuildingTerrainCells_UsesBuildingsOnlyAndMapsToMeshCellIndex()
|
||||
{
|
||||
var info = new LandBlockInfo
|
||||
{
|
||||
Objects =
|
||||
{
|
||||
new Stab
|
||||
{
|
||||
Id = 0x02000001u,
|
||||
Frame = new Frame { Origin = new Vector3(120, 72, 0) },
|
||||
},
|
||||
},
|
||||
Buildings =
|
||||
{
|
||||
new BuildingInfo
|
||||
{
|
||||
ModelId = 0x020000AAu,
|
||||
Frame = new Frame { Origin = new Vector3(141.5f, 7.2f, 94f) },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
var cells = LandblockLoader.BuildBuildingTerrainCells(info);
|
||||
|
||||
Assert.Single(cells);
|
||||
Assert.Contains(5, cells); // cy=0, cx=5 => mesh index cy * 8 + cx.
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildEntitiesFromInfo_WithLandblockId_NamespacesIdsForGlobalUniqueness()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue