Skip to content

perf: ask for previews the size they are shown at - #31

Merged
skjnldsv merged 1 commit into
mainfrom
perf/preview-and-bundle
Sep 10, 2026
Merged

skjnldsv merged 1 commit into
mainfrom
perf/preview-and-bundle

Conversation

@skjnldsv

Copy link
Copy Markdown
Contributor

Every image preview was requested at the full size of the display, in device pixels, whatever space it was actually shown in:

+ `&x=${Math.floor(screen.width * devicePixelRatio)}`
+ `&y=${Math.floor(screen.height * devicePixelRatio)}`

On a 4K HiDPI client that is 7680x4320 asked of the preview endpoint for a photo shown beside an open sidebar, and the server renders and caches one preview per size asked for. The handler knows the room it has, so it asks for that, capped at what the display can show and rounded up to a multiple of 256 so a window a few pixels wider reuses the preview that already exists instead of starting a new render.

Three smaller ones of the same kind:

  • the one @nextcloud/vue barrel import is now the component itself, so a consumer's bundler is not handed the whole library for a loading spinner: from "@nextcloud/vue/components/NcLoadingIcon" in dist/chunks/Images.mjs
  • the svg sanitizer loads from the branch that sanitizes, so it no longer rides along with every image: import("dompurify") is the only reference left in that chunk
  • the plyr controls take their listeners once rather than on every update, which a resize triggers many times over (the test counts 4 and 6 addEventListener calls across two updates before, 0 after)
  • a folder listing is dropped when the viewer is asked for something else, instead of running to completion for a folder nobody is looking at any more

Six tests fail without the changes. The two source-level guards in __tests__/imports.spec.ts are there because neither import shows up in a unit test, only in the built chunk.

Left out on purpose: caching the sorting config across openFolder calls, which would save one small request per folder open at the cost of showing a stale order after the user changes their sorting, with no event to invalidate it on.

👾 This pull request was assisted by Claude Code, commits carry an Assisted-by trailer.

@skjnldsv skjnldsv added status: review Waiting for reviews type: enhancement 🚀 New feature or request AI assisted labels Sep 10, 2026
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.75%. Comparing base (abbbb9c) to head (3e87a70).

Files with missing lines Patch % Lines
lib/views/Viewer.vue 50.00% 5 Missing ⚠️
lib/components/Videos.vue 0.00% 1 Missing ⚠️
lib/utils/previewUtils.ts 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
+ Coverage   84.33%   84.75%   +0.42%     
==========================================
  Files          36       36              
  Lines        2432     2467      +35     
  Branches      486      499      +13     
==========================================
+ Hits         2051     2091      +40     
+ Misses        377      372       -5     
  Partials        4        4              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`x` and `y` were the whole display, in device pixels, for every image
whatever the space it was shown in: a 4K HiDPI client had the server
render 7680x4320 for a photo beside an open sidebar. The handler knows
what room it has, so it asks for that, capped at the display and rounded
up to a multiple of 256 so a window a few pixels wider reuses the
preview the server already has rather than starting a new render.

Three more of the same kind. The one import of the `@nextcloud/vue`
barrel is now the component itself, and the svg sanitizer is loaded from
the branch that sanitizes rather than with every image; both are guarded
by a test over the sources, as neither shows up in a unit test. The plyr
controls take their listeners once instead of on every update, which a
resize triggers many times over. And a folder listing is dropped when
the viewer is asked for something else, instead of running to completion
for a folder nobody is looking at.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
skjnldsv force-pushed the perf/preview-and-bundle branch from 0776bc8 to 3e87a70 Compare September 10, 2026 16:25
@skjnldsv
skjnldsv merged commit d2d8ebf into main Sep 10, 2026
18 checks passed
@skjnldsv
skjnldsv deleted the perf/preview-and-bundle branch September 10, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI assisted status: review Waiting for reviews type: enhancement 🚀 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant