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

[FEATURE] Configure how many pages to preload #210

Closed
iwa opened this issue Dec 5, 2023 · 8 comments
Closed

[FEATURE] Configure how many pages to preload #210

iwa opened this issue Dec 5, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@iwa
Copy link

iwa commented Dec 5, 2023

Is your feature request related to a problem? Please describe.
from what i've seen, pages are loaded one-by-one, and the loading is triggered by going to the next page
that's kind of annoying when used on mobile with an unstable connection, because sometimes the next page will load quickly, but sometimes it'll takes dozens of the seconds to load the page

Describe the solution you'd like
A setting so we can enable and choose how many pages to prefetch
that way, we could have a "margin" of already loaded pages, and avoid the issue explained above when using a poor internet connection

@iwa iwa added the enhancement New feature or request label Dec 5, 2023
@aaronleopold
Copy link
Collaborator

aaronleopold commented Dec 5, 2023

Hey 👋

Funny enough, this is actually already implemented. The relevant code section is here. If you can confirm a couple of things, we can transition this to a bug and I can look into it sometime this week:

  1. Does your network tab show the API calls being made? It should be 3 book page API calls when you first land on the reader
  2. Have you ensured you haven't disabled caching in your browser?

Edit for clarity: the pre-fetching itself is implemented, however your suggestion of having a config for controlling how many pages to fetch is not. If the baseline issue you described is in fact a bug, I'll be sure to leave this feature request, instead of converting it to a bug, in order to capture that aspect

@iwa
Copy link
Author

iwa commented Dec 6, 2023

I've done some testing:

Specs

  • Macbook pro 13" M1
  • Arc Browser
  • Fiber connection through wifi (theoretical 500mbps up/down)
  • Browser cache is enabled
  • Connecting to my server through Tailscale

Testing

  • Using network throttling in order to really test if network speed impacts the loading time

  • When opening a book, it seems to load the first 15 pages

Screenshot 2023-12-06 at 14 53 11

All the URLs follow this pattern:

http://homiwa:10801/api/v1/media/7b45fc79-cc2e-414d-bc43-3b8ff527e380/page/10

  • Then, the first 15 pages are correctly cached and only the progress api calls are made
Screenshot 2023-12-06 at 14 59 38
  • When crossing the 16th page, it returns to loading page by page
Screenshot 2023-12-06 at 15 00 28

You can see the pages' loading time averaging 5s, the throttling is effective
I've been waiting at least a minute before going to the next page

So i don't think this is working properly, the only way to "cache" images is to open the bottom bar showing all the thumbnails, that will trigger the loading of the pages, but i don't see any 3 pages ahead of prefetch :/

@aaronleopold
Copy link
Collaborator

Thank you for capturing all of this - it is incredibly useful, so I appreciate it!

This is definitely a bug then, the loading of 15 pages at once then 1-by-1 afterwards is definitely not the intended functionality. I'll be sure to give you an update once I figure it out and/or push up a fix 👍

@iwa
Copy link
Author

iwa commented Dec 6, 2023

you are very welcome! if you need me to make another batch of tests, don't hesitate to ping me

@aaronleopold
Copy link
Collaborator

aaronleopold commented Dec 9, 2023

Hello again! I had a very brief debugging session after work today and figured out what was happening. I wanted to give a little outline in case you are curious. The bug is actually a combination of two bugs and a more complicated inefficiency:

  1. The first bug that jumped out at me, in particular the (start, 3) part is just wrong
  2. The second bug is in the ToolBar component for image-based readers. It doesn't present realistically unless you start reading from a page past the first one. Essentially, the row of images at the bottom would always start at the first page, even if I start reading at the middle of the book. When I open the toolbar in the middle of the book, it would scroll to the middle and load every single page and then plus some after the current 😅
  3. The complicated inefficiency is very intertwined with the second bug, but isn't exactly a bug itself. The images on the bottom are virtualized, or in other words they are not all rendered at once. The virtualized count depends on how wide your screen is, in your case it was 15 and in mine during testing it was 23. This means that when the toolbar renders, even if not visible, those pages will be fetched. This, in combination with the first bug, is why I believe you observed a working cache of 15 images and then no more.

I've fixed the first bug and tentatively the second (just want to test that one a bit more locally). I'll want to spend a bit more time thinking through the third issue, but then hopefully the issue is resolved once merged to develop 👍

@aaronleopold
Copy link
Collaborator

aaronleopold commented Dec 13, 2023

Hello 👋

This should (hopefully) be resolved in the latest nightly build. Let me know if you face any issues, otherwise I can close this ticket I'll update the request to reflect the configurable aspect 👍

@aaronleopold aaronleopold added bug Something isn't working and removed enhancement New feature or request labels Dec 13, 2023
@aaronleopold aaronleopold changed the title [FEATURE] Pre-fetch next pages [BUG] Pre-fetch next pages Dec 13, 2023
@aaronleopold aaronleopold changed the title [BUG] Pre-fetch next pages [FEATURE] Pre-fetch next pages Dec 13, 2023
@aaronleopold aaronleopold added feature A planned, integral feature enhancement New feature or request and removed bug Something isn't working feature A planned, integral feature labels Dec 13, 2023
@aaronleopold aaronleopold changed the title [FEATURE] Pre-fetch next pages [FEATURE] Configure how many pages to preload Dec 13, 2023
@iwa
Copy link
Author

iwa commented Dec 15, 2023

Hey! thanks alot for your hard work
it works fine! only progress api calls are made after images are downloaded
Screenshot 2023-12-16 at 00 14 15

thanks again!

@github-project-automation github-project-automation bot moved this to Backlog in v0.1.x Jan 20, 2024
@aaronleopold
Copy link
Collaborator

aaronleopold commented Feb 25, 2024

I have implemented this preference option locally. I'll be sure to link the PR once I create one, I'll plan to have it go out with 0.0.2

Screenshot 2024-02-25 at 9 46 20 AM

@aaronleopold aaronleopold moved this from Backlog to In Progress in v0.1.x Feb 25, 2024
aaronleopold added a commit that referenced this issue Feb 25, 2024
aaronleopold added a commit that referenced this issue Feb 25, 2024
* Add discord notification for experimental build

* WIP: unify image-based readers a bit

* Stylistic tweaks

* Fix Yomu reader

* Improve preloading options

Relates to #210

* Touchups
@github-project-automation github-project-automation bot moved this from In Progress to Done in v0.1.x Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

2 participants