On Thu, Jun 04, 2015 at 01:44:11PM +0800, Jonas Ådahl wrote: > On Wed, Jun 03, 2015 at 03:18:44PM +1000, Peter Hutterer wrote: > > On Tue, Jun 02, 2015 at 04:06:23PM +0800, Jonas Ådahl wrote: > > > Hi, > > > > > > > > > I think this looks pretty decent now. Currently I have no more issues > > > with the protocol, only minor nits and comments on wording etc. that can > > > be found inline. Assuming those are addressed, this patch has my > > > Reviewed-by. > > > > > > On Thu, May 07, 2015 at 03:54:08PM +1000, Peter Hutterer wrote: > > > > The axis_source event determines how an axis event was generated. That > > > > 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 v2: > > > > - more documentation, should be self-explanatory now > > > > - drop time and axis values from the events where they are redundant > > > > - axis_stop now carries an array so we can end more axes in one go > > > > > > > > wl_pointer.axis_discrete.discrete stayed an int after some IRC chat with > > > > Jonas. I've expanded the documentation a bit to hopefully make it clear > > > > this > > > > event won't be generated for some devices. Let's leave converting from > > > > pixel-space to discrete events up to the clients, they have more > > > > semantic > > > > knowledge of what exactly is required. > > > > > > > > protocol/wayland.xml | 98 > > > > +++++++++++++++++++++++++++++++++++++++++++++++++--- > > > > 1 file changed, 94 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > > > index c5963e8..15f8d72 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,100 @@ > > > > <description summary="release the pointer object"/> > > > > </request> > > > > > > > > + <!-- Version 5 additions --> > > > > + > > > > + <enum name="axis_source"> > > > > + <description summary="axis source types"> > > > > + Describes the source types for axis events. This indicates to > > > > the > > > > > > nit: Seems you have a tab here while not in the other paragraphs. > > > > I've replaced all of these with tabs now, which is the majority of the rest > > of the file. a global search/replace is needed here at some point. > > > > > > > > > + client how an axis event was physically generated, a client may > > > > > > nit: I'm not a native speaker, but shouldn't this be either a full stop > > > (.) or a semi colon (;), instead of a comma (,)? > > > > > > > + adjust the user interface accordingly. For example, scroll > > > > events > > > > + from a "finger" source may be in a smooth coordinate space with > > > > + kinetic scrolling whereas a "wheel" source may be in discrete > > > > steps > > > > + of a number of lines. > > > > + > > > > + The "continous" axis source is a device generating events in a > > > > + continuous coordinate space, but using something other than a > > > > + finger. One example for this source is button-based scrolling > > > > where > > > > + the vertical motion of a device is converted to scroll events > > > > while > > > > + a button is held down. > > > > + </description> > > > > + <entry name="wheel" value="0" summary="A physical wheel" /> > > > > + <entry name="finger" value="1" summary="Finger on a touch > > > > surface" /> > > > > > > Just thought about the ambiguity of using "finger" as one usually uses a > > > finger to scroll on a wheel. But whatever. Using "touch" would be rather > > > ambiguous too I suppose. > > > > fwiw, I specifically chose finger because it leaves the potential open for > > thumb or other touch-like interactions (that we'll probably never see, but > > it's nice to leave that avenue open). > > > > > > + <entry name="continuous" value="2" summary="Continuous > > > > coordinate space"/> > > > > + </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 source specifies how this event was generated. If the > > > > source is > > > > + wl_pointer.axis_source.finger, a wl_pointer.axis_stop event > > > > will be > > > > + sent when the user lifts the finger off the device. > > > > + > > > > + If the source is wl_pointer.axis_source.wheel or > > > > + wl_pointer.axis_source.continuous, a wl_pointer.axis_stop > > > > event may > > > > + be sent but is not guaranteed to be sent. > > > > + > > > > + This event is optional. If the source is unknown for a > > > > particular > > > > + axis event sequence, no event is sent. > > > > + > > > > + The order of wl_pointer.axis_discrete and > > > > wl_pointer.axis_source is > > > > + not guaranteed. > > > > + </description> > > > > + <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 types, 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. > > > > > > Maybe should clarify thats its a stat of a new motion from the same > > > source type as was just stopped. A stopped finger touch, does not affect > > > the state of a ongoing continuous scroll. > > > > fixed, "Any wl_pointer.axis events with the same axis_source after this > > event should be considered as the start of a new axis motion." > > > > > > + > > > > + The timestamp is to be interpreted identical to the timestamp > > > > in the > > > > + wl_pointer.axis event. > > > > > > It is unclear what axis event you are referring to here, as this is not > > > a latched state event. Maybe we should just "warn" that it may be > > > identical > > > to the previous axis event of the same source type? > > > > how about: "For documentation on the timestamp value see the wl_pointer.axis > > event"? > > Sounds more clear to me. Should it be spelled out it may be the same the > axis event? Anyway, either way is fine by me.
makes sense, added. Cheers, Peter > > > > > > > + > > > > + The axes array lists all axes stopped with this event. > > > > + </description> > > > > + <arg name="time" type="uint" summary="timestamp with millisecond > > > > granularity"/> > > > > + <arg name="axes" type="array" summary="the axes stopped with > > > > this event"/> > > > > + </event> > > > > + > > > > + <event name="axis_discrete" since="5"> > > > > + <description summary="axis click event"> > > > > + Scroll and other axis discrete step information. > > > > > > This sentence sounds odd. Maybe "Scroll and other discrete step > > > information"? > > > > I'll rephrase all of them then: "Discrete step information for scroll or > > other axes". > > and likewise for source and stop. > > > > thanks for the review, much appreciated. > > > > Cheers, > > Peter > > > > > > + > > > > + 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 and do not generate this event. > > > > + > > > > + The discrete value carries the directional information. e.g. a > > > > value > > > > + of -2 is two steps towards the negative direction of this axis. > > > > + > > > > + The order of wl_pointer.axis_discrete and > > > > wl_pointer.axis_source is > > > > + not guaranteed. > > > > + <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 +1780,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.5 > > > > _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
