Erik a4693954d8 fix(scenery): #48 unify scenery Z with physics-path triangle picker

Closes #48. Trees on sloped cells visibly hovered above the visible
terrain because GameWindow.SampleTerrainZ (the bilinear fallback used
during scenery hydration before physics registers a landblock) had
its diagonal arms swapped — used the SEtoNW triangle test on SWtoNE
cells and vice versa. The ACDREAM_DUMP_SCENERY_Z=1 diagnostic showed
every scenery line ran through the bilinear path (streaming race),
so on hilly terrain scenery was placed at a Z up to ~1.5 m off from
the visible mesh.

Latent since ff325ab (2026-04-17 "feat(ui): debug overlay + refined
input controls" carrying along the upgrade). That commit reached for
WorldBuilder TerrainUtils.GetHeight as the secondary oracle and
re-derived the triangle-pair tests; the named-retail / ACE algorithm
in TerrainSurface.SampleZ (used by the physics path / player Z) was
always correct, so player feet stayed flush — the two paths just
disagreed and only scenery noticed.

Fix:
- TerrainSurface.InterpolateZInTriangle (private static) — single
  source of truth for the triangle pick + barycentric Z, sourced
  from FUN_00532a50 / ACE LandblockStruct.ConstructPolygons.
- TerrainSurface.SampleZFromHeightmap (public static) — heightmap-
  byte-array variant for the scenery hydration fallback. Both this
  and TerrainSurface.SampleZ (instance) now delegate to the same
  InterpolateZInTriangle.
- GameWindow.SampleTerrainZ — thin wrapper over the new static.
- TerrainSurfaceTests.SampleZFromHeightmap_AgreesWithInstance_AcrossWholeLandblock
  asserts both sampler paths agree at 1500 sample points across both
  diagonals, so future drift gets caught.

The ACDREAM_DUMP_SCENERY_Z=1 diagnostic in BuildSceneryEntitiesForStreaming
is kept committed (env-var gated, zero cost when off) — useful for
the related #49 scenery (X, Y) placement investigation filed in the
same commit.

Visual verified at Holtburg landblock 0xA9B30001 2026-05-06: the
formerly floating 32 m pines (setups 0x020002D3 / 0x020002D9) now
sit flush on the visible terrain mesh.

Test baseline: dotnet test reports the same 8 pre-existing motion /
BSP step-up failures as the handoff doc warned about — no new
failures introduced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

2026-05-07 14:30:25 +02:00

4.2 KiB

Raw Blame History

Issue #48 fix — bilinear fallback triangle-pair test was swapped

Diagnosis (from the diagnostic dump)

ACDREAM_DUMP_SCENERY_Z=1 in landblock 0xA9B3 around the user's spawn showed:

Every scenery line had source=bilinear (physics engine had not registered the landblock at hydration time — typical streaming race).
The trees flagged as "floating" had multi-part setups whose lowest vertex extended into the ground per our calc (partWorldZMin = groundZ - 3.825) — i.e., per the diagnostic the tree should sit flush. The retail client at the same world coords does sit flush.
Player Z at the same world coords (~91 m) came from the physics sampler (TerrainSurface.SampleZ), which is correct.

That meant the bilinear fallback in GameWindow.SampleTerrainZ was producing a different ground Z than the visible terrain mesh and the player physics path on sloped cells, so trees were placed at a ground-Z that didn't match the terrain the player was walking on.

Root cause

Two terrain Z samplers exist:

AcDream.Core.Physics.TerrainSurface.SampleZ (instance) — used by the physics engine for player + entity ground-snap. Triangle-aware, matches the visible terrain mesh.
AcDream.App.Rendering.GameWindow.SampleTerrainZ (private bilinear fallback) — used by scenery hydration when physics has not yet built a TerrainSurface for a streaming-in landblock.

Both choose a per-cell diagonal with the AC2D FSplitNESW formula (constants 0x0CCAC033, 0x421BE3BD, 0x6C1AC587, 0x519B8F25). Both then choose one of the cell's two triangles based on the fractional position within the cell. The fallback's two arms were swapped relative to the chosen diagonal:

Diagonal	Correct dividing test	Correct triangles	Bilinear-fallback test (BUGGY)
`SWtoNE` (BL→TR, line y=x)	`tx > ty`	{BL,BR,TR} below / {BL,TR,TL} above	`s + t <= 1` (wrong — that's the SEtoNW test)
`SEtoNW` (BR→TL, line x+y=1)	`tx + ty <= 1`	{BL,BR,TL} below / {BR,TR,TL} above	`s >= t` (wrong — that's the SWtoNE test)

On sloped cells the wrong triangle's plane gives a Z that disagrees with the rendered terrain by up to ~1.5 m. Flat cells happen to mask the bug because all four corners share one Z.

Fix

Extract the correct triangle-picker math from TerrainSurface.SampleZ (instance) into a new public static method TerrainSurface.SampleZFromHeightmap(byte[] heights, float[] heightTable, uint landblockX, uint landblockY, float localX, float localY). Same algorithm, but reads the four corner heights directly from the landblock's raw heightmap byte array instead of the pre-resolved instance cache. One source of truth for the triangle math.
Replace GameWindow.SampleTerrainZ body with a call to that static.
Conformance test in tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs exercising a sloped heightmap on both diagonals, asserting that the new static and the existing instance method return the same Z at the same (localX, localY) (especially at points near the cell diagonal where the previous bug manifested).

Why this is the right fix

Retail (docs/research/named-retail/acclient_2013_pseudo_c.txt) places scenery via CLandBlock::get_land_scenes (0x00530460) → Plane::set_height(plane, &pos) (0x0052f050), which projects the position onto the terrain triangle's plane. The split direction comes from the same FSplitNESW formula. Our TerrainSurface.SampleZ is a faithful port of that algorithm and is already used by physics; the bilinear fallback should be identical.
The bug is purely in the fallback path. Player Z is unaffected.
No retail behavior changes; only acdream consistency.
"Don't break" constraints from the handoff are satisfied:
- Player Z snap untouched (different code path).
- Species that already render flush still do (their Z was correct on cells where bilinear and physics agreed; now it's correct everywhere).

Pre-existing bugs out of scope

The user reports separate (X, Y) misplacement at other locations. That's a different bug — likely in SceneryGenerator's placement math or one of the terrain-mesh / region tables — and outside the scope of this fix. File as a follow-up issue.

4.2 KiB Raw Blame History