Skip to content

Reserve thumbnail box size upfront to stop layout-shift pop/reflow - #158

Merged
philips merged 1 commit into
mainfrom
thumbnail-aspect-ratio-issue-154
Jul 31, 2026
Merged

Reserve thumbnail box size upfront to stop layout-shift pop/reflow#158
philips merged 1 commit into
mainfrom
thumbnail-aspect-ratio-issue-154

Conversation

@philips-clanker

Copy link
Copy Markdown
Collaborator

Summary

Addresses both issues reported after #156 (merged): thumbnails visibly "pop" once their image loads, and scrolling the thumbnail sidebar for a while (~50 pages) still crashed.

I think these are the same underlying bug. A thumbnail's `` has no `src`/dimensions until `ensurePageImage()` loads it, so it collapses to ~0 height in the meantime - that's the visible "pop," but it's not just cosmetic: every pop shifts every later thumbnail's position, reflowing the whole sidebar. A reflow that moves many items at once can re-trigger `thumbObserver` across a large swath of them, even though nothing was actually scrolled - turning a scroll through a long sidebar into a cascade of extra load/evict churn well beyond whatever's actually visible at any moment, which #156's zero-`rootMargin` fix alone doesn't bound (it only limits how many are visible at once, not how often the whole list reflows).

Change

Sets each thumbnail's `aspect-ratio` from the note's own `pageWidth`/`pageHeight` (uniform across all pages - already used for `nativeWidth`/`nativeHeight` elsewhere) before any image data exists. Width is already fixed at 100% of the item via CSS, so `aspect-ratio` derives the correct height immediately - the box is right from the start, nothing to pop, and an item's intersection state can now only change because of an actual scroll, not a sibling loading in and shifting everything below it.

What I could not verify

Same caveat as the last few PRs on this issue: no Obsidian/Electron/X server in my environment. This is my best-reasoned fix connecting both symptoms, but I can't confirm the crash is actually gone at ~50 pages of thumbnail scrolling without a real device. Please retest that specific scenario - if it still crashes after this, the cause is something else and worth a fresh look rather than assuming this was it.

Test plan

  • npx tsc --noEmit - clean
  • npx eslint src/main.ts - 0 errors
  • npx vitest run - 95 passed
  • npm run build - clean
  • Manual: thumbnails no longer visibly pop/resize as they load
  • Manual: scroll the thumbnail sidebar through 50+ pages without a crash

Two issues reported after #156 (thumbnail rootMargin fix): thumbnails
"pop" once their image loads (nothing reserved the box's size before
that), and scrolling the thumbnail sidebar for a while (~50 pages)
still crashed.

These are likely the same bug. An <img> with no src/dimensions
collapses to ~0 height, so it visibly pops to full size once
ensurePageImage() sets `src` - but that's not just cosmetic: every pop
shifts every later thumbnail's position, reflowing the whole sidebar.
A reflow that moves many items can re-trigger thumbObserver for all of
them at once, even though nothing was actually scrolled - turning a
smooth scroll through a long sidebar into a cascade of extra
load/evict churn well beyond whatever's actually visible at any given
moment, which #156's zero-rootMargin fix alone doesn't bound.

Sets each thumbnail <img>'s aspect-ratio from the note's own
pageWidth/pageHeight (uniform across all pages, already used for
nativeWidth/nativeHeight) before any image data exists - width is
already fixed at 100% of the item via CSS, so aspect-ratio derives the
correct height immediately. A stable layout from the start means an
item's intersection state can only change because of an actual scroll,
not a sibling loading in and shifting everything below it.
@philips
philips merged commit 48db5bf into main Jul 31, 2026
7 checks passed
pull Bot pushed a commit to ben-vargas/supernote-obsidian-plugin that referenced this pull request Jul 31, 2026
Reported after testing 3.0.2-beta.17 (issue philips#154): scrolling the
thumbnail sidebar slowly is fine, but scrolling quickly through it
still crashes - even bounded to "only what's currently visible"
(philips#156) and with the pop/reflow cascade fixed (philips#158).

A fast scroll flings most thumbnails past the viewport within a
couple hundred milliseconds. Every one of those still triggered a
full, real-page-resolution rasterization (ensurePageImage() has no
separate cheap/low-res path for thumbnails) immediately on becoming
intersecting, then got evicted again almost as fast once it left -
dozens of full rasterizations back-to-back in quick succession during
one fast flick through a long sidebar.

Debounces the *load* trigger ~200ms per thumbnail (not eviction -
freeing memory promptly on scroll-away has no downside either way): a
thumbnail only actually loads if it's still intersecting once its
timer fires, so one that was only ever transiently in view during a
fast scroll never triggers a rasterization at all. Only pages the
user actually lingers near for a moment do.

Also clears any pending per-page debounce timers in onLoadFile()'s
reset and onClose() - disconnecting the observer stops new callbacks,
but doesn't cancel timers an earlier one already scheduled, which
could otherwise fire after a file switch and redundantly load a page
from a note the user's already navigated away from.
philips-clanker added a commit that referenced this pull request Aug 1, 2026
Requested for testing on desktop, where DevTools' Memory tab is
available to cross-check against: "scrolling all 100 pages still
causes a crash" even after #155/#156/#158/#159's fixes, and it isn't
obvious from the outside whether eviction is actually keeping up or
something's still accumulating.

Logs every page load/evict (main view and thumbnails alike, since both
go through the same ensurePageImage()/evictPageImage()) with a rough
memory estimate for that page (decoded ImageBitmap + canvas backing
store - the latter often the larger of the two, up to 9x the bitmap's
pixel count at capped 3x devicePixelRatio) and a running total across
every currently-loaded page. Watching whether "currently loaded" keeps
climbing over a long scroll (something isn't actually being freed) or
stays roughly bounded (eviction is working, and the cause is something
else - e.g. pure CPU/GC pressure from rasterization itself, see
supernote-typescript#40's thumbnail-resolution angle on that) should
narrow down what's actually still going wrong.
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.

2 participants