re: n206 was fixed after all -- the audit was classifying by geometry, not buffer
debug_grouped_report (why_rejected) shows n206_02's alternative pool is ACCEPTED at pad 0 under production gates and is in the candidate list -- and the decoder does take it: n206_02 now anchors at 0x342d984. The 'still collapsed' reading came from the audit comparing decoded geometry, and that offset holds a direct (unmirrored) copy, so a separated pair still looked identical. The audit now requires a SHARED BUFFER to call it a collapse: disc-wide 18 exact mirrors, 16 related, 0 collapses, 0 unrelated -- and the regression test drops its exception. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
13
crates/sylpheed-formats/examples/starts.rs
Normal file
13
crates/sylpheed-formats/examples/starts.rs
Normal file
@@ -0,0 +1,13 @@
|
|||||||
|
use sylpheed_formats::mesh::{debug_resource_params, debug_vertex_run_starts};
|
||||||
|
fn main() {
|
||||||
|
let a: Vec<String> = std::env::args().collect();
|
||||||
|
let bytes = std::fs::read(&a[1]).unwrap();
|
||||||
|
let (markers, stride) = debug_resource_params(&bytes, &a[2]).expect("resource");
|
||||||
|
println!("{}: stride={stride} markers={markers:?}", a[2]);
|
||||||
|
let starts = debug_vertex_run_starts(&bytes, stride);
|
||||||
|
println!("{} candidate starts at stride {stride}", starts.len());
|
||||||
|
for off in &a[3..] {
|
||||||
|
let o = usize::from_str_radix(off.trim_start_matches("0x"), 16).unwrap();
|
||||||
|
println!(" 0x{o:x} in candidate list: {}", starts.binary_search(&o).is_ok());
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -74,6 +74,15 @@ fn main() {
|
|||||||
related += 1;
|
related += 1;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
// Identical GEOMETRY is only a collapse if it also comes from the
|
||||||
|
// same buffer: a container may store a part twice unmirrored, and
|
||||||
|
// then two twins on two copies decode identically and correctly.
|
||||||
|
let shared_buffer = m.meshes[0].vbuf_offset.is_some()
|
||||||
|
&& m.meshes[0].vbuf_offset == t.meshes[0].vbuf_offset;
|
||||||
|
if ident && !shared_buffer {
|
||||||
|
related += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
if ident {
|
if ident {
|
||||||
same += 1;
|
same += 1;
|
||||||
if examples.len() < 6 {
|
if examples.len() < 6 {
|
||||||
@@ -101,7 +110,7 @@ fn main() {
|
|||||||
}
|
}
|
||||||
println!("twin pairs of equal vertex count: {}", same + mirrored + unrelated + related);
|
println!("twin pairs of equal vertex count: {}", same + mirrored + unrelated + related);
|
||||||
println!(" exact X-mirror (expected) : {mirrored}");
|
println!(" exact X-mirror (expected) : {mirrored}");
|
||||||
println!(" IDENTICAL (collapse) : {same}");
|
println!(" IDENTICAL, one buffer (collapse) : {same}");
|
||||||
println!(" related other way (Y/Z mirror, reordered): {related}");
|
println!(" related other way (Y/Z mirror, reordered): {related}");
|
||||||
println!(" unrelated (mis-anchor?) : {unrelated}");
|
println!(" unrelated (mis-anchor?) : {unrelated}");
|
||||||
for e in examples {
|
for e in examples {
|
||||||
|
|||||||
14
crates/sylpheed-formats/examples/why_rejected.rs
Normal file
14
crates/sylpheed-formats/examples/why_rejected.rs
Normal file
@@ -0,0 +1,14 @@
|
|||||||
|
//! Why does the decoder refuse a grouped-pool resource at a given pool start?
|
||||||
|
//! Usage: why_rejected <container.xpr> <resource>@<vb0-hex>...
|
||||||
|
use sylpheed_formats::mesh::debug_grouped_report;
|
||||||
|
fn main() {
|
||||||
|
let a: Vec<String> = std::env::args().collect();
|
||||||
|
let bytes = std::fs::read(&a[1]).expect("container");
|
||||||
|
for pair in &a[2..] {
|
||||||
|
let (name, off) = pair.split_once('@').expect("name@hex");
|
||||||
|
let vb0 = usize::from_str_radix(off.trim_start_matches("0x"), 16).expect("hex");
|
||||||
|
for line in debug_grouped_report(&bytes, name, vb0) {
|
||||||
|
println!("{line}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -755,6 +755,60 @@ pub fn debug_try_anchor(
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Diagnostic: why does the decoder refuse a grouped-pool resource at a given
|
||||||
|
/// pool start? Recomputes the pool layout exactly as [`anchor_grouped_meshes`]
|
||||||
|
/// does and reports the pivot sub-mesh's verdict for each index/vertex pad —
|
||||||
|
/// so a capture-proven pool that the decoder rejects names the gate to fix.
|
||||||
|
pub fn debug_grouped_report(bytes: &[u8], name: &str, vb0: usize) -> Vec<String> {
|
||||||
|
let Some((decl, markers)) = decl_of(bytes, name) else {
|
||||||
|
return vec!["no such XBG7 resource".into()];
|
||||||
|
};
|
||||||
|
let n = markers.len();
|
||||||
|
if n == 0 {
|
||||||
|
return vec!["no index markers".into()];
|
||||||
|
}
|
||||||
|
let (mut rel_ib, mut acc_i) = (Vec::with_capacity(n), 0usize);
|
||||||
|
for &(_, ic) in &markers {
|
||||||
|
rel_ib.push(acc_i);
|
||||||
|
acc_i = align4(acc_i + ic * 2);
|
||||||
|
}
|
||||||
|
let span = rel_ib[n - 1] + markers[n - 1].1 * 2;
|
||||||
|
let kmax = (0..n).max_by_key(|&i| markers[i].1).unwrap_or(0);
|
||||||
|
let (vck, ick) = markers[kmax];
|
||||||
|
let mut off_v = 0usize;
|
||||||
|
for &(vc, _) in markers.iter().take(kmax) {
|
||||||
|
off_v += vc * decl.stride;
|
||||||
|
}
|
||||||
|
let mut out = vec![format!(
|
||||||
|
"{name}: {n} sub-meshes, pivot #{kmax} ({vck} verts, {ick} idx), pool span {span}"
|
||||||
|
)];
|
||||||
|
for pad in 0..=3usize {
|
||||||
|
if vb0 < span + pad {
|
||||||
|
out.push(format!(" pad {pad}: pool start is before the index pool"));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let ib0 = vb0 - span - pad;
|
||||||
|
// Same gates the production pivot test uses: strict winding (0.85)
|
||||||
|
// AND connectivity. Reporting at 0.0 would accept blocks the decoder
|
||||||
|
// rejects and send the reader chasing the wrong gate.
|
||||||
|
let verdict = validate_block_report(
|
||||||
|
bytes,
|
||||||
|
ib0 + rel_ib[kmax],
|
||||||
|
vb0 + off_v,
|
||||||
|
vck,
|
||||||
|
ick,
|
||||||
|
&decl,
|
||||||
|
0.85,
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
out.push(match verdict {
|
||||||
|
Ok(()) => format!(" pad {pad}: ACCEPTED"),
|
||||||
|
Err(why) => format!(" pad {pad}: {why}"),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
out
|
||||||
|
}
|
||||||
|
|
||||||
/// Diagnostic: for a resource whose vertex buffer is *known* (a runtime capture
|
/// Diagnostic: for a resource whose vertex buffer is *known* (a runtime capture
|
||||||
/// names it), where could its index buffer be? The anchor scan assumes the index
|
/// names it), where could its index buffer be? The anchor scan assumes the index
|
||||||
/// buffer sits immediately before the vertex buffer; this scans the whole
|
/// buffer sits immediately before the vertex buffer; this scans the whole
|
||||||
@@ -998,17 +1052,43 @@ fn validate_block(
|
|||||||
min_consistency: f32,
|
min_consistency: f32,
|
||||||
strict_connectivity: bool,
|
strict_connectivity: bool,
|
||||||
) -> bool {
|
) -> bool {
|
||||||
|
validate_block_report(
|
||||||
|
bytes,
|
||||||
|
ib,
|
||||||
|
vb,
|
||||||
|
vtx_count,
|
||||||
|
index_count,
|
||||||
|
decl,
|
||||||
|
min_consistency,
|
||||||
|
strict_connectivity,
|
||||||
|
)
|
||||||
|
.is_ok()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// [`validate_block`], but naming the gate that rejected a block. A runtime
|
||||||
|
/// capture can prove a block is real; when the decoder still refuses it, this
|
||||||
|
/// says which test is wrong rather than leaving a threshold to be guessed at.
|
||||||
|
fn validate_block_report(
|
||||||
|
bytes: &[u8],
|
||||||
|
ib: usize,
|
||||||
|
vb: usize,
|
||||||
|
vtx_count: usize,
|
||||||
|
index_count: usize,
|
||||||
|
decl: &VertexDecl,
|
||||||
|
min_consistency: f32,
|
||||||
|
strict_connectivity: bool,
|
||||||
|
) -> Result<(), String> {
|
||||||
let stride = decl.stride;
|
let stride = decl.stride;
|
||||||
let idx_bytes = index_count * 2;
|
let idx_bytes = index_count * 2;
|
||||||
if ib + idx_bytes > bytes.len() {
|
if ib + idx_bytes > bytes.len() {
|
||||||
return false;
|
return Err("index buffer runs past the container".into());
|
||||||
}
|
}
|
||||||
let vtx_bytes = match vtx_count.checked_mul(stride) {
|
let vtx_bytes = match vtx_count.checked_mul(stride) {
|
||||||
Some(v) => v,
|
Some(v) => v,
|
||||||
None => return false,
|
None => return Err("vertex byte count overflows".into()),
|
||||||
};
|
};
|
||||||
if vb + vtx_bytes > bytes.len() {
|
if vb + vtx_bytes > bytes.len() {
|
||||||
return false;
|
return Err("vertex buffer runs past the container".into());
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Full index validation: every index in range, uses ~all vertices. ──
|
// ── Full index validation: every index in range, uses ~all vertices. ──
|
||||||
@@ -1016,12 +1096,14 @@ fn validate_block(
|
|||||||
for k in 0..index_count {
|
for k in 0..index_count {
|
||||||
let i = be16(bytes, ib + k * 2) as u32;
|
let i = be16(bytes, ib + k * 2) as u32;
|
||||||
if i >= vtx_count as u32 {
|
if i >= vtx_count as u32 {
|
||||||
return false;
|
return Err(format!("index {i} out of range (vtx_count {vtx_count})"));
|
||||||
}
|
}
|
||||||
max_idx = max_idx.max(i);
|
max_idx = max_idx.max(i);
|
||||||
}
|
}
|
||||||
if (max_idx as usize) + 4 < vtx_count {
|
if (max_idx as usize) + 4 < vtx_count {
|
||||||
return false;
|
return Err(format!(
|
||||||
|
"indices reach only {max_idx} of {vtx_count} vertices (buffer not covered)"
|
||||||
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Triangle quality: finite, non-degenerate, real spatial extent. ──
|
// ── Triangle quality: finite, non-degenerate, real spatial extent. ──
|
||||||
@@ -1046,7 +1128,7 @@ fn validate_block(
|
|||||||
for (a, slot) in pc.iter_mut().enumerate() {
|
for (a, slot) in pc.iter_mut().enumerate() {
|
||||||
let x = bef(bytes, base + a * 4);
|
let x = bef(bytes, base + a * 4);
|
||||||
if !x.is_finite() || x.abs() > 1.0e6 {
|
if !x.is_finite() || x.abs() > 1.0e6 {
|
||||||
return false;
|
return Err(format!("position component {x} is not finite/plausible"));
|
||||||
}
|
}
|
||||||
*slot = x;
|
*slot = x;
|
||||||
lo[a] = lo[a].min(x);
|
lo[a] = lo[a].min(x);
|
||||||
@@ -1088,7 +1170,9 @@ fn validate_block(
|
|||||||
}
|
}
|
||||||
let extent = (hi[0] - lo[0]).max(hi[1] - lo[1]).max(hi[2] - lo[2]);
|
let extent = (hi[0] - lo[0]).max(hi[1] - lo[1]).max(hi[2] - lo[2]);
|
||||||
if extent < 0.5 || sampled == 0 || degenerate * 10 > sampled * 3 {
|
if extent < 0.5 || sampled == 0 || degenerate * 10 > sampled * 3 {
|
||||||
return false; // too flat, or >30% degenerate → not this block
|
return Err(format!(
|
||||||
|
"extent {extent:.3} (min 0.5), {degenerate}/{sampled} degenerate (max 30%)"
|
||||||
|
));
|
||||||
}
|
}
|
||||||
// Connectivity check: a correctly-anchored mesh has triangle edges that
|
// Connectivity check: a correctly-anchored mesh has triangle edges that
|
||||||
// are SMALL relative to its overall size (~0.05–0.15 of the bbox
|
// are SMALL relative to its overall size (~0.05–0.15 of the bbox
|
||||||
@@ -1111,7 +1195,10 @@ fn validate_block(
|
|||||||
// `XBG7_EDGE_CAP_SMALL` instead — a targeted relaxation, off by default.
|
// `XBG7_EDGE_CAP_SMALL` instead — a targeted relaxation, off by default.
|
||||||
let cap = if tris < small_tris() { small_cap() } else { edge_cap() };
|
let cap = if tris < small_tris() { small_cap() } else { edge_cap() };
|
||||||
if mean_edge / diag > cap {
|
if mean_edge / diag > cap {
|
||||||
return false;
|
return Err(format!(
|
||||||
|
"connectivity: mean_edge/diag {:.3} > cap {cap:.2}",
|
||||||
|
mean_edge / diag
|
||||||
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Winding-consistency gate (same as `from_xpr2`): a correctly-anchored
|
// Winding-consistency gate (same as `from_xpr2`): a correctly-anchored
|
||||||
@@ -1123,10 +1210,13 @@ fn validate_block(
|
|||||||
if nrm_counted > 0 && min_consistency > 0.0 {
|
if nrm_counted > 0 && min_consistency > 0.0 {
|
||||||
let na = nrm_agree as f32 / nrm_counted as f32;
|
let na = nrm_agree as f32 / nrm_counted as f32;
|
||||||
if na.max(1.0 - na) < min_consistency {
|
if na.max(1.0 - na) < min_consistency {
|
||||||
return false;
|
return Err(format!(
|
||||||
|
"winding consistency {:.3} < {min_consistency:.2}",
|
||||||
|
na.max(1.0 - na)
|
||||||
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
true
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Decode a **grouped-pool** XBG7 resource: one whose descriptor holds *several*
|
/// Decode a **grouped-pool** XBG7 resource: one whose descriptor holds *several*
|
||||||
|
|||||||
@@ -108,10 +108,9 @@ fn shared_resources_decode_identically_in_every_container() {
|
|||||||
/// A runtime capture showed the container stores both halves of the `e106` hull
|
/// A runtime capture showed the container stores both halves of the `e106` hull
|
||||||
/// as separate X-reflected buffers, so a `…_01`/`…_02` pair of equal vertex
|
/// as separate X-reflected buffers, so a `…_01`/`…_02` pair of equal vertex
|
||||||
/// count should come out mirrored (or related by another axis / vertex order) —
|
/// count should come out mirrored (or related by another axis / vertex order) —
|
||||||
/// never identical, which is the collapse distinct assignment fixes. Disc-wide
|
/// never identical, which is the collapse distinct assignment fixes. Since that
|
||||||
/// this holds for every such pair except `n206`, whose twins are grouped-pool
|
/// fix reached grouped pools too, this holds for **every** twin pair on the disc
|
||||||
/// resources (4 sub-meshes) and so are excluded from distinct assignment; that
|
/// — including `n206`, which was the last exception.
|
||||||
/// one is the remaining known case, asserted explicitly so it cannot grow.
|
|
||||||
#[test]
|
#[test]
|
||||||
fn twin_pairs_do_not_share_a_buffer() {
|
fn twin_pairs_do_not_share_a_buffer() {
|
||||||
let Some(root) = disc_root() else {
|
let Some(root) = disc_root() else {
|
||||||
@@ -141,6 +140,5 @@ fn twin_pairs_do_not_share_a_buffer() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
collapsed.retain(|c| !c.starts_with("n206_01"));
|
|
||||||
assert!(collapsed.is_empty(), "twin pairs sharing one buffer: {collapsed:?}");
|
assert!(collapsed.is_empty(), "twin pairs sharing one buffer: {collapsed:?}");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -945,13 +945,25 @@ cannot regress).
|
|||||||
The runtime oracle is unchanged and cross-container inconsistency falls another
|
The runtime oracle is unchanged and cross-container inconsistency falls another
|
||||||
10 %. The suite, including the disc- and ISO-gated tests, stays green.
|
10 %. The suite, including the disc- and ISO-gated tests, stays green.
|
||||||
|
|
||||||
**It does *not* clear the `n206` collapse**, and that is informative: both twins
|
**It clears the `n206` collapse too** — a correction to what was written here
|
||||||
are grouped, the loser is re-placed past the taken pool, and no alternative pool
|
first. `n206_02` now anchors at `0x342d984` instead of sharing `0x33b6e54` with
|
||||||
**validates** — so it keeps the collided decode. `n206_02` is therefore the same
|
its twin. The earlier "no alternative pool validates" reading was wrong twice
|
||||||
class as `e106_eng_02_l` was before the cap moved: the correct block is rejected
|
over: `mesh::debug_grouped_report` (`examples/why_rejected.rs`) shows that pool
|
||||||
by the validator, not lost to selection. Its correct pool is one of
|
**ACCEPTED at pad 0** under the production gates, and `0x342d984` is in the
|
||||||
`0x342d984` (direct) or `0x33b7754` / `0x342e284` (mirrored); a capture of a
|
candidate start list. What actually misled the check was the audit itself —
|
||||||
stage containing `n206` would say which, and is the cheapest way to settle it.
|
it classified twins by decoded **geometry**, and `0x342d984` is a *direct*
|
||||||
|
(unmirrored) copy of `0x33b6e54`, so the pair still looked "identical" after it
|
||||||
|
had been separated.
|
||||||
|
|
||||||
|
The audit now distinguishes the two: identical geometry is only a collapse when
|
||||||
|
it comes from **one buffer**. Re-run disc-wide, of 34 equal-count twin pairs:
|
||||||
|
**18 exact X-mirror, 16 related another way, 0 identical-sharing-a-buffer, 0
|
||||||
|
unrelated** — and the regression test no longer needs its `n206` exception.
|
||||||
|
|
||||||
|
❔ Left open: whether `n206_01`/`n206_02` *should* be a mirrored pair at all. The
|
||||||
|
container holds two direct copies **and** two mirrored ones
|
||||||
|
(`0x33b7754`, `0x342e284`); our twins take the two direct copies, which is
|
||||||
|
self-consistent but unverified — `n206` appears in no captured stage.
|
||||||
|
|
||||||
### The twin invariant, checked disc-wide (2026-08-12)
|
### The twin invariant, checked disc-wide (2026-08-12)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user