[PATCH weston] input: Empty the current input region when configuring pointer surfaces

[Ander Conselvan de Oliveira]

On 02/01/2014 10:00 AM, Pekka Paalanen wrote:
> On Fri, 31 Jan 2014 16:07:51 +0200
> Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com> wrote:
>
>> The input region of the cursor surface is set to empty in
>> pointer_cursor_surface_configure(). Since during the commit
>> process this function is called before the pending input region
>> is made current, it empties surface->pending.input instead of
>> surface->input.
>>
>> But pointer_cursor_surface_configure() is also called from
>> pointer_set_cursor() in order to map the cursor even if there
>> isn't a subsequent attach and commit to the cursor surface. In
>> that case, surface->input is never emptied, since the configure
>> function emptied only the pending input region and there wasn't a
>> commit that made it effective.
>>
>> Fix this by emptying both pending and current input regions. The
>> latter shouldn't cause problems since the surface can't have a
>> role prior to being assigned the cursor role, so it shouldn't be
>> mapped in the first place.
>>
>> Also change toytoolkit so that it triggers the bug.
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=73711
>> ---
>>   clients/window.c | 6 +++---
>>   src/input.c      | 1 +
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/clients/window.c b/clients/window.c
>> index d8d79d0..75e6418 100644
>> --- a/clients/window.c
>> +++ b/clients/window.c
>> @@ -3458,13 +3458,13 @@ input_set_pointer_image_index(struct
>> input *input, int index) if (!buffer)
>>   		return;
>>
>> -	wl_pointer_set_cursor(input->pointer,
>> input->pointer_enter_serial,
>> -			      input->pointer_surface,
>> -			      image->hotspot_x,
>> image->hotspot_y); wl_surface_attach(input->pointer_surface,
>> buffer, 0, 0); wl_surface_damage(input->pointer_surface, 0, 0,
>>   			  image->width, image->height);
>>   	wl_surface_commit(input->pointer_surface);
>> +	wl_pointer_set_cursor(input->pointer,
>> input->pointer_enter_serial,
>> +			      input->pointer_surface,
>> +			      image->hotspot_x,
>> image->hotspot_y); }
>>
>>   static const struct wl_callback_listener
>> pointer_surface_listener; diff --git a/src/input.c b/src/input.c
>> index 25ed133..5ce7f39 100644
>> --- a/src/input.c
>> +++ b/src/input.c
>> @@ -1543,6 +1543,7 @@ pointer_cursor_surface_configure(struct
>> weston_surface *es, weston_view_set_position(pointer->sprite, x,
>> y);
>>   	empty_region(&es->pending.input);
>> +	empty_region(&es->input);
>>
>>   	if (!weston_surface_is_mapped(es)) {
>>   		wl_list_insert(&es->compositor->cursor_layer.view_list,
>
> Looks good to me!
>
> I wonder, do we have a similar problem with drag icon surfaces?

There isn't a problem because weston_pointer_start_drag() doesn't map 
the drag icon. It takes an attach/commit to do it and at that point 
emptying the pending input region does the trick.

The protocol is not clear whether the surface should be 
mapped/configured when the request is received. If we change this to 
behave similarly to wl_pointer::set_cursor, then we need a similar fix.

I'm not sure if that change would make sense though. Since 
wl_data_device::start_drag doesn't have a "hotspot" parameter, it is 
necessary to set the offset from the pointer on the first attach, unless 
that offset would be (0, 0). So if the drag_surface already has a 
buffer, the client would have to first attach NULL to be able to ensure 
the icon wouldn't be rendered in the wrong position due to the attach 
that sets the offset being processed a frame later than the start_drag 
request.

Thanks,
Ander

>
>
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>