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? > > > + <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? 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). > > > > > + </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? Jonas > > > > > > + > > > + 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. > > removed, thanks. > > Cheers, > Peter > > > > > > + <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 > > > _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
