From e8fc990f0bf8d871ec49d2f6cc873f6894bd066d Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 9 Apr 2020 18:21:16 +0200 Subject: [PATCH] Add a less hacky workaround for the XEmbed issues --- README.md | 13 +++------ src/wine-host/editor.cpp | 47 ++++++++++++++++++--------------- src/wine-host/editor.h | 10 ------- src/wine-host/plugin-bridge.cpp | 36 ++++++------------------- 4 files changed, 37 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index d89132aa..880df260 100644 --- a/README.md +++ b/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. diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 05f97961..19b27632 100644 --- a/src/wine-host/editor.cpp +++ b/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(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( + 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); diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index f1d4c69b..9799ffe2 100644 --- a/src/wine-host/editor.h +++ b/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 diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index 64f874cc..61cfbe3f 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/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(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(value), - static_cast(index)}); - - return DefaultDataConverter::read(opcode, index, value, data); - break; default: return DefaultDataConverter::read(opcode, index, value, data); break;