[RFC PATCH v2 wayland] protocol: add wl_pointer.axis_source, axis_stop and axis_discrete events

[Jonas Ådahl]

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.

> +
> +    <!-- 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"/>
> +      <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.

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

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


Jonas

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