On Tue, 26 Jun 2018 09:28:42 +0000
Marius-cristian Vlad <[email protected]> wrote:

> On Thu, 2018-06-21 at 15:53 +0300, Pekka Paalanen wrote:
> > From: Pekka Paalanen <[email protected]>
> > 
> > When the compositor has multiple outputs (not clones) and one of them
> > is
> > removed, the ones remaining to the right will be moved to close the
> > gap.
> > Because reflowing the remaining outputs happens before removing the
> > wl_output global, we get the new output x,y before the removal. This
> > causes us to consider the remaining output immediately to the right
> > of
> > the removed output to be a clone of the removed output whose x,y
> > don't
> > get updated. That will then hit the two assertions this patch
> > removes.
> > 
> > The reason the assertions were not actually hit is because of a
> > compositor bug which moved the remaining outputs in the wrong
> > direction.
> > The next patch will fix the reflow, so we need this patch first to
> > avoid
> > the asserts.
> > 
> > Remove the assertions and hand over the background and panel if the
> > "clone" does not already have them. If the clone already has them, we
> > destroy the unnecessary background and panel.
> > 
> > Signed-off-by: Pekka Paalanen <[email protected]>
> > ---
> >  clients/desktop-shell.c | 37 ++++++++++++++++++++++++-------------
> >  1 file changed, 24 insertions(+), 13 deletions(-)
> > 
> > diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> > index 6d19d029..fcc0b657 100644
> > --- a/clients/desktop-shell.c
> > +++ b/clients/desktop-shell.c
> > @@ -1337,19 +1337,30 @@ output_remove(struct desktop *desktop, struct
> > output *output)
> >     }
> >  
> >     if (rep) {
> > -           /* If found, hand over the background and panel so
> > they don't
> > -            * get destroyed. */
> > -           assert(!rep->background);
> > -           assert(!rep->panel);
> > -
> > -           rep->background = output->background;
> > -           output->background = NULL;
> > -           rep->background->owner = rep;
> > -
> > -           rep->panel = output->panel;
> > -           output->panel = NULL;
> > -           if (rep->panel)
> > -                   rep->panel->owner = rep;
> > +           /* If found and it does not already have a
> > background or panel,
> > +            * hand over the background and panel so they don't
> > get
> > +            * destroyed.
> > +            *
> > +            * We never create multiple backgrounds or panels
> > for clones,
> > +            * but if the compositor moves outputs, a pair of
> > wl_outputs
> > +            * might become "clones". This may happen
> > temporarily when
> > +            * an output is about to be removed and the rest are
> > reflowed.
> > +            * In this case it is correct to let the
> > background/panel be
> > +            * destroyed.
> > +            */
> > +
> > +           if (!rep->background) {
> > +                   rep->background = output->background;
> > +                   output->background = NULL;
> > +                   rep->background->owner = rep;
> > +           }
> > +
> > +           if (!rep->panel) {
> > +                   rep->panel = output->panel;
> > +                   output->panel = NULL;
> > +                   if (rep->panel)  
> Guess copy-pasta from above, but is the test necessary? 

A background is mandatory, but a panel is optional: output->panel can
be NULL. Therefore the test is not needed for the background but it is
needed for the panel. This is the same as in the old code.

> 
> Reviewed-by: Marius Vlad <[email protected]>

Thanks,
pq


> 
> > +                           rep->panel->owner = rep;
> > +           }
> >     }
> >  
> >     output_destroy(output)  

Attachment: pgppGKQshxttW.pgp
Description: OpenPGP digital signature

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

Reply via email to