[RFC PATCH v2 wayland] protocol: add wl_pointer.axis_source, axis_stop and axis_discrete events
Peter Hutterer
peter.hutterer at who-t.net
Mon Apr 6 20:19:24 PDT 2015
On Fri, Mar 27, 2015 at 10:23:43AM +0800, Jonas Ådahl wrote:
> On Fri, Mar 27, 2015 at 11:04:42AM +1000, Peter Hutterer wrote:
> > On Wed, Mar 25, 2015 at 09:14:08AM +0800, Jonas Ådahl wrote:
> > > On Wed, Mar 25, 2015 at 10:27:10AM +1000, Peter Hutterer wrote:
> > > > The axis_source event determines how an axis event was generated. This enables
> > > > clients to judge when to use kinetic scrolling.
> > > >
> > > > The axis_stop event notifies a client about the termination of a scroll
> > > > sequence, likewise needed to calculate kinetic scrolling parameters.
> > > >
> > > > The axis_discrete event provides the wheel click count. Previously the axis
> > > > value was some hardcoded number (10), with the discrete steps this enables a
> > > > client to differ between line-based scrolling on a mouse wheel and smooth
> > > > scrolling with a touchpad.
> > > >
> > > > We can't extend the existing wl_pointer.axis events so we introduce a new
> > > > concept: latching events. These events (axis_source and axis_discrete)
> > > > are prefixed before a wl_pointer.axis or axis_stop event, i.e. the sequence
> > > > becomes:
> > > >
> > > > wl_pointer.axis_source
> > > > wl_pointer.axis
> > > > wl_pointer.axis_source
> > > > wl_pointer.axis
> > > > wl_pointer.axis_source
> > > > wl_pointer.axis
> > > > wl_pointer.axis_source
> > > > wl_pointer.axis_stop
> > > >
> > > > or:
> > > >
> > > > wl_pointer.axis_source
> > > > wl_pointer.axis_discrete
> > > > wl_pointer.axis_axis
> > > >
> > > > Clients need to combine the state of all events where needed.
> > > > ---
> > > > Changes to v1:
> > > > - introduce axis_stop and axis_discrete as separate events instead of flags
> > > > - couple of documentation updates
> > > > - wl_seat/keyboard/touch/pointer bumped to version 5
> > > >
> > > > This is for the API review only so far, I don't have the weston patches to
> > > > match yet.
> > > >
> > > > protocol/wayland.xml | 87 +++++++++++++++++++++++++++++++++++++++++++++++++---
> > > > 1 file changed, 83 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > > index c5963e8..29e6f01 100644
> > > > --- a/protocol/wayland.xml
> > > > +++ b/protocol/wayland.xml
> > > > @@ -1337,7 +1337,7 @@
> > > > wl_seat.get_keyboard or wl_seat.get_touch, the returned object is
> > > > always of the same version as the wl_seat.
> > > > -->
> > > > - <interface name="wl_seat" version="4">
> > > > + <interface name="wl_seat" version="5">
> > > > <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
> > > > @@ -1411,7 +1411,7 @@
> > > > </interface>
> > > >
> > > > <!-- for the version see the comment in wl_seat -->
> > > > - <interface name="wl_pointer" version="3">
> > > > + <interface name="wl_pointer" version="5">
> > > > <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
> > > > @@ -1572,10 +1572,89 @@
> > > > <description summary="release the pointer object"/>
> > > > </request>
> > > >
> > > > + <!-- Version 4 additions: none -->
> > >
> > > Usually don't tend to add these it seems. If we'd want to be consistent,
> > > we'd have to add comments like this in several other places.
> >
> > ACK, removed locally.
> >
> > >
> > > > +
> > > > + <!-- Version 5 additions -->
> > > > +
> > > > + <enum name="axis_source">
> > > > + <description summary="axis source types">
> > > > + Describes the axis types of scroll events.
> > > > + </description>
> > > > + <entry name="unknown" value="0"/>
>
> Should we really send events that has an unknown source? How would the
> client interpret it? If we want to change the source from 'unknown' to
> some new source, wouldn't that break backward compatibility?
ACK, removed locally.
> > > > + <entry name="wheel" value="1"/>
> > > > + <entry name="finger" value="2"/>
> > > > + <entry name="continuous" value="3"/>
> > > > + </enum>
> > > > +
> > > > + <event name="axis_source" since="5">
> > > > + <description summary="axis source event">
> > > > + Scroll and other axis source notification.
> > > > +
> > > > + This event does not occur on its own. It is sent before a
> > > > + wl_pointer.axis or wl_pointer.axis_stop event and carries the source
> > > > + information for that event. A client is expected to accumulate the
> > > > + data in both events before proceeding.
> > > > +
> > > > + The axis and timestamp values are identical to the one in the
> > > > + subsequent wl_pointer.axis or wl_pointer.axis_stop event. For the
> > > > + interpretation of the axis value, see the wl_pointer.axis event
> > > > + documentation.
> > > > +
> > > > + The source specifies how this event was generated. If the source is
> > > > + finger, a wl_pointer.axis_stop event will be sent when the user
> > > > + lifts the finger off the device.
> > > > + </description>
> > > > + <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> > >
> > > Why have a "time" parameter here, considering that this state does
> > > nothing on its own and which data should simply be just kept around until
> > > the committing event is received? Feels redundant.
> > >
> > > > + <arg name="axis" type="uint"/>
> > >
> > > This one seems also redundant.
> >
> > both removed
> >
> > > > + <arg name="axis_source" type="uint"/>
> > > > + </event>
> > > > +
> > > > + <event name="axis_stop" since="5">
> > > > + <description summary="axis stop event">
> > > > + Scroll and other axis stop notification.
> > > > +
> > > > + For some wl_pointer.axis_source type, a wl_pointer.axis_stop event
> > > > + is sent to notify a client that the axis sequence has terminated.
> > > > + This enables the client to implement kinetic scrolling.
> > > > + See the wl_pointer.axis_source documentation for information on when
> > > > + this event may be generated.
> > > > +
> > > > + Any wl_pointer.axis events after this event should be considered as
> > > > + the start of a new axis motion.
> > > > +
> > > > + The axis and timestamp values are to be interpreted identical to the
> > > > + ones in the wl_pointer.axis event.
> > > > + </description>
> > > > + <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> > > > + <arg name="axis" type="uint"/>
> > >
> > > Same as axis_source::time and axis_source.axis.
> >
> > the axis_stop event stands on its own and is not followed by an axis event,
> > it needs those two values.
>
> Ah right, thought it was to be coupled with axis length 0 but didn't
> read carefully enough, I suppose having it on its own is better.
>
> Follow up questions:
>
> Do we expect to get axis_stop on both the horizontal and vertical axes,
> and is the client supposed to trigger its kinetic scrolling when both
> axises has stopped or just the first one? How would one interpret only
> one axis receiving the stop event but not the other?
I'm not sure here tbh. I think it'd be good enough to say that this affects
all scroll directions currently active, but that's based on a couple of
conditions:
* the case of kinetic scrolling on two devices independently is a niche case
we don't care about. that's easy
* we only have scroll axes so far, if we add unrelated axes to the
wl_pointer.axes set this gets a bit hairy. e.g. does a kinetic scrolling
stop affect a future rotation axis?
the solution here could be to add flags to the stop event to specify the
axes that stopped scrolling.
> Will we ever send axis_stop when the source is 'continuous' (or any
> other possibe source), and how to determine what source had its movement
> stopped in that case? With the current version, "axis_stop" is rather
> hard coded to "axis_finger_lifted" behind a generic name (which maybe is
> not that bad anyway though).
I'll have to double check the code but iirc we send axis stop events in
libinput for some continuous scrolling. e.g. trackstick middle button
scrolling - releasing the middle button sends the scroll termination event.
we don't have a pre-notification in libinput though, so you won't know until
you get that stop event that you will get one. That's fixable but whether
it's needed is another matter: if you get it you'll get it fast enough to
trigger kinetics, otherwise you time out (and the stop event may come in
after the timeout).
the axis_source is still prefixed to the axis_stop event, but that doesn't
scale if you have to independent scroll devices that use the same type.
Again, a question of whether that's required, if so we could assign some
sort of tracking ID to the scroll sequence like the touch sequences
have.
> > > > + </event>
> > > > +
> > > > + <event name="axis_discrete" since="5">
> > > > + <description summary="axis click event">
> > > > + Scroll and other axis discrete step information.
> > > > +
> > > > + This event does not occur on its own. It is sent before a
> > > > + wl_pointer.axis event and carries the axis value of the
> > > > + wl_pointer.axis event in discrete steps (e.g. mouse wheel clicks).
> > > > +
> > > > + This event is optional, continuous scrolling devices
> > > > + like two-finger scrolling on touchpads do not have discrete
> > > > + steps.
> > >
> > > Hmm. This means we wouldn't be able to convert the touch based scroll
> > > motions into the old scroll step clicks, since we wouldn't (reliably)
> > > know when a device would emit their own, since it doesn't specify
> > > exactly when to expect and not to expect. The client won't know whether
> > > there are "continuous scrolling devices" or not.
> > >
> > > Wouldn't it be easier to just provide XI2 style smooth scroll vectors,
> > > i.e. steps in "discrete scale", where a mouse scroll click is -1.0 or
> > > 1.0 and continuous scrolling is a fraction of such. Clients implementing
> > > version 5 can simply use the accumulation method for emitting scroll
> > > clicks without having to rely on the old "divide by 10" hack.
> > >
> > > This event is actually more like what I described in my previous mail
> > > that were more or less concluded to be not worth it, isn't it?
> >
> > the wl_pointer.axis documentation defines the axis events to be in the same
> > coordinate space as motion data, there is nothing accommodating for mouse
> > wheel clicks. it's just magic knowledge with the value 10.
> > Adding this event set removes that magic knowledge, if you get an
> > axis_discrete event you can safely ignore the axis value and just scroll by
> > the discrete value.
> >
> > I don't think the two should be merged into the same coordinate space. It'd
> > be easier but also more error-prone. For example, on a touchpad you probably
> > want scrolling to move at the same speed as the cursor would otherwise so
> > the content follows the finger (this is a lot more obvious when natural
> > scrolling is enabled). But a click is a click and maps to say 3 lines, or a
> > page, or whatever in the context. They are physically two different
> > coordinate spaces, I don't think merging them will help.
> >
> > So I think of this less as "add continuous sources" because that's what the
> > protocol already is. I think of it as adding wheel click information for
> > when a continuous space isn't right. Does that make sense or did I miss the
> > point?
>
> I didn't say we should merge the coordinate spaces, but rather make
> axis_discrete value a wl_fixed_t value in discrete coordinate space. We
> cannot change wl_pointer.axis and doing so would loose information which
> is bad. The problem is that clients often still need to continue emulate
> "clicks" for all type of scrolling methods including touch scroll, and
> since the current proposal of wl_pointer.axis_discrete is only sent for
> actual scroll clicks, that situation isn't improved at all. We still
> need the magic / 10 in the clients, except now with an extra if (source
> != wheel). We'd also not get rid of the XI2 smooth scroll scaling in
> clients.
>
> Or are you saying we should keep the scroll click and XI2 smooth scroll
> emulation where they are now?
tbh, I'm not a big fan of sending things we don't have. if a client needs to
convert a continuous space into discrete clicks, IMO it should do on its
own terms. changing to a wl_fixed_t as data type is no problem, but sending
discrete values for e.g. two-finger scrolling isn't ideal.
Cheers,
Peter
More information about the wayland-devel
mailing list