Work around local<->global coordinate issues

For reparented Wine windows. This is a similar approach as LinVST uses.

src/wine-host/editor.cpp

@@ -3,13 +3,24 @@
 // The Win32 API requires you to hardcode identifiers for tiemrs
 constexpr size_t idle_timer_id = 1337;
 
+/**
+ * The most significant bit in an event's response type is used to indicate
+ * whether the event source.
+ */
+constexpr uint16_t event_type_mask = ((1 << 7) - 1);
+
+/**
+ * Return the X11 window handle for the window if it's currently open.
+ */
+xcb_window_t get_x11_handle(HWND win32_handle);
+
 ATOM register_window_class(std::string window_class_name);
 
 Editor::Editor(std::string window_class_name)
     : window_class(register_window_class(window_class_name)),
       x11_connection(xcb_connect(nullptr, nullptr), &xcb_disconnect) {}
 
-HWND Editor::open(AEffect* effect, xcb_window_t parent_window_handle) {
+HWND Editor::open(AEffect* effect) {
     // Create a window without any decoratiosn for easy embedding. The
     // combination of `WS_EX_TOOLWINDOW` and `WS_POPUP` causes the window to be
     // drawn without any decorations (making resizes behave as you'd expect) and
@@ -25,7 +36,6 @@ HWND Editor::open(AEffect* effect, xcb_window_t parent_window_handle) {
 
     // Needed to send update messages on a timer
     plugin = effect;
-    parent_window = parent_window_handle;
 
     // 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
@@ -35,26 +45,21 @@ HWND Editor::open(AEffect* effect, xcb_window_t parent_window_handle) {
     // the plugin is not busy.
     SetTimer(win32_handle->get(), idle_timer_id, 100, nullptr);
 
-    // Embed the Win32 window into the window provided by the host. Instead of
-    // using the XEmbed protocol, we'll register a few events and manage the
-    // child window ourselves. This is a hack to work around the issue's
-    // described in `Editor`'s docstring'.
-    const size_t child_window = get_x11_handle().value();
-    xcb_reparent_window(x11_connection.get(), child_window, parent_window, 0,
-                        0);
-    xcb_map_window(x11_connection.get(), child_window);
-    xcb_flush(x11_connection.get());
-
-    const uint32_t event_mask = XCB_EVENT_MASK_VISIBILITY_CHANGE;
-    xcb_change_window_attributes(x11_connection.get(), parent_window,
-                                 XCB_CW_EVENT_MASK, &event_mask);
-    xcb_flush(x11_connection.get());
-
-    ShowWindow(win32_handle->get(), SW_SHOWNORMAL);
-
     return win32_handle->get();
 }
 
+bool Editor::resize(const VstRect& new_size) {
+    if (!win32_handle.has_value()) {
+        return false;
+    }
+
+    SetWindowPos(win32_handle->get(), HWND_TOP, new_size.left, new_size.top,
+                 new_size.right - new_size.left, new_size.bottom - new_size.top,
+                 0);
+
+    return true;
+}
+
 void Editor::close() {
     // RAII will destroy the window and tiemrs for us
     win32_handle = std::nullopt;
@@ -63,6 +68,37 @@ void Editor::close() {
     //       everything for us?
 }
 
+bool Editor::embed_into(const size_t parent_window_handle) {
+    if (!win32_handle.has_value()) {
+        return false;
+    }
+
+    child_window = get_x11_handle(win32_handle->get());
+    parent_window = parent_window_handle;
+
+    // See the X11 events part of `Editor::handle_events`.
+    // const uint32_t child_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY;
+    // xcb_change_window_attributes(x11_connection.get(), child_window,
+    //                              XCB_CW_EVENT_MASK, &child_event_mask);
+    const uint32_t parent_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY;
+    xcb_change_window_attributes(x11_connection.get(), parent_window,
+                                 XCB_CW_EVENT_MASK, &parent_event_mask);
+    xcb_flush(x11_connection.get());
+
+    // Embed the Win32 window into the window provided by the host. Instead of
+    // using the XEmbed protocol, we'll register a few events and manage the
+    // child window ourselves. This is a hack to work around the issue's
+    // described in `Editor`'s docstring'.
+    xcb_reparent_window(x11_connection.get(), child_window, parent_window, 0,
+                        0);
+    xcb_map_window(x11_connection.get(), child_window);
+    xcb_flush(x11_connection.get());
+
+    ShowWindow(win32_handle->get(), SW_SHOWNORMAL);
+
+    return true;
+}
+
 void Editor::handle_events() {
     // Process any remaining events, otherwise we won't be able to interact with
     // the window
@@ -90,30 +126,49 @@ void Editor::handle_events() {
         }
 
         // Handle X11 events
-        xcb_generic_event_t* event;
+        // TODO: Check if we should forward other events mostly to prevent
-        while ((event = xcb_poll_for_event(x11_connection.get())) != nullptr) {
+        //       unnecessary GUI processing in the background
-            // The most significant bit in an event's response type is used to
+        xcb_generic_event_t* generic_event;
-            // indicate whether the event source
+        while ((generic_event = xcb_poll_for_event(x11_connection.get())) !=
-            switch (event->response_type & ((1 << 7) - 1)) {
+               nullptr) {
-                case XCB_VISIBILITY_NOTIFY: {
+            switch (generic_event->response_type & event_type_mask) {
-                    // TODO: Handle configuration changes
+                case XCB_CONFIGURE_NOTIFY: {
+                    xcb_configure_notify_event_t event =
+                        *reinterpret_cast<xcb_configure_notify_event_t*>(
+                            generic_event);
+                    if (event.window != parent_window) {
+                        break;
+                    }
+
+                    // We're purposely not using XEmbed. This has the
+                    // consequence that wine still thinks that any X and Y
+                    // coordinates are relative to the x11 window root instead
+                    // of the parent window provided by the DAW, causing all
+                    // sorts of GUI interactions to break. To alleviate this
+                    // we'll just lie to Wine and tell it that it's located at
+                    // the parent window's location. We'll only send the event
+                    // instead of actually configuring the window.
+                    xcb_configure_notify_event_t translated_event{};
+                    translated_event.response_type = XCB_CONFIGURE_NOTIFY;
+                    translated_event.event = child_window;
+                    translated_event.window = child_window;
+                    translated_event.width = event.width;
+                    translated_event.height = event.height;
+                    translated_event.x = event.x;
+                    translated_event.y = event.y;
+
+                    xcb_send_event(x11_connection.get(), false, child_window,
+                                   XCB_EVENT_MASK_STRUCTURE_NOTIFY |
+                                       XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY,
+                                   reinterpret_cast<char*>(&translated_event));
+                    xcb_flush(x11_connection.get());
                 } break;
             }
-
+            free(generic_event);
-            free(event);
         }
     }
 }
 
-std::optional<size_t> Editor::get_x11_handle() {
-    if (!win32_handle.has_value()) {
-        return std::nullopt;
-    }
-
-    return reinterpret_cast<size_t>(
-        GetProp(win32_handle.value().get(), "__wine_x11_whole_window"));
-}
-
 LRESULT CALLBACK window_proc(HWND handle,
                              UINT message,
                              WPARAM wParam,
@@ -155,6 +210,11 @@ LRESULT CALLBACK window_proc(HWND handle,
     return DefWindowProc(handle, message, wParam, lParam);
 }
 
+xcb_window_t get_x11_handle(HWND win32_handle) {
+    return reinterpret_cast<size_t>(
+        GetProp(win32_handle, "__wine_x11_whole_window"));
+}
+
 ATOM register_window_class(std::string window_class_name) {
     WNDCLASSEX window_class{};
 

src/wine-host/editor.h

@@ -22,7 +22,7 @@
  * embed the window in a VST host provided X11 window.
  *
  * This was originally implemented using XEmbed. While that sounded like the
- * right thing to do, there were a few minor issues with Wine's XEmbed
+ * right thing to do, there were a few small issues with Wine's XEmbed
  * implementation. The most important of which is that resizing GUIs sometimes
  * works fine, but often fails to expand the embedded window's client area
  * leaving part of the window inaccessible. There are also some a small number
@@ -32,6 +32,9 @@
  * issues, please let me know and I'll switch to using XEmbed again.
  *
  * This workaround was inspired by LinVST.
+ *
+ * TODO: Refactor this so we don't need to stage initialization and just add an
+ *       `std::optional<Editor>` to `PluginBridge` instead
  */
 class Editor {
    public:
@@ -47,11 +50,33 @@ class Editor {
      *
      * @param effect The plugin this window is being created for. Used to send
      *   `effEditIdle` messages on a timer.
+     *
+     * @return The Win32 window handle of the newly created window.
+     */
+    HWND open(AEffect* effect);
+
+    void close();
+
+    /**
+     * Resize the window to match the given size, if open.
+     *
+     * @param new_size The rectangle with the plugin's current position.
+     *
+     * @return Whether the resizing was succesful. Will return false if the
+     *   editor isn't open.
+     */
+    bool resize(const VstRect& new_size);
+
+    /**
+     * Embed the (open) window into a parent window.
+     *
      * @param parent_window_handle The X11 window handle passed by the VST host
      *   for the editor to embed itself into.
+     *
+     * @return Whether the embedding was succesful. Will return false if the
+     *   window is not open.
      */
-    HWND open(AEffect* effect, xcb_window_t parent_window_handle);
+    bool embed_into(const size_t parent_window_handle);
-    void close();
 
     /**
      * Pump messages from the editor GUI's event loop until all events are
@@ -63,16 +88,15 @@ class Editor {
     // Needed to handle idle updates through a timer
     AEffect* plugin;
 
+   private:
     /**
      * The window handle of the editor window created by the DAW.
      */
     xcb_window_t parent_window;
-
-   private:
     /**
-     * Return the X11 window handle for the window if it's currently open.
+     * The X11 window handle of the window belonging to  `win32_handle`.
      */
-    std::optional<size_t> get_x11_handle();
+    xcb_window_t child_window;
 
     /**
      * The Win32 window class registered for the windows window.

src/wine-host/plugin-bridge.cpp

@@ -238,13 +238,20 @@ intptr_t PluginBridge::dispatch_wrapper(AEffect* plugin,
                                       option);
         } break;
         case effEditOpen: {
-            const auto x11_handle = reinterpret_cast<size_t>(data);
+            const auto win32_handle = editor.open(plugin);
-            const auto win32_handle = editor.open(plugin, x11_handle);
 
-            // The created Win32 window has already been reparented to the host
+            const auto return_value = plugin->dispatcher(
-            // provided window
+                plugin, opcode, index, value, win32_handle, option);
-            return plugin->dispatcher(plugin, opcode, index, value,
+            if (return_value == 0) {
-                                      win32_handle, option);
+                return 0;
+            }
+
+            // If opening the editor was succesful, reparent it to the window
+            // provided by the DAW
+            const auto x11_handle = reinterpret_cast<size_t>(data);
+            editor.embed_into(x11_handle);
+
+            return return_value;
         } break;
         case effEditClose: {
             const intptr_t return_value =
@@ -291,6 +298,17 @@ class HostCallbackDataConverter : DefaultDataConverter {
             case audioMasterGetTime:
                 return WantsVstTimeInfo{};
                 break;
+            case audioMasterSizeWindow:
+                // Plugins use this opcode to indicate that their editor should
+                // be resized, so we'll have to update the Wine window
+                // accordingly
+                // TODO: Can we just do this when handling XCB_CONFIGURE_NOTIFY
+                //       instead?
+                editor.resize(VstRect{0, 0, static_cast<short>(value),
+                                      static_cast<short>(index)});
+
+                return DefaultDataConverter::read(opcode, index, value, data);
+                break;
             case audioMasterIOChanged:
                 // This is a helpful event that indicates that the VST plugin's
                 // `AEffect` struct has changed. Writing these results back is