Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
65 changes: 46 additions & 19 deletions apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ function WaveformCanvas(props: {
const { project } = useEditorContext();

let canvas: HTMLCanvasElement | undefined;
let rafId: number | null = null;
const { width } = useSegmentContext();
const { secsPerPixel } = useTimelineContext();

Expand All @@ -65,11 +66,22 @@ function WaveformCanvas(props: {
) => {
const maxAmplitude = h;

// yellow please
ctx.fillStyle = color;
ctx.beginPath();

const step = 0.05 / secsPerPixel();
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);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 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.4 seconds
  • 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=ts

Length 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=ts

Length 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.0 second (skips 10 samples)
  • Zoomed in more with samplesPerPixel = 0.01: sampleStep = 10.0 seconds (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.


ctx.moveTo(0, h);

Expand All @@ -78,35 +90,39 @@ function WaveformCanvas(props: {
return 1.0 - Math.max(ww + gain, -60) / -60;
};

let prevX = 0;
let prevY = h;

for (
let segmentTime = props.segment.start;
segmentTime <= props.segment.end + 0.1;
segmentTime += 0.1
let segmentTime = startTime;
segmentTime <= endTime + 0.1;
segmentTime += sampleStep
) {
const index = Math.floor(segmentTime * 10);
const xTime = index / 10;
const index = Math.floor(segmentTime * samplesPerSecond);
if (index < 0 || index >= waveform.length) continue;

const xTime = index / samplesPerSecond;

const currentDb =
typeof waveform[index] === "number" ? waveform[index] : -60;
const amplitude = norm(currentDb) * maxAmplitude;

const x = (xTime - props.segment.start) / secsPerPixel();
const x = (xTime - startTime) / secsPerPixel();
const y = h - amplitude;

const prevX = (xTime - 0.1 - props.segment.start) / secsPerPixel();
const prevDb =
typeof waveform[index - 1] === "number" ? waveform[index - 1] : -60;
const prevAmplitude = norm(prevDb) * maxAmplitude;
const prevY = h - prevAmplitude;

const cpX1 = prevX + step / 2;
const cpX2 = x - step / 2;
if (prevX !== x) {
const cpX1 = prevX + step / 2;
const cpX2 = x - step / 2;

ctx.bezierCurveTo(cpX1, prevY, cpX2, y, x, y);
ctx.bezierCurveTo(cpX1, prevY, cpX2, y, x, y);

prevX = x;
prevY = y;
}
}

ctx.lineTo(
(props.segment.end + 0.3 - props.segment.start) / secsPerPixel(),
(endTime + 0.3 - startTime) / secsPerPixel(),
h,
);

Expand Down Expand Up @@ -146,14 +162,25 @@ function WaveformCanvas(props: {
}

createEffect(() => {
renderWaveforms();
// track reactive deps
void width();
void secsPerPixel();
void project.audio.micVolumeDb;
void project.audio.systemVolumeDb;
if (rafId !== null) cancelAnimationFrame(rafId);
rafId = requestAnimationFrame(renderWaveforms);
});

onCleanup(() => {
if (rafId !== null) cancelAnimationFrame(rafId);
});

return (
<canvas
ref={(el) => {
canvas = el;
renderWaveforms();
if (rafId !== null) cancelAnimationFrame(rafId);
rafId = requestAnimationFrame(renderWaveforms);
}}
class="absolute inset-0 w-full h-full pointer-events-none"
height={52}
Expand Down
20 changes: 12 additions & 8 deletions apps/desktop/src/routes/editor/Timeline/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { createElementBounds } from "@solid-primitives/bounds";
import { createEventListener } from "@solid-primitives/event-listener";
import { throttle } from "@solid-primitives/scheduled";
import { platform } from "@tauri-apps/plugin-os";
import { cx } from "cva";
import { batch, createRoot, createSignal, For, onMount, Show } from "solid-js";
Expand Down Expand Up @@ -38,6 +39,10 @@ export function Timeline() {

const secsPerPixel = () => transform().zoom / (timelineBounds.width ?? 1);

const setPreviewTimeThrottled = throttle((time: number) => {
setEditorState("previewTime", time);
}, 16);

onMount(() => {
if (!project.timeline) {
const resume = projectHistory.pause();
Expand Down Expand Up @@ -194,14 +199,13 @@ export function Timeline() {
});
});
}}
onMouseMove={(e) => {
const { left } = timelineBounds;
if (editorState.playing) return;
setEditorState(
"previewTime",
transform().position + secsPerPixel() * (e.clientX - left!),
);
}}
onMouseMove={(e) => {
const { left } = timelineBounds;
if (editorState.playing || left == null) return;
setPreviewTimeThrottled(
transform().position + secsPerPixel() * (e.clientX - left),
);
}}
onMouseEnter={() => setEditorState("timeline", "hoveredTrack", null)}
onMouseLeave={() => {
setEditorState("previewTime", null);
Expand Down
Loading