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

[Bryce Harrington]

On Wed, Oct 07, 2015 at 09:05:18PM +0300, Giulio Camuffo wrote:
> 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.

Indeed it does make the review easier, thanks for thinking of us!

However I also think the temp variables can go too, and that would make
for a great follow up patch.

Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>

> 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);
> >>
> >
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel