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

fix(xbg7): grouped selection prefers the candidate explaining the whole pool; per-sub-mesh decls on by default

Browse Source 
With per-sub-mesh declarations enabled, n201_01 decoded as a 2-part fragment 4 bytes
off. Both starts validate for the pivot — 0x32BA718 at pad 2 (earlier in file order,
so first-match took it) and the capture-proven 0x32BA71C at pad 0 — so the pivot
alone cannot separate them; at the early one two of four sub-meshes fall out as
out-of-range.

anchor_grouped_meshes now builds each accepted candidate and keeps the one that
explains the most of the declared pool: it returns immediately when a candidate
explains all n sub-meshes, else keeps the best partial, so it can never decode less
than first-match did. n201_01 lands on all four capture-proven offsets
(0x32BA71C / 0x32BEFF4 / 0x32C416C / 0x32C536C) and its two sibling copies take their
own pools, so the twin collapse is gone.

XBG7_SUBMESH_DECLS is therefore on by default (=0 reverts):

  resources that never decode        85 -> 47
  resources decoding in no container 63 -> 30
  degenerate index runs               1 -> 1   (unchanged)
  cross-container minority decodes   96 -> 96  (unchanged)
  captured index runs, stage-02      93/93     (unchanged)
  captured index runs, stage-05     124/128 -> 128/128

The last line is the point: the buffers the capture could not name are the n201
family, and they now decode and match the GPU's indices byte for byte. Suite green
including twin_pairs_do_not_share_a_buffer, apart from the pre-existing
known-failing cross-container consistency test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NptfmpjdpNCKEez6d2xvA9
This commit is contained in:
Claude (auto-RE)
2026-08-13 12:27:56 +00:00
parent 4cdbbc2d48
commit 3b35c48fce
2 changed files with 115 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1143,23 +1143,23 @@ fn pad0_consistency() -> f32 {
/// [`anchor_pool_mesh`] takes the FIRST pad that validates (the pre-fix
/// behaviour) instead of the pad whose index run is cleanest. Kept so the two
/// behaviours can be diffed on the disc; see docs/re/structures/xbg7-mesh.md.
/// Use the **per-sub-mesh** vertex declarations in a grouped pool
/// (`XBG7_SUBMESH_DECLS=1`, default off).
/// Use the **per-sub-mesh** vertex declarations in a grouped pool — **on by
/// default**; `XBG7_SUBMESH_DECLS=0` restores the single-declaration reading.
///
/// The format truth is not in question: each index marker is followed by its own
/// element triples and they can differ — `n201_01` (`Stage_S02.xpr`) declares
/// strides 24, 24, 24, **28**, which a runtime capture confirms draw-for-draw. With
/// this on, the disc-wide misses fall **85 → 47** and the resources that decode in
/// no container at all fall **63 → 30**.
/// Each index marker is followed by its own element triples and they can differ:
/// `n201_01` (`Stage_S02.xpr`) declares strides 24, 24, 24, **28**, which a runtime
/// capture confirms draw-for-draw (`stride=28` on the fourth draw, and a distinct
/// vertex shader per sub-mesh). Reading only the first declaration walked the last
/// buffer out of phase and declined the whole resource.
///
/// It is off by default because selection has not caught up: the three `n201_0x`
/// copies then land on ONE pool (`tests/mesh_consistency_disc.rs::twin_pairs_do_not_share_a_buffer`
/// fails), and four newly decoded `ptc_pack` `.dat` composites carry degenerate
/// triangles. The capture-proven pool start now VALIDATES (`debug_grouped_report`
/// reports `pad 0: ACCEPTED` where it used to report a NaN position), so what
/// remains is choosing it — see docs/re/structures/xbg7-mesh.md.
/// With this and the completeness-based candidate choice in
/// [`anchor_grouped_meshes`], `n201_01` anchors at the capture-proven pool start
/// with all four sub-meshes at the captured offsets, its two sibling copies take
/// their own pools, disc-wide misses fall **85 → 47**, and the captured index runs
/// of the stage-05 mission rise **124 → 128** identical. See
/// docs/re/structures/xbg7-mesh.md.
fn submesh_decls() -> bool {
std::env::var("XBG7_SUBMESH_DECLS").map(|v| v == "1").unwrap_or(false)
std::env::var("XBG7_SUBMESH_DECLS").map(|v| v != "0").unwrap_or(true)
}
fn pad_first_match() -> bool {
@@ -1710,6 +1710,49 @@ fn anchor_grouped_meshes(
let kmax = (0..n).max_by_key(|&i| markers[i].1).unwrap_or(0);
let (vck, ick) = markers[kmax];
// Build the pool at a candidate (vb0, pad). Sub-meshes that fail the
// structural requirements (every index inside its own buffer, indices
// reaching its end) are skipped, so the returned length says how much of the
// declared pool this candidate actually explains — which is what selects
// between candidates below.
let build = |vb0: usize, pad: usize| -> Vec<GameMesh> {
let ib0 = vb0 - span - pad;
let mut meshes = Vec::with_capacity(n);
let mut vb = vb0;
for i in 0..n {
let (vc, ic) = markers[i];
let ib = ib0 + rel_ib[i];
if ib + ic * 2 > bytes.len()
|| vc.checked_mul(decls[i].stride).map_or(true, |b| vb + b > bytes.len())
{
break;
}
let ok = validate_block(bytes, ib, vb, vc, ic, &decls[i], 0.85, false);
if !ok && i > kmax {
break; // chain diverged — emit the validated prefix, no garbage
}
let mut max_idx = 0usize;
let in_range = (0..ic).all(|k| {
let i = be16(bytes, ib + k * 2) as usize;
max_idx = max_idx.max(i);
i < vc
});
if in_range && max_idx + cover_slack() >= vc {
meshes.push(read_pool_mesh(bytes, ib, vb, ic, vc, &decls[i]));
}
vb += vc * decls[i].stride;
}
meshes
};
// A candidate that explains the WHOLE pool beats one that explains part of it,
// however early it sits in file order. `n201_01` is the case that forced this:
// a `vb0` **4 bytes before** the capture-proven start also validates for the
// pivot (at pad 2) and, being earlier in the scan, used to win — then two of
// the four sub-meshes fell out as out-of-range and the resource decoded as a
// 2-part fragment 4 bytes off. The proven start explains all four.
let mut partial: Option<Vec<GameMesh>> = None;
for &vb0 in starts {
// Distinct assignment: a pool another resource already claimed is not a
// candidate (see the collision resolution in `anchor_models_filtered`).
@@ -1749,58 +1792,19 @@ fn anchor_grouped_meshes(
}
}
if let Some((_, _, pad)) = best {
let ib0 = vb0 - span - pad;
// Pivot confirmed the exact alignment ⇒ every marker up to the pivot
// is correctly placed; read those unconditionally (a legitimately
// tiny/flat lead part may fail the quality gates yet still be real).
// Markers after the pivot are validated so a stray trailing marker
// ends the chain instead of appending garbage.
let mut meshes = Vec::with_capacity(n);
let mut vb = vb0;
for i in 0..n {
let (vc, ic) = markers[i];
let ib = ib0 + rel_ib[i];
if ib + ic * 2 > bytes.len()
|| vc.checked_mul(decls[i].stride).map_or(true, |b| vb + b > bytes.len())
{
break;
}
// Parts are placed deterministically; in-range + consistency pins
// them, so the connectivity heuristic (which mis-rejects small
// flat fins) is relaxed here.
let ok = validate_block(bytes, ib, vb, vc, ic, &decls[i], 0.85, false);
if !ok && i > kmax {
break; // chain diverged — emit the validated prefix, no garbage
}
// Sub-meshes BEFORE the pivot are emitted even when they fail the
// quality gates (a tiny flat lead part is legitimately poor), but
// an index that addresses past its own vertex buffer is not a
// quality question — it is unusable. Measured 2026-08-12: 18
// sub-meshes disc-wide carried indices up to 364 vertices past
// the end (`coverage_audit`), which any renderer would fault on.
// Same two structural requirements the searched path enforces:
// every index inside the buffer, and the indices reaching the
// end of it. Real geometry covers its pool exactly — 8 586 of
// 8 636 decoded sub-meshes reference their last vertex, none
// more than 3 short (`coverage_audit`) — so a sub-mesh whose
// indices stop well short is reading the wrong block, not a
// sparse one.
let mut max_idx = 0usize;
let in_range = (0..ic).all(|k| {
let i = be16(bytes, ib + k * 2) as usize;
max_idx = max_idx.max(i);
i < vc
});
if in_range && max_idx + cover_slack() >= vc {
meshes.push(read_pool_mesh(bytes, ib, vb, ic, vc, &decls[i]));
}
vb += vc * decls[i].stride;
// Pivot confirmed the alignment; how much of the pool does it explain?
let meshes = build(vb0, pad);
if meshes.len() == n {
return meshes;
}
if partial.as_ref().map_or(true, |p| meshes.len() > p.len()) {
partial = Some(meshes);
}
return meshes;
}
}
Vec::new()
// No candidate explained the whole pool — keep the best partial one, so this
// can never decode less than the previous first-match behaviour.
partial.unwrap_or_default()
}
/// How clean is the triangle list at `ib` against the pool at `vb`?

50
docs/re/structures/xbg7-mesh.md
View File

@@ -1603,7 +1603,7 @@ Flying stage 16 did **not** get the `e901` wings drawn (the boss appears later i
the mission), which is the next lesson: choosing the mission puts a container in
memory, but the unit still has to be **on screen** for a draw to exist.
### 🔎 The descriptor carries a declaration PER SUB-MESH — worth 38 misses, held behind a knob (2026-08-13)
### ✅ The descriptor carries a declaration PER SUB-MESH — worth 38 misses (2026-08-13)
Following the `n201` mixed-stride finding: the descriptor was dumped around every
index marker (`examples/desc_dump.rs`), and the layout is unambiguous — **each
@@ -1637,7 +1637,7 @@ And `debug_grouped_report` at `n201_01`'s **capture-proven** pool start now read
`pad 0: ACCEPTED`, where it used to read `position component NaN` — the block the
engine draws from is finally acceptable to the decoder.
**Why it is off by default.** Selection has not caught up:
**Selection had to catch up first** (it now has — see the next section):
* the three `n201_0x` copies all settle on ONE pool, so
`tests/mesh_consistency_disc.rs::twin_pairs_do_not_share_a_buffer` fails — a twin
@@ -1654,3 +1654,49 @@ So the format question is **settled** (and capture-confirmed), the coverage win
real and measured, and what stands between the two is the same
selection/distinct-assignment machinery that the pad work already improved once.
That is the next step, with `n201`'s proven offsets as the acceptance test.
### ✅ …and the selection bug it exposed: prefer the candidate that explains the WHOLE pool
With per-sub-mesh declarations on, `n201_01` decoded as a 2-part fragment **4 bytes**
off. The reason, from `debug_grouped_report` at both offsets:
```
vb0 0x32BA718 (4 bytes early) pad 2: ACCEPTED ← earlier in file order, so first-match took it
vb0 0x32BA71C (capture-proven) pad 0: ACCEPTED
```
Both validate for the **pivot** — the pivot alone cannot separate them. At the early
one, two of the four sub-meshes then fall out as out-of-range, so the resource decoded
as a fragment whose first sub-mesh sat at `0x32BEFF0` (= the early `vb0` + 777·24).
`anchor_grouped_meshes` now **builds** each accepted candidate and keeps the one that
explains the most of the declared pool, returning immediately when a candidate
explains all `n` sub-meshes and falling back to the best partial otherwise (so it can
never decode less than first-match did). The result is exact:
| `n201_01` sub-mesh | decoded vertex offset | capture-proven |
|---|---|---|
| #0 | `0x32BA71C` | `0x32BA71C` ✅ |
| #1 | `0x32BEFF4` | `0x32BEFF4` ✅ |
| #2 | `0x32C416C` | `0x32C416C` ✅ |
| #3 | `0x32C536C` | `0x32C536C` ✅ |
and `n201_02` / `n201_03` take their own distinct pools (`0x3353BEC`, `0x3388BEC`), so
the twin collapse is gone.
**Both changes are now the default** (`XBG7_SUBMESH_DECLS=0` reverts the declaration
reading):
| | before | after |
|---|---|---|
| resources that never decode | 85 | **47** |
| resources decoding in **no** container | 63 | **30** |
| decoded index runs with a degenerate triangle | 1 | **1** |
| cross-container minority decodes | 96 | 96 |
| captured index runs identical, stage-02 capture | 93/93 | **93/93** |
| captured index runs identical, stage-05 mission capture | 124/128 (4 undecoded) | **128/128** |
The last row is the one that matters most: the four buffers that capture could not
name are the `n201` family, and now they decode **and** their index runs match the
GPU byte for byte. Suite green (`twin_pairs_do_not_share_a_buffer` included) apart
from the pre-existing known-failing cross-container consistency test.