Reverse horizontal scroll direction

[emilk]

EDIT: previously the PR only changed the direction on Mac, now I aim to changed it for all platforms.

With this PR, positive scroll values indicate that the content (that which is being moved when scrolling) should move RIGHT and DOWN, i.e. following the common GUI coordinates (used by winit elsewhere) where the X axis increases to the right, and the Y axis increases downwards.

With this PR we now match what GLFW does: https://github.com/glfw/glfw/blob/53d86c64d709ff52886580d338d9b3b2b1f27266/src/cocoa_window.m#L614:L627 https://github.com/glfw/glfw/blob/53d86c64d709ff52886580d338d9b3b2b1f27266/src/win32_window.c#L935-L941

This will remove the need for a workaround in my egui crate at https://github.com/emilk/egui/blob/master/egui-winit/src/lib.rs#L457

This is a breaking change to make winit coordinates consistent. It also adds documentation so winit users actually know what the scroll values mean.

• Tested on all platforms changed
• Compilation warnings were addressed
• cargo fmt has been run on this branch
• cargo doc builds successfully
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

[madsmtm]

Linking the previous issue #1695 and PR #1696 that introduced the current behaviour.

Tagging author of that PR @michaelkirk, I don't feel experienced enough to either approve or disapprove this change.

[madsmtm]

Sorry, didn't see the linked windows PR #2101. Does it make sense to merge this before that, or should they really be done together?

And do you know how the other platforms fare in this?

[michaelkirk]

It was a while ago that I worked on this, so unfortunately most of the details are now lost on me. I'll say though that before this change, the scrolling behavior in my apps matches other system apps (e.g. like Safari).

So that after this change, horizontal scrolling in my apps becomes undesirably reversed.

e.g. if you want to try one of the applications I work on...

git clone https://github.com/a-b-street/abstreet.git
cd abstreet
cargo run --bin widgetry_demo

Shrinking the window horizontally narrow enough should allow you to scroll the overflow horizontally:

Screen.Recording.2022-01-01.at.8.41.17.PM.mov.mp4

---

So if this change is working for people, there must be something wrong with how I'm handling winit events, which I'm very willing to accept, but haven't found my mistake yet.

I'm aware of egui, but I haven't personally used it. I was trying to test out horizontal scrolling in egui, but couldn't get the feature to work. Can someone more familiar with egui point me to an example?

Here's some things I tried to get horizontal scrolling to occur in egui, but none of them seemed to induce horizontal scrollability: https://github.com/michaelkirk/egui/tree/mkirk/hscroll-demo

[parasyte]

I was trying to test out horizontal scrolling in egui, but couldn't get the feature to work. Can someone more familiar with egui point me to an example?

The egui_demo_app has a lot of places where horizontal scroll can be used:

• "Code Example" window; shrink the window so the scroll bar appears
• "Context menu"; the plot is scrollable in both directions
• "Plot"; this plot is also scrollable in both directions
• "Widget Gallery"; has another scrollable plot
• "Color" tab; resize the entire app window and you can get a horizontal scrollbar to show here

I just personally tested this, and horizontal scroll works appropriately on Windows 11 (with a tilt wheel) and macOS (trackpad) despite this PR and #2101 because egui-winit reverses the horizontal scroll direction on both platforms: https://github.com/emilk/egui/blob/b1fd6a44e82c8cde7fab0b952851d2d2fd785349/egui-winit/src/lib.rs#L456-L463

[msiglreith]

As mentioned in the other thread: it seems to me (but I can't test it..) that the current behavior follows the documented coordinate system for the scroll delta values https://docs.rs/winit/latest/winit/event/enum.MouseScrollDelta.html#variant.LineDelta (up-right), while the more common coordinate system appears up-left/down-right which also aligns more with the window coordinates.

If that's the case it should be treated as breaking change and the documentation being updated as well.

[emilk]

The common GUI coordinates have the origin (0,0) in the top left corner, and having X and Y increase to the right and down. So if I move my fingers on a trackpad down and to the right, I should get positive deltas. With this PR, I get just that on my Mac (with the default "natural" scrolling, i.e. the same scrolling as on a touch screen).

[emilk]

I've updated the example, documentation, and made this a breaking change in the changelog. I've also cherry-picked the commit e9fe74e from #2101 so that this PR now reverses horizontal scrolling on both mac and windows.

I need volunteers to test the behavior on Linux and web (or instruction on how to test it on web myself).

[madsmtm]

abstreet's widgetry is handling horizontal and vertical scrolling differently, so the issue is probably on their end (see also a-b-street/abstreet#831 that @michaelkirk submitted).

I've tested this on macOS, can approve that the mouse_wheel example worked weirdly before and now it works as it should.

Agree that we need to ensure this is correct on Linux and Web as well.

[madsmtm]

I don't have the setup to test winit on the web, but a quick test by comparing the sign of WheelEvent.deltaX with the sign of MouseScrollDelta does suggest that we should be inverting it (like we do with deltaY here):

winit/src/platform_impl/web/web_sys/event.rs

Lines 51 to 52 in 25ff30e

	let x = event.delta_x();
	let y = -event.delta_y();

[madsmtm]

@dabreegster tested this in a-b-street/abstreet#831 (comment), and that suggests that this is also reversed on Linux.

[emilk]

I've inverted hscroll on all five platforms. I believe they are now all correct and the same.

• Mac: I've tested extensively, and it now matches GLFW
• Windows: no matches GLFW, and matches experiences of egui users
• X11: now matches GLFW
• web_sys matches what I do in egui_web
• Wayland: according to this comment, the previous behavior was wrong, so it should now be correct!

In other words, I think this can be merged!

[madsmtm]

This is probably missing a change to the X11 backend:

winit/src/platform_impl/linux/x11/event_processor.rs

Lines 780 to 783 in 001fb7e

	// X11 vertical scroll coordinates are opposite to winit's
	ScrollOrientation::Vertical => {
	LineDelta(0.0, -delta as f32)
	}

Otherwise I approve, I'll try to test the WebAssembly backend, but I don't have the setup for testing the Linux implementation, @kchibisov and @maroider?

[emilk]

This is probably missing a change to the X11 backend:

Good catch! I reversed hscroll there too in 7c7953d

[michaelkirk]

I wanted to check in, since the scope of this PR has changed.

Originally the PR was "invert horizontal scrolling on mac", which by itself would have made winit inconsistent across platforms, which had me perplexed.

But then windows was inverted, and then X11 and wayland were inverted, and finally web_sys was inverted.

So that with these additional changes, the scope of the PR is now "invert horizontal scrolling in winit" (on every single platform).

A summary of the transformations being done on scroll delta's across platforms:

	before this PR (x,y)	after (x,y)
wayland	(➕,➖)	(➖,➖)
x11	(➕,➖)	(➖,➖)
mac	(➖,➕)	(➕,➕)
web_sys	(➕,➖)	(➖,➖)
windows	(➕,➖)	(➖,➖)

It's nice to no longer have mixed signs within a single platform (either both the x and y are inverted or neither), and the new units arguably do make more sense than the old (positive being "down and to the right").

I just wanted to try to make sure I understood the tradeoff being proposed: The cost of this is a one-time breakage of horizontal scrolling for every existing winit consumer. Right?

[madsmtm]

Thank you @michaelkirk for the summary!

Yes, the idea is to change the coordinate-system for scrolling to match the window position coordinate system (explained better by @msiglreith here).

And yes, this will break for downstream users, but if they were handling scroll directions properly before (like aabstreet is) the fix for them is most likely that they won't have to work with mixed signs for the deltas any more.

[maroider]

So if I'm reading this correctly, macOS has a coordinate system that's "+y = up", "+x = left", and then we transform that into "+y = down", "+x = right"?

[madsmtm]

Yes, macOS events have that coordinate system (macOS windows have "+x = right" and "+y = up", see this).

Source: The documentation for NSEvent -deltaX and NSEvent -deltaY says:

-1 for [...] right and 1 for [...] left.

-1 for [...] down and 1 for [...] up.

(So although not stated explicitly in the documentation for scrollingDeltaX and scrollingDeltaY, deltaX and deltaY link directly to them, so I think it's safe to assume that they work the same. Also, this matches results from testing)

[emilk]

@michaelkirk makes an interesting point: with this PR, winit will report negated scroll directions compared to what native libraries report, on all platforms but on Mac. This could perhaps be a bit surprising, and we could consider flipping this.

From my point of view, the most important thing is that we now have a coordinate system that is the same everywhere in winit (X=right, Y=down), but scroll values can be interpreted two ways: how the content should move, or how the view should move. There is no right or wrong here. In this PR we are committing ourselves to having the scroll direction indicate how the content should move. I can think of two good arguments for this:

A) it is less of a breaking change for winit: this PR flips the less-used horizontal scroll direction, instead of the more commonly used vertical scroll direction.
B) On touch screen and touch-pads with natural scrolling, the scroll direction corresponds to how your fingers move.

[dhardy]

From my point of view changing the direction of the x-axis of LineDelta makes sense, but it's just a coordinate system so no big deal. Is PixelDelta similarly affected?

[emilk]

From my point of view changing the direction of the x-axis of LineDelta makes sense, but it's just a coordinate system so no big deal. Is PixelDelta similarly affected?

Yes, this PR flips the X sign for both LineDelta and PixelDelta. It is all consistent.

[msiglreith]

Are there open points in this PR? (cc @madsmtm). Otherwise this looks good to go with the current implementation (according to the Mac axis definition).

[msiglreith]

thanks!