From 0c7dbe8a4a18d46a78ce6c274ff8333fd5ec2697 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 26 Apr 2021 18:47:58 +0200 Subject: [PATCH] Reparent to the root window before deferring close We did this before implementing the deferred close in yabridge 3.0.0. It didn't seem necessary anymore so we got rid of it, but without this closing an iZotope Rx plugin's editor in Renoise was guaranteed to trigger an X11 error and crash Renoise. Doing this reparent doesn't seem to cause any slowdown but it does at least fix the specific combination of iZotope Rx and Renoise. --- CHANGELOG.md | 8 ++++++++ src/wine-host/editor.cpp | 22 +++++++++++++++++++--- src/wine-host/editor.h | 9 +++++++-- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5975e71c..62a4c07b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,14 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- In certain rare circumstances, closing a plugin editor would trigger an X11 + error and crash the Wine plugin host, and with that likely the entire DAW. + This happened because Wine would try to destroy a window that had already been + destroyed. I've seen this happen in Renoise and to a lesser degree in REAPER + with plugins that take a while to close their editors, such as the iZotope Rx + plugins. We now explicitly reparent the window to back the root window first + before deferring the window closing. This should work around the issue, while + still keeping editor closing nice and snappy. - _PSPaudioware InifniStrip_ would fail to initialize because the plugin expects the host to always be using Microsoft COM and it won't initialize it by itself. InfiniStrip loads as expected now. diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 558255cc..4b97c31a 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -114,10 +114,25 @@ xcb_window_t get_x11_handle(HWND win32_handle); */ ATOM get_window_class(); -DeferredWindow::DeferredWindow(MainContext& main_context, HWND window) - : handle(window), main_context(main_context) {} +DeferredWindow::DeferredWindow(MainContext& main_context, + std::shared_ptr x11_connection, + HWND window) + : handle(window), + main_context(main_context), + x11_connection(x11_connection) {} DeferredWindow::~DeferredWindow() { + // NOTE: For some rason, Wine will sometimes try to delete a window twice if + // the parent window no longer exists. I've only seen this cause + // issues with plugins that hang when their window is hidden, like the + // iZotope Rx plugins. In Renoise this would otherwise trigger an X11 + // error every time you close such a plugin's editor, and in other + // DAWs I've also seen it happen from time to time. + const xcb_window_t wine_window = get_x11_handle(handle); + const xcb_window_t root_window = + get_root_window(*x11_connection, wine_window); + xcb_reparent_window(x11_connection.get(), wine_window, root_window, 0, 0); + // XXX: We are already deferring this closing by posting `WM_CLOSE` to the // message loop instead of calling `DestroyWindow()` ourselves, but we // can take it one step further. If we post this message directly then @@ -156,6 +171,7 @@ Editor::Editor(MainContext& main_context, // expect) and also causes mouse coordinates to be relative to the window // itself. win32_window(main_context, + x11_connection, CreateWindowEx(WS_EX_TOOLWINDOW, reinterpret_cast(get_window_class()), "yabridge plugin", @@ -288,7 +304,7 @@ Editor::Editor(MainContext& main_context, // As explained above, we can't do this directly in the initializer // list win32_child_window.emplace( - main_context, + main_context, x11_connection, CreateWindowEx(WS_EX_TOOLWINDOW, reinterpret_cast(get_window_class()), "yabridge plugin child", WS_CHILD, CW_USEDEFAULT, diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 38c57fac..6b9015bd 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -78,9 +78,13 @@ class DeferredWindow { * * @param main_context This application's main IO context running on the GUI * thread. + * @param x11_connection The X11 connection handle we're using for this + * editor. * @param window A `HWND` obtained through a call to `CreateWindowEx` */ - DeferredWindow(MainContext& main_context, HWND window); + DeferredWindow(MainContext& main_context, + std::shared_ptr x11_connection, + HWND window); /** * Post a `WM_CLOSE` message to the `handle`'s message queue as described @@ -92,6 +96,7 @@ class DeferredWindow { private: MainContext& main_context; + std::shared_ptr x11_connection; }; /** @@ -222,7 +227,7 @@ class Editor { */ void do_xembed() const; - std::unique_ptr x11_connection; + std::shared_ptr x11_connection; /** * The Wine window's client area, or the maximum size of that window. This