[Wayland] Versioned interfaces ignored when compiling

[jwinarske]

Tested versions

4.3 stable

System information

Linux - Godot 4.3

Issue description

Wayland interface version values should be used to control when extra fields are available, or not.

The below build errors can be avoided by checking interface version vs available fields. Control code inclusion/exclusion based on interface version.

| platform/linuxbsd/wayland/wayland_thread.h:667:4: error: field designator 'preferred_buffer_scale' does not refer to any field in type 'const struct wl_surface_listener'
|   667 |                 .preferred_buffer_scale = _wl_surface_on_preferred_buffer_scale,
|       |                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| platform/linuxbsd/wayland/wayland_thread.h:668:4: error: field designator 'preferred_buffer_transform' does not refer to any field in type 'const struct wl_surface_listener'
|   668 |                 .preferred_buffer_transform = _wl_surface_on_preferred_buffer_transform,
|       |                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| platform/linuxbsd/wayland/wayland_thread.h:704:4: error: field designator 'axis_relative_direction' does not refer to any field in type 'const struct wl_pointer_listener'
|   704 |                 .axis_relative_direction = _wl_pointer_on_axis_relative_direction,
|       |                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| 3 errors generated.

Steps to reproduce

Build 4.3 stable on Yocto scarthgap.

Minimal reproduction project (MRP)

build issue

[Riteo]

Thanks for your report! We don't use versioned macros as, for simplicity, we vendor the latest protocol headers in the source tree directly. This allows us to keep the code simple and to avoid compilation issues with older build servers which might not have the latest version of wayland-protocols installed.

Could you please share some further details about the way you build godot stable?

[z-erica]

i am hitting the same issues when building 4.3-stable with the option use_sowrap=false. perhaps it's too coarse a solution, but removing the headers in thirdparty/linuxbsd_headers/wayland seems to let the build process continue

[jwinarske]

@Riteo

When the scanner generates C code from the Wayland XML protocol the #defines are added which are there to help manage version differences.

Examples from xdg shell client header, post scanner

/**
 * @ingroup iface_xdg_toplevel
 */
#define XDG_TOPLEVEL_CONFIGURE_SINCE_VERSION 1
/**
 * @ingroup iface_xdg_toplevel
 */
#define XDG_TOPLEVEL_CLOSE_SINCE_VERSION 1
/**
 * @ingroup iface_xdg_toplevel
 */
#define XDG_TOPLEVEL_CONFIGURE_BOUNDS_SINCE_VERSION 4
/**
 * @ingroup iface_xdg_toplevel
 */
#define XDG_TOPLEVEL_WM_CAPABILITIES_SINCE_VERSION 5

If the *_SINCE_VERSION exists for the specific field, then it's available to define in my source code. This of course is independent of what version of interface the registrar returns. It's up to the client implementer to manage this.

If I were to develop a client interface against Weston 13, or tip of tree mutter and I ignore versioning, my client would suffer portability problems, and usage of that client would be limited.

You can see how I manage this in my waypp library: https://github.com/jwinarske/waypp

I check if the protocol exists, setting a #define, and conditionally include based on *_SINCE_VERSION. What dictates compatibility then is the minimum version I bind to in the registry callback.

[jwinarske]

I will be adding a port of libdecor to waypp in the next couple of weeks, as I don't want to use it direct; it's a terrible mess.

[Riteo]

@jwinarske I'm very sorry, I think I'm missing something. Since we vendor in the protocol definitions^1, the scanner should return the same headers independently from the environment. This way, we have to check for version differences in just a few places where the protocol's behavior changes drastically.

Or is the issue related to something else? I'm having some trouble understanding the nature of the issue.

[jwinarske]

@Riteo This is my Yocto build recipe: https://github.com/jwinarske/meta-godot/blob/scarthgap/recipes-graphics/godot/godot_4.3.bb

These are the patches in use to build 4.3-stable with Clang, Weston 13.0.1, and Wayland protocols 1.33: https://github.com/jwinarske/meta-godot/blob/scarthgap/recipes-graphics/godot/files/0001-Enable-build-config-wayland-yes-use_sowrap-no.patch https://github.com/jwinarske/meta-godot/blob/scarthgap/recipes-graphics/godot/files/0001-enable-clang.patch https://github.com/jwinarske/meta-godot/blob/scarthgap/recipes-graphics/godot/files/0001-wayland-remove-fields.patch

The last listed patch is the work around for this issue; quick hack. The proper solution should involve *_SINCE_VERSION to exclude the interfaces.

I do a lot of Wayland work at Toyota

[Riteo]

@jwinarske thank you for sharing the patches! I think I can see why no-sowrap builds are giving issues.

Could you try switching the #include <wayland-egl.h> line to #include <wayland-egl-core.h>?

Since we use our own up-to-date protocol headers, the transitive wayland-client.h include (which has the whole wayland protocol in it) is conflicting somehow with our own header. That's why we use wayland-*-core for everything, as they have just the bare API and nothing else.

It's easy to miss. I didn't notice it either while approving #92324 :sweat_smile:

[jwinarske]

@Riteo I see. Okay I will try that swap and get back to you

[jwinarske]

@Riteo With this change it produces the same error as reported.

$ git diff
diff --git a/recipes-graphics/godot/files/0001-Enable-build-config-wayland-yes-use_sowrap-no.patch b/recipes-graphics/godot/files/0001-Enable-build-config-wayland-yes-use_sowrap-no.patch
index d963a74..44ab580 100644
--- a/recipes-graphics/godot/files/0001-Enable-build-config-wayland-yes-use_sowrap-no.patch
+++ b/recipes-graphics/godot/files/0001-Enable-build-config-wayland-yes-use_sowrap-no.patch
@@ -19,7 +19,7 @@ index ce8a53a856..2638bec065 100644
  #include "wayland/egl_manager_wayland.h"
  #include "wayland/egl_manager_wayland_gles.h"
 +#ifndef SOWRAP_ENABLED
-+#include <wayland-egl.h>
++#include <wayland-egl-core.h>
 +#endif
  #endif

diff --git a/recipes-graphics/godot/godot_4.3.bb b/recipes-graphics/godot/godot_4.3.bb
index 8570f38..7c0b996 100644
--- a/recipes-graphics/godot/godot_4.3.bb
+++ b/recipes-graphics/godot/godot_4.3.bb
@@ -29,7 +29,6 @@ SRC_URI = " \
     git://github.com/godotengine/godot.git;protocol=https;lfs=0;branch=master \
     file://0001-Enable-build-config-wayland-yes-use_sowrap-no.patch \
     file://0001-enable-clang.patch \
-    file://0001-wayland-remove-fields.patch \
 "

 S = "${WORKDIR}/git"

| In file included from platform/linuxbsd/wayland/display_server_wayland.h:36:
| platform/linuxbsd/wayland/wayland_thread.h:667:4: error: field designator 'preferred_buffer_scale' does not refer to any field in type 'const struct wl_surface_listener'
|   667 |                 .preferred_buffer_scale = _wl_surface_on_preferred_buffer_scale,
|       |                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| platform/linuxbsd/wayland/wayland_thread.h:668:4: error: field designator 'preferred_buffer_transform' does not refer to any field in type 'const struct wl_surface_listener'
|   668 |                 .preferred_buffer_transform = _wl_surface_on_preferred_buffer_transform,
|       |                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| platform/linuxbsd/wayland/wayland_thread.h:704:4: error: field designator 'axis_relative_direction' does not refer to any field in type 'const struct wl_pointer_listener'
|   704 |                 .axis_relative_direction = _wl_pointer_on_axis_relative_direction,
|       |                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[Riteo]

@jwinarske did you also switch the include in platform/linuxbsd/wayland/wayland_thread.h?

[jwinarske]

@Riteo That did it. So these are my updated patches:

0001-enable-clang.patch

From 2fee9b39adc0d40bd5cc534efa9d4fa2da06e78a Mon Sep 17 00:00:00 2001
From: Joel Winarske <joel.winarske@gmail.com>
Date: Tue, 21 May 2024 13:42:54 -0700
Subject: [PATCH] enable clang

Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
---
 platform/linuxbsd/detect.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/platform/linuxbsd/detect.py b/platform/linuxbsd/detect.py
index 59cc6e7962..4d8d927fef 100644
--- a/platform/linuxbsd/detect.py
+++ b/platform/linuxbsd/detect.py
@@ -107,9 +107,6 @@ def configure(env: "Environment"):
         env["use_llvm"] = True

     if env["use_llvm"]:
-        if "clang++" not in os.path.basename(env["CXX"]):
-            env["CC"] = "clang"
-            env["CXX"] = "clang++"
         env.extra_suffix = ".llvm" + env.extra_suffix

     if env["linker"] != "default":
-- 
2.45.0

0001-use-wayland-egl-core.patch

From d3b21603ccb460de9cafe0ec7b8c2ef6c8c76836 Mon Sep 17 00:00:00 2001
From: Joel Winarske <joel.winarske@gmail.com>
Date: Thu, 22 Aug 2024 18:46:26 +0000
Subject: [PATCH] use wayland-egl-core

Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
---
 platform/linuxbsd/wayland/display_server_wayland.cpp | 3 +++
 platform/linuxbsd/wayland/wayland_thread.h           | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/platform/linuxbsd/wayland/display_server_wayland.cpp b/platform/linuxbsd/wayland/display_server_wayland.cpp
index 93096fcdcc..b23882e021 100644
--- a/platform/linuxbsd/wayland/display_server_wayland.cpp
+++ b/platform/linuxbsd/wayland/display_server_wayland.cpp
@@ -48,6 +48,9 @@
 #include "drivers/gles3/rasterizer_gles3.h"
 #include "wayland/egl_manager_wayland.h"
 #include "wayland/egl_manager_wayland_gles.h"
+#ifndef SOWRAP_ENABLED
+#include <wayland-egl-core.h>
+#endif
 #endif

 String DisplayServerWayland::_get_app_id_from_context(Context p_context) {
diff --git a/platform/linuxbsd/wayland/wayland_thread.h b/platform/linuxbsd/wayland/wayland_thread.h
index 775ca71346..76a14dbd6a 100644
--- a/platform/linuxbsd/wayland/wayland_thread.h
+++ b/platform/linuxbsd/wayland/wayland_thread.h
@@ -44,7 +44,7 @@
 #include <wayland-client-core.h>
 #include <wayland-cursor.h>
 #ifdef GLES3_ENABLED
-#include <wayland-egl.h>
+#include <wayland-egl-core.h>
 #endif
 #include <xkbcommon/xkbcommon.h>
 #endif // SOWRAP_ENABLED
-- 
2.46.0

Let me know if you want a PR for the second one.

[Riteo]

@jwinarske great!

Let me know if you want a PR for the second one.

Oh yeah please do upstream this.

Small nitpick: why do you need to add the same include to display_server_wayland.cpp? display_server_wayland.h imports wayland_thread.h which has the EGL include already. Is GLES3_ENABLED not defined in your case?

I'm aware that we're relying on a transitive dependency but since this approach has been taken for the whole file already (for the time being), I think that for consistency and simplicity the PR should do the same.

[jwinarske]

@jwinarske great!

Let me know if you want a PR for the second one.

Oh yeah please do upstream this.

Small nitpick: why do you need to add the same include to display_server_wayland.cpp? display_server_wayland.h imports wayland_thread.h which has the EGL include already. Is GLES3_ENABLED not defined in your case?

The build fails without the addition to display_server_wayland.cpp

This is the config I just validated with

scons p=linuxbsd target=editor arch=rv64 \
        use_llvm=yes \
        use_static_cpp=yes \
        optimize=speed \
        lto=thin \
        progress=yes \
        no_editor_splash=yes \
        num_jobs=32 \
         alsa=yes dbus=yes  debug_symbols=no fontconfig=yes libdecor=yes opengl3=yes  openxr=no pulseaudio=yes  use_sowrap=no builtin_brotli=no builtin_freetype=no builtin_graphite=no builtin_harfbuzz=no builtin_icu4c=no builtin_libogg=no builtin_libpng=no builtin_libtheora=no builtin_libvorbis=no builtin_libwebp=no builtin_zlib=no builtin_zstd=no touch=yes udev=yes vulkan=yes use_volk=yes wayland=yes  x11=no \
        CC="riscv64-poky-linux-clang -target riscv64-poky-linux     -mlittle-endian --dyld-prefix=/usr -Qunused-arguments -fstack-protector-strong  -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot" cflags=" -O2 -pipe -g -feliminate-unused-debug-types   -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot=  -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot=  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot-native= " \
        CXX="riscv64-poky-linux-clang++ -target riscv64-poky-linux     -mlittle-endian --dyld-prefix=/usr -Qunused-arguments -fstack-protector-strong  -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot" cxxflags=" -O2 -pipe -g -feliminate-unused-debug-types   -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot=  -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot=  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot-native=  -fvisibility-inlines-hidden -stdlib=libc++" \
        AS="riscv64-poky-linux-as " AR="riscv64-poky-linux-llvm-ar" RANLIB="riscv64-poky-linux-llvm-ranlib" \
        LINK="riscv64-poky-linux-clang++ -target riscv64-poky-linux     -mlittle-endian --dyld-prefix=/usr -Qunused-arguments -fstack-protector-strong  -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed   -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/git=/usr/src/debug/godot/4.3  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot=  -fmacro-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot=  -fdebug-prefix-map=/mnt/raid10/yocto/master/visionfive2/tmp/work/riscv64-poky-linux/godot/4.3/recipe-sysroot-native=  -Wl,-z,relro,-z,now  -stdlib=libc++ -fuse-ld=lld" \
        import_env_vars=PATH,PKG_CONFIG_DIR,PKG_CONFIG_DISABLE_UNINSTALLED,PKG_CONFIG_LIBDIR,PKG_CONFIG_PATH,PKG_CONFIG_SYSROOT_DIR,PKG_CONFIG_SYSTEM_INCLUDE_PATH,PKG_CONFIG_SYSTEM_LIBRARY_PATH

[Riteo]

@jwinarske I see. Could you share the error it gives? I just successfully built 568589c9d8c763bfb3a4348174d53b42d7c59f21 with use_sowrap=no wayland=yes x11=no udev=no openxr=no (last two flags unrelated) and it succeeded (with the right include we just found obv).

I think that the header chain is quite short. Something else related to your setup is probably causing the fail. I might need some more info to find out.

If this investigation takes too much time or if you don't feel like going further, I'd appreciate if, for the time being, you could upstream only the change that #includes the right header in wayland_thread.h.

Since your config is quite different from the usual kind, I'd like to avoid merging setup-specific patches (assuming that's actually the case). I hope you understand.

Edit: oops I forgot a "probably" I'm not assuming anything, be assured :P

[jwinarske]

@Riteo

Could you share the error it gives?

It's the same error as documented at the top of this issue

[Riteo]

@jwinarske I'm sorry, are you sure this happens even with the latest fix we found? The error at the top of the issue refers to a completely different file from display_server_wayland.cpp and from wayland-egl-core.h in the first place.

[jwinarske]

@Riteo I'll take a closer look tomorrow

[jwinarske]

@Riteo I have confirmed that only this patch is required to resolved this issue. I will submit a PR shortly

From 5444711cb8475d362a9047e8ee715f86780c8ab2 Mon Sep 17 00:00:00 2001
From: Joel Winarske <joel.winarske@gmail.com>
Date: Fri, 23 Aug 2024 20:59:23 +0000
Subject: [PATCH 1/2] wayland thread

Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
---
 platform/linuxbsd/wayland/wayland_thread.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/linuxbsd/wayland/wayland_thread.h b/platform/linuxbsd/wayland/wayland_thread.h
index 775ca71346..76a14dbd6a 100644
--- a/platform/linuxbsd/wayland/wayland_thread.h
+++ b/platform/linuxbsd/wayland/wayland_thread.h
@@ -44,7 +44,7 @@
 #include <wayland-client-core.h>
 #include <wayland-cursor.h>
 #ifdef GLES3_ENABLED
-#include <wayland-egl.h>
+#include <wayland-egl-core.h>
 #endif
 #include <xkbcommon/xkbcommon.h>
 #endif // SOWRAP_ENABLED
-- 
2.46.0

[jwinarske]

@Riteo https://github.com/godotengine/godot/pull/96010