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.
> 
> Signed-off-by: Pekka Paalanen <[email protected]>

Nice simplification, just one minor question down below.

Reviewed-by: Armin Krezović <[email protected]>

Thanks, Armin.

> ---
>  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
> @@ -4459,8 +4459,10 @@ weston_output_move(struct weston_output *output, int 
> x, int y)
>       }
>  }
>  
> -/** Adds an output to the compositor's output list and
> - *  send the compositor's output_created signal.
> +/** Signal that a pending output is taken into use.
> + *
> + * Removes the output from the pending list and adds it
> + * to the compositor's live outputs list. The output created signal is 
> emitted.
>   *

Again, live -> enabled?

>   * \param compositor The compositor instance.
>   * \param output The output to be added.
> @@ -4471,7 +4473,9 @@ weston_compositor_add_output(struct weston_compositor 
> *compositor,
>  {
>       struct weston_view *view, *next;
>  
> +     wl_list_remove(&output->link);
>       wl_list_insert(compositor->output_list.prev, &output->link);
> +
>       wl_signal_emit(&compositor->output_created_signal, output);
>  
>       wl_list_for_each_safe(view, next, &compositor->view_list, link)
> @@ -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);
>  
>       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);
>  }
>  
>  /** Sets the output scale for a given output.
> @@ -4651,11 +4659,14 @@ weston_output_init(struct weston_output *output,
>   *
>   * Also notifies the compositor that an output is pending for
>   * configuration.
> + *
> + * The opposite of this operation is built into weston_output_destroy().
>   */
>  WL_EXPORT void
>  weston_compositor_add_pending_output(struct weston_output *output,
>                                    struct weston_compositor *compositor)
>  {
> +     wl_list_remove(&output->link);
>       wl_list_insert(compositor->pending_output_list.prev, &output->link);
>       wl_signal_emit(&compositor->output_pending_signal, output);
>  }
> @@ -4713,9 +4724,6 @@ weston_output_enable(struct weston_output *output)
>       /* Make sure we have a transform set */
>       assert(output->transform != UINT32_MAX);
>  
> -     /* Remove it from pending/disabled output list */
> -     wl_list_remove(&output->link);
> -
>       /* Verify we haven't reached the limit of 32 available output IDs */
>       assert(ffs(~c->output_id_pool) > 0);
>  
> @@ -4735,7 +4743,6 @@ weston_output_enable(struct weston_output *output)
>       wl_list_init(&output->animation_list);
>       wl_list_init(&output->resource_list);
>       wl_list_init(&output->feedback_list);
> -     wl_list_init(&output->link);
>  
>       /* Invert the output id pool and look for the lowest numbered
>        * switch (the least significant bit).  Take that bit's position
> @@ -4758,8 +4765,6 @@ weston_output_enable(struct weston_output *output)
>               weston_log("Enabling output \"%s\" failed.\n", output->name);
>  
>               weston_output_enable_undo(output);
> -             wl_list_insert(output->compositor->pending_output_list.prev,
> -                            &output->link);
>               return -1;
>       }
>  
> @@ -4812,10 +4817,6 @@ weston_output_disable(struct weston_output *output)
>       if (output->enabled) {
>               weston_compositor_remove_output(output);
>               weston_output_enable_undo(output);
> -
> -             /* We need to preserve it somewhere so it can be destroyed on 
> shutdown
> -                if nobody wants to configure it again */
> -             wl_list_insert(output->compositor->pending_output_list.prev, 
> &output->link);
>       }
>  
>       output->destroying = 0;
> @@ -4844,6 +4845,7 @@ weston_output_destroy(struct weston_output *output)
>               weston_output_enable_undo(output);
>       }
>  
> +     wl_list_remove(&output->link);
>       free(output->name);
>  }
>  
> 


Attachment: signature.asc
Description: OpenPGP digital signature

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

Reply via email to