[PATCH weston v3] input: use doubles in the interfaces to notify of input events

[Pekka Paalanen]

On Mon, 21 Mar 2016 13:29:00 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> This patch is a further step in the wl_fixed_t internal sanitization.
> It changes the notify_* functions to take doubles instead of wl_fixed_t
> but does not change how these are stored in the various input structs
> yet, except for weston_pointer_axis_event.
> However this already allows to remove all wl_fixed_t usage in places
> like the libinput or the x11 backend.
> ---
> 
> v3: rebased on master. This required quite many changes, due to the
> introduction of the input events
> 
>  desktop-shell/shell.c    |  2 +-
>  src/compositor-rdp.c     | 12 +++-------
>  src/compositor-wayland.c | 58 ++++++++++++++++++++++++++++++------------------
>  src/compositor-x11.c     | 22 +++++++++---------
>  src/compositor.c         | 12 +++++-----
>  src/compositor.h         | 12 +++++-----
>  src/input.c              | 15 ++++++++-----
>  src/libinput-device.c    | 26 +++++++++-------------
>  src/screen-share.c       |  5 +++--
>  9 files changed, 86 insertions(+), 78 deletions(-)
> 

Hi Giulio,

I see you already sent v4, so I'll leave reviewing v3 unfinished. Here
are my comments so far.

I have checked these changes:
- weston_pointer_axis_event::value
- notify_motion_absolute()

> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index e1fefae..837c18b 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -4786,7 +4786,7 @@ surface_opacity_binding(struct weston_pointer *pointer, uint32_t time,
>  	if (!shsurf)
>  		return;
>  
> -	shsurf->view->alpha -= wl_fixed_to_double(event->value) * step;
> +	shsurf->view->alpha -= event->value * step;
>  
>  	if (shsurf->view->alpha > 1.0)
>  		shsurf->view->alpha = 1.0;

Desktop-shell is missing another place: do_zoom() expects a wl_fixed_t,
but gets fed a event->value directly.

I found it my making weston_pointer_axis_event::value
__attribute__((deprecated)) and inspecting all the sites it raises.



> diff --git a/src/input.c b/src/input.c
> index 5d13b08..f52db4b 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -352,7 +352,8 @@ weston_pointer_send_axis(struct weston_pointer *pointer,
>  
>  		if (event->value)
>  			wl_pointer_send_axis(resource, time,
> -					     event->axis, event->value);
> +					     event->axis,
> +					     wl_fixed_from_double(event->value));

The change is fine, but I just wanted to point out the 'if
(event->value)' test. I suppose it's fine even when it's a double,
right? We don't need a threshold check?

>  		else if (wl_resource_get_version(resource) >=
>  			 WL_POINTER_AXIS_STOP_SINCE_VERSION)
>  			wl_pointer_send_axis_stop(resource, time,


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160322/04fc7037/attachment.sig>