[PATCH weston 09/15] libweston: specify weston_output::enabled

[Armin Krezović]

On 04.04.2017 12:58, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 

Hi,

> It was ambiguous what this flag meant - it did not mean whether the
> backend is considering this output to be enabled, because
> weston_output_destroy() unsets it while deliberately not calling the
> backend disable() vfunc.
> 
> Perhaps the most clear definition is with respect to the output's
> assignment in the pending vs. live output lists. There is also a whole
> bunch of variables that are allocated only when enabled is true.
> 

Can you guess what I'm doing to ask here? :P

> Since the flag is related to the list membership, set and clear the flag
> only when manipulating the lists.
> 
> Assert that weston_compositor_add_output() and
> weston_compositor_remove_output() are not called in a wrong state.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin at gmail.com>

> ---
>  libweston/compositor.c | 9 ++++-----
>  libweston/compositor.h | 2 +-
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 4e6d9f7..e7293ea 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4478,8 +4478,10 @@ weston_compositor_add_output(struct weston_compositor *compositor,
>  {
>  	struct weston_view *view, *next;
>  
> +	assert(!output->enabled);
>  	wl_list_remove(&output->link);
>  	wl_list_insert(compositor->output_list.prev, &output->link);
> +	output->enabled = true;
>  
>  	wl_signal_emit(&compositor->output_created_signal, output);
>  
> @@ -4535,8 +4537,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);
> -
> -	output->enabled = false;
>  }
>  
>  /** Removes output from compositor's live outputs list
> @@ -4574,6 +4574,7 @@ weston_compositor_remove_output(struct weston_output *output)
>  	struct weston_view *view;
>  
>  	assert(output->destroying);
> +	assert(output->enabled);
>  
>  	wl_list_for_each(view, &compositor->view_list, link) {
>  		if (view->output_mask & (1u << output->id))
> @@ -4593,6 +4594,7 @@ 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;
>  }
>  
>  /** Sets the output scale for a given output.
> @@ -4668,7 +4670,6 @@ weston_output_init(struct weston_output *output,
>  	assert(output->name);
>  
>  	wl_list_init(&output->link);
> -
>  	output->enabled = false;
>  
>  	/* Add some (in)sane defaults which can be used
> @@ -4788,8 +4789,6 @@ weston_output_enable(struct weston_output *output)
>  		wl_global_create(c->wl_display, &wl_output_interface, 3,
>  				 output, bind_output);
>  
> -	output->enabled = true;
> -
>  	/* Enable the output (set up the crtc or create a
>  	 * window representing the output, set up the
>  	 * renderer, etc)
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index 28e9466..4eebe6c 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -233,7 +233,7 @@ struct weston_output {
>  
>  	struct weston_timeline_object timeline;
>  
> -	bool enabled;
> +	bool enabled; /**< is in the output_list, not pending list */

live output list? :P

>  	int scale;
>  
>  	int (*enable)(struct weston_output *output);
> 

Thanks, Armin.

-------------- 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/a750699c/attachment-0001.sig>