From 949ddaf67325c6c3d854ded70e9c045c49b5f179 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 30 Apr 2021 01:15:18 +0200 Subject: [PATCH] Use mutual recursion for IEditController::setState With this we should be able to handle `setState()`s that try to resize the currently open editor. This could pop up when using the preset browser in REAPER with plugins that recall their old size when loading a preset. --- CHANGELOG.md | 4 +-- .../bridges/vst3-impls/plug-view-proxy.cpp | 7 ++++- .../bridges/vst3-impls/plug-view-proxy.h | 26 +++++++++++++------ .../bridges/vst3-impls/plugin-proxy.cpp | 22 +++++++++++++--- src/plugin/bridges/vst3-impls/plugin-proxy.h | 6 +++++ 5 files changed, 50 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c101b248..1f59e968 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,8 +81,8 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Fixed the VST3 version of _W. A. Production ImPerfect_ from crashing during audio setup. - Fixed _UVI Plugsound Free_ crashing during initialization. -- Fixed a rare potential freeze when loading a VST3 plugin preset while the - plugin is being resized. +- Fixed potential freezing when loading a VST3 preset that tries to resize an + open editor window. - Because of the new transport information prefetching, the excessive DSP usage in _SWAM Cello_ is now been fixed without requiring any manual compatibility options. diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp index ba5bc9b3..a84f1ed8 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp @@ -85,10 +85,15 @@ RunLoopTasks::onFDIsSet(Steinberg::Linux::FileDescriptor /*fd*/) { Vst3PlugViewProxyImpl::Vst3PlugViewProxyImpl( Vst3PluginBridge& bridge, + std::atomic_bool& is_active, Vst3PlugViewProxy::ConstructArgs&& args) - : Vst3PlugViewProxy(std::move(args)), bridge(bridge) {} + : Vst3PlugViewProxy(std::move(args)), bridge(bridge), is_active(is_active) { + is_active = true; +} Vst3PlugViewProxyImpl::~Vst3PlugViewProxyImpl() { + is_active = false; + // Also drop the plug view smart pointer on the Wine side when this gets // dropped send_mutually_recursive_message( diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.h b/src/plugin/bridges/vst3-impls/plug-view-proxy.h index a3215a74..a6386e2e 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.h +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.h @@ -106,6 +106,7 @@ class RunLoopTasks : public Steinberg::Linux::IEventHandler { class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { public: Vst3PlugViewProxyImpl(Vst3PluginBridge& bridge, + std::atomic_bool& is_active, Vst3PlugViewProxy::ConstructArgs&& args); /** @@ -201,14 +202,6 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { // From `IPlugViewContentScaleSupport` tresult PLUGIN_API setContentScaleFactor(ScaleFactor factor) override; - /** - * The `IPlugFrame` object passed by the host passed to us in - * `IPlugView::setFrame()`. When the plugin makes a callback on the - * `IPlugFrame` proxy object, we'll pass the call through to this object. - */ - Steinberg::IPtr plug_frame; - - private: /** * Send a message from this `IPlugView` instance. This function will be * called by the host on its GUI thread, so until this function returns @@ -268,8 +261,25 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { return response_promise.get_future().get(); } + /** + * The `IPlugFrame` object passed by the host passed to us in + * `IPlugView::setFrame()`. When the plugin makes a callback on the + * `IPlugFrame` proxy object, we'll pass the call through to this object. + */ + Steinberg::IPtr plug_frame; + + private: Vst3PluginBridge& bridge; + /** + * We'll use this to signal to the `Vst3PluginProxyImpl` that this object + * has been destroyed. We use this to handle mutual recursion when + * `IEditController::setState()` calls end up calling + * `IPlugFrame::resizeView()`, which should also be handled from the GUI + * thread. + */ + std::atomic_bool& is_active; + /** * The IO contexts used in `send_mutually_recursive_message()` to be able to * execute functions from that same calling thread while we're waiting for a diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp index 19723b52..874a3424 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.cpp @@ -362,8 +362,21 @@ tresult PLUGIN_API Vst3PluginProxyImpl::setState(Steinberg::IBStream* state) { if (state) { // Since both interfaces contain this function, this is used for both // `IComponent::setState()` as well as `IEditController::setState()` - return bridge.send_message(Vst3PluginProxy::SetState{ - .instance_id = instance_id(), .state = state}); + const Vst3PluginProxy::SetState message{.instance_id = instance_id(), + .state = state}; + + // NOTE: This will likely be called from the GUI thread, and some + // plugins will try to resize as part of setting their new state. + // That `IPlugFrame::resizeView()` _also_ has to be handled on the + // GUI thread. So if the GUI is active, we'll use the mutual + // recursion mechanism to allow this resize call to also be + // performed from the GUI thread. + if (last_created_plug_view_active) { + return last_created_plug_view->send_mutually_recursive_message( + std::move(message)); + } else { + return bridge.send_message(std::move(message)); + } } else { bridge.logger.log( "WARNING: Null pointer passed to " @@ -724,8 +737,9 @@ Vst3PluginProxyImpl::createView(Steinberg::FIDString name) { if (response.plug_view_args) { // The host should manage this. Returning raw pointers feels scary. - auto plug_view_proxy = new Vst3PlugViewProxyImpl( - bridge, std::move(*response.plug_view_args)); + auto plug_view_proxy = + new Vst3PlugViewProxyImpl(bridge, last_created_plug_view_active, + std::move(*response.plug_view_args)); // We also need to store an (unmanaged, since we don't want to // affect the reference counting) pointer to this to be able to diff --git a/src/plugin/bridges/vst3-impls/plugin-proxy.h b/src/plugin/bridges/vst3-impls/plugin-proxy.h index a3262d9b..386abd7b 100644 --- a/src/plugin/bridges/vst3-impls/plugin-proxy.h +++ b/src/plugin/bridges/vst3-impls/plugin-proxy.h @@ -354,6 +354,12 @@ class Vst3PluginProxyImpl : public Vst3PluginProxy { */ Vst3PlugViewProxyImpl* last_created_plug_view = nullptr; + /** + * Whether `last_created_plug_view` is currently active. This field is + * written to from `Vst3PlugViewProxyImpl`'s constructor and destructor. + */ + std::atomic_bool last_created_plug_view_active = false; + /** * A pointer to a context menu returned by the host as a response to a call * to `IComponentHandler3::createContextMenu`, as well as all targets we've