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

@@ -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
 

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;

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

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(); });
 }

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();
 }

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;
 

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);

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

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<std::chrono::milliseconds>(
-                        config.event_loop_interval())
-                        .count())
-              : Win32Timer()),
-      idle_timer_proc(std::move(timer_proc)),
+          Win32Timer(win32_window.handle,
+                     idle_timer_id,
+                     std::chrono::duration_cast<std::chrono::milliseconds>(
+                         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<POINT> 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,

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<DeferredWin32Window> 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<fu2::unique_function<void()>> idle_timer_proc;
+    fu2::unique_function<void()> idle_timer_proc;
 
     /**
      * The atom corresponding to `WM_STATE`.

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();
 }