From ec05d58fdab92582c7922e7e5333117eacf8518a Mon Sep 17 00:00:00 2001 From: Akanksha Trehun Date: Wed, 21 Oct 2026 10:48:56 +0530 Subject: [PATCH 01/11] feat: redesign editor UI, remove animation overhead, add perf instrumentation UI - One design system on the existing tokens (--bg/--surface/--border/--text/ --muted unchanged) plus surface-2, border-strong, accent-text, on-accent, focus, status, radius and motion tokens; film-red accent used sparingly - System font stack (no font requests); defines the font-heading/display and animate-fade-in utilities that were used everywhere but never existed - Editor layout: preview first with a toolbar below it, edit tools as accordions with value summaries, sticky Output panel with pinned Export - High-contrast theme is now reachable (toggle + prefers-contrast) - Clearer upload, export, error/retry and success states; honest format and size copy; shared .btn/.choice/.field component classes Performance - Remove the universal transition and `transition: all`; reduced-motion support - Stop re-rendering the editor on every timeupdate; debounce URL sync - Thumbnails: capped, abortable, seek timeouts, batched, URLs revoked - Lazy-load Datadog; drop looping Lottie animations; fix privacy-banner CLS - Worker: dedupe progress, always pass -t (#1289 only patched the unused copy), cap encoder threads under cross-origin isolation (core-mt deadlock) - Restart the worker after a failed export; settle loads on terminate Telemetry: privacy-safe export/ffmpeg/thumbnail events and duration vitals, action-name masking, query strings stripped. Tests: 61 new (175 total). Lighthouse mobile (local build): 86 -> 100, LCP 3.40s -> 1.12s, CLS 0.096 -> 0. See docs/PERFORMANCE.md. Co-Authored-By: Claude Opus 5.5 --- .gitignore | 5 + .storybook/preview.css | 8 +- docs/PERFORMANCE.md | 284 +++++ scripts/perf/bench.js | 198 ++++ scripts/perf/browser.js | 18 + scripts/perf/lighthouse.sh | 25 + scripts/perf/make-fixture.js | 45 + scripts/perf/motion.js | 27 + scripts/perf/serve.mjs | 41 + src/app/contact/page.tsx | 14 +- src/app/globals.css | 466 ++++++-- src/app/layout.tsx | 55 +- src/app/not-found.tsx | 6 +- src/app/page.tsx | 18 +- src/app/privacy/page.tsx | 6 +- src/components/AdjustmentsControl.tsx | 74 ++ src/components/AudioSpeedControl.tsx | 76 +- src/components/ColorPicker.tsx | 14 +- src/components/ComparisonPreview.tsx | 4 +- src/components/DatadogInit.tsx | 86 +- src/components/DownloadResult.tsx | 144 ++- src/components/DraggableTextOverlays.tsx | 2 +- src/components/ExportOverlay.tsx | 154 +-- src/components/ExportSettings.tsx | 52 +- src/components/FileUpload.tsx | 279 ++--- src/components/FontSelector.tsx | 4 +- src/components/Footer.tsx | 32 +- src/components/FormatSelector.tsx | 24 +- src/components/FramingControl.tsx | 19 +- src/components/ImageOverlay.tsx | 24 +- src/components/NativeShareButton.tsx | 6 +- src/components/OnboardingTour.tsx | 14 +- src/components/PresetSelector.tsx | 71 +- src/components/PrivacyBanner.tsx | 30 +- src/components/RotateControl.tsx | 14 +- src/components/ScrollToTop.tsx | 10 +- src/components/TextControls.tsx | 2 +- src/components/ThemeProvider.tsx | 70 +- src/components/ThemeToggle.tsx | 73 +- src/components/ThumbnailStrip.tsx | 596 ++++------ src/components/TipCarousel.tsx | 80 +- src/components/TrimControl.tsx | 34 +- src/components/VideoEditor.tsx | 1005 +++++++---------- src/components/VideoPreview.tsx | 215 ++-- src/components/__tests__/performance.test.tsx | 255 +++++ src/components/components.tsx | 10 +- src/components/ui/BaseButton.tsx | 24 +- src/hooks/tests/useVideoEditor.export.test.ts | 194 ++++ src/hooks/tests/useVideoEditor.test.ts | 3 + src/hooks/useKeyboardShortcuts.ts | 18 +- src/hooks/useVideoEditor.ts | 172 ++- src/lib/exportEstimate.test.ts | 8 +- src/lib/ffmpeg.ts | 73 +- src/lib/ffmpeg.worker.ts | 51 +- src/lib/telemetry.ts | 134 ++- src/lib/tests/ffmpegClient.test.ts | 165 +++ src/lib/tests/telemetry.test.ts | 91 ++ src/lib/tests/thumbnails.test.ts | 156 +++ src/lib/thumbnails.ts | 195 ++++ tailwind.config.ts | 49 +- 60 files changed, 3999 insertions(+), 2023 deletions(-) create mode 100644 docs/PERFORMANCE.md create mode 100644 scripts/perf/bench.js create mode 100644 scripts/perf/browser.js create mode 100755 scripts/perf/lighthouse.sh create mode 100644 scripts/perf/make-fixture.js create mode 100644 scripts/perf/motion.js create mode 100644 scripts/perf/serve.mjs create mode 100644 src/components/AdjustmentsControl.tsx create mode 100644 src/components/__tests__/performance.test.tsx create mode 100644 src/hooks/tests/useVideoEditor.export.test.ts create mode 100644 src/lib/tests/ffmpegClient.test.ts create mode 100644 src/lib/tests/telemetry.test.ts create mode 100644 src/lib/tests/thumbnails.test.ts create mode 100644 src/lib/thumbnails.ts diff --git a/.gitignore b/.gitignore index 1d982f0b4..fa2d8350c 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,8 @@ chromatic.log # Local Netlify folder .netlify + +# Local performance measurement output (docs/PERFORMANCE.md) +scripts/perf/lh/ +scripts/perf/fixtures/ +bench-*.json diff --git a/.storybook/preview.css b/.storybook/preview.css index a6942d212..cf593925f 100644 --- a/.storybook/preview.css +++ b/.storybook/preview.css @@ -1,10 +1,10 @@ /* * Snapshot stability for Chromatic. * - * src/app/globals.css applies a 200ms transition to *every* element - * (`*, *::before, *::after`) and several components animate on mount. A visual - * regression service screenshots at an arbitrary moment, so those in-flight - * transitions produce diffs that have nothing to do with a real change. + * Components use short, targeted transitions (globals.css no longer transitions + * every element) and a few animate on mount. A visual regression service + * screenshots at an arbitrary moment, so any in-flight transition produces a + * diff that has nothing to do with a real change. * * Freezing them here makes every story render at its settled end state. This * only affects the Storybook preview iframe — the app itself still animates. diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md new file mode 100644 index 000000000..a093740a7 --- /dev/null +++ b/docs/PERFORMANCE.md @@ -0,0 +1,284 @@ +# Performance + +This document records Reframe's performance targets, how to measure them, and +the results that have actually been collected. Anything not measured is +labelled **goal**. Lab numbers (Lighthouse, scripted Chrome) are not field +data and don't prove real-user performance. + +## Targets + +| Metric | Target | Status | +|---|---|---| +| Lighthouse mobile performance | ≥ 90 | **Met in lab**: 100 (local production build) | +| Largest Contentful Paint | ≤ 2.5 s | **Met in lab**: 1.12 s (simulated mobile) | +| Interaction to Next Paint | ≤ 200 ms | **Goal**: no field data yet. Lab proxy: slowest input event during a 30-step slider drag was 32–40 ms | +| Cumulative Layout Shift | ≤ 0.1 | **Met in lab**: 0 (was 0.096) | +| Long tasks (> 50 ms) during normal editing | Minimise | **Met in lab**: 0 during 5 s playback and a 30-step slider drag | +| UI responsiveness during export | No prolonged blocking | **Met in lab**: 0 long tasks, worst frame gap 17–33 ms across all exports | +| Export regression tests | All pass | **Met**: `bun run test`, 175 tests | +| Resource cleanup | No accumulation over export/cancel cycles | **Met in lab**: worker count stable over 3 export→cancel cycles | + +Field targets (INP, LCP and CLS at p75) need Datadog RUM data with enough +traffic. They are goals until that data exists. + +## Test environment (for every number below) + +- Apple M3 Pro (11 cores, 18 GB RAM), macOS 26.3.1 +- Chrome 155.0.8059.85 (headless), Lighthouse 13.5.0 +- Lighthouse runs use the default mobile preset: simulated throttling, 4× CPU + slowdown, 150 ms RTT, ~1.6 Mbps. +- **Local production build** (`bun run build`, the static `out/` directory), + served by `scripts/perf/serve.mjs` with brotli. Unless stated otherwise it + sends the same COOP/COEP and security headers as `vercel.json`. +- Interaction benchmarks use a 1440×1000 viewport and a synthetic fixture: 6.02 s, + 640×360, H.264 + AAC, 571 KB, generated by `scripts/perf/make-fixture.js`. +- "Before" is `main` at `4af01e5`; "after" is this branch. + +## Results + +### First page load (Lighthouse mobile, median of 5) + +| | Perf | FCP | LCP | TBT | CLS | A11y | Transfer | +|---|---|---|---|---|---|---|---| +| Before (local build) | 86 | 2.38 s | 3.40 s | 47 ms | 0.096 | 95 | 315 KB | +| **After (local build)** | **100** | **0.90 s** | **1.12 s** | **19 ms** | **0** | **100** | **258 KB** | +| Live deployment, before (3 runs, 2026-10-21) | 91 | 1.79 s | 2.80 s | 36 ms | 0.096 | 92 | 280 KB | + +Where the gain came from (Lighthouse diagnostics, before → after): + +- **Render-blocking Google Fonts `@import` removed.** It loaded Inter, Roboto, + Poppins and Montserrat. No CSS used them, and Lighthouse attributed about + 930 ms of blocking to it. The UI now uses the platform font stack, with no + font requests at all. +- **CLS 0.096 → 0.** The privacy notice mounted hidden and then appeared after + hydration, pushing the upload area down. It now renders in the static HTML; + for visitors who dismissed it, an inline script hides it before first paint. +- **Less JavaScript before first paint.** The Datadog SDK (~53 KB) now loads + as a separate chunk at idle, and the looping Lottie upload animation + (`lottie-web`, ~63 KB) is replaced by a static icon. Lottie now loads only + for the one-shot success animation. + +### Motion audit (landing page; `scripts/perf/motion.js`) + +| | Elements with an active transition | With `prefers-reduced-motion: reduce` | Infinite animations | +|---|---|---|---| +| Before | 357 of 357 (every element) | 362 of 362 (reduced motion ignored) | 2 | +| **After** | **48** (interactive controls only) | **0** | **0** | + +### Editing interactions (same single-threaded engine in both builds) + +| Scenario | Metric | Before | After | +|---|---|---|---| +| 5 s of playback | Page script time | 40 ms | **5 ms** | +| | Style recalculations | 80 | **33** | +| | Long tasks | 0 | 0 | +| 30 brightness steps, 40 ms apart | Main-thread task time | 275 ms | **133–143 ms** | +| | Style recalculations | 103 | **38** | +| | Input events ≥ 16 ms | 58 | **24–26** | +| | Slowest input event | 32 ms | 32–40 ms | +| | `history.replaceState` calls | 62 | **2** | +| Upload → preview ready | | 236 ms | 223–317 ms | +| Upload → frame strip done (4 frames) | | 516 ms | 440–633 ms | + +Upload timings with this small fixture vary by about ±100 ms between runs. +Neither build shows a meaningful difference there. + +### Export (`scripts/perf/bench.js`) + +| Output | Before, single-thread | After, single-thread | After, multi-thread (COOP/COEP) | +|---|---|---|---| +| 1280×720 MP4 | 5.17 s | 5.19 s | **1.22 s** | +| 1920×1080 MP4 | 11.09 s | 11.43 s | **2.57 s** | +| 1080×1920 MP4 | 7.33 s | 7.36 s | **1.74 s** | +| Before, multi-thread | did not finish: stuck at 0% for 60 s+ (headed and headless) | | | + +All three columns had 0 long tasks during export and a worst frame gap of +17–33 ms. Encoding stays in the worker, so playback and ordinary controls +stay responsive. + +| Lifecycle scenario | Before | After | +|---|---|---| +| Cancel → editor idle | 8–198 ms | 9–24 ms | +| Workers alive across 3 export→cancel cycles, single-thread | 3, 1, 3 | 3, 3, 3 | +| Same, multi-thread | n/a (exports hang) | 35 → 35 → 35, stable (1 + inner worker + pthread pool) | +| Engine restart after cancel (from HTTP cache) | 2.87 s | 2.5–2.95 s | +| Engine download blocked, then Retry | "Error: Failed to fetch"; retry OK | Actionable message; retry OK (5.7 s total including download and 1080p export) | + +## Problems found and fixed + +1. **Global transitions.** `*, *::before, *::after` transitioned colour + properties on every node, and `button { transition: all }` applied + everywhere. Both are gone. Controls use explicit 120–220 ms transitions, + and theme changes crossfade only for 200 ms around an explicit toggle. +2. **Playback re-rendered the whole editor.** `useVideoEditor` mirrored + `timeupdate` into React state through an effect with no dependency array, + which re-subscribed on every render. The playhead is now read on demand, + and only the frame strip subscribes; it updates state only when the + active frame changes. +3. **The URL updated on every slider tick.** `history.replaceState` ran on + every recipe change; it is now debounced to 250 ms. Safari throws after + 100 calls in 30 s. The sync also wrote `textOverlays` (an array) into the + query string as `"[object Object]"`, which broke reloads. +4. **Thumbnail extraction.** + - Frame count is now capped at 48 for long videos. + - Every seek has a timeout; before, a seek that never fired `seeked` + hung forever. + - Work is abortable, and every object URL is either handed off or revoked. + - The decoder is released when extraction finishes. + - React updates arrive in batches of 6 frames. + - The first frame no longer renders black: extraction waits for + `loadeddata`, not `loadedmetadata`. + - Failures show a recoverable message with a retry. +5. **Progress was flooded.** FFmpeg reports progress per frame; both the + worker and the client now forward only whole-percent changes. The dialog + also opened at "100%", left over from engine load; it now starts at 0%. +6. **Duplicate metadata load per export.** The export now reuses the + duration the editor already has. +7. **A failed export could poison the next one.** After a non-cancel failure + the worker is restarted, so Retry starts from a clean engine and + filesystem. Settings are always kept. +8. **Terminating during engine load left the load promise pending forever.** + It now rejects. +9. **The `-t` output-duration fix (#1289) never reached the worker.** It only + changed the unused main-thread copy of `buildArguments`. The worker now + always passes `-t`, which prevents endless exports with looped music on + silent clips. +10. **The multi-threaded core deadlocked.** On "auto" thread count, + libx264/libvpx asked for more threads than the core-mt pthread pool holds. + Encoder threads are now capped at 4 when cross-origin isolated (see + "Cross-origin isolation" below). +11. **`ResizeObserver` for preview sizing.** It replaces a `window` resize + listener that missed layout-driven size changes. Sub-pixel changes no + longer re-render. +12. **A drop on the drop zone was handled twice**, once by the zone and once + by the document listener, so validation and metadata extraction ran twice. + +## Cross-origin isolation: production finding (needs a maintainer) + +The live deployment sends **none** of the headers in `vercel.json`. On +2026-10-21, `curl -sI https://reframe-lilac.vercel.app/` showed only Vercel's +default `strict-transport-security`: no COOP/COEP, `X-Frame-Options`, +`X-Content-Type-Options`, `Referrer-Policy` or `Permissions-Policy`. + +The likely cause is that `vercel.json` combines the legacy `routes` key with +`headers`, which Vercel documents as mutually exclusive. This has not been +verified on Vercel. Consequences: + +- Production runs the **single-threaded** core (exports are 4–4.5× slower + than multi-threaded in the table above). +- The intended security headers are not applied. +- Until this branch, **turning the headers on would have broken exports**: + the multi-threaded path deadlocked (fix 10). + +`vercel.json` is deliberately unchanged here. Suggested follow-up: remove the +`routes` block (`handle: filesystem` is already the default for static +output), deploy a preview, confirm the headers with `curl -sI`, and run an +export on the preview before promoting it. + +## Datadog RUM + +Configured in `src/components/DatadogInit.tsx`, with helpers in +`src/lib/telemetry.ts`. + +- The SDK loads lazily at idle. Calls made before it is ready are queued + (up to 50) and replayed. Every call is wrapped, so a telemetry failure can + never interrupt editing. +- Session replay stays disabled (`sessionReplaySampleRate: 0`). +- Privacy: + - `enablePrivacyForActionName: true`, because auto-named clicks could + otherwise carry the file name. + - `beforeSend` strips query strings, where recipe settings live, from view + URLs and referrers, and drops `blob:` resources. + - Custom events carry only buckets and enums, never file names, captions, + paths or exact values. +- `NEXT_PUBLIC_DATADOG_ENV` (default `production`) lets local and preview + runs be separated. + +| Event | Type | Properties | +|---|---|---| +| `ffmpeg_init` | duration vital + action | `threading` (`multi`/`single`), `durationMs` | +| `ffmpeg_load_success` / `ffmpeg_load_failure` | action | `durationMs`, `threading` / `reason` | +| `export_start` | action | `format`, `resolution` bucket, `input_size` bucket, `clip_length` bucket, `has_text`, `has_music`, `has_image_overlay` | +| `export` | duration vital + action | encoding time only (engine start is `ffmpeg_init`) + the above | +| `export_success` | action | the above + `output_size` bucket, `durationMs` | +| `export_failure` | action | the above + `stage` (`engine`/`encode`), `category` (`engine`/`network`/`codec`/`memory`/`ffmpeg`) | +| `export_cancel` | action | `stage`, `durationMs` | +| `thumbnails_generated` | duration vital + action | `frames`, `skipped`, `durationMs` | +| `thumbnails_failed` | action | `reason` (`timeout`/`decode`) | +| Global context | — | `cross_origin_isolated`, `shared_array_buffer`, `cpu_cores` bucket, `device_memory_gb` bucket, `reduced_motion` | + +Slow interactions and long tasks come from RUM's built-in collection +(`trackLongTasks: true`, plus INP/LCP/CLS on view events), so no custom code +duplicates them. + +Suggested dashboard widgets (not created here; there is no Datadog account +access from this change): +- p75 `@view.interaction_to_next_paint`, `@view.largest_contentful_paint` + and `@view.cumulative_layout_shift`, by `@device.type`. +- p50/p90 of the `export` vital by `@context.resolution` and + `@context.cross_origin_isolated`. +- Count of `export_failure` by `@context.category` and `@context.stage`. +- p90 of the `ffmpeg_init` vital by `@context.threading`. + +## How to measure + +```bash +# Unit/regression tests (CI-safe; no real video or network) +bun run test + +# Production build served like production +bun run build +node scripts/perf/serve.mjs out 4300 # with COOP/COEP (vercel.json intent) +NOISO=1 node scripts/perf/serve.mjs out 4301 # without (what production serves today) + +# Lighthouse, mobile preset, 5 runs, prints per-run and median +scripts/perf/lighthouse.sh http://localhost:4300/ local 5 +scripts/perf/lighthouse.sh https://reframe-lilac.vercel.app/ live 3 + +# Browser scenarios need puppeteer-core (not a project dependency) +npm i --no-save puppeteer-core # or: PUPPETEER_CORE=/path/to/puppeteer-core +node scripts/perf/make-fixture.js 6 640 360 scripts/perf/fixtures/sample-6s.mp4 http://localhost:4300/privacy +node scripts/perf/bench.js http://localhost:4300/ after scripts/perf/fixtures/sample-6s.mp4 +node scripts/perf/motion.js after=http://localhost:4300/ +``` + +`bench.js` covers: +- upload and metadata +- 5 s of playback +- rapid slider adjustment +- thumbnail generation +- a failed engine load (CDN blocked) followed by retry +- exports at 720p, 1080p and 1080×1920 +- three export→cancel cycles with worker counts +- an export after cancelling + +It writes `bench-