From 833d167ebc7ba3b5e732abc6daf23aa2b6af2070 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 7 May 2026 21:15:11 +0200 Subject: [PATCH] =?UTF-8?q?fix(scenery):=20#49=209=C3=979=20loop,=20per-sp?= =?UTF-8?q?awn=20building=20check,=20triangle=20slope?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes to match retail CLandBlock::get_land_scenes (0x00530460): 1. Loop bound: iterate 9×9 vertices (side_vertex_count=9), not 8×8 cells. Edge vertices (x=8 or y=8) produce valid spawns when the per-object displacement shifts the position back into [0, 192). Confirmed by named retail decomp do-while condition, WorldBuilder vertLength=9, ACViewer Terrain.Count=81, AC2D wTopo[9][9]. 2. Building suppression: check at the DISPLACED position's cell (CSortCell::has_building per spawn), not at the loop vertex index. Matches WorldBuilder buildingsGrid[gx2, gy2] pattern. 3. Slope filter: replace finite-difference gradient approximation with triangle-aware normal sampling via new static method TerrainSurface.SampleNormalZFromHeightmap. Picks the correct triangle via IsSplitSWtoNE, matching retail find_terrain_poly → polygon->plane.N.z and WorldBuilder's GetNormal(). Tests: 5 new tests for SampleNormalZFromHeightmap (flat=1.0, sloped<1, cross-validates with SampleSurface instance method) and DisplaceObject edge-vertex validity. Co-Authored-By: Claude Opus 4.6 --- src/AcDream.Core/Physics/TerrainSurface.cs | 67 ++++++++++++++++++ src/AcDream.Core/World/SceneryGenerator.cs | 69 +++++++----------- .../Physics/TerrainSurfaceTests.cs | 46 ++++++++++++ .../World/SceneryGeneratorTests.cs | 70 +++++++++++++++++-- 4 files changed, 203 insertions(+), 49 deletions(-) diff --git a/src/AcDream.Core/Physics/TerrainSurface.cs b/src/AcDream.Core/Physics/TerrainSurface.cs index fe51188..525569e 100644 --- a/src/AcDream.Core/Physics/TerrainSurface.cs +++ b/src/AcDream.Core/Physics/TerrainSurface.cs @@ -198,6 +198,73 @@ public sealed class TerrainSurface return InterpolateZInTriangle(hBL, hBR, hTR, hTL, tx, ty, splitSWtoNE); } + /// + /// Sample the terrain triangle's surface-normal Z component at (localX, localY) + /// from a raw heightmap. Returns the upward component of the unit normal for + /// the specific triangle the point lies in — flat ground returns 1.0, steeper + /// slopes return smaller values. Used by for + /// the retail slope filter (CLandCell::find_terrain_poly → polygon.plane.N.z). + /// + public static float SampleNormalZFromHeightmap( + byte[] heights, float[] heightTable, + uint landblockX, uint landblockY, + float localX, float localY) + { + ArgumentNullException.ThrowIfNull(heights); + ArgumentNullException.ThrowIfNull(heightTable); + if (heights.Length < 81) + throw new ArgumentException("heights must have 81 entries", nameof(heights)); + if (heightTable.Length < 256) + throw new ArgumentException("heightTable must have 256 entries", nameof(heightTable)); + + float fx = Math.Clamp(localX / CellSize, 0f, CellsPerSide - 0.001f); + float fy = Math.Clamp(localY / CellSize, 0f, CellsPerSide - 0.001f); + int cx = (int)fx; + int cy = (int)fy; + cx = Math.Clamp(cx, 0, CellsPerSide - 1); + cy = Math.Clamp(cy, 0, CellsPerSide - 1); + + float tx = fx - cx; + float ty = fy - cy; + + float hBL = heightTable[heights[cx * HeightmapSide + cy ]]; + float hBR = heightTable[heights[(cx+1) * HeightmapSide + cy ]]; + float hTR = heightTable[heights[(cx+1) * HeightmapSide + (cy+1)]]; + float hTL = heightTable[heights[cx * HeightmapSide + (cy+1)]]; + + bool splitSWtoNE = IsSplitSWtoNE(landblockX, (uint)cx, landblockY, (uint)cy); + + float dzdx, dzdy; + if (splitSWtoNE) + { + if (tx > ty) + { + dzdx = (hBR - hBL) / CellSize; + dzdy = (hTR - hBR) / CellSize; + } + else + { + dzdx = (hTR - hTL) / CellSize; + dzdy = (hTL - hBL) / CellSize; + } + } + else + { + if (tx + ty <= 1f) + { + dzdx = (hBR - hBL) / CellSize; + dzdy = (hTL - hBL) / CellSize; + } + else + { + dzdx = (hTR - hTL) / CellSize; + dzdy = (hTR - hBR) / CellSize; + } + } + + return 1f / MathF.Sqrt(dzdx * dzdx + dzdy * dzdy + 1f); + } + /// /// Pick the cell's triangle for the chosen diagonal and barycentric- /// interpolate Z. Single source of truth shared by both diff --git a/src/AcDream.Core/World/SceneryGenerator.cs b/src/AcDream.Core/World/SceneryGenerator.cs index 5c88128..610d4fe 100644 --- a/src/AcDream.Core/World/SceneryGenerator.cs +++ b/src/AcDream.Core/World/SceneryGenerator.cs @@ -28,12 +28,6 @@ namespace AcDream.Core.World; /// dividing by 2^32. This is equivalent to our unchecked((uint)(...)) cast. /// ACViewer's reference omits this cast and is subtly wrong for negative inputs. /// We deliberately match the decompiled client, not ACViewer. -/// -/// We deliberately skip the slope/road/building-overlap checks the original does; -/// those prevent scenery from floating in roads or clipping buildings but -/// require walkable-polygon lookups that we don't yet have. Accepting visual -/// artifacts (trees inside roads, scenery clipping buildings) for a first pass -/// and deferring the filters to a later phase. /// public static class SceneryGenerator { @@ -72,14 +66,15 @@ public static class SceneryGenerator uint blockX = (landblockId >> 24) * 8; // 8 cells per landblock uint blockY = ((landblockId >> 16) & 0xFFu) * 8; - // RETAIL iterates 8×8 = 64 CELLS, not 9×9 = 81 vertices. - // Decompiled FUN_005311a0 at chunk_00530000.c:1123-1253 uses - // `while (local_94 < 8)` and `while (local_8c < 8)` — bound by - // `param_1+0x40` which is SideCellCount=8 for outdoor landblocks. - // The terrain word at each cell's SW corner drives that cell's scenery. - for (int x = 0; x < CellsPerSide; x++) + // RETAIL iterates 9×9 = 81 VERTICES, not 8×8 = 64 cells. + // Named retail: CLandBlock::get_land_scenes (0x00530460) uses + // `side_vertex_count` (offset 0x40, value 9) as the loop bound. + // The do-while condition `(var+1) < side_vertex_count` runs var 0..8. + // Edge vertices (x=8 or y=8) produce valid spawns when the per-object + // displacement shifts the position back into the [0, 192) range. + for (int x = 0; x < VerticesPerSide; x++) { - for (int y = 0; y < CellsPerSide; y++) + for (int y = 0; y < VerticesPerSide; y++) { int i = x * VerticesPerSide + y; ushort raw = block.Terrain[i]; @@ -92,9 +87,6 @@ public static class SceneryGenerator // polygonal OnRoad check (see below). Removing the // pre-displacement early-exit restores retail behavior. - // Skip cells that contain buildings. - if (buildingCells is not null && buildingCells.Contains(i)) continue; - if (terrainType >= region.TerrainInfo.TerrainTypes.Count) continue; var sceneTypeList = region.TerrainInfo.TerrainTypes[(int)terrainType].SceneTypes; if (sceneType >= sceneTypeList.Count) continue; @@ -166,34 +158,27 @@ public static class SceneryGenerator continue; } - // L-fix2 (2026-04-28): the extra cell-origin road-vertex - // guard previously here is REMOVED. It wasn't in the - // retail decomp — it was a heuristic added to widen - // road margins visually. The proper retail post- - // displacement road check (FUN_00530d30 port via - // IsOnRoad above) already handles road exclusion. - // The extra guard was over-suppressing — every cell - // whose SW corner happened to touch a road vertex - // had ALL of its scenery dropped, even when the - // displaced position was well clear of the ribbon. - // User reported missing trees they could see in - // retail; this is the most likely cause. + // Per-spawn building check on the DISPLACED position's cell. + // Retail: CSortCell::has_building(cell) per spawn, not per vertex. + // WorldBuilder: buildingsGrid[gx2, gy2] with 8×8 cell grid. + if (buildingCells is not null) + { + int dcx = Math.Clamp((int)(lx / CellSize), 0, CellsPerSide - 1); + int dcy = Math.Clamp((int)(ly / CellSize), 0, CellsPerSide - 1); + if (buildingCells.Contains(dcx * VerticesPerSide + dcy)) + continue; + } - // Slope filter (ACME conformance fix 4e): compute terrain normal - // Z-component at the displaced position and check against the - // object's MinSlope/MaxSlope bounds. + // Slope filter: retail uses CLandCell::find_terrain_poly → + // polygon->plane.N.z to get the triangle-specific normal. + // SampleNormalZFromHeightmap picks the correct triangle via + // the cell's split direction, matching retail + WorldBuilder. if (heightTable is not null && (obj.MinSlope > 0f || obj.MaxSlope < 1f)) { - int sx = Math.Clamp((int)(lx / CellSize), 0, VerticesPerSide - 2); - int sy = Math.Clamp((int)(ly / CellSize), 0, VerticesPerSide - 2); - int sxR = sx + 1; - int syU = sy + 1; - float h00 = heightTable[block.Height[sx * VerticesPerSide + sy]]; - float h10 = heightTable[block.Height[sxR * VerticesPerSide + sy]]; - float h01 = heightTable[block.Height[sx * VerticesPerSide + syU]]; - float dx = (h10 - h00) / CellSize; - float dy = (h01 - h00) / CellSize; - float nz = 1f / MathF.Sqrt(dx * dx + dy * dy + 1f); // normal Z component + float nz = AcDream.Core.Physics.TerrainSurface.SampleNormalZFromHeightmap( + block.Height, heightTable, + landblockId >> 24, (landblockId >> 16) & 0xFFu, + lx, ly); if (nz < obj.MinSlope || nz > obj.MaxSlope) continue; } @@ -396,7 +381,7 @@ public static class SceneryGenerator /// Decompiled normalises signed-int LCG results with "if (val < 0) val += 2^32"; our /// unchecked((uint)(...)) is exactly equivalent. /// - private static Vector3 DisplaceObject(ObjectDesc obj, uint ix, uint iy, uint iq) + internal static Vector3 DisplaceObject(ObjectDesc obj, uint ix, uint iy, uint iq) { float x, y; var baseLoc = obj.BaseLoc.Origin; diff --git a/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs b/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs index c26b214..17a756c 100644 --- a/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs +++ b/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs @@ -139,6 +139,52 @@ public class TerrainSurfaceTests Assert.Contains(sample.Vertices, v => v.X == 0f && v.Y == 0f); } + [Fact] + public void SampleNormalZFromHeightmap_FlatTerrain_ReturnsOne() + { + var heights = FlatHeightmap(50); + var hTable = LinearHeightTable(); + float nz = TerrainSurface.SampleNormalZFromHeightmap(heights, hTable, 0, 0, 96f, 96f); + Assert.Equal(1f, nz, precision: 5); + } + + [Fact] + public void SampleNormalZFromHeightmap_SlopedTerrain_ReturnsLessThanOne() + { + var heights = new byte[81]; + for (int x = 0; x < 9; x++) + for (int y = 0; y < 9; y++) + heights[x * 9 + y] = (byte)(x * 20); + var hTable = LinearHeightTable(); + float nz = TerrainSurface.SampleNormalZFromHeightmap(heights, hTable, 0, 0, 12f, 12f); + Assert.True(nz < 1f, $"Sloped terrain should have nz < 1.0, got {nz}"); + Assert.True(nz > 0f, $"nz should be positive, got {nz}"); + } + + [Fact] + public void SampleNormalZFromHeightmap_AgreesWithSampleSurface() + { + var heights = new byte[81]; + for (int x = 0; x < 9; x++) + for (int y = 0; y < 9; y++) + heights[x * 9 + y] = (byte)((x * 17 + y * 13) % 256); + + var hTable = LinearHeightTable(); + const uint lbX = 0xA9, lbY = 0xB3; + var instance = new TerrainSurface(heights, hTable, lbX, lbY); + + for (float lx = 0.5f; lx < 192f; lx += 8f) + for (float ly = 0.5f; ly < 192f; ly += 8f) + { + var (_, normal) = instance.SampleSurface(lx, ly); + float staticNz = TerrainSurface.SampleNormalZFromHeightmap( + heights, hTable, lbX, lbY, lx, ly); + Assert.True( + Math.Abs(normal.Z - staticNz) < 0.0001f, + $"NormalZ mismatch at ({lx:F1},{ly:F1}): instance={normal.Z:F4} static={staticNz:F4}"); + } + } + [Fact] public void ComputeOutdoorCellId_Origin_ReturnsFirst() { diff --git a/tests/AcDream.Core.Tests/World/SceneryGeneratorTests.cs b/tests/AcDream.Core.Tests/World/SceneryGeneratorTests.cs index d003ea8..3963a5c 100644 --- a/tests/AcDream.Core.Tests/World/SceneryGeneratorTests.cs +++ b/tests/AcDream.Core.Tests/World/SceneryGeneratorTests.cs @@ -1,13 +1,13 @@ +using System.Numerics; using AcDream.Core.World; using DatReaderWriter.Types; namespace AcDream.Core.Tests.World; /// -/// Tests for SceneryGenerator road-exclusion logic. -/// The full Generate() pipeline requires real dat files (Region, Scene, etc.) -/// so road-check behavior is tested via the internal IsRoadVertex helper, -/// which is the single gate that guards against placing trees on roads. +/// Tests for SceneryGenerator: road-exclusion, loop bounds, building +/// suppression, and slope filter. The full Generate() pipeline requires +/// real dat files so behavior is tested via internal helpers. /// public class SceneryGeneratorTests { @@ -32,15 +32,12 @@ public class SceneryGeneratorTests [Fact] public void IsRoadVertex_ZeroTerrain_IsNotRoad() { - // A fully blank terrain entry (no type, no road, no scene) is not a road. Assert.False(SceneryGenerator.IsRoadVertex(0)); } [Fact] public void IsRoadVertex_MatchesTerrainInfoRoadProperty() { - // Verify that IsRoadVertex agrees with the typed TerrainInfo.Road property - // for a sample of raw values, ensuring the bit convention is consistent. for (ushort raw = 0; raw < 4; raw++) { TerrainInfo ti = raw; @@ -50,4 +47,63 @@ public class SceneryGeneratorTests $"raw=0x{raw:X4}: IsRoadVertex={actual} but TerrainInfo.Road={ti.Road}"); } } + + // --- Edge vertex displacement tests --- + // Retail iterates 9×9 vertices (0..8 on each axis). Vertices at x=8 or y=8 + // have base positions at 192 (= 8 * 24), which is AT the landblock boundary. + // These produce valid scenery when displacement shifts them back into [0, 192). + + [Fact] + public void DisplaceObject_EdgeVertex_CanProduceValidPosition() + { + // Vertex (3, 8): base_y = 8 * 24 = 192. + // With DisplaceY > 0, some LCG seeds will produce negative displacement, + // shifting the Y back below 192 into the valid range. + var obj = new ObjectDesc + { + DisplaceX = 12f, + DisplaceY = 12f, + BaseLoc = new Frame { Origin = new Vector3(0, 0, 0) } + }; + + // Search across a range of global cell coords to find at least one + // case where vertex y=8 displaces into [0, 192). + bool foundValid = false; + for (uint gx = 0; gx < 64 && !foundValid; gx++) + { + for (uint gy = 0; gy < 64 && !foundValid; gy++) + { + var localPos = SceneryGenerator.DisplaceObject(obj, gx, gy, 0); + // Vertex (3, 8): cell corner at (3*24, 8*24) = (72, 192) + float lx = 3 * 24f + localPos.X; + float ly = 8 * 24f + localPos.Y; + if (ly >= 0 && ly < 192f && lx >= 0 && lx < 192f) + foundValid = true; + } + } + + Assert.True(foundValid, + "Expected at least one (globalCellX, globalCellY) where vertex y=8 " + + "displaces back into [0, 192) — retail's 9×9 loop relies on this"); + } + + [Fact] + public void DisplaceObject_InteriorVertex_AlwaysNearOrigin() + { + var obj = new ObjectDesc + { + DisplaceX = 12f, + DisplaceY = 12f, + BaseLoc = new Frame { Origin = new Vector3(0, 0, 0) } + }; + + // For interior vertices (x < 8, y < 8), displacement is bounded by + // DisplaceX/Y (max 12 units each), so the result stays within one + // cell of the origin. + var localPos = SceneryGenerator.DisplaceObject(obj, 100, 100, 0); + Assert.True(Math.Abs(localPos.X) <= 12f, + $"Interior displacement X={localPos.X} exceeds DisplaceX=12"); + Assert.True(Math.Abs(localPos.Y) <= 12f, + $"Interior displacement Y={localPos.Y} exceeds DisplaceY=12"); + } }