Skip to content

UX: Search view render performance improvements #2433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

heikomat
Copy link
Contributor

@heikomat heikomat commented Jun 17, 2022

This is a followup to #2292

After implementing the "Virtualization", the app no longer got slower and slower the further you scrolled.
I was however not satisfied with how long the placeholder-elements were visible when scrolling a little faster.

The main reason for the time it took to render the real elements instead of the placeholders came down to the fact that one element in the mosaic- or card-views just renders a lot of components, mostly buttons and icons.
However, most of these Buttons and icons aren't actually visible most of the time, but are rendered anyway.

By only rendering the Buttons and icons when required, i reduced rendering times by about 50%.
(UPDATE: I halfed the rendering times again by replacing <v-img> and <v-card> with <div>s), making scrolling through the pictures much more fun! (it also sped up loading the next batch, because the browser spends less time rendering stuff and more time processing requests)

(UPDATE2: improved performance again by 1. reducing layout-time by scoping css in mosaic view, 2. replacing <v-data-table> with a regular html-table and 3. moving some inline-styles to css-classes)

(These screenshots only include the conditional button-rendering, not the other performance improvements!)

before (laptop) after (laptop)
Screenshot 2022-05-08 at 03 05 59 Screenshot 2022-05-08 at 03 00 45
before (smartphone) after (smartphone)
scroll-ohne-changes scroll-mit-changes

See how effortlessly one can scroll through 2000 pictures without waiting seconds for them to appear and without impacting selection-performance:

Screen.Recording.2022-05-21.at.17.15.30.mp4

There was of course a reason why all buttons and icons were rendered at all times. It was implemented (here) to prevent lag when selecting pictures.
AFAIK the cause of the lag was, that selecting a picture updated the whole view, rerendering all loaded elements.
The solution was to always render the icons with display: none and when required apply a css-class to the photo that would make the corresponding icons appear.

Because of the virtualization however, the root cause no longer exists, because only a handful of elements are ever rendered at a time, so when a photo is selected, only the visible few elements need updating (instead of thousands of elements)


There is at least one Bug in these changes that causes the selection-icon to not show after clicking it. it only appeares after moving the mouse away from the photo and back over the icon. The icons "highlight" is visible however. This is probably something really small to fix.
UPDATE: I Fixed that bug

Acceptance Criteria:

  • Features and improvements are fully implemented so that they can be released at any time without additional work
  • Automated unit and/or acceptance tests have been added to ensure the changes work as expected and to reduce repetitive manual work
  • User interface changes are fully responsive and have been tested on all major browsers and various devices
  • Database-related changes are compatible with SQLite and MariaDB
  • Translations have been / will be updated (specify if needed)
  • Documentation has been / will be updated (specify if needed)
  • Contributor License Agreement (CLA) has been signed

Heiko Mathes and others added 30 commits April 18, 2022 23:28
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ heikomat
❌ Heiko Mathes


Heiko Mathes seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@lastzero lastzero added please-test Ready for acceptance test ux Impacts User Experience performance Performance Optimization labels Jun 17, 2022
@lastzero lastzero merged commit 580de58 into photoprism:develop Jun 17, 2022
@lastzero lastzero changed the title search view render performance improvements UX: Search view render performance improvements Jun 17, 2022
@lastzero
Copy link
Member

Our development preview and demo.photoprism.app have been updated so everyone can help test these changes! 🎂

Note that the full-screen viewer has recently been optimized to initialize only with the required data, which is already in the right format. However, it will load all the results from the top and additional results further down, so you can click "Next" for a while or start a slideshow:

That was good enough with the old search results views due to other bottlenecks that now seem to be mostly resolved. We will provide a new viewer, but not in the very short term:

In the worst case, we'll have to implement another workaround, such as using an offset when loading viewer content.

@gyto6
Copy link
Contributor

gyto6 commented Jun 28, 2022

I've tested from Firefox, chrome and iOS App. The scrolling is way smoother and I'm no more interrupted. Sounds good for now.
No ressources issues seen from the server side.

@graciousgrey graciousgrey added released Available in the stable release and removed please-test Ready for acceptance test labels Jul 30, 2022
carck pushed a commit to carck/photoprism that referenced this pull request Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance Optimization released Available in the stable release ux Impacts User Experience
Projects
Status: Release 🌈
Development

Successfully merging this pull request may close these issues.

5 participants