Skip to content

fix(detail): stop the switch + zoom-load flicker - #532

Merged
Zheaoli merged 1 commit into
mainfrom
fix/detail-flicker
Jun 11, 2026
Merged

fix(detail): stop the switch + zoom-load flicker#532
Zheaoli merged 1 commit into
mainfrom
fix/detail-flicker

Conversation

@Zheaoli

@Zheaoli Zheaoli commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Two flicker problems reported after the zoom-blank fixes deployed.

② Switching photos flickers the info panel

  • Histogram/tone re-fetched + re-decoded a ~30MP image every switch. They were fed preview_url, which on this deployment is the full-resolution original (preview compression off). Each switch did new Image(preview_url) + getImageData → a 30MP fetch + decode → the panel went empty, recomputed, then popped in. Fix: feed them the same ~1280 display variant the slide already has browser-cached (the histogram downscales to ~300px, so a variant is plenty) → no extra fetch, tiny decode. Falls back to preview_url only when the photo has no variants.
  • Histogram flashed its backdrop-blur loading spinner (and dimmed its canvas) on every recompute. Show the spinner only on the very first load (loading && !histogram); on a switch keep the previous histogram visible and let the existing spring animation morph to the new one.

① Zoom flickers continuously while the high-res loads

The high-res XHR's loadingProgress updates re-render ProgressiveImage repeatedly during a multi-MB load, which re-rendered the WebGL viewer on every tick. Fix: memo the viewer so progress ticks don't touch it — its props (src/dimensions) are stable until the image actually loads. Mount/unmount (the #510 destroy lifecycle) and a real src change still re-touch it.

Verification & follow-ups

  • tsc + eslint clean.
  • ② is observable/confirmed (Impl profiled: switch re-fetched a raw -o.jpg + CLS); ① (zoom) can't be exercised headless (no GPU / soft-nav intercept) — needs a real-device check that the continuous flicker is gone.
  • Known minor, not in this PR: switching still measures CLS ~0.06 (mostly the image area resizing to different photo aspect ratios; the panel scrolls internally so it doesn't shift siblings) and ~313 DOM mutations per switch (full PreviewImage re-render — memoizing slides/thumbnails would cut it). Both are follow-up polish; this PR targets the visible flicker.

Two flicker problems reported after the zoom fixes deployed:

Switching photos flickered the info panel:
- HistogramChart/ToneAnalysis were fed `preview_url`, which on this deployment is
  the full-resolution (~30MP) image, so every switch RE-FETCHED + RE-DECODED a
  30MP image just to compute the histogram (empty → recomputed → popped in). Feed
  them the same ~1280 display variant the slide already has cached instead (the
  histogram downscales to ~300px, so a variant is more than enough). No extra
  fetch, tiny decode. Falls back to preview_url only when there are no variants.
- The histogram flashed its backdrop-blur loading spinner (and dimmed its canvas)
  on every recompute. Show the spinner only on the very first load; on a switch
  keep the previous histogram visible and let the spring animation morph to the
  new one.

Zoom flickered continuously while the high-res image loaded:
- The XHR's `loadingProgress` updates re-render ProgressiveImage many times during
  a multi-MB load, which re-rendered the WebGL viewer each time. Memoize the
  viewer so progress ticks don't touch it — its props are stable until the image
  actually loads. Mount/unmount (the #510 destroy lifecycle) is unchanged.

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

vercel Bot commented Jun 11, 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 11, 2026 3:46pm

@Zheaoli
Zheaoli merged commit b304339 into main Jun 11, 2026
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>
pull Bot pushed a commit to candies404/PicImpact that referenced this pull request Jun 12, 2026
…letterbox blur, keep tone panel)

Rebuilt on top of besscroft#535 (which removed the preview fade animation). besscroft#535 alone
did not fix the dominant flicker because it kept `object-contain md:max-h-[90vh]`
and didn't touch the info panel. This adds the remaining, verified fixes:

- Image height: `<img>` (preview + high-res) `md:max-h-[90vh]` →
  `object-contain w-full sm:h-full`, and the ProgressiveImage root wrapper
  `relative` → `relative sm:h-full` so the height:100% chain resolves
  (container sm:h-[90vh] → viewport → slide → wrapper → img). The img element
  then stays a constant 90vh for every aspect ratio instead of resizing
  ~607↔1366px and overflowing on each switch. Verified live: injecting the
  wrapper height made the slide img go 607→810px.

- Blur placeholder: `style={{ objectFit: 'contain' }}` on the preview image so
  next/image renders the blur with `background-size: contain` (it reads
  style.objectFit, not the className) — otherwise the blur filled the fixed box
  via `cover` and the real image shrank to letterbox on load = a load flash.

- Tone analysis panel: `if (loading)` → `if (loading && !toneData)` so a switch
  keeps the previous tone values rendered instead of collapsing to a spinner and
  shifting the whole info panel's layout. Mirrors the histogram guard from besscroft#532
  (which was never applied to tone analysis).

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