[PATCH weston] compositor: Separate hardcoded backend related code from compositor

[Giulio Camuffo]

2015-06-23 21:57 GMT+03:00 JoonCheol Park <jooncheol at gmail.com>:
>
>
> 2015-06-24 2:49 GMT+09:00 Giulio Camuffo <giuliocamuffo at gmail.com>:
>>
>> 2015-06-23 20:40 GMT+03:00 JoonCheol Park <jooncheol at gmail.com>:
>> > Hi Giulio, pq!
>> >
>> >
>> > 2015-06-23 20:57 GMT+09:00 Pekka Paalanen <ppaalanen at gmail.com>:
>> >>
>> >> On Tue, 23 Jun 2015 14:14:53 +0300
>> >> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>> >>
>> >> > Hi,
>> >> >
>> >> > This kinda goes in the opposite direction to my libweston patches. In
>> >> > that series i move the command line handling away from the backends
>> >> > code to make them work in multiple compositors. This moves even more
>> >> > compositor-specific stuff in the backends so we need to decide if we
>> >> > want this or libweston.
>> >>
>> >> We want libweston.
>> >
>> >
>> > I just quick review the libweston on the github
>> > (https://github.com/giucam/weston/). When it will be applied to
>> > mainstream ?
>> >
>> > My understanding about libweston is for reuse the x11/drm related code
>> > for
>> > other compositor project (like libinput). Yes, It is everyone want :-)
>> > But, since the backend structure which manages multiple plugins and load
>> > symbol 'backend_xxx()'  is weston compositor specific stuff and it is
>> > not in
>> > the scope of libweston, I believe my proposal is not the opposite idea
>> > to
>> > the libweston what you are preparing.
>> > (In your repository, my proposal may change the src/weston.c to remove
>> > hardcoded backend option outputs. It is not part of libweston library. )
>>
>
>
> Which branch should I see on your repository? libweston ? libweston-v2 ?
> I found that you mentioned structure in branch "libweston-v2". (right ?)

libweston-v2 was slighly old, i updated it now.

>
>>
>> Well, but you are adding weston specific stuff into the backends code,
>> that is the options handling.
>
>
> backend plugin (and option handling) is the *weston* compositor specific.
> so this is not the problem I think.

The backends are not weston specific, that is one of the point of
libweston, to share them. Only the initialization is compositor
specific.

>
>
>>
>> In libweston the backends do not parse
>> the command line or the config file, so all the backend options are
>> weston specific.
>>
> In your code (src/weston.c, branch libweston-v2), as you explained, the
> weston (compositor main) have the code for parse options for all known
> backend plugin before init it. But, there are many hard code option outputs
> for only known backend plugins. In this case, we don't have to load plugin
> file dynamically.
>
> And it use the "weston_compositor_init_backend" of the libweston.so which
> has hard coded backend list.
> I think this function should not be included in the libweston.so if its goal
> is to be reused for other compositor. Since the
> "weston_compositor_init_backend" is for weston specific backend plugin
> initialization, it is not necessary function for others.

actually the version i sent to the list doesn't have the backends
enum, they are loaded like now by using the filename of the .so or the
absolute path.
and as i said above the backends are meant to be shared between compositor.

>
>
>
>>
>> >
>> >
>> >>
>>
>> >> > 2015-06-23 13:29 GMT+03:00 JoonCheol Park <jooncheol at gmail.com>:
>> >> > > Instead of adding available backends and usage outputs at build
>> >> > > time,
>> >> > > this patch finds all available backend plugins in MODULEDIR and
>> >> > > prints
>> >> > > them. It also prints usage output for selected backend by calling
>> >> > > "backend_usage()" in the plugin.
>> >> > >
>> >> > > By this patch, we can remove all hardcode for backends from
>> >> > > compositor.
>> >> > > 3rd party backend plugin can be listed in help output. Backend
>> >> > > developer
>> >> > > can freely add additional description for backend.
>> >> > >
>> >> > > Signed-off-by: JoonCheol Park <jooncheol at gmail.com>
>> >> > > ---
>> >> > >  src/compositor-drm.c      |  11 ++++
>> >> > >  src/compositor-fbdev.c    |   9 +++
>> >> > >  src/compositor-headless.c |  11 ++++
>> >> > >  src/compositor-rdp.c      |  15 +++++
>> >> > >  src/compositor-rpi.c      |  12 ++++
>> >> > >  src/compositor-wayland.c  |  14 +++++
>> >> > >  src/compositor-x11.c      |  13 +++++
>> >> > >  src/compositor.c          | 141
>> >> > > ++++++++++++----------------------------------
>> >> > >  src/compositor.h          |   3 +
>> >> > >  9 files changed, 125 insertions(+), 104 deletions(-)
>> >>
>> >> JoonCheol, any particular reason you are proposing this?
>> >> Are you building external backends?
>> >
>> >
>> > Yeap, I'm trying to make some fun experiments based on headless backend
>> > code.
>> > When I tried to add new options for backend plugin, I had to add option
>> > outputs for my backend into main compositor code. not in the plugin
>> > itself.
>> > Since it is kind of hardcode, I thought it doesn't look good. I think
>> > that
>> > plugin should have such information of itself and compositor should load
>> > such information from plugin file.
>> >
>> > (similar case: In case of NPAPI, Web browser can show plugin's
>> > information
>> > by getting/calling "NP_GetMIMEDescription()" from each plugin .so file.
>> > e.g
>> > - about:config in Mozilla)
>> >
>> > So, I created this patch for better (weston specific) backend plugin
>> > management structure.
>> >
>> >>
>> >> AFAIK the Weston SDK (the installed headers) for external plugins never
>> >> supported external backends, so I'm curious.
>> >>
>> >
>> > Since this kind of plugin just need header files for building and
>> > weston.pc
>> > is also already supported, I thought that building external backend
>> > plugin
>> > is supported (and ideally possible in current version of weston)... but
>> > wasn't it??
>> > If it can be supported, it would be good for like my case. Developer can
>> > create the backend plugin without build all weston source.. (like other
>> > '-dev' pkg supported program)
>> >
>> >
>> >> We are going in the direction of backends becoming libweston internal
>> >> details, not something you can plug and switch arbitrarily, at least
>> >> for the middle-term.
>> >>
>> >>
>> >>
>> >>
>> >> Thanks,
>> >> pq
>> >
>> >
>
>