1
0
Fork 0

Greatly simplify Params trait

This now is a single vector with all of the information in the correct
order instead of the hashmaps and a vector. This avoids deduplication,
and it especially makes manual `Params` implementations a lot more
convenient since you can't mess up with mismatching IDs between the
methods.

To accommodate exactly this, the persistent fields methods also have a
default implementation and the trait has been marked as `unsafe` since
it's the programmer's responsibility to make sure these `ParamPtr`s will
remain valid.
This commit is contained in:
Robbert van der Helm 2022-03-20 13:03:03 +01:00
parent 8371f767ce
commit c22e522629
5 changed files with 113 additions and 126 deletions

View file

@ -29,9 +29,7 @@ pub fn derive_params(input: TokenStream) -> TokenStream {
// those fields individually (so they can be added and removed independently of eachother) using // those fields individually (so they can be added and removed independently of eachother) using
// JSON. The `nested` fields should also implement the `Params` trait and their fields will be // JSON. The `nested` fields should also implement the `Params` trait and their fields will be
// inherited and added to this field's lists. // inherited and added to this field's lists.
let mut param_mapping_insert_tokens = Vec::new(); let mut param_mapping_self_tokens = Vec::new();
let mut param_groups_insert_tokens = Vec::new();
let mut param_id_string_tokens = Vec::new();
let mut field_serialize_tokens = Vec::new(); let mut field_serialize_tokens = Vec::new();
let mut field_deserialize_tokens = Vec::new(); let mut field_deserialize_tokens = Vec::new();
let mut nested_params_field_idents: Vec<syn::Ident> = Vec::new(); let mut nested_params_field_idents: Vec<syn::Ident> = Vec::new();
@ -146,18 +144,16 @@ pub fn derive_params(input: TokenStream) -> TokenStream {
.into(); .into();
} }
// The specific parameter types know how to convert themselves into the correct ParamPtr // These are pairs of `(parameter_id, param_ptr, param_group)`. The specific
// variant // parameter types know how to convert themselves into the correct ParamPtr variant.
param_mapping_insert_tokens
.push(quote! { param_map.insert(String::from(#param_id), self.#field_name.as_ptr()); });
// Top-level parameters have no group, and we'll prefix the group name specified in // Top-level parameters have no group, and we'll prefix the group name specified in
// the `#[nested = "..."]` attribute to fields coming from nested groups // the `#[nested = "..."]` attribute to fields coming from nested groups
param_groups_insert_tokens param_mapping_self_tokens.push(
.push(quote! { param_groups.insert(String::from(#param_id), String::new()); }); quote! { (String::from(#param_id), self.#field_name.as_ptr(), String::new()) },
param_id_string_tokens.push(quote! { String::from(#param_id), }); );
} }
(None, Some(stable_name)) => { (None, Some(persist_key)) => {
if !persist_ids.insert(stable_name.clone()) { if !persist_ids.insert(persist_key.clone()) {
return syn::Error::new( return syn::Error::new(
field.span(), field.span(),
"Multiple persisted fields with the same ID found", "Multiple persisted fields with the same ID found",
@ -175,15 +171,15 @@ pub fn derive_params(input: TokenStream) -> TokenStream {
::nih_plug::param::internals::serialize_field, ::nih_plug::param::internals::serialize_field,
) { ) {
Ok(data) => { Ok(data) => {
serialized.insert(String::from(#stable_name), data); serialized.insert(String::from(#persist_key), data);
} }
Err(err) => { Err(err) => {
::nih_plug::nih_log!("Could not serialize '{}': {}", #stable_name, err) ::nih_plug::nih_log!("Could not serialize '{}': {}", #persist_key, err)
} }
}; };
}); });
field_deserialize_tokens.push(quote! { field_deserialize_tokens.push(quote! {
#stable_name => { #persist_key => {
match ::nih_plug::param::internals::deserialize_field(&data) { match ::nih_plug::param::internals::deserialize_field(&data) {
Ok(deserialized) => { Ok(deserialized) => {
::nih_plug::param::internals::PersistentField::set( ::nih_plug::param::internals::PersistentField::set(
@ -194,7 +190,7 @@ pub fn derive_params(input: TokenStream) -> TokenStream {
Err(err) => { Err(err) => {
::nih_plug::nih_log!( ::nih_plug::nih_log!(
"Could not deserialize '{}': {}", "Could not deserialize '{}': {}",
#stable_name, #persist_key,
err err
) )
} }
@ -221,43 +217,29 @@ pub fn derive_params(input: TokenStream) -> TokenStream {
} }
quote! { quote! {
impl #impl_generics Params for #struct_name #ty_generics #where_clause { unsafe impl #impl_generics Params for #struct_name #ty_generics #where_clause {
fn param_map( fn param_map(
self: std::pin::Pin<&Self>, self: std::pin::Pin<&Self>,
) -> std::collections::HashMap<String, nih_plug::param::internals::ParamPtr> { ) -> Vec<(String, nih_plug::param::internals::ParamPtr, String)> {
// This may not be in scope otherwise, used to call .as_ptr() // This may not be in scope otherwise, used to call .as_ptr()
use ::nih_plug::param::Param; use ::nih_plug::param::Param;
let mut param_map = std::collections::HashMap::new(); let mut param_map = vec![#(#param_mapping_self_tokens),*];
#(#param_mapping_insert_tokens)*
let nested_params_fields: &[&dyn Params] = &[#(&self.#nested_params_field_idents),*];
for nested_params in nested_params_fields {
unsafe { param_map.extend(Pin::new_unchecked(*nested_params).param_map()) };
}
param_map
}
fn param_groups(
self: std::pin::Pin<&Self>,
) -> std::collections::HashMap<String, String> {
let mut param_groups = std::collections::HashMap::new();
#(#param_groups_insert_tokens)*
let nested_params_fields: &[&dyn Params] = &[#(&self.#nested_params_field_idents),*]; let nested_params_fields: &[&dyn Params] = &[#(&self.#nested_params_field_idents),*];
let nested_params_groups: &[String] = &[#(String::from(#nested_params_group_names)),*]; let nested_params_groups: &[String] = &[#(String::from(#nested_params_group_names)),*];
for (nested_params, group_name) in for (nested_params, group_name) in
nested_params_fields.into_iter().zip(nested_params_groups) nested_params_fields.into_iter().zip(nested_params_groups)
{ {
let nested_param_groups = let nested_param_map =
unsafe { std::pin::Pin::new_unchecked(*nested_params).param_groups() }; unsafe { std::pin::Pin::new_unchecked(*nested_params).param_map() };
let prefixed_nested_param_groups = let prefixed_nested_param_map =
nested_param_groups nested_param_map
.into_iter() .into_iter()
.map(|(param_id, nested_group_name)| { .map(|(param_id, param_ptr, nested_group_name)| {
( (
param_id, param_id,
param_ptr,
if nested_group_name.is_empty() { if nested_group_name.is_empty() {
group_name.to_string() group_name.to_string()
} else { } else {
@ -266,21 +248,10 @@ pub fn derive_params(input: TokenStream) -> TokenStream {
) )
}); });
param_groups.extend(prefixed_nested_param_groups); param_map.extend(prefixed_nested_param_map);
} }
param_groups param_map
}
fn param_ids(self: std::pin::Pin<&Self>) -> Vec<String> {
let mut ids = vec![#(#param_id_string_tokens)*];
let nested_params_fields: &[&dyn Params] = &[#(&self.#nested_params_field_idents),*];
for nested_params in nested_params_fields {
unsafe { ids.append(&mut Pin::new_unchecked(*nested_params).param_ids()) };
}
ids
} }
fn serialize_fields(&self) -> ::std::collections::HashMap<String, String> { fn serialize_fields(&self) -> ::std::collections::HashMap<String, String> {

View file

@ -39,43 +39,37 @@ pub use serde_json::to_string as serialize_field;
/// ///
/// This implementation is safe when using from the wrapper because the plugin object needs to be /// This implementation is safe when using from the wrapper because the plugin object needs to be
/// pinned, and it can never outlive the wrapper. /// pinned, and it can never outlive the wrapper.
pub trait Params { pub unsafe trait Params {
// NOTE: These types use `String` even though for the `Params` derive macro `&'static str` would /// Create a mapping from unique parameter IDs to parameters along with the name of the
// have been fine to be able to support custom reusable Params implemnetations. /// group/unit/module they are in. The order of the `Vec` determines the display order in the
/// (host's) generic UI. The group name is either an empty string for top level parameters, or a
// TODO: Combine `param_map`, `param_groups`, and `param_ids` into a single function that /// slash/delimited `"Group Name 1/Group Name 2"` path for parameters in nested groups. All
// returns a vector of tuples /// components of a group path must exist or may encounter panics. The derive macro does this
/// for every parameter field marked with `#[id = "stable"]`, and it also inlines all fields
/// Create a mapping from unique parameter IDs to parameters. This is done for every parameter /// from child `Params` structs marked with `#[nested = "Group Name"]`, prefixing that group
/// field marked with `#[id = "stable_name"]`. Dereferencing the pointers stored in the values /// name before the parameter's originanl group name. Dereferencing the pointers stored in the
/// is only valid as long as this pinned object is valid. /// values is only valid as long as this pinned object is valid.
fn param_map(self: Pin<&Self>) -> HashMap<String, ParamPtr>;
/// Contains group names for each parameter in [`param_map()`][Self::param_map()]. This is
/// either an empty string for top level parameters, or a slash/delimited `"Group Name 1/Group
/// Name 2"` string for parameters that belong to `#[nested = "Name"]` parameter objects.
fn param_groups(self: Pin<&Self>) -> HashMap<String, String>;
/// All parameter IDs from [`param_map()`][Self::param_map()], in a stable order. This order
/// will be used to display the parameters.
/// ///
/// TODO: This used to be a static slice, but now that we supported nested parameter objects /// # Note
/// that's become a bit more difficult since Rust does not have a convenient way to ///
/// concatenate an arbitrary number of static slices. There's probably a better way to do /// This uses `String` even though for the `Params` derive macro `&'static str` would have been
/// this. /// fine to be able to support custom reusable Params implemnetations.
fn param_ids(self: Pin<&Self>) -> Vec<String>; fn param_map(self: Pin<&Self>) -> Vec<(String, ParamPtr, String)>;
/// Serialize all fields marked with `#[persist = "stable_name"]` into a hash map containing /// Serialize all fields marked with `#[persist = "stable_name"]` into a hash map containing
/// JSON-representations of those fields so they can be written to the plugin's state and /// JSON-representations of those fields so they can be written to the plugin's state and
/// recalled later. This uses [`serialize_field()`] under the hood. /// recalled later. This uses [`serialize_field()`] under the hood.
fn serialize_fields(&self) -> HashMap<String, String>; fn serialize_fields(&self) -> HashMap<String, String> {
HashMap::new()
}
/// Restore all fields marked with `#[persist = "stable_name"]` from a hashmap created by /// Restore all fields marked with `#[persist = "stable_name"]` from a hashmap created by
/// [`serialize_fields()`][Self::serialize_fields()]. All of thse fields should be wrapped in a /// [`serialize_fields()`][Self::serialize_fields()]. All of thse fields should be wrapped in a
/// [`PersistentField`] with thread safe interior mutability, like an `RwLock` or a `Mutex`. /// [`PersistentField`] with thread safe interior mutability, like an `RwLock` or a `Mutex`.
/// This gets called when the plugin's state is being restored. This uses [deserialize_field()] /// This gets called when the plugin's state is being restored. This uses [deserialize_field()]
/// under the hood. /// under the hood.
fn deserialize_fields(&self, serialized: &HashMap<String, String>); #[allow(unused_variables)]
fn deserialize_fields(&self, serialized: &HashMap<String, String>) {}
} }
/// Internal pointers to parameters. This is an implementation detail used by the wrappers for type /// Internal pointers to parameters. This is an implementation detail used by the wrappers for type

View file

@ -135,7 +135,7 @@ pub trait Plugin: Default + Send + Sync + 'static {
/// this function with the same maximum block size first before calling /// this function with the same maximum block size first before calling
/// [`Smoother::next_block()`][crate::prelude::Smoother::next_block()]. /// [`Smoother::next_block()`][crate::prelude::Smoother::next_block()].
fn initialize_block_smoothers(&mut self, max_block_size: usize) { fn initialize_block_smoothers(&mut self, max_block_size: usize) {
for param in self.params().param_map().values_mut() { for (_, mut param, _) in self.params().param_map() {
unsafe { param.initialize_block_smoother(max_block_size) }; unsafe { param.initialize_block_smoother(max_block_size) };
} }
} }

View file

@ -52,7 +52,7 @@ use parking_lot::RwLock;
use raw_window_handle::RawWindowHandle; use raw_window_handle::RawWindowHandle;
use std::any::Any; use std::any::Any;
use std::cmp; use std::cmp;
use std::collections::{HashMap, VecDeque}; use std::collections::{HashMap, HashSet, VecDeque};
use std::ffi::{c_void, CStr}; use std::ffi::{c_void, CStr};
use std::mem; use std::mem;
use std::os::raw::c_char; use std::os::raw::c_char;
@ -327,45 +327,56 @@ impl<P: ClapPlugin> Wrapper<P> {
let host_callback = unsafe { ClapPtr::new(host_callback) }; let host_callback = unsafe { ClapPtr::new(host_callback) };
// This is a mapping from the parameter IDs specified by the plugin to pointers to thsoe // This is a mapping from the parameter IDs specified by the plugin to pointers to thsoe
// parameters. Since the object returned by `params()` is pinned, these pointers are safe to // parameters. These pointers are assumed to be safe to dereference as long as
// dereference as long as `wrapper.plugin` is alive // `wrapper.plugin` is alive. The plugin API identifiers these parameters by hashes, which
let param_map = plugin.read().params().param_map(); // we'll calculate from the string ID specified by the plugin. These parameters should also
let param_ids = plugin.read().params().param_ids(); // remain in the same order as the one returned by the plugin.
let param_groups = plugin.read().params().param_groups(); let param_id_hashes_ptrs_groups: Vec<_> = plugin
nih_debug_assert!( .read()
!param_map.contains_key(BYPASS_PARAM_ID), .params()
"The wrapper already adds its own bypass parameter" .param_map()
); .into_iter()
.map(|(id, ptr, group)| {
// Only calculate these hashes once, and in the stable order defined by the plugin let hash = hash_param_id(&id);
let param_id_hashes_ptrs_groups: Vec<_> = param_ids (id, hash, ptr, group)
.iter()
.map(|id| {
// If any of these keys are missing then that's a bug in the Params implementation
let param_ptr = param_map[id];
let param_group = &param_groups[id];
(id, hash_param_id(id), param_ptr, param_group)
}) })
.collect(); .collect();
if cfg!(debug_assertions) {
let param_map = plugin.read().params().param_map();
let param_ids: HashSet<_> = param_id_hashes_ptrs_groups
.iter()
.map(|(id, _, _, _)| id.clone())
.collect();
nih_debug_assert!(
!param_ids.contains(BYPASS_PARAM_ID),
"The wrapper already adds its own bypass parameter"
);
nih_debug_assert_eq!(
param_map.len(),
param_ids.len(),
"The plugin has duplicate parameter IDs, weird things may happen"
);
}
let param_hashes = param_id_hashes_ptrs_groups let param_hashes = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(_, hash, _, _)| hash) .map(|(_, hash, _, _)| *hash)
.collect(); .collect();
let param_by_hash = param_id_hashes_ptrs_groups let param_by_hash = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(_, hash, ptr, _)| (hash, ptr)) .map(|(_, hash, ptr, _)| (*hash, *ptr))
.collect(); .collect();
let param_group_by_hash = param_id_hashes_ptrs_groups let param_group_by_hash = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(_, hash, _, group)| (hash, group.to_string())) .map(|(_, hash, _, group)| (*hash, group.clone()))
.collect(); .collect();
let param_defaults_normalized = param_id_hashes_ptrs_groups let param_defaults_normalized = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(_, hash, ptr, _)| (hash, unsafe { ptr.normalized_value() })) .map(|(_, hash, ptr, _)| (*hash, unsafe { ptr.normalized_value() }))
.collect(); .collect();
let param_id_to_hash = param_id_hashes_ptrs_groups let param_id_to_hash = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(id, hash, _, _)| (id.clone(), hash)) .map(|(id, hash, _, _)| (id.clone(), *hash))
.collect(); .collect();
let param_ptr_to_hash = param_id_hashes_ptrs_groups let param_ptr_to_hash = param_id_hashes_ptrs_groups
.into_iter() .into_iter()

View file

@ -2,7 +2,7 @@ use atomic_refcell::AtomicRefCell;
use crossbeam::atomic::AtomicCell; use crossbeam::atomic::AtomicCell;
use parking_lot::RwLock; use parking_lot::RwLock;
use std::cmp::Reverse; use std::cmp::Reverse;
use std::collections::{BinaryHeap, HashMap, VecDeque}; use std::collections::{BinaryHeap, HashMap, HashSet, VecDeque};
use std::mem::MaybeUninit; use std::mem::MaybeUninit;
use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
use std::sync::Arc; use std::sync::Arc;
@ -137,25 +137,36 @@ impl<P: Vst3Plugin> WrapperInner<P> {
let editor = plugin.read().editor().map(Arc::from); let editor = plugin.read().editor().map(Arc::from);
// This is a mapping from the parameter IDs specified by the plugin to pointers to thsoe // This is a mapping from the parameter IDs specified by the plugin to pointers to thsoe
// parameters. Since the object returned by `params()` is pinned, these pointers are safe to // parameters. These pointers are assumed to be safe to dereference as long as
// dereference as long as `wrapper.plugin` is alive // `wrapper.plugin` is alive. The plugin API identifiers these parameters by hashes, which
let param_map = plugin.read().params().param_map(); // we'll calculate from the string ID specified by the plugin. These parameters should also
let param_groups = plugin.read().params().param_groups(); // remain in the same order as the one returned by the plugin.
let param_ids = plugin.read().params().param_ids(); let param_id_hashes_ptrs_groups: Vec<_> = plugin
nih_debug_assert!( .read()
!param_map.contains_key(BYPASS_PARAM_ID), .params()
"The wrapper already adds its own bypass parameter" .param_map()
); .into_iter()
.map(|(id, ptr, group)| {
// Only calculate these hashes once, and in the stable order defined by the plugin let hash = hash_param_id(&id);
let param_id_hashes_ptrs_groups: Vec<_> = param_ids (id, hash, ptr, group)
.iter()
.filter_map(|id| {
let param_ptr = param_map.get(id)?;
let param_group = param_groups.get(id)?;
Some((id, hash_param_id(id), param_ptr, param_group))
}) })
.collect(); .collect();
if cfg!(debug_assertions) {
let param_map = plugin.read().params().param_map();
let param_ids: HashSet<_> = param_id_hashes_ptrs_groups
.iter()
.map(|(id, _, _, _)| id.clone())
.collect();
nih_debug_assert!(
!param_ids.contains(BYPASS_PARAM_ID),
"The wrapper already adds its own bypass parameter"
);
nih_debug_assert_eq!(
param_map.len(),
param_ids.len(),
"The plugin has duplicate parameter IDs, weird things may happen"
);
}
let param_hashes = param_id_hashes_ptrs_groups let param_hashes = param_id_hashes_ptrs_groups
.iter() .iter()
@ -163,25 +174,25 @@ impl<P: Vst3Plugin> WrapperInner<P> {
.collect(); .collect();
let param_by_hash = param_id_hashes_ptrs_groups let param_by_hash = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(_, hash, ptr, _)| (hash, *ptr)) .map(|&(_, hash, ptr, _)| (hash, ptr))
.collect(); .collect();
let param_units = ParamUnits::from_param_groups( let param_units = ParamUnits::from_param_groups(
param_id_hashes_ptrs_groups param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(_, hash, _, group_name)| (hash, group_name.as_str())), .map(|(_, hash, _, group_name)| (*hash, group_name.as_str())),
) )
.expect("Inconsistent parameter groups"); .expect("Inconsistent parameter groups");
let param_defaults_normalized = param_id_hashes_ptrs_groups let param_defaults_normalized = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(_, hash, ptr, _)| (hash, unsafe { ptr.normalized_value() })) .map(|(_, hash, ptr, _)| (*hash, unsafe { ptr.normalized_value() }))
.collect(); .collect();
let param_id_to_hash = param_id_hashes_ptrs_groups let param_id_to_hash = param_id_hashes_ptrs_groups
.iter() .iter()
.map(|&(id, hash, _, _)| (id.clone(), hash)) .map(|(id, hash, _, _)| (id.clone(), *hash))
.collect(); .collect();
let param_ptr_to_hash = param_id_hashes_ptrs_groups let param_ptr_to_hash = param_id_hashes_ptrs_groups
.into_iter() .into_iter()
.map(|(_, hash, ptr, _)| (*ptr, hash)) .map(|(_, hash, ptr, _)| (ptr, hash))
.collect(); .collect();
let wrapper = Self { let wrapper = Self {