fabi/Syplheed-Reborn
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

revert(mesh): withdraw the neighbourhood anchor -- it regressed the e106 twin mirror

Browse Source 
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>
This commit is contained in:
Claude (auto-RE)
2026-08-12 01:08:41 +00:00
parent 0f9c95c52e
commit 64d372c7e8
6 changed files with 85 additions and 159 deletions
Download Patch File Download Diff File Expand all files Collapse all files

160
crates/sylpheed-formats/src/mesh.rs
View File

@@ -487,7 +487,6 @@ impl Xbg7Model {
decl: VertexDecl,
}
let mut resources: Vec<Res> = Vec::new();
let mut asked_for: Vec<bool> = Vec::new();
for _ in 0..header.num_resources {
let e = match Xpr2ResourceEntry::read(&mut cur) {
Ok(e) => e,
@@ -517,18 +516,16 @@ impl Xbg7Model {
}
let name = read_cstr(bytes, e.name_offset as usize + DIR_BASE)
.unwrap_or_else(|| "XBG7".to_string());
// 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));
if let Some(w) = wanted {
if !w.contains(&name) {
continue;
}
}
resources.push(Res {
name,
markers,
decl,
});
asked_for.push(asked);
}
if resources.is_empty() {
return out;
@@ -559,30 +556,20 @@ 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.
// `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>) {
let decode_one = |r: &Res| -> Option<Xbg7Model> {
if should_cancel() {
return (None, None);
return 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_near(
bytes, starts, index_count, vtx_count, &r.decl, min_consistency, near,
)
.map(|(m, vb)| {
anchored_at = Some(vb);
m
})
.into_iter()
.collect()
anchor_pool_mesh(bytes, starts, index_count, vtx_count, &r.decl, min_consistency)
.into_iter()
.collect()
} else {
// Several sub-meshes sharing grouped index/vertex pools → the
// deterministic grouped-pool decode (hero ships et al.).
@@ -595,106 +582,25 @@ 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_near(
bytes, starts, index_count, vtx_count, &r.decl, min_consistency, near,
)
.map(|(m, vb)| {
anchored_at = Some(vb);
m
})
.into_iter()
.collect()
anchor_pool_mesh(bytes, starts, index_count, vtx_count, &r.decl, min_consistency)
.into_iter()
.collect()
}
};
let model = (!meshes.is_empty()).then(|| Xbg7Model {
(!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<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 mut vbs: Vec<Option<usize>> = pass1.iter().map(|(_, vb)| *vb).collect();
// Refine the anchor map before using it: pass 1's anchors include the
// very mistakes this is meant to correct, so a resource next to a
// mis-anchored neighbour inherits a bad reference. Re-anchoring against
// the improving map and repeating converges quickly; two rounds is
// enough on this disc (a third changes nothing).
for _ in 0..2 {
let refined: Vec<Option<usize>> = (0..resources.len())
.map(|i| match (need_pass1[i], neighbourhood(&vbs, i)) {
(true, Some(anchor)) => decode_one_near(&resources[i], Some(anchor)).1.or(vbs[i]),
_ => vbs[i],
})
.collect();
if refined == vbs {
break;
}
vbs = refined;
}
// 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"))]
{
use rayon::prelude::*;
out = resources.par_iter().enumerate().filter_map(finish).collect();
out = resources.par_iter().filter_map(decode_one).collect();
}
#[cfg(target_arch = "wasm32")]
{
out = resources.iter().enumerate().filter_map(finish).collect();
out = resources.iter().filter_map(decode_one).collect();
}
out
}
@@ -747,35 +653,8 @@ fn anchor_pool_mesh(
decl: &VertexDecl,
min_consistency: f32,
) -> 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 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 {
for &vb in starts {
// 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
@@ -795,10 +674,7 @@ fn anchor_pool_mesh_near(
};
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),
vb,
));
return Some(read_pool_mesh(bytes, ib, vb, index_count, vtx_count, decl));
}
}
}

21
crates/sylpheed-formats/src/ship.rs
View File

@@ -615,6 +615,27 @@ mod tests {
}
}
}
// ── Extras: the direction this test could not previously fail in. ──
// The loop above walks the CAPTURE's parts and looks each up in ours, so
// a static placement with no counterpart was invisible to it — which is
// how a resource decoded 100x too large (`e303_wep_01`, 2026-08-12) sat
// here unnoticed. Pin the set instead: the capture legitimately misses
// repeated instances of a shared resource (vbase dedup), so `e303_wep_01`
// is expected; anything else appearing only in the static assembly is a
// regression.
let captured: std::collections::BTreeSet<&str> =
cap.parts.iter().map(|p| p.part.as_str()).collect();
let extra: std::collections::BTreeSet<&str> = placed
.iter()
.map(|p| p.resource.as_str())
.filter(|r| !captured.contains(r))
.collect();
let allowed: std::collections::BTreeSet<&str> = ["e303_wep_01"].into_iter().collect();
assert_eq!(
extra, allowed,
"static placements with no counterpart in the runtime capture"
);
// Multi-instance coverage the capture couldn't see (vbase dedup).
let count = |res: &str| placed.iter().filter(|p| p.resource == res).count();
assert_eq!(count("e106_eng_01"), 2, "both engine nacelles placed");

2
crates/sylpheed-formats/tests/mesh_consistency_disc.rs
View File

@@ -53,7 +53,7 @@ fn span(m: &Xbg7Model) -> Option<[i64; 3]> {
}
#[test]
#[ignore = "known-failing: 51 of 681 shared resources still decode inconsistently (125 before the neighbourhood anchor, 63 before refining it — 2026-08-12)"]
#[ignore = "known-failing: 125 of 681 shared resources decode inconsistently. A neighbourhood anchor took this to 51 but regressed the e106 twin-mirror decision and was withdrawn — see docs/re/structures/xbg7-mesh.md"]
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)");