[PATCH v2 weston 2/4] zoom: Call weston_output_activate_zoom() appropriately

[Bryce Harrington]

On Mon, Jul 27, 2015 at 01:03:37PM -0500, Derek Foreman wrote:
> On 23/07/15 07:02 PM, Bryce Harrington wrote:
> > On Thu, Jul 23, 2015 at 02:55:13PM -0500, Derek Foreman wrote:
> >> No longer call weston_output_update_zoom() when trying to zoom out
> >> on an unzoomed output.
> >>
> >> Add an assert() to make sure update_zoom is never called without an
> >> active zoom.
> >>
> >> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> > 

It still looks weird to me but it isn't wrong.

To ssh://git.freedesktop.org/git/wayland/weston
   859b52b..25bd8a7  master -> master


> > 
> >> ---
> >>  desktop-shell/shell.c | 5 ++++-
> >>  src/zoom.c            | 3 +++
> >>  2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> >> index 0137ca3..8f90710 100644
> >> --- a/desktop-shell/shell.c
> >> +++ b/desktop-shell/shell.c
> >> @@ -4804,7 +4804,10 @@ do_zoom(struct weston_seat *seat, uint32_t time, uint32_t key, uint32_t axis,
> >>  				output->zoom.level = 0.0;
> >>  			else if (output->zoom.level > output->zoom.max_level)
> >>  				output->zoom.level = output->zoom.max_level;
> >> -			else if (!output->zoom.active) {
> >> +
> >> +			if (!output->zoom.active) {
> > 
> > What exactly is zoom.active tracking?  That there is a zoom operation
> > undergoing, or the individual zoom animation steps?
> 
> That zoom is enabled for an output.  It's not a temporary transitional
> state, it just indicates we've got output magnification turned on.
> 
> We get into do_zoom() from either mousewheel bind or key bind.  We could
> be starting a new zoom (output->zoom.active is false) or adjusting an
> old one (output->zoom.active is true)
> 
> 
> >> +				if (output->zoom.level <= 0.0)
> >> +					continue;
> > 
> > The first part of the earlier if statement already checks for zoom.level
> > less than 0; perhaps it could be altered to check for <= 0.0 and
> > continue at that point?  Might make for clearer logic.
> 
> Still need to only do the continue when !output->zoom.active so I don't
> think there's a great simplification to be had here.
> 
> The first part is just clamping to min/max legitimate values, and is
> pretty clear as it is.  I don't think I'd want to combine the "do we
> need to enable zoom?" check with that.
> 
> > (Does negative zoom have any particular meaning?  If not, why not make
> > the value unsigned?)
> 
> Need the float precision, so we get the sign for free :)
> 
> >>  				weston_output_activate_zoom(output);
> >>  			}
> >> diff --git a/src/zoom.c b/src/zoom.c
> >> index 878ecc2..39515d4 100644
> >> --- a/src/zoom.c
> >> +++ b/src/zoom.c
> >> @@ -25,6 +25,7 @@
> >>  
> >>  #include "config.h"
> >>  
> >> +#include <assert.h>
> >>  #include <stdlib.h>
> >>  #include <stdbool.h>
> >>  
> >> @@ -135,6 +136,8 @@ weston_output_update_zoom(struct weston_output *output)
> >>  {
> >>  	struct weston_seat *seat = weston_zoom_pick_seat(output->compositor);
> >>  
> >> +	assert(output->zoom.active);
> >> +
> >>  	output->zoom.current.x = seat->pointer->x;
> >>  	output->zoom.current.y = seat->pointer->y;
> >>  
> >> -- 
> >> 2.1.4
> >>
> >> _______________________________________________
> >> wayland-devel mailing list
> >> wayland-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > 
> > Bryce
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >