Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,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
- **`playback.loop-api` bridge no longer fires dozens of times per second.** Every `window.feedBack.getLoop()` call recorded a full bridge hit — compat-shim bookkeeping, a `playback:bridge-hit` event, and a diagnostics snapshot rebuild + stringify — so a plugin polling loop state from a HUD tick (note_detect at ~30 Hz) flooded the capability inspector and burned main-thread time even with no song playing. `_recordPlaybackBridge` now throttles per bridge/surface (5 s window): bridge hits are a "surface still in use" signal, not a call counter. The manual A/B loop buttons (`setLoopEnd`) also now emit the same `loop-set` transport event as `setLoop()`, so plugins can react to loop changes via `playback:loop-set` / `playback:loop-cleared` events instead of polling `getLoop()`.
- **3D Highway: recover from a WebGL context loss instead of crashing on alt-tab.** Switching the active window / alt-tabbing away from the app (most often on Windows) can trigger a GPU context reset; the 3D highway's WebGL renderer had **no `webglcontextlost` handler**, so a lost context was left to escalate into a render-process crash — matching the intermittent "randomly crashes when I change windows" desktop reports. The renderer now binds `webglcontextlost`/`webglcontextrestored` on its own WebGL canvas (`ren.domElement`): the loss is `preventDefault()`'d so the browser keeps the context restorable, `draw()` bails while the context is down so no GL work runs on a dead context, and on restore the viewport is re-applied and rendering resumes (Three re-uploads scene resources on the next frame). Listeners are torn down with the renderer. `plugins/highway_3d` → 3.31.3. Tests: `tests/js/highway_3d_context_loss.test.js`. (The sibling `keys_highway_3d` / `drum_highway_3d` renderers share the same gap — tracked as a follow-up in their repos.)
- **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).
- **v3 Songs grid: fixed the scroll stutter that "skips every so many scrolls," up or down.** The virtualized grid rebuilt its **entire** visible window (`grid.innerHTML = …` + a full `wireCards` pass) every time it slid by one row, so each row-boundary crossing was a heavy synchronous frame that stalled the main thread and buffered held-arrow key-repeats into a visible lurch (a tester's "super fast for a second then slowed down") at fixed scroll offsets — in **both directions and regardless of whether the page was already loaded** (the cost was DOM teardown, not fetching, which is why scrolling back up over cached songs hitched too). `renderWindow()` now **reconciles the window in place**: it reuses the card nodes that stay on-screen and builds only the row that enters/leaves (~6 nodes per slide instead of ~60), keyed by absolute index with a real-vs-skeleton + select-mode signature so hole-fills (after a page fetch) and select-mode toggles still rebuild exactly the nodes that changed. `wireCards`'s `data-wired` guard then wires only the freshly-built nodes, so per-slide listener churn drops with it. Follow-up to the stage-2 virtualized grid (got-feedback/feedBack#636 item 3). Frontend-only: `static/v3/songs.js`. Tests: `tests/js/v3_songs_window_recycle.test.js` (window stays `[start,end)` contiguous + in-window node identity reused across a down-then-up scroll; select-mode toggle and rail-seek jump rebuild correctly).
Expand Down
19 changes: 19 additions & 0 deletions static/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@
if (_getArrangementNamingMode() === 'smart') {
filters.arrHas = _toSmartArrs(filters.arrHas);
filters.arrLacks = _toSmartArrs(filters.arrLacks);
}

Check warning on line 1501 in static/app.js

View workflow job for this annotation

GitHub Actions / ci / lint

File has too many lines (11852). Maximum allowed is 1500
return filters;
} catch {
return _defaultLibFilters();
Expand Down Expand Up @@ -5564,9 +5564,24 @@
: null;
}

// Bridge hits are a "this legacy surface is still in use" signal, not a call
// counter — but recordBridgeHit is not cheap (compat-shim bookkeeping, a
// playback:bridge-hit event, and a diagnostics snapshot rebuild per call).
// Plugins legitimately poll read surfaces like window.feedBack.getLoop() from
// HUD ticks (note_detect polled at ~30 Hz), which turned every tick into a
// snapshot serialization on the main thread and saturated the inspector's
// hitCount. Throttle per surface: the first call records immediately, repeats
// within the window are dropped.
const _bridgeRecordLast = new Map();
const _BRIDGE_RECORD_MIN_MS = 5000;
function _recordPlaybackBridge(bridgeId, legacySurface, reason) {
const playback = _playbackApi();
if (!playback || typeof playback.recordBridgeHit !== 'function') return;
const key = `${bridgeId}|${legacySurface}`;
const now = Date.now();
const last = _bridgeRecordLast.get(key);
if (last != null && now - last < _BRIDGE_RECORD_MIN_MS) return;
_bridgeRecordLast.set(key, now);
playback.recordBridgeHit({
bridgeId,
legacySurface,
Expand Down Expand Up @@ -7940,6 +7955,10 @@
if (loopB <= loopA) { loopB = null; return; }
document.getElementById('btn-loop-b').className = 'px-3 py-1.5 bg-green-900/50 rounded-lg text-xs text-green-300 transition';
updateLoopUI();
// Manual A/B arming is a loop mutation like setLoop()'s — emit the same
// transport event so event-driven consumers (note_detect drill sync) see
// button-armed loops without having to poll getLoop().
window.feedBack?.playback?.transportEvent?.('loop-set', { requesterId: 'core.loop', loopA, loopB, loop: { startTime: loopA, endTime: loopB, enabled: true, state: 'active' } });
}

function clearLoop(options) {
Expand Down
4 changes: 3 additions & 1 deletion static/capabilities/audio-effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

const SCHEMA = 'feedBack.audio_effects.diagnostics.v1';
const PLAN_SCHEMA = 'feedBack.audio_effects.chain_plan.v1';
// Pre-rebrand plugins (rig_builder <= 2.9.x) still send the old schema id — accept it as an alias.
const LEGACY_PLAN_SCHEMA = 'slopsmith.audio_effects.chain_plan.v1';
Comment on lines +12 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Version number in comment conflicts with PR description.

Comment says rig_builder <= 2.9.x but the PR objectives state the affected legacy plugin version is rig_builder ≤ 3.0.6. Fix the comment to avoid confusing future maintainers about which plugin versions require this alias.

📝 Proposed fix
-// Pre-rebrand plugins (rig_builder <= 2.9.x) still send the old schema id — accept it as an alias.
+// Pre-rebrand plugins (rig_builder <= 3.0.6) still send the old schema id — accept it as an alias.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Pre-rebrand plugins (rig_builder <= 2.9.x) still send the old schema id — accept it as an alias.
const LEGACY_PLAN_SCHEMA = 'slopsmith.audio_effects.chain_plan.v1';
// Pre-rebrand plugins (rig_builder <= 3.0.6) still send the old schema id — accept it as an alias.
const LEGACY_PLAN_SCHEMA = 'slopsmith.audio_effects.chain_plan.v1';
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@static/capabilities/audio-effects.js` around lines 12 - 13, The inline
comment above LEGACY_PLAN_SCHEMA in audio-effects.js has the wrong
affected-version range and conflicts with the PR intent. Update that comment to
match the actual legacy plugin scope described for the schema alias, keeping the
code behavior unchanged, so future maintainers can correctly identify which
rig_builder versions still send the old schema id.

const OWNER_ID = 'core.audio.effects';
const DEFAULT_ROUTE_KEY = 'desktop-main';
const DEFAULT_TIMEOUT_MS = 2000;
Expand Down Expand Up @@ -734,7 +736,7 @@
const errors = [];
const source = _plainObject(rawPlan);
const schema = _string(source.schema || source.version, PLAN_SCHEMA);
if (schema !== PLAN_SCHEMA && schema !== '1') errors.push('Unsupported chain plan schema');
if (schema !== PLAN_SCHEMA && schema !== LEGACY_PLAN_SCHEMA && schema !== '1') errors.push('Unsupported chain plan schema');
const planRoute = _safeRoute(source.routeKey || source.route || routeKey);
if (planRoute !== routeKey) errors.push('Chain plan route does not match selected route');
const providerId = _safeId(source.providerId || provider.providerId, provider.providerId);
Expand Down
Loading