Add a less hacky workaround for the XEmbed issues

README.md

@@ -7,15 +7,10 @@ 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. The current basic implementation with XEmbed suffers from flickering
-    during redrwas (in Serum, depends on the plugin) and it has the usual
-    problems with resizing. Reparenting without XEmbed works great, but it
-    breaks a lot of GUI elements because the plugin still thinks it's in the top
-    left corner of the screen. If that could be fixed that would be ideal.
-- Fix implementation bugs:
-  - Closing Serum's editor takes a full second to execute `DestroyWindow`. After
-    fixing XEmbed it might be possible to at least make it feel responsive by
-    just hiding the window first.
+  - Small quality of life related GUI fixes. Wine's XEmbed implementation
+    doesn't always update the reparented window's client area when the window
+    gets resized. The current workaround works much better than not doing
+    anything at all, but it isn't fully reliably yet.
 - 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.

src/wine-host/editor.cpp

@@ -62,9 +62,7 @@ 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);
 
-    // We'll only start the xembed procedure after the host has givne the window
-    // the correct size, otherwise Wine can't draw correctly
-    const uint32_t event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY;
+    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());
@@ -80,19 +78,6 @@ void Editor::close() {
     //       everything for us?
 }
 
-// TODO: I feel like this shouldn't necessary with xembed
-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;
-}
-
 bool Editor::xembed() {
     if (!win32_handle.has_value()) {
         return false;
@@ -152,15 +137,33 @@ void Editor::handle_events() {
         }
 
         // Handle X11 events
+        // TODO: We don't listen for XEmbed property changes for mapping and
+        //       unmapping the window as described in the spec. Is this
+        //       something we should do?
+        // TODO: Also, increasing window size does not work reliably, any way we
+        //       could make this work better?
         xcb_generic_event_t* event;
         while ((event = xcb_poll_for_event(x11_connection.get())) != nullptr) {
-            if ((event->response_type & ~0x80) == XCB_CONFIGURE_NOTIFY) {
-                xcb_configure_notify_event_t configuration =
-                    *reinterpret_cast<xcb_configure_notify_event_t*>(event);
+            // The most significant bit in an event's response type is used to
+            // indicate whether the event source
+            switch (event->response_type & ((1 << 7) - 1)) {
+                case XCB_VISIBILITY_NOTIFY: {
+                    xcb_visibility_notify_event_t visibility_event =
+                        *reinterpret_cast<xcb_visibility_notify_event_t*>(
+                            event);
 
-                // TODO: Only has to be done once, and the problems mentioned in
-                //       the readme are still here.
-                xembed();
+                    // Wine's XEmbed implementation is a bit iffy when it comes
+                    // to window size changes, including editor windows being
+                    // increased in size by the DAW when first opening the
+                    // editor. Restarting the XEmbed protocol on visibility and
+                    // size changes is an easy workaround, but it's not 100%
+                    // reliable.
+                    if (visibility_event.window == parent_window &&
+                        visibility_event.state !=
+                            XCB_VISIBILITY_FULLY_OBSCURED) {
+                        xembed();
+                    }
+                } break;
             }
 
             free(event);

src/wine-host/editor.h

@@ -41,16 +41,6 @@ class Editor {
     HWND open(AEffect* effect, xcb_window_t parent_window_handle);
     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);
-
     /**
      * 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

src/wine-host/plugin-bridge.cpp

@@ -101,10 +101,10 @@ PluginBridge::PluginBridge(std::string plugin_dll_path,
     host_vst_process_replacing.connect(socket_endpoint);
     vst_host_aeffect.connect(socket_endpoint);
 
-    // Initialize after communication has been set up We'll try to do the same
-    // `get_bridge_isntance` trick as in `plugin/plugin.cpp`, but since the
-    // plugin will probably call the host callback while it's initializing we
-    // sadly have to use a global here.
+    // Initialize after communication has been set up
+    // We'll try to do the same `get_bridge_isntance` trick as in
+    // `plugin/plugin.cpp`, but since the plugin will probably call the host
+    // callback while it's initializing we sadly have to use a global here.
     current_bridge_isntance = this;
     plugin = vst_entry_point(host_callback_proxy);
     if (plugin == nullptr) {
@@ -112,14 +112,14 @@ PluginBridge::PluginBridge(std::string plugin_dll_path,
                                  "' failed to initialize.");
     }
 
-    // Send the plugin's information to the Linux VST plugin. Any updates during
-    // runtime are handled using the `audioMasterIOChanged` host callback.
-    write_object(vst_host_aeffect, *plugin);
-
     // We only needed this little hack during initialization
     current_bridge_isntance = nullptr;
     plugin->ptr1 = this;
 
+    // Send the plugin's information to the Linux VST plugin. Any updates during
+    // runtime are handled using the `audioMasterIOChanged` host callback.
+    write_object(vst_host_aeffect, *plugin);
+
     // This works functionally identically to the `handle_dispatch()` function
     // below, but this socket will only handle midi events. This is needed
     // because of Win32 API limitations.
@@ -254,17 +254,6 @@ intptr_t PluginBridge::dispatch_wrapper(AEffect* plugin,
 
             return return_value;
         } break;
-        case effEditGetRect: {
-            const intptr_t return_value =
-                plugin->dispatcher(plugin, opcode, index, value, data, option);
-
-            // Intercept these calls to make sure that the window (embedded
-            // within the X11 window) is large enough.
-            const auto size = **static_cast<VstRect**>(data);
-            editor.resize(size);
-
-            return return_value;
-        } break;
         default:
             return plugin->dispatcher(plugin, opcode, index, value, data,
                                       option);
@@ -308,15 +297,6 @@ class HostCallbackDataConverter : DefaultDataConverter {
                 // done inside of `passthrough_event`.
                 return AEffect(*plugin);
                 break;
-            case audioMasterSizeWindow:
-                // TODO: Does the plugin not do this automatically? Check Some
-                //       plugins will the host that their size has changed, so
-                //       we'll have to change the window for it.
-                editor.resize(VstRect{0, 0, static_cast<short>(value),
-                                      static_cast<short>(index)});
-
-                return DefaultDataConverter::read(opcode, index, value, data);
-                break;
             default:
                 return DefaultDataConverter::read(opcode, index, value, data);
                 break;