Detect incorrect GuiContext method usage

In debug builds.

CHANGELOG.md

@@ -10,6 +10,15 @@ Since there is no stable release yet, the changes are organized per day in
 reverse chronological order. The main purpose of this document in its current
 state is to list breaking changes.
 
+## [2023-03-07]
+
+This document is now also used to keep track of non-breaking changes.
+
+### Added
+
+- Debug builds now include debug assertions that detect incorrect use of the
+  `GuiContext`'s parameter setting methods.
+
 ## [2023-02-28]
 
 ### Breaking changes

src/wrapper/clap/context.rs

@@ -47,6 +47,9 @@ pub(crate) struct WrapperProcessContext<'a, P: ClapPlugin> {
 /// with the host for things like setting parameters.
 pub(crate) struct WrapperGuiContext<P: ClapPlugin> {
     pub(super) wrapper: Arc<Wrapper<P>>,
+    #[cfg(debug_assertions)]
+    pub(super) param_gesture_checker:
+        atomic_refcell::AtomicRefCell<crate::wrapper::util::context_checks::ParamGestureChecker>,
 }
 
 impl<P: ClapPlugin> Drop for WrapperInitContext<'_, P> {
@@ -139,6 +142,17 @@ impl<P: ClapPlugin> GuiContext for WrapperGuiContext<P> {
             }
             None => nih_debug_assert_failure!("Unknown parameter: {:?}", param),
         }
+
+        #[cfg(debug_assertions)]
+        match self.wrapper.param_id_from_ptr(param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .begin_set_parameter(param_id),
+            None => nih_debug_assert_failure!(
+                "raw_begin_set_parameter() called with an unknown ParamPtr"
+            ),
+        }
     }
 
     unsafe fn raw_set_parameter_normalized(&self, param: ParamPtr, normalized: f32) {
@@ -165,6 +179,17 @@ impl<P: ClapPlugin> GuiContext for WrapperGuiContext<P> {
             }
             None => nih_debug_assert_failure!("Unknown parameter: {:?}", param),
         }
+
+        #[cfg(debug_assertions)]
+        match self.wrapper.param_id_from_ptr(param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .set_parameter(param_id),
+            None => {
+                nih_debug_assert_failure!("raw_set_parameter() called with an unknown ParamPtr")
+            }
+        }
     }
 
     unsafe fn raw_end_set_parameter(&self, param: ParamPtr) {
@@ -182,6 +207,17 @@ impl<P: ClapPlugin> GuiContext for WrapperGuiContext<P> {
             }
             None => nih_debug_assert_failure!("Unknown parameter: {:?}", param),
         }
+
+        #[cfg(debug_assertions)]
+        match self.wrapper.param_id_from_ptr(param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .end_set_parameter(param_id),
+            None => {
+                nih_debug_assert_failure!("raw_end_set_parameter() called with an unknown ParamPtr")
+            }
+        }
     }
 
     fn get_state(&self) -> crate::wrapper::state::PluginState {

src/wrapper/clap/wrapper.rs

@@ -719,7 +719,11 @@ impl<P: ClapPlugin> Wrapper<P> {
     }
 
     fn make_gui_context(self: Arc<Self>) -> Arc<WrapperGuiContext<P>> {
-        Arc::new(WrapperGuiContext { wrapper: self })
+        Arc::new(WrapperGuiContext {
+            wrapper: self,
+            #[cfg(debug_assertions)]
+            param_gesture_checker: Default::default(),
+        })
     }
 
     /// # Note
@@ -742,6 +746,16 @@ impl<P: ClapPlugin> Wrapper<P> {
         }
     }
 
+    /// Get a parameter's ID based on a `ParamPtr`. Used in the `GuiContext` implementation for the
+    /// gesture checks.
+    #[allow(unused)]
+    pub fn param_id_from_ptr(&self, param: ParamPtr) -> Option<&str> {
+        self.param_ptr_to_hash
+            .get(&param)
+            .and_then(|hash| self.param_id_by_hash.get(hash))
+            .map(|s| s.as_str())
+    }
+
     /// Queue a parameter output event to be sent to the host at the end of the audio processing
     /// cycle, and request a parameter flush from the host if the plugin is not currently processing
     /// audio. The parameter's actual value will only be updated at that point so the value won't

src/wrapper/standalone/context.rs

@@ -34,6 +34,9 @@ pub(crate) struct WrapperProcessContext<'a, P: Plugin, B: Backend<P>> {
 /// with the host for things like setting parameters.
 pub(crate) struct WrapperGuiContext<P: Plugin, B: Backend<P>> {
     pub(super) wrapper: Arc<Wrapper<P, B>>,
+    #[cfg(debug_assertions)]
+    pub(super) param_gesture_checker:
+        atomic_refcell::AtomicRefCell<crate::wrapper::util::context_checks::ParamGestureChecker>,
 }
 
 impl<P: Plugin, B: Backend<P>> InitContext<P> for WrapperInitContext<'_, P, B> {
@@ -111,13 +114,46 @@ impl<P: Plugin, B: Backend<P>> GuiContext for WrapperGuiContext<P, B> {
 
     unsafe fn raw_begin_set_parameter(&self, _param: ParamPtr) {
         // Since there's no automation being recorded here, gestures don't mean anything
+
+        #[cfg(debug_assertions)]
+        match self.wrapper.param_id_from_ptr(_param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .begin_set_parameter(param_id),
+            None => nih_debug_assert_failure!(
+                "raw_begin_set_parameter() called with an unknown ParamPtr"
+            ),
+        }
     }
 
     unsafe fn raw_set_parameter_normalized(&self, param: ParamPtr, normalized: f32) {
         self.wrapper.set_parameter(param, normalized);
+
+        #[cfg(debug_assertions)]
+        match self.wrapper.param_id_from_ptr(param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .set_parameter(param_id),
+            None => {
+                nih_debug_assert_failure!("raw_set_parameter() called with an unknown ParamPtr")
+            }
+        }
     }
 
-    unsafe fn raw_end_set_parameter(&self, _param: ParamPtr) {}
+    unsafe fn raw_end_set_parameter(&self, _param: ParamPtr) {
+        #[cfg(debug_assertions)]
+        match self.wrapper.param_id_from_ptr(_param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .end_set_parameter(param_id),
+            None => {
+                nih_debug_assert_failure!("raw_end_set_parameter() called with an unknown ParamPtr")
+            }
+        }
+    }
 
     fn get_state(&self) -> crate::wrapper::state::PluginState {
         self.wrapper.get_state_object()

src/wrapper/standalone/wrapper.rs

@@ -378,6 +378,13 @@ impl<P: Plugin, B: Backend<P>> Wrapper<P, B> {
         Ok(())
     }
 
+    /// Get a parameter's ID based on a `ParamPtr`. Used in the `GuiContext` implementation for the
+    /// gesture checks.
+    #[allow(unused)]
+    pub fn param_id_from_ptr(&self, param: ParamPtr) -> Option<&str> {
+        self.param_ptr_to_id.get(&param).map(|s| s.as_str())
+    }
+
     /// Set a parameter based on a `ParamPtr`. The value will be updated at the end of the next
     /// processing cycle, and this won't do anything if the parameter has not been registered by the
     /// plugin.
@@ -554,7 +561,11 @@ impl<P: Plugin, B: Backend<P>> Wrapper<P, B> {
     }
 
     fn make_gui_context(self: Arc<Self>) -> Arc<WrapperGuiContext<P, B>> {
-        Arc::new(WrapperGuiContext { wrapper: self })
+        Arc::new(WrapperGuiContext {
+            wrapper: self,
+            #[cfg(debug_assertions)]
+            param_gesture_checker: Default::default(),
+        })
     }
 
     fn make_init_context(&self) -> WrapperInitContext<'_, P, B> {

src/wrapper/util.rs

@@ -5,6 +5,8 @@ use std::os::raw::c_char;
 
 use crate::util::permit_alloc;
 
+pub(crate) mod context_checks;
+
 /// The bit that controls flush-to-zero behavior for denormals in 32 and 64-bit floating point
 /// numbers on AArch64.
 ///

src/wrapper/util/context_checks.rs

@@ -0,0 +1,67 @@
+//! Common validation used for the [contexts][crate::context].
+
+use std::collections::HashSet;
+
+/// Ensures that parameter changes send from the GUI are wrapped in parameter gestures, and that the
+/// gestures are handled consistently (no duplicate starts and ends, no end before start, etc.).
+///
+/// Should only be used in debug builds.
+#[derive(Debug, Default)]
+pub struct ParamGestureChecker {
+    /// The parameters with an active gesture.
+    active_params: HashSet<String>,
+}
+
+impl Drop for ParamGestureChecker {
+    fn drop(&mut self) {
+        nih_debug_assert!(
+            self.active_params.is_empty(),
+            "GuiContext::end_set_parameter() was never called for {} {} {:?}",
+            self.active_params.len(),
+            if self.active_params.len() == 1 {
+                "parameter"
+            } else {
+                "parameters"
+            },
+            self.active_params
+        );
+    }
+}
+
+impl ParamGestureChecker {
+    /// Called for
+    /// [`GuiContext::begin_set_parameter()`][crate::prelude::GuiContext::begin_set_parameter()].
+    /// Triggers a debug assertion failure if the state is inconsistent.
+    pub fn begin_set_parameter(&mut self, param_id: &str) {
+        nih_debug_assert!(
+            !self.active_params.contains(param_id),
+            "GuiContext::begin_set_parameter() was called twice for parameter '{}'",
+            param_id
+        );
+        self.active_params.insert(param_id.to_owned());
+    }
+
+    /// Called for [`GuiContext::set_parameter()`][crate::prelude::GuiContext::set_parameter()].
+    /// Triggers a debug assertion failure if the state is inconsistent.
+    pub fn set_parameter(&self, param_id: &str) {
+        nih_debug_assert!(
+            self.active_params.contains(param_id),
+            "GuiContext::set_parameter() was called for parameter '{}' without a preceding \
+             begin_set_parameter() call",
+            param_id
+        );
+    }
+
+    /// Called for
+    /// [`GuiContext::end_set_parameter()`][crate::prelude::GuiContext::end_set_parameter()].
+    /// Triggers a debug assertion failure if the state is inconsistent.
+    pub fn end_set_parameter(&mut self, param_id: &str) {
+        nih_debug_assert!(
+            self.active_params.contains(param_id),
+            "GuiContext::end_set_parameter() was called for parameter '{}' without a preceding \
+             begin_set_parameter() call",
+            param_id
+        );
+        self.active_params.remove(param_id);
+    }
+}

src/wrapper/vst3/context.rs

@@ -52,6 +52,9 @@ pub(crate) struct WrapperProcessContext<'a, P: Vst3Plugin> {
 /// with the host for things like setting parameters.
 pub(crate) struct WrapperGuiContext<P: Vst3Plugin> {
     pub(super) inner: Arc<WrapperInner<P>>,
+    #[cfg(debug_assertions)]
+    pub(super) param_gesture_checker:
+        atomic_refcell::AtomicRefCell<crate::wrapper::util::context_checks::ParamGestureChecker>,
 }
 
 impl<P: Vst3Plugin> Drop for WrapperInitContext<'_, P> {
@@ -144,6 +147,17 @@ impl<P: Vst3Plugin> GuiContext for WrapperGuiContext<P> {
             },
             None => nih_debug_assert_failure!("Component handler not yet set"),
         }
+
+        #[cfg(debug_assertions)]
+        match self.inner.param_id_from_ptr(param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .begin_set_parameter(param_id),
+            None => nih_debug_assert_failure!(
+                "raw_begin_set_parameter() called with an unknown ParamPtr"
+            ),
+        }
     }
 
     unsafe fn raw_set_parameter_normalized(&self, param: ParamPtr, normalized: f32) {
@@ -174,6 +188,17 @@ impl<P: Vst3Plugin> GuiContext for WrapperGuiContext<P> {
             },
             None => nih_debug_assert_failure!("Component handler not yet set"),
         }
+
+        #[cfg(debug_assertions)]
+        match self.inner.param_id_from_ptr(param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .set_parameter(param_id),
+            None => {
+                nih_debug_assert_failure!("raw_set_parameter() called with an unknown ParamPtr")
+            }
+        }
     }
 
     unsafe fn raw_end_set_parameter(&self, param: ParamPtr) {
@@ -186,6 +211,17 @@ impl<P: Vst3Plugin> GuiContext for WrapperGuiContext<P> {
             },
             None => nih_debug_assert_failure!("Component handler not yet set"),
         }
+
+        #[cfg(debug_assertions)]
+        match self.inner.param_id_from_ptr(param) {
+            Some(param_id) => self
+                .param_gesture_checker
+                .borrow_mut()
+                .end_set_parameter(param_id),
+            None => {
+                nih_debug_assert_failure!("raw_end_set_parameter() called with an unknown ParamPtr")
+            }
+        }
     }
 
     fn get_state(&self) -> PluginState {

src/wrapper/vst3/inner.rs

@@ -369,7 +369,11 @@ impl<P: Vst3Plugin> WrapperInner<P> {
     }
 
     pub fn make_gui_context(self: Arc<Self>) -> Arc<WrapperGuiContext<P>> {
-        Arc::new(WrapperGuiContext { inner: self })
+        Arc::new(WrapperGuiContext {
+            inner: self,
+            #[cfg(debug_assertions)]
+            param_gesture_checker: Default::default(),
+        })
     }
 
     /// # Note
@@ -431,6 +435,16 @@ impl<P: Vst3Plugin> WrapperInner<P> {
         }
     }
 
+    /// Get a parameter's ID based on a `ParamPtr`. Used in the `GuiContext` implementation for the
+    /// gesture checks.
+    #[allow(unused)]
+    pub fn param_id_from_ptr(&self, param: ParamPtr) -> Option<&str> {
+        self.param_ptr_to_hash
+            .get(&param)
+            .and_then(|hash| self.param_id_by_hash.get(hash))
+            .map(|s| s.as_str())
+    }
+
     /// Convenience function for setting a value for a parameter as triggered by a VST3 parameter
     /// update. The same rate is for updating parameter smoothing.
     ///