[PATCH weston v2] input: Make setting the same pointer cursor state again a no-op

[Jonas Ådahl]

On Thu, Mar 19, 2015 at 12:37:02PM -0500, Derek Foreman wrote:
> On 18/03/15 09:23 PM, Jonas Ådahl wrote:
> > If the client calls wl_pointer.set_cursor with the same surface and hot
> > spot coordinate that is already set, don't do anything as no state was
> > changed.
> > 
> > This avoids an issue where a client setting the same cursor surface
> > multiple times would receive wl_surface.leave/enter on that surface
> > every time.
> > 
> > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > ---
> > 
> > Changes since v1:
> > 
> > Only no-op if both surface and hotspot was unchanged.
> > 
> >  src/input.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/input.c b/src/input.c
> > index 3867de2..469d5ce 100644
> > --- a/src/input.c
> > +++ b/src/input.c
> > @@ -1655,6 +1655,10 @@ pointer_set_cursor(struct wl_client *client, struct wl_resource *resource,
> >  			return;
> >  	}
> >  
> > +	if (pointer->sprite && pointer->sprite->surface == surface &&
> > +	    pointer->hotspot_x == x && pointer->hotspot_y == y)
> > +		return;
> 
> Sorry to bikeshed this - this does fix the hotspot problem for me, but I
> think there are still spurious enter/leave events when, for example,
> moving from the decor to the text area in weston-terminal.
> 
> what about...
> 
> if (pointer->sprite && pointer->sprite->surface) {
> 	pointer->hotspot_x = x;
> 	pointer->hotspot_y = y;
> 	pointer_cursor_surface_configure(surface, 0, 0);
> 	return;
> }

That'd probably rather need to be
if (pointer->sprite && pointer->sprite->surface == surface) ...
otherwise we would never change the sprite view. We'd also need to only
configure if the surface has a buffer committed. Anyway, my intention
was to fix not changing the state being a no-op as this triggers a busy
loop when GTK+ runs on weston (GTK+ calls wl_pointer.set_cursor when
output changes even though the cursor surfaces never changes - which
should be harmless). Fixing all the cases makes sense though, but I
think that can be done with a later patch.


Jonas

> 
> Technically, the cursor surface configure is only required when the
> hotspot changes, but I don't know how complicated you want to make this. :)
> 
> > +
> >  	if (pointer->sprite)
> >  		pointer_unmap_sprite(pointer);
> >  
> > 
>