Parameter pointer tweaks (#561)

* Fixing OptionalPointer move ops, and more tweaks for ForwardingParameterManager * Update tests * Forwarding tricks
Chowdhury-DSP · Oct 23, 2024 · 17edebc · 17edebc
1 parent b22d183
commit 17edebc
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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<ForwardingParameter> (id, pluginState, "Blank");
+            auto id = Provider::getForwardingParameterID (static_cast<int> (i));
+            forwardedParams[i] = OptionalPointer<ForwardingParameter> (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<ForwardingParameter> (id, nullptr, "Blank");
+            forwardedParams[i] = OptionalPointer<ForwardingParameter> (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<ForwardingParameter*, (size_t) totalNumForwardingParameters> forwardedParams;
+    std::array<OptionalPointer<ForwardingParameter>, (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
@@ -130,4 +130,21 @@ TEST_CASE ("Optional Pointer Test", "[common][data-structures]")
         REQUIRE (x == y);
         REQUIRE_FALSE (x != y);
     }
+
+    SECTION ("Move Constructor")
+    {
+        chowdsp::OptionalPointer<TestType> x { 4, 5 };
+        chowdsp::OptionalPointer<TestType> y { std::move (x) };
+        REQUIRE (x == nullptr); // NOLINT
+        REQUIRE (y->x == 4);
+        REQUIRE (y->y == 5);
+    }
+
+    SECTION ("Move Assignment")
+    {
+        chowdsp::OptionalPointer<TestType> y {};
+        y = chowdsp::OptionalPointer<TestType> { 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
@@ -46,8 +46,17 @@ TEST_CASE ("Packed Pointer Test", "[common][data-structures]")
 
     SECTION ("Modify")
     {
-        chowdsp::PackedPointer<TestType> 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<TestType> ptr2 {};
+        ptr.swap (ptr2);
+        REQUIRE (ptr == nullptr);
+        REQUIRE (std::as_const (ptr2)->x == t1.x);
+    }
 }