[PATCH weston 11/15] libweston: move output id into add/remove_output()

[Armin Krezović]

On 04.04.2017 12:58, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> Move the output id management into weston_compositor_add_output() and
> weston_compositor_remove_output(). This is a more logical place, and
> works towards assimilating weston_output_enable_undo().
> 
> The output id is no longer available to the backend enable() vfuncs, but
> it was not used there to begin with.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>  libweston/compositor.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 987dd99..e2d28db 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4467,6 +4467,8 @@ weston_output_move(struct weston_output *output, int x, int y)
>   * Removes the output from the pending list and adds it
>   * to the compositor's live outputs list. The output created signal is emitted.
>   *
> + * The output gets an internal ID assigned.
> + *
>   * \param compositor The compositor instance.
>   * \param output The output to be added.
>   *
> @@ -4478,6 +4480,16 @@ weston_compositor_add_output(struct weston_compositor *compositor,
>  {
>  	struct weston_view *view, *next;
>  
> +	/* Verify we haven't reached the limit of 32 available output IDs */
> +	assert(ffs(~compositor->output_id_pool) > 0);
> +
> +	/* Invert the output id pool and look for the lowest numbered
> +	 * switch (the least significant bit).  Take that bit's position
> +	 * as our ID, and mark it used in the compositor's output_id_pool.
> +	 */
> +	output->id = ffs(~compositor->output_id_pool) - 1;
> +	compositor->output_id_pool |= 1u << output->id;
> +
>  	assert(!output->enabled);

Hi,

I'd move this assert line before/after the assert above. You might end up overwriting
an already assigned id if the output is previously enabled (not that it would matter,
given that this assert would terminate the compositor immediately).

Thanks, Armin.

>  	wl_list_remove(&output->link);
>  	wl_list_insert(compositor->output_list.prev, &output->link);
> @@ -4527,7 +4539,6 @@ weston_output_transform_coordinate(struct weston_output *output,
>   * Removes the repaint timer.
>   * Destroys the Wayland global assigned to the output.
>   * Destroys pixman regions allocated to the output.
> - * Deallocates output's ID and updates compositor's output_id_pool.
>   */
>  static void
>  weston_output_enable_undo(struct weston_output *output)
> @@ -4536,7 +4547,6 @@ weston_output_enable_undo(struct weston_output *output)
>  
>  	pixman_region32_fini(&output->region);
>  	pixman_region32_fini(&output->previous_damage);
> -	output->compositor->output_id_pool &= ~(1u << output->id);
>  }
>  
>  /** Removes output from compositor's live outputs list
> @@ -4563,6 +4573,8 @@ weston_output_enable_undo(struct weston_output *output)
>   *
>   * - The output is put back in the pending outputs list.
>   *
> + * - The output's internal ID is released.
> + *
>   * \memberof weston_output
>   * \internal
>   */
> @@ -4595,6 +4607,9 @@ weston_compositor_remove_output(struct weston_output *output)
>  	wl_list_remove(&output->link);
>  	wl_list_insert(compositor->pending_output_list.prev, &output->link);
>  	output->enabled = false;
> +
> +	compositor->output_id_pool &= ~(1u << output->id);
> +	output->id = 0xffffffff; /* invalid */
>  }
>  
>  /** Sets the output scale for a given output.
> @@ -4765,9 +4780,6 @@ weston_output_enable(struct weston_output *output)
>  	/* Make sure we have a transform set */
>  	assert(output->transform != UINT32_MAX);
>  
> -	/* Verify we haven't reached the limit of 32 available output IDs */
> -	assert(ffs(~c->output_id_pool) > 0);
> -
>  	output->x = x;
>  	output->y = y;
>  	output->dirty = 1;
> @@ -4785,13 +4797,6 @@ weston_output_enable(struct weston_output *output)
>  	wl_list_init(&output->resource_list);
>  	wl_list_init(&output->feedback_list);
>  
> -	/* Invert the output id pool and look for the lowest numbered
> -	 * switch (the least significant bit).  Take that bit's position
> -	 * as our ID, and mark it used in the compositor's output_id_pool.
> -	 */
> -	output->id = ffs(~output->compositor->output_id_pool) - 1;
> -	output->compositor->output_id_pool |= 1u << output->id;
> -
>  	output->global =
>  		wl_global_create(c->wl_display, &wl_output_interface, 3,
>  				 output, bind_output);
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 870 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170404/b6998029/attachment.sig>