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

Smooth scrolling is terrible #627

Closed
glacambre opened this issue May 29, 2018 · 17 comments · Fixed by #1623
Closed

Smooth scrolling is terrible #627

glacambre opened this issue May 29, 2018 · 17 comments · Fixed by #1623
Labels

Comments

@glacambre
Copy link
Member

glacambre commented May 29, 2018

After having set smoothscroll true, just try keeping j pressed on any page (e.g: https://en.wikibooks.org/wiki/Ada_Programming/Generics). Scrolling will become slower and slower until the browser doesn't respond anymore.

Smooth scrolling doesn't have to be slow though. Vim-vixen's scrolling is fine. Unfortunately we can't just steal their implementation because their scrolling algorithm doesn't work everywhere (e.g. https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html).

I think we might be able to fix this by caching the last element recursiveScroll scrolled.

@bovine3dom
Copy link
Member

Scrolling will become slower and slower until the browser doesn't respond anymore.

That sounds like an async problem. Perhaps we can just have a scroll lock, and if anything is scrolling in the same direction subsequent scrolls become a no-op?

@glacambre
Copy link
Member Author

glacambre commented May 29, 2018

I've already implemented a lock here, so elements that are already being scrolled aren't scrolled more. We could make the lock global instead of per-element but I'm not sure it would help.

From what the profiler tells me I'd say the main problem is that I'm constantly recomputing the page's style here. I'll try to fix that first.

Then there are also frequent GC pauses. This could be fixed by re-using ScrollingData objects instead of overwriting them when they're done scrolling.

@glacambre
Copy link
Member Author

I can still reproduce this issue, even with #628 merged. I still see lots of GC pauses, some 50 milliseconds long.

@snippins
Copy link

snippins commented Jun 1, 2018

Hello, I just want to ask that is it possible to implement precise scrolling, which based solely on key up/down events (like movements in games)?

I also have smooth scrolling in firefox disabled, I prefer precise scrolling over any kind of smoothness. However when scrolling with keyboard (with firefox built-in and tridactyl, initial delay before key repeat make the scrolling really unpleasant compare to mouse scroll, it also needed only for editing texts rather than going through and read something IMHO.

@glacambre
Copy link
Member Author

snippins: I'm not sure I understand what you mean by precise scrolling. If you mean scrolling pixel by pixel, you can already do that with bind j scrollpx 0 10 for example.

I could be wrong but I think implementing a way to bypass the delay before key repeat would be pretty hard. This would require modifying the parser to listen for a "keyup" event and try to guess if the delay between the "keydown" and the "keyup" event meant that the user wanted to press the key multiple times or just once, basically re-implementing the whole key-repeat functionality ourselves.

You could try lowering the key repeat delay for your whole OS but it might not be a good idea.

@snippins
Copy link

snippins commented Jun 1, 2018

@glacambre By "precise scrolling" I just mean scrolling without the delay before key repeat and without any smoothing.

I do have key bindings to lower/reset the delay, but I have to toggle it on/off everytime I want to scroll is pretty inconvenient.

I think after keydown event we just keep calling scrollline after a small period. I do not think we need to consider if the user mean once or more key pressed. On Linux, I basically disable it (with xset r rate 1 60) and find it very comfortable for reading (using built-in up/down arrows with toolkit.scrollbox.verticalScrollDistance set to 1 in about:config). Of course I have to reset the delay before I can do anything else other than scrolling.

I am still not sure how hard it is to be implemented though.

@glacambre
Copy link
Member Author

I do not think we need to consider if the user mean once or more key pressed

We do need to know if the user wanted to press the key once or twice, otherwise Tridactyl will become as unusable as your computer when you disable the delay. Without such a verification, a user who wishes to type "gh" might be slightly too slow and end up with "ggh" instead.

@glacambre
Copy link
Member Author

Can anybody else reproduce this issue? Even on the computer where I can reproduce it, changing profile or even Tridactyl version (e.g. loading Tridactyl through about:debugging instead of downloading it from the betas website) makes this bug disappear.

@huffstler
Copy link

I think I do, though it may be slightly different from what you're seeing.

I had smooth scrolling turned on in my Preferences, though it never was smooth when using the mouse or keyboard.

Pressing and holding 'j' or 'k' on moderately large webpages sees about half a second of okay scrolling and then it seems like the screen kinda stops. Sometimes there's a good second of no movement and then the screen "catches" up a little bit.

Same thing happens with smooth scrolling on or off btw.

@glacambre
Copy link
Member Author

By smooth scrolling on or off, do you mean toggling it in about:preferences or by using set smoothscroll true/false?

@huffstler
Copy link

huffstler commented Jun 5, 2018 via email

@glacambre
Copy link
Member Author

We used to fall back to about:preferences if users hadn't explicitly defined the smoothscroll setting in Tridactyl but this was very slow and would only be updated after a restart (there's no real API to read/write to about:preferences, we had to work around that by reading files that are only updated when Firefox shuts down).
Currently the only setting that matters is whether smoothscroll is set to true or false in Tridactyl. If you didn't change this value (it's set to false by default) I think Tridactyl might not be responsible for the issue you're experiencing.
Has scrolling always been like this for you?

@huffstler
Copy link

Ahhh, I really should heed that popular saying about assumptions!

I hadn't set smoothscroll in tridactly. Trying it out now, it actually seems (anecdotally) that the scrolling performance has improved a little bit, but I still wouldn't call it smooth by any means.

And to be a little more descriptive of my assumptions: When I say smooth scrolling, I'm thinking about it acting like scrolling using the scroll bar or mouse wheel, not jumps.

Yes, I would say that scrolling has been like this since probably Firefox 60 for me. (I'm on Nightly, so that was 2 releases ago for reference.) I just kinda figured it would work itself out eventually.

@chrisduerr
Copy link

Note that this seems to work great in Vimium by default, maybe some inspiration could be taken from this addon?

With the key repetition rate/scrolling amount combination that can be configured in Tridactyl, it has been very tough to justify to myself to switch to it. But I've heard great things so I'd love to do so!

As far as I can tell, Vimium's scrolling is done here.

@glacambre
Copy link
Member Author

glacambre commented Aug 11, 2018

Unfortunately we can't just steal Vimium's code because Vimium can't scroll on the following websites:

Saka-Key doesn't seem to work on:

Vim-Vixen has trouble with:

SurfingKeys doesn't like these:

And testing Tridactyl again on these websites, I notice that we're having trouble on https://bugzilla.mozilla.org/show_bug.cgi?id=1415792 again and I don't understand why because I fixed it at least two or three times.
If we're going to steal code, it should probably be Saka-Key's. I'm not sure what their problem is exactly but I know what problems we had with these websites and maybe they're the same.

Edit: we magically stopped having issues with bugzilla.mozilla.org so Tridactyl's the best at non-smooth scrolling again!

@glacambre glacambre added the bug label Jan 31, 2019
@svenstaro
Copy link

I think having a smooth scrolling function that works on most sites (as vimium's does) would already be a big gain. There is also only little risk since you'd be able to turn it off easily. Also, after a short test, some of the sites you posted actually do work fine with vimium in Firefox as of this writing. Would you accept a PR from a contrib for this issue even if it's not a perfect fix?

@bovine3dom
Copy link
Member

bovine3dom commented Apr 20, 2019 via email

glacambre added a commit that referenced this issue May 30, 2019
This commit fixes smooth scrolling. The issue with smooth scrolling was
that from time to time, pressing `j` would only scroll the page by a
pixel. This happened because the computed scrolling step was smaller
than a pixel. When writing to elem.scrollTop, this value would be
rounded to the nearest integer, which sometimes was equal to the
previous scrollTop value. This resulted in Tridactyl believing that the
element couldn't be further scrolled and stopping there.

This is fixed by using Math.ceil()/round() on the step in order to make
sure a round value is obtained. This value is then compared to the
current scrollTop value and incremented in case they're equal, in order
to make sure scrolling makes progress.

Fixes #627 and maybe
#54 ?
@glacambre glacambre reopened this May 30, 2019
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 a pull request may close this issue.

6 participants