Optimize waveform rendering and timeline preview updates #1317
Conversation
Improves waveform rendering in ClipTrack by adjusting sample stepping and using requestAnimationFrame for smoother updates. Timeline preview time updates are now throttled to reduce unnecessary state changes and improve performance during mouse movement.
WalkthroughImplements an animation-frame-driven waveform rendering flow with dynamic per-render sampling in the timeline clip track, and adds a throttled, guarded preview-time update for timeline mouse-move events to reduce update frequency. Changes
Sequence Diagram(s)sequenceDiagram
participant Canvas as ClipTrack Canvas
participant RAF as requestAnimationFrame
participant Renderer as renderWaveforms()
participant State as Timeline State (width, secsPerPixel, volumes)
Note over Canvas, State: On mount/update or state change
State->>Canvas: trigger createEffect read of deps
Canvas->>RAF: cancel existing RAF (if any)
Canvas->>RAF: requestAnimationFrame(renderWaveforms)
RAF->>Renderer: invoke renderWaveforms
Renderer->>Canvas: compute samplesPerSecond / samplesPerPixel / sampleStep
Renderer->>Canvas: draw bezier segments (clip to start/end)
Renderer->>RAF: (if needed) requestAnimationFrame(next render)
sequenceDiagram
participant Mouse as User Mouse
participant Throttle as Throttled setPreviewTime
participant Timeline as Timeline State (previewTime)
Mouse->>Throttle: mousemove events (high frequency)
Throttle-->>Mouse: rate-limited calls
Throttle->>Timeline: setPreviewTime(time) [guard: not playing, within bounds]
Timeline->>Timeline: update previewTime
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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
🧹 Nitpick comments (2)
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (2)
96-122: Review the prevX check and consider prevY for completeness.The rendering loop correctly implements dynamic sampling with bounds checking. However, the condition
if (prevX !== x)only checks x-coordinate changes:if (prevX !== x) { ctx.bezierCurveTo(cpX1, prevY, cpX2, y, x, y); prevX = x; prevY = y; }While this optimization prevents drawing when x hasn't changed, it may skip drawing vertical amplitude changes at the same x-coordinate (which can happen with the dynamic
sampleStep). Consider whether you should also checkprevY !== yor remove the condition entirely if vertical changes are significant.If vertical changes should always be rendered, apply this diff:
-if (prevX !== x) { +if (prevX !== x || prevY !== y) { const cpX1 = prevX + step / 2; const cpX2 = x - step / 2; ctx.bezierCurveTo(cpX1, prevY, cpX2, y, x, y); prevX = x; prevY = y; }
164-172: Consider using createComputed or moving logic to renderWaveforms.The
createEffectexplicitly tracks dependencies withvoidstatements, which works but is somewhat unconventional. In Solid.js, effects automatically track accessed signals. The current pattern forces tracking and cancels/schedules RAF on every dependency change.Based on learnings, minimize
useEffect(orcreateEffectin Solid) and prefer computing during render or in event handlers. Consider these alternatives:
- Move RAF scheduling to
renderWaveforms(preferred): Let the function access reactive values directly, and Solid will track them automatically.- Use
createComputedif you need fine-grained tracking: This runs synchronously and is more appropriate for derived state.Option 1 (preferred): Remove explicit void tracking and simplify
createEffect(() => { - // track reactive deps - void width(); - void secsPerPixel(); - void project.audio.micVolumeDb; - void project.audio.systemVolumeDb; if (rafId !== null) cancelAnimationFrame(rafId); rafId = requestAnimationFrame(renderWaveforms); });Solid will automatically track dependencies accessed in
renderWaveforms.Option 2: Use createComputed for synchronous updates
-createEffect(() => { +createComputed(() => { // track reactive deps void width(); void secsPerPixel(); void project.audio.micVolumeDb; void project.audio.systemVolumeDb; if (rafId !== null) cancelAnimationFrame(rafId); rafId = requestAnimationFrame(renderWaveforms); });Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx(4 hunks)apps/desktop/src/routes/editor/Timeline/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/Timeline/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/Timeline/index.tsx
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/src/**/*.{ts,tsx}: Desktop icons are auto-imported (unplugin-icons); do not import icons manually
Desktop IPC: Call generated tauri_specta commands/events; listen to generated events and use typed interfaces
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/Timeline/index.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-14T10:15:44.019Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T10:15:44.019Z
Learning: Applies to apps/web/**/*.{tsx} : Minimize useEffect; compute during render, put logic in event handlers, and clean up subscriptions/timers
Applied to files:
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
apps/desktop/src/routes/editor/Timeline/index.tsx (3)
3-3: LGTM!The throttle import is appropriate for the performance optimization objective.
42-44: LGTM! Throttle interval aligns with 60fps.The 16ms throttle interval is appropriate for smooth UI updates at approximately 60 frames per second.
202-208: LGTM! Guards and throttling improve performance.The guards for
playingstate and bounds availability prevent unnecessary updates. The throttled call reduces state churn during mouse movement.apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (4)
56-56: LGTM! Proper RAF ID tracking.The
rafIdvariable correctly tracks the animation frame request for cleanup.
93-94: LGTM! Previous point tracking for optimization.Initializing
prevXandprevYto the starting point(0, h)correctly sets up the Bézier curve drawing optimization.
124-127: LGTM! Relative positioning is consistent.The
lineTocalculation now uses relative positioning(endTime + 0.3 - startTime) / secsPerPixel(), which is consistent with the updated coordinate system in the rendering loop.
174-184: LGTM! Proper RAF cleanup and initialization.The
onCleanuphook and canvas ref handler correctly manage the animation frame lifecycle:
onCleanupcancels any pending RAF on unmount- Canvas ref cancels existing RAF before scheduling a new one on mount
This prevents memory leaks and ensures the waveform renders immediately when the canvas becomes available.
| const samplesPerSecond = 10; | ||
|
|
||
| const startTime = props.segment.start; | ||
| const endTime = props.segment.end; | ||
|
|
||
| const pixelsPerSecond = 1 / secsPerPixel(); | ||
| const samplesPerPixel = samplesPerSecond / pixelsPerSecond; | ||
|
|
||
| let sampleStep = 0.1; | ||
| if (samplesPerPixel < 0.5) { | ||
| sampleStep = Math.max(0.1, Math.ceil(1 / samplesPerPixel) * 0.1); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the sampleStep calculation logic.
The dynamic sampleStep calculation aims to reduce oversampling when zoomed out, but the logic may need verification:
When samplesPerPixel < 0.5 (i.e., more than 2 pixels per sample), the code sets:
sampleStep = Math.max(0.1, Math.ceil(1 / samplesPerPixel) * 0.1)
This increases the step, but the formula Math.ceil(1 / samplesPerPixel) * 0.1 may not align with the actual sampling needs. For example:
- If
samplesPerPixel = 0.25(4 pixels per sample),sampleStep = Math.ceil(4) * 0.1 = 0.4seconds - At 10 samples/sec, 0.4 seconds = 4 samples, which seems reasonable
However, when samplesPerPixel >= 0.5, sampleStep remains at the default 0.1 (1 sample), even if there are multiple samples per pixel. Consider whether additional downsampling is needed when samplesPerPixel > 1.
Run this script to check waveform sampling behavior at different zoom levels:
🏁 Script executed:
#!/bin/bash
# Description: Check for potential oversampling issues in waveform rendering
# Search for any related comments or documentation about sampling rates
rg -nP -C3 'samplesPerSecond|samplesPerPixel|sampleStep' --type=tsLength of output: 2455
Let me gather more context to understand the full rendering logic:
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Get broader context around the waveform drawing function
rg -nP -B10 -A20 'const step = 0.05 / secsPerPixel' apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx --type=tsLength of output: 780
Verify inverted sampling logic in sampleStep calculation.
The original review comment correctly identifies a logic issue. The sampleStep calculation is inverted:
Problem: When samplesPerPixel < 0.5 (zoomed in, many pixels per sample), the code calculates:
sampleStep = Math.ceil(1 / samplesPerPixel) * 0.1
This formula grows as samplesPerPixel shrinks. For example:
- Zoomed in with
samplesPerPixel = 0.1:sampleStep = 1.0second (skips 10 samples) - Zoomed in more with
samplesPerPixel = 0.01:sampleStep = 10.0seconds (skips 100 samples)
Expected behavior: Zoomed-in views should render more sample points (smaller step), not fewer.
Current behavior: The code does the opposite—as zoom increases, it skips more samples, creating sparse/missing points in detailed views.
Consider inverting the logic: when samplesPerPixel < 0.5, downsampling is unnecessary; when samplesPerPixel > 1 (multiple samples per pixel), increase the step instead.
🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx around lines 73-84, the
sampleStep logic is inverted: currently it increases step as samplesPerPixel
decreases, causing sparser sampling when zoomed in. Change the condition so that
when samplesPerPixel > 1 (multiple samples per pixel, zoomed out) you increase
sampleStep to downsample (e.g., sampleStep = Math.max(0.1,
Math.ceil(samplesPerPixel) * 0.1)); otherwise keep sampleStep at the minimum
(0.1) so zoomed-in views render more points. Ensure you use samplesPerPixel (not
1 / samplesPerPixel) in the ceil calculation and preserve the minimum step of
0.1.
|
might still need some work Screen.Recording.2025-10-30.at.6.09.14.pm.mov |
Improves waveform rendering in ClipTrack by adjusting sample stepping and using requestAnimationFrame for smoother updates. Timeline preview time updates are now throttled to reduce unnecessary state changes and improve performance during mouse movement
Summary by CodeRabbit
Performance Improvements
Enhancements