mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-15 05:07:15 +00:00
refactor(highway): lift 79 per-instance closure vars into hwState (R3c H lift) (#849)
* refactor(highway): lift 79 per-instance closure vars into `hwState` (R3c H lift) Collapses createHighway()'s 79 mutable closure `let`s into one per-instance `hwState` object. Scope-resolved rewrite via acorn + eslint-scope: 1059 edits (1057 references + 79 defs - the deleted `let canvas, ctx, ws`), with the four names shadowed in inner scopes (chartTime/ctx/notes/chordTemplates) resolved correctly so only closure-bound refs move. Enables the later module split: extracted renderer/ws modules close over `hwState` as a factory arg, so multi-panel plugins (highway_3d, note_detect, splitscreen) don't share one highway's state. Container is `hwState`, NOT `H` — `H` is already canvas height (70 uses). The frame-time gate caught that collision instantly (0 draws, `H._drawHooks is not iterable` in the shared draw-hook path). PERF (the whole risk): identical to the pre-lift baseline. Draw p50 2.1-2.2 ms, p95 2.7-3.0 ms (pre-lift 2.7-3.2), measured on the Arcturus feedpak, headless. Each closure-slot read became a `hwState.<slot>` monomorphic property load; the hot loop pays nothing. On-device: Byron confirmed the 2D highway plays smoothly. Tests: the ~30 highway JS suites brace-extract functions/patterns from the source; their state references + the monotonic-clock vm sandbox now use `hwState.<slot>` (the const _CHART_MAX_INTERP_MS etc. stay top-level, not lifted). node --test: 1030/1030 green. Two self-inflicted over-replacements caught and reverted (`_lefty` is a prefix of the 3D-local `_leftyCached`; `STRING_COLORS` a suffix of `DEFAULT_STRING_COLORS`) — substring replaces on the brace-extract regexes need word care. Transformer saved at ~/.local/share/feedback-editor/highway-h-lift.mjs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(highway): pin the setNoteStateProvider assertion to hwState._noteStateProvider (CodeRabbit) The [^}]* form matched an unqualified _noteStateProvider =, so a regression to closure-level state could still pass. Require the hwState-qualified assignment. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
b6098e3695
commit
2281cac438
@@ -32,14 +32,14 @@ function extractBlock(src, signature) {
|
||||
|
||||
test('highway declares visibility state (_visibleOverride + _lastVisible)', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
assert.match(src, /let\s+_visibleOverride\s*=\s*null/, 'missing _visibleOverride (override sentinel)');
|
||||
assert.match(src, /let\s+_lastVisible\s*=\s*null/, 'missing _lastVisible (last-emitted state)');
|
||||
assert.match(src, /hwState\._visibleOverride\s*=\s*null/, 'missing _visibleOverride (override sentinel)');
|
||||
assert.match(src, /hwState\._lastVisible\s*=\s*null/, 'missing _lastVisible (last-emitted state)');
|
||||
});
|
||||
|
||||
test('_isHighwayVisible respects _visibleOverride and falls back to offsetParent', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const fn = extractBlock(src, 'function _isHighwayVisible()');
|
||||
assert.match(fn, /_visibleOverride\s*!==\s*null/, 'must check the override before the DOM');
|
||||
assert.match(fn, /hwState\._visibleOverride\s*!==\s*null/, 'must check the override before the DOM');
|
||||
assert.match(fn, /canvas\.offsetParent\s*!==\s*null/, 'DOM fallback must use offsetParent !== null');
|
||||
});
|
||||
|
||||
@@ -47,9 +47,9 @@ test('_emitVisibilityIfChanged is transition-only (no per-frame spam)', () => {
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const fn = extractBlock(src, 'function _emitVisibilityIfChanged()');
|
||||
// Must short-circuit when the current state equals the cached one.
|
||||
assert.match(fn, /v\s*===\s*_lastVisible/, 'must compare current vs _lastVisible and bail when equal');
|
||||
assert.match(fn, /v\s*===\s*hwState\._lastVisible/, 'must compare current vs _lastVisible and bail when equal');
|
||||
// Must update the cache and emit the event with the documented payload shape.
|
||||
assert.match(fn, /_lastVisible\s*=\s*v/, 'must update _lastVisible after a transition');
|
||||
assert.match(fn, /hwState\._lastVisible\s*=\s*v/, 'must update _lastVisible after a transition');
|
||||
assert.match(
|
||||
fn,
|
||||
/window\.feedBack\.emit\(\s*['"]highway:visibility['"][\s\S]*?visible:\s*v[\s\S]*?canvas/,
|
||||
@@ -66,7 +66,7 @@ test('rAF draw() loop calls _emitVisibilityIfChanged and skips when hidden', ()
|
||||
// transitions during loading/reconnect windows still propagate.
|
||||
const emitIdx = fn.search(/_emitVisibilityIfChanged\(\)/);
|
||||
const skipIdx = fn.search(/if\s*\(\s*!_rendering\s*\)\s*return/);
|
||||
const readyIdx = fn.search(/if\s*\(\s*!ready\s*\)\s*return/);
|
||||
const readyIdx = fn.search(/if\s*\(\s*!hwState\.ready\s*\)\s*return/);
|
||||
const drawIdx = fn.search(/_renderer\.draw\(/);
|
||||
assert.ok(emitIdx !== -1 && skipIdx !== -1 && readyIdx !== -1 && drawIdx !== -1, 'all four landmarks must be present');
|
||||
assert.ok(emitIdx < readyIdx, 'emit must run BEFORE the !ready gate (transitions during loading must still fire)');
|
||||
@@ -84,19 +84,19 @@ test('draw() keeps an active custom renderer painting through an override-hide (
|
||||
const src = fs.readFileSync(highwayJs, 'utf8');
|
||||
const fn = extractBlock(src, 'function draw()');
|
||||
// Single render decision drives both the perf-HUD reset and the gate.
|
||||
assert.match(fn, /let\s+_rendering\s*=\s*_lastVisible/, 'must derive a single _rendering decision from _lastVisible');
|
||||
assert.match(fn, /let\s+_rendering\s*=\s*hwState\._lastVisible/, 'must derive a single _rendering decision from _lastVisible');
|
||||
// Assert the exact boolean RELATIONSHIP, not just the tokens (CodeRabbit):
|
||||
// the exemption must AND together override-hide, an active custom renderer,
|
||||
// and the canvas still in layout. A weakened guard (e.g. `||`, or a dropped
|
||||
// offsetParent clause) must fail this — that's the regression being fixed.
|
||||
assert.match(
|
||||
fn,
|
||||
/!_rendering\s*&&\s*_visibleOverride\s*===\s*false\s*&&\s*_renderer\s*!==\s*_defaultRenderer\s*&&\s*canvas\s*&&\s*canvas\.offsetParent\s*!==\s*null/,
|
||||
/!_rendering\s*&&\s*hwState\._visibleOverride\s*===\s*false\s*&&\s*hwState\._renderer\s*!==\s*_defaultRenderer\s*&&\s*hwState\.canvas\s*&&\s*hwState\.canvas\.offsetParent\s*!==\s*null/,
|
||||
'exemption must AND override-hide + active custom renderer + canvas-in-layout (genuine off-screen still pauses, #246)',
|
||||
);
|
||||
// Both the HUD reset and the gate key off _rendering, not _lastVisible,
|
||||
// so the HUD doesn\'t churn while the custom renderer is actually drawing.
|
||||
assert.match(fn, /_perfHud\s*&&\s*\(\s*!_rendering/, 'perf-HUD reset must key off _rendering, not _lastVisible');
|
||||
assert.match(fn, /hwState\._perfHud\s*&&\s*\(\s*!_rendering/, 'perf-HUD reset must key off _rendering, not _lastVisible');
|
||||
assert.match(fn, /if\s*\(\s*!_rendering\s*\)\s*return/, 'the draw gate must bail on !_rendering');
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user