[PATCH weston] compositor: quick fix for sub-surface mapping

[Pekka Paalanen]

On Mon, 30 Jun 2014 10:49:40 -0700
Jason Ekstrand <jason at jlekstrand.net> wrote:

> I don't like it but, sure, it shouldn't break anything much further.

Ok, thanks. I pushed it.
- pq

> On Mon, Jun 30, 2014 at 2:10 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >
> > If a client does this:
> > 1. create a main window and map it
> > 2. create a wl_surface, and make it a sub-surface of the main window
> > 3. set the sub-surface to desync
> > 4. commit content to the sub-surface to map it
> >
> > Then step 4 should cause the sub-surface to become mapped. However,
> > Weston fails to schedule a repaint in that case, so the sub-surface will
> > not appear until something else causes a repaint on that output, e.g.
> > the main window.
> >
> > A quick and dirty fix is to set the output mask for the surface in
> > Weston, which allows the repaint to be scheduled. This patch implements
> > that, and might only work right on single-output systems.
> >
> > A proper fix would involve rewriting the whole "is surface mapped"
> > mechanism in Weston, to not rely on output assignments but to have a
> > separate flag for "mapped", and figuring out how to schedule repaints
> > for the right outputs.
> >
> > Following is the actual protocol sequence used to trigger the problem:
> >
> > [3224648.125]  -> wl_compositor at 4.create_surface(new id wl_surface at 3)
> > [3224648.206]  -> xdg_shell at 7.get_xdg_surface(new id xdg_surface at 8,
> > wl_surface at 3)
> > [3224648.311]  -> xdg_surface at 8.set_title("simple-shm")
> > [3224648.378]  -> wl_surface at 3.damage(0, 0, 250, 250)
> > [3224649.888]  -> wl_shm at 6.create_pool(new id wl_shm_pool at 9, fd 6, 250000)
> > [3224650.031]  -> wl_shm_pool at 9.create_buffer(new id wl_buffer at 10, 0,
> > 250, 250, 1000, 1)
> > [3224650.244]  -> wl_shm_pool at 9.destroy()
> > [3224651.975]  -> wl_surface at 3.attach(wl_buffer at 10, 0, 0)
> > [3224652.100]  -> wl_surface at 3.damage(20, 20, 210, 210)
> > [3224652.243]  -> wl_surface at 3.frame(new id wl_callback at 11)
> > [3224652.317]  -> wl_surface at 3.commit()
> > [3228652.535]  -> wl_compositor at 4.create_surface(new id wl_surface at 12)
> > [3228652.610]  -> wl_subcompositor at 5.get_subsurface(new id
> > wl_subsurface at 13, wl_surface at 12, wl_surface at 3)
> > [3228652.644]  -> wl_subsurface at 13.set_desync()
> > [3228652.659]  -> wl_subsurface at 13.set_position(100, 100)
> > [3228654.090]  -> wl_shm at 6.create_pool(new id wl_shm_pool at 14, fd 6,
> > 250000)
> > [3228654.140]  -> wl_shm_pool at 14.create_buffer(new id wl_buffer at 15, 0,
> > 250, 250, 1000, 1)
> > [3228654.180]  -> wl_shm_pool at 14.destroy()
> > [3228654.408]  -> wl_surface at 12.attach(wl_buffer at 15, 0, 0)
> > [3228654.436]  -> wl_surface at 12.damage(0, 0, 250, 250)
> > [3228654.462]  -> wl_surface at 12.commit()
> >
> > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > Cc: George Kiagiadakis <george.kiagiadakis at collabora.com>
> > Cc: Jason Ekstrand <jason.ekstrand at intel.com>
> >
> > ---
> >
> > Do we want this patch in Weston upstream master or even 1.5 stable?
> > I am not sure when I would have time for the proper fix.
> > ---
> >  src/compositor.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index fa8730f..d78314a 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -2512,6 +2512,8 @@ subsurface_configure(struct weston_surface *surface,
> > int32_t dx, int32_t dy)
> >          * will not be drawn either.
> >          */
> >         if (!weston_surface_is_mapped(surface)) {
> > +               struct weston_output *output;
> > +
> >                 /* Cannot call weston_surface_update_transform(),
> >                  * because that would call it also for the parent surface,
> >                  * which might not be mapped yet. That would lead to
> > @@ -2521,11 +2523,14 @@ subsurface_configure(struct weston_surface
> > *surface, int32_t dx, int32_t dy)
> >                  * Instead just assing any output, to make
> >                  * weston_surface_is_mapped() return true, so that when the
> >                  * parent surface does get mapped, this one will get
> > -                * included, too. See surface_list_add().
> > +                * included, too. See view_list_add().
> >                  */
> >                 assert(!wl_list_empty(&compositor->output_list));
> > -               surface->output =
> > container_of(compositor->output_list.next,
> > -                                              struct weston_output, link);
> > +               output = container_of(compositor->output_list.next,
> > +                                     struct weston_output, link);
> > +
> > +               surface->output = output;
> > +               weston_surface_update_output_mask(surface, 1 <<
> > output->id);
> >         }
> >  }
> >
> > --
> > 1.8.5.5
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >