perf(highway): held-sustain glow without shadowBlur (Mac stutter fix)

[Jafz2001]

Problem

User report: frequent hitches while playing on macOS, worst during held notes.

drawSustains set ctx.shadowBlur per lit trail per frame. Blur cost scales with the blurred device-pixel area — the trail can span half the canvas and the canvas is DPR-scaled (4× pixels on a 2× Mac), so this was a large per-frame raster pass that runs exactly while a sustain is held.

Fix

Three inflated low-alpha fills of the same trail quad instead of the blur — same soft shimmering glow (the shimmer LUT still drives the per-frame flicker, feedBack#254 intent preserved), flat cost independent of trail size. No shadowBlur uses remain in highway.js.

Testing

• node --check clean.
• Visual: glow/shimmer/crackle intact on lit sustains (3-pass inflate reads within a hair of the blurred original; dim/miss trails untouched).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Updated the visual treatment of held sustain notes so their glowing trail is rendered with smoother layered highlights and more natural shimmer.
  • Kept the miss/unglow appearance unchanged.

[coderabbitai]

📝 Walkthrough

Walkthrough

The glow rendering for held sustain trails in drawSustains was changed from a single shadowBlur-based fill to multiple fills of the same trail quad at varying inflation sizes and globalAlpha values derived from shimmer noise. The subsequent crackling current stroke logic remains unchanged.

Changes

Sustain Glow Rendering

Layer / File(s)	Summary
Layered glow fill for held sustains static/highway.js	Replaces ctx.shadowBlur/shadowColor glow with a fillTrail(inflate) helper drawing multiple inflated quad fills using globalAlpha from _shimmerNoise(seedBase), keeping the crackling current stroke logic intact.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: replacing held-sustain glow shadowBlur to improve macOS performance.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/highway-sustain-glow-no-shadowblur

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

static/highway.js (1)

2017-2030: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid allocating fillTrail per note per frame in the hot path.

fillTrail is a fresh arrow-function closure created for every lit sustain on every animation frame — exactly the kind of per-frame allocation this PR (and the rest of the file, e.g. _bundleReused, _shimmerLut, the stable-reference comment on _noteState) otherwise takes care to avoid. Hoisting it to a top-level helper (same pattern as roundRect) that accepts the geometry as parameters removes the closure allocation without changing behavior.

♻️ Proposed refactor

+    // Fills the sustain-trail quad inflated by `inflate` px on each side —
+    // shared by all three glow-layer passes in drawSustains. Top-level
+    // (not a per-call closure) so it isn't reallocated per lit sustain
+    // per frame.
+    function fillSustainTrail(x0, y0, x1, y1, sw0, sw1, inflate) {
+        ctx.beginPath();
+        ctx.moveTo(x0 - sw0 - inflate, y0);
+        ctx.lineTo(x0 + sw0 + inflate, y0);
+        ctx.lineTo(x1 + sw1 + inflate, y1);
+        ctx.lineTo(x1 - sw1 - inflate, y1);
+        ctx.fill();
+    }
+
     function drawSustains(W, H) {
         ...
-                const fillTrail = (inflate) => {
-                    ctx.beginPath();
-                    ctx.moveTo(x0 - sw0 - inflate, y0);
-                    ctx.lineTo(x0 + sw0 + inflate, y0);
-                    ctx.lineTo(x1 + sw1 + inflate, y1);
-                    ctx.lineTo(x1 - sw1 - inflate, y1);
-                    ctx.fill();
-                };
                 ctx.globalAlpha = baseA * 0.22;
-                fillTrail(glowPx);
+                fillSustainTrail(x0, y0, x1, y1, sw0, sw1, glowPx);
                 ctx.globalAlpha = baseA * 0.4;
-                fillTrail(glowPx * 0.45);
+                fillSustainTrail(x0, y0, x1, y1, sw0, sw1, glowPx * 0.45);
                 ctx.globalAlpha = baseA;
-                fillTrail(0);
+                fillSustainTrail(x0, y0, x1, y1, sw0, sw1, 0);

🤖 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/highway.js` around lines 2017 - 2030, The per-frame `fillTrail` arrow
function inside the sustain rendering hot path is creating a new closure for
every lit note, which should be avoided. Hoist `fillTrail` out to a reusable
helper in the same style as `roundRect`, and pass the geometry and context
values it needs as parameters so the existing drawing calls in this
sustain-rendering block can reuse it without changing behavior.

🤖 Prompt for all review comments with 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.

Nitpick comments:
In `@static/highway.js`:
- Around line 2017-2030: The per-frame `fillTrail` arrow function inside the
sustain rendering hot path is creating a new closure for every lit note, which
should be avoided. Hoist `fillTrail` out to a reusable helper in the same style
as `roundRect`, and pass the geometry and context values it needs as parameters
so the existing drawing calls in this sustain-rendering block can reuse it
without changing behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1c5602ba-8fce-484a-8193-5c1d1c0138aa

📥 Commits

Reviewing files that changed from the base of the PR and between 612b1f2 and 62df3ae.

📒 Files selected for processing (1)

• static/highway.js