[PATCH v2 wayland] protocol: add wl_pointer.axis_frame events

[Bryce Harrington]

Peter sounds like this patch is close to being ready to land.  Mind
doing a reroll of the two patches (or squash them as Jonas suggests)?

Bryce

On Mon, Jul 13, 2015 at 03:21:21PM +0800, Jonas Ådahl wrote:
> On Mon, Jul 13, 2015 at 04:01:20PM +1000, Peter Hutterer wrote:
> > On Sun, Jul 12, 2015 at 03:58:47PM +0800, Jonas Ådahl wrote:
> > > On Thu, Jun 25, 2015 at 04:01:48PM +1000, Peter Hutterer wrote:
> > > > To group separate vertical/horizontal scroll events together. Likewise it
> > > > enables axis_stop events to be grouped so that the final vector for kinetic
> > > > scrolling may be calculated correctly.
> > > > 
> > > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > > ---
> > > > Changes to v1:
> > > > - drop the option of having multiple axis sources within the same frame
> > > >   (and reword the documentation accordingly)
> > > > 
> > > > Two comments here:
> > > > - the protocol feels a bit unbalanced now when discrete events are in use,
> > > >   these must still be sent before the matching axis events. i.e. we have 4
> > > >   events that commit with the axis_frame, but one event (discrete) that
> > > >   commit with the axis event:
> > > >       axis_source
> > > >       axis
> > > >       axis_discrete
> > > >       axis
> > > >       axis_frame
> > > >   though sending the source just before the frame may make this a bit easier
> > > >   on the eye
> > > 
> > > The "unbalancedness" I feel is that "axis_source" is a bit "let-loose"
> > > as it doesn't extend some other event, but kind of belong to axis_frame.
> > > So ...
> > > 
> > > > - we could now skip the source and merge it into the axis_frame event,
> > > >   though that would require the return of the "unknown" source.
> > > 
> > > .. now that we always have a single source per frame, I think this would
> > > make sense to do. An 'unknown' source I guess is not that big of a
> > > change, semantically. IIRC the issue with it was that a device
> > > which previously would have source "unknown" would suddenly change if we
> > > learned about it, but as long as we say that might happen, maybe its not
> > > an issue really.
> > > 
> > > Otherwise, maybe it'd be best to make axis_source extend axis_frame
> > > like all other such events, i.e. use the common way of adding
> > > information to an already existing event. That'd will also make it a bit
> > > less unbalanced IMHO.
> > 
> > looking at this again with fresher eyes I think the unbalancedness simply
> > doesn't matter. if we visualise it as a tree, then the sequence above looks
> > like this:
> > 
> > axis_frame
> >   +----> axis
> >   +----> axis
> >   |        +----> axis_discrete
> >   +----> axis_source 
> > 
> > i.e. the source is at the same level that axis/axis_stop are. that makes it
> > less unbalanced than it initially looked. and it avoids the "unknown" mess.
> > so I think we should leave it as-is.
> 
> Yea, the only slightly odd thing is that source is like a mix between
> "axis" and "axis_discrete" semantics wise (ignoring they belong extend
> different events). But in this case it's pretty clear how to interpret
> it so its no big deal.
> 
> > 
> > > >  
> > > >  protocol/wayland.xml | 30 +++++++++++++++++++++++++++---
> > > >  1 file changed, 27 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > > index 48d0332..db9b164 100644
> > > > --- a/protocol/wayland.xml
> > > > +++ b/protocol/wayland.xml
> > > > @@ -1579,6 +1579,30 @@
> > > >      </request>
> > > >  
> > > >      <!-- Version 5 additions -->
> > > > +    <event name="axis_frame" since="5">
> > > > +      <description summary="end of axis set event">
> > > > +	Indicates the end of a set of wl_pointer.axis events that logically
> > > > +	belong together.
> > > > +
> > > > +	All wl_pointer.axis, wl_pointer.axis_stop, and
> > > > +	wl_pointer.axis_source before a wl_pointer.axis_frame event belong
> > > > +	logically together. For example, in a diagonal scroll motion the
> > > > +	compositor will send an optional wl_pointer.axis_source event, two
> > > > +	wl_pointer.axis events (horizontal and vertical) and finally a
> > > > +	wl_pointer.axis_frame event. The client may use this information to
> > > > +	calculate a diagonal vector for scrolling.
> > > > +
> > > > +	When multiple wl_pointer.axis events occur within the same frame,
> > > > +	the motion vector is the combined motion of all events.
> > > > +	When a wl_pointer.axis and a wl_pointer.axis_stop event occur within
> > > > +	the same frame, this indicates that axis movement in one axis has
> > > > +	stopped but continues in the other axis.
> > > > +	When multiple wl_pointer.axis_stop events occur within in the same
> > > > +	frame, this indicates that these axes stopped in the same instance.
> > > > +
> > > > +	Only one wl_pointer.axis_source event is permitted per axis frame.
> > > 
> > > Should we maybe point out that axis_frame is guaranteed to come for
> > > every axis event group, event though such a group might only be a single
> > > axis event? Meaning, you'd never get an axis without an axis_frame.
> > 
> > yep, easy enough. added:
> > 
> >         Only one wl_pointer.axis_source event is permitted per axis frame.
> > +
> > +       A wl_pointer.axis_frame event is sent for every logical axis event
> > +       group, even if the group only contains a single wl_pointer.axis or
> > +       wl_pointer.axis_stop event. In other words, a client may get the
> > +       sequence: axis, axis_frame, axis, axis_frame, ...
> > 
> > Cheers,
> >    Peter
> 
> With that, this patch (or even better, it squashed together with the
> axis source patch) can have my RB.
> 
> 
> Jonas
> 
> >  
> > > 
> > > > +      </description>
> > > > +    </event>
> > > >  
> > > >      <enum name="axis_source">
> > > >        <description summary="axis source types">
> > > > @@ -1605,9 +1629,9 @@
> > > >  	Source information for scroll and other axes.
> > > >  
> > > >  	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.
> > > > +	wl_pointer.axis_frame event and carries the source information for
> > > > +	all events within that frame. A client is expected to accumulate the
> > > > +	data in all events events within the frame before proceeding.
> > > >  
> > > >  	The source specifies how this event was generated. If the source is
> > > >  	wl_pointer.axis_source.finger, a wl_pointer.axis_stop event will be
> > > > -- 
> > > > 2.4.3
> > > > 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel