[vlc-devel] [PATCH v2 3/3] add vulkan wayland platform

[Alexandre Janniaux]

Hi,

On Thu, May 07, 2020 at 09:52:08PM +0300, Rémi Denis-Courmont wrote:
> Le torstaina 7. toukokuuta 2020, 20.42.34 EEST Alexandre Janniaux a écrit :
> > ---
> >  modules/video_output/wayland/Makefile.am | 11 +++++
> >  modules/video_output/wayland/vulkan.c    | 57 ++++++++++++++++++++++++
> >  2 files changed, 68 insertions(+)
> >  create mode 100644 modules/video_output/wayland/vulkan.c
> >
> > diff --git a/modules/video_output/wayland/Makefile.am
> > b/modules/video_output/wayland/Makefile.am index 65dc2ad2992..2e0467ebb23
> > 100644
> > --- a/modules/video_output/wayland/Makefile.am
> > +++ b/modules/video_output/wayland/Makefile.am
> > @@ -73,6 +73,14 @@ libegl_wl_plugin_la_CPPFLAGS = $(AM_CPPFLAGS)
> > -DUSE_PLATFORM_WAYLAND=1 libegl_wl_plugin_la_CFLAGS = $(AM_CFLAGS)
> > $(EGL_CFLAGS) $(WAYLAND_EGL_CFLAGS) libegl_wl_plugin_la_LIBADD =
> > $(EGL_LIBS) $(WAYLAND_EGL_LIBS)
> >
> > +libvk_wl_plugin_la_SOURCES = $(VULKAN_COMMONSOURCES) \
> > +	video_output/vulkan/surface.c \
> > +	video_output/wayland/vulkan.c
>
> The shorcuts taken by surface.c are okay as long as there was at mosty one
> "vulkan" module per install. surface.c is not too big, so it's okay to
> replicate the code but e.g. you should not have the same name for config items
> of different backends.
>
> Also static strings are better defined as arrays than constant pointers (to an
> array), and you can't have macros in module name and description as it breaks
> gettext.
>
> It's a bit unfair that it hits the Wayland patch, but it's effectively the first
> time that a single build would have multiple backends.

I've fixed those issue locally by moving each platform into a separate
module capability, moving the placebo code from surface.c to display.c
including options and removing the previous surface.c.

However, the options that I moved are not prefixed and some of them
don't make sense for now (like the «vulkan» capability that was only
implemented by the surface.c module and should probably be created
again once we can have different vulkan backend, ie. when we refactor
the code to create the VkInstance and forward it to libplacebo
instead of the reverse).

Does it make sense if I remove these? Do I need to make the previous
options obsolete before prefixing them?

Thanks for feedback :)

Regards,
--
Alexandre Janniaux
Videolabs