fix(gpx): clamp partial final BCFS sector so GP6 .gpx import works

Every real Guitar Pro 6 (.gpx) file failed to import with
"GPX BCFS sector pointer out of range (malformed file)".

A real .gpx's BCFZ-declared decompressed size isn't 0x1000-aligned, so
its last (small) container file lands in a partial trailing sector.
_parse_bcfs raised whenever a sector read would run past the buffer
end, rejecting the whole container before score.gpif could be extracted
-- so no GP6 file could be charted in the song editor. (GP7/GP8 .gp
files take the ZIP path, not BCFS, which is why this wasn't caught
earlier.)

Clamp the final sector read to the buffer end (the per-file size field
trims the padding anyway), matching canonical GPX readers (alphaTab /
PyGuitarPro). A sector whose start is past the end still raises, so the
malformed-file guard is preserved.

Verified against two real GP6 files -- both now unpack to valid GPIF
with all tracks. Adds the previously-missing positive BCFS round-trip
coverage: partial-final-sector, multi-file, sector-aligned baseline,
and the preserved out-of-range guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
ChrisBeWithYou 2026-07-03 23:28:46 -05:00
parent b6169af6aa
commit 4106f25ee2
3 changed files with 85 additions and 3 deletions

View File

@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **Player frame-time hotspots removed (trace-backed) + weak-hardware hardening.** A Chrome performance trace of a 3D-highway session surfaced two core per-frame layout-thrash sources, now fixed: the highway's visibility check read `canvas.offsetParent` every rAF frame (forces style/layout recalc — now sampled every 10th frame with a cached value, force-refreshed on init/canvas-replace/resize/override-clear), and the v3 player chrome loop called `matches(':hover')` per frame and unconditionally rewrote the Up-Next pill's `textContent`/bar width at 6 Hz (now hover-tracked via mouseenter/mouseleave, DOM writes only on value change, progress bar moved from `width` to compositor-only `scaleX`). The 3D highway pre-warms shader programs (`ren.compile`) and deterministic label textures at init — and chart-dependent chord/section label textures on first draw — so first-appearance shader-compile/texture-upload frame spikes move into the load spinner. For weaker hardware: the per-frame renderer bundle is now a single reused object instead of a fresh ~35-field allocation per frame (object identity is stable and meaningless; array fields still swap reference on chart changes), custom viz get `bundle.lowerBoundT`/`bundle.lowerBoundTime` binary-search helpers for visible-window culling, the default 2D highway's beat lines no longer scan every beat in the song per frame, and the 3D highway stops reading `localStorage` per frame (1 Hz poll) and caches its lyrics text-measurement layout per displayed line instead of re-measuring every syllable every frame. A second, throttled-CPU trace pass additionally removed: shader-program re-resolution churn from label texture swaps (`material.needsUpdate` is now only set on a null↔texture transition — swapping between two cached label textures never changes the compiled program), the 3D highway's per-frame `getBoundingClientRect` layout read in its canvas-size self-check (now every 10th frame, still immediate on backing-store change), and the core 60 Hz HUD clock rewriting `textContent` on every tick (now write-on-change, ~1/s). The dominant residual — steady `getParameters` shader-program re-resolution (~4% of throttled main thread) — turned out to be Three r158+'s transparent-DoubleSide two-pass rendering, which sets `material.needsUpdate` twice per object per frame; all 18 of the 3D highway's transparent DoubleSide materials are flat unlit quads (labels, rails, chord frames, lanes), so they now declare `forceSinglePass: true`, eliminating the recompile churn and halving those objects' draw calls.
### Fixed
- **Guitar Pro 6 (`.gpx`) import no longer fails on every real file.** The GPX BCFS container reader (`lib/gp2rs_gpx.py`) rejected any file whose final sector wasn't a full `0x1000` block — but a real `.gpx`'s BCFZ-declared decompressed size isn't sector-aligned, so the last (small) container file always lands in a partial trailing sector. The bounds check *raised* `GPX BCFS sector pointer out of range (malformed file)` instead of clamping the tail read, so `_load_gpif` threw before `score.gpif` could be extracted and **no GP6 file could be imported into the song editor** (both real test files failed identically — this wasn't file-specific). GP7/GP8 `.gp` files were unaffected — they take the ZIP path, not BCFS, which is why prior GP-import work didn't surface it. The reader now **clamps the final sector read to the buffer end** (the per-file size field trims the padding anyway), matching canonical GPX readers (alphaTab / PyGuitarPro); a sector whose *start* is past the end still raises, preserving the malformed-file guard. Verified against two real GP6 files — both now unpack to valid GPIF with all tracks. Tests: `tests/test_gp2rs_gpx.py` (partial-final-sector round-trip, multi-file container, sector-aligned baseline, and the preserved out-of-range guard).
- **Edit Metadata now writes into `.feedpak` files, not just legacy `.sloppak` ones.** `lib/songmeta.py`'s suffix gate predated the format rename — core reads both suffixes everywhere else (`sloppak.SONG_EXTS`), but the metadata writer only dispatched on `.sloppak`, so editing a zip-form `.feedpak`'s title/artist/album/year silently fell back to a DB-only update. That looked fine until the next **full library rescan** re-derived metadata from the file and reverted the edit (directory-form packages were unaffected — they dispatch on manifest presence, not suffix). The gate now accepts both package suffixes. Tests: `tests/test_songmeta.py` `TestWriteSongMetadata` (both zip suffixes, mixed-case suffix, directory form, unknown-suffix fallback).
- **3D Drum & Keys highways now re-frame on fullscreen/layout drift under splitscreen.** The guitar/bass `highway_3d` self-detects when its panel canvas changes size and re-runs `applySize()` every frame, because the splitscreen host overrides `hw.resize` and never calls `renderer.resize()`. The drum and keys highways lacked that fallback — they only re-framed when the host explicitly called `resize(w, h)` — so their panels stayed framed for the pre-fullscreen size while the guitar/bass panels adapted (visible as a too-small, off-center highway after maximizing a split-screen session). Both draw loops now port `highway_3d`'s per-frame drift check: they re-apply on backing-store change (`canvas.width/height`) AND on CSS-box drift (`clientWidth/clientHeight` vs the last applied logical size, throttled to every 10th frame), and reset the tracking in `destroy()` so a reused instance re-frames on the next song. `plugins/drum_highway_3d` → 0.3.1, `plugins/keys_highway_3d` → 0.1.1. Tests: `tests/js/drum_keys_highway_3d_resize_reframe.test.js`.
- **Tuner: finished the "remove unused settings" cleanup and fixed the sidebar panel position.** The Floating Button and Tuning Visibility settings sections were removed, but their config was still live: `disabledTunings` still filtered the tuner menu (with no UI left to re-enable a hidden tuning — a one-way trap) and `showFloatingButton` still gated the floating launcher. Both are now fully retired — the enforcement paths in `plugins/tuner/screen.js`/`utils/ui.js` and the persistence in `plugins/tuner/routes.py` are gone (and `routes.py` strips the retired keys on write, so stale values are purged). The tuner panel opened from the v3 sidebar Plugins rail popover now anchors beside it via the host's stable plugin-control slot API (falling back to the popover id), is **clamped to the viewport** so it can't open off the right/bottom edge on narrow/short windows, and re-anchors on window resize. `plugins/tuner` → 1.3.3.

View File

@ -121,10 +121,18 @@ def _parse_bcfs(bcfs: bytes) -> dict:
while sc <= max_sectors:
s = _gi(po + 4 * sc); sc += 1
if s == 0: break
so = s * SECTOR
if HDR + so + SECTOR > len(data):
start = HDR + s * SECTOR
# Real .gpx files' final sector is a few bytes short of a full
# 0x1000 block: the BCFZ-declared decompressed size isn't
# sector-aligned, so the last (small) container file lands in a
# partial trailing sector. Clamp the read to the buffer end —
# the per-file size field (`fs`, applied below) trims any
# padding — matching canonical GPX readers (alphaTab /
# PyGuitarPro slice-and-clamp). Only a sector whose *start* is
# past the end is genuinely malformed.
if start < 0 or start >= len(data):
raise ValueError("GPX BCFS sector pointer out of range (malformed file)")
fb.extend(data[HDR + so: HDR + so + SECTOR])
fb.extend(data[start: min(start + SECTOR, len(data))])
else:
raise ValueError("GPX BCFS sector chain too long (malformed file)")
files[fn] = bytes(fb[:fs])

View File

@ -122,6 +122,79 @@ def test_parse_bcfs_rejects_bad_magic():
_parse_bcfs(b"NOPE" + b"\x00" * 16)
# ── _parse_bcfs container round-trip (GP6 .gpx partial final-sector) ─────────
def _build_bcfs(entries, short_by=0):
"""Assemble a minimal in-memory BCFS container for _parse_bcfs.
``entries`` is ``[(name: bytes, payload: bytes, data_sector: int), ...]``.
The directory entry for entry *i* is written to sector ``i + 1``; each
entry's payload goes in the sector index it names. ``short_by`` truncates
the final buffer by N bytes to emulate a real .gpx's partial trailing
sector (the BCFZ-declared decompressed size isn't 0x1000-aligned). Layout
mirrors the reader: a 4-byte ``BCFS`` header, then 0x1000-byte sectors,
with every value read at ``HDR + sector * 0x1000``.
"""
SECTOR = 0x1000
HDR = 4
max_sector = max([e[2] for e in entries] + [len(entries)])
buf = bytearray(b"BCFS" + b"\x00" * ((max_sector + 1) * SECTOR))
def put_u32(off, val):
struct.pack_into("<I", buf, HDR + off, val)
for i, (name, payload, data_sector) in enumerate(entries):
dir_off = (i + 1) * SECTOR # directory entry -> sector i+1
put_u32(dir_off + 0x00, 2) # entry type: file
nm = name[:127]
buf[HDR + dir_off + 0x04: HDR + dir_off + 0x04 + len(nm)] = nm
put_u32(dir_off + 0x8C, len(payload)) # declared file size
put_u32(dir_off + 0x94, data_sector) # first data-sector pointer
put_u32(dir_off + 0x94 + 4, 0) # chain terminator
dpos = HDR + data_sector * SECTOR
buf[dpos: dpos + len(payload)] = payload
if short_by:
del buf[len(buf) - short_by:]
return bytes(buf)
def test_parse_bcfs_reads_short_final_sector():
"""The regression: a real .gpx ends a byte short of a full 0x1000 sector,
so its last (small) container file lands in a partial trailing sector. The
reader must clamp that read, not reject the whole container rejecting it
is what made every GP6 .gpx fail to import with 'sector pointer out of
range'."""
bcfs = _build_bcfs([(b"score.gpif", b"hello", 2)], short_by=1)
assert (len(bcfs) - 4) % 0x1000 == 0x1000 - 1 # final sector is 1 short
assert _parse_bcfs(bcfs)["score.gpif"] == b"hello"
def test_parse_bcfs_full_sector_round_trip():
"""A sector-aligned container round-trips unchanged (baseline)."""
assert _parse_bcfs(_build_bcfs([(b"misc.xml", b"<x/>", 2)]))["misc.xml"] == b"<x/>"
def test_parse_bcfs_multi_file_short_final_sector():
"""Real-world shape: score.gpif plus small config files, the last one in
the partial trailing sector."""
out = _parse_bcfs(_build_bcfs([
(b"score.gpif", b"<GPIF/>", 3),
(b"LayoutConfiguration", b"AB", 4),
], short_by=1))
assert out["score.gpif"] == b"<GPIF/>"
assert out["LayoutConfiguration"] == b"AB"
def test_parse_bcfs_rejects_sector_starting_past_end():
"""A sector pointer whose *start* is beyond the container is genuinely
malformed and must still raise the clamp tolerates a partial final
sector, not arbitrary out-of-range pointers."""
bcfs = bytearray(_build_bcfs([(b"x", b"y", 2)]))
struct.pack_into("<I", bcfs, 4 + 0x1000 + 0x94, 9999) # absurd data-sector ptr
with pytest.raises(ValueError, match="out of range"):
_parse_bcfs(bytes(bcfs))
# ── _note_is_tie ────────────────────────────────────────────────────────────
def test_note_is_tie_destination():