On 04.04.2017 12:58, Pekka Paalanen wrote:
> From: Pekka Paalanen <[email protected]>
> 

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 <[email protected]>
Reviewed-by: Armin Krezović <[email protected]>

> ---
>  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.

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