[PATCH v3 weston 2/2] Support axis source, axis discrete, frame and axis stop events

[Peter Hutterer]

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?

[...]

> > 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?

Cheers,
   Peter