Skip to content

perf(detail): stop re-analyzing/re-decoding on every in-place photo switch#521

Merged
Zheaoli merged 2 commits into
mainfrom
feat/detail-perf
Jun 10, 2026
Merged

perf(detail): stop re-analyzing/re-decoding on every in-place photo switch#521
Zheaoli merged 2 commits into
mainfrom
feat/detail-perf

Conversation

@Zheaoli

@Zheaoli Zheaoli commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Why

After the detail-view work deployed, opening a photo and switching between photos felt janky. Devtools profiling (6× CPU throttle to simulate a slower machine), corroborated by network-trace and code-read forensics:

  • Open: ~1.25s main-thread block (one 725ms task), 800ms worst frame.
  • Switch: a 68ms forced reflow + CLS 0.06 + the histogram/tone pipeline re-running (network and CPU) on every switch.

Root cause = the new in-place switching re-runs per-photo work that the old full-page-nav detail only did once.

Fixes (pure rendering/compute — no LRU / WebGL-lifecycle / data-layer change)

  1. Remove the ?_t=Date.now() cache-buster in HistogramChart + ToneAnalysis. It re-fetched the cross-origin preview on every switch. Cross-origin CORS requests are cached separately from the grid's non-CORS <img>, so the canvas still reads cleanly — without the per-switch refetch.
  2. Memoize histogram/tone per image url (bounded LRU, cap 256, mirroring the decoded-blurhash cache) — revisiting a photo no longer re-fetches/re-decodes/re-scans.
  3. Debounce the histogram/tone source: updates only after switching settles ~220ms, so fast-swiping analyses just the photo you stop on. Initialised to the opened photo → still immediate on open, no pop-in.
  4. Thumbnail strip: active-thumb scrollIntoView now runs in a requestAnimationFrame (instant, still centered) → no forced synchronous reflow per switch; buttons get content-visibility:auto + contain-intrinsic-size → off-screen thumbnails in a large album aren't laid out/painted on open.
  5. Carousel placeholder slides decode their real blurhash only when near the current index; farther slides share one cached default decode → opening a large album doesn't decode ~20 thumbhashes up front.

Backdrop-blur on the strip is left as-is — that's a visual change (frosted-glass look), tracked separately.

Verification

tsc + eslint clean for changed files. Profiled the current prod build to find the causes; the perf numbers are observational and want a post-deploy before/after re-measure via devtools (switch should no longer fire a preview fetch or histogram recompute except for the first analysis of a newly-settled photo; open decodes far fewer thumbhashes).

…witch

Devtools (6x CPU) showed the in-place detail view jank: opening blocked the main
thread ~1.25s and each switch caused a ~68ms forced reflow plus a re-run of the
histogram + tone analysis (network + CPU). Root causes and fixes:

- HistogramChart / ToneAnalysis re-fetched the preview every switch via a
  `?_t=Date.now()` cache-buster and re-ran getImageData + a full-pixel scan.
  Removed the cache-buster (cross-origin CORS requests are cached separately
  from the grid's <img>, so the canvas still reads cleanly) and memoized the
  computed result per image url — revisiting a photo no longer recomputes.
- Debounced the histogram/tone source: it now updates only after switching
  settles for ~220ms, so fast-swiping through many photos analyses just the one
  you stop on instead of every frame's photo. Initialised to the opened photo so
  it still shows immediately on open (no pop-in).
- Thumbnail strip: the active thumbnail's scrollIntoView now runs in a rAF
  (instant, still centered) so it no longer forces a synchronous reflow on every
  switch, and the buttons use content-visibility:auto so off-screen thumbnails in
  a large album aren't laid out/painted on open.
- Carousel placeholder slides only decode their real blurhash when near the
  current index; farther off-screen slides share one cached default decode, so
  opening a large album doesn't decode ~20 thumbhashes up front.

Pure rendering/compute optimisation — no change to the LRU, the WebGL viewer
lifecycle, or the data layer. Backdrop-blur on the strip left as-is (a visual
call for @Zheaoli, tracked separately).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
picimpact Ready Ready Preview, Comment Jun 10, 2026 3:35am

Pre-empts the unbounded-growth concern: the per-url histogram/tone caches are now
bounded LRU (Map insertion order = LRU, re-set on hit keeps hot entries, evict the
oldest past 256) so a long session over many distinct photos can't grow them
without limit. Mirrors the decoded-blurhash cache.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Zheaoli Zheaoli merged commit c963d97 into main Jun 10, 2026
5 of 6 checks passed
Zheaoli added a commit that referenced this pull request Jun 12, 2026
…oesn't flicker

ToneAnalysis returned a spinner unconditionally while `loading`
(`if (loading) return <spinner>`), so every photo switch collapsed the whole
tone section to a tiny spinner and re-expanded it when the new analysis
finished. That shrink/grow shifted the entire info panel's layout (histogram
and device-info rows below jumped up then back) — the "info panel flashes/
reloads on switch" the user reported.

#532 added the `loading && !histogram` guard to the histogram chart but not to
tone analysis, so the histogram stopped flickering while this one kept doing
it. Apply the same guard here: only show the spinner on the first analysis
(`loading && !toneData`); on a switch keep the previous tone values rendered
until the new ones are computed (the in-session LRU cache from #521 still
returns instantly for revisited photos). No size change → no panel shift.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant