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

[Giulio Camuffo]

2016-03-22 14:32 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> 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.

Ah, good finding.

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

Yeah, i think it's fine. Even if it's a 1e-10 i don't think there's
harm in sending it.


Thanks,
Giulio

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