From 66dec90846b3bf839279d6dc6fb81c5e8d18027c Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 11 Dec 2020 07:21:33 +0100 Subject: [PATCH 1/2] Weak pointer for VST --- vst/CMakeLists.txt | 3 +- vst/SfizzVstController.cpp | 1 - vst/SfizzVstEditor.h | 10 ++- vst/WeakPtr.h | 122 +++++++++++++++++++++++++++++++++++++ 4 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 vst/WeakPtr.h diff --git a/vst/CMakeLists.txt b/vst/CMakeLists.txt index 9d13eac0..404c897d 100644 --- a/vst/CMakeLists.txt +++ b/vst/CMakeLists.txt @@ -35,7 +35,8 @@ set(VSTPLUGIN_HEADERS SfizzSettings.h X11RunLoop.h NativeHelpers.h - FileTrie.h) + FileTrie.h + WeakPtr.h) if(APPLE) set(VSTPLUGIN_MAC_SOURCES diff --git a/vst/SfizzVstController.cpp b/vst/SfizzVstController.cpp index 2e6854f1..9e5bf897 100644 --- a/vst/SfizzVstController.cpp +++ b/vst/SfizzVstController.cpp @@ -143,7 +143,6 @@ IPlugView* PLUGIN_API SfizzVstController::createView(FIDString _name) withStateLock([this]() { _uiState = _editor->getCurrentUiState(); }); - _editor.reset(); } SfizzVstEditor* editor = new SfizzVstEditor(this); diff --git a/vst/SfizzVstEditor.h b/vst/SfizzVstEditor.h index e5fa0d8d..b23f7a4f 100644 --- a/vst/SfizzVstEditor.h +++ b/vst/SfizzVstEditor.h @@ -7,6 +7,7 @@ #pragma once #include "SfizzVstController.h" #include "editor/EditorController.h" +#include "WeakPtr.h" #include "public.sdk/source/vst/vstguieditor.h" #include class Editor; @@ -18,8 +19,11 @@ using namespace Steinberg; using namespace VSTGUI; class SfizzVstEditor : public Vst::VSTGUIEditor, - public EditorController { + public EditorController, + public Weakable { public: + using Self = SfizzVstEditor; + explicit SfizzVstEditor(SfizzVstController* controller); ~SfizzVstEditor(); @@ -41,6 +45,10 @@ public: SfizzUiState getCurrentUiState() const; void receiveMessage(const void* data, uint32_t size); + void remember() override { SfizzVstEditor::addRef(); } + void forget() override { SfizzVstEditor::release(); } + WEAKABLE_REFCOUNT_METHODS(SfizzVstEditor) + private: void processOscQueue(); diff --git a/vst/WeakPtr.h b/vst/WeakPtr.h new file mode 100644 index 00000000..55481e35 --- /dev/null +++ b/vst/WeakPtr.h @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: BSD-2-Clause + +// This code is part of the sfizz library and is licensed under a BSD 2-clause +// license. You should have receive a LICENSE.md file along with the code. +// If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz + +#pragma once +#include "base/source/fobject.h" +#include +#include + +/** + * A weak reference implementation for Steinberg FObject. + * + * Implementation + * ============== + * + * This takes over the ordinary addRef() and release() methods. + * The variable `refCount` is accessed manually, under a shared mutex. + * There is a unique data block which is shared with all weak pointers, the + * system will null it atomically when the reference count hits zero. + * + * Usage + * ===== + * + * class MyObject : public FObject, public Weakable { + * [...] + * WEAKABLE_REFCOUNT_METHODS(MyObject) + * }; + * + * WeakPtr ptr = myObject.getWeakPtr(); + */ + +template +class Weakable; + +#define WEAKABLE_REFCOUNT_METHODS(T) \ +public: \ + Steinberg::uint32 PLUGIN_API addRef() SMTG_OVERRIDE { return weakAddRef(); } \ + Steinberg::uint32 PLUGIN_API release() SMTG_OVERRIDE { return weakRelease(); } \ +private: \ + friend class Weakable; \ + friend class WeakPtr; + +/// +template +struct WeakPtrSharedData : public std::enable_shared_from_this> { + explicit WeakPtrSharedData(T* self) : self_(self) {} + std::mutex mutex_; + T* self_ = nullptr; +}; + +/// +template +class WeakPtr { + friend class Weakable; + using SharedData = WeakPtrSharedData; + +public: + WeakPtr() = default; + + Steinberg::IPtr lock() + { + std::shared_ptr data = data_.lock(); + if (!data) + return nullptr; + std::lock_guard lock { data->mutex_ }; + T* self = data->self_; + if (self) + ++self->refCount; // manually because we are holding the lock + return Steinberg::IPtr(self, false); + } + +private: + explicit WeakPtr(std::weak_ptr data) : data_(data) {} + std::weak_ptr data_; +}; + +/// +template +class Weakable { + using SharedData = WeakPtrSharedData; + +public: + Weakable() + : weakData_(new SharedData(static_cast(this))) + { + } + + WeakPtr getWeakPtr() + { + return WeakPtr(weakData_); + } + +protected: + Steinberg::uint32 weakAddRef() //override + { + T* self = static_cast(this); + std::lock_guard lock { weakData_->mutex_ }; + return ++self->refCount; + } + + Steinberg::uint32 weakRelease() //override + { + T* self = static_cast(this); + std::shared_ptr data = weakData_; + std::unique_lock lock { data->mutex_ }; + Steinberg::uint32 count = --self->refCount; + if (count == 0) { + data->self_ = nullptr; + weakData_.reset(); + self->refCount = -1000; + lock.unlock(); + delete self; + return 0; + } + return count; + } + +private: + std::shared_ptr weakData_; +}; From 219babcb7a46a4f99e7663b6e8119f0ecb79ebd4 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 11 Dec 2020 07:52:19 +0100 Subject: [PATCH 2/2] Avoid making the controller take a reference on the editor --- vst/SfizzVstController.cpp | 30 +++++++++++++++--------------- vst/SfizzVstController.h | 3 ++- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/vst/SfizzVstController.cpp b/vst/SfizzVstController.cpp index 9e5bf897..9ac894c4 100644 --- a/vst/SfizzVstController.cpp +++ b/vst/SfizzVstController.cpp @@ -139,14 +139,14 @@ IPlugView* PLUGIN_API SfizzVstController::createView(FIDString _name) if (name != Vst::ViewType::kEditor) return nullptr; - if (_editor) { - withStateLock([this]() { - _uiState = _editor->getCurrentUiState(); + if (IPtr editor = _editor.lock()) { + withStateLock([this, editor]() { + _uiState = editor->getCurrentUiState(); }); } - SfizzVstEditor* editor = new SfizzVstEditor(this); - _editor = Steinberg::owned(editor); + IPtr editor = Steinberg::owned(new SfizzVstEditor(this)); + _editor = editor->getWeakPtr(); withStateLock([this, editor]() { editor->updateState(_state); @@ -209,14 +209,14 @@ tresult PLUGIN_API SfizzVstController::setParamNormalized(Vst::ParamID tag, Vst: if (slotF32 && *slotF32 != value) { withStateLock([this, slotF32, value]() { *slotF32 = value; - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->updateState(_state); }); } else if (slotI32 && *slotI32 != (int32)value) { withStateLock([this, slotI32, value]() { *slotI32 = (int32)value; - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->updateState(_state); }); } @@ -234,7 +234,7 @@ tresult PLUGIN_API SfizzVstController::setState(IBStream* stream) withStateLock([this, &s]() { _uiState = s; - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->updateUiState(_uiState); }); @@ -246,8 +246,8 @@ tresult PLUGIN_API SfizzVstController::getState(IBStream* stream) tresult result; withStateLock([this, stream, &result]() { - if (_editor) - _uiState = _editor->getCurrentUiState(); + if (IPtr editor = _editor.lock()) + _uiState = editor->getCurrentUiState(); result = _uiState.store(stream); }); @@ -272,7 +272,7 @@ tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* stream) withStateLock([this, &s]() { _state = s; - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->updateState(_state); }); @@ -300,7 +300,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message) withStateLock([this, data, size]() { _state.sfzFile.assign(static_cast(data), size); - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->updateState(_state); }); } @@ -314,7 +314,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message) withStateLock([this, data, size]() { _state.scalaFile.assign(static_cast(data), size); - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->updateState(_state); }); } @@ -328,7 +328,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message) withStateLock([this, data]() { _playState = *static_cast(data); - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->updatePlayState(_playState); }); } @@ -340,7 +340,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message) if (result != kResultTrue) return result; - if (SfizzVstEditor* editor = _editor) + if (IPtr editor = _editor.lock()) editor->receiveMessage(data, size); } diff --git a/vst/SfizzVstController.h b/vst/SfizzVstController.h index 9e5ba8d4..42094b8f 100644 --- a/vst/SfizzVstController.h +++ b/vst/SfizzVstController.h @@ -9,6 +9,7 @@ #include "public.sdk/source/vst/vsteditcontroller.h" #include "public.sdk/source/vst/vstparameters.h" #include "vstgui/plugin-bindings/vst3editor.h" +#include "WeakPtr.h" #include #include class SfizzVstState; @@ -66,5 +67,5 @@ private: SfizzVstState _state {}; SfizzUiState _uiState {}; // updated on UI open/close/state-request SfizzPlayState _playState {}; - Steinberg::IPtr _editor; + WeakPtr _editor; };