[PATCH v3 weston 2/2] Support axis source, axis discrete, frame and axis stop events
Jonas Ådahl
jadahl at gmail.com
Mon Jan 11 16:07:25 PST 2016
On Tue, Jan 12, 2016 at 09:05:40AM +1000, Peter Hutterer wrote:
> On Mon, Jan 11, 2016 at 03:59:56PM +0800, Jonas Ådahl wrote:
> > On Tue, Dec 08, 2015 at 11:15:01AM +1000, Peter Hutterer wrote:
> > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >
> > Looks correct to me, so consider this (and the previous patch)
> > Reviewed-by: Jonas Ådahl <jadahl at gmail.com>. I do, however, have some
> > comments on the overall design inlined below.
> >
> > > ---
> > > Changes to v2:
> > > - use the new weston axis event struct, client-side is still a per-event
> > > handler (could be done at some later point)
> > > - couple of minor improvements to extracting the information out of libinput
> > >
> > > clients/eventdemo.c | 57 ++++++++++++++++-
> > > clients/window.c | 97 ++++++++++++++++++++++++++++-
> > > clients/window.h | 31 +++++++++
> > > desktop-shell/exposay.c | 12 ++++
> > > desktop-shell/shell.c | 33 ++++++++++
> > > ivi-shell/hmi-controller.c | 15 +++++
> > > src/compositor-wayland.c | 66 ++++++++++++++++++++
> > > src/compositor-x11.c | 13 ++++
> > > src/compositor.h | 16 +++++
> > > src/data-device.c | 12 ++++
> > > src/input.c | 93 ++++++++++++++++++++++++---
> > > src/libinput-device.c | 128 ++++++++++++++++++++++++++++----------
> > > tests/weston-test-client-helper.c | 32 ++++++++++
> > > 13 files changed, 564 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/clients/eventdemo.c b/clients/eventdemo.c
> > > index bdad6fd..e323aa5 100644
> > > --- a/clients/eventdemo.c
> > > +++ b/clients/eventdemo.c
> > > @@ -259,6 +259,54 @@ axis_handler(struct widget *widget, struct input *input, uint32_t time,
> > > wl_fixed_to_double(value));
> > > }
> > >
> > > +static void
> > > +pointer_frame_handler(struct widget *widget, struct input *input, void *data)
> > > +{
> > > + printf("pointer frame\n");
> > > +}
> > > +
> > > +static void
> > > +axis_source_handler(struct widget *widget, struct input *input,
> > > + uint32_t source, void *data)
> > > +{
> > > + const char *axis_source;
> > > +
> > > + switch (source) {
> > > + case WL_POINTER_AXIS_SOURCE_WHEEL:
> > > + axis_source = "wheel";
> > > + break;
> > > + case WL_POINTER_AXIS_SOURCE_FINGER:
> > > + axis_source = "finger";
> > > + break;
> > > + case WL_POINTER_AXIS_SOURCE_CONTINUOUS:
> > > + axis_source = "continuous";
> > > + break;
> > > + default:
> > > + axis_source = "<invalid source value>";
> > > + break;
> > > + }
> > > +
> > > + printf("axis source: %s\n", axis_source);
> > > +}
> > > +
> > > +static void
> > > +axis_stop_handler(struct widget *widget, struct input *input,
> > > + uint32_t time, uint32_t axis,
> > > + void *data)
> > > +{
> > > + printf("axis stop time: %d, axis: %s\n",
> > > + time,
> > > + axis == WL_POINTER_AXIS_VERTICAL_SCROLL ? "vertical" :
> > > + "horizontal");
> > > +}
> > > +
> > > +static void
> > > +axis_discrete_handler(struct widget *widget, struct input *input,
> > > + uint32_t axis, int32_t discrete, void *data)
> > > +{
> > > + printf("axis discrete axis: %d value: %d\n", axis, discrete);
> > > +}
> > > +
> > > /**
> > > * \brief CALLBACK function, Waylands informs about pointer motion
> > > * \param widget widget
> > > @@ -347,8 +395,15 @@ eventdemo_create(struct display *d)
> > > /* Set the callback motion handler for the window */
> > > widget_set_motion_handler(e->widget, motion_handler);
> > >
> > > + /* Set the callback pointer frame handler for the window */
> > > + widget_set_pointer_frame_handler(e->widget, pointer_frame_handler);
> > > +
> > > /* Set the callback axis handler for the window */
> > > - widget_set_axis_handler(e->widget, axis_handler);
> > > + widget_set_axis_handlers(e->widget,
> > > + axis_handler,
> > > + axis_source_handler,
> > > + axis_stop_handler,
> > > + axis_discrete_handler);
> > >
> > > /* Initial drawing of the window */
> > > window_schedule_resize(e->window, width, height);
> > > diff --git a/clients/window.c b/clients/window.c
> > > index 5d69116..7d45acd 100644
> > > --- a/clients/window.c
> > > +++ b/clients/window.c
> > > @@ -282,6 +282,10 @@ struct widget {
> > > widget_touch_frame_handler_t touch_frame_handler;
> > > widget_touch_cancel_handler_t touch_cancel_handler;
> > > widget_axis_handler_t axis_handler;
> > > + widget_pointer_frame_handler_t pointer_frame_handler;
> > > + widget_axis_source_handler_t axis_source_handler;
> > > + widget_axis_stop_handler_t axis_stop_handler;
> > > + widget_axis_discrete_handler_t axis_discrete_handler;
> >
> > Should we really make the toytoolkit API this low level? I guess it's
> > easier this way, so I won't complain further, but it'd be good to have
> > our toy toolkit try to construct the higher level events the protocol
> > try to communicate.
>
> I think the question is: how much we want the toytoolkit to stay a
> toy? Composing those events is doable, but how many toytoolkit users will
> really care about them?
Probably none. It's just a matter of a "toytoolkit should represent how
the protocol should be interpreted" kind of philosophy. But I understand
that doing all that extra work just for that reason isn't all that
reasonable.
>
> [...]
>
> > > diff --git a/src/compositor.h b/src/compositor.h
> > > index bf38784..c9e61a5 100644
> > > --- a/src/compositor.h
> > > +++ b/src/compositor.h
> > > @@ -256,6 +256,8 @@ struct weston_pointer_motion_event {
> > > struct weston_pointer_axis_event {
> > > uint32_t axis;
> > > wl_fixed_t value;
> > > + bool has_discrete;
> > > + int32_t discrete;
> > > };
> > >
> > > struct weston_pointer_grab;
> > > @@ -268,6 +270,8 @@ struct weston_pointer_grab_interface {
> > > void (*axis)(struct weston_pointer_grab *grab,
> > > uint32_t time,
> > > struct weston_pointer_axis_event *event);
> > > + void (*axis_source)(struct weston_pointer_grab *grab, uint32_t source);
> >
> > I suppose you have this as a separate event because the axis event is
> > only per-axis instead of in 2D? Wouldn't it be better to make
> > weston_pointer_axis_event contain the actual axis events including all
> > the changed axis, source tag?
>
> it's per frame, and it's optional. So you may get axis events without a
> source tag, which would require an "unknown" define. Not the end of the
> world though, but again, the question is: how many toytoolkit clients will
> care?
While the previous comment was about toytoolkit, this is about the
compositor/shell API, so libweston users might care. Anyway, I don't
have a strong opinion about how this, so lets leave it like this.
Jonas
>
> Cheers,
> Peter
>
More information about the wayland-devel
mailing list