From 17edebcc64d9dc96a9c0b0c7e9b9bd1673a585f9 Mon Sep 17 00:00:00 2001 From: jatinchowdhury18 Date: Wed, 23 Oct 2024 02:51:47 -0700 Subject: [PATCH] Parameter pointer tweaks (#561) * Fixing OptionalPointer move ops, and more tweaks for ForwardingParameterManager * Update tests * Forwarding tricks --- .../Structures/chowdsp_OptionalPointer.h | 20 ++++++++-- .../Structures/chowdsp_PackedPointer.h | 8 ++++ .../chowdsp_ForwardingParametersManager.h | 38 +++++++++---------- .../OptionalPointerTest.cpp | 17 +++++++++ .../PackedPointerTest.cpp | 11 +++++- 5 files changed, 71 insertions(+), 23 deletions(-) diff --git a/modules/common/chowdsp_data_structures/Structures/chowdsp_OptionalPointer.h b/modules/common/chowdsp_data_structures/Structures/chowdsp_OptionalPointer.h index fd423424..2b89cf07 100644 --- a/modules/common/chowdsp_data_structures/Structures/chowdsp_OptionalPointer.h +++ b/modules/common/chowdsp_data_structures/Structures/chowdsp_OptionalPointer.h @@ -64,14 +64,28 @@ struct OptionalPointer } /** Move constructor */ - OptionalPointer (OptionalPointer&&) noexcept = default; + OptionalPointer (OptionalPointer&& other) noexcept + { + invalidate(); + pointer.swap (other.pointer); + } /** Move assignment */ - OptionalPointer& operator= (OptionalPointer&&) noexcept = default; + OptionalPointer& operator= (OptionalPointer&& other) noexcept + { + invalidate(); + pointer.swap (other.pointer); + return *this; + } OptionalPointer (const OptionalPointer&) = delete; OptionalPointer& operator= (const OptionalPointer&) = delete; + ~OptionalPointer() + { + invalidate(); + } + /** Returns true if this pointer owns the data it's pointing to */ [[nodiscard]] bool isOwner() const noexcept { return pointer.get_flags() == Owning; } @@ -89,7 +103,7 @@ struct OptionalPointer } /** - * Resets this object ot nullptr. If this object currently + * Resets this object to nullptr. If this object currently * owns the underlying data, it will free the underlying data. */ void invalidate() diff --git a/modules/common/chowdsp_data_structures/Structures/chowdsp_PackedPointer.h b/modules/common/chowdsp_data_structures/Structures/chowdsp_PackedPointer.h index 7fcd2284..53eb92df 100644 --- a/modules/common/chowdsp_data_structures/Structures/chowdsp_PackedPointer.h +++ b/modules/common/chowdsp_data_structures/Structures/chowdsp_PackedPointer.h @@ -62,6 +62,14 @@ struct PackedPointer [[nodiscard]] T& operator*() { return *get_ptr(); } [[nodiscard]] const T& operator*() const { return *get_ptr(); } + void swap (PackedPointer& other) noexcept + { + std::swap (other.ptr_with_flags, ptr_with_flags); +#if JUCE_DEBUG + std::swap (other.actual_ptr, actual_ptr); +#endif + } + private: T* ptr_with_flags = nullptr; #if JUCE_DEBUG diff --git a/modules/plugin/chowdsp_parameters/Forwarding/chowdsp_ForwardingParametersManager.h b/modules/plugin/chowdsp_parameters/Forwarding/chowdsp_ForwardingParametersManager.h index 00aa9e8a..a6511c21 100644 --- a/modules/plugin/chowdsp_parameters/Forwarding/chowdsp_ForwardingParametersManager.h +++ b/modules/plugin/chowdsp_parameters/Forwarding/chowdsp_ForwardingParametersManager.h @@ -23,21 +23,21 @@ class ForwardingParametersManager #if JUCE_MODULE_AVAILABLE_chowdsp_plugin_state /** Initializes handles to the forwarding parameters, and connects them to the given processor */ explicit ForwardingParametersManager (juce::AudioProcessor& audioProcessor, PluginState& pluginState) - : ForwardingParametersManager { audioProcessor } + : ForwardingParametersManager { &audioProcessor } { - for (int i = 0; i < totalNumForwardingParameters; ++i) + for (size_t i = 0; i < forwardedParams.size(); ++i) { - auto id = Provider::getForwardingParameterID (i); - auto forwardedParam = std::make_unique (id, pluginState, "Blank"); + auto id = Provider::getForwardingParameterID (static_cast (i)); + forwardedParams[i] = OptionalPointer (id, pluginState, "Blank"); + forwardedParams[i]->setProcessor (processor); - forwardedParam->setProcessor (&processor); - forwardedParams[(size_t) i] = forwardedParam.get(); - processor.addParameter (forwardedParam.release()); + if (processor != nullptr) + processor->addParameter (forwardedParams[i].release()); } } /** Initializes the manager without initializing the parameters */ - explicit ForwardingParametersManager (juce::AudioProcessor& audioProcessor) : processor (audioProcessor) + explicit ForwardingParametersManager (juce::AudioProcessor* audioProcessor) : processor (audioProcessor) { } #else @@ -47,16 +47,16 @@ class ForwardingParametersManager } /** Initializes handles to the forwarding parameters, and connects them to the given processor */ - explicit ForwardingParametersManager (juce::AudioProcessor& audioProcessor) : processor (audioProcessor) + explicit ForwardingParametersManager (juce::AudioProcessor& audioProcessor) : processor (&audioProcessor) { for (int i = 0; i < totalNumForwardingParameters; ++i) { auto id = Provider::getForwardingParameterID (i); - auto forwardedParam = std::make_unique (id, nullptr, "Blank"); + forwardedParams[i] = OptionalPointer (id, nullptr, "Blank"); + forwardedParams[i]->setProcessor (processor); - forwardedParam->setProcessor (&processor); - forwardedParams[(size_t) i] = forwardedParam.get(); - processor.addParameter (forwardedParam.release()); + if (processor != nullptr) + processor->addParameter (forwardedParams[i].release()); } } #endif @@ -90,8 +90,8 @@ class ForwardingParametersManager ~ScopedForceDeferHostNotifications() { mgr.forceDeferHostNotifications = previousForceValue; - if (! mgr.forceDeferHostNotifications) - ForwardingParameter::reportParameterInfoChange (&mgr.processor); + if (! mgr.forceDeferHostNotifications && mgr.processor != nullptr) + ForwardingParameter::reportParameterInfoChange (mgr.processor); } private: @@ -121,8 +121,8 @@ class ForwardingParametersManager forwardedParams[(size_t) i]->setParam (param, paramName, deferHostNotification || forceDeferHostNotifications); } - if (deferHostNotification && ! forceDeferHostNotifications) - ForwardingParameter::reportParameterInfoChange (&processor); + if (deferHostNotification && ! forceDeferHostNotifications && processor != nullptr) + ForwardingParameter::reportParameterInfoChange (processor); } /** @@ -143,9 +143,9 @@ class ForwardingParametersManager } protected: - std::array forwardedParams; + std::array, (size_t) totalNumForwardingParameters> forwardedParams; - juce::AudioProcessor& processor; + juce::AudioProcessor* processor = nullptr; private: bool forceDeferHostNotifications = false; diff --git a/tests/common_tests/chowdsp_data_structures_test/OptionalPointerTest.cpp b/tests/common_tests/chowdsp_data_structures_test/OptionalPointerTest.cpp index 72e2741f..9c4a5646 100644 --- a/tests/common_tests/chowdsp_data_structures_test/OptionalPointerTest.cpp +++ b/tests/common_tests/chowdsp_data_structures_test/OptionalPointerTest.cpp @@ -130,4 +130,21 @@ TEST_CASE ("Optional Pointer Test", "[common][data-structures]") REQUIRE (x == y); REQUIRE_FALSE (x != y); } + + SECTION ("Move Constructor") + { + chowdsp::OptionalPointer x { 4, 5 }; + chowdsp::OptionalPointer y { std::move (x) }; + REQUIRE (x == nullptr); // NOLINT + REQUIRE (y->x == 4); + REQUIRE (y->y == 5); + } + + SECTION ("Move Assignment") + { + chowdsp::OptionalPointer y {}; + y = chowdsp::OptionalPointer { 4, 5 }; + REQUIRE (y->x == 4); + REQUIRE (y->y == 5); + } } diff --git a/tests/common_tests/chowdsp_data_structures_test/PackedPointerTest.cpp b/tests/common_tests/chowdsp_data_structures_test/PackedPointerTest.cpp index 1972a2dd..85f5c569 100644 --- a/tests/common_tests/chowdsp_data_structures_test/PackedPointerTest.cpp +++ b/tests/common_tests/chowdsp_data_structures_test/PackedPointerTest.cpp @@ -46,8 +46,17 @@ TEST_CASE ("Packed Pointer Test", "[common][data-structures]") SECTION ("Modify") { - chowdsp::PackedPointer ptr { &t1, 5 }; + chowdsp::PackedPointer ptr { &t1, 5 }; ptr->x = 100; REQUIRE (std::as_const (ptr)->x == t1.x); } + + SECTION ("Swap") + { + chowdsp::PackedPointer ptr { &t1, 5 }; + chowdsp::PackedPointer ptr2 {}; + ptr.swap (ptr2); + REQUIRE (ptr == nullptr); + REQUIRE (std::as_const (ptr2)->x == t1.x); + } }