[PATCH weston] desktop-shell: Don't crash on zoom without a pointer in the seat

[Peter Hutterer]

On Tue, Jan 06, 2015 at 07:49:50PM -0600, Derek Foreman wrote:
> On 06/01/15 04:04 PM, Peter Hutterer wrote:
> > On Tue, Jan 06, 2015 at 02:28:13PM -0600, Derek Foreman wrote:
> >> The zoom effect zooms at the seat's current pointer location.  When no
> >> pointer is present the zoom key bindings cause a crash.
> >>
> >> Instead, check for the absence of a pointer and log a warning.
> >>
> >> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> > 
> > it would be a bit more involved but I think it'd be more obvious to a user
> > if the zoom just happened around the center of the output. Or if you want to
> 
> Which output? :)

in that order:
- the one with the pointer on it (obviously not applicable here)
- the one with a keyboard focus
- optional: if only one output has a client surface - that one
- the "first" one (or built-in one if you want to get fancier)

> > get fancy on the center of the surface with the keyboard focus.
> 
> I actually started off with that in mind, and chickened out when things
> got sticky.  Wasn't really sure I could come up with sensible semantics
> for all the cases.  But I can give it a try.
> 
> > that way the user still sees something happening when the key is hit.
> 
> If I have multiple outputs, I don't know which to use - though if a
> window has keyboard focus I could use the center of that (as you suggest).
> 
> If I plug in a mouse, should zoom begin to follow it?  Hooking up the
> motion handler on hotplug isn't intractable, but the pointer gets pretty
> silly default co-ordinates and this will result in a jump.  no big deal?
>  (Maybe the pointer should instantiate at the center of the zoomed area?
>  Multiple outputs confuse life yet again?)

yes, it should follow the pointer (imo). as for default coordinates, why not
pick the center of the output the pointer starts out on? this is the default
in X and is imo the most justifiable and obvious solution.
 
> If I unplug the mouse the pointer isn't freed, it's just sort of
> orphaned.  So if I use the hot key after unplugging the mouse it still
> zooms about the old cursor co-ordinates.  I'll send a patch for that
> right away - I'm a little worried about any potential side effects it
> might have.  I don't know if the current behavior is intentional.

I'd say that behaviour is fine, as long as there's a wl_pointer, zoom at
that position and leave the rest up to the logic of when to remove the
wl_pointer.

Cheers,
   Peter

> 
> > having said that, this patch looks fine for what it does
> > Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
> > 
> > Cheers,
> >    Peter
> > 
> >> ---
> >>  desktop-shell/shell.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> >> index a7514f7..0ec6d33 100644
> >> --- a/desktop-shell/shell.c
> >> +++ b/desktop-shell/shell.c
> >> @@ -4671,6 +4671,11 @@ do_zoom(struct weston_seat *seat, uint32_t time, uint32_t key, uint32_t axis,
> >>  	struct weston_output *output;
> >>  	float increment;
> >>  
> >> +	if (!seat->pointer) {
> >> +		weston_log("Zoom hotkey pressed but seat '%s' contains no pointer.\n", seat->seat_name);
> >> +		return;
> >> +	}
> >> +
> >>  	wl_list_for_each(output, &compositor->output_list, link) {
> >>  		if (pixman_region32_contains_point(&output->region,
> >>  						   wl_fixed_to_double(seat->pointer->x),
> >> -- 
> >> 2.1.4
> >>
> >> _______________________________________________
> >> wayland-devel mailing list
> >> wayland-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >>
>