On Fri, Mar 27, 2015 at 10:23:43AM +0800, Jonas Ådahl wrote: > 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?
ACK, removed locally. > > > > + <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? I'm not sure here tbh. I think it'd be good enough to say that this affects all scroll directions currently active, but that's based on a couple of conditions: * the case of kinetic scrolling on two devices independently is a niche case we don't care about. that's easy * we only have scroll axes so far, if we add unrelated axes to the wl_pointer.axes set this gets a bit hairy. e.g. does a kinetic scrolling stop affect a future rotation axis? the solution here could be to add flags to the stop event to specify the axes that stopped scrolling. > 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). I'll have to double check the code but iirc we send axis stop events in libinput for some continuous scrolling. e.g. trackstick middle button scrolling - releasing the middle button sends the scroll termination event. we don't have a pre-notification in libinput though, so you won't know until you get that stop event that you will get one. That's fixable but whether it's needed is another matter: if you get it you'll get it fast enough to trigger kinetics, otherwise you time out (and the stop event may come in after the timeout). the axis_source is still prefixed to the axis_stop event, but that doesn't scale if you have to independent scroll devices that use the same type. Again, a question of whether that's required, if so we could assign some sort of tracking ID to the scroll sequence like the touch sequences have. > > > > + </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? tbh, I'm not a big fan of sending things we don't have. if a client needs to convert a continuous space into discrete clicks, IMO it should do on its own terms. changing to a wl_fixed_t as data type is no problem, but sending discrete values for e.g. two-finger scrolling isn't ideal. Cheers, Peter _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
