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

Overhaul inertial scrolling +fixes #47931

Closed
wants to merge 1 commit into from

Conversation

threethan
Copy link

@threethan threethan commented Apr 15, 2021

This is a fix to a previous pull request I made, which ended up getting mangled due to a botched rebase. The original description is as follows:

Summary

I made quite a few changes, however all are interlocked so they can't really be split to multiple branches.
I tried to be as thorough as possible with the description below, however, in short:

  • Scroll behaves much nicer on mobile
  • A bunch of options have been added

List of Changes

  • New options have been added to project settings under a new gui>scroll section
  • Completely rewrote inertial scrolling on touch. This fixes various issues, and make things far more consistent.
    • Various issues involving scroll_deadzone are now fixed. (Resolves ScrollContainer with a deadzone suddenly snaps/flickers on touch #45402)
    • Inertial scroll now decelerates quadratically instead of linear. This is more accurate to real-life friction, more consistent with other mobile applications, and subjectively feels better. It also fixes an issue where deceleration would be far too much on scaled nodes or windows.
    • The amount of inertia can be configured in project settings (gui>scroll>intertial_scroll_duration_touch)
  • Added a scroll_step parameter. When scrolling with the mouse wheel, each notch will scroll by one page * scroll_step. The default is 1/8, which is the same as the previously hard-coded behavior.
    • The default value of scroll_step can be set in project settings (gui>scroll>default_scroll_step)
  • When focus is within a scroll container, ui_page_up and ui_page_down input actions will now work as expected. (Previously they did nothing)
  • Added a scroll_smooth parameter. This will enable Windows 10 - style smooth scrolling when scrolling using the mouse wheel, page up & down, or with follow_focus
    • The default value of scroll_smooth can be set in project settings (gui>scroll>default_scroll_smoothed)
    • The duration of the smooth scrolling animation can be set in project settings (gui>scroll>smooth_scroll_duration_button)

Changes to default behavior

I have done my best to make these changes without breaking compatibility or changing default behavior. By default only the following are changed:

  • The default focus mode of a scroll_container is not CLICK instead of NONE. This is required for page up & down to work. It can manually be changed back to NONE, which revert page up & down to doing nothing, unless a child element is focused.
  • Inertial scrolling will always use the new, fixed physics.
  • The default_scroll_deadzone project setting is now under gui>scroll instead of gui>common
    All other new functionality requires settings to be manually changed by the user.

Context

Tested with:

  • 3.2.3:
    • Windows (in editor, exported debug, exported release)
    • Android (debug, release, custom build)
  • 4.0.dev:
    • Windows (in editor, debug)
    • Android (debug)

I previously submitted a similar pull request that only fixed bugs, however I closed it in favor of this one. This pull fixes all of the bugs fixed by the first one and more, by fixing the underlying issues.
This pull request does not include documentation for the new options. If/once it is merged to master, I will submit a second pull with new documentation added.
I have a 3.2 compatible version ready if desired

Bugsquad edit: This closes godotengine/godot-proposals#5386.

@threethan threethan requested a review from a team as a code owner April 15, 2021 18:27
@threethan
Copy link
Author

It's worth noting that, when I build from godot-latest, many options are missing from project settings. I am unsure the cause of this issue, but it has nothing to do with the changes made in this PR. It's most likely already a known issue.
This does, however, mean that the entire gui->scroll section is not visible.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 15, 2021

It's probably hidden under Advanced Settings.

@threethan
Copy link
Author

It's probably hidden under Advanced Settings.

That was it, thanks! I think hiding it makes sense, so I suppose that isn't actually an issue.

@reduz
Copy link
Member

reduz commented Aug 31, 2021

This looks good, although I would appreciate more pictures/videos are posted so other contributors can see how it works without recompiling. Otherwise, it is difficult to tell for everyone what the differences are in look and feel by just looking at the code.

@zaksnet
Copy link
Contributor

zaksnet commented Sep 27, 2021

Here is a preview of this PR:
image

Smooth scroll OFF:
SmoothScrollOff

Smooth scroll ON:
SmoothScrollOn

And a more obvious example with smooth_scroll_animation = 1, scroll_step = 0.5:
SmoothScrollOn2

@PoisonousGame

This comment has been minimized.

@Calinou
Copy link
Member

Calinou commented Nov 30, 2021

@GiantArtisan Please don't bump issues without contributing significant new information. Use the 👍 reaction button on the first post instead.

@SpyrexDE
Copy link

SpyrexDE commented Dec 2, 2021

I've made an inofficial SmoothScroll plugin a while ago: SpyrexDE/SmoothScroll
It's still not feature-complete and looking for contributors but could be the way to go when you can't wait to get smooth scrolling into your game.
Preview of my implementation can be found on YouTube: https://youtu.be/B3GjqV2c6yQ

@allkhor
Copy link
Contributor

allkhor commented Dec 9, 2021

Great job. But I see a noticeable delay with emitting "scroll_ended" signal. I'm testing this PR on the 3.x branch. The delay depends on inertial_scroll_duration_touch. With or without smooth scrolling enabled.
MRP: scroll_emmit.zip
This PR:
3 4 1 scroll_pr
Old behaviour:
3 4 1 rc1
Can you confirm this, or maybe I was wrong when porting to the 3.x branch?

@YuriSizov
Copy link
Contributor

Doesn't look like we're going to assess this for Beta 1, but we should review this and related PRs (#52128) for 4.0. This seems like an improvement many want and need.

@threethan
Copy link
Author

threethan commented Sep 8, 2022

Glad this is still getting looked at.
A few quick things to mention, since people seem mostly focused on the optional smoothing:

  1. This still is the only fix to ScrollContainer with a deadzone suddenly snaps/flickers on touch #45402 (Well, at least that I'm aware of)
  2. This makes scrolling on touch feel much closer to native (iOS/Android style) - The old/current method is very slippery, and even acts differently based on resolution
  3. @allkhor found an issue where scroll_ended is delayed when using a scroll bar. Seems like dragging the scroll bar is triggering the code that handles touch screen drag scrolling. This issue isn't actually unique to this PR, theres a slight delay even in the before vid; its just longer in this PR because of scrolling physics changes. This is actually an issue with the scroll_bar widget! (Could maybe be fixed by calling _cancel_drag from the bar when it's released, but that's for another PR to deal with...)

@Maran23
Copy link
Contributor

Maran23 commented Jan 3, 2023

@threethan can you rebase this branch to fix the merge conflicts?

@YuriSizov
Copy link
Contributor

I rebased it: #72072.
Feel free to test it and comment there.

@YuriSizov YuriSizov closed this Jan 25, 2023
@YuriSizov YuriSizov removed this from the 4.0 milestone Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet