On Fri, 7 Apr 2017 20:58:45 +0200 Armin Krezović <[email protected]> wrote:
> On 04.04.2017 12:58, Pekka Paalanen wrote: > > From: Pekka Paalanen <[email protected]> > > > > A weston_output available to the compositor should always be either in > > the pending or the live outputs list. Let weston_compositor_add_output() > > and weston_compositor_remove_output() handle the moves between the > > lists. > > > > This way weston_output_enable() does not need to remove and > > oops-it-failed-add-it-back. weston_output_disable() does not need to > > manually re-add the output back to the pending list. > > > > To make everything nicely symmetric and fix any unbalancing caused by > > this: > > - weston_output_destroy() explicitly wl_list_remove()s > > - weston_compositor_add_pending_output() first removes then inserts, as > > we have the assumption that the link is always valid, even if empty. > > > > Update the documentations, too. > > > > Hi again. > > > Signed-off-by: Pekka Paalanen <[email protected]> > > --- > > libweston/compositor.c | 28 +++++++++++++++------------- > > 1 file changed, 15 insertions(+), 13 deletions(-) > > > > diff --git a/libweston/compositor.c b/libweston/compositor.c > > index 60cbae0..09a3db2 100644 > > --- a/libweston/compositor.c > > +++ b/libweston/compositor.c > > @@ -4537,6 +4541,8 @@ weston_output_enable_undo(struct weston_output > > *output) > > * > > * - wl_output protocol objects referencing this weston_output > > * are made inert. > > + * > > + * - The output is put back in the pending outputs list. > > */ > > static void > > weston_compositor_remove_output(struct weston_output *output) > > @@ -4555,7 +4561,6 @@ weston_compositor_remove_output(struct weston_output > > *output) > > weston_presentation_feedback_discard_list(&output->feedback_list); > > > > weston_compositor_reflow_outputs(compositor, output, output->width); > > - wl_list_remove(&output->link); > > > > When I looked more closely at this part when I applied your patches, I found a > potential problem with moving this down below. The original code first removed > the output from the enabled output list, then emitted the signals. > > Signal listeners might iterate through output list before this function > "returns", > ie, to pick a new output for a view, and could potentially pick this output > again. > > Even worse, wl_list_empty(&c->output_list) will return false even if this is > the > last output going away, throwing previous work about "all outputs gone at > runtime" > down the drain. Hi Armin, yes, a very good catch. I'll fix that, probably by moving both remove and insert to where the remove used to be. In fact, I'm thinking if that list manipulation should be the very first thing that function does. I've also fixed the "live outputs" wording. Thanks, pq > > wl_signal_emit(&compositor->output_destroyed_signal, output); > > wl_signal_emit(&output->destroy_signal, output); > > @@ -4563,6 +4568,9 @@ weston_compositor_remove_output(struct weston_output > > *output) > > wl_resource_for_each(resource, &output->resource_list) { > > wl_resource_set_destructor(resource, NULL); > > } > > + > > + wl_list_remove(&output->link); > > + wl_list_insert(compositor->pending_output_list.prev, &output->link); > > } > > > >
pgpH8ofN5DsRs.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
