[PATCH weston] zoom: Remove unneeded usage of wl_fixed_ts in favour of doubles

[Giulio Camuffo]

2015-10-07 20:29 GMT+03:00 Derek Foreman <derekf at osg.samsung.com>:
> On 07/10/15 12:14 PM, Giulio Camuffo wrote:
>> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>
>> ---
>>
>> This is the first of a series of patches i plan on doing to remove the
>> internal use of wl_fixed_t in weston, if people agree.
>> There is really no reason as far as i can see to use it unless you're
>> sending a wayland event, and usually code using wl_fixed_ts end
>> up doing a lot of conversions back and forth.
>
> yes, yes, yes and yes again.
>
> I find their usage confusing - let's not forget that the intent of
> weston is to be an easy to read piece of reference code. :)
>
>>  src/compositor.h |  8 +++-----
>>  src/zoom.c       | 22 +++++++++++-----------
>>  2 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/compositor.h b/src/compositor.h
>> index c4c81f0..2e2a185 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -144,20 +144,18 @@ struct weston_spring {
>>       uint32_t clip;
>>  };
>>
>> -struct weston_fixed_point {
>> -     wl_fixed_t x, y;
>> -};
>> -
>>  struct weston_output_zoom {
>>       bool active;
>>       float increment;
>>       float level;
>>       float max_level;
>>       float trans_x, trans_y;
>> +     struct {
>> +             double x, y;
>> +     } current;
>>       struct weston_seat *seat;
>>       struct weston_animation animation_z;
>>       struct weston_spring spring_z;
>> -     struct weston_fixed_point current;
>>       struct wl_listener motion_listener;
>>  };
>>
>> diff --git a/src/zoom.c b/src/zoom.c
>> index 8eb20fe..edffa89 100644
>> --- a/src/zoom.c
>> +++ b/src/zoom.c
>> @@ -65,13 +65,13 @@ weston_zoom_frame_z(struct weston_animation *animation,
>>
>>  static void
>>  zoom_area_center_from_point(struct weston_output *output,
>> -                         wl_fixed_t *x, wl_fixed_t *y)
>> +                         double *x, double *y)
>>  {
>>       float level = output->zoom.spring_z.current;
>> -     wl_fixed_t offset_x = wl_fixed_from_int(output->x);
>> -     wl_fixed_t offset_y = wl_fixed_from_int(output->y);
>> -     wl_fixed_t w = wl_fixed_from_int(output->width);
>> -     wl_fixed_t h = wl_fixed_from_int(output->height);
>> +     double offset_x = output->x;
>> +     double offset_y = output->y;
>> +     double w = output->width;
>> +     double h = output->height;
>>
>>       *x = (*x - offset_x) * level + w / 2;
>>       *y = (*y - offset_y) * level + h / 2;
>> @@ -81,8 +81,8 @@ static void
>>  weston_output_update_zoom_transform(struct weston_output *output)
>>  {
>>       float global_x, global_y;
>> -     wl_fixed_t x = output->zoom.current.x; /* global pointer coords */
>> -     wl_fixed_t y = output->zoom.current.y;
>> +     double x = output->zoom.current.x; /* global pointer coords */
>> +     double y = output->zoom.current.y;
>>       float level;
>>
>>       level = output->zoom.spring_z.current;
>> @@ -93,8 +93,8 @@ weston_output_update_zoom_transform(struct weston_output *output)
>>
>>       zoom_area_center_from_point(output, &x, &y);
>>
>> -     global_x = wl_fixed_to_double(x);
>> -     global_y = wl_fixed_to_double(y);
>> +     global_x = x;
>> +     global_y = y;
>
> I think global_x and global_y can just go away now?
>
> In fact, I think a lot of the temp variable usage isn't worthwhile
> anymore and would result in simpler code.
>
> zoom_area_center_from_pointer, for example, we're using each temp var
> once...

Indeed, i was unsure if i should remove them or try to keep the code
as similar as before so that review is easier and it's easier to see
there is no functional difference... Maybe let's hear what other
people think.


Thanks,
Giulio

>
> Either way,
> Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
>
>>       output->zoom.trans_x = global_x - output->width / 2;
>>       output->zoom.trans_y = global_y - output->height / 2;
>> @@ -133,8 +133,8 @@ weston_output_update_zoom(struct weston_output *output)
>>
>>       assert(output->zoom.active);
>>
>> -     output->zoom.current.x = pointer->x;
>> -     output->zoom.current.y = pointer->y;
>> +     output->zoom.current.x = wl_fixed_to_double(pointer->x);
>> +     output->zoom.current.y = wl_fixed_to_double(pointer->y);
>>
>>       weston_zoom_transition(output);
>>       weston_output_update_zoom_transform(output);
>>
>