[PATCH weston 04/11] make the backends compositor-neutral

[Pekka Paalanen]

On Sat, 27 Jun 2015 12:43:09 +0300
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> 2015-06-24 4:46 GMT+03:00 Jon A. Cruz <jonc at osg.samsung.com>:
> > On 06/22/2015 10:51 PM, Giulio Camuffo wrote:
> >> 2015-06-23 3:52 GMT+03:00 Bill Spitzak <spitzak at gmail.com>:
> >>> It seems like prev
> >>>
> >>> On Mon, Jun 22, 2015 at 1:02 PM, Giulio Camuffo <giuliocamuffo at gmail.com>
> >>> wrote:
> >>>>
> >>>> The backends used to have lots of code dealing with weston specific
> >>>> configs.
> >>>> To allow them to be used by other compositors with their own
> >>>> configurations
> >>>> remove all the usage of weston_config from the backends code and move it
> >>>> in weston.c.

> > First, the aspect that the backends are half-dynamic and half-hardcoded
> > is an aspect that raised some flags in my mind right away. Since patch 2
> > added an API to manage instances, it seems good to try to go with that
> > all the way. And then minimizing the use of void* would help, since that
> > is often a source of problems. (to clarify, using 'void *' for an opaque
> > value to be returned for callbacks is good, but using one as a key input
> > that must be of the correct type for the situation gets to be very
> > problematic).

As I see it, the main reason the backends are .so files instead of
built into libweston or the main executable is that we can restrict the
dependencies.

I can well imagine a packager splitting Weston into several packages,
for example: core, gl-renderer, x11-backend, drm-backend, xwayland

- gl-renderer pulls in dependency to libEGL and GLESv2
- x11-backend and xwayland pull in X11 libs
- drm-backend pulls in libdrm and libgbm (which probably pulls in Mesa
  or something)

In all other aspects, I consider backends an integral part of
libweston, and could just be built in to libweston instead of
separate .so files.

It should also improve startup time if you don't have to load all
possible libraries but only the ones you are going to need.

> > One design choice would be to have backends parse out what they need.

That is precisely where I would not want to go.

> > Another would be to remove all knowledge of parsing and parameters and
> > leave that up to the hosting code.

That is what I believe should work best, even if it causes some burden
on the hosting code.

> > This patch had started on the route of moving things out of the parser,
> > but then we end up with a lot in main.c/weston.c. If this was chosen
> > then I'd suggest breaking those parts out to a separate file so the ugly
> > guts of backend-specific hardcoding was more encapsulated.
> >
> > However... I personally would prefer to see more follow-up on the API
> > added in patch 2.
> >
> > One way to facilitate this would be to add a call for fetching the
> > option-parser struct details from a backend. Then adding argc and argv
> > to the *init() entrypoint would remove the need for all the hardcoding
> > in weston.c/main.c. The main code could pull out known parameters from
> > the argv list and pass a controlled (or entirely synthesized) list to
> > the backend.
> 
> If the main code needs to check the argv and remove unknown parameters
> i don't see what we gain, to be honest, since we'll still have code
> dealing with the backend parameters outside of the backends code.
> And letting the backends parse out their options from the original
> argv instead is not an option imho. For example, the drm backend has
> the --use-pixman option, but that may collide with a compositor's
> requirement to use opengl.

I agree with Giulio here. We cannot avoid the "main.c" knowing about
all possible parameters a backend might take. Main needs to know about
them, and main chooses which parameters it exposes to the user via
command line and/or config file options.

Thinking about the goal of an API-stable libweston, having a
backend-specific struct with all parameters might not be the best way;
an alternative is the libinput way of exposing configuration options
which has AFAIU three functions for each knob. Adding new knobs only
adds to the API/ABI without breaking it.

We could implement the same with structs, too. Like this:
struct weston_backend_config {
	enum backend_type type;
	int magic;
};

struct foo_backend_config {
	struct weston_backend_config base;
	...
};

The magic would be sizeof(struct foo_backend_config). The type would
indicate for which backend the caller prepared this, and is just an
additional safety.

An old main with a new libweston which added new options for a backend
would have old magic: the backend would know to ignore the new entries.

These are only suggestions, but I believe the right approach is to have
the main in charge of all options exposed to the user. I do not think
letting libweston and backends to define arbitrary new options without
the main ever knowing about them is a good idea. If you accept this,
then the remaining question is what should the option-setting API of
libweston look like. We can very well leave that question open for now,
and just use whatever works at the moment.

Manufacturing an argv-like array of strings seems silly to me. It only
leads to "snprintf" followed by "sscanf" on the other side, while we
could just pass already parsed values through. Main needs to parse them
anyway if it wants to filter them.

There is also the question of runtime-appearing objects that need
configuration. Outputs are a perfect example: a hotplug may create a
new output, and then you may want to have a special configuration for
it. Can a main predict everything and encode all configurations a
priori during the compositor init, or we allow the main to make up a
configuraton when something new appears?

The latter cannot really be implemented without some kind of callbacks.
The benefit is that a main can e.g. detect a class of outputs and apply
particular configuration on them, without the need to list every
possible output name in advance to achieve the same (if that is even
possible).

One more question are parameters that a main might want to change after
initialization. I think those are out of scope for this discussion, as
we are replacing config file and command line options which were only
ever set at startup. If an option becomes more dynamic than that, we
need new API for it, maybe a la libinput.


Thanks,
pq