Skip to content
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

Mobile performance improvements #417

Merged
merged 10 commits into from
Aug 8, 2022
Merged

Conversation

matthinc
Copy link
Contributor

@matthinc matthinc commented Aug 1, 2022

Different strategies (optimized rendering, caching) to improve the performance of the mobile app.

Progress:

  • Allow > 60 FPS rendering on some Android devices
  • Load assets in 3 steps (thumbnail -> SD -> HD) instead of 2 (thumbnail -> HD)
  • Prevent all gestures on remote_photo_view until HD version is loaded (prevents jumping)
  • Loading indicator in toolbar of image_viewer_page

@matthinc matthinc force-pushed the dev/mobile-performance-tweaks branch from 1c902c9 to d4b3bdd Compare August 5, 2022 18:57
@matthinc matthinc force-pushed the dev/mobile-performance-tweaks branch from 3e3862f to 6aa4704 Compare August 6, 2022 12:02
@matthinc matthinc requested a review from alextran1502 August 7, 2022 13:26
@matthinc matthinc marked this pull request as ready for review August 7, 2022 13:26
@alextran1502
Copy link
Contributor

alextran1502 commented Aug 7, 2022

There are two issues I noticed

  1. The loading circulation is a little bit distracting and I don't think we need it there. Additionally, it takes a while to finish loading all three stages, is it supposed to be that long?

  2. The action swipe up to show the photo's detail is rendering blank, probably null properties are passing in. Clicking the horizontal three dot works fine. See video below for more details

Android.Emulator.-.PixelXLAPI30_5554.2022-08-07.13-16-17.mp4

@matthinc
Copy link
Contributor Author

matthinc commented Aug 7, 2022

I think the loading indicator (or something like it) is necessary here because zooming is disabled in stage 1 and 2.
Especially stage 2 looks almost like the full-scale image, so I think it could be confusing without some information that the asset is still loading. But I'm open to suggestions on how we could solve this differently.

Good catch with the exif sheet! I think I messed this up during a rebase.

@alextran1502
Copy link
Contributor

I think the loading indicator (or something like it) is necessary here because zooming is disabled in stage 1 and 2. Especially stage 2 looks almost like the full-scale image, so I think it could be confusing without some information that the asset is still loading. But I'm open to suggestions on how we could solve this differently.

Good catch with the exif sheet! I think I messed this up during a rebase.

So this poses the question, do we need 3 stages of loading? Especially on mobile, since it increases data consumption with a little trade-off on quality.

@alextran1502 alextran1502 merged commit b46e834 into main Aug 8, 2022
@alextran1502 alextran1502 deleted the dev/mobile-performance-tweaks branch August 8, 2022 00:43
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