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 <[email protected]> > > > > --- > > > > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
