[RFC PATCH v2 wayland] protocol: add wl_pointer.axis_source, axis_stop and axis_discrete events
Jonas Ådahl
jadahl at gmail.com
Thu Mar 26 19:23:43 PDT 2015
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?
> > > + <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?
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).
>
>
> > > + </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?
Jonas
>
> >
> > > +
> > > + The axis and timestamp values are to be interpreted identical to the
> > > + ones in the wl_pointer.axis event.
> > > +
> > > + The discrete value carries the directional information. e.g. a value
> > > + of -2 is two steps towards the negative direction of this axis.
> > > + <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> > > + <arg name="axis" type="uint"/>
> >
> > Same as axis_source::time and axis_source.axis.
>
> removed, thanks.
>
> Cheers,
> Peter
>
> >
> > > + <arg name="discrete" type="int"/>
> > > + </description>
> > > + </event>
> > > </interface>
> > >
> > > <!-- for the version see the comment in wl_seat -->
> > > - <interface name="wl_keyboard" version="4">
> > > + <interface name="wl_keyboard" version="5">
> > > <description summary="keyboard input device">
> > > The wl_keyboard interface represents one or more keyboards
> > > associated with a seat.
> > > @@ -1690,7 +1769,7 @@
> > > </interface>
> > >
> > > <!-- for the version see the comment in wl_seat -->
> > > - <interface name="wl_touch" version="3">
> > > + <interface name="wl_touch" version="5">
> > > <description summary="touchscreen input device">
> > > The wl_touch interface represents a touchscreen
> > > associated with a seat.
> > > --
> > > 2.3.3
> > >
More information about the wayland-devel
mailing list