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

[Derek Foreman]

On 19/03/15 08:06 PM, Jonas Ådahl wrote:
> 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) ...

Yup, I meant to copy your conditional verbatim but screwed up.  :)

> 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.

Fair enough, the patch as you've written it works for me and
dramatically reduces extra enter/leave with animated cursors.

Reviewed-By: Derek Foreman <derekf at osg.samsung.com>

> 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);
>>>  
>>>
>>