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

Trackpad scrolling behaviour on OSX is wrong. Both bounce and directionality. #16442

Open
trishume opened this issue Apr 13, 2017 · 4 comments
Open
Labels

Comments

@trishume
Copy link

@trishume trishume commented Apr 13, 2017

Playing around with Servo on OSX with a MBP trackpad and the number one thing I noticed was how broken scrolling is. I noticed a number of issues:

  • Vertical scrolling on a page that also has horizontal scrolling is very difficult. Any amount of diagonal motion leads to hitting the sides, and the scroll bounce when hitting the sides also kills vertical motion. Edit: This is mostly fixed by #16498 but that doesn't properly allow diagonal scrolling at low speeds.
    • Servo should do what Cocoa and Chrome do which is lock the scroll direction to either vertical or horizontal (whichever it is closer to) except at very slow speeds. In Cocoa and Chrome it is only possible to scroll diagonally when scrolling very slowly. So slow that it doesn't trigger scroll bounce so I can't find out if Cocoa scroll bounce kills vertical velocity.
    • Alternatively, you can do what Sublime Text does, and allow diagonal scrolling at any speed, but preserve velocity in the other direction when hitting an edge (Sublime scrolling doesn't bounce).
    • Allow diagonal scrolling at low speeds like Cocoa
  • Scroll bounce feels wrong. It feels so bad that I prefer Sublime Text's total lack of it to the bouncing in Servo. This is due to a number of things:
    • It's inconsistent: Sometimes it bounces a huge distance and sometimes it bounces very little, on scrolls that subjectively feel very similar. I'm guessing this might have something to do with logic based on scroll events and event distances rather than an actual estimate of velocity, like described in https://pavelfatin.com/scrolling-with-pleasure/.
    • Sometimes it hangs at the top: when scrolling quickly and hitting the top of the page, sometimes (seemingly nondeterministically) it hangs motionless at the top of the bounce for 200ms or so, which looks really weird and unnatural. This may be due to the same dependency on exact scroll event timings and sizes as the previous point.
    • They seem too small: This may just be an artifact of the previous two points, but it also feels like the bounces are substantially smaller in distance than the bounces for an equivalent scroll velocity in Cocoa or Chrome.
  • They totally break when using a scroll wheel. This is already filed as #11985
  • It doesn't actually scroll link hitboxes. This is already filed as #16189
@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Apr 17, 2017

Servo should do what Cocoa and Chrome do which is lock the scroll direction to either vertical or horizontal (whichever it is closer to)

Hmm, this used to work:

fn scroll_window(&self, scroll_location: ScrollLocation, phase: TouchEventType) {

As for the bouncing, see #9982

@jdm jdm added A-input P-mac labels Apr 17, 2017
@trishume
Copy link
Author

@trishume trishume commented Apr 17, 2017

@paulrouget interesting, I don't see why that code wouldn't work, but I just double checked and with the nightly of the date this issue was submitted, I can totally scroll at a 45 degree angle or any other direction at high speeds.

Also that code if it worked wouldn't match Cocoa behaviour since it allows scrolling in any direction at very low speeds.

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Apr 17, 2017

Yeah. There's a regression.

As for the more precise behavior (snapping only on high velocity and proper bounce), we can try to reproduce this ourself, or rely on the platform to only provide us with already post-processed events. Not sure if the latter is possible though.

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Apr 17, 2017

Hmm, the delta is probably copied. And the fact that scroll_location is not mutable is a red flag. I'll do a PR to at least address that.

bors-servo added a commit that referenced this issue Apr 17, 2017
Properly modify scroll_location

As described in #16442, scroll orientation is not locked. `delta` was copied.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16498)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.