[RFC PATCH wayland] protocol: add wl_pointer.axis_source events

[Jonas Ådahl]

On Mon, Mar 09, 2015 at 01:28:04PM +1000, Peter Hutterer wrote:
> The axis source determines how an event was generated. That enables clients to
> judge when to use kinetic scrolling.

Nice to see this happening!

I have not looked at the implementation so far, only the protocol. I have
some comments below.

> 
> We can't extend the existing wl_pointer.axis events so instead this new event
> is prefixed before each wl_pointer.axis 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
> 
> Clients need to combine the state of the two events where needed.
> 
> Supported are a flag to notify the client when a sequence stops, which is the
> only time when no axis events follows the axis_source event.
> [Note: nothing in the protocol prevents us from sending 0/0 axis events, so
>  we may not need this special case]
> 
> The step_distance indicates how the value in the following event should be
> interpreted (i.e. divided) to get a discrete count of scroll events (i.e.
> wheel clicks.
> [Note: arguably it is more flexible to include the discrete count here rather
>  than the step_distance. This would split the information across two events
>  though]
> ---
>  protocol/wayland.xml | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index 88bbbc0..880f90a 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -1402,7 +1402,7 @@
>  
>    </interface>
>  
> -  <interface name="wl_pointer" version="3">
> +  <interface name="wl_pointer" version="4">

Hmm. I think we should also increase the verison of wl_seat, wl_touch and
wl_keyboard as part of this.

>      <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
> @@ -1563,6 +1563,52 @@
>        <description summary="release the pointer object"/>
>      </request>
>  
> +    <!-- Version 4 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>
> +
> +    <enum name="axis_source_flags">
> +      <description summary="axis flags">
> +      </description>
> +      <entry name="none" value="0x0" summary="emty flags"/>
> +      <entry name="stop_scroll" value="0x1"
> +        summary="indicates this is the final scroll event on this sequence"/>
> +    </enum>
> +
> +    <event name="axis_source" since="4">
> +      <description summary="axis source event">
> +        Scroll and other axis source notification.
> +
> +        This event is sent before a wl_pointer.axis event and carries the
> +        source information for the following axis event. Unless the
> +        stop_scroll axis flag is set, this event is always followed by a
> +        wl_pointer.axis event.
> +
> +        The axis and timestamp values are identical to the one in the
> +        wl_pointer.axis event. For the interpretation of the axis value, see
> +        the wl_pointer.axis event documentation.
> +
> +        The source specifies how this event was generated.
> +
> +        The step_distance denotes how to interpret the wl_pointer.axis value
> +        in terms of discrete steps (i.e. mouse wheel clicks). If zero, the
> +        value does not represent discrete steps.
> +      </description>
> +      <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> +      <arg name="axis" type="uint"/>
> +      <arg name="axis_source" type="uint"/>
> +      <arg name="axis_source_flags" type="uint"/>
> +      <arg name="step_distance" type="uint"/>

Not a big fan of putting so many different things in the same event.

Regarding the flags, why can't such state also be made a latched state?
If we want to add source-specific information to an axis event (like the
stop_scroll flag), that might just as well require some additional data,
and then a flag would not be flexible enough. For the stop_scroll flag,
if just a 0 length axis event is not enough for that use case, we could
add 'stop_scroll' event that is a dispatched event on a 0 length axis
event (as you said, a 0 length axis event is not invalid).

Regarding the step_distance, I don't think it`s a bad thing to add
information like that as a separate event working as a latched state
adding information to wl_pointer.axis in the same way as the proposed
'axis_source' event already do; it is the way to extend events. I do
think it'd be better to avoid the sometimes-latched-sometimes-not
situation and make new events that extend another one always do that and
nothing else.

I'm thinking, should there be yet another event for doing actual step
triggering? I assume clients need to more or less reset their step
distance counter per source type and then trigger the step "clicks" when
the distance reaches the threshold. While there will most likely ever
only be one device per source type generating events, the protocol still
makes it possible to not generate the "correct" number of discrete
"steps" since the sources still clump together separate devices which
should be processed indivdually. My gut feeling its a bit unnecessary,
but just wanted to put the question out there.


Jonas

> +    </event>
> +
>    </interface>
>  
>    <interface name="wl_keyboard" version="4">
> -- 
> 2.1.0
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel