On Mon, Mar 23, 2015 at 01:21:38PM +1000, Peter Hutterer wrote: > On Mon, Mar 23, 2015 at 10:23:18AM +0800, Jonas Ådahl wrote: > > On Mon, Mar 09, 2015 at 01:28:04PM +1000, Peter Hutterer wrote: > > > The axis source determines how an event was generated. That enables > > > clients to > > > judge when to use kinetic scrolling. > > > > Nice to see this happening! > > > > I have not looked at the implementation so far, only the protocol. I have > > some comments below. > > > > > > > > We can't extend the existing wl_pointer.axis events so instead this new > > > event > > > is prefixed before each wl_pointer.axis 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 > > > > > > Clients need to combine the state of the two events where needed. > > > > > > Supported are a flag to notify the client when a sequence stops, which is > > > the > > > only time when no axis events follows the axis_source event. > > > [Note: nothing in the protocol prevents us from sending 0/0 axis events, > > > so > > > we may not need this special case] > > > > > > The step_distance indicates how the value in the following event should be > > > interpreted (i.e. divided) to get a discrete count of scroll events (i.e. > > > wheel clicks. > > > [Note: arguably it is more flexible to include the discrete count here > > > rather > > > than the step_distance. This would split the information across two > > > events > > > though] > > > --- > > > protocol/wayland.xml | 48 > > > +++++++++++++++++++++++++++++++++++++++++++++++- > > > 1 file changed, 47 insertions(+), 1 deletion(-) > > > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > > index 88bbbc0..880f90a 100644 > > > --- a/protocol/wayland.xml > > > +++ b/protocol/wayland.xml > > > @@ -1402,7 +1402,7 @@ > > > > > > </interface> > > > > > > - <interface name="wl_pointer" version="3"> > > > + <interface name="wl_pointer" version="4"> > > > > Hmm. I think we should also increase the verison of wl_seat, wl_touch and > > wl_keyboard as part of this. > > Is there a requirement to do this whenever one of them changes? I honestly > don't know, but I'd be surprised if we can't move them independently.
wl_touch and wl_keyboard might not be necessary, but you need to at least bump the wl_seat version as well. Any later additions to wl_keyboard and wl_touch would then need to bump to wl_seat.version + 1 (i.e. increase by > 1), which is why I thought it could be reasonable to bump all at once to avoid those future confusion, but it doesn't seem what has been done before. Since wl_seat is already at version 4 from a previous addition to wl_kebyoard, we actually need to bump both wl_pointer and wl_seat to 5. The reason for the need to also bump the wl_seat version is that clients does not specify a version when creating the wl_pointer object, meaning the wl_pointer/wl_touch/wl_keyboard will always be the same version as the wl_seat they were created from. > > > > > > <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 > > > @@ -1563,6 +1563,52 @@ > > > <description summary="release the pointer object"/> > > > </request> > > > > > > + <!-- Version 4 additions --> > > > + > > > + <enum name="axis_source"> > > > + <description summary="axis source types"> > > > + Describes the axis types of scroll events. > > > + </description> > > > + <entry name="unknown" value="0"/> > > > + <entry name="wheel" value="1"/> > > > + <entry name="finger" value="2"/> > > > + <entry name="continuous" value="3"/> > > > + </enum> > > > + > > > + <enum name="axis_source_flags"> > > > + <description summary="axis flags"> > > > + </description> > > > + <entry name="none" value="0x0" summary="emty flags"/> > > > + <entry name="stop_scroll" value="0x1" > > > + summary="indicates this is the final scroll event on this > > > sequence"/> > > > + </enum> > > > + > > > + <event name="axis_source" since="4"> > > > + <description summary="axis source event"> > > > + Scroll and other axis source notification. > > > + > > > + This event is sent before a wl_pointer.axis event and carries the > > > + source information for the following axis event. Unless the > > > + stop_scroll axis flag is set, this event is always followed by a > > > + wl_pointer.axis event. > > > + > > > + The axis and timestamp values are identical to the one in the > > > + wl_pointer.axis event. For the interpretation of the axis value, > > > see > > > + the wl_pointer.axis event documentation. > > > + > > > + The source specifies how this event was generated. > > > + > > > + The step_distance denotes how to interpret the wl_pointer.axis > > > value > > > + in terms of discrete steps (i.e. mouse wheel clicks). If zero, > > > the > > > + value does not represent discrete steps. > > > + </description> > > > + <arg name="time" type="uint" summary="timestamp with millisecond > > > granularity"/> > > > + <arg name="axis" type="uint"/> > > > + <arg name="axis_source" type="uint"/> > > > + <arg name="axis_source_flags" type="uint"/> > > > + <arg name="step_distance" type="uint"/> > > > > Not a big fan of putting so many different things in the same event. > > > > Regarding the flags, why can't such state also be made a latched state? > > If we want to add source-specific information to an axis event (like the > > stop_scroll flag), that might just as well require some additional data, > > and then a flag would not be flexible enough. For the stop_scroll flag, > > if just a 0 length axis event is not enough for that use case, we could > > add 'stop_scroll' event that is a dispatched event on a 0 length axis > > event (as you said, a 0 length axis event is not invalid). > > > > Regarding the step_distance, I don't think it`s a bad thing to add > > information like that as a separate event working as a latched state > > adding information to wl_pointer.axis in the same way as the proposed > > 'axis_source' event already do; it is the way to extend events. I do > > think it'd be better to avoid the sometimes-latched-sometimes-not > > situation and make new events that extend another one always do that and > > nothing else. > > ok, just to avoid any confusion: with 'latched' you mean add events that > release once the wl_pointer.axis event arrives. So instead of the above, the > event sequence for a stop event would be: > wl_pointer.axis_source > wl_pointer.scroll_stop > wl_pointer.step_distance > wl.pointer.axis > > rather than the flags and whatnot. but you're still proposing to send these > for every event, rather than only when the value changes, right? So a > full touchpad scroll sequence would be: > 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.scroll_stop > wl.pointer.axis > > and _not_: > wl_pointer.axis_source > wl.pointer.axis > wl.pointer.axis > wl.pointer.axis > wl.pointer.axis > wl_pointer.scroll_stop > wl.pointer.axis > > correct? If so, I like it, it certainly feels cleaner than the various > flags. Exactly. Meaning a "latched state" is purely an addition to a "committing" event, and not a persistent state. Might be a good idea to describe this as a good way to extend events. Shouldn't just as well just go with wl_pointer.axis_discrete instead of wl_pointer.step_distance? It's less confusing since we don't need to define how a client should deal with step_distance changing. > > > I'm thinking, should there be yet another event for doing actual step > > triggering? I assume clients need to more or less reset their step > > distance counter per source type and then trigger the step "clicks" when > > the distance reaches the threshold. While there will most likely ever > > only be one device per source type generating events, the protocol still > > makes it possible to not generate the "correct" number of discrete > > "steps" since the sources still clump together separate devices which > > should be processed indivdually. My gut feeling its a bit unnecessary, > > but just wanted to put the question out there. > > hmm, scrolling on two devices simultaneously seems like a very niche use-case. > The event sounds like it should work but I wonder if it's worth the effort? > or even if it is whether we should wait for a couple of real-world use-cases > for this first before we end up with a solution that doesn't fix them. > > For the case of scrolling with a mouse and then a touchpad, the axis sources > should cover that - unless it's a finger source, you can't rely on another > scroll event to come anyway, so you need to always process the one you > currently have. Which makes handling of less than a threshold iffy on those > devices anyway. The point would be to just get rid of the threshold stuff from the clients completely. If a client would want to do the classic click scroll events, then it'd ignore everything except a wl_pointer.axis_step which would just be converted to whatever internal format for click scrolls the client would have. But yea, not sure its worth the effort. Jonas > > Cheers, > Peter > > _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
