[PATCH weston 01/12 v2] libweston: Add more functionality for handling weston_output objects

[Pekka Paalanen]

On Sun, 14 Aug 2016 17:28:10 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:

> This patch implements additional functionality that will be used
> for configuring, enabling and disabling weston's outputs. Its
> indended use is by the compositors or user programs that want to
> be able to configure, enable or disable an output at any time. An
> output can only be configured while it's disabled.
> 
> The compositor and backend specific functionality is required
> for these functions to be useful, and those will come later in
> this series.
> 
> All the new functions have been documented, so I'll avoid
> describing them here.

Hi Armin,

the function re-structuring looks much better now. Maybe partly because
it is better, partly because I now have an idea where it aims to be. :-)

This patch is:
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

More tomorrow. Let's see if I can make it for landing enough of these
patches before the alpha release, to be able to land the rest before
beta. I we don't make it, no worries. We can still work on this in a
branch and you can go forward with your GSoC. It would only mean
landing the patches gets delayed by a month.


There are some minor nits below, but I would be ok merging this patch
as is. I might fix them while landing, too.

> v2:
> 
>  - Minor documentation improvements.
>  - Rename output-initialized to output->enabled.
>  - Split weston_output_disable() further into
>    weston_compositor_rename_output().

ITYM remove_output. :-)

>  - Rename weston_output_deinit() to weston_output_enable_undo().
> 
>  - Make weston_output_disable() call two functions mentioned
>    above instead of calling weston_output_disable() directly.
>    This means that backend needs to take care of doing backend
>    specific disable in backend specific destroy function.
> 
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
>  libweston/compositor.c | 351 ++++++++++++++++++++++++++++++++++++++++++++-----
>  libweston/compositor.h |  29 ++++
>  2 files changed, 345 insertions(+), 35 deletions(-)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 98c5bd6..f78f06f 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4138,41 +4138,6 @@ weston_compositor_reflow_outputs(struct weston_compositor *compositor,
>  }
>  
>  WL_EXPORT void
> -weston_output_destroy(struct weston_output *output)
> -{
> -	struct wl_resource *resource;
> -	struct weston_view *view;
> -
> -	output->destroying = 1;
> -
> -	wl_list_for_each(view, &output->compositor->view_list, link) {
> -		if (view->output_mask & (1u << output->id))
> -			weston_view_assign_output(view);
> -	}
> -
> -	wl_event_source_remove(output->repaint_timer);
> -
> -	weston_presentation_feedback_discard_list(&output->feedback_list);
> -
> -	weston_compositor_reflow_outputs(output->compositor, output, output->width);
> -	wl_list_remove(&output->link);
> -
> -	wl_signal_emit(&output->compositor->output_destroyed_signal, output);
> -	wl_signal_emit(&output->destroy_signal, output);
> -
> -	free(output->name);
> -	pixman_region32_fini(&output->region);
> -	pixman_region32_fini(&output->previous_damage);
> -	output->compositor->output_id_pool &= ~(1u << output->id);
> -
> -	wl_resource_for_each(resource, &output->resource_list) {
> -		wl_resource_set_destructor(resource, NULL);
> -	}
> -
> -	wl_global_destroy(output->global);
> -}
> -
> -WL_EXPORT void
>  weston_output_update_matrix(struct weston_output *output)
>  {
>  	float magnification;
> @@ -4362,6 +4327,8 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c,
>  	output->global =
>  		wl_global_create(c->wl_display, &wl_output_interface, 3,
>  				 output, bind_output);
> +
> +	output->enabled = true;
>  }
>  
>  /** Adds an output to the compositor's output list and
> @@ -4400,6 +4367,314 @@ weston_output_transform_coordinate(struct weston_output *output,
>  	*y = p.f[1] / p.f[3];
>  }
>  
> +/** Undoes changes to an output done by weston_output_enable()
> + *
> + * \param output The weston_output object that needs the changes undone.
> + *
> + * 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)
> +{
> +	wl_event_source_remove(output->repaint_timer);
> +
> +	wl_global_destroy(output->global);
> +
> +	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 output list
> + *
> + * \param output The weston_output object that is being removed.
> + *
> + * Presentation feedback is discarded.
> + * Compositor is notified that outputs were changed and
> + * applies the necessary changes.
> + * All views assigned to the weston_output object are
> + * moved to a new output.
> + * Signal is emited to notify all users of the weston_output
> + * object that the output is being destroyed.
> + * Resources assigned to an output are destroyed.

Not destroyed. I'd say the wl_output protocol objects referencing this
weston_output are made inert, because we cannot destroy them without a
client request or disconnect.

> + */
> +static void
> +weston_compositor_remove_output(struct weston_output *output)
> +{
> +	struct wl_resource *resource;
> +	struct weston_view *view;
> +

The following loop depends on output->destroing being set to true, so
we should assert that it really is.

> +	wl_list_for_each(view, &output->compositor->view_list, link) {
> +		if (view->output_mask & (1u << output->id))
> +			weston_view_assign_output(view);
> +	}
> +
> +	weston_presentation_feedback_discard_list(&output->feedback_list);
> +
> +	weston_compositor_reflow_outputs(output->compositor, output, output->width);
> +
> +	wl_signal_emit(&output->compositor->output_destroyed_signal, output);
> +	wl_signal_emit(&output->destroy_signal, output);
> +
> +	wl_resource_for_each(resource, &output->resource_list) {
> +		wl_resource_set_destructor(resource, NULL);
> +	}
> +}


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160815/a897b0fd/attachment-0001.sig>