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

Fix scrolling in code editor when using macbook trackpad #82260

Closed
wants to merge 4 commits into from

Conversation

BoydWiglaf
Copy link

This is motivated my #28149 Script editor doesn't track scrolling via Touchpad smoothly (or at all when scrolling is slow).
This was a PR before by Kurble with PR #73502. It seems reviewers were good with the changes but when it came ready to be pulled in the branch had some merge conflicts. I made the changes in an updated forked branch.

---- Copied from #73502 ----
This PR fixes two closely related problems:

Scrolling slowly in the code editor with a track pad would not do anything;
Smooth scrolling would animate track pad scrolls, but they are already animated as the pixel deltas are provided on a frame-by-frame basis.
The first problem is caused by the step size that's set on the VScrollBar, any scroll event going through TextEdit::_scroll_up and TextEdit::_scroll_down got rounded down to an integer number of lines. Since scrolling on a track pad happens every frame, the individual deltas are relatively small, and they would be rounded down to zero unless you're scrolling quite fast. This is addressed differently for the smooth scrolling enabled and disabled case:

When disabled the scrolling deltas are accumulated the same way they were already being accumulated for the smooth scrolling case, so when slow scrolling for long enough the step size threshold is reached;
When smooth scrolling is enabled the step size is set to 0, allowing all fractional increments.
The second problem becomes quickly apparent when the first problem is fixed: scroll delta's less than 1 line are applied immediately, while scroll delta's larger than 1 line are animated. This feels really snappy as long as the target scroll value is less than 1 line away from the current scroll value, only to suddenly change to a fixed speed when the delta becomes too big. This has been addressed by directly applying any scroll delta coming from InputEventPanGesture events, instead of allowing it to animate.
---- end of copy ----

@BoydWiglaf
Copy link
Author

This fix will work in godot3.x as well.

@AThousandShips
Copy link
Member

AThousandShips commented Sep 24, 2023

Since you are copying the code from #73502 you will need to add a co-authorship to this PR

Get the author identity from the log of the original pr commit, and add it like so:


Co-authored-by: Name <Email>

An empty line after your commit message is required

@AThousandShips
Copy link
Member

I am unsure however why this PR was made when the original wasn't abandoned, if the original author is active and revives their PR it will be merged instead of this

@BoydWiglaf
Copy link
Author

I am happy to have the original PR go through. I only created this one because I had not seen activity on the other one for several months from Kurble. I really am only interested in seeing these changes pulled in.
I am not very familiar with how PR work as I am new to this. So I could be completely going about this the wrong way. I will try messaging Kurble and see if he Is able to bring his PR up to date with the current master branch. If not I will add him to this one.
Thanks for all the info.

@AThousandShips
Copy link
Member

You didn't quite follow the instructions for co-author:

An empty line after your commit message is required

@AThousandShips
Copy link
Member

In the future please note that opening a salvage PR should only be done after checking that the original author is not able or willing to do the work, a simple question on the PR is enough

@BoydWiglaf
Copy link
Author

BoydWiglaf commented Sep 25, 2023

I tried putting an empty line but i did it in command line. it looks like the terminal took out my empty line. In researching it looks like I needed to put in an additional -m.
git commit -m "description" -m "Co-authored-by: Name email
Is that correct? Is there a way I can correct this now?

@AThousandShips
Copy link
Member

You need to edit it not by the command line message

You should also squash your commits into one

But no rush, wait until it's confirmed that the original author has abandoned this

@AThousandShips
Copy link
Member

The original author has updated their PR so this is now not needed

Thank you for your contribution nonetheless

@AThousandShips AThousandShips removed this from the 4.x milestone Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants