From c28f5901c7f9ac8885032f9b30db788e08e72911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Tue, 9 Apr 2024 11:47:06 +0200 Subject: [PATCH] Polish interaction between windowing, input and rendering - Adapt GL make/release API to the current architecture. - Fix DisplayServer being locked while dispatching input (prevent deadlocks). --- drivers/egl/egl_manager.cpp | 15 ------------ drivers/egl/egl_manager.h | 1 - .../wayland/display_server_wayland.cpp | 8 ------- .../linuxbsd/wayland/display_server_wayland.h | 1 - platform/linuxbsd/x11/display_server_x11.cpp | 15 +++--------- platform/linuxbsd/x11/display_server_x11.h | 1 - platform/linuxbsd/x11/gl_manager_x11.cpp | 14 ----------- platform/linuxbsd/x11/gl_manager_x11.h | 1 - platform/macos/display_server_macos.h | 1 - platform/macos/display_server_macos.mm | 18 ++++++++++---- platform/macos/gl_manager_macos_legacy.h | 1 - platform/macos/gl_manager_macos_legacy.mm | 13 +--------- platform/windows/display_server_windows.cpp | 17 +++++++++---- platform/windows/display_server_windows.h | 1 - .../windows/gl_manager_windows_native.cpp | 24 +++---------------- platform/windows/gl_manager_windows_native.h | 4 ---- scene/main/window.cpp | 1 + servers/display_server.cpp | 4 ---- servers/display_server.h | 1 - servers/physics_server_2d_wrap_mt.cpp | 1 - servers/physics_server_3d_wrap_mt.cpp | 1 - .../rendering/rendering_server_default.cpp | 3 +-- 22 files changed, 34 insertions(+), 112 deletions(-) diff --git a/drivers/egl/egl_manager.cpp b/drivers/egl/egl_manager.cpp index 10c2119260d..4eddfd027b9 100644 --- a/drivers/egl/egl_manager.cpp +++ b/drivers/egl/egl_manager.cpp @@ -260,21 +260,6 @@ void EGLManager::release_current() { eglMakeCurrent(current_display.egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); } -void EGLManager::make_current() { - if (!current_window) { - return; - } - - if (!current_window->initialized) { - WARN_PRINT("Current OpenGL window is uninitialized!"); - return; - } - - GLDisplay ¤t_display = displays[current_window->gldisplay_id]; - - eglMakeCurrent(current_display.egl_display, current_window->egl_surface, current_window->egl_surface, current_display.egl_context); -} - void EGLManager::swap_buffers() { if (!current_window) { return; diff --git a/drivers/egl/egl_manager.h b/drivers/egl/egl_manager.h index 61d3289b2d6..83779349f0f 100644 --- a/drivers/egl/egl_manager.h +++ b/drivers/egl/egl_manager.h @@ -98,7 +98,6 @@ public: void window_destroy(DisplayServer::WindowID p_window_id); void release_current(); - void make_current(); void swap_buffers(); void window_make_current(DisplayServer::WindowID p_window_id); diff --git a/platform/linuxbsd/wayland/display_server_wayland.cpp b/platform/linuxbsd/wayland/display_server_wayland.cpp index d00d5deb2cb..a815db1c05e 100644 --- a/platform/linuxbsd/wayland/display_server_wayland.cpp +++ b/platform/linuxbsd/wayland/display_server_wayland.cpp @@ -1174,14 +1174,6 @@ void DisplayServerWayland::release_rendering_thread() { #endif } -void DisplayServerWayland::make_rendering_thread() { -#ifdef GLES3_ENABLED - if (egl_manager) { - egl_manager->make_current(); - } -#endif -} - void DisplayServerWayland::swap_buffers() { #ifdef GLES3_ENABLED if (egl_manager) { diff --git a/platform/linuxbsd/wayland/display_server_wayland.h b/platform/linuxbsd/wayland/display_server_wayland.h index b7d7bee0052..368f1b402b1 100644 --- a/platform/linuxbsd/wayland/display_server_wayland.h +++ b/platform/linuxbsd/wayland/display_server_wayland.h @@ -276,7 +276,6 @@ public: virtual void process_events() override; virtual void release_rendering_thread() override; - virtual void make_rendering_thread() override; virtual void swap_buffers() override; virtual void set_context(Context p_context) override; diff --git a/platform/linuxbsd/x11/display_server_x11.cpp b/platform/linuxbsd/x11/display_server_x11.cpp index 9069dd423f5..4c7dfbb107c 100644 --- a/platform/linuxbsd/x11/display_server_x11.cpp +++ b/platform/linuxbsd/x11/display_server_x11.cpp @@ -4268,7 +4268,7 @@ bool DisplayServerX11::_window_focus_check() { } void DisplayServerX11::process_events() { - _THREAD_SAFE_METHOD_ + _THREAD_SAFE_LOCK_ #ifdef DISPLAY_SERVER_X11_DEBUG_LOGS_ENABLED static int frame = 0; @@ -5097,6 +5097,8 @@ void DisplayServerX11::process_events() { */ } + _THREAD_SAFE_UNLOCK_ + Input::get_singleton()->flush_buffered_events(); } @@ -5111,17 +5113,6 @@ void DisplayServerX11::release_rendering_thread() { #endif } -void DisplayServerX11::make_rendering_thread() { -#if defined(GLES3_ENABLED) - if (gl_manager) { - gl_manager->make_current(); - } - if (gl_manager_egl) { - gl_manager_egl->make_current(); - } -#endif -} - void DisplayServerX11::swap_buffers() { #if defined(GLES3_ENABLED) if (gl_manager) { diff --git a/platform/linuxbsd/x11/display_server_x11.h b/platform/linuxbsd/x11/display_server_x11.h index 715a8e48e66..8a7062857c4 100644 --- a/platform/linuxbsd/x11/display_server_x11.h +++ b/platform/linuxbsd/x11/display_server_x11.h @@ -526,7 +526,6 @@ public: virtual void process_events() override; virtual void release_rendering_thread() override; - virtual void make_rendering_thread() override; virtual void swap_buffers() override; virtual void set_context(Context p_context) override; diff --git a/platform/linuxbsd/x11/gl_manager_x11.cpp b/platform/linuxbsd/x11/gl_manager_x11.cpp index 602dd784e04..febb7ae5844 100644 --- a/platform/linuxbsd/x11/gl_manager_x11.cpp +++ b/platform/linuxbsd/x11/gl_manager_x11.cpp @@ -311,20 +311,6 @@ void GLManager_X11::window_make_current(DisplayServer::WindowID p_window_id) { _internal_set_current_window(&win); } -void GLManager_X11::make_current() { - if (!_current_window) { - return; - } - if (!_current_window->in_use) { - WARN_PRINT("current window not in use!"); - return; - } - const GLDisplay &disp = get_current_display(); - if (!glXMakeCurrent(_x_windisp.x11_display, _x_windisp.x11_window, disp.context->glx_context)) { - ERR_PRINT("glXMakeCurrent failed"); - } -} - void GLManager_X11::swap_buffers() { if (!_current_window) { return; diff --git a/platform/linuxbsd/x11/gl_manager_x11.h b/platform/linuxbsd/x11/gl_manager_x11.h index 235c7d22f9f..06e147e39f5 100644 --- a/platform/linuxbsd/x11/gl_manager_x11.h +++ b/platform/linuxbsd/x11/gl_manager_x11.h @@ -117,7 +117,6 @@ public: void window_resize(DisplayServer::WindowID p_window_id, int p_width, int p_height); void release_current(); - void make_current(); void swap_buffers(); void window_make_current(DisplayServer::WindowID p_window_id); diff --git a/platform/macos/display_server_macos.h b/platform/macos/display_server_macos.h index 7c9d073afc7..5d38bf55ea4 100644 --- a/platform/macos/display_server_macos.h +++ b/platform/macos/display_server_macos.h @@ -426,7 +426,6 @@ public: virtual void force_process_and_drop_events() override; virtual void release_rendering_thread() override; - virtual void make_rendering_thread() override; virtual void swap_buffers() override; virtual void set_native_icon(const String &p_filename) override; diff --git a/platform/macos/display_server_macos.mm b/platform/macos/display_server_macos.mm index b7a9fb1bbd7..cfe925e79b3 100644 --- a/platform/macos/display_server_macos.mm +++ b/platform/macos/display_server_macos.mm @@ -358,7 +358,6 @@ void DisplayServerMacOS::_dispatch_input_events(const Ref &p_event) } void DisplayServerMacOS::_dispatch_input_event(const Ref &p_event) { - _THREAD_SAFE_METHOD_ if (!in_dispatch_input_event) { in_dispatch_input_event = true; @@ -2986,7 +2985,7 @@ Key DisplayServerMacOS::keyboard_get_label_from_physical(Key p_keycode) const { } void DisplayServerMacOS::process_events() { - _THREAD_SAFE_METHOD_ + _THREAD_SAFE_LOCK_ while (true) { NSEvent *event = [NSApp @@ -3019,7 +3018,9 @@ void DisplayServerMacOS::process_events() { if (!drop_events) { _process_key_events(); + _THREAD_SAFE_UNLOCK_ Input::get_singleton()->flush_buffered_events(); + _THREAD_SAFE_LOCK_ } for (KeyValue &E : windows) { @@ -3045,6 +3046,8 @@ void DisplayServerMacOS::process_events() { } } } + + _THREAD_SAFE_UNLOCK_ } void DisplayServerMacOS::force_process_and_drop_events() { @@ -3056,9 +3059,14 @@ void DisplayServerMacOS::force_process_and_drop_events() { } void DisplayServerMacOS::release_rendering_thread() { -} - -void DisplayServerMacOS::make_rendering_thread() { +#if defined(GLES3_ENABLED) + if (gl_manager_angle) { + gl_manager_angle->release_current(); + } + if (gl_manager_legacy) { + gl_manager_legacy->release_current(); + } +#endif } void DisplayServerMacOS::swap_buffers() { diff --git a/platform/macos/gl_manager_macos_legacy.h b/platform/macos/gl_manager_macos_legacy.h index bafe825efbf..af9be8f5ba2 100644 --- a/platform/macos/gl_manager_macos_legacy.h +++ b/platform/macos/gl_manager_macos_legacy.h @@ -73,7 +73,6 @@ public: void window_resize(DisplayServer::WindowID p_window_id, int p_width, int p_height); void release_current(); - void make_current(); void swap_buffers(); void window_make_current(DisplayServer::WindowID p_window_id); diff --git a/platform/macos/gl_manager_macos_legacy.mm b/platform/macos/gl_manager_macos_legacy.mm index 701de6df789..6ce3831d9c4 100644 --- a/platform/macos/gl_manager_macos_legacy.mm +++ b/platform/macos/gl_manager_macos_legacy.mm @@ -117,6 +117,7 @@ void GLManagerLegacy_MacOS::release_current() { } [NSOpenGLContext clearCurrentContext]; + current_window = DisplayServer::INVALID_WINDOW_ID; } void GLManagerLegacy_MacOS::window_make_current(DisplayServer::WindowID p_window_id) { @@ -133,18 +134,6 @@ void GLManagerLegacy_MacOS::window_make_current(DisplayServer::WindowID p_window current_window = p_window_id; } -void GLManagerLegacy_MacOS::make_current() { - if (current_window == DisplayServer::INVALID_WINDOW_ID) { - return; - } - if (!windows.has(current_window)) { - return; - } - - GLWindow &win = windows[current_window]; - [win.context makeCurrentContext]; -} - void GLManagerLegacy_MacOS::swap_buffers() { GLWindow &win = windows[current_window]; [win.context flushBuffer]; diff --git a/platform/windows/display_server_windows.cpp b/platform/windows/display_server_windows.cpp index 74665664b1e..b6b713687f9 100644 --- a/platform/windows/display_server_windows.cpp +++ b/platform/windows/display_server_windows.cpp @@ -2962,7 +2962,7 @@ String DisplayServerWindows::keyboard_get_layout_name(int p_index) const { } void DisplayServerWindows::process_events() { - _THREAD_SAFE_METHOD_ + _THREAD_SAFE_LOCK_ MSG msg; @@ -2977,7 +2977,10 @@ void DisplayServerWindows::process_events() { if (!drop_events) { _process_key_events(); + _THREAD_SAFE_UNLOCK_ Input::get_singleton()->flush_buffered_events(); + } else { + _THREAD_SAFE_UNLOCK_ } } @@ -2990,9 +2993,14 @@ void DisplayServerWindows::force_process_and_drop_events() { } void DisplayServerWindows::release_rendering_thread() { -} - -void DisplayServerWindows::make_rendering_thread() { +#if defined(GLES3_ENABLED) + if (gl_manager_angle) { + gl_manager_angle->release_current(); + } + if (gl_manager_native) { + gl_manager_native->release_current(); + } +#endif } void DisplayServerWindows::swap_buffers() { @@ -3433,7 +3441,6 @@ void DisplayServerWindows::_dispatch_input_events(const Ref &p_event } void DisplayServerWindows::_dispatch_input_event(const Ref &p_event) { - _THREAD_SAFE_METHOD_ if (in_dispatch_input_event) { return; } diff --git a/platform/windows/display_server_windows.h b/platform/windows/display_server_windows.h index 4792b658010..2fe1b0733d3 100644 --- a/platform/windows/display_server_windows.h +++ b/platform/windows/display_server_windows.h @@ -679,7 +679,6 @@ public: virtual void force_process_and_drop_events() override; virtual void release_rendering_thread() override; - virtual void make_rendering_thread() override; virtual void swap_buffers() override; virtual void set_native_icon(const String &p_filename) override; diff --git a/platform/windows/gl_manager_windows_native.cpp b/platform/windows/gl_manager_windows_native.cpp index da837b3f949..80cf818199f 100644 --- a/platform/windows/gl_manager_windows_native.cpp +++ b/platform/windows/gl_manager_windows_native.cpp @@ -427,10 +427,6 @@ Error GLManagerNative_Windows::window_create(DisplayServer::WindowID p_window_id return OK; } -void GLManagerNative_Windows::_internal_set_current_window(GLWindow *p_win) { - _current_window = p_win; -} - void GLManagerNative_Windows::window_destroy(DisplayServer::WindowID p_window_id) { GLWindow &win = get_window(p_window_id); if (_current_window == &win) { @@ -447,6 +443,8 @@ void GLManagerNative_Windows::release_current() { if (!gd_wglMakeCurrent(_current_window->hDC, nullptr)) { ERR_PRINT("Could not detach OpenGL context from window marked current: " + format_error_message(GetLastError())); } + + _current_window = nullptr; } void GLManagerNative_Windows::window_make_current(DisplayServer::WindowID p_window_id) { @@ -467,17 +465,7 @@ void GLManagerNative_Windows::window_make_current(DisplayServer::WindowID p_wind ERR_PRINT("Could not switch OpenGL context to other window: " + format_error_message(GetLastError())); } - _internal_set_current_window(&win); -} - -void GLManagerNative_Windows::make_current() { - if (!_current_window) { - return; - } - const GLDisplay &disp = get_current_display(); - if (!gd_wglMakeCurrent(_current_window->hDC, disp.hRC)) { - ERR_PRINT("Could not switch OpenGL context to window marked current: " + format_error_message(GetLastError())); - } + _current_window = &win; } void GLManagerNative_Windows::swap_buffers() { @@ -491,7 +479,6 @@ Error GLManagerNative_Windows::initialize() { void GLManagerNative_Windows::set_use_vsync(DisplayServer::WindowID p_window_id, bool p_use) { GLWindow &win = get_window(p_window_id); - GLWindow *current = _current_window; if (&win != _current_window) { window_make_current(p_window_id); @@ -506,11 +493,6 @@ void GLManagerNative_Windows::set_use_vsync(DisplayServer::WindowID p_window_id, } else { WARN_PRINT("Could not set V-Sync mode. V-Sync is not supported."); } - - if (current != _current_window) { - _current_window = current; - make_current(); - } } bool GLManagerNative_Windows::is_using_vsync(DisplayServer::WindowID p_window_id) const { diff --git a/platform/windows/gl_manager_windows_native.h b/platform/windows/gl_manager_windows_native.h index 829c53b3d22..b4e2a3acdf5 100644 --- a/platform/windows/gl_manager_windows_native.h +++ b/platform/windows/gl_manager_windows_native.h @@ -68,9 +68,6 @@ private: PFNWGLSWAPINTERVALEXTPROC wglSwapIntervalEXT = nullptr; - // funcs - void _internal_set_current_window(GLWindow *p_win); - GLWindow &get_window(unsigned int id) { return _windows[id]; } const GLWindow &get_window(unsigned int id) const { return _windows[id]; } @@ -91,7 +88,6 @@ public: void window_resize(DisplayServer::WindowID p_window_id, int p_width, int p_height) {} void release_current(); - void make_current(); void swap_buffers(); void window_make_current(DisplayServer::WindowID p_window_id); diff --git a/scene/main/window.cpp b/scene/main/window.cpp index 65f1365e679..3c0f2b3d44d 100644 --- a/scene/main/window.cpp +++ b/scene/main/window.cpp @@ -1579,6 +1579,7 @@ bool Window::_can_consume_input_events() const { } void Window::_window_input(const Ref &p_ev) { + ERR_MAIN_THREAD_GUARD; if (EngineDebugger::is_active()) { // Quit from game window using the stop shortcut (F8 by default). // The custom shortcut is provided via environment variable when running from the editor. diff --git a/servers/display_server.cpp b/servers/display_server.cpp index 9ceb6909fec..351c03c158a 100644 --- a/servers/display_server.cpp +++ b/servers/display_server.cpp @@ -697,10 +697,6 @@ void DisplayServer::release_rendering_thread() { WARN_PRINT("Rendering thread not supported by this display server."); } -void DisplayServer::make_rendering_thread() { - WARN_PRINT("Rendering thread not supported by this display server."); -} - void DisplayServer::swap_buffers() { WARN_PRINT("Swap buffers not supported by this display server."); } diff --git a/servers/display_server.h b/servers/display_server.h index f1a98c2c178..aab51644c06 100644 --- a/servers/display_server.h +++ b/servers/display_server.h @@ -559,7 +559,6 @@ public: virtual void force_process_and_drop_events(); virtual void release_rendering_thread(); - virtual void make_rendering_thread(); virtual void swap_buffers(); virtual void set_native_icon(const String &p_filename); diff --git a/servers/physics_server_2d_wrap_mt.cpp b/servers/physics_server_2d_wrap_mt.cpp index a23bb5e7019..f0d31ddb7ab 100644 --- a/servers/physics_server_2d_wrap_mt.cpp +++ b/servers/physics_server_2d_wrap_mt.cpp @@ -96,7 +96,6 @@ void PhysicsServer2DWrapMT::end_sync() { void PhysicsServer2DWrapMT::init() { if (create_thread) { - //OS::get_singleton()->release_rendering_thread(); thread.start(_thread_callback, this); while (!step_thread_up.is_set()) { OS::get_singleton()->delay_usec(1000); diff --git a/servers/physics_server_3d_wrap_mt.cpp b/servers/physics_server_3d_wrap_mt.cpp index feb17cad844..9cdd1445257 100644 --- a/servers/physics_server_3d_wrap_mt.cpp +++ b/servers/physics_server_3d_wrap_mt.cpp @@ -96,7 +96,6 @@ void PhysicsServer3DWrapMT::end_sync() { void PhysicsServer3DWrapMT::init() { if (create_thread) { - //OS::get_singleton()->release_rendering_thread(); thread.start(_thread_callback, this); while (!step_thread_up) { OS::get_singleton()->delay_usec(1000); diff --git a/servers/rendering/rendering_server_default.cpp b/servers/rendering/rendering_server_default.cpp index 5bf0ab0ba66..268b49ae807 100644 --- a/servers/rendering/rendering_server_default.cpp +++ b/servers/rendering/rendering_server_default.cpp @@ -356,8 +356,7 @@ void RenderingServerDefault::_thread_callback(void *_instance) { void RenderingServerDefault::_thread_loop() { server_thread = Thread::get_caller_id(); - DisplayServer::get_singleton()->make_rendering_thread(); - + DisplayServer::get_singleton()->gl_window_make_current(DisplayServer::MAIN_WINDOW_ID); // Move GL to this thread. _init(); draw_thread_up.set();