From 4ae1f03e4cdc06d24ce8932ec4fb1832a3f208af Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Thu, 19 Mar 2020 21:35:23 +0100
Subject: [PATCH] Simplify GUI event handling

---
 README.md                       |  4 ++--
 src/wine-host/editor.cpp        | 34 +++++++++++++--------------------
 src/wine-host/editor.h          | 14 ++++++++++----
 src/wine-host/plugin-bridge.cpp | 16 +++++++++++++---
 4 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/README.md b/README.md
index 5abceab4..928a86f8 100644
--- a/README.md
+++ b/README.md
@@ -7,8 +7,8 @@ Yet Another way to use Windows VST2 plugins in Linux VST hosts.
 There are a few things that should be done before releasing this, including:
 
 - Implement missing features:
-  - GUIs. Right now I'm just ignoring all of the opcodes related to GUIs so that
-    the plugins don't crash when you open their GUI.
+  - GUIs. It mostly works right now, but there are a few small problems
+    remaining regarding window placement and child windows (i.e. droopdowns).
 - Add missing details if any to the architecture section.
 - Document what this has been tested on and what does or does not work.
 - Document wine32 support.
diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp
index 610b29c4..185d5e62 100644
--- a/src/wine-host/editor.cpp
+++ b/src/wine-host/editor.cpp
@@ -24,7 +24,7 @@ HWND Editor::open() {
 }
 
 void Editor::close() {
-    // RAII does the rest for us
+    // RAII will destroy the window for us
     win32_handle = std::nullopt;
 
     // TODO: Do we need to do something on the X11 side or does the host do
@@ -44,26 +44,6 @@ bool Editor::resize(const VstRect& new_size) {
     return true;
 }
 
-// TODO: Below function shouldn't be needed
-bool Editor::update() {
-    if (!win32_handle.has_value()) {
-        return false;
-    }
-
-    // TODO: Doing this manually should not be needed
-    UpdateWindow(win32_handle->get());
-
-    // TODO: This should also be done somewhere else
-    // Pump events since the Win32 API won't do it for us
-    MSG msg;
-    while (PeekMessage(&msg, win32_handle->get(), 0, 0, PM_REMOVE)) {
-        TranslateMessage(&msg);
-        DispatchMessage(&msg);
-    }
-
-    return true;
-}
-
 bool Editor::embed_into(const size_t parent_window_handle) {
     if (!win32_handle.has_value()) {
         return false;
@@ -101,6 +81,18 @@ bool Editor::embed_into(const size_t parent_window_handle) {
     return true;
 }
 
+void Editor::handle_events() {
+    // Process any remaining events, otherwise we won't be able to interact with
+    // the window
+    if (win32_handle.has_value()) {
+        MSG msg;
+        while (PeekMessage(&msg, win32_handle->get(), 0, 0, PM_REMOVE)) {
+            TranslateMessage(&msg);
+            DispatchMessage(&msg);
+        }
+    }
+}
+
 std::optional<size_t> Editor::get_x11_handle() {
     if (!win32_handle.has_value()) {
         return std::nullopt;
diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h
index fc1cbaca..b9fb6d48 100644
--- a/src/wine-host/editor.h
+++ b/src/wine-host/editor.h
@@ -46,9 +46,12 @@ class Editor {
      */
     bool resize(const VstRect& new_size);
 
-    // TODO: This should not be needed, and is just a test to see if this works
-    //       at all
-    bool update();
+    /**
+     * Pump messages from the editor GUI's event loop until all events are
+     * process. Must be run from the same thread the GUI was created in because
+     * of Win32 limitations. I guess that's what `effEditIdle` is for.
+     */
+    void handle_events();
 
     /**
      * Embed the (open) window into a parent window.
@@ -61,12 +64,15 @@ class Editor {
      */
     bool embed_into(const size_t parent_window_handle);
 
+   private:
     /**
      * Return the X11 window handle for the window if it's currently open.
      */
     std::optional<size_t> get_x11_handle();
 
-   private:
+    /**
+     * The Win32 window class registered for the window window.
+     */
     ATOM window_class;
 
     /**
diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp
index 405edafa..97fb9f7f 100644
--- a/src/wine-host/plugin-bridge.cpp
+++ b/src/wine-host/plugin-bridge.cpp
@@ -199,13 +199,23 @@ intptr_t PluginBridge::dispatch_wrapper(AEffect* plugin,
     // the X11 window handle passed by the host
     switch (opcode) {
         case effEditIdle:
-            // TODO: Hack, shouldn't be needed. We'll just have to process
-            //       events somewhere else.
-            editor.update();
+            // Because of the way the Win32 APi works we have to process events
+            // on the same thread the window was created, and that thread is the
+            // `dispatch_handler` thread
+            editor.handle_events();
 
             return plugin->dispatcher(plugin, opcode, index, value, data,
                                       option);
             break;
+        case effClose: {
+            // Closing the editor will also shut down the thread that's
+            // currently handling events
+            editor.close();
+
+            return plugin->dispatcher(plugin, opcode, index, value, data,
+                                      option);
+            break;
+        }
         case effEditOpen: {
             const auto win32_handle = editor.open();