diff --git a/crates/sylpheed-formats/src/mesh.rs b/crates/sylpheed-formats/src/mesh.rs index 1eea288..5546d1e 100644 --- a/crates/sylpheed-formats/src/mesh.rs +++ b/crates/sylpheed-formats/src/mesh.rs @@ -487,6 +487,7 @@ impl Xbg7Model { decl: VertexDecl, } let mut resources: Vec = Vec::new(); + let mut asked_for: Vec = Vec::new(); for _ in 0..header.num_resources { let e = match Xpr2ResourceEntry::read(&mut cur) { Ok(e) => e, @@ -516,16 +517,18 @@ impl Xbg7Model { } let name = read_cstr(bytes, e.name_offset as usize + DIR_BASE) .unwrap_or_else(|| "XBG7".to_string()); - if let Some(w) = wanted { - if !w.contains(&name) { - continue; - } - } + // Keep NON-wanted resources too: a resource is anchored by where its + // descriptor NEIGHBOURS anchor, so filtering them out here would + // leave a filtered decode with no neighbourhood (and the pre-2026-08 + // first-in-file-order behaviour). Only a ±2 window is actually + // decoded — see `need_pass1` below. + let asked = wanted.map_or(true, |w| w.contains(&name)); resources.push(Res { name, markers, decl, }); + asked_for.push(asked); } if resources.is_empty() { return out; @@ -556,20 +559,30 @@ 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 decode_one = |r: &Res| -> Option { + // `near`: prefer candidate anchors close to this offset (see + // `anchor_pool_mesh_near`). Pass 1 runs with `None` to learn where each + // resource lands; pass 2 re-runs with each resource's neighbourhood. + let decode_one_near = |r: &Res, near: Option| -> (Option, Option) { if should_cancel() { - return None; + return (None, None); } let starts = &starts_by_stride[&r.decl.stride]; + let mut anchored_at = None; let meshes = if r.markers.len() == 1 { // Single sub-mesh → the proven per-block adjacency anchor // (index buffer immediately before its vertex buffer). Stages and // 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_near( + bytes, starts, index_count, vtx_count, &r.decl, min_consistency, near, + ) + .map(|(m, vb)| { + anchored_at = Some(vb); + m + }) + .into_iter() + .collect() } else { // Several sub-meshes sharing grouped index/vertex pools → the // deterministic grouped-pool decode (hero ships et al.). @@ -582,25 +595,88 @@ 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_near( + bytes, starts, index_count, vtx_count, &r.decl, min_consistency, near, + ) + .map(|(m, vb)| { + anchored_at = Some(vb); + m + }) + .into_iter() + .collect() } }; - (!meshes.is_empty()).then(|| Xbg7Model { + let model = (!meshes.is_empty()).then(|| Xbg7Model { name: r.name.clone(), meshes, - }) + }); + (model, anchored_at) }; + /// Median of a resource's neighbours' anchors — the reference a resource + /// should sit near. `None` when too few neighbours anchored to be useful. + fn neighbourhood(vbs: &[Option], i: usize) -> Option { + const SPAN: usize = 2; + let lo = i.saturating_sub(SPAN); + let hi = (i + SPAN + 1).min(vbs.len()); + let mut near: Vec = (lo..hi).filter(|&k| k != i).filter_map(|k| vbs[k]).collect(); + if near.len() < 2 { + return None; + } + near.sort_unstable(); + Some(near[near.len() / 2]) + } + + // Pass 1 — first-match, to learn each resource's neighbourhood. Only the + // asked-for resources and their ±2 neighbours need it, so a filtered + // decode stays proportional to what was asked for. + let need_pass1: Vec = (0..resources.len()) + .map(|i| { + let lo = i.saturating_sub(2); + let hi = (i + 3).min(asked_for.len()); + asked_for[lo..hi].iter().any(|&a| a) + }) + .collect(); + let run1 = |(i, r): (usize, &Res)| -> (Option, Option) { + if need_pass1[i] { + decode_one_near(r, None) + } else { + (None, None) + } + }; + #[cfg(not(target_arch = "wasm32"))] + let pass1: Vec<(Option, Option)> = { + use rayon::prelude::*; + resources.par_iter().enumerate().map(run1).collect() + }; + #[cfg(target_arch = "wasm32")] + let pass1: Vec<(Option, Option)> = + resources.iter().enumerate().map(run1).collect(); + + let vbs: Vec> = pass1.iter().map(|(_, vb)| *vb).collect(); + + // Pass 2 — re-anchor preferring the resource's own neighbourhood, which + // is what separates its data from another resource's identically-shaped + // block. Resources without a usable neighbourhood keep pass 1's result. + let finish = |(i, r): (usize, &Res)| -> Option { + if !asked_for[i] { + return None; + } + match neighbourhood(&vbs, i) { + Some(anchor) => decode_one_near(r, Some(anchor)) + .0 + .or_else(|| pass1[i].0.clone()), + None => pass1[i].0.clone(), + } + }; #[cfg(not(target_arch = "wasm32"))] { use rayon::prelude::*; - out = resources.par_iter().filter_map(decode_one).collect(); + out = resources.par_iter().enumerate().filter_map(finish).collect(); } #[cfg(target_arch = "wasm32")] { - out = resources.iter().filter_map(decode_one).collect(); + out = resources.iter().enumerate().filter_map(finish).collect(); } out } @@ -653,8 +729,35 @@ fn anchor_pool_mesh( decl: &VertexDecl, min_consistency: f32, ) -> Option { + anchor_pool_mesh_near(bytes, starts, index_count, vtx_count, decl, min_consistency, None) + .map(|(m, _)| m) +} + +/// As [`anchor_pool_mesh`], but when `near` is given the candidates are tried in +/// order of distance from it, and the accepted vertex-buffer offset is returned +/// alongside the mesh. +/// +/// Why: the candidate list is one scan of the **whole container** per stride and +/// is shared by every resource of that stride, so first-in-file-order can hand a +/// resource a block belonging to something else that happens to share its vertex +/// and index counts. Both blocks are real geometry and both pass every quality +/// gate, so only *position* separates them — a resource's own data sits near its +/// descriptor neighbours' (`docs/re/structures/xbg7-mesh.md`). +fn anchor_pool_mesh_near( + bytes: &[u8], + starts: &[usize], + index_count: usize, + vtx_count: usize, + decl: &VertexDecl, + min_consistency: f32, + near: Option, +) -> Option<(GameMesh, usize)> { let idx_bytes = index_count * 2; - for &vb in starts { + let mut order: Vec = starts.to_vec(); + if let Some(anchor) = near { + order.sort_by_key(|&vb| vb.abs_diff(anchor)); + } + for &vb in &order { // 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 @@ -674,7 +777,10 @@ fn anchor_pool_mesh( }; if validate_block(bytes, ib, vb, vtx_count, index_count, decl, mc, true) { // ── Accepted: read the full mesh. ── - return Some(read_pool_mesh(bytes, ib, vb, index_count, vtx_count, decl)); + return Some(( + read_pool_mesh(bytes, ib, vb, index_count, vtx_count, decl), + vb, + )); } } } diff --git a/crates/sylpheed-formats/tests/mesh_consistency_disc.rs b/crates/sylpheed-formats/tests/mesh_consistency_disc.rs index a2a0738..69ff646 100644 --- a/crates/sylpheed-formats/tests/mesh_consistency_disc.rs +++ b/crates/sylpheed-formats/tests/mesh_consistency_disc.rs @@ -53,7 +53,7 @@ fn span(m: &Xbg7Model) -> Option<[i64; 3]> { } #[test] -#[ignore = "known-failing: 125 of 681 shared resources decode inconsistently (2026-08-11)"] +#[ignore = "known-failing: 63 of 681 shared resources still decode inconsistently (was 125 before the neighbourhood anchor, 2026-08-12)"] fn shared_resources_decode_identically_in_every_container() { let Some(root) = disc_root() else { eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)"); diff --git a/docs/re/BACKLOG.md b/docs/re/BACKLOG.md index 6f9a34e..07ea891 100644 --- a/docs/re/BACKLOG.md +++ b/docs/re/BACKLOG.md @@ -153,6 +153,11 @@ report needs re-grounding against a specific ship and a specific expectation. --- +## ✅ FIXED 2026-08-12 — it was a mis-decode, and the anchor now uses locality + +> 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 @@ -255,3 +260,19 @@ worth fixing regardless — it is the same one-way-test shape as the earlier 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). + +- decoded **5 480 / 6 294 unchanged**, inconsistent **125 → 63** +- `e106` renders correctly ([after](captures/e106-static-assembly-fixed.png)) +- the user-reported "capital ships assemble wrong" is **resolved** for this cause + +Still open from this entry: `static_assembly_matches_runtime_capture` walks only +the capture's parts, so **extra** static placements still cannot fail it. diff --git a/docs/re/captures/e106-static-assembly-fixed.png b/docs/re/captures/e106-static-assembly-fixed.png new file mode 100644 index 0000000..6b24875 Binary files /dev/null and b/docs/re/captures/e106-static-assembly-fixed.png differ diff --git a/docs/re/structures/xbg7-mesh.md b/docs/re/structures/xbg7-mesh.md index de74547..732f397 100644 --- a/docs/re/structures/xbg7-mesh.md +++ b/docs/re/structures/xbg7-mesh.md @@ -393,11 +393,37 @@ and prefer candidates close to the previous resource's anchor), falling back to first-match when there is no neighbour yet. That needs no new format knowledge, and it selects `52 257 440` here. -**Not implemented.** It changes the anchor for every one of the 6 294 resources, -so it needs the before/after measurement — decoded count must not fall from -5 480, and the inconsistency count should fall from 125 — plus the ignored test +### ✅ Implemented (2026-08-12) — inconsistency halved, coverage unchanged + +`anchor_pool_mesh_near` tries candidates in order of distance from a reference, +and `anchor_models_filtered` runs **two passes**: pass 1 anchors first-match to +learn where resources land, then pass 2 re-anchors each resource preferring its +**neighbourhood** — the median anchor of its ±2 descriptor neighbours. A resource +with too few anchored neighbours keeps pass 1's result, so nothing regresses to +guesswork. + +| | decoded | shared | inconsistent | +|---|---|---|---| +| before | 5 480 / 6 294 | 681 | **125** | +| after | 5 480 / 6 294 | 681 | **63** | + +**Coverage is unchanged and inconsistency halves.** `e303_wep_01` now decodes to +49 × 23 × 42 in *all* containers, and `e106` renders as a destroyer instead of a +slab ([before](../captures/e106-static-assembly-volume-bug.png) · +[after](../captures/e106-static-assembly-fixed.png)) — its two shared turrets sit +symmetrically at X[−203,−154] and X[154,203]. + +**The filtered path needed care.** `models_named` (what the viewer's ship +rendering uses) drops non-wanted resources, which would leave a filtered decode +with no neighbourhood at all — and silently keep the old behaviour. Resources are +now collected regardless of the filter, but only the asked-for ones and their ±2 +neighbours are decoded in pass 1, so a filtered decode stays proportional to what +was asked for. + +**63 remain.** The ignored test [`mesh_consistency_disc.rs`](../../crates/sylpheed-formats/tests/mesh_consistency_disc.rs) -un-ignored once it passes. +still asserts the target state and now records 63 rather than 125; the remaining +cases are where the neighbourhood is itself wrong or absent. ### Where the mis-decode is *not*: the grouped-pool anchor