diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index ee80fb30..b061ae07 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -395,8 +395,8 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, // while it's loading its editor from preempting the audio // thread. set_realtime_priority(false); - Editor& editor_instance = - editor.emplace(config, x11_handle, [plugin = this->plugin]() { + Editor& editor_instance = editor.emplace( + main_context, config, x11_handle, [plugin = this->plugin]() { plugin->dispatcher(plugin, effEditIdle, 0, 0, nullptr, 0.0); }); const intptr_t result = diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp index b01a10e7..f7ebb351 100644 --- a/src/wine-host/bridges/vst3.cpp +++ b/src/wine-host/bridges/vst3.cpp @@ -621,7 +621,8 @@ void Vst3Bridge::run() { set_realtime_priority(false); Editor& editor_instance = object_instances[request.owner_instance_id] - .editor.emplace(config, x11_handle); + .editor.emplace(main_context, config, + x11_handle); const tresult result = object_instances[request.owner_instance_id] .plug_view_instance->plug_view->attached( diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index ed89d7f0..2f6b5de0 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -18,6 +18,8 @@ #include +using namespace std::literals::chrono_literals; + /** * The Win32 timer ID we'll use to periodically call the VST2 `effeditidle` * function with. We have to do this on a timer because the function has to be @@ -112,26 +114,37 @@ xcb_window_t get_x11_handle(HWND win32_handle); */ ATOM get_window_class(); -DeferredWindow::DeferredWindow(HWND window) : handle(window) {} +DeferredWindow::DeferredWindow(MainContext& main_context, HWND window) + : handle(window), main_context(main_context) {} DeferredWindow::~DeferredWindow() { - // XXX: I'm pretty sure this is a Wine bug (or, well, an unfortunate - // interaction of Wine's behaviour and our embedding). If we don't - // explicitly hide the window before sending a `WM_CLOSE` (in - // `destroy_window_async()`), then we might get an X11 error because - // the closing of the window will trigger an X11 event in Wine's X11drv - // which then tries to interact with the no longer existing window. - // Manually hiding the window seems to work around this. - // TODO: Check if we also have to do something special for - // editor_double_embed (probalby not) - // TODO: Retest XEmbed after all of these changes - ShowWindow(handle, SW_MINIMIZE); + // 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 + // we might still get a delay, for instance if our event loop timer + // would tick exactly between `IPlugView::removed()` and + // `IPlugView::~IPlugView`. Delaying this seems to be a best of both + // worlds solution that works as expected in every host I've tested. + std::shared_ptr destroy_timer = + std::make_shared(main_context.context); + destroy_timer->expires_after(1s); - // The actual destroying will happen as part of the Win32 message loop - PostMessage(handle, WM_CLOSE, 0, 0); + // Note that we capture a copy of `destroy_timer` here. This way we don't + // have to manage the timer instance ourselves as it will just clean itself + // up after this lambda gets called. + destroy_timer->async_wait([destroy_timer, handle = this->handle]( + const boost::system::error_code& error) { + if (error.failed()) { + return; + } + + // The actual destroying will happen as part of the Win32 message loop + PostMessage(handle, WM_CLOSE, 0, 0); + }); } -Editor::Editor(const Configuration& config, +Editor::Editor(MainContext& main_context, + const Configuration& config, const size_t parent_window_handle, std::optional> timer_proc) : use_xembed(config.editor_xembed), @@ -142,7 +155,8 @@ Editor::Editor(const Configuration& config, // be drawn without any decorations (making resizes behave as you'd // expect) and also causes mouse coordinates to be relative to the window // itself. - win32_window(CreateWindowEx(WS_EX_TOOLWINDOW, + win32_window(main_context, + CreateWindowEx(WS_EX_TOOLWINDOW, reinterpret_cast(get_window_class()), "yabridge plugin", WS_POPUP, @@ -273,11 +287,14 @@ Editor::Editor(const Configuration& config, if (config.editor_double_embed) { // As explained above, we can't do this directly in the initializer // list - win32_child_window.emplace(CreateWindowEx( - WS_EX_TOOLWINDOW, reinterpret_cast(get_window_class()), - "yabridge plugin child", WS_CHILD, CW_USEDEFAULT, CW_USEDEFAULT, - client_area.width, client_area.height, win32_window.handle, - nullptr, GetModuleHandle(nullptr), this)); + win32_child_window.emplace( + main_context, + CreateWindowEx(WS_EX_TOOLWINDOW, + reinterpret_cast(get_window_class()), + "yabridge plugin child", WS_CHILD, CW_USEDEFAULT, + CW_USEDEFAULT, client_area.width, + client_area.height, win32_window.handle, nullptr, + GetModuleHandle(nullptr), this)); ShowWindow(win32_child_window->handle, SW_SHOWNORMAL); } @@ -294,17 +311,6 @@ Editor::Editor(const Configuration& config, } } -Editor::~Editor() { - // Reparent the window back to the root window, as the actual window will be - // destroyed as part of the next Win32 message loop cycle - xcb_window_t root = - xcb_setup_roots_iterator(xcb_get_setup(x11_connection.get())) - .data->root; - - xcb_reparent_window(x11_connection.get(), wine_window, root, 0, 0); - xcb_flush(x11_connection.get()); -} - HWND Editor::get_win32_handle() const { // FIXME: The double embed and XEmbed options don't work together right now if (win32_child_window && !use_xembed) { diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 7591392a..82d95781 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -59,7 +59,9 @@ struct Size { * A RAII wrapper around windows created using `CreateWindow()` that will post a * `WM_CLOSE` message to the window's message loop so it can clean itself up * later. Directly calling `DestroyWindow()` might hang for a second or two, so - * deferring this increases responsiveness. + * deferring this increases responsiveness. We actually defer this even further + * by calling this function a little while after the editor has closed to + * prevent any potential delays. * * This is essentially an alternative around `std::unique_ptr` with a non-static * custom deleter. @@ -74,9 +76,11 @@ class DeferredWindow { * Manage a window so that it will be asynchronously closed when this object * gets dropped. * + * @param main_context This application's main IO context running on the GUI + * thread. * @param window A `HWND` obtained through a call to `CreateWindowEx` */ - DeferredWindow(HWND window); + DeferredWindow(MainContext& main_context, HWND window); /** * Post a `WM_CLOSE` message to the `handle`'s message queue as described @@ -85,6 +89,9 @@ class DeferredWindow { ~DeferredWindow(); const HWND handle; + + private: + MainContext& main_context; }; /** @@ -113,6 +120,9 @@ class Editor { * Open a window, embed it into the DAW's parent window and create a handle * to the new Win32 window that can be used by the hosted VST plugin. * + * @param main_context The application's main IO context running on the GUI + * thread. We use this to defer closing the window in + * `DestroyWindow::~DestroyWindow()`. * @param config This instance's configuration, used to enable alternative * editor behaviours. * @param parent_window_handle The X11 window handle passed by the VST host @@ -124,12 +134,11 @@ class Editor { * @see win32_window */ Editor( + MainContext& main_context, const Configuration& config, const size_t parent_window_handle, std::optional> timer_proc = std::nullopt); - ~Editor(); - /** * Get the Win32 window handle so it can be passed to an `effEditOpen()` * call. This will return the child window's handle if double editor