[PATCH weston v5 14/36] libweston: new head-based output management API

[Pekka Paalanen]

On Fri, 2 Feb 2018 15:00:09 -0600
Derek Foreman <derekf at osg.samsung.com> wrote:

> On 2017-12-14 05:40 AM, Pekka Paalanen wrote:
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > 
> > Introduce the API for users (compositors) to create an output from a
> > head, attach and detach heads, and destroy outputs created this way.
> > This also adds the backend-facing API to libweston.
> > 
> > In the new API design, a backend creates heads, and the compositor
> > chooses one or more heads (clone mode) to be driven by an output.
> > In the future backends will be converted to not create outputs directly
> > but only in the new create_output hook.
> > 
> > The user subscribes to a heads_changed hook and arranges heads into
> > outputs from there.
> > 
> > Adding the API this way will allow frontends (main.c) and backends to be
> > converted one by one. This adds compatiblity paths in
> > weston_compositor_create_output_with_head() and weston_output_destroy()
> > so that frontends can be converted first to call these, and then
> > backends can be converted one by one to the new design. Afterwards, the
> > compatibility paths will be removed along with weston_output::head.
> > 
> > Currently heads can be added to a disabled output only. This is less
> > than ideal for clone mode hotplug and should be improved on later.
> > 
> > v4: Remove the wl_output global on head detach if output is enabled.
> > 
> > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > ---
> >   libweston/compositor.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++---
> >   libweston/compositor.h |  78 +++++++++++++++++++++
> >   2 files changed, 256 insertions(+), 9 deletions(-)
> > 
> > diff --git a/libweston/compositor.c b/libweston/compositor.c

> > @@ -4583,7 +4629,7 @@ weston_compositor_iterate_heads(struct weston_compositor *compositor,
> >    *
> >    * \memberof weston_output
> >    */
> > -static int
> > +WL_EXPORT int
> >   weston_output_attach_head(struct weston_output *output,
> >   			  struct weston_head *head)
> >   {
> > @@ -4593,9 +4639,13 @@ weston_output_attach_head(struct weston_output *output,
> >   	if (!wl_list_empty(&head->output_link))
> >   		return -1;
> >   
> > -	/* XXX: no support for multi-head yet */
> > -	if (!wl_list_empty(&output->head_list))
> > +	if (output->attach_head) {
> > +		if (output->attach_head(output, head) < 0)
> > +			return -1;
> > +	} else if (!wl_list_empty(&output->head_list)) {
> > +		/* No support for clones in the legacy path. */
> >   		return -1;
> > +	}
> >   
> >   	head->output = output;
> >   	wl_list_insert(output->head_list.prev, &head->output_link);

> > +/** Create an output for an unused head
> > + *
> > + * \param compositor The compositor.
> > + * \param head The head to attach to the output.
> > + * \return A new \c weston_output, or NULL on failure.
> > + *
> > + * This creates a new weston_output that starts with the given head attached.
> > + * The output inherits the name of the head. The head must not be already
> > + * attached to another output.
> > + *
> > + * An output must be configured before it can be enabled.
> > + *
> > + * \memberof weston_compositor
> > + */
> > +WL_EXPORT struct weston_output *
> > +weston_compositor_create_output_with_head(struct weston_compositor *compositor,
> > +					  struct weston_head *head)
> > +{
> > +	struct weston_output *output;
> > +
> > +	if (head->allocator_output) {
> > +		/* XXX: compatibility path to be removed after all converted */
> > +		output = head->allocator_output;
> > +	} else {
> > +		assert(compositor->backend->create_output);
> > +		output = compositor->backend->create_output(compositor,
> > +							    head->name);
> > +	}
> > +
> > +	if (!output)
> > +		return NULL;
> > +
> > +	if (weston_output_attach_head(output, head) < 0) {
> > +		if (!head->allocator_output)
> > +			output->destroy(output);
> > +
> > +		return NULL;
> > +	}
> > +
> > +	return output;
> > +}
> > +
> > +/** Destroy an output
> > + *
> > + * \param output The output to destroy.
> > + *
> > + * The heads attached to the given output are detached and become unused again.
> > + *
> > + * It is not necessary to explicitly destroy all outputs at compositor exit.
> > + * weston_compositor_destroy() will automatically destroy any remaining
> > + * outputs.
> > + *
> > + * \memberof weston_output
> > + */
> > +WL_EXPORT void
> > +weston_output_destroy(struct weston_output *output)
> > +{
> > +	struct weston_head *head;
> > +
> > +	/* XXX: compatibility path to be removed after all converted */
> > +	head = weston_output_get_first_head(output);  
> 
> This took me a couple of reads...  if I understand correctly, the full 
> list of outputs is going to be torn down in the backend 
> output->destroy()?  And we hit that path if we don't have the legacy 
> allocator_output.

Yes, the backend will call weston_output_release() which will detach
all attached heads.

In the legacy backend case, the backend is in charge of creating and
destroying weston_outputs, so we cannot let weston_output_destroy()
call output->destroy(). With legacy frontend things are as they were,
but a migrated frontend will call weston_output_destroy().

In the migrated backend case, weston_outputs are created and destroyed
by the frontend's command.

If we have a migrated frontend and a legacy backend, we just pretend
the frontend is in charge of the weston_output lifetime.

It is enough to check only the first head for the legacy backend case,
because in that case there cannot be more heads attached.
weston_output_attach_head() ensures that.

> I think the code all looks sound and the API looks reasonable to me, but 
> I have to concede that I don't know the output handling path enough to 
> understand weston_compositor_reflow_output's setting up of the 
> allocator_output.

Why do you mention weston_compositor_reflow_outputs()?

allocator_output is a note in weston_head to say we have a legacy
backend that created a weston_output automatically. So when a migrated
frontend goes to weston_compositor_create_output_with_head() to create
a new weston_output, we can return the (pending) weston_output that
already exists.

> Acked-by: Derek Foreman <derekf at osg.samsung.com>

Thanks,
pq

> > +	if (head->allocator_output) {
> > +		/* The old design: backend is responsible for destroying the
> > +		 * output, so just undo create_output_with_head()
> > +		 */
> > +		weston_head_detach(head);
> > +		return;
> > +	}
> > +
> > +	output->destroy(output);
> > +}
> > +
> >   /** When you need a head...
> >    *
> >    * This function is a hack, used until all code has been converted to become
> > diff --git a/libweston/compositor.h b/libweston/compositor.h
> > index f7b7050c..f9d034c3 100644
> > --- a/libweston/compositor.h
> > +++ b/libweston/compositor.h
> > @@ -172,6 +172,8 @@ struct weston_head {
> >   
> >   	char *name;			/**< head name, e.g. connector name */
> >   	bool connected;			/**< is physically connected */
> > +
> > +	struct weston_output *allocator_output;	/**< XXX: to be removed */
> >   };
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180205/b8e2899c/attachment.sig>