Skip to content

Fix scroll to seek/change volume/playback rate changing too fast with touchpad #6666

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

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented Jan 29, 2025

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

Closes #6539

Description

wheel triggered N times more than mouse wheel with smaller delta values to have smooth scrolling
This PR throttles event handler on value change side only (prevent default/stop propagation still called all the time

Screenshots

With 100ms wait time, trackpad

Screen.Recording.2025-01-29.at.10.21.52.mp4

Testing

A) Those without trackpad
See #6539, test with mouse wheel ensure it works similar to/same as before

B) Those with trackpad
#6539

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

I tested 50ms wait and it seems too fast
(Too lazy to record that too

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 29, 2025 02:24
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 29, 2025
@PikachuEXE
Copy link
Collaborator Author

@efb4f5ff-1298-471a-8973-3d47447115dc

it seems slow but that could be because im testing on a trackpad

@PikachuEXE
Copy link
Collaborator Author

Feel free to change the 100ms values to something else to see how it feels
To me 50ms seems too fast (on macOS trackpad
No idea how other OSes handle trackpad scrolling

@efb4f5ff-1298-471a-8973-3d47447115dc

I let the others chime into this one as i find it difficult to evaluate what a good speed for this is

@PikachuEXE
Copy link
Collaborator Author

I rarely use trackpad not to mention scrolling
So same as you

Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@NgigaN

This comment has been minimized.

This comment has been minimized.

@efb4f5ff-1298-471a-8973-3d47447115dc

@absidue @ChunkyProgrammer @kommunarr your input is needed

Copy link
Contributor

github-actions bot commented Apr 1, 2025

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@ChunkyProgrammer
Copy link
Member

@absidue @ChunkyProgrammer @kommunarr your input is needed

I don't have a laptop so I can't really test this/suggest a good speed

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Apr 12, 2025
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@checktext00
Copy link

scrolling rate/speed is perfect with the new build,

but, there is a a bit of rubber-banding feeling with the scrolling (like I could scroll the volume from 50% to 100%, but it will settle on 95%), and also it doesn't always scroll smoothly, but that's not a major issue.

also there is still the lack of ability to reverse the scrolling direction for the following 3 settings in the FreeTube app settings > Player section:

  • Scroll Volume Over Video Player
  • Scroll Playback Rate Over Video Player
  • Skip by Scrolling Over Video Player

@efb4f5ff-1298-471a-8973-3d47447115dc

@PikachuEXE can you rebase into dev. Im having playback issues

@PikachuEXE PikachuEXE force-pushed the fix/trackpad-scroll-too-fast branch from 08f9b5e to 8007c4e Compare April 17, 2025 00:03
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@PikachuEXE
Copy link
Collaborator Author

@PikachuEXE PikachuEXE added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 17, 2025
Copy link
Contributor

github-actions bot commented May 2, 2025

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@efb4f5ff-1298-471a-8973-3d47447115dc

@PikachuEXE can you rebase into dev. Im having playback issues

Probably best to rebase again and provide a new build

@PikachuEXE PikachuEXE force-pushed the fix/trackpad-scroll-too-fast branch from 8007c4e to c96f0a8 Compare May 6, 2025 07:11
@PikachuEXE
Copy link
Collaborator Author

Rebased and build started: https://github.com/PikachuEXE/FreeTube/actions/runs/14853765114

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented May 6, 2025

I do have mixed feelings about this PR. It makes the UX with a scroll wheel objectively worse but makes trackpad more manageable.

I've tested this with trackpad again on nightly and with this change. The before doesnt seem that bad. It requires some getting used to because the swiping gestures have to be made slower in order to decrease/increase the volume by a bit.

I think i would choose having a bad trackpad over a bad scrollwheel + the fact that only one user reported this makes me think that this isnt that big of a deal

Not blaming the implementation here but this seems to be a case of pick the lesser evil one. if no acceptable middle ground can be found then we should close this

@absidue
Copy link
Member

absidue commented May 6, 2025

The way our mouse volume/skip scrolling currently works is by using hardcoded jumps for each event, so one event equals:

  • 5% volume
  • default skip interval * playback rate
  • 0.05x playback rate

On trackpads scrolling on a page is generally smoother whereas with a mouse it tends to jump a bit. Unconfirmed theory: Trackpads send events more frequently but each event is for a smaller "scroll distance", mice send less frequently but each event has a larger "scroll distance".

This pull request currently addresses the too large jumps on touch pads by ignoring various events. If my theory above is correct we may instead be able to base the increases/decreases on the distance that was scrolled (deltaX, deltaY and deltaMode) instead of hardcoded values (we would likely have to normalise the values a bit instead of using the raw "scroll distance"), that way it would hopefully even out/make it more similar for mice and trackpads.

@PikachuEXE
Copy link
Collaborator Author

The values in event.deltaY are +/-120 via wheels and +/-1 in trackpad
Problem is that the no. of events is too big to normalize the value changed (e.g. can't change volume by 0.1%... 1% per event = still too fast

Let me see if I can make it only use throttle when delta is small enough (and verify if what I mentioned above is correct, forgot

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels May 7, 2025
Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

Copy link
Contributor

github-actions bot commented Jun 6, 2025

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

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.

[Bug]: two-finger scrolling issues with touchpad
6 participants