Skip to contentSkip to conversation

package/sdl2: support building for Wayland

PATCH In review Alexander Bau 2 participants 2 messages May 22, 2023 – Jul 29, 2023

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 package/sdl2: support building for Wayland

---
 package/sdl2/Config.in | 7 +++++++
 package/sdl2/sdl2.mk   | 7 ++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)
package/sdl2/Config.in +7 -0
index 7c8258fe73..85d7c09385 100644
@@ -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
package/sdl2/sdl2.mk +6 -1
index 5f2e1e0ed1..223497942f 100644
@@ -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
Signature · 6 lines
2.40.0

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#1 lore raw

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 <[email protected]> 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.

Quoted 10 lines
---
 package/sdl2/Config.in | 7 +++++++
 package/sdl2/sdl2.mk   | 7 ++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)
package/sdl2/Config.in +0 -0
index 7c8258fe73..85d7c09385 100644

package/sdl2/Config.in comment "X11 video driver needs X.org" · line 37

Quoted 9 lines
package/sdl2/Config.in +6 -0
@@ -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

package/sdl2/Config.in comment "X11 video driver needs X.org" · line 39

Quoted 2 lines
package/sdl2/Config.in +2 -0
+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

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot