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

Debounces scroll events in web viewer. #5208

Merged
merged 1 commit into from
Aug 19, 2014

Conversation

CodingFabian
Copy link
Contributor

As requested in #5178, this change debounces the scroll events.
The reason for doing so is that browsers can event-storm especially on
scroll, communicating hundreds of subpixel changes.

The main reason for this resulting in poor performance is that on each
scroll event scrollTop was queried, which forces layouting.

This change will set the scrolling to happen after 75ms of no more
scrolling happening. This timeout suppresses consecutive scrolls but almost
immediately fires as soon as the user slows down.

Fixes #5178, should improve #5207

For easier review use: https://github.com/mozilla/pdf.js/pull/5208/files?w=1

@CodingFabian
Copy link
Contributor Author

When reviewing / trying this, you should note that even a timeout of 4ms (which is the minimum timeout by spec) improves the responsiveness. I think 250 is an ok value, but would love to get feedback about your experience.

@yurydelendik
Copy link
Contributor

Can we cancel previous scheduled timeouts for the case when user rapidly scrolls? At least if two are already scheduled, last one.

@CodingFabian
Copy link
Contributor Author

I just pushed a version where I cancel all scrolls except the last one. Because of that I needed to choose a much lower timeout.
How does this one feel to you?

@CodingFabian
Copy link
Contributor Author

maybe add requestAnimationFrame polyfill (https://gist.github.com/paulirish/1579671) and use that as well?

@timvandermeij
Copy link
Contributor

When scrolling a bit rapidly, pages appear to be empty for a longer time, which in my opinion is worse than the 250 ms version.

@CodingFabian
Copy link
Contributor Author

i agree.
Let me push a version not relying on own timers but using requestAnimationFrame (including the above polyfill. (is it ok to include with that license?))

@yurydelendik
Copy link
Contributor

we don't need rely on rAF in this case -- its only useful when we ready to paint.

@CodingFabian
Copy link
Contributor Author

yes, but rAF does automatic debouncing and scheduling for us. For me it feels like an improvement over manually doing it.

As requested in mozilla#5178, this change debounces the scroll events.
The reason for doing so is that browsers can event-storm especially on
scroll, communicating hundreds of subpixel changes.

The main reason for this resulting in poor performance is that on each
scroll event `scrollTop` was queried, which forces layouting.

This change will use `requestAnimationFrame` to make sure the browser can
allocate enough time to other tasks. The delay is however quite small, thus
the reduction in executions is less noticeable. Modern browsers however utilize
`requestAnimationFrame` to smoothen out rendering.
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/c2fdb87333af63d/output.txt

yurydelendik added a commit that referenced this pull request Aug 19, 2014
Debounces scroll events in web viewer.
@yurydelendik yurydelendik merged commit 2f5c6d6 into mozilla:master Aug 19, 2014
@yurydelendik
Copy link
Contributor

Thanks

@CodingFabian CodingFabian deleted the debounce-scroll branch August 19, 2014 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

viewer.js should debounce scroll
4 participants