Greatly increase reliability of deferred closing

It's pretty hard to find a solution that checks all of the boxes. I want
something that:

- Closes instantly when you close the editor, and in REAPER you should
  be able to instantly switch between docked and floating modes
- Where there should not be a delay in user interaction when quickly
  reopening the editor (or doing that switching thing in REAPER since that's
  the same thing)
- Where the window manager should not try to reparent the window during
  the losing process as that can cause some jarring flickering
- And, of course, there should be no weird Wine X11Drv crashes

And it should do all of that in Bitwig, REAPER, Carla, Ardour and
Renoise. Apparently it's quite the task to find an approach that checks
all the boxes there.
This commit is contained in:
Robbert van der Helm
2021-01-21 16:47:35 +01:00
parent 5155863673
commit 2ca1d5b8ca
4 changed files with 55 additions and 39 deletions
+2 -2
View File
@@ -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 =
+2 -1
View File
@@ -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(
+38 -32
View File
@@ -18,6 +18,8 @@
#include <iostream>
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<boost::asio::steady_timer> destroy_timer =
std::make_shared<boost::asio::steady_timer>(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<fu2::unique_function<void()>> 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<LPCSTR>(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<LPCSTR>(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<LPCSTR>(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) {
+13 -4
View File
@@ -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<fu2::unique_function<void()>> 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