Hi Kristian,

>-----Original Message-----
>From: Kristian Hoegsberg [mailto:[email protected]]
>Sent: Monday, March 12, 2012 4:23 AM
>To: [email protected]
>Cc: [email protected]
>Subject: Re: [PATCH] compositor: Fix pick an incorrect surface due to dirty
>surface_list
>
>On Fri, Mar 09, 2012 at 11:57:36AM +0800, [email protected] wrote:
>> From: Alex Wu <[email protected]>
>>
>> When a new toplevel window is activated and positioned under the
>> cursor, the pointer focus still on the previous window. The botton
>> event can not deliver to the new window. The root cause is that
>> weston_surface_restack() change the surface stack but not update the
>> weston_compositor::surface_list, and then calling
>> weston_compositor_repick() picks an incorrect surface.
>
>Ah yes, nice catch.  The patch does go a bit in the other direction than
where
>I'd like to go with the layer code and input handling though.  I've been
trying to
>move most real work to the repaint loop and rather than rebuilding the
surface
>list in more places, I prefer to just move repicking (in case of restacking
or new
>or deleted
>surfaces) to the repaint loop also.

[Wu, Zhiwen] Glad to see that.
I think, there are other shell-specific things should be done in the repaint
loop after restack/new/delete surface.
e.g. display mode may change due to the top fullscreen surface switching to
non-fullscreen or being deleted.
Another case is that if a focused surface is deleted, shell/compositor may
move the focus to another
proper surface automatically. All these things are proper to be done in the
repaint loop. So how about adding a
vmethod in the struct weston_shell and calling it in repaint loop to handle
these things? 
This is really helpful to deal with the fullscreen stuff.

>
>What I'd like to do is something like the attached patch, but we're not
quite
>ready to do that yet, since the X backend still handles events whenever
they
>comes in.  That means we may hit notify_motion with an out-of-date surface
>list.  I think we can just move the X11 fd into the input_loop and that
should be
>ok, but I haven't had time to try it.
>
>Kristian
>
>
>> ---
>>  src/compositor.c |   25 ++++++++++++++++++-------
>>  1 files changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/compositor.c b/src/compositor.c index
>> 96e7fe7..2b84e4b 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -165,6 +165,20 @@ weston_client_launch(struct weston_compositor
>> *compositor,  }
>>
>>  static void
>> +weston_compositor_rebuild_surface_list(struct weston_compositor *ec)
>> +{
>> +    struct weston_layer *layer;
>> +    struct weston_surface *es;
>> +
>> +    wl_list_init(&ec->surface_list);
>> +    wl_list_for_each(layer, &ec->layer_list, link) {
>> +            wl_list_for_each(es, &layer->surface_list, layer_link) {
>> +                    wl_list_insert(ec->surface_list.prev, &es->link);
>> +            }
>> +    }
>> +}
>> +
>> +static void
>>  surface_handle_buffer_destroy(struct wl_listener *listener,
>>                            struct wl_resource *resource, uint32_t time)
{ @@
>-546,6
>> +560,7 @@ weston_compositor_pick_surface(struct weston_compositor
>> *compositor,  {
>>      struct weston_surface *surface;
>>
>> +    weston_compositor_rebuild_surface_list(compositor);
>>      wl_list_for_each(surface, &compositor->surface_list, link) {
>>              weston_surface_from_global(surface, x, y, sx, sy);
>>              if (pixman_region32_contains_point(&surface->input,
>> @@ -914,13 +929,9 @@ weston_output_repaint(struct weston_output
>*output, int msecs)
>>      glViewport(0, 0, width, height);
>>
>>      /* Rebuild the surface list and update surface transforms up front.
*/
>> -    wl_list_init(&ec->surface_list);
>> -    wl_list_for_each(layer, &ec->layer_list, link) {
>> -            wl_list_for_each(es, &layer->surface_list, layer_link) {
>> -                    weston_surface_update_transform(es);
>> -                    wl_list_insert(ec->surface_list.prev, &es->link);
>> -            }
>> -    }
>> +    weston_compositor_rebuild_surface_list(ec);
>> +    wl_list_for_each(es, &ec->surface_list, link)
>> +            weston_surface_update_transform(es);
>>
>>      if (output->assign_planes)
>>              /*
>> --
>> 1.7.5.4
>>
>> _______________________________________________
>> wayland-devel mailing list
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to