Files
Syplheed-Reborn/docs/re/BACKLOG.md
Claude (auto-RE) 64d372c7e8 revert(mesh): withdraw the neighbourhood anchor -- it regressed the e106 twin mirror
The neighbourhood anchor (f18d591) and its refinement (27a0701) took
cross-container inconsistency from 125 to 51 with coverage unchanged, and made
e106 render as a destroyer rather than a slab. Both are reverted.

ship::tests::static_assembly_matches_runtime_capture is gated on SYLPHEED_ISO, so
it SKIPS in an ordinary cargo test -- which is why the regression was invisible
in every suite run so far. With the ISO it fails:

  e106_bdy_01: static M row0 [-1.0, 0.0, 0.0] != captured [1.0, 0.0, 0.0]

e106_bdy_01 and _02 are a mirrored pair whose two buffers hold the same geometry
reflected in X, and BOTH resources currently decode to the SAME buffer (identical
counts, span and mean_x). apply_twin_mirrors picks which instance to reflect from
the sign of that mean_x, so which buffer wins flips the decision:

  before  both twins mean_x = -66.83  -> mirror bdy_02  (matches the capture)
  after   both twins mean_x = +66.83  -> mirror bdy_01  (contradicts it)

Neither is right -- two resources sharing one decode is itself the bug and the
mirror heuristic has been compensating. The capture is ground truth, so a change
that contradicts it does not ship. The real fix must give each twin its own
buffer first.

Kept from the attempt: this test now also asserts the SET of static placements
against the capture (allow-list {e303_wep_01} for vbase dedup), so extra
placements can finally fail it -- the direction it could never fail in before.

Docs, backlog, INDEX and the ignored test's message all corrected to say
diagnosed-not-fixed rather than fixed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-12 01:08:41 +00:00

283 lines
15 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# RE backlog
Open items that are *not* being worked right now. Each entry says what is wrong or
unknown, what evidence exists, and what the first step would be. Move an item into
`INDEX.md` (with a `structures/…md` or a parser + test) once it is actually settled.
---
## Capital ships assemble wrong in the viewer
**Reported:** 2026-07-30, by the user. **Status:** 🔎 **diagnosed 2026-08-10 — the
format layer is exonerated.** Runtime captures of three classes (`f105`, `e105`,
`e106`) at controlled range reproduce `assemble_ship` to ≤0.43 units in translation
and to 0.000 in rotation for every part that does not move; see
[`ship-placement-capture-generalisation.md`](ship-placement-capture-generalisation.md)
§4. So look at **the viewer**: first that it passes `include_external = true`
(`iso_loader.rs:4012` — with `false` an e106 loses its bridge and both nacelles,
5 parts instead of 11), then its own transform stack.
One real format-side bug was found on the way and is **fixed**: index-less parts
(`e105_brg`) never matched their `GN_Bridge_01` hardpoint, so 34 (stage, ship) entries
`e102`, `e104`, `e105` across Stages 0229 — assembled without a bridge. The other
apparent exception (`e105_eng_01` rotation) was an aggregation artefact and is 0.000.
The original report and its reasoning follow.
The reborn viewer builds capital ships from the split XBG7 parts via
`sylpheed-formats::ship::assemble_ship`, and they come out **wrong** — parts in the
wrong place / wrong orientation.
**Why this is a real finding and not a known limitation:** the RE write-up
[`ship-placement-runtime-capture.md`](ship-placement-runtime-capture.md) declares
static assembly ✅ **exact** as of 2026-07-26 — 9-channel joint tables
`[TX TY TZ RY RX RZ SX SY SZ]`, Euler `Ry·Rx·Rz`, with
`ship::tests::static_assembly_matches_runtime_capture` asserting static == runtime
capture (T < 1.0, R < 0.02). So either the viewer is not using that path, or the
claim generalises worse than the test suggests.
**The likely gap:** that test is **one ship** — the `e106` destroyer, 8 parts plus
two nacelles, two turrets and the hull mirror. Nothing pins the other classes.
Rules that were derived from `e106` and could easily be `e106`-specific:
- the engine cluster rig mounted at `GN_Engine_01` (two mirrored nacelles + centre);
- "X-reflect the shared-geometry twin whose lateral offset opposes the geometry's
dominant side" — a heuristic, not a decoded flag;
- cross-id turret instancing (×2).
**First step (the oracle already exists):** re-run the runtime capture on a *different*
capital ship and diff static vs captured, exactly as `e106` was done — F10 in the
`capture-ship-placement` build of `xenia-canary-native` dumps the ship shader's
`c0..c2` WorldViewProjection rows per part; `WV_ref⁻¹ · WV_p` is the ship-space rigid
transform, which is ground truth. Pick a class whose rig differs from `e106`
(different engine count, a ship with no `sld`, a carrier). Then extend
`static_assembly_matches_runtime_capture` into a per-ship table so a regression in one
class cannot hide behind `e106` passing.
**Also worth ruling out first, cheaply:** that the viewer's own transform stack (scale,
handedness, node-instance recursion) is not re-breaking a correct assembly — compare
the viewer's placement against `assemble_ship`'s output directly before blaming the
format layer.
---
## Viewer: `include_external` is already on — that hypothesis is dead
**Checked 2026-08-11.** The item above names "first that it passes
`include_external = true` (`iso_loader.rs:4012`)" as the cheap first step. It
does: `ShipBrowser::show_external` defaults to `true`
(`iso_loader.rs:643`), the checkbox reads it (`ui.rs:1593`) and it is threaded
through `RequestShipRender``build_ship_model``assemble_ship` unchanged
(`ui.rs:1689`, `iso_loader.rs:4012`). So a ship rendered by the viewer is the
full external assembly, not the bare hull.
The viewer also does not have a transform stack of its own to blame: it bakes
`ScenePart::apply` straight into the vertices and rotates normals by the same
`p.m` (`iso_loader.rs:4030-4062`), so its placement is `assemble_ship`'s output
by construction. What remains unexcluded, in order of cheapness: the mirror
handling (`det < 0` reverses triangle winding only — a reflected part keeps its
reflected geometry), `Xbg7Model::models_named` resolving the wrong sub-model when
a resource name repeats, and the exhaust cones. **Next step is a visual**: the
diagnosis has run out of things it can settle by reading, so the viewer needs to
be run against a known-good class (`e106`) and its render compared with
`ship_render`'s.
---
## Viewer: the duplicate-resource-name hypothesis is dead too
**Checked 2026-08-11.** The diagnosis above left three candidates for why capital
ships assemble wrong in the viewer: mirror handling, `Xbg7Model::models_named`
resolving the wrong sub-model when a resource name repeats, and the exhaust
cones. The second is now **refuted**, and comprehensively.
`build_ship_model` resolves each placement with
`base.iter().find(|m| m.name == p.resource)` (`iso_loader.rs:4041`) — first match
wins — so a repeated resource name inside a container would silently draw the
wrong geometry. It cannot happen: decoding **every** XBG7 resource in **all 22
stage containers** gives **4 603 resources and zero repeated names**.
```
Stage_S01 62/62 Stage_S07 323/323 Stage_S13 290/290 Stage_S25 351/351
Stage_S02 304/304 Stage_S08 388/388 Stage_S14 22/22 Stage_S26 318/318
Stage_S03 214/214 Stage_S09 316/316 Stage_S15 386/386 Stage_S27 321/321
Stage_S04 179/179 Stage_S10 7/7 Stage_S16 65/65 Stage_S28 118/118
Stage_S05 92/92 Stage_S11 157/157 Stage_S24 162/162 Stage_S29 386/386
Stage_S06 266/266 Stage_S12 376/376
```
Per-ship it is tighter still: `e106` wants 9 distinct names and decodes exactly
9 models for 11 placements; `e105` 9 for 9; `f105` 5 for 6. Every placement
resolves to the one model it names.
**So two of the three candidates are gone** (this one and `include_external`),
leaving **mirror handling** and **the exhaust cones** — and the still-untried
visual comparison, which remains the right next step.
---
## Viewer: mirror handling and the exhaust cones are cleared too — the static avenue is exhausted
**Checked 2026-08-11.** Both remaining candidates were tested across every ship
on the disc, and neither shows the reported signature.
**Mirror handling.** The concern was that `ScenePart::apply` bakes `R·(S·v)+T`
while the viewer takes its winding-flip decision from `det(m)` alone and rotates
normals by `m` alone — both ignoring `s`. A mirror encoded as a *negative scale*
would then reflect geometry without flipping winding, drawing the part
inside-out. It never happens: across **1 485 assembled parts** in all 22
containers there are **22 mirrored parts, every one with `det(m) < 0`**, and
**zero** parts with a negative scale or a non-uniform one. `apply_twin_mirrors`
writes the reflection into `m` (negating its X column), so the viewer's flip
always fires, and ignoring `s` for normals is harmless because `s` is always
uniform.
**Exhaust cones.** These are the one piece of geometry the viewer *invents* — a
cone at each `GN_Jet`/`GN_SJet` frame, because the real engine geometry is
recessed and the game draws FX there instead. If they landed wrongly they would
read exactly as "a part in the wrong place". Across **335 assembled ships, 192 of
which have exhaust frames, not one cone sits outside its hull's bounding box**
(tolerance 10 % of the axis span).
**Caveat, stated rather than glossed:** "inside the hull box" does not prove a
cone is *right* — orientation and size are untested, and a cone could be wrong
while still inside. What it does rule out is the reported symptom for that part.
So every mechanism this diagnosis proposed is now eliminated: `include_external`,
duplicate resource names, mirror handling, and cones-in-the-wrong-place. The
format and assembly layers pass every static test available, and **the visual
comparison is no longer merely the next step — it is the only remaining one.**
Render `e106` in the viewer beside `ship_render`'s output of the same
`assemble_ship` result; if they agree, the bug is in neither and the original
report needs re-grounding against a specific ship and a specific expectation.
---
## ⚠️ DIAGNOSED 2026-08-12 — a mis-decode; the locality fix was written, then withdrawn
> Resolution at the end of this entry. Kept in full because the two wrong turns
> along the way (a "stray volume", then "monotonic anchoring") are the useful part.
## ⚠️ The format layer is NOT exonerated — but the cause is a MIS-DECODE, not a stray volume
**Found 2026-08-11 by finally doing the visual**, which the notes above kept
naming as the next step. It overturns their conclusion.
Render `e106` from the static assembly and from the baked runtime capture and
compare — `ship_render` does both:
| | placements | parts |
|---|---|---|
| runtime capture (ground truth) | **8** | `bdy_01…04`, `brg_01`, `eng_01`, `eng_02`, `wep_02_01` |
| `assemble_ship(--static)` | **11** | the same 8, **plus `e303_wep_01` ×2** and a second `e106_eng_01` |
The render makes it obvious: the destroyer sits inside a white slab that dwarfs
it ([capture](captures/e106-static-assembly-volume-bug.png)). That slab is
`e303_wep_01`, and its own geometry is:
```
e303_wep_01 172 verts, 110 tris bounds X[-1000, 600] Y[-1050, 1050] Z[-2400, 2400] 1600 x 2100 x 4800
e106_wep_02_01 1002 verts, 772 tris 269 x 179 x 417 ← what a real e106 turret looks like
e106_brg_01 202 verts, 202 tris 105 x 76 x 305
```
**110 triangles, perfectly round axis-aligned bounds, and bigger than the ship it
is mounted on.**
### CORRECTION (same day, one iteration later): it is not a volume — it is a bad decode
The first reading of this was that `e303_wep_01` is a collision/trigger volume
the assembler wrongly draws. **That is wrong, and the evidence that settles it is
decoding the same resource from every container that holds it:**
```
Stage_S01 172 verts 110 tris X[-24.5, 24.5] Y[0.0, 23.4] Z[-20.8, 20.8] ← 49 × 23 × 42, a turret
Stage_S02 172 verts 110 tris X[-1000, 600] Y[±1050] Z[±2400] ← 1600 × 2100 × 4800
Stage_S03… 172 verts 110 tris 49 × 23 × 42 (correct)
Stage_S08 … 1600 × 2100 × 4800
Stage_S26 … 1600 × 2100 × 4800
```
Same resource, same vertex and triangle count, **decoding correctly in eleven
containers and wrongly in exactly three** (`Stage_S02`, `S08`, `S26`). So:
- the **placement is legitimate**`e303_wep_01` is a small shared turret,
cross-mounted on `e101` and `e106`, and at its true size it is unremarkable;
- the original author's explanation of the capture's silence (**vbase dedup**)
stands, and my "dedup would show one, not zero" objection does not survive:
with the correct decode the turret is small, ordinary geometry;
- **the defect is in the mesh decoder**, which resolved this resource's vertex
data differently in three containers.
The render and the symptom are real; the cause named in the first version of this
entry was not.
### The part that matters more than this one resource
**The decoder can produce wrong geometry without declining.** The
[XBG7 audit](structures/xbg7-mesh.md) counted 814 resources it *refuses* — a
visible, honest failure. This is the other kind: `e303_wep_01` decodes "fine" in
`Stage_S02` and is silently 100× too large. Screening for the signature (bounds
that are exact multiples of 50 with a span over 1000) flags 2232 models in each
of `S02`, `S03`, `S08`, `S26`, `S27` — **but that screen also catches legitimate
`e_rou_*` composite proxies**, so it is a candidate list, not a count of bugs.
**Next:** diff the anchor scan's chosen `vb0` for `e303_wep_01` between
`Stage_S01` (correct) and `Stage_S02` (wrong) — same resource, two outcomes, so
the divergence is directly observable — then use whatever distinguishes them to
add a post-decode sanity check, so a silent 100× mis-decode becomes a decline.
### Why this was missed
`assemble_ship` treats **every** `rou_*` node in the composite as a drawable
part, and the doc comment states the cross-id mount as intended behaviour —
`"INCLUDING repeated instances and cross-id turret mounts (rou_e303_wep_01_root
×2 on the e106 hull)"` — with
`ship::tests::static_assembly_matches_runtime_capture` asserting
`count("e303_wep_01") == 2`. The absence from the capture was explained away as
vbase dedup, but **dedup would show one instance, not zero**.
The test cannot catch it either: it walks the capture's parts and looks each up
in the static output, so **extra** static placements are invisible to it. That is
the same shape of gap as the earlier `include_external` hypothesis — a test that
can only fail one way.
### Scope, stated carefully
Sweeping all 335 assembled ships for the signature *ship-scale span with under
400 triangles* flags **20 ships and 58 placements** over 28 distinct resources
(`e005_ant_*`, `f001_ant_*`, `f002_bdy_*`, `f301_barrel`, `f303_body`,
`e303_wep_01`, …). **Only the `e106`/`e303_wep_01` case is proven** — by render,
by capture absence, and by geometry. Some of the others may be legitimately large
low-poly parts, and each needs the same three checks before being called a bug.
**Still true, and independent of the correction above:**
`static_assembly_matches_runtime_capture` walks the capture's parts and looks each
up in the static output, so **extra static placements can never fail it**. That is
worth fixing regardless — it is the same one-way-test shape as the earlier
`include_external` hypothesis.
Also unchanged: only **two** cross-id placements exist fleet-wide (`e303_wep_01`
on `e101` ×24 and `e106` ×36, across 335 assembled ships), so cross-id mounting is
a narrow, real feature rather than a systemic guess.
---
## Resolution (2026-08-12)
`anchor_pool_mesh` took the **first** candidate in file order from a
container-global scan, so a resource could be handed another resource's block
whenever both shared `(stride, vertex count, index count)`. Fixed by anchoring
each resource near its **descriptor neighbours** (two-pass: learn, then re-anchor).
- it took inconsistency **125 → 51** with coverage unchanged, and made `e106`
render correctly ([after](captures/e106-static-assembly-fixed.png))
- **but it flipped the `e106` twin-mirror decision**, which
`static_assembly_matches_runtime_capture` (ISO-gated, so it skips in a plain
`cargo test`) catches against the runtime capture — so it was **reverted**
- the user-reported "capital ships assemble wrong" is therefore **diagnosed, not
yet fixed**; see [xbg7](structures/xbg7-mesh.md) for what the real fix needs
Still open from this entry: `static_assembly_matches_runtime_capture` walks only
the capture's parts, so **extra** static placements still cannot fail it.