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

Window scroll listener causes scroll stuttering in Chrome #136

Open
pieh opened this issue Mar 12, 2019 · 13 comments
Open

Window scroll listener causes scroll stuttering in Chrome #136

pieh opened this issue Mar 12, 2019 · 13 comments

Comments

@pieh
Copy link

pieh commented Mar 12, 2019

This is cross-posted issue from gatsbyjs/gatsby#12520

Description

In Chrome DevTools using the FPS meter, the scroll listener that Gatsby seems to add causes poor scrolling performance. I noticed this on my website and on React's website. I don't add a scroll listener anywhere so I guess it's coming from Gatsby.

Steps to reproduce

  1. Visit atomiks.github.io/tippyjs
  2. Open FPS Meter in Chrome DevTools
  3. For me it's showing ~45 - 55 FPs with lots of frame drops and stuttering
  4. Type this into the console
getEventListeners(window).scroll.forEach(({ listener }) => {
  window.removeEventListener('scroll', listener)
})
  1. Much improved scrolling, way less frame drops once the listener has been removed.

Before: gyazo.com/57edc6fb31e1e06725f7e31c3b2a51a0
After: gyazo.com/3b4070170309a1c0037272a3cb3d3d17

I don't have any context on internals of scroll-behavior package, but maybe adding some debounce to listener function could help?

/cc @atomiks

@taion
Copy link
Owner

taion commented Mar 12, 2019

It is debounced, no? We only do work if the rAF handle isn’t set. If you have a test setup, maybe try making it passive? It shouldn’t help, but maybe it will.

@pieh
Copy link
Author

pieh commented Mar 12, 2019

Is it debounced? Maybe dom-helpers/events/on debounce it, but I didn't find debounce in scroll-behavior code from quick scan - but as I mentioned I don't have much context, and I probably am wrong.

I just updated issue description with content of issue that was reported in Gatsby so it's all in one place - I should have done this from the start.

@taion
Copy link
Owner

taion commented Mar 12, 2019

I mean, “not doing anything if the timer is already set” is pretty well debounced, no?

@atomiks
Copy link

atomiks commented Mar 13, 2019

It looks like the function calls from this package are taking <100 microseconds so not sure what could cause it. Every so often there's one or two 30+ms frames

With listener (here there was a long period of dropped frames):

img

Without listener:

img


Chrome still drops frames without it but not as many it seems. It's also completely random. Maybe it's just Chrome's fault!

@taion
Copy link
Owner

taion commented Mar 13, 2019

Okay, thinking about this a little more – a lot of the work in the rAF handler (which is causing the stutter) on window scroll is to work around quirks in Firefox (when using hash navigation). Probably the best thing to do is to be conservative about when to save window scroll position.

Ick.

The best thing to do would be to only save window position before navigation in cases where we know that this is safe. This really depends on details of when e.g. popstate events fire, though.

@sebpettersson
Copy link

I mean, “not doing anything if the timer is already set” is pretty well debounced, no?

The scroll event is fired at about the same rate as the animation frames, so theoretically this will save the scroll position up 60 times per second (on a 60 Hz monitor) which seems a bit much, and sometimes causes constant style recalculations in scrollLeft.js. I noticed this while doing some performance critical animation work that was tied to the scroll position.

Would it be possible to have timeout based debounced scroll event handler instead? I would rather have a 300+ ms (or something) delay on scroll position saving than degraded scroll performance.

@taion
Copy link
Owner

taion commented Mar 27, 2019

That sounds like a good idea to me. Ideally we should avoid checking/saving stuff while the user is actively scrolling as much as possible.

@taion
Copy link
Owner

taion commented Mar 27, 2019

Care to take a stab at a PR?

@sebpettersson
Copy link

Sure! It might take a few days before I have time to look at it though.

@aousaja
Copy link

aousaja commented Mar 30, 2019

The first thing was to realize that the page was not scrolling on click. I was able to test this with window.pageYOffset in the console to get the currents scroll position. As the #acctab-description tab was being hidden on click of #acctab-additional tab. this created the illusion of the page scrolling as the documents height was being reduced.
add the below JavaScript on the click event of the tabs to scroll the page to the correct position.

// offset of elements above the tabs
var offset1 = jQuery("div.product-secondary-column.grid12-3").offset().top;
var offset2 = jQuery("div.product-shop.grid12-5").offset().top;
var offset3 = jQuery("div.product-img-box.grid12-4").offset().top;
var offsetTotal = offset1 + offset2 + offset3;

jQuery(".acctab:not(.current)").click(function(){
    if(window.innerWidth < 977){
        var body = jQuery("html, body");
        body.animate({scrollTop:offsetTotal}, '10000', 'swing');
    }
});

@taion
Copy link
Owner

taion commented Apr 5, 2019

requestIdleCallback would be good, if there were a good polyfill.

@karlcow
Copy link

karlcow commented Mar 20, 2020

It probably creates a minor performance issue on firefox android
webcompat/web-bugs#47941

@satazor
Copy link

satazor commented Aug 25, 2020

@taion I'm also experiencing this. #147 adds debouncing but seems outdated and do not address elements registered with registerElement.

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

No branches or pull requests

7 participants