diff --git a/CHANGELOG.md b/CHANGELOG.md index bfde62b5..34011e65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +31,11 @@ Versioning](https://semver.org/spec/v2.0.0.html). expected. - And probably many more improvements. - I have been testing this extensively to make sure that the change does not - introduce any regressions, but please let me know if this does break anything - for you. + Aside from these more noticeable changes, this has also made it possible to + remove a lot of checks and behaviour that existed solely to work around the + limitations introduced by the event handling system. I have been testing this + extensively to make sure that the change does not introduce any regressions, + but please let me know if this does break anything for you. TODO: Remove known issue about opening Kontakt and certain other plugins causing playback to stall, since this is no longer the case diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index bb4eb6d5..d81788ca 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -161,22 +161,6 @@ void GroupBridge::handle_incoming_connections() { main_context.run(); } -bool GroupBridge::should_skip_message_loop() { - // We do not need additional locking since the call to `AEffect::dispatcher` - // and the actual event handling and message loop handling are performed - // within the IO context and these values thus can't change while another - // the message loop is being running - std::lock_guard lock(active_plugins_mutex); - for (auto& [parameters, value] : active_plugins) { - auto& [thread, bridge] = value; - if (bridge->should_skip_message_loop()) { - return true; - } - } - - return false; -} - void GroupBridge::accept_requests() { group_socket_acceptor.async_accept( [&](const boost::system::error_code& error, @@ -245,25 +229,21 @@ void GroupBridge::async_handle_events() { } } - // Handle Win32 messages unless plugins are in the middle of opening - // their editor - if (!should_skip_message_loop()) { - std::lock_guard lock(active_plugins_mutex); + std::lock_guard lock(active_plugins_mutex); - MSG msg; + MSG msg; - // Keep the loop responsive by not handling too many events at once - // - // For some reason the Melda plugins run into a seemingly infinite - // timer loop for a little while after opening a second editor. - // Without this limit everything will get blocked indefinitely. How - // could this be fixed? - for (int i = 0; i < max_win32_messages && - PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE); - i++) { - TranslateMessage(&msg); - DispatchMessage(&msg); - } + // Keep the loop responsive by not handling too many events at once + // + // For some reason the Melda plugins run into a seemingly infinite timer + // loop for a little while after opening a second editor. Without this + // limit everything will get blocked indefinitely. How could this be + // fixed? + for (int i = 0; i < max_win32_messages && + PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE); + i++) { + TranslateMessage(&msg); + DispatchMessage(&msg); } }); } diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 13f4fcf1..0313d249 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -155,17 +155,6 @@ class GroupBridge { */ void handle_incoming_connections(); - /** - * Returns true if the message loop should not be run at this time. This is - * necessary because hosts will always call either `effEditOpen()` and then - * `effEditGetRect()` or the other way around. If the message loop is - * handled in between these two actions, then some plugins will either - * freeze or sometimes outright crash. Because every plugin has to be run - * from the same thread, this is a simple way to synchronize blocking the - * mesage loop between the different plugin instances. - */ - bool should_skip_message_loop(); - private: /** * Listen on the group socket for incoming requests to host a new plugin diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index d0f276c7..12c47872 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -262,10 +262,6 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context, }); } -bool Vst2Bridge::should_skip_message_loop() const { - return std::holds_alternative(editor); -} - void Vst2Bridge::handle_dispatch() { sockets.host_vst_dispatch.receive_events( std::nullopt, [&](Event& event, bool /*on_main_thread*/) { @@ -337,20 +333,6 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, // We have to intercept GUI open calls since we can't use // the X11 window handle passed by the host switch (opcode) { - case effEditGetRect: { - // Some plugins will have a race condition if the message loops gets - // handled between the call to `effEditGetRect()` and - // `effEditOpen()`, although this behavior never appears on Windows - // as hosts will always either call these functions in sequence or - // in reverse. We need to temporarily stop handling messages when - // this happens. - if (!std::holds_alternative(editor)) { - editor = EditorOpening{}; - } - - return plugin->dispatcher(plugin, opcode, index, value, data, - option); - } break; case effEditOpen: { // Create a Win32 window through Wine, embed it into the window // provided by the host, and let the plugin embed itself into @@ -361,8 +343,8 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, // should get a unique window class const std::string window_class = "yabridge plugin " + sockets.base_dir.string(); - Editor& editor_instance = editor.emplace( - config, window_class, x11_handle, plugin); + Editor& editor_instance = + editor.emplace(config, window_class, x11_handle, plugin); return plugin->dispatcher(plugin, opcode, index, value, editor_instance.get_win32_handle(), @@ -373,7 +355,7 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, plugin->dispatcher(plugin, opcode, index, value, data, option); // Cleanup is handled through RAII - editor = std::monostate(); + editor.reset(); return return_value; } break; @@ -385,29 +367,24 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, } void Vst2Bridge::handle_win32_events() { - std::visit(overload{[](Editor& editor) { editor.handle_win32_events(); }, - [](std::monostate&) { - MSG msg; + if (editor) { + editor->handle_win32_events(); + } else { + MSG msg; - for (int i = 0; - i < max_win32_messages && - PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE); - i++) { - TranslateMessage(&msg); - DispatchMessage(&msg); - } - }, - [](EditorOpening&) { - // Don't handle any events in this particular case - // as explained in `Vst2Bridge::editor` - }}, - editor); + for (int i = 0; i < max_win32_messages && + PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE); + i++) { + TranslateMessage(&msg); + DispatchMessage(&msg); + } + } } void Vst2Bridge::handle_x11_events() { - std::visit(overload{[](Editor& editor) { editor.handle_x11_events(); }, - [](auto&) {}}, - editor); + if (editor) { + editor->handle_x11_events(); + } } class HostCallbackDataConverter : DefaultDataConverter { diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index de195f03..b1c700c3 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -35,13 +35,6 @@ #include "../editor.h" #include "../utils.h" -/** - * A marker struct to indicate that the editor is about to be opened. - * - * @see Vst2Bridge::editor - */ -struct EditorOpening {}; - /** * This hosts a Windows VST2 plugin, forwards messages sent by the Linux VST * plugin and provides host callback function for the plugin to talk back. @@ -78,19 +71,6 @@ class Vst2Bridge { std::string plugin_dll_path, std::string endpoint_base_dir); - /** - * Returns true if the message loop should be skipped. This happens when the - * editor is in the process of being opened. In VST hosts on Windows - * `effEditOpen()` and `effEditGetRect()` will always be called in sequence, - * but in our approach there will be an opportunity to handle events in - * between these two calls. Most plugins will handle this just fine, but - * some plugins end up blocking indefinitely while waiting for the other - * function to be called, hence why this function is needed. For - * individually hosted plugins this check is done implicitely in - * `Vst2Bridge::handle_win32_events()`. - */ - bool should_skip_message_loop() const; - /** * Handle events until the plugin exits. The actual events are posted to * `main_context` to ensure that all operations to could potentially @@ -105,17 +85,17 @@ class Vst2Bridge { void handle_dispatch(); /** - * Handle X11 events for the editor window if it is open. This can be run - * safely from any thread. + * Handle X11 events for the editor window if it is open. This can safely be + * run from any thread. */ void handle_x11_events(); /** * Run the message loop for this plugin. This is only used for the - * individual plugin host. When hosting multiple plugins, a simple central - * message loop with a check to `should_skip_message_loop()` should be used - * instead. This is run on a timer in the same IO context as the one that - * handles the events, i.e. `main_context`. + * individual plugin host, so that we can filter out some unnecessary timer + * events. When hosting multiple plugins, a simple central message loop + * should be used instead. This is run on a timer in the same IO context as + * the one that handles the events, i.e. `main_context`. * * Because of the way the Win32 API works we have to process events on the * same thread as the one the window was created on, and that thread is the @@ -224,25 +204,7 @@ class Vst2Bridge { * Wine window, and embedding that Wine window into a window provided by the * host. Should be empty when the editor is not open. * - * This field can have three possible states: - * - * - `std::nullopt` when the editor is closed. - * - An `Editor` object when the editor is open. - * - `EditorOpening` when the editor is not yet open, but the host has - * already called `effEditGetRect()` and is about to call `effEditOpen()`. - * This is needed because there is a race condition in some bugs that - * cause them to crash or enter an infinite Win32 message loop when - * `effEditGetRect()` gets dispatched and we then enter the message loop - * loop before `effEditOpen()` gets called. Most plugins will handle this - * just fine, but a select few plugins make the assumption that the editor - * is already open once `effEditGetRect()` has been called, even if - * `effEditOpen` has not yet been dispatched. VST hsots on Windows will - * call these two events in sequence, so the bug would never occur there. - * To work around this we'll use this third state to temporarily stop - * processing Windows events in the one or two ticks between these two - * events. - * * @see should_postpone_message_loop */ - std::variant editor; + std::optional editor; };