Speed up crop preview and reduce timeline overhead

[richiemcilroy]

• Capture and show an immediate crop preview from the current canvas frame when opening the crop dialog
• Keep the accurate crop frame fetch as a fallback path, with screenshot and load-failure handling preserved
• Remove the old crop video preloader flow in favor of the new preview strategy
• Reduce repeated timeline computation by memoizing selection lookups, row counts, segment ordering, and aggregate clip duration
• Limit desktop devtools to debug builds and add crop-frame profiling logs for local performance investigation

Greptile Summary

This PR has two independent improvements: a faster crop-dialog preview strategy and timeline computation memoization.

Crop preview UX — cropVideoPreloader.ts is removed entirely. Instead, cropDialogHandler in Player.tsx synchronously captures the current canvas frame (canvasControls.captureFrame()) and passes the resulting blob URL as previewUrl in the crop dialog state. The dialog component in Editor.tsx displays this canvas snapshot immediately, then schedules an accurate backend frame fetch (getDisplayFrameForCropping) via requestIdleCallback (500 ms timeout) or a 16 ms setTimeout fallback. If the preview image fails to load, onError triggers the accurate-frame fetch as a fallback. URL lifecycle is well-managed: the initial preview URL is revoked by setPreviewBlob when the accurate frame arrives, and by onCleanup when the dialog is dismissed.

Timeline memoization — trackOptions, textTrackRows, maskTrackRows, visibleTrackCount, and trackHeight in Timeline/index.tsx are promoted to createMemo. In every track component, per-segment isSelected memos now check against a pre-built Set instead of calling Array.includes. MaskTrack and TextTrack add laneSegmentPositionByIndex (Map) and sortedLaneSegments memos to avoid repeated O(n log n) sorts in findPlacement. ClipTrack replaces its O(n) reduce in drag handlers with a memoized totalClipTimelineDuration. timelineTracks.ts replaces spread-into-Math.max with explicit loops.

Rust / Tauri — get_display_frame_for_cropping gains Instant-based phase profiling emitted at DEBUG level (no production overhead). The devtools Tauri feature flag is removed from Cargo.toml and the window builder gates it on cfg!(debug_assertions).

Findings:

• The logCropProfile helper is copy-pasted identically into both Player.tsx and Editor.tsx; a shared module would prevent drift.
• When frameSource is "accurate-frame" and the blob image fires onError, accurateFrameRequested is already true so requestAccurateFrame is a no-op. frameBlobUrl remains non-null, so screenshotSrc is never tried and the user sees a broken image.
• previewUrl is created before setDialog is called; a synchronous exception between the two (very unlikely in practice) would leave an unrevoked object URL.

Confidence Score: 5/5

• Safe to merge — all findings are P2 (style/edge-case) with no present runtime defects on the critical path.
• All three comments are P2: one is pure code duplication, one is a theoretical edge case where a successfully-decoded accurate-frame blob somehow fails to render (practically impossible with valid backend PNG output), and one is a near-impossible race condition. No P0 or P1 issues were found. The memoization changes are correct, URL lifecycle is well-managed, and the <Index> vs <For> index access patterns are used correctly throughout.
• apps/desktop/src/routes/editor/Editor.tsx (accurate-frame error fallback gap) and apps/desktop/src/routes/editor/Player.tsx (logCropProfile duplication)

Important Files Changed

Filename	Overview
apps/desktop/src/routes/editor/Editor.tsx	New crop-preview flow: canvas snapshot shown immediately, accurate backend frame fetched via idle callback with screenshot fallback. Missing fallback when the accurate-frame blob itself fails to render.
apps/desktop/src/routes/editor/Player.tsx	Captures current canvas frame as immediate crop preview via canvasControls.captureFrame(); removes preloadCropVideoFull hooks; logCropProfile duplicated from Editor.tsx.
apps/desktop/src/routes/editor/cropVideoPreloader.ts	File deleted — preload strategy replaced by direct canvas frame capture in Player.tsx.
apps/desktop/src/routes/editor/Timeline/index.tsx	Converts trackOptions, textTrackRows, maskTrackRows, visibleTrackCount, and trackHeight from plain reactive functions to createMemo, avoiding redundant recomputation on every render.
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx	Adds selectedClipIndices memo (Set-based) and totalClipTimelineDuration memo, replacing inline selection.indices.includes and per-render reduce calls inside drag handlers.
apps/desktop/src/routes/editor/timelineTracks.ts	Rewrites getUsedTrackCount and getTrackRowsWithCount to use imperative loops instead of spread-into-Math.max and Array.from, avoiding O(n) spread allocation on each call.
apps/desktop/src-tauri/src/lib.rs	Adds Instant-based profiling spans (lookup / decode / convert / encode) to get_display_frame_for_cropping, emitted via debug! — no production-log noise.
apps/desktop/src-tauri/src/windows.rs	Gates Tauri devtools on cfg!(debug_assertions) so they are not exposed in release builds.

Sequence Diagram

sequenceDiagram
    participant User
    participant Player as Player.tsx
    participant Canvas as canvasControls
    participant Editor as Editor.tsx (CropDialog)
    participant Backend as Tauri Backend

    User->>Player: Click "Crop"
    Player->>Canvas: hasRenderedFrame()?
    Canvas-->>Player: true
    Player->>Canvas: captureFrame()
    Canvas-->>Player: Blob (current frame)
    Player->>Player: URL.createObjectURL(blob) → previewUrl
    Player->>Editor: setDialog({ type:"crop", previewUrl })
    Editor->>Editor: frameBlobUrl = previewUrl (immediate)
    Editor->>Editor: frameSource = "captured-preview"
    Editor-->>User: Show crop dialog with canvas preview

    Editor->>Editor: onMount → scheduleAccurateFrame(idleTimeoutMs=500)
    Note over Editor: requestIdleCallback / setTimeout(16ms)

    Editor->>Backend: getDisplayFrameForCropping(FPS)
    Backend-->>Editor: PNG bytes
    Editor->>Editor: setPreviewBlob → revokeObjectURL(previewUrl), set accurateFrameUrl
    Editor-->>User: Update with accurate frame

    alt Image load error (any source)
        Editor->>Editor: onError → requestAccurateFrame (if not already done)
        Editor->>Backend: getDisplayFrameForCropping(FPS)
        Backend-->>Editor: PNG bytes or error
    end

    User->>Editor: Close dialog
    Editor->>Editor: onCleanup → cancelled=true, revokeObjectURL(frameBlobUrl)

Loading

Comments Outside Diff (2)

1. apps/desktop/src/routes/editor/Editor.tsx, line 1125-1163 (link)

   No fallback when the accurate-frame blob itself fails to render

   When frameSource is "accurate-frame" and the image fires onError (e.g. the backend returned bytes that the browser cannot decode as PNG), requestAccurateFrame is a no-op because accurateFrameRequested is already true. At that point frameBlobUrl() is non-null, so the expression frameBlobUrl() ?? screenshotSrc never reaches the screenshot fallback, and the user is left with a broken image.

   To handle this gracefully you could reset frameBlobUrl to null on an accurate-frame load failure so the screenshot path is tried:

   onError={() => {
     const failedSource = frameSource();
     logCropProfile("preview-image-failed", {
       elapsedMs: Number((performance.now() - cropOpenedAt).toFixed(2)),
       source: failedSource,
     });
     if (failedSource === "accurate-frame") {
       // Blob rendered correctly by the backend but browser can't display it;
       // fall back to the static screenshot.
       setFrameBlobUrl(null);
       return;
     }
     requestAccurateFrame(
       failedSource === "screenshot"
         ? "screenshot-load-failed"
         : "preview-load-failed",
     );
   }}

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/routes/editor/Editor.tsx
   Line: 1125-1163
   
   Comment:
   **No fallback when the accurate-frame blob itself fails to render**
   
   When `frameSource` is `"accurate-frame"` and the image fires `onError` (e.g. the backend returned bytes that the browser cannot decode as PNG), `requestAccurateFrame` is a no-op because `accurateFrameRequested` is already `true`. At that point `frameBlobUrl()` is non-null, so the expression `frameBlobUrl() ?? screenshotSrc` never reaches the screenshot fallback, and the user is left with a broken image.
   
   To handle this gracefully you could reset `frameBlobUrl` to `null` on an accurate-frame load failure so the screenshot path is tried:
   
   ```ts
   onError={() => {
     const failedSource = frameSource();
     logCropProfile("preview-image-failed", {
       elapsedMs: Number((performance.now() - cropOpenedAt).toFixed(2)),
       source: failedSource,
     });
     if (failedSource === "accurate-frame") {
       // Blob rendered correctly by the backend but browser can't display it;
       // fall back to the static screenshot.
       setFrameBlobUrl(null);
       return;
     }
     requestAccurateFrame(
       failedSource === "screenshot"
         ? "screenshot-load-failed"
         : "preview-load-failed",
     );
   }}
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

2. apps/desktop/src/routes/editor/Player.tsx, line 416-425 (link)

   previewUrl blob not revoked on capture failure path

   previewUrl is created with URL.createObjectURL, then stored verbatim in the dialog state as previewUrl. The onCleanup inside the Show block in Editor.tsx revokes the URL when the dialog unmounts — that path is correct. However, if captureFrame() succeeds and previewUrl is set, but setDialog is then not reached (e.g. an uncaught synchronous error between lines 175 and 185 — admittedly very unlikely), the URL would never be revoked.

   A small defensive measure is to unconditionally revoke previewUrl in the crop dialog's onCleanup path, or alternatively create the blob URL inside the dialog component where it's more naturally owned. No immediate action required given the low probability, but worth keeping in mind if cropDialogHandler grows in complexity.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/routes/editor/Player.tsx
   Line: 416-425
   
   Comment:
   **`previewUrl` blob not revoked on capture failure path**
   
   `previewUrl` is created with `URL.createObjectURL`, then stored verbatim in the dialog state as `previewUrl`. The `onCleanup` inside the `Show` block in `Editor.tsx` revokes the URL when the dialog unmounts — that path is correct. However, if `captureFrame()` succeeds and `previewUrl` is set, but `setDialog` is then not reached (e.g. an uncaught synchronous error between lines 175 and 185 — admittedly very unlikely), the URL would never be revoked.
   
   A small defensive measure is to unconditionally revoke `previewUrl` in the crop dialog's `onCleanup` path, or alternatively create the blob URL inside the dialog component where it's more naturally owned. No immediate action required given the low probability, but worth keeping in mind if `cropDialogHandler` grows in complexity.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Player.tsx
Line: 35-41

Comment:
**`logCropProfile` duplicated across two files**

`logCropProfile` is defined identically in both `Player.tsx` (lines 35–41) and `Editor.tsx`. If the signature or behaviour ever needs to change (e.g. adding a timestamp, swapping `console.info` for a structured logger, or changing the env guard), the two copies will drift. Consider extracting it to a shared utility module (e.g. `cropProfile.ts`) and importing it in both files.

```ts
// apps/desktop/src/routes/editor/cropProfile.ts
export function logCropProfile(
  stage: string,
  data: Record<string, number | string | boolean | null> = {},
) {
  if (!import.meta.env.DEV) return;
  console.info("[crop-profile]", stage, data);
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Editor.tsx
Line: 1125-1163

Comment:
**No fallback when the accurate-frame blob itself fails to render**

When `frameSource` is `"accurate-frame"` and the image fires `onError` (e.g. the backend returned bytes that the browser cannot decode as PNG), `requestAccurateFrame` is a no-op because `accurateFrameRequested` is already `true`. At that point `frameBlobUrl()` is non-null, so the expression `frameBlobUrl() ?? screenshotSrc` never reaches the screenshot fallback, and the user is left with a broken image.

To handle this gracefully you could reset `frameBlobUrl` to `null` on an accurate-frame load failure so the screenshot path is tried:

```ts
onError={() => {
  const failedSource = frameSource();
  logCropProfile("preview-image-failed", {
    elapsedMs: Number((performance.now() - cropOpenedAt).toFixed(2)),
    source: failedSource,
  });
  if (failedSource === "accurate-frame") {
    // Blob rendered correctly by the backend but browser can't display it;
    // fall back to the static screenshot.
    setFrameBlobUrl(null);
    return;
  }
  requestAccurateFrame(
    failedSource === "screenshot"
      ? "screenshot-load-failed"
      : "preview-load-failed",
  );
}}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Player.tsx
Line: 416-425

Comment:
**`previewUrl` blob not revoked on capture failure path**

`previewUrl` is created with `URL.createObjectURL`, then stored verbatim in the dialog state as `previewUrl`. The `onCleanup` inside the `Show` block in `Editor.tsx` revokes the URL when the dialog unmounts — that path is correct. However, if `captureFrame()` succeeds and `previewUrl` is set, but `setDialog` is then not reached (e.g. an uncaught synchronous error between lines 175 and 185 — admittedly very unlikely), the URL would never be revoked.

A small defensive measure is to unconditionally revoke `previewUrl` in the crop dialog's `onCleanup` path, or alternatively create the blob URL inside the dialog component where it's more naturally owned. No immediate action required given the low probability, but worth keeping in mind if `cropDialogHandler` grows in complexity.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Remove deferred accurate crop preview lo..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[paragon-review]

Paragon Summary

This pull request review analyzed 16 files and found no issues. The review examined code changes, potential bugs, security vulnerabilities, performance issues, and code quality concerns using automated analysis tools.

Paragon did not detect any problems in the current diff. Proceed with merge after your normal checks.

This PR speeds up the editor by showing an immediate crop preview from the current canvas frame, while keeping the accurate crop-frame fallback and removing the old preloader flow. It also cuts timeline overhead through memoization and adds debug-only devtools plus profiling logs for local performance investigation.

Key changes:

• Immediate crop preview now shows the current canvas frame on crop-dialog open, with screenshot/load-failure fallback preserved.
• Removed the old crop video preloader flow in favor of a preview-first crop path.
• Memoized timeline selection lookups, row counts, segment ordering, and total clip duration to cut repeated work.
• Scoped desktop devtools to debug builds and added crop-frame profiling logs for local perf debugging.

Confidence score: 5/5

• This PR has low risk with no critical or high-priority issues identified
• Score reflects clean code review with only minor suggestions or no issues found
• Code quality checks passed - safe to proceed with merge

16 files reviewed, 0 comments

Dashboard

[tembo]

If stopPlayback() throws here, playing stays true (and the crop modal is already open). Making this try/catch/finally keeps state consistent.

Suggested change

	await commands.stopPlayback();
	try {
	await commands.stopPlayback();
	logCropProfile("playback-stopped", {
	elapsedMs: Number((performance.now() - startedAt).toFixed(2)),
	});
	} catch (error) {
	console.warn("Failed to stop playback:", error);
	logCropProfile("playback-stop-failed", {
	elapsedMs: Number((performance.now() - startedAt).toFixed(2)),
	});
	} finally {
	setEditorState("playing", false);
	}

[tembo]

Right now the accurate frame fetch gets queued on every crop-open (even when a fresh previewUrl is available). If the intent is mostly fallback/upgrade, a small delay when we already have a preview can avoid unnecessary work for quick open/close.

Suggested change

	scheduleAccurateFrame("immediate", {
	scheduleAccurateFrame(initialPreviewUrl ? "preview-available" : "immediate", {
	delayMs: initialPreviewUrl ? 250 : undefined,
	idleTimeoutMs: 500,
	fallbackDelayMs: 16,
	});

[greptile-apps]

logCropProfile duplicated across two files

logCropProfile is defined identically in both Player.tsx (lines 35–41) and Editor.tsx. If the signature or behaviour ever needs to change (e.g. adding a timestamp, swapping console.info for a structured logger, or changing the env guard), the two copies will drift. Consider extracting it to a shared utility module (e.g. cropProfile.ts) and importing it in both files.

// apps/desktop/src/routes/editor/cropProfile.ts
export function logCropProfile(
  stage: string,
  data: Record<string, number | string | boolean | null> = {},
) {
  if (!import.meta.env.DEV) return;
  console.info("[crop-profile]", stage, data);
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Player.tsx
Line: 35-41

Comment:
**`logCropProfile` duplicated across two files**

`logCropProfile` is defined identically in both `Player.tsx` (lines 35–41) and `Editor.tsx`. If the signature or behaviour ever needs to change (e.g. adding a timestamp, swapping `console.info` for a structured logger, or changing the env guard), the two copies will drift. Consider extracting it to a shared utility module (e.g. `cropProfile.ts`) and importing it in both files.

```ts
// apps/desktop/src/routes/editor/cropProfile.ts
export function logCropProfile(
  stage: string,
  data: Record<string, number | string | boolean | null> = {},
) {
  if (!import.meta.env.DEV) return;
  console.info("[crop-profile]", stage, data);
}
```

How can I resolve this? If you propose a fix, please make it concise.