Multiple page resizes during loading

AlfredMadere commented 6 days ago

First off, this is an awesome project!

I noticed some jitter when scrolling to an area allowed the user to watch the pages render, so I throttled the cpu to 20x and noticed that the pages seem to go through 5-6 different size changes each time the user scrolls quickly. Without CPU throttling this just looks like jitter but when slowed down as shown in the attached screenshot its clear whats going on. I'm doing a deep dive right now trying to fix this and will let you know if i'm successful.

https://github.com/user-attachments/assets/696996c8-0a83-4f3e-bbe2-d15c5b9e1a9a

AlfredMadere commented 6 days ago

Two of the resizing events are related to these two lines in Page.tsx.

          height: `${shouldRender ? 'fit-content' : `${virtualItem.size}px`}`,
          minHeight: `${virtualItem.size-100}px`

shouldRender is set to true before the renderPageLayer actually renders which results in the following behavior:

shouldRender starts as false so the page size is equal to virtualItem.size
shouldRender is set to true but there is no content in the pageLayer yet so the height is set to the minHeight which is 100px less than the virtualItem.size
The content is rendered in the pageLayer and because the 'fit-content' class is now applied, the height of the page jumps back up to approximately virtualItem.size

jkgenser commented 5 days ago

Two of the resizing events are related to these two lines in Page.tsx.
          height: `${shouldRender ? 'fit-content' : `${virtualItem.size}px`}`,
          minHeight: `${virtualItem.size-100}px`
shouldRender is set to true before the renderPageLayer actually renders which results in the following behavior:

shouldRender starts as false so the page size is equal to virtualItem.size

shouldRender is set to true but there is no content in the pageLayer yet so the height is set to the minHeight which is 100px less than the virtualItem.size

The content is rendered in the pageLayer and because the 'fit-content' class is now applied, the height of the page jumps back up to approximately virtualItem.size

@AlfredMadere : Thanks for filing this Issue.

Appreciate you pointing out some of the potential root causes. Do you have a working patch that works to fix the issue? If not totally cool, I think you provided enough information for me to reproduce on my end - I plan on looking into this!

AlfredMadere commented 5 days ago

Hey there, unfortunately I don't yet have a patch. I'm working on a stripped down version as I don't need rotation or zoom features for my project, If i can get it working there, I will post my findings and or create a PR if the fix is generalizable.

AlfredMadere commented 5 days ago

Another thing i'm noticing is that shouldRender toggles on and off twice in each page component every time the scroll stops resulting in the loading component flashing on and off twice. Looking at the profiler it doesn't seem like there are big performance gains from waiting to render until the scrolling stops so i just removed it and it seemed to solve that issue without any real downsides. I also solved the height jittering by just forcing the page-inner-box to always be the same height like so:

 height: `${`${virtualItem.size - EXTRA_HEIGHT}px`}`

And then in order to allow for a loading component that can fill the whole page (maybe centered or something) I made the page take up the full width and height of the page-inner-box. I used tailwind classes but you could translate that to a style tag.

  <div className={'size-full relative'}>
            {renderPageLayer({
              pageIndex: virtualItem.index,
              scale: scale,
              rotate: rotation,
              rotationAdjustment
            })}
          </div>

Let me know if you want any more info if you decided to fix. Also happy to make a PR in the future when I have a little more time.

AlfredMadere commented 5 days ago

Here's what it looks like after those changes at 20x cpu slowdown

https://github.com/user-attachments/assets/3908e582-28d0-4812-a46c-100e620d199a

jkgenser commented 5 days ago

That looks awesome! I started with incorporating fixing the height on page-inner-box. I can confirm this stops the jittering on when CPU is throttled.

However, shouldRender I think is important to keep. The reason is that if you are on ike 100 page document and scroll from page 1 to 100, pdf.js is attempting to render each page. By the time it gets to page 100, it takes noticeably longer before that last page renders. Similarly, if just scrolling manually.

The purpose of shouldRender is to prevent these pdfjs render tasks from getting queued up and only attempt to render whenever the scrollbar isn't moving quickly. I re-tested and I think it's important to keep in for that reason.

That being said, I'll look into whether there's a way to optimize it.

AlfredMadere commented 4 days ago

I agree that shouldRender is a good feature that is worth keeping.

I temporarily removed it in my project because I wasn't sure how to solve the issue of the loading component flashing on and off. Here are two videos, one with, and one without conditional rendering based on shouldRender. You can see how the loading component flashes on and off and at full speed with no CPU throttling it looks like a glitch. Are you experiencing the same thing?

Using conditional rendering

https://github.com/user-attachments/assets/738f3221-ad73-4153-bcf7-3d16fdb4db56

Not using conditional rendering

https://github.com/user-attachments/assets/fe46e0b1-095f-457c-88ac-468e2db06c54

jkgenser / react-pdf-headless

Multiple page resizes during loading #12