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

Line-based scaling for high precision scrolling #4694

Closed
xsrvmy opened this issue Feb 11, 2022 · 14 comments
Closed

Line-based scaling for high precision scrolling #4694

xsrvmy opened this issue Feb 11, 2022 · 14 comments

Comments

@xsrvmy
Copy link
Contributor

xsrvmy commented Feb 11, 2022

There is currently an inconsistency in kitty with regards to how low precision and high precision scrolling is handled. With low precision scrolling, the scale is based on the number of lines. With high precision scrolling, the scale is based on the number of pixel. This leads to a situation where the perceived scrolling speed changes for high precision devices when different display scalings are involved. This is especially annoying on wayland because all devices are reported as high precision, so a mouse tick will scroll different number of lines on different dpi screens.

I am proposing a new settings called something like scroll_pixel_per_line. When this setting is set, kitty would ignore touch_scroll_multiplier, and instead use this new setting to convert scrolling pixels per line, then apply wheel_scroll_multiplier. When the value of this new setting is set correctly, this means that mouse scrolling would work the same on wayland and x11. It would also be a better fix for #4680.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 12, 2022 via email

@xsrvmy
Copy link
Contributor Author

xsrvmy commented Feb 12, 2022

Actually there are two separate problems:

  1. On wayland everything is considered high resolution. This traces all the way back to glfw/wl_init.c setting the high resolution flag to 1. The wayland protocol does expose this information so should be assigned per event like on macOS instead.
  2. The scrolling for touchpads is applied as physical pixels, but they should be applied as scaled pixels.

divide the number of pixels reported by the OS by the scale of the OS Window

Do you mean by this the line height or the mouse scroll amount? The mouse scroll amount doesn't change on screens. The line height changes, but isn't that expected for hidpi?

where it isnt needed as far as I know.

Wouldn't that be because macOS renders everything to a HiDPI buffer and then downscales in a mixed DPI environment (at least that's what I've heard)? So scroll would still slower than it should be.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 12, 2022 via email

@kovidgoyal
Copy link
Owner

Also, I dont see where in the wayland protocol is there information about high resolution. Looking at https://wayland-client-d.dpldocs.info/wayland.client.protocol.wl_pointer_listener.axis.html all it says is
the value parameter is the length of a vector along the specified axis in a coordinate space identical to those of motion events

which i think means they are pixels.

@xsrvmy
Copy link
Contributor Author

xsrvmy commented Feb 12, 2022

https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_pointer

There is an axis_source event which tells you if it is a wheel, touchpad, etc. There is also an event for handling wheel steps separately. This requires a wl_seat version bump from 4 to 5 though. I got it working locally, but I'll have to check if any other changes are required to prevent crashes somewhere else.
BTW a bonus of wl_seat updating to 5 is that it makes it possible to implement acceleration.

The scrolling issue still affects touchpads actually.
But yeah, the first issue is the one that's causing me the most problems. I don't use my touchpad when I have a second monitor.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 12, 2022 via email

@xsrvmy
Copy link
Contributor Author

xsrvmy commented Feb 12, 2022

It is possible to use axis_discrete as well. This tells the client that the number of click of the mouse wheel. The next axis event along the same axis should be ignored in this case, because it is guaranteed to follow the axis_discrete event.

The axis_source event is actually a bad way of doing things now that I read the spec more carefully, because it's optional.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 12, 2022 via email

@xsrvmy
Copy link
Contributor Author

xsrvmy commented Feb 12, 2022

xsrvmy@40bf12a

Went ahead and wrote the code for it. If it looks good I'll send a PR for it.

@kovidgoyal
Copy link
Owner

Form the spec you have to match axis numbers and there can be multiple
axis events before the paired one

@xsrvmy
Copy link
Contributor Author

xsrvmy commented Feb 12, 2022

you have to match axis numbers

I'm already handling x and y axis separately.

there can be multiple axis events before the paired one

I can make the x and y variables counters instead.
This could technically still fail because spec isn't actually clear if events from multiple devices can occur in one frame. I checked the source of some other libraries that use axis_discrete (xwayland, qt5, winit aka the one alacritty uses) though and all of them basically assume that only one pointer source occurs in one frame. Winit even straight up ignores all continuous input in the same frame as discrete input.

My main concern about my code is actually the use of global static variables.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 12, 2022 via email

@xsrvmy
Copy link
Contributor Author

xsrvmy commented Feb 12, 2022

I sent in PR #4699

Should we keep this issue open for the scaling issue with touchpad scrolling, or should I create a new issue for that?

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 13, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants