RFC: Wayland high-resolution wheel scrolling

Submitted by Peter Hutterer on April 3, 2019, 2:36 a.m.

Details

Message ID 20190403023619.GA5351@jelly
State New
Headers show
Series "RFC: Wayland high-resolution wheel scrolling" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Peter Hutterer April 3, 2019, 2:36 a.m.
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.

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.

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.

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

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.



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.




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%.

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.


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

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

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 

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.

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

Patch hide | download patch | download mbox

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>
+
+    <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.
+
+	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.

Comments

On 4/3/19 5:36 AM, Peter Hutterer wrote:
> 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.

Small correction: KWin doesn't use QtWayland (neither directly nor 
indirectly as far as I know); instead it uses KWayland.

> kwin:
> - doesn't use discrete so won't be affected by changes
It's worth to mention that there is a patch to make KWin forward 
value/discrete as they come from libinput and as with the rest of 
compositors value of 0 would be interpreted as axis_stop.

Cheers,
Vlad
On Wednesday, April 3, 2019 5:36 AM, Peter Hutterer <peter.hutterer@who-t.net> 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.

> 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).

> 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.

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

> 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.

> 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?
* 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.
* 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)

> 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.

> 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?

> 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?

> +	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
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
On 6/4/19 02:11 , Simon Ser wrote:
> On Wednesday, April 3, 2019 5:36 AM, Peter Hutterer <peter.hutterer@who-t.net> 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
>> wayland-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel