From fa045fb770be8ac865c62666e40fde411202248b Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 30 Mar 2020 00:47:46 +0200 Subject: [PATCH] Delay the XEmbed messages This works, but we now have the same issues with flickering and resizing found in some other implementations such as Airwave. --- README.md | 9 ++--- src/wine-host/editor.cpp | 62 +++++++++++++++++++++++---------- src/wine-host/editor.h | 16 +++++---- src/wine-host/plugin-bridge.cpp | 18 +++------- 4 files changed, 63 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 204c0d30..d89132aa 100644 --- a/README.md +++ b/README.md @@ -7,10 +7,11 @@ 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. Wine's XEmbed implementation is causing X11 draw calls to fail so - there's probably something right. Right now GUIs do work if you disable the - XEmbed messages or skip reparenting altogether, but that's of course not - ideal. + - 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 diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 8c3b1679..4a0a661c 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -36,7 +36,7 @@ Editor::Editor(std::string window_class_name) ->atom; } -HWND Editor::open(AEffect* effect) { +HWND Editor::open(AEffect* effect, xcb_window_t parent_window_handle) { // 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 @@ -52,6 +52,22 @@ HWND Editor::open(AEffect* effect) { // 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 + // update in the background. Because of this we send `effEditIdle` to the + // plugin on a timer. The refresh rate is purposely fairly low since we + // we'll also trigger this manually in `Editor::handle_events()` whenever + // 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; + xcb_change_window_attributes(x11_connection.get(), parent_window, + XCB_CW_EVENT_MASK, &event_mask); + xcb_flush(x11_connection.get()); return win32_handle->get(); } @@ -64,7 +80,7 @@ void Editor::close() { // everything for us? } -// TODO: I feel like this should only have to be done once +// TODO: I feel like this shouldn't necessary with xembed bool Editor::resize(const VstRect& new_size) { if (!win32_handle.has_value()) { return false; @@ -77,7 +93,7 @@ bool Editor::resize(const VstRect& new_size) { return true; } -bool Editor::embed_into(const size_t parent_window_handle) { +bool Editor::xembed() { if (!win32_handle.has_value()) { return false; } @@ -87,31 +103,24 @@ bool Editor::embed_into(const size_t parent_window_handle) { // under 'Embedding life cycle // Sadly there's doesn't seem to be any implementation of this available as // a library - const size_t child_window_handle = get_x11_handle().value(); + const size_t child_window = get_x11_handle().value(); - xcb_reparent_window(x11_connection.get(), child_window_handle, - parent_window_handle, 0, 0); + xcb_reparent_window(x11_connection.get(), child_window, parent_window, 0, + 0); // Tell the window from Wine it's embedded into the window provided by the // host - send_xembed_event(child_window_handle, xembed_embedded_notify_msg, 0, - parent_window_handle, xembed_protocol_version); + send_xembed_event(child_window, xembed_embedded_notify_msg, 0, + parent_window, xembed_protocol_version); - send_xembed_event(child_window_handle, xembed_focus_in_msg, - xembed_focus_first, 0, 0); - send_xembed_event(child_window_handle, xembed_window_activate_msg, 0, 0, 0); + send_xembed_event(child_window, xembed_focus_in_msg, xembed_focus_first, 0, + 0); + send_xembed_event(child_window, xembed_window_activate_msg, 0, 0, 0); - xcb_map_window(x11_connection.get(), child_window_handle); + xcb_map_window(x11_connection.get(), child_window); xcb_flush(x11_connection.get()); ShowWindow(win32_handle->get(), SW_SHOWNORMAL); - // 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 - // plugin on a timer. The refresh rate is purposely fairly low since we - // we'll also trigger this manually in `Editor::handle_events()` whenever - // the plugin is not busy. - SetTimer(win32_handle->get(), idle_timer_id, 100, nullptr); return true; } @@ -141,6 +150,21 @@ void Editor::handle_events() { if (!gui_was_updated) { SendMessage(win32_handle->get(), WM_TIMER, idle_timer_id, 0); } + + // Handle X11 events + 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); + + // TODO: Only has to be done once, and the problems mentioned in + // the readme are still here. + xembed(); + } + + free(event); + } } } diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 200286a8..f1d4c69b 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -35,8 +35,10 @@ class Editor { * * @param effect The plugin this window is being created for. Used to send * `effEditIdle` messages on a timer. + * @param parent_window_handle The X11 window handle passed by the VST host + * for the editor to embed itself into. */ - HWND open(AEffect* effect); + HWND open(AEffect* effect, xcb_window_t parent_window_handle); void close(); /** @@ -57,19 +59,21 @@ class Editor { void handle_events(); /** - * 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. + * Embed the (open) window into the parent window. * * @return Whether the embedding was succesful. Will return false if the * window is not open. */ - bool embed_into(const size_t parent_window_handle); + bool xembed(); // Needed to handle idle updates through a timer AEffect* plugin; + /** + * 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. diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index 7bb80123..f4b225ed 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/src/wine-host/plugin-bridge.cpp @@ -238,21 +238,13 @@ intptr_t PluginBridge::dispatch_wrapper(AEffect* plugin, option); } break; case effEditOpen: { - const auto win32_handle = editor.open(plugin); - - // The plugin will return 0 if it can not open its - // editor window (or if it does not support it, but in - // that case the DAW should be hiding the option) - const intptr_t return_value = plugin->dispatcher( - plugin, opcode, index, value, win32_handle, option); - if (return_value == 0) { - return 0; - } - const auto x11_handle = reinterpret_cast(data); - editor.embed_into(x11_handle); + const auto win32_handle = editor.open(plugin, x11_handle); - return return_value; + // The actual XEmbed handling is done after the host's window has + // been set to the correct size, in `Editor::handle_events()` + return plugin->dispatcher(plugin, opcode, index, value, + win32_handle, option); } break; case effEditClose: { const intptr_t return_value =