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>
This commit is contained in:
Erik
parent
c1bb43ab89
commit
a4693954d8
5 changed files with 352 additions and 60 deletions
105
docs/ISSUES.md
105
docs/ISSUES.md
|
|
@ -46,9 +46,112 @@ Copy this block when adding a new issue:
|
|||
|
||||
# Active issues
|
||||
|
||||
## #48 — A few specific scenery trees hover above terrain (per-GfxObj Z misplacement)
|
||||
## #49 — Scenery (X, Y) placement drifts from retail at some landblocks
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** MEDIUM (visible misplacement; species-specific or per-cell, not a global offset)
|
||||
**Filed:** 2026-05-06
|
||||
**Component:** scenery placement / `SceneryGenerator`
|
||||
|
||||
**Description:** While verifying the `#48` Z fix at Holtburg
|
||||
landblock `0xA9B30001`, the user spotted a scenery tree placed at
|
||||
the **wrong (X, Y)** in acdream relative to retail at the same
|
||||
character coords. Specifically: a large tree that retail places far
|
||||
across the road on the right (east) side appears in acdream on the
|
||||
left (west) side, near a chess board / picnic-bench area. Side-by-
|
||||
side screenshot pair captured 2026-05-06.
|
||||
|
||||
This is **not** a Z bug — every tree in the same screenshot has its
|
||||
trunk meeting the visible terrain (the `#48` `SampleTerrainZ` fix is
|
||||
working). It's also **not** the LandBlockInfo Stab path — the chess
|
||||
board / bench themselves are correctly placed, so the landblock
|
||||
origin and `lbOffset` math are right.
|
||||
|
||||
**Hypotheses (need cdb retail trace to disambiguate):**
|
||||
|
||||
1. The displacement-noise math in `SceneryGenerator` differs from
|
||||
retail's `chunk_005A0000` LCG by a constant or a sign flip. Audit
|
||||
`eeee4c5` claimed "all MATCH" against the decomp, but a runtime
|
||||
trace would prove or disprove.
|
||||
2. Coordinate-system handedness: cell-local `(lx, ly)` in our path
|
||||
may map to retail's `(ly, lx)` somewhere, rotating tree XY 90°
|
||||
around the cell's NW corner.
|
||||
3. The `obj.Align != 0` path in retail (`FUN_005a6f60`, aligns the
|
||||
object to the landcell polygon's normal) may use a different
|
||||
reference point than ours, drifting placement on sloped cells.
|
||||
4. Slope filter could reject a cell retail accepts (or vice versa),
|
||||
pushing trees into adjacent cells.
|
||||
5. Region-table / `SceneInfo` lookup might select a different
|
||||
scenery list for the cell type.
|
||||
|
||||
**Investigation plan (gold-standard, per `project_retail_debugger.md`):**
|
||||
|
||||
1. Run the existing `ACDREAM_DUMP_SCENERY_Z=1` diagnostic to capture
|
||||
acdream's full per-spawn (gfx, world XY, scale, partT) for
|
||||
landblock `0xA9B3FFFF`.
|
||||
2. Attach cdb to a live retail client at the same Holtburg spot
|
||||
(`tools/pdb-extract/check_exe_pdb.py` confirms PDB pairs with
|
||||
v11.4186). Set a breakpoint on `CLandBlock::get_land_scenes` (or
|
||||
the inner `chunk_005A0000` placement function); capture every
|
||||
`(gfxObjId, worldX, worldY, scale, heading)` retail emits for
|
||||
the same landblock.
|
||||
3. Diff the two tables. The spawn that's offset will be obvious;
|
||||
the offset pattern (one tree, all trees, one species, constant
|
||||
delta, etc.) determines which hypothesis above is correct.
|
||||
|
||||
**Files:**
|
||||
|
||||
- [`src/AcDream.Core/World/SceneryGenerator.cs`](src/AcDream.Core/World/SceneryGenerator.cs) — placement math (LCG noise, displacement, rotation, scale, slope filter)
|
||||
- `acclient!CLandBlock::get_land_scenes` (`docs/research/named-retail/acclient_2013_pseudo_c.txt`) — retail entry point
|
||||
- `chunk_005A0000.c` — referenced retail source per `SceneryGenerator.cs` comments
|
||||
- [`docs/research/named-retail/symbols.json`](docs/research/named-retail/symbols.json) — for cdb breakpoints
|
||||
|
||||
**Acceptance:** Side-by-side outdoor screenshot pair (acdream vs
|
||||
retail, same character coords, same time of day) shows scenery
|
||||
positions matching at multiple landblocks. The cdb trace + diagnostic
|
||||
diff documents quantitative agreement (zero offset within float
|
||||
precision) on at least one landblock end-to-end.
|
||||
|
||||
**Out of scope here (kept under `#48`):** Z floating. That's fixed.
|
||||
|
||||
---
|
||||
|
||||
## #48 — [DONE 2026-05-06 · (this commit)] A few specific scenery trees hover above terrain (per-GfxObj Z misplacement)
|
||||
|
||||
**Resolution:** Hypothesis 2 (physics-sampler vs bilinear-fallback Z
|
||||
mismatch). The bilinear fallback in `GameWindow.SampleTerrainZ` had
|
||||
its two diagonal arms swapped — used the SEtoNW triangle test on
|
||||
SWtoNE cells and vice versa. Every scenery hydration in our
|
||||
diagnostic ran through the bilinear path (`source=bilinear` in all
|
||||
`[scenery-z]` log lines) because physics hadn't yet built a
|
||||
`TerrainSurface` for the streaming-in landblock — so on sloped
|
||||
cells, scenery sat at a different Z than the visible terrain mesh
|
||||
by up to ~1.5 m. The bug was latent since `ff325ab` (2026-04-17)
|
||||
which upgraded the fallback from naive 4-corner bilinear to
|
||||
triangle-aware barycentric, but with the diagonal-pair tests
|
||||
swapped. `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: extracted the canonical triangle-pick math into
|
||||
`TerrainSurface.InterpolateZInTriangle` (private static); added
|
||||
`TerrainSurface.SampleZFromHeightmap` (public static) that reads
|
||||
heights directly from the landblock byte array using the same
|
||||
canonical math; redirected `GameWindow.SampleTerrainZ` to delegate
|
||||
to it. New conformance test
|
||||
`SampleZFromHeightmap_AgreesWithInstance_AcrossWholeLandblock` pins
|
||||
both sampler paths together at 1500 sample points across both
|
||||
diagonals, so future drift gets caught. User visually confirmed
|
||||
2026-05-06.
|
||||
|
||||
The diagnostic dump (`ACDREAM_DUMP_SCENERY_Z=1`,
|
||||
`GameWindow.cs:4661`) is kept committed — it's gated by env var,
|
||||
zero cost when off, and is the right starting point for `#49`
|
||||
(scenery X/Y placement) too.
|
||||
|
||||
Pseudocode: [`docs/research/2026-05-06-issue-48-fix-pseudocode.md`](docs/research/2026-05-06-issue-48-fix-pseudocode.md).
|
||||
|
||||
**Status:** DONE
|
||||
**Severity:** LOW (cosmetic; ~3 trees per landblock, easy to ignore but obvious once spotted)
|
||||
**Filed:** 2026-05-06
|
||||
**Component:** rendering / scenery placement / terrain Z sampling
|
||||
|
|
|
|||
86
docs/research/2026-05-06-issue-48-fix-pseudocode.md
Normal file
86
docs/research/2026-05-06-issue-48-fix-pseudocode.md
Normal file
|
|
@ -0,0 +1,86 @@
|
|||
# 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:
|
||||
|
||||
1. `AcDream.Core.Physics.TerrainSurface.SampleZ` (instance) — used by the
|
||||
physics engine for player + entity ground-snap. Triangle-aware,
|
||||
matches the visible terrain mesh.
|
||||
2. `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
|
||||
|
||||
1. 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.
|
||||
2. Replace `GameWindow.SampleTerrainZ` body with a call to that static.
|
||||
3. 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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue