From d2bf5c35a49420e95e348965acc129acb8826ccf Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Tue, 21 Apr 2020 16:50:13 +0200
Subject: [PATCH] Fix hanging when closing the editor

---
 README.md                       |  1 -
 src/wine-host/editor.cpp        | 24 ++++++++++++++++++++++--
 src/wine-host/editor.h          |  4 +++-
 src/wine-host/plugin-bridge.cpp |  3 ---
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/README.md b/README.md
index 9cbf865d..39276c3a 100644
--- a/README.md
+++ b/README.md
@@ -10,7 +10,6 @@ as possible.
 There are a few things that should be done before releasing this, including:
 
 - Fix implementation bugs:
-  - Closing editor windows is slow, this can be improved,
   - Polish GUIs even further. There are some todos left in
     `src/wine-host/editor.{h,cpp}`.
   - There are likely some minor issues left.
diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp
index 43d4f1d8..08194300 100644
--- a/src/wine-host/editor.cpp
+++ b/src/wine-host/editor.cpp
@@ -58,12 +58,12 @@ Editor::Editor(const std::string& window_class_name,
                                   nullptr,
                                   GetModuleHandle(nullptr),
                                   this),
-                   &DestroyWindow),
+                   DestroyWindow),
       parent_window(parent_window_handle),
       child_window(get_x11_handle(win32_handle.get())),
       // Needed to send update messages on a timer
       plugin(effect),
-      x11_connection(xcb_connect(nullptr, nullptr), &xcb_disconnect) {
+      x11_connection(xcb_connect(nullptr, nullptr), xcb_disconnect) {
     // The Win32 API will block the `DispatchMessage` call when opening e.g. a
     // dropdown, but it will still allow timers to be run so the GUI can still
     // update in the background. Because of this we send `effEditIdle` to the
@@ -90,6 +90,26 @@ Editor::Editor(const std::string& window_class_name,
     ShowWindow(win32_handle.get(), SW_SHOWNORMAL);
 }
 
+Editor::~Editor() {
+    // Wine will wait for the parent window to properly delete the window during
+    // `DestroyWindow()`. Instead of implementing this behavior ourselves we
+    // just reparent the window back to the window root and let the WM handle
+    // it.
+    xcb_window_t root =
+        xcb_setup_roots_iterator(xcb_get_setup(x11_connection.get()))
+            .data->root;
+
+    xcb_reparent_window(x11_connection.get(), child_window, root, 0, 0);
+    xcb_flush(x11_connection.get());
+
+    // FIXME: I have no idea why, but for some reason the window still hangs
+    //        some of the times without manually resetting the
+    //        `std::unique_ptr`` to the window handle` (which calls
+    //        `DestroyWindow()`), even though the behavior should be identical
+    //        without this line.
+    win32_handle.reset();
+}
+
 void Editor::send_idle_event() {
     plugin->dispatcher(plugin, effEditIdle, 0, 0, nullptr, 0);
 }
diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h
index b02546c6..f6db65fd 100644
--- a/src/wine-host/editor.h
+++ b/src/wine-host/editor.h
@@ -67,6 +67,8 @@ class Editor {
            AEffect* effect,
            const size_t parent_window_handle);
 
+    ~Editor();
+
     /**
      * Send a single `effEditIdle` event to the plugin to allow it to update its
      * GUI state. This is called periodically from a timer while the GUI is
@@ -92,7 +94,7 @@ class Editor {
      * The handle for the window created through Wine that the plugin uses to
      * embed itself in.
      */
-    const std::unique_ptr<std::remove_pointer_t<HWND>, decltype(&DestroyWindow)>
+    std::unique_ptr<std::remove_pointer_t<HWND>, decltype(&DestroyWindow)>
         win32_handle;
 
    private:
diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp
index d1c25970..69d71309 100644
--- a/src/wine-host/plugin-bridge.cpp
+++ b/src/wine-host/plugin-bridge.cpp
@@ -263,9 +263,6 @@ intptr_t PluginBridge::dispatch_wrapper(AEffect* plugin,
                 plugin->dispatcher(plugin, opcode, index, value, data, option);
 
             // Cleanup is handled through RAII
-            // TODO: We might want to manually unmap the X11 window instead of
-            //       having the host do it. Right now the window editor window
-            //       stays open for a second when removing a plugin.
             editor = std::nullopt;
 
             return return_value;