[Buildroot] [PATCH] package/sdl2: support building for Wayland

[Buildroot] [PATCH] package/sdl2: support building for Wayland

[Alexander Bau]

---
 package/sdl2/Config.in | 7 +++++++
 package/sdl2/sdl2.mk   | 7 ++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/package/sdl2/Config.in b/package/sdl2/Config.in
index 7c8258fe73..85d7c09385 100644
--- a/package/sdl2/Config.in
+++ b/package/sdl2/Config.in
@@ -29,6 +29,13 @@ comment "X11 video driver needs X.org"
 	depends on !BR2_PACKAGE_XORG7
 	depends on BR2_USE_MMU
 
+config BR2_PACKAGE_SDL2_WAYLAND
+	bool "Wayland video driver"
+	depends on BR2_PACKAGE_WAYLAND
+	depends on BR2_PACKAGE_HAS_LIBEGL
+	depends on BR2_PACKAGE_SDL2_OPENGLES
+	depends on BR2_PACKAGE_LIBXKBCOMMON
+
 config BR2_PACKAGE_SDL2_KMSDRM
 	bool "KMS/DRM video driver"
 	depends on BR2_TOOLCHAIN_HAS_THREADS  # libdrm
diff --git a/package/sdl2/sdl2.mk b/package/sdl2/sdl2.mk
index 5f2e1e0ed1..223497942f 100644
--- a/package/sdl2/sdl2.mk
+++ b/package/sdl2/sdl2.mk
@@ -23,7 +23,6 @@ SDL2_CONF_OPTS += \
 	--disable-video-vivante \
 	--disable-video-cocoa \
 	--disable-video-metal \
-	--disable-video-wayland \
 	--disable-video-dummy \
 	--disable-video-offscreen \
 	--disable-video-vulkan \
@@ -147,6 +146,12 @@ else
 SDL2_CONF_OPTS += --disable-video-x11 --without-x
 endif
 
+ifeq ($(BR2_PACKAGE_SDL2_WAYLAND),y)
+SDL2_CONF_OPTS += --enable-video-wayland
+else
+SDL2_CONF_OPTS += --disable-video-wayland
+endif
+
 ifeq ($(BR2_PACKAGE_SDL2_OPENGL),y)
 SDL2_CONF_OPTS += --enable-video-opengl
 SDL2_DEPENDENCIES += libgl
-- 
2.40.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/sdl2: support building for Wayland

[Thomas Petazzoni via buildroot]

Hello Alexander,

Thanks for your patch, and sorry for the long delay in providing this
review. See below some comments.

On Mon, 22 May 2023 12:14:37 +0200
Alexander Bau <dev@abau.org> wrote:

Your commit log needs to have a Signed-off-by line with your name and
e-mail. And if needed some extra explanation about the change.

> ---
>  package/sdl2/Config.in | 7 +++++++
>  package/sdl2/sdl2.mk   | 7 ++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/package/sdl2/Config.in b/package/sdl2/Config.in
> index 7c8258fe73..85d7c09385 100644
> --- a/package/sdl2/Config.in
> +++ b/package/sdl2/Config.in
> @@ -29,6 +29,13 @@ comment "X11 video driver needs X.org"
>  	depends on !BR2_PACKAGE_XORG7
>  	depends on BR2_USE_MMU
>  
> +config BR2_PACKAGE_SDL2_WAYLAND
> +	bool "Wayland video driver"
> +	depends on BR2_PACKAGE_WAYLAND
> +	depends on BR2_PACKAGE_HAS_LIBEGL
> +	depends on BR2_PACKAGE_SDL2_OPENGLES
> +	depends on BR2_PACKAGE_LIBXKBCOMMON

I don't think we want to use "depends on" for all those dependencies. I
think we probably want something like this:

	depends on BR2_PACKAGE_WAYLAND
	depends on BR2_PACKAGE_HAS_LIBEGL
	depends on BR2_PACKAGE_HAS_LIBGLES
	select BR2_PACKAGE_SDL2_OPENGLES
	select BR2_PACKAGE_LIBXKBCOMMON

with also:

comment "wayland video driver needs Wayland, OpenGLES+EGL"
	depends on !BR2_PACKAGE_WAYLAND || !BR2_PACKAGE_HAS_LIBEGL || !BR2_PACKAGE_HAS_LIBGLES

> +ifeq ($(BR2_PACKAGE_SDL2_WAYLAND),y)
> +SDL2_CONF_OPTS += --enable-video-wayland

No build dependencies? This looks odd. I'm pretty you need to depend on
host-pkgconf (for pkg-config), on wayland, and libxkbcommon.

Could you have a look at those comments and if possible, send an
updated version that takes them into account?

See
https://buildroot.org/downloads/manual/manual.html#submitting-patches
for some guidelines on submitting patches if needed.

Thanks a lot for your contribution!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot