From c98a9519fe062263ae095baae81e61b535ce9a36 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 31 Jul 2021 15:19:44 +0200 Subject: [PATCH] Handle X11 events within the Win32 event loop This unifies event handling and it allows X11 events to still be processed even when the event loop is blocked. --- CHANGELOG.md | 7 +++++++ src/wine-host/bridges/common.cpp | 2 +- src/wine-host/bridges/common.h | 21 +++++++++------------ src/wine-host/bridges/group.cpp | 22 ++++------------------ src/wine-host/bridges/vst2.cpp | 6 ------ src/wine-host/bridges/vst2.h | 2 -- src/wine-host/bridges/vst3.cpp | 10 ---------- src/wine-host/bridges/vst3.h | 2 -- src/wine-host/editor.cpp | 28 ++++++++++++++-------------- src/wine-host/editor.h | 22 +++++++++++++--------- src/wine-host/individual-host.cpp | 5 +---- 11 files changed, 49 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e35eebf..723c2a91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Added more tracing for input focus handling when using the `+editor` `YABRIDGE_DEBUG_LEVEL` flag. +- In addition to the other editor and event handling related changes mentioned + in the fixes section below, yabridge will now handle X11 events from within + the Win32 event loop. What this means is that X11 events are now handled even + when the plugin is blocking the GUI thread, which could potentially increase + responsiveness and help with graphical issues in certain situations (although + at the moment there aren't any known situations where the old approach caused + any issues). ### Fixed diff --git a/src/wine-host/bridges/common.cpp b/src/wine-host/bridges/common.cpp index 5cd06d8c..820f800e 100644 --- a/src/wine-host/bridges/common.cpp +++ b/src/wine-host/bridges/common.cpp @@ -31,7 +31,7 @@ HostBridge::HostBridge(MainContext& main_context, HostBridge::~HostBridge() noexcept {} -void HostBridge::handle_win32_events() noexcept { +void HostBridge::handle_events() noexcept { MSG msg; for (int i = 0; diff --git a/src/wine-host/bridges/common.h b/src/wine-host/bridges/common.h index 74d5a946..08229113 100644 --- a/src/wine-host/bridges/common.h +++ b/src/wine-host/bridges/common.h @@ -67,17 +67,14 @@ class HostBridge { virtual void run() = 0; /** - * Handle X11 events for the editor window if it is open. This can safely be - * run from any thread. - */ - virtual void handle_x11_events() = 0; - - /** - * Run the message loop for this plugin. This is only used for the - * individual plugin host, so that we can filter out some unnecessary timer - * events. When hosting multiple plugins, a simple central message loop - * should be used instead. This is run on a timer in the same IO context as - * the one that handles the events, i.e. `main_context`. + * Run the message loop for this plugin. This should be called from a timer. + * X11 events for the open editors are also handled in this same way, + * because they are run from a Win32 timer. This lets us still process those + * events even when the Win32 event loop blocks the GUI thread. Since this + * function doesn't have any per-plugin behavior, only a single invocation + * of this is needed when hosting multiple plugins. This is run on a timer + * in the same IO context as the one that handles the events, i.e. + * `main_context`. * * Because of the way the Win32 API works we have to process events on the * same thread as the one the window was created on, and that thread is the @@ -88,7 +85,7 @@ class HostBridge { * because of incorrect assumptions made by the plugin. See the dostring for * `Vst2Bridge::editor` for more information. */ - void handle_win32_events() noexcept; + static void handle_events() noexcept; /** * Used as part of the watchdog. This will check whether the remote host diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 9abd0e32..abf3ee8f 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -264,31 +264,17 @@ void GroupBridge::accept_requests() { void GroupBridge::async_handle_events() { main_context.async_handle_events( [&]() { - { - // Always handle X11 events - std::lock_guard lock(active_plugins_mutex); - for (auto& [parameters, value] : active_plugins) { - auto& [thread, bridge] = value; - bridge->handle_x11_events(); - } - } - std::lock_guard lock(active_plugins_mutex); - MSG msg; - - // Keep the loop responsive by not handling too many events at once + // Keep the loop responsive by not handling too many events at once. + // All X11 events are handled from a Win32 timer so they'll still be + // handled even when the GUI is blocked. // // For some reason the Melda plugins run into a seemingly infinite // timer loop for a little while after opening a second editor. // Without this limit everything will get blocked indefinitely. How // could this be fixed? - for (int i = 0; i < max_win32_messages && - PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE); - i++) { - TranslateMessage(&msg); - DispatchMessage(&msg); - } + HostBridge::handle_events(); }, [&]() { return !is_event_loop_inhibited(); }); } diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 72e24fb7..b4fdf23f 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -492,12 +492,6 @@ void Vst2Bridge::run() { }); } -void Vst2Bridge::handle_x11_events() noexcept { - if (editor) { - editor->handle_x11_events(); - } -} - void Vst2Bridge::close_sockets() { sockets.close(); } diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index c2e74d4c..5f92aa3a 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -67,8 +67,6 @@ class Vst2Bridge : public HostBridge { */ void run() override; - void handle_x11_events() noexcept override; - protected: void close_sockets() override; diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp index a2745cb5..2a2f50c2 100644 --- a/src/wine-host/bridges/vst3.cpp +++ b/src/wine-host/bridges/vst3.cpp @@ -1190,16 +1190,6 @@ void Vst3Bridge::run() { }); } -void Vst3Bridge::handle_x11_events() noexcept { - std::lock_guard lock(object_instances_mutex); - - for (auto& [instance_id, object] : object_instances) { - if (object.editor) { - object.editor->handle_x11_events(); - } - } -} - bool Vst3Bridge::maybe_resize_editor(size_t instance_id, const Steinberg::ViewRect& new_size) { Vst3PluginInstance& instance = object_instances.at(instance_id); diff --git a/src/wine-host/bridges/vst3.h b/src/wine-host/bridges/vst3.h index bcae26d9..ae4c37de 100644 --- a/src/wine-host/bridges/vst3.h +++ b/src/wine-host/bridges/vst3.h @@ -290,8 +290,6 @@ class Vst3Bridge : public HostBridge { */ void run() override; - void handle_x11_events() noexcept override; - /** * If the plugin instance has an editor, resize the wrapper window to match * the new size. This is called from `IPlugFrame::resizeView()` to make sure diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index fc640e1d..acb930b5 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -304,15 +304,17 @@ Editor::Editor(MainContext& main_context, // `ShowWindow()` on `win32_window` we'll run into X11 errors. win32_child_window(std::nullopt), idle_timer( - timer_proc - ? Win32Timer( - win32_window.handle, - idle_timer_id, - std::chrono::duration_cast( - config.event_loop_interval()) - .count()) - : Win32Timer()), - idle_timer_proc(std::move(timer_proc)), + Win32Timer(win32_window.handle, + idle_timer_id, + std::chrono::duration_cast( + config.event_loop_interval()) + .count())), + idle_timer_proc([this, timer_proc = std::move(timer_proc)]() mutable { + handle_x11_events(); + if (timer_proc) { + (*timer_proc)(); + } + }), xcb_wm_state_property( get_atom_by_name(*x11_connection, wm_state_property_name)), parent_window(parent_window_handle), @@ -848,10 +850,8 @@ void Editor::set_input_focus(bool grab) const { xcb_flush(x11_connection.get()); } -void Editor::maybe_run_timer_proc() { - if (idle_timer_proc) { - (*idle_timer_proc)(); - } +void Editor::run_timer_proc() { + idle_timer_proc(); } std::optional Editor::get_current_pointer_position() const { @@ -1121,7 +1121,7 @@ LRESULT CALLBACK window_proc(HWND handle, // the plugin will get keep periodically updating its editor either // when the host sends `effEditIdle` themself, or periodically when // the GUI is being blocked by a dropdown or a message box. - editor->maybe_run_timer_proc(); + editor->run_timer_proc(); return 0; } break; // In case the WM does not support the EWMH active window property, diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 658aaa82..4848f60a 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -248,12 +248,13 @@ class Editor { void set_input_focus(bool grab) const; /** - * Run the timer proc function passed to the constructor, if one was passed. + * Run the X11 event loop plus the timer proc function passed to the + * constructor, if one was passed. * * @see idle_timer * @see idle_timer_proc */ - void maybe_run_timer_proc(); + void run_timer_proc(); /** * Whether to use XEmbed instead of yabridge's normal window embedded. Wine @@ -357,10 +358,13 @@ class Editor { std::optional win32_child_window; /** - * A timer we'll use to periodically run `idle_timer_proc`, if set. Thisi is - * only needed for VST2 plugins, as they expected the host to periodically - * send an idle event. We used to just pass through the calls from the host - * before yabridge 3.x, but doing it ourselves here makes things m much more + * A timer we'll use to periodically run the X11 event loop plus + * `idle_timer_proc`, if that is set. We handle X11 events from within the + * Win32 event loop because that allows us to still process those while the + * GUI is blocked. Additionally for VST2 plugins we also need this + * `idle_timer_proc`, as they expected the host to periodically send an idle + * event. We used to just pass through the calls from the host before + * yabridge 3.x, but doing it ourselves here makes things m much more * manageable and we'd still need a timer anyways for when the GUI is * blocked. */ @@ -368,10 +372,10 @@ class Editor { /** * A function to call when the Win32 timer procs. This is used to - * periodically call `effEditIdle()` for VST2 plugins even if the GUI is - * being blocked. + * periodically call `handle_x11_events()`, as well as `effEditIdle()` for + * VST2 plugins even if the GUI is being blocked. */ - std::optional> idle_timer_proc; + fu2::unique_function idle_timer_proc; /** * The atom corresponding to `WM_STATE`. diff --git a/src/wine-host/individual-host.cpp b/src/wine-host/individual-host.cpp index 3aa2f5fc..9c6d312a 100644 --- a/src/wine-host/individual-host.cpp +++ b/src/wine-host/individual-host.cpp @@ -145,10 +145,7 @@ __cdecl // Handle Win32 messages and X11 events on a timer, just like in // `GroupBridge::async_handle_events()`` main_context.async_handle_events( - [&]() { - bridge->handle_x11_events(); - bridge->handle_win32_events(); - }, + [&]() { bridge->handle_events(); }, [&]() { return !bridge->inhibits_event_loop(); }); main_context.run(); }