perf: draw vu meter and timeline ruler on canvases#44
Conversation
📝 WalkthroughWalkthroughPlayback timing now reuses project duration calculations, ruler drawing can run in a dedicated OffscreenCanvas worker, audio meters render through canvases, and preview property evaluation is limited to the active playhead clip. ChangesPlayback and rendering updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Timeline
participant app.js
participant ruler-worker.js
participant OffscreenCanvas
Timeline->>app.js: request ruler update
app.js->>ruler-worker.js: send scroll, scale, markers, work area, time, and fps
ruler-worker.js->>OffscreenCanvas: render ruler ticks, labels, markers, and playhead
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ruler-worker.js`:
- Around line 95-99: Update ensureRulerWorker and the ruler-worker init/draw
protocol so worker activation is confirmed only after successful canvas transfer
and 2D context initialization. Add explicit init acknowledgment/error signaling,
defer or discard worker-only draw messages until readiness, and clear or
recreate rulerWorker on startup or runtime errors so fallback rendering remains
available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 885c4ad2-432e-4069-bffd-bac3d817cd09
📒 Files selected for processing (3)
app.jsruler-worker.jsstyle.css
| if (msg.type === "init") { | ||
| cv = msg.canvas; | ||
| g = cv.getContext("2d"); | ||
| } else if (msg.type === "draw") { | ||
| draw(msg); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'transferControlToOffscreen|new Worker|rulerWorker\s*=|onerror|onmessage|getContext\("2d"\)' app.js ruler-worker.jsRepository: ronak-create/FableCut
Length of output: 7170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== app.js ruler section =="
sed -n '1585,1705p' app.js | nl -ba -v1585
echo
echo "== ruler-worker.js =="
sed -n '1,130p' ruler-worker.js | nl -ba -v1
echo
echo "== current git diff stat and ruler app.js/workers changes =="
git diff --stat -- app.js ruler-worker.js || true
git diff -- app.js ruler-worker.js | sed -n '1,260p' || true
echo
echo "== all ruler-related occurrences =="
rg -n -C2 'drawRuler|ensureRulerWorker|rulerWorker|ruler|OffscreenCanvas|transferControlToOffscreen' app.js ruler-worker.jsRepository: ronak-create/FableCut
Length of output: 226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== app.js ruler section =="
sed -n '1585,1705p' app.js | cat -n | sed 's#^`#app.js`:'
echo
echo "== ruler-worker.js =="
sed -n '1,145p' ruler-worker.js | cat -n | sed 's#^`#ruler-worker.js`:'
echo
echo "== current git diff stat and ruler app.js/workers changes =="
git diff --stat -- app.js ruler-worker.js || true
git diff -- app.js ruler-worker.js | cat -n | sed 's#^`#diff`:' || true
echo
echo "== all ruler-related occurrences =="
rg -n -C3 'drawRuler|ensureRulerWorker|rulerWorker|ruler|OffscreenCanvas|transferControlToOffscreen' app.js ruler-worker.jsRepository: ronak-create/FableCut
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== app.js ruler section =="
sed -n '1585,1705p' app.js
echo
echo "== ruler-worker.js =="
sed -n '1,145p' ruler-worker.js
echo
echo "== current git diff stat and ruler app.js/workers changes =="
git diff --stat -- app.js ruler-worker.js || true
git diff -- app.js ruler-worker.js || true
echo
echo "== all ruler-related occurrences =="
rg -n -C3 'drawRuler|ensureRulerWorker|rulerWorker|ruler|OffscreenCanvas|transferControlToOffscreen' app.js ruler-worker.jsRepository: ronak-create/FableCut
Length of output: 14459
Add worker readiness/error signaling before disabling the fallback.
ensureRulerWorker() treats rulerWorker as active immediately after creating the worker and posting the transferred canvas. If the worker fails to load, or cv.getContext("2d") fails inside ruler-worker.js, g remains null, draw messages are dropped, and the fallback can no longer render because the main canvas was already transferred. Add init acknowledgment or error signaling, queue/drop worker-only draws until the worker is ready, and remove/marked-recreate the worker when an error occurs before falling back.
🤖 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 `@ruler-worker.js` around lines 95 - 99, Update ensureRulerWorker and the
ruler-worker init/draw protocol so worker activation is confirmed only after
successful canvas transfer and 2D context initialization. Add explicit init
acknowledgment/error signaling, defer or discard worker-only draw messages until
readiness, and clear or recreate rulerWorker on startup or runtime errors so
fallback rendering remains available.
1 participant
What does this PR do?
VU meter drawing was potentially affecting performance (16 divs per a channel), so in this PR it is being drawn on a canvas - a separate canvas per each channel, as the number of channels is not fixed as per #41 The number of actual draw operations was also reduced, redrawing is called only when lit/hold changes.
Also, timeline ruler is drawn on a canvas to reduce the DOM size. Additionally, the logic was moved to a worker (but there is a fallback to use the main thread in case worker is not available).
Type of change
How was it verified?
node --check server.js && node --check app.js && node --check mcp-server.jspassesCLAUDE.md/README.mdif the schema, props, or API changedChecklist
Summary by CodeRabbit
Performance
Visual Improvements