On Mon, 21 Mar 2016 13:29:00 +0200
Giulio Camuffo <[email protected]> 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

Attachment: pgpViJXsOR1uk.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to