fix(mesh): anchor XBG7 resources by neighbourhood -- inconsistency 125 -> 63, ships render right

anchor_pool_mesh took the FIRST candidate in file order from a container-global
vertex-run scan, so a resource could be handed another resource's block whenever
both shared (stride, vertex count, index count). Both blocks are real geometry and
both pass every quality gate, so only position separates them.

anchor_pool_mesh_near now 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, pass 2 re-anchors each resource preferring the median anchor
of its +/-2 descriptor neighbours. Too few anchored neighbours -> keep pass 1, so
nothing regresses to guesswork.

  before  decoded 5480/6294  shared 681  inconsistent 125
  after   decoded 5480/6294  shared 681  inconsistent  63

Coverage unchanged, inconsistency halved. e303_wep_01 decodes to 49x23x42 in ALL
containers now, and e106 renders as a destroyer instead of a slab -- its two
shared turrets symmetric at X[-203,-154] and X[154,203]. That resolves the
user-reported "capital ships assemble wrong" for this cause.

The filtered path needed care: models_named (what the viewer uses) dropped
non-wanted resources, which would have left filtered decodes with no
neighbourhood and silently kept 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.

63 cases remain; mesh_consistency_disc stays ignored and now records 63, not 125.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-12 00:31:20 +00:00
parent d660705c47
commit f18d5919f7
5 changed files with 177 additions and 24 deletions

View File

@@ -487,6 +487,7 @@ impl Xbg7Model {
decl: VertexDecl, decl: VertexDecl,
} }
let mut resources: Vec<Res> = Vec::new(); let mut resources: Vec<Res> = Vec::new();
let mut asked_for: Vec<bool> = Vec::new();
for _ in 0..header.num_resources { for _ in 0..header.num_resources {
let e = match Xpr2ResourceEntry::read(&mut cur) { let e = match Xpr2ResourceEntry::read(&mut cur) {
Ok(e) => e, Ok(e) => e,
@@ -516,16 +517,18 @@ impl Xbg7Model {
} }
let name = read_cstr(bytes, e.name_offset as usize + DIR_BASE) let name = read_cstr(bytes, e.name_offset as usize + DIR_BASE)
.unwrap_or_else(|| "XBG7".to_string()); .unwrap_or_else(|| "XBG7".to_string());
if let Some(w) = wanted { // Keep NON-wanted resources too: a resource is anchored by where its
if !w.contains(&name) { // descriptor NEIGHBOURS anchor, so filtering them out here would
continue; // 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 { resources.push(Res {
name, name,
markers, markers,
decl, decl,
}); });
asked_for.push(asked);
} }
if resources.is_empty() { if resources.is_empty() {
return out; return out;
@@ -556,20 +559,30 @@ impl Xbg7Model {
// preserves resource order, so the output is identical to the sequential // preserves resource order, so the output is identical to the sequential
// decode. `should_cancel()` is polled per resource so a superseded load // decode. `should_cancel()` is polled per resource so a superseded load
// stops promptly. // stops promptly.
let decode_one = |r: &Res| -> Option<Xbg7Model> { // `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<usize>| -> (Option<Xbg7Model>, Option<usize>) {
if should_cancel() { if should_cancel() {
return None; return (None, None);
} }
let starts = &starts_by_stride[&r.decl.stride]; let starts = &starts_by_stride[&r.decl.stride];
let mut anchored_at = None;
let meshes = if r.markers.len() == 1 { let meshes = if r.markers.len() == 1 {
// Single sub-mesh → the proven per-block adjacency anchor // Single sub-mesh → the proven per-block adjacency anchor
// (index buffer immediately before its vertex buffer). Stages and // (index buffer immediately before its vertex buffer). Stages and
// simple props take this path; `min_consistency` behaviour is // simple props take this path; `min_consistency` behaviour is
// exactly as before. // exactly as before.
let (vtx_count, index_count) = r.markers[0]; let (vtx_count, index_count) = r.markers[0];
anchor_pool_mesh(bytes, starts, index_count, vtx_count, &r.decl, min_consistency) anchor_pool_mesh_near(
.into_iter() bytes, starts, index_count, vtx_count, &r.decl, min_consistency, near,
.collect() )
.map(|(m, vb)| {
anchored_at = Some(vb);
m
})
.into_iter()
.collect()
} else { } else {
// Several sub-meshes sharing grouped index/vertex pools → the // Several sub-meshes sharing grouped index/vertex pools → the
// deterministic grouped-pool decode (hero ships et al.). // deterministic grouped-pool decode (hero ships et al.).
@@ -582,25 +595,88 @@ impl Xbg7Model {
// to the original single-block adjacency anchor on the first // to the original single-block adjacency anchor on the first
// marker so coverage is never *below* the pre-grouped decode. // marker so coverage is never *below* the pre-grouped decode.
let (vtx_count, index_count) = r.markers[0]; let (vtx_count, index_count) = r.markers[0];
anchor_pool_mesh(bytes, starts, index_count, vtx_count, &r.decl, min_consistency) anchor_pool_mesh_near(
.into_iter() bytes, starts, index_count, vtx_count, &r.decl, min_consistency, near,
.collect() )
.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(), name: r.name.clone(),
meshes, 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<usize>], i: usize) -> Option<usize> {
const SPAN: usize = 2;
let lo = i.saturating_sub(SPAN);
let hi = (i + SPAN + 1).min(vbs.len());
let mut near: Vec<usize> = (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<bool> = (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<Xbg7Model>, Option<usize>) {
if need_pass1[i] {
decode_one_near(r, None)
} else {
(None, None)
}
};
#[cfg(not(target_arch = "wasm32"))]
let pass1: Vec<(Option<Xbg7Model>, Option<usize>)> = {
use rayon::prelude::*;
resources.par_iter().enumerate().map(run1).collect()
};
#[cfg(target_arch = "wasm32")]
let pass1: Vec<(Option<Xbg7Model>, Option<usize>)> =
resources.iter().enumerate().map(run1).collect();
let vbs: Vec<Option<usize>> = 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<Xbg7Model> {
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"))] #[cfg(not(target_arch = "wasm32"))]
{ {
use rayon::prelude::*; 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")] #[cfg(target_arch = "wasm32")]
{ {
out = resources.iter().filter_map(decode_one).collect(); out = resources.iter().enumerate().filter_map(finish).collect();
} }
out out
} }
@@ -653,8 +729,35 @@ fn anchor_pool_mesh(
decl: &VertexDecl, decl: &VertexDecl,
min_consistency: f32, min_consistency: f32,
) -> Option<GameMesh> { ) -> Option<GameMesh> {
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<usize>,
) -> Option<(GameMesh, usize)> {
let idx_bytes = index_count * 2; let idx_bytes = index_count * 2;
for &vb in starts { let mut order: Vec<usize> = 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 // 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 // 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 // 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) { if validate_block(bytes, ib, vb, vtx_count, index_count, decl, mc, true) {
// ── Accepted: read the full mesh. ── // ── 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,
));
} }
} }
} }

View File

@@ -53,7 +53,7 @@ fn span(m: &Xbg7Model) -> Option<[i64; 3]> {
} }
#[test] #[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() { fn shared_resources_decode_identically_in_every_container() {
let Some(root) = disc_root() else { let Some(root) = disc_root() else {
eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)"); eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)");

View File

@@ -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 ## ⚠️ 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 **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` 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 on `e101` ×24 and `e106` ×36, across 335 assembled ships), so cross-id mounting is
a narrow, real feature rather than a systemic guess. 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.

Binary file not shown.

After

Width:  |  Height:  |  Size: 26 KiB

View File

@@ -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, first-match when there is no neighbour yet. That needs no new format knowledge,
and it selects `52 257 440` here. and it selects `52 257 440` here.
**Not implemented.** It changes the anchor for every one of the 6 294 resources, ### ✅ Implemented (2026-08-12) — inconsistency halved, coverage unchanged
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 `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) [`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 ### Where the mis-decode is *not*: the grouped-pool anchor