Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wayland/OpenXR: support X11 and wayland graphics apis #4

Closed

Conversation

technobaboo
Copy link

No description provided.

Riteo and others added 2 commits January 26, 2024 01:17
Not everything is yet implemented, either for Godot or personal
limitations (I don't have all hardware in the world). A brief list of
the most important issues follows:

- Single-window only: the `DisplayServer` API doesn't expose enough
information for properly creating XDG shell windows.

- Very dumb rendering loop: this is very complicated, just know that
the low consumption mode is forced to 2000 Hz and some clever hacks are
in place to overcome a specific Wayland limitation. This will be
improved to the extent possible both downstream and upstream.

- Features to implement yet: IME, touch input, native file dialog,
drawing tablet (commented out due to a refactor), screen recording.

- Mouse passthrough can't be implement through a poly API, we need a
rect-based one.

- The cursor doesn't yet support fractional scaling.

- Auto scale is rounded up when using fractional scaling as we don't
have a per-window scale query API (basically we need
`DisplayServer::window_get_scale`).

- Building with `x11=no wayland=yes opengl=yes openxr=yes` fails.

This also adds a new project property and editor setting for selecting the
default DisplayServer to start, to allow this backend to start first in
exported projects (X11 is still the default for now). The editor setting
always overrides the project setting.

Special thanks to Drew Devault, toger5, Sebastian Krzyszkowiak, Leandro
Benedet Garcia, Subhransu, Yury Zhuravlev and Mara Huldra.
graphics_binding_gl.xDisplay = (Display *)display_handle;
graphics_binding_gl.glxContext = (GLXContext)glxcontext_handle;
graphics_binding_gl.glxDrawable = (GLXDrawable)glxdrawable_handle;
void *display_handle = (void *)display_server->window_get_native_handle(DisplayServer::DISPLAY_HANDLE);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work? I've not plumbed in window_get_native_handle yet.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably piggybacking on the x11 platform code, so probably this indeed will not return the correct value. Its very possible seeing this is OpenGL that the runtime ignores the value we're passing. It's probably far more important to have these values correct for Vulkan and there they are coming from the Vulkan context.

That said, it would be worth implementing window_get_native_handle for the wayland platform, shouldn't be too hard to do.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BastiaanOlij yeah it should be absolutely trivial given the WaylandThread API

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window_get_native_handle() should definitely be implemented for Wayland

Copy link
Owner

@Riteo Riteo Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I quickly whipped up a patch if you want to further test things up with this. Note that this is not tested; I'm on battery (and a pretty low one) so I can't build stuff.

As you can see this is pretty trivial :D

Edit: had a chance to at least build it and found out a build error. Fixed.

diff --git a/platform/linuxbsd/wayland/display_server_wayland.cpp b/platform/linuxbsd/wayland/display_server_wayland.cpp
index e22eddd08e..8d38b2d09d 100644
--- a/platform/linuxbsd/wayland/display_server_wayland.cpp
+++ b/platform/linuxbsd/wayland/display_server_wayland.cpp
@@ -557,6 +557,35 @@ Vector<DisplayServer::WindowID> DisplayServerWayland::get_window_list() const {
 	return ret;
 }
 
+int64_t DisplayServerWayland::window_get_native_handle(HandleType p_handle_type, WindowID p_window) const {
+	MutexLock mutex_lock(wayland_thread.mutex);
+
+	switch (p_handle_type) {
+		case DISPLAY_HANDLE: {
+			return (int64_t)wayland_thread.get_wl_display();
+		} break;
+
+		case WINDOW_HANDLE: {
+			return (int64_t)wayland_thread.window_get_wl_surface(p_window);
+		} break;
+
+		case WINDOW_VIEW: {
+			return 0; // Not supported.
+		} break;
+
+		case OPENGL_CONTEXT: {
+			if (egl_manager) {
+				return (int64_t)egl_manager->get_context(p_window);
+			}
+			return 0;
+		} break;
+
+		default: {
+			return 0;
+		} break;
+	}
+}
+
 DisplayServer::WindowID DisplayServerWayland::get_window_at_screen_position(const Point2i &p_position) const {
 	// Standard Wayland APIs don't support this.
 	return MAIN_WINDOW_ID;
diff --git a/platform/linuxbsd/wayland/display_server_wayland.h b/platform/linuxbsd/wayland/display_server_wayland.h
index f0aabb8c52..56e24fe3f3 100644
--- a/platform/linuxbsd/wayland/display_server_wayland.h
+++ b/platform/linuxbsd/wayland/display_server_wayland.h
@@ -200,6 +200,8 @@ public:
 
 	virtual Vector<DisplayServer::WindowID> get_window_list() const override;
 
+	virtual int64_t window_get_native_handle(HandleType p_handle_type, WindowID p_window = MAIN_WINDOW_ID) const override;
+
 	virtual WindowID get_window_at_screen_position(const Point2i &p_position) const override;
 
 	virtual void window_attach_instance_id(ObjectID p_instance, WindowID p_window_id = MAIN_WINDOW_ID) override;

@BastiaanOlij
Copy link

I have no way of testing but on face value this looks correct. The only thing I'm missing is that we need to define XR_USE_PLATFORM_WAYLAND in openxr_platform_inc.h based on WAYLAND_ENABLED, see the current code there for other platforms.

@technobaboo
Copy link
Author

@BastiaanOlij i don't see XR_USE_PLATFORM anywhere

@BastiaanOlij
Copy link

@BastiaanOlij i don't see XR_USE_PLATFORM anywhere

Just noticed they are already set in /modules/openxr/SCsub, I think that will be enough. These defines are used in the OpenXR platform headers.

@Riteo
Copy link
Owner

Riteo commented Nov 13, 2024

Hi, this has been fixed upstream.

Thank you nonetheless!

@Riteo Riteo closed this Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants