On 6/4/19 02:11 , Simon Ser wrote:
On Wednesday, April 3, 2019 5:36 AM, Peter Hutterer <[email protected]> 
wrote:
I've been trying to sort out the new hi-res wheel scrolling that was added to
linux 5.0 but it's been a bit of a struggle to say the least. For the
impatient, skip forward to the protocol diff but the background info is
important.

Thanks for taking the time to write this big summary. It's pretty helpful!

Everything below applies equally to horizontal and vertical scrolling, both
directions and wheels and wheel tilts.

The APIs that are involved for low-resolution wheel scrolling:

kernel:
- REL_WHEEL for every wheel click

libinput uses pointer axis events for wheels:
- pointer axis value: the movement of the wheel in degrees
- pointer axis discrete value: the number of wheel clicks
- pointer axis source: set to 'wheel'

Usally this means you get discrete 1, value 15 for the standard 15-degree mouse
wheel, discrete 2 value 30 is two wheel clicks within one hw frame.

There is one implicit assumption here: the discrete value must
be 1 or more for every event. Otherwise it's impossible to use the value and
calculate the angle. More on that later.

This API is effectively the same in the wayland protocol except for the unit of
the 'value' which is is in screen coordinates, not degrees.

wayland protocol:
- pointer axis value: the movement in screen coordinates
- pointer axis discrete: the number of wheel clicks
- pointer axis source: set to 'wheel'

The big difference here though: libinput's API is per-device, wayland's API is
per wl-seat pointer. There's no way to tell in the protocol that you're
scrolling simultaneously with wheels on two different mice.

Let's see how the common compositors implement this:

weston:
- for source wheel, only the discrete value is handled and:
- pointer axis discrete is sent as-is from libinput
- pointer axis source is sent as-is from libinput
- pointer axis value is always (discrete * 10)
- otherwise the libinput pointer value (degrees) from a source wheel is ignored

The magic multiplier 10 is afaik for historical reasons, weston used to do 10
so early libinput took that. When libinput switched to use physical dimensions
instead of 10, that value was kept for backwards compatibility with existing
wayland clients.

mutter:
- for source wheel, only the discrete value is handled and:
- clutter-internal event for the discrete **direction** from libinput
   - i.e. discrete > 1 will still only generate one event,some events
     drop to the floor here for fast wheel movements
- clutter-internal event with (value * 10) marked as 'emulated' but
   that event is ignored later when we're writing to the protocol
- the discrete event is written as wl_pointer.axis_discrete and
   wl_pointer.axis with a value of 10

So, the same multiplier of 10 but fast scroll events with more than one click
per frame appear to get dropped.

kwin (through qtwayland):
- doesn't handle discrete at all afaict
- pointer axis events are passed through but I got a bit confused over the
   actual value written on the wire here.

wlroots:
- pointer axis discrete is sent as-is from libinput
- pointer axis source is sent as-is from libinput
- pointer axis value is sent as-is from libinput

Because the physical angle is passed on from libinput as screen coordinates,
this means wlroots has different scroll behaviour to GNOME/sway and it does
scroll a larger distance for wheels with higher click angles. This is the
opposite of the intended behaviour, manufacturers advertise these wheels
as slow-scrolling wheels. Realistically though the physical movements is the
same-ish and if a client uses discrete it doesn't matter anyway.

Hmm, that's interesting. We wlroots devs have had assumed you'd want a given
angle to always scroll the same amount of pixels. But this doesn't matter that
much because we have a configuration option for the scroll multiplier anyway.
We still should have good defaults of course.


first, apologies if i got some of the details wrong, I hope the overall summary is still correct enough :)

the thing with the angle in this case is that you cannot really decide that angle. The wheel forces you to whatever it's built as and that's it (more recent high-res wheels excepted). Even the earlier free-floating wheels still only send events every X degrees. Users (and manufacturers) thus don't think as angles as much as wheel clicks. A 20 deg wheel feels slower than the standard 15 deg wheel simply because you have to scroll more to get one click. And that's what's advertised too.

The angle == pixels mapping makes sense for a continuous axis, I'm not sure it's as useful for the wheel as we have them. Then again, it's relatively niche and only a small subset of users notice and/or care about this particular issue.

xf86-input-libinput:
- X devices need to set up a scroll distance for scroll motion, the driver
   uses 120 (since Feb, before it was 15). This means any multiple of 120.0
   triggers legacy button emulation in the server, otherwise the number has no
   meaning.
- for source wheel it uses the discrete value only, so effectively 120 *
   discrete.

This driver had a division by 0 for discrete values of 0, fixed in the
current release. More on this later.



Let's look at how wheel events are processed in some of the clients:

GTK:
- for a discrete event, GDK_SCROLL_UP is passed on flagged as 'emulated'
- the axis value is passed on as GDK_SCROLL_SMOOTH

Qt:
- doesn't handle discrete
- multiplies the value by -12. This gives it a 120-based value which
   matches the Windows API provided the axis value is 10 of course.
   Qt on wlroots is probably off here.

Xwayland:
- X devices need to set up a scroll distance for scroll motion, Xwayland
   currently uses 1.0 for that. This means any multiple of 1.0 triggers legacy
   button emulation in the server.
- wl_pointer.axis_discrete are used where available
- wl_pointer.axis uses the value * 0.1. This effectively means Xwayland
   relies on the deltas to be 10 like in mutter/weston, X clients under
   wlroots will have off wheel click emulation.

Xwayland uses axis_discrete when available, so wlroots should be fine (we sent
the patch doing that when we decided not to implement axis like Weston IIRC).

yep, this must've been scrambled notes by me. xwayland uses discrete but where discrete is missing, the axis value is missing it uses the 0.1. So if we were to drop axis discrete and rely on axis only, the 10 vs $click-angle mismatch will cause weird effects.


With me so far? Hooray, now let's introduce hi-res scrolling.

All the examples below are for a quarter scroll wheel movement, i.e. you get
4 hi-res events for every 1 lo-res event. The kernel uses a fraction/multiple
of 120 for logical clicks, same as the Windows API. So one wheel click is 120,
half a click is 60, etc. I'm calling this 120-based value the v120 because I'm
very creative.

kernel:
- REL_WHEEL for every wheel click
- REL_WHEEL_HI_RES for every fraction of a wheel click (as portion of 120),
   so a quarter-wheel stop gives you a value of 30.
   - there is no guarantee that REL_WHEEL is sent every full 120 because e.g.
     Logitech mice may reset halfway through a wheel motion. So the two axes
     must be considered completely independent.

libinput:

Basically: the current API is fairly useless in handling fractional scroll
because libinput uses physical distances. libinput could send four events:
- value 3.75, discrete 0
- value 3.75, discrete 0
- value 3.75, discrete 0
- value 3.75, discrete 1

Which is technically correct. But callers have no reliable way how much of a
fraction of a wheel the 3.75 represents until the first discrete event comes
in. Guessing is unreliable, if you scroll faster you may get 2+ fractional
units in one event. And mice may reset halfway through a scroll motion so you
don't always get the same number of events per discrete. So libinput needs some
extra API that gives us some baseline to compare values against, i.e. same as
the kernel's 120-based API does.

There are other problems, specifically related to sending discrete 0 events:

weston:
- Current weston: a discrete value of 0 ends up as value of 0 which weston
   treats as axis_stop event. So our event sequence is axis_source,
   axis_stop, pointer_frame. this breaks scrolling.

mutter:
- Current mutter: a discrete value of 0 ends up in noops.
   mutter still generates internal clutter events but they are never pushed
   onto the wire. The normal values are ignored, so we basically
   get low-res scrolling just as before.
   Note: this is based on reading the code, not test runs

wlroots:
- will forward value/discrete as it comes in from libinput

We're in the same boat as Weston here: the special value 0 will be sent as
axis_stop.


kwin:
- doesn't use discrete so won't be affected by changes

xf86-input-libinput:
- anything pre 0.28.2 will get a division by 0
- current state: discrete values of 0 end up as noops and we we get lo-res
   scrolling only

So basically: if we change libinput to send discrete 0 events, we'll break
weston and all but the most recent xf86-input-libinput. Code changes are needed
just to cope with the different event sequence, let alone the actual hi-res 
bits.

Agreed.

The solution to this is to add a new event, LIBINPUT_EVENT_POINTER_AXIS_WHEEL.
That event provides libinput_event_pointer_axis_value_v120() which is basically
a mirror of the kernel API. This event obsoletes POINTER_AXIS events for
sources WHEEL and WHEEL_TILT, so callers should just ignore those if they
support the new event. A branch for this is available here:
https://gitlab.freedesktop.org/whot/libinput/commits/wip/hi-res-scrolling

The event sequence would now be:

- WHEEL: value 3.75, v120 30
- WHEEL: value 3.75, v120 30
- WHEEL: value 3.75, v120 30
- WHEEL: value 3.75, v120 30
- AXIS: value 3.75, discrete 1, v120 120

Implementation-wise: AXIS and WHEEL are independent, so there's no guarantee
that they add up to the same 120 values. This new event is easy to add to
compositors.

This makes sense to me. It's unfortunate to introduce a new API, but I guess as
discussed before there's no way around it.

Let's look at the Wayland protocol with the current API given hi-res scroll
events:
- value 3.75, no discrete event
- value 3.75, no discrete event
- value 3.75, no discrete event
- value 3.75, discrete 1

This is assuming 15 degrees/4 == 3.75 but if the compositor force wheels
to a scale of 10, the value would be 2.5.

Qt:
- doesn't use discrete so won't be affected by changes

GTK:
- generates 4 GDK_SCROLL_SMOOTH events,
- generates 1 GDK_SCROLL_DOWN event ('emulated')
- GTK doesn't really care as such about whether the distance-to-wheel is 10 or
   something else, it'll just have different scroll distances (e.g. under
   wlroots).

Afaict this works correctly (tested with stock F29 nautilus)

Xwayland:
- current Xwayland: values are handled as fraction as before, so we get 3
   smooth-scroll events at 25%. Event 4 has a discrete event which is handled
   as full event, so we scroll by a logical click. Total distance is 175%.

Hmm. Is a logical click 150%? I think I'm missing something here.

yeah, I wasn't clear enough. A full wheel would cause 3 smooth events and 1 discrete one, ending up in 25% + 25% + 25% + 100% = 175% total. If we were to keep the discrete at 0, it'd be 25% for the last event too and add up to the correct 100%.

But yeah, if we stop sending axis_discrete and continue sending axis, it'll
break clients.

Writing the above, now I'm wondering: if we deprecate axis_discrete and stop sending them, I think clients will keep working (they have to, axis-discrete were never guaranteed). So we *might* be able to replace the discrete events with something new like the v120 event and be good. Need to look into that.

This means: Xwayland cannot work at all without any extra wayland protocol
unless it assumes factor 10 is the base unit.


So we have at least one ubiquitous Wayland client that cannot handle it.
And the issues are very similar to the ones libinput's API has, added to that
is that you cannot guarantee that two events come from the same device.
What we need is a new event, but I'm struggling to add this to the wayland
protocol in a good way, mostly for backwards-compatibility reasons.


Much of the below would be easier to solve if we can say "if you claim to
support wl_pointer version 8, you get different behaviour for axis events". I
don't think we can trust clients to handle this correctly, I got burned by
those assumptions before (in XI2). So I'm ruling out changing any of the
existing protocol behaviour.

Hmm. We have to be careful -- anything we decide will be set in stone.

The protocol would look less ugly if we change wl_pointer's meaning in a new
version. (A similar change has already been done to the wl_data_offer API)
Though it's true that until we still have users of the old version we'll need to
be careful.

Basically: I'm assuming we will always have users of the old version for whatever value of old version. To expand on the XI2 case - we changed the behaviour there based on the version the client announces (XI2.1 or XI2.2, can't remember) and, a few years later, discovered that the same display connection was used by different parts supporting different values. So the toolkit assumed version X, the application assumed version X+1. Events compatible with X were forwarded to the toolkit, but if you change the behaviour, it'll break.


My current idea is to add a wl_pointer.axis_v120 event:

    wl_pointer.axis_source    wheel
    wl_pointer.axis_v120      30
    wl_pointer.frame
    wl_pointer.axis_source    wheel
    wl_pointer.axis_v120      30
    wl_pointer.frame
    wl_pointer.axis_source    wheel
    wl_pointer.axis_v120      30
    wl_pointer.frame
    wl_pointer.axis_source    wheel
    wl_pointer.axis           10
    wl_pointer.axis_discrete  1
    wl_pointer.axis_v120      30
    wl_pointer.frame

I can think of a few issues with this:

* Can the compositor still have a scroll multiplier setting? Is a 120
   granularity enough? (ie. what happens if the user has a very precise mouse 
and
   configures a scale multiplier of 0.5?)
   Maybe this value could be a wl_fixed and named e.g. axis_fraction?

The 120 is so deeply embedded in the windows API that I don't think it'll change anytime soon. And in the kernel now too. 120 is simply a magic value because it has lots of integer factors, and let's be honest: a factor of 120 would give you less than a degree granularity on your average scroll wheel, that's probably enough until mouse wheels go dodo.
That factor is also in the hardware. The highest so far I've seen is 16.

* What do we do about wl_pointer.axis? Maybe we can just phase it out? I'm not
   a fan of having to support two APIs forever. Plus the current axis event is
   ill-defined.

I don't think we can ever phase out wl_pointer.axis or change its meaning. All we can do is add to it to reframe its interpretation.

* Can't we just define a axis_full_turn event sent after binding to wl_pointer,
   which defines what is the amount of axis you need to wait for until you reach
   a full turn? (The axis event already carries a wl_fixed value)

The problem with this is that wl_seat multiplexes pointers. If you have two mice, the full turn may differ between the mice and you may be at a fraction of a full turn on any mouse at any time. It simply doesn't work, sorry (was my first idea too :)

Main problems here: this puts a fair bit of implementation requirements on the
compositor - skip source/v120/frame for clients < supported version.

Well this is inevitable anyway -- having a new protocol version will require
compositors to add more version checks.

wl_pointer.source cannot currently occur on its own, it requires a
wl_pointer.axis event in the current protocol. Same with axis_discrete. Empty
frames are allowed, I think, but pointless.

It also puts a bit of extra logic in the client, you need to remember if you've
seen a v120 in this frame and discard the axis/axis_discrete events. If not,
then axis events must be processed as normal.

A possible workaround would be to add v120 to all axis frames (including
touchpad scrolling) but that would then put the compositor in charge of
deciding what distance is a wheel click and encodes that information in the
protocol.

Side note, the current protocol already requires compositors to decide of an
axis event value for touchpads.

Not necessarily. libinput at least gives you touchpad events in the same coordinate space as the input, so forwarding that 1:1 is fine. The problem is converting that to wheel clicks, but that's done client-side.


The v120 event must also carry the equivalent to the wl_pointer.axis value so
that can be passed on into existing code. So the above would have a v120 event
with v120 value of 30 and an axis value of 2.5 (if enforcing 10 units per
wheel click).

Because the hi-res and lo-res sources are independent, we need to allow for a
0-value v120 event, which basically just tags that frame as

as low-res?

huh, yeah. copy/paste error? but yes, if you have a 0-value v120 event, that's a marker that "this source has high-res scrolling and if you're handling the v120, you can ignore the events in this frame"


The compositor also need to *always* send v120 events for wheel/tilt sources
because otherwise the client code becomes even more of a nightmare.
Where not backed by libinput the compositor needs to handle that itself.

Agreed.

Anyway, the below is the best I've come up with so far though I haven't
implemented it client-side yet. I'd really appreciate any feedback and/or
epiphanies.

Cheers,
   Peter

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index b1c930d..cd45407 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1662,7 +1662,7 @@
      </request>
     </interface>

-  <interface name="wl_seat" version="7">
+  <interface name="wl_seat" version="8">
      <description summary="group of input devices">
        A seat is a group of keyboards, pointer and touch devices. This
        object is published as a global during start up, or when such a
@@ -1771,7 +1771,7 @@

    </interface>

-  <interface name="wl_pointer" version="7">
+  <interface name="wl_pointer" version="8">
      <description summary="pointer input device">
        The wl_pointer interface represents one or more input devices,
        such as mice, which control the pointer location and pointer_focus
@@ -2092,9 +2092,97 @@
        <arg name="axis" type="uint" enum="axis" summary="axis type"/>
        <arg name="discrete" type="int" summary="number of steps"/>
      </event>

Nit: we generally add a

     <!-- Version 8 additions -->

comment here.

+    <event name="axis_v120" since="8">
+      <description summary="axis high-resolution scroll event">
+       Discrete high-resolution scroll information.
+
+       This event carries high-resolution wheel scroll information,
+       with each multiple of 120 representing one logical scroll step.
+       For example, a v120 of 30 is one-quarter of a logical
+       scroll step in the positive direction, the v120 of -60 is
+       one-half of a logical scroll step in the negative direction.
+       Clients that rely on discrete scrolling should accumulate the
+       v120's to multiples of 120 before processing the event.
+
+       This event also carries the axis motion value in the same coordinate
+       space as the wl_pointer.axis event's value. See the
+       wl_pointer.axis documentation for details.

Why do we need to duplicate this information?

I'm not 100% convinced on this yet but it allows a compositor to completely shortcut the other axis handling bits and just deal with the information in the v120 event. It's not necessary, but it makes the event more self-contained and the parsing slightly simpler. I had thought about adding the source to it too but decided against that, it didn't feel right.

Thanks for the feedback, much appreciated

Cheers,
  Peter


+       This event is guaranteed for axis movement from axis sources wheel
+       and wheel_tilt even if the underlying device does not support
+       high-resolution scrolling.
+
+       Where a wl_pointer.axis_source event occurs in the same
+       wl_pointer.frame, the axis source applies to this event.
+
+       Where wl_pointer.axis and/or wl_pointer.axis_discrete events
+       occur in the same wl_pointer.frame, those events reflect an
+       accumulated scroll amount. Clients handling axis_v120 events can
+       usually ignore the axis and axis_discrete events.
+
+       For example, a wheel generating events every 25% of each physical
+       wheel detent may create the following event sequence:
+
+         wl_pointer.axis_source wheel
+         wl_pointer.axis_v120 30
+         wl_pointer.frame
+         wl_pointer.axis_source wheel
+         wl_pointer.axis_v120 30
+         wl_pointer.frame
+         wl_pointer.axis_source wheel
+         wl_pointer.axis_v120 30
+         wl_pointer.frame
+         wl_pointer.axis_source wheel
+         wl_pointer.axis_v120 30
+         wl_pointer.axis 10
+         wl_pointer.axis_discrete 1
+         wl_pointer.frame
+
+       For backwards compatibility, the wl_pointer.axis event for a wheel
+       or wheel tilt must contain the value of the full wheel movement. It
+       must not be sent for a fraction of a physical detent.
+
+       For backwards compatibility, the wl_pointer.discrete event for a wheel
+       must contain the value of the full wheel movement. It must not be
+       sent for a fraction of a physical detent and it must not be 0.
+
+       For backwards compatibility, the wl_pointer.axis_source event in a
+       frame containing only a wl_pointer.axis_v120 event must not be sent
+       to clients that do not support wl_pointer version 8 or later. In
+       other words, the above sequence to such clients is reduced to:
+         wl_pointer.axis_source wheel
+         wl_pointer.axis 10
+         wl_pointer.axis_discrete 1
+         wl_pointer.frame
+
+       Clients must treat wl_pointer.axis_v120 and
+       wl_pointer.axis/axis_discrete as two independent event sources.
+       There is no guarantee that the steps indicated by the v120 events
+       match the steps by wl_pointer.axis_discrete events. In other words,
+       an accumulated v120 value of 360 does not guarantee an accumulated
+       axis_discrete value of 3.
+
+       The value of a wl_pointer.axis_v120 event may be 0.
+       This is the case where a low-resolution event has no equivalent
+       high-resolution event. The value of 0 indicates that any
+       wl_pointer.axis/axis_discrete events in the same event can be
+       ignored.
+
+       The axis motion's coordinate space is identical to the
+       wl_pointer.axis event.
+
+       The order of wl_pointer.axis_v120 and wl_pointer.axis_source is
+       not guaranteed.
+      </description>
+      <arg name="time" type="uint" summary="timestamp with millisecond 
granularity"/>
+      <arg name="axis" type="uint" enum="axis" summary="axis type"/>
+      <arg name="motion" type="fixed" summary="length of vector in surface-local 
coordinate space"/>
+      <arg name="v120" type="int" summary="scroll distance as fraction of 
120"/>
+    </event>
    </interface>

-  <interface name="wl_keyboard" version="7">
+  <interface name="wl_keyboard" version="8">
      <description summary="keyboard input device">
        The wl_keyboard interface represents one or more keyboards
        associated with a seat.
@@ -2208,7 +2296,7 @@
      </event>
    </interface>

-  <interface name="wl_touch" version="7">
+  <interface name="wl_touch" version="8">
      <description summary="touchscreen input device">
        The wl_touch interface represents a touchscreen
        associated with a seat.

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to