From d83bdd3b6861fe37db937de084e557015e22bb85 Mon Sep 17 00:00:00 2001 From: "Claude (auto-RE)" Date: Wed, 12 Aug 2026 03:16:51 +0000 Subject: [PATCH] fix(mesh): distinct anchor assignment -- no two resources may claim one buffer Selection was per-resource and greedy, so two resources could take one vertex buffer while a valid one sat unused. A runtime capture proved that wrong for the mirrored e106 hull twins: the container holds both halves and the engine draws each from its own address. Now the first claimant keeps a buffer and later resources re-anchor past everything already claimed (coverage can never regress; grouped-pool models untouched). Against the 46 capture-named Stage_S02 buffers: exact anchors 29 -> 40, unclaimed 12 -> 4. Disc-wide: 5480 resources decoded (unchanged), cross-container inconsistency 125 -> 46. The twins' mirror therefore lives in the DATA, not in the placement matrix: the embedded e106_bdy_02 row and the two assertions encoding the old convention are updated, each with the reason recorded. Full suite green incl. disc/ISO gates. Co-Authored-By: Claude Opus 5 (1M context) --- .../sylpheed-formats/data/ship_placements.txt | 7 +- crates/sylpheed-formats/src/mesh.rs | 100 ++++++++++++++++-- crates/sylpheed-formats/src/ship.rs | 9 +- crates/sylpheed-formats/src/ship_capture.rs | 11 +- docs/re/structures/xbg7-mesh.md | 32 ++++++ 5 files changed, 143 insertions(+), 16 deletions(-) diff --git a/crates/sylpheed-formats/data/ship_placements.txt b/crates/sylpheed-formats/data/ship_placements.txt index c1081b6..da31bf4 100644 --- a/crates/sylpheed-formats/data/ship_placements.txt +++ b/crates/sylpheed-formats/data/ship_placements.txt @@ -12,7 +12,12 @@ ship e106 ref=e106_bdy_04 e106_bdy_01 1 0 0 0 1 0 0 0 1 -263.9948 -150.43242 1164.8667 - e106_bdy_02 -1 0 0 0 1 0 0 0 1 264.0088 -150.41339 1164.8651 +# 2026-08-12: this rotation was diag(-1,1,1) - the X-flip `correlate` bakes in +# when a draw's buffer is the mirror of the decoded one. With distinct anchor +# assignment the twins now decode to their OWN buffers (the container carries +# both halves), so the mirror lives in the data and the placement is plain +# identity. Re-emitting from the capture agrees: `1 0 0 0 1 0 0 0 1 264.04343`. + e106_bdy_02 1 0 0 0 1 0 0 0 1 264.0088 -150.41339 1164.8651 e106_bdy_03 1 0 0 0 1 0 0 0 1 0.0029247368 -34.517372 1075.878 e106_bdy_04 1 0 0 0 1 0 0 0 1 0 0 0 e106_brg_01 1 0 0 0 1 0 0 0 1 -0.005471501 153.31795 -164.03748 diff --git a/crates/sylpheed-formats/src/mesh.rs b/crates/sylpheed-formats/src/mesh.rs index 404220b..8a13953 100644 --- a/crates/sylpheed-formats/src/mesh.rs +++ b/crates/sylpheed-formats/src/mesh.rs @@ -563,6 +563,7 @@ impl Xbg7Model { // preserves resource order, so the output is identical to the sequential // decode. `should_cancel()` is polled per resource so a superseded load // stops promptly. + let empty_taken: std::collections::HashSet = std::collections::HashSet::new(); let decode_one = |r: &Res| -> Option { if should_cancel() { return None; @@ -574,9 +575,17 @@ impl Xbg7Model { // simple props take this path; `min_consistency` behaviour is // exactly as before. let (vtx_count, index_count) = r.markers[0]; - anchor_pool_mesh(bytes, starts, index_count, vtx_count, &r.decl, min_consistency) - .into_iter() - .collect() + anchor_pool_mesh( + bytes, + starts, + index_count, + vtx_count, + &r.decl, + min_consistency, + &empty_taken, + ) + .into_iter() + .collect() } else { // Several sub-meshes sharing grouped index/vertex pools → the // deterministic grouped-pool decode (hero ships et al.). @@ -589,9 +598,17 @@ impl Xbg7Model { // to the original single-block adjacency anchor on the first // marker so coverage is never *below* the pre-grouped decode. let (vtx_count, index_count) = r.markers[0]; - anchor_pool_mesh(bytes, starts, index_count, vtx_count, &r.decl, min_consistency) - .into_iter() - .collect() + anchor_pool_mesh( + bytes, + starts, + index_count, + vtx_count, + &r.decl, + min_consistency, + &empty_taken, + ) + .into_iter() + .collect() } }; (!meshes.is_empty()).then(|| Xbg7Model { @@ -601,14 +618,67 @@ impl Xbg7Model { }; #[cfg(not(target_arch = "wasm32"))] - { + let decoded: Vec<(usize, Xbg7Model)> = { use rayon::prelude::*; - out = resources.par_iter().filter_map(decode_one).collect(); - } + resources + .par_iter() + .enumerate() + .filter_map(|(i, r)| decode_one(r).map(|m| (i, m))) + .collect() + }; #[cfg(target_arch = "wasm32")] - { - out = resources.iter().filter_map(decode_one).collect(); + let decoded: Vec<(usize, Xbg7Model)> = resources + .iter() + .enumerate() + .filter_map(|(i, r)| decode_one(r).map(|m| (i, m))) + .collect(); + + // ── Distinct assignment ────────────────────────────────────────── + // + // Selection above is per-resource and greedy: each takes the first + // candidate that validates, so two resources can claim ONE buffer while + // a valid buffer sits unused. A runtime capture proves that is wrong for + // the mirrored `e106_bdy_0{1,2}_l` twins — the container holds both + // halves (`0x3b3ee8` and its X-mirror `0x3c55d8`) and the engine draws + // each from its own — and both offsets validate for both names, so the + // correct block merely lost the first-match race. + // + // So: walk the decodes in container order, let the first claimant keep a + // buffer, and re-anchor any later resource that wanted the same one, + // skipping everything already claimed. Only single-sub-mesh resources + // (the adjacency-anchor path) take part; grouped-pool models are left + // exactly as they were. + let mut taken: std::collections::HashSet = std::collections::HashSet::new(); + let mut models: Vec = Vec::with_capacity(decoded.len()); + for (i, mut m) in decoded { + let r = &resources[i]; + if m.meshes.len() == 1 && r.markers.len() == 1 { + if let Some(vb) = m.meshes[0].vbuf_offset { + if taken.contains(&vb) { + let starts = &starts_by_stride[&r.decl.stride]; + let (vtx_count, index_count) = r.markers[0]; + if let Some(alt) = anchor_pool_mesh( + bytes, + starts, + index_count, + vtx_count, + &r.decl, + min_consistency, + &taken, + ) { + m.meshes[0] = alt; + } + // No free candidate → keep the collided decode rather + // than drop the resource; coverage never regresses. + } + if let Some(vb2) = m.meshes[0].vbuf_offset { + taken.insert(vb2); + } + } + } + models.push(m); } + out = models; out } } @@ -855,9 +925,17 @@ fn anchor_pool_mesh( vtx_count: usize, decl: &VertexDecl, min_consistency: f32, + taken: &std::collections::HashSet, ) -> Option { let idx_bytes = index_count * 2; for &vb in starts { + // A buffer another resource already claimed is not a candidate: the + // engine draws each part from its own buffer (proved for the mirrored + // `e106_bdy_0{1,2}_l` twins by a runtime capture), so two resources + // landing on one offset means at least one of them is wrong. + if taken.contains(&vb) { + continue; + } // The index buffer sits just before the vertex buffer, which is 4-byte // aligned — so 0..=3 bytes of padding may separate them (`ib = vb − // idx_bytes − pad`). pad 0 is the immediate-adjacency case (all stages so diff --git a/crates/sylpheed-formats/src/ship.rs b/crates/sylpheed-formats/src/ship.rs index 50138a0..8abb5dd 100644 --- a/crates/sylpheed-formats/src/ship.rs +++ b/crates/sylpheed-formats/src/ship.rs @@ -640,14 +640,19 @@ mod tests { let count = |res: &str| placed.iter().filter(|p| p.resource == res).count(); assert_eq!(count("e106_eng_01"), 2, "both engine nacelles placed"); assert_eq!(count("e303_wep_01"), 2, "both shared turrets placed"); - // The mirrored starboard hull reflects (det < 0), the port one doesn't. + // NEITHER hull reflects: the twins' geometry is mirrored on the disc, + // so both placements are proper rotations. This flipped on 2026-08-12 — + // while both twins decoded to one buffer, `apply_twin_mirrors` had to + // synthesise the reflection here; a runtime capture showed the container + // holds both halves, and distinct anchor assignment now hands each twin + // its own (see docs/re/structures/xbg7-mesh.md). let det = |m: &[[f32; 3]; 3]| { m[0][0] * (m[1][1] * m[2][2] - m[1][2] * m[2][1]) - m[0][1] * (m[1][0] * m[2][2] - m[1][2] * m[2][0]) + m[0][2] * (m[1][0] * m[2][1] - m[1][1] * m[2][0]) }; let one = |res: &str| placed.iter().find(|p| p.resource == res).unwrap(); - assert!(det(&one("e106_bdy_02").m) < 0.0, "starboard hull mirrored"); + assert!(det(&one("e106_bdy_02").m) > 0.0, "starboard hull plain (mirror is in the data)"); assert!(det(&one("e106_bdy_01").m) > 0.0, "port hull plain"); } diff --git a/crates/sylpheed-formats/src/ship_capture.rs b/crates/sylpheed-formats/src/ship_capture.rs index 77c93e9..070b687 100644 --- a/crates/sylpheed-formats/src/ship_capture.rs +++ b/crates/sylpheed-formats/src/ship_capture.rs @@ -717,10 +717,17 @@ mod tests { assert_eq!(e106.reference, "e106_bdy_04"); assert_eq!(e106.parts.len(), 8, "all 8 e106 parts placed"); let get = |p: &str| e106.parts.iter().find(|x| x.part == p).unwrap(); - // Port/starboard hull pair: X = ∓264, the starboard copy mirrored. + // Port/starboard hull pair: X = ∓264, **both plain**. The mirror is + // baked into the disc data, not into the placement: a runtime capture + // shows the container carrying two 119-vertex buffers whose contents are + // exact X-reflections, each drawn from its own address (see + // docs/re/structures/xbg7-mesh.md). Until 2026-08-12 both twins decoded + // to ONE buffer and this row carried diag(-1,1,1) to compensate; with + // distinct anchor assignment they decode to their own, and re-emitting + // from the capture produces identity here. assert!((get("e106_bdy_01").t[0] + 264.0).abs() < 0.1); assert!((get("e106_bdy_02").t[0] - 264.0).abs() < 0.1); - assert_eq!(get("e106_bdy_02").m[0][0], -1.0); + assert_eq!(get("e106_bdy_02").m[0][0], 1.0); assert_eq!(get("e106_bdy_01").m[0][0], 1.0); // Bridge: centreline, above and aft of the hull reference. let brg = get("e106_brg_01"); diff --git a/docs/re/structures/xbg7-mesh.md b/docs/re/structures/xbg7-mesh.md index 3ac2f5d..9775f1c 100644 --- a/docs/re/structures/xbg7-mesh.md +++ b/docs/re/structures/xbg7-mesh.md @@ -842,6 +842,38 @@ fixes the twins but cannot reach blocks the validator still rejects. The pair of changes is the fix; either alone is a half-measure, which is why neither has been made yet. +### ✅ Fixed: distinct anchor assignment (2026-08-12) + +The selection is no longer per-resource-greedy. After the parallel decode, the +models are walked in container order: the first claimant keeps a buffer, and any +later resource that chose the same one is **re-anchored past every buffer already +claimed**. A resource that finds no free candidate keeps its collided decode, so +coverage can never regress. Grouped-pool models are untouched. + +Measured against the 46 capture-named `Stage_S02` buffers, and disc-wide: + +| | exact anchors | unclaimed | resources decoded | shared inconsistent | +|---|---|---|---|---| +| greedy, cap 0.28 (before) | 29 | 12 | 5 480 | 125 | +| **distinct, cap 0.28 (now)** | **40** | **4** | 5 480 | **46** | +| distinct, cap 0.42 | 45 | 0 | 6 069 | 62 | + +Nothing decodes that did not decode before, cross-container inconsistency drops +by **63 %**, and 11 more of the ship's drawn buffers get exactly the right +resource. The cap stays at its shipped 0.28 — that is a separate change with its +own evidence (section 9), and this one is worth being able to revert alone. + +**One convention changed, and it is the point of the fix.** With the twins +sharing a buffer, the reflection had to be synthesised downstream: +`ship::apply_twin_mirrors` flipped one hull, and `correlate` baked an X-flip into +`e106_bdy_02`'s captured matrix (`diag(-1,1,1)`). Now each twin decodes to its +own, already-mirrored buffer, so **the mirror lives in the data** and both +placements are proper rotations. Re-emitting the block from the capture confirms +it independently (`1 0 0 0 1 0 0 0 1 264.04343 …`). The embedded placement row +and the two assertions that encoded the old convention were updated, each with +the reason in place; the full suite including the disc- and ISO-gated ship tests +is green. + Not settled: `e106_brg_01_b_02` ≡ `e106_brg_01_l` (51 verts). A second 51-vertex `vbase` exists in the logs but is **not** from this container, and the container holds three near-identical 51-vertex runs, so the pair has no oracle yet.