feat: implement SmoothScroll

[SpyrexDE]

Please check if the PR fulfills these requirements:

• The commit message follows our guidelines.
• For bug fixes and features:
  • You tested the changes.
  • You updated the docs or changelog. - Im not exactly sure how

Related issue (if applicable): #164

What kind of change does this PR introduce?
It adds a SmoothScroll addon and implements it's SmoothScrollContainer class by replacing every ScrollContainer with a SmoothScrollContainer.

Does this PR introduce a breaking change?
No.

New feature or change

• Smooth scroll for scroll containers

What is the current behavior?
Jaggy scrolling makes it hard to read and feels unresponsive.

What is the new behavior?
Adjustable, smooth scrolling with some "overdrag" for more responsiveness.

Video footage on YouTube

Other information

• To add a new SmoothScrollContainer, just open the add node menu, and click on "SmoothScrollContainer".
• To replace a ScrollContainer with a SmoothScrollContainer, right-click the node and select change type -> "SmoothScrollContainer"
• Horizontal scroll is not supported yet

Maintainance

I will try my best to maintain this add-on in the future. For any SmoothScroll-related issues please assign me so I get notified.

[NathanLovato]

I tested it in lessons, it's working fine. It does conflict with existing code to scroll with keyboard shortcuts, so that needs fixes. It'd be nice to add keyboard focus and standard web shortcuts to the smooth scroll container. See UICore.gd._input():

	if event.is_action_pressed("scroll_up_one_page"):
		scroll_container.scroll_vertical -= 800
	elif event.is_action_pressed("scroll_down_one_page"):
		scroll_container.scroll_vertical += 800
	elif event.is_action("scroll_up") and (event as InputEventMouseButton).pressed:
		scroll_container.scroll_vertical -= 80
	elif event.is_action("scroll_down") and (event as InputEventMouseButton).pressed:
		scroll_container.scroll_vertical += 80
	elif event.is_action_pressed("scroll_to_top"):
		scroll_container.scroll_vertical = 0
	elif event.is_action_pressed("scroll_to_bottom"):
		scroll_container.scroll_vertical = 1000000

The plugin.gd file also causes an error on load in Godot.

@pycbouh could you check this and tell me what you think? I think I'd like to refactor the code to make it match the rest of our codebase, make it a bit easier to read, and maintain it internally.

[SpyrexDE]

With the latest commit, the shortcuts are working again.

"The plugin.gd file also causes an error on load in Godot."
What's the error? I can't reproduce it :/

[NathanLovato]

Thanks. The update cleared the error - it was likely just a godot / caching issue so don't worry about it.

[NathanLovato]

The PR should also fix #114, but I'd need people who face the bug to test this.

[NathanLovato]

@SpyrexDE are you okay with us refactoring your code? It's about formatting, renaming some variables, replacing some comments with names in code.

[SpyrexDE]

@SpyrexDE are you okay with us refactoring your code? It's about formatting, renaming some variables, replacing some comments with names in code.

No problem 👍

[NathanLovato]

Thanks, I refactored the code, and I'll add the setting shortly

[NathanLovato]

It's working pretty well by the way, thanks much for the contribution!

[NathanLovato]

The current implementation of smooth scrolling is a bit limiting, to me. That's because of the use of velocity rather than target scrolling positions. With keyboard shortcuts, we don't have fine control over the scrolling intervals.

Now, with the scrolling sensitivity setting, there's no good way to only apply it to scrolling with the mouse/touchpad. I think we could benefit from an implementation that tweens (or rather, probably steers) scrolling to a target position. It would make the overscroll control perhaps slightly more difficult to implement but would make the input more consistent and easier to unify.

[NathanLovato]

Okay so I gave this idea a spin.You can find the result in commit 54f95dd

This prototype removes support for over-drag (it needs to be re-added). However, as mentioned above, it gives us precise control over the scroll level. We can use that to:

• Programmatically scroll to a specific position on the page (we have a feature like that in lessons)
• Smooth scroll with keyboard shortcuts
• And preserve smooth scroll with the mouse and touchpad (touch screens too?)
• Bonus: this implementation should support horizontal smooth scroll out of the box thanks to the use of steering

This also re-adds support for scrolling with the up and down arrow keys, including support for echo events.

Also, over-dragging shouldn't be too hard to achieve, it can leverage the steering equation.

@pycbouh could you give this a try when you have a moment and tell me what you think?

Current known issues:

• No support for dragging past a container's edges.
• Smooth scrolling with Home and End is a bit too slow. Not sure if it should be instantaneous or really fast.

[SpyrexDE]

Looks great! The implementation using a _target_position doesn't feel as spongy but I see the benefits.
There are some minor bugs:

• _max_y_position is too high (because in line 109 your are dividing the rect_size.y by 2)
• _content_position doesn't update when using the scroll bar (I did it by using the scrolling signal of the VScrollBar)
• When scrolling to the top you can scroll just a view pixels further
• Page up/down buttons do not scroll exactly one page on different scroll containers

[SpyrexDE]

• Programmatically scroll to a specific position on the page (we have a feature like that in lessons)

That's also possible with velocity-based scrolling (see velocity_scroll branch):

2022-01-29.15-53-24.mp4

• Smooth scroll with keyboard shortcuts

Already implemented in velocity-based scrolling.

• And preserve smooth scroll with the mouse and touchpad (touch screens too?)

Well, scrolling with touchscreens works much differently but is possible to implement.

• Bonus: this implementation should support horizontal smooth scroll out of the box thanks to the use of steering

It's actually the same with velocity-based scrolling, I don't get your point there 🤔

[NathanLovato]

Steering is just a different equation that simplifies the code - it's still velocity-based.

You just need less code, less state to keep track of. I was not saying other things are not possible, I was saying some weren't implemented in the PR before.

The use of steering gives you horizontal + vertical support only with a couple of lines of code - there's no need for extra code or conditions.

The less state you have in your code, the less maintenance work you have. For that reason, we favor implementations that reduce the need for extra boolean switches and whatnot.

[NathanLovato]

The latest commit should address the issues you listed above. I can't reproduce the issue of scrolling past the top, do you still have it?

[SpyrexDE]

You are right, maintainability might be more important at this point.

The latest commit should address the issues you listed above. I can't reproduce the issue of scrolling past the top, do you still have it?

Unfortunately, yes:

2022-01-29.16-50-14.mp4

(It's the same at the bottom)

[NathanLovato]

Okay, I couldn't reproduce it but I think the code change should've fixed it - I lowered the arrive threshold and moved the distance check to the top of _process.

[SpyrexDE]

Okay, I couldn't reproduce it but I think the code change should've fixed it - I lowered the arrive threshold and moved the distance check to the top of _process.

yep, works perfectly now 👌

[NathanLovato]

After testing smooth scrolling in Brave/Chrome and Firefox, I made the base settings a bit faster to get a bit closer. I'd like your thoughts on that. You can still lower the sensitivity to make it slower.

The last commit also makes scrolling to top/bottom about 4x faster.

@Xananax @pycbouh when you have a moment I'd also like your feedback on this smooth scrolling feature before merging (no hurry though).

[SpyrexDE]

It definitely feels more like Brave/Chrome and Firefox now but these web browsers have no max scroll speed. It feels a bit undynamic when having a constant velocity while scrolling fast.
Is that scroll speed capping really needed?

[NathanLovato]

Is that scroll speed capping really needed?

No, it could scale based on the distance to the target. I was seeing firefox's scrolling accelerates when you quickly scroll the mouse wheel. We can try that and see. I need you guys' feedback for that actually, I don't know what feels and works best for other people.

[SpyrexDE]

I agree. Feedback is very important for this kind of thing. I tried it on my touchpad again and it's pretty okay using the lowest scroll speed option.

[NathanLovato]

I tried a few changes. The latest commit makes speed scale with the distance to the target. I tried with some easing curves but proportional seems to work fine. It makes scroll to top / bottom really snappy. There's also a distance threshold so it doesn't accelerate below a certain scroll amount.

Now, please feel free to tweak the constants and settings. It works fine on my computer, with a pretty fast base scroll. I think the SCROLL_INCREMENT could be a bit too high for the average user for instance. Maybe the base speed too. I tend to use fast anims, scroll, key repeat speeds etc. so I'm not the right person to set good default.

[SpyrexDE]

For me, these settings feel good when using a mouse wheel.
Using a touchpad still does not feel great. 😕

[NathanLovato]

I added damping based on the frequency of mouse wheel events. Because dragging on the touchpad generates many more events than when using the mouse wheel, this helps to make the touchpad scroll more like the mouse in terms of amplitude.

What it does is dampen the change in target position with successive events happening in a short time span.

Could you try and tell me how it feels for you now?

[SpyrexDE]

It still doesn't feel great. I think that it is not really possible to achieve good touchpad scrolling because of the lack of input information the scroll input event is giving you.
(See Calinou: "The mouse wheel is actually considered to be a mouse button, which is pressed and then released instantly.")
Some workarounds like what you are trying right now might improve it a little but without Godot telling more details about the scroll input, I can't imagine how to get this working so it actually feels good.
Additionally, the chance of Godot getting better touchpad support is a lot higher than the smooth scrolling issue.
Like reduz said about smooth scroll in 2017: "This is more work and no gain, so I would prefer not."
In contrast to that, the more detailed mousepad events already have an issue open with MacBooks already checked. So it's just a question of time for the feature to come to Godot I guess.

For me, the best results are still on the 748c52d commit.

[NathanLovato]

Definitely, at some point, this will need work on Godot's side. Especially if some users experience bugs. I reverted the last changes.

[NathanLovato]

And the comments from reduz... he used to instantly reject some ideas back when Godot didn't have as many contributors and users.

These days with proposals things might be different: if enough people vouched for the need for smooth scrolling, or, probably more in line with Godot's philosophy, for improvements that would allow for that (more control on scroll bars, distinguishing touchpad events) that could get accepted.

[NathanLovato]

Report from my teammate @Xananax. Seems input works differently in different browsers too.

I'm getting different results in FFx, Chromium, and Desktop (all Linux... Will try Windows later maybe)
I tested touchpad, mousewheel (wireless a4Tech standard mouse) and dragging the scrollbar

On FFx:

• touchpad & wheel & drag: some stuttering (maybe my pc's performance? But I don't remember that from before)
• mousewheel: there's a small bounciness sometimes, as if it was on a spring. It overshoots before going backwards. This happens mostly when I scroll down. Maybe a mechanical issue with my mouse, but it doesn't happen anywhere else

on Chrome:

• wheel & drag: no problem
• touchpad; Ziiiiiip!!!! I can be at the beginning, or the end of the doc (on a small doc like "your first error")

Desktop:
all good

[NathanLovato]

My teammate confirmed that without dampening or skipping some input events, this bug #114 still happens. The issues are:

• It seems some browsers fire more touchpad input events than others.
• Also some touchpads/laptops fire more events than others.

This causes scrolling to be too fast for some people. Dampening like in the previous commit was one option, another is to throttle the input events - discarding a number of them when they occur too fast.

[NathanLovato]

From @Xananax

This commit (with dampening) works better for me, in both FFx and Chrome. I don't get the overshooting in FFx, and in Chrome, though it's rather fast, it's still possible to only scroll a few lines
Whereas before it was like half a page anytime you barely touched the pad
but dampening isn't going to work as well as throttling for this
You want to have a set timeout that sends at regular intervals as long as a scroll event was received within the last tick
This way, your rate of event reception is constant, and you don't have to deal with that anymore

[NathanLovato]

Okay I'm gonna merge what we have so far as it's an improvement and we'll add input event throttling later to try and fix #114

[NathanLovato]

Thanks much for the contribution and testing @SpyrexDE !