diff --git a/node-graph/libraries/core-types/src/list.rs b/node-graph/libraries/core-types/src/list.rs index 399f573bdca..832d6a58cab 100644 --- a/node-graph/libraries/core-types/src/list.rs +++ b/node-graph/libraries/core-types/src/list.rs @@ -6,6 +6,7 @@ use dyn_any::{DynAny, StaticType, StaticTypeSized}; use glam::DAffine2; use graphene_hash::CacheHash; use std::fmt::Debug; +use std::sync::Arc; // ================================================= // Standard attribute keys used across the data flow @@ -199,7 +200,7 @@ fn implicit_default_value(key: &str) -> Option> { } /// Appends `count` copies of `key`'s implicit default to `attribute` (see [`implicit_default_value`]). -fn pad_with_implicit_default(key: &str, attribute: &mut Box, count: usize) { +fn pad_with_implicit_default(key: &str, attribute: &mut dyn AnyAttribute, count: usize) { match implicit_default_value(key) { Some(default) => attribute.push_repeated(&*default, count), None => { @@ -210,6 +211,60 @@ fn pad_with_implicit_default(key: &str, attribute: &mut Box, c } } +// ============ +// AttributeKey +// ============ + +/// An attribute's name. Short names are stored inline and longer ones shared, so copying an item's or a list's attributes allocates nothing for their names. +#[derive(Clone)] +enum AttributeKey { + Inline { length: u8, bytes: [u8; AttributeKey::INLINE_CAPACITY] }, + Allocated(Arc), +} + +impl AttributeKey { + /// The longest name stored inline, which keeps a key the size of a `String`. + const INLINE_CAPACITY: usize = 22; + + fn new(key: &str) -> Self { + let mut bytes = [0; Self::INLINE_CAPACITY]; + + match bytes.get_mut(..key.len()) { + Some(inline) => { + inline.copy_from_slice(key.as_bytes()); + Self::Inline { length: key.len() as u8, bytes } + } + None => Self::Allocated(key.into()), + } + } + + fn as_bytes(&self) -> &[u8] { + match self { + Self::Inline { length, bytes } => bytes.get(..*length as usize).unwrap_or_default(), + Self::Allocated(key) => key.as_bytes(), + } + } + + fn as_str(&self) -> &str { + match self { + // The inline bytes are a whole `&str` copied in, so they always read back as one + Self::Inline { .. } => std::str::from_utf8(self.as_bytes()).unwrap_or_default(), + Self::Allocated(key) => key, + } + } + + /// Whether this is the attribute named `key`. + fn is(&self, key: &str) -> bool { + self.as_bytes() == key.as_bytes() + } +} + +impl PartialEq for AttributeKey { + fn eq(&self, other: &Self) -> bool { + self.as_bytes() == other.as_bytes() + } +} + // ======================== // TRAIT: AnyAttributeValue // ======================== @@ -220,6 +275,9 @@ pub trait AnyAttributeValue: std::any::Any + Send + Sync { /// Clones this value into a new boxed trait object. fn clone_box(&self) -> Box; + /// Clones this value into a new shared trait object. + fn clone_arc(&self) -> Arc; + /// Returns a shared reference to the underlying concrete type for downcasting. fn as_any(&self) -> &dyn std::any::Any; @@ -229,6 +287,9 @@ pub trait AnyAttributeValue: std::any::Any + Send + Sync { /// Consumes the box and returns the underlying concrete type for downcasting. fn into_any(self: Box) -> Box; + /// Converts the shared value into one that can be downcast to its concrete type. + fn into_any_arc(self: Arc) -> Arc; + /// Returns a debug-formatted string representation of this value. fn display_string(&self) -> String; @@ -240,7 +301,7 @@ pub trait AnyAttributeValue: std::any::Any + Send + Sync { fn eq_dyn(&self, other: &dyn AnyAttributeValue) -> bool; /// Wraps this scalar value into a new attribute, preceded by `preceding_defaults` implicit defaults for `key`. - fn into_attribute(self: Box, key: &str, preceding_defaults: usize) -> Box; + fn into_attribute(self: Arc, key: &str, preceding_defaults: usize) -> Arc; } impl AnyAttributeValue for T { @@ -249,6 +310,11 @@ impl Arc { + Arc::new(self.clone()) + } + /// Returns a shared reference to the underlying concrete type for downcasting. fn as_any(&self) -> &dyn std::any::Any { self @@ -264,6 +330,11 @@ impl) -> Arc { + self + } + /// Returns a debug-formatted string representation of this value. fn display_string(&self) -> String { format!("{:?}", self) @@ -271,7 +342,7 @@ impl, key: &str, preceding_defaults: usize) -> Box { - let mut attribute: Box = Box::new(Attribute::(Vec::with_capacity(preceding_defaults + 1))); + fn into_attribute(self: Arc, key: &str, preceding_defaults: usize) -> Arc { + let mut attribute = Attribute::(Vec::with_capacity(preceding_defaults + 1)); pad_with_implicit_default(key, &mut attribute, preceding_defaults); - attribute.push(self); - attribute + attribute.0.push(Arc::unwrap_or_clone(self)); + + Arc::new(attribute) } } @@ -295,14 +367,23 @@ impl Clone for Box { } } +/// Gives write access to an attribute value, first copying it if clones of its item still share it. +fn unshared_value(value: &mut Arc) -> &mut dyn AnyAttributeValue { + if Arc::get_mut(value).is_none() { + *value = (**value).clone_arc(); + } + + Arc::get_mut(value).expect("A value that was just copied has no other owner") +} + // =================== // TRAIT: AnyAttribute // =================== /// Enables type-erased storage for parallel attribute lists in a [`List`]. pub trait AnyAttribute: std::any::Any + Send + Sync { - /// Clones this attribute into a new boxed trait object. - fn clone_box(&self) -> Box; + /// Clones this attribute into a new shared trait object. + fn clone_arc(&self) -> Arc; /// Returns a shared reference to the underlying concrete type for downcasting. fn as_any(&self) -> &dyn std::any::Any; @@ -310,8 +391,11 @@ pub trait AnyAttribute: std::any::Any + Send + Sync { /// Returns a mutable reference to the underlying concrete type for downcasting. fn as_any_mut(&mut self) -> &mut dyn std::any::Any; + /// Converts the shared attribute into one that can be downcast to its concrete type. + fn into_any_arc(self: Arc) -> Arc; + /// Pushes a scalar attribute value onto the end of this attribute. - fn push(&mut self, value: Box); + fn push(&mut self, value: Arc); /// Pushes a default value onto the end of this attribute. fn push_default(&mut self); @@ -325,7 +409,7 @@ pub trait AnyAttribute: std::any::Any + Send + Sync { fn set_at(&mut self, index: usize, value: Box); /// Creates a new attribute of the same type filled with `count` number of default values. - fn new_with_defaults(&self, count: usize) -> Box; + fn new_with_defaults(&self, count: usize) -> Arc; /// Returns the number of elements in this attribute. fn len(&self) -> usize; @@ -336,7 +420,7 @@ pub trait AnyAttribute: std::any::Any + Send + Sync { } /// Appends all values from another attribute of the same type. - fn extend(&mut self, other: Box); + fn extend(&mut self, other: Arc); /// Returns a shared reference to the value at the requested index. fn get_any(&self, index: usize) -> Option<&dyn std::any::Any>; @@ -344,11 +428,11 @@ pub trait AnyAttribute: std::any::Any + Send + Sync { /// Returns a debug-formatted display string for the value at the requested index. fn display_at(&self, index: usize) -> Option; - /// Clones a single value from this attribute into a boxed scalar attribute value. - fn clone_value(&self, index: usize) -> Option>; + /// Clones a single value from this attribute into a shared scalar attribute value. + fn clone_value(&self, index: usize) -> Option>; - /// Drains all values out of this attribute into a Vec of scalar attribute values. - fn drain(self: Box) -> Vec>; + /// Turns this attribute into a Vec of scalar attribute values, moving them out unless clones of its list still share it. + fn drain(self: Arc) -> Vec>; /// Hashes every value in this attribute into the given hasher (object-safe wrapper around `CacheHash`). fn cache_hash_dyn(&self, state: &mut dyn core::hash::Hasher); @@ -359,23 +443,71 @@ pub trait AnyAttribute: std::any::Any + Send + Sync { } /// Adapts a `&mut dyn Hasher` so generic `CacheHash::cache_hash` calls (which require `H: Sized + Hasher`) can -/// drive a trait-object hasher. Forwards both `Hasher` methods to the inner `dyn Hasher`. -struct DynHasher<'a>(&'a mut dyn core::hash::Hasher); +/// drive a trait-object hasher. Writes gather in a buffer that reaches the inner hasher whole, when the next write +/// would not fit and when this is dropped, which spares a virtual call for every value hashed. +struct DynHasher<'a> { + inner: &'a mut dyn core::hash::Hasher, + buffer: [u8; DynHasher::CAPACITY], + used: usize, +} + +impl<'a> DynHasher<'a> { + const CAPACITY: usize = 128; + + fn new(inner: &'a mut dyn core::hash::Hasher) -> Self { + Self { + inner, + buffer: [0; Self::CAPACITY], + used: 0, + } + } + + fn flush(&mut self) { + if let Some(gathered) = self.buffer.get(..self.used).filter(|gathered| !gathered.is_empty()) { + self.inner.write(gathered); + } + self.used = 0; + } +} + impl core::hash::Hasher for DynHasher<'_> { + /// Reads the inner hasher's state without the bytes still buffered, which only reach it once this hasher is dropped. fn finish(&self) -> u64 { - self.0.finish() + debug_assert_eq!(self.used, 0, "The buffered bytes reach the inner hasher only once this hasher is dropped"); + self.inner.finish() } + fn write(&mut self, bytes: &[u8]) { - self.0.write(bytes) + if bytes.len() > Self::CAPACITY - self.used { + self.flush(); + } + + // Anything too long for the buffer passes straight through, in order since the buffer was just emptied + match self.buffer.get_mut(self.used..self.used + bytes.len()) { + Some(space) => { + space.copy_from_slice(bytes); + self.used += bytes.len(); + } + None => self.inner.write(bytes), + } } } -impl Clone for Box { - fn clone(&self) -> Self { - (**self).clone_box() +impl Drop for DynHasher<'_> { + fn drop(&mut self) { + self.flush(); } } +/// Gives write access to an attribute, first copying it if clones of its list still share it. +fn unshared(attribute: &mut Arc) -> &mut dyn AnyAttribute { + if Arc::get_mut(attribute).is_none() { + *attribute = (**attribute).clone_arc(); + } + + Arc::get_mut(attribute).expect("An attribute that was just copied has no other owner") +} + // ============ // Attribute // ============ @@ -384,9 +516,9 @@ impl Clone for Box { pub struct Attribute(pub Vec); impl AnyAttribute for Attribute { - /// Clones this attribute into a new boxed trait object. - fn clone_box(&self) -> Box { - Box::new(Attribute(self.0.clone())) + /// Clones this attribute into a new shared trait object. + fn clone_arc(&self) -> Arc { + Arc::new(Attribute(self.0.clone())) } /// Returns a shared reference to the underlying concrete type for downcasting. @@ -399,14 +531,16 @@ impl self } + /// Converts the shared attribute into one that can be downcast to its concrete type. + fn into_any_arc(self: Arc) -> Arc { + self + } + /// Pushes an attribute value onto the end of this attribute's list, downcasting it to `T`. /// Falls back to a default value if the type doesn't match, to maintain the attribute-length invariant. - fn push(&mut self, value: Box) { - if let Ok(value) = value.into_any().downcast::() { - self.0.push(*value); - } else { - self.0.push(T::default()); - } + fn push(&mut self, value: Arc) { + let value = value.into_any_arc().downcast::().map(Arc::unwrap_or_clone).unwrap_or_default(); + self.0.push(value); } /// Pushes a default `T` value onto the end of this attribute list. @@ -435,8 +569,8 @@ impl } /// Creates a new attribute filled with `count` default `T` values. - fn new_with_defaults(&self, count: usize) -> Box { - Box::new(Attribute(vec![T::default(); count])) + fn new_with_defaults(&self, count: usize) -> Arc { + Arc::new(Attribute(vec![T::default(); count])) } /// Returns the number of elements in this attribute. @@ -446,12 +580,13 @@ impl /// Appends all values from another attribute, downcasting it to the same `Attribute` type. /// Falls back to padding with defaults if the type doesn't match, to maintain the attribute-length invariant. - fn extend(&mut self, other: Box) { + fn extend(&mut self, other: Arc) { let other_len = other.len(); - if let Ok(other) = (other as Box).downcast::() { - self.0.extend(other.0); - } else { - self.0.extend(std::iter::repeat_with(T::default).take(other_len)); + + match other.into_any_arc().downcast::().map(Arc::try_unwrap) { + Ok(Ok(other)) => self.0.extend(other.0), + Ok(Err(shared)) => self.0.extend_from_slice(&shared.0), + Err(_) => self.0.extend(std::iter::repeat_with(T::default).take(other_len)), } } @@ -465,19 +600,24 @@ impl self.0.get(index).map(|v| format!("{v:?}")) } - /// Clones the value at the given index into a boxed scalar attribute value. - fn clone_value(&self, index: usize) -> Option> { - self.0.get(index).map(|v| Box::new(v.clone()) as Box) + /// Clones the value at the given index into a shared scalar attribute value. + fn clone_value(&self, index: usize) -> Option> { + self.0.get(index).map(|v| Arc::new(v.clone()) as Arc) } - /// Consumes this attribute and returns all values as a Vec of boxed scalar attribute values. - fn drain(self: Box) -> Vec> { - self.0.into_iter().map(|v| Box::new(v) as Box).collect() + /// Returns all values as a Vec of shared scalar attribute values, moved out unless clones of the list still share them. + fn drain(self: Arc) -> Vec> { + let values = match Arc::try_unwrap(self) { + Ok(attribute) => attribute.0, + Err(shared) => shared.0.clone(), + }; + + values.into_iter().map(|v| Arc::new(v) as Arc).collect() } /// Hashes every value in this attribute into the given hasher (object-safe wrapper around `CacheHash`). fn cache_hash_dyn(&self, state: &mut dyn core::hash::Hasher) { - self.0.cache_hash(&mut DynHasher(state)); + self.0.cache_hash(&mut DynHasher::new(state)); } /// Compares this attribute to another for value-by-value equality (object-safe wrapper around `PartialEq`). @@ -535,75 +675,51 @@ unsafe impl StaticType for AttributeValueDyn { /// Type-erased view of a `List` exposing only its attributes and item count, used as a node graph parameter type. /// Lets a node accept any `List` source via the auto-inserted `Convert` without monomorphizing over `U`, /// for cases where the element type is irrelevant (such as nodes that read out a named attribute regardless of the carrier `List`). -#[derive(Default)] +#[derive(Clone, Default)] pub struct ListDyn { - attributes: Vec<(String, Box)>, - len: usize, + attributes: Attributes, } impl ListDyn { /// Number of items in the underlying `List`. pub fn len(&self) -> usize { - self.len + self.attributes.len } /// Whether the underlying `List` has zero items. pub fn is_empty(&self) -> bool { - self.len == 0 + self.attributes.len == 0 } /// Returns a reference to the attribute value at the given key and item index, downcast to `U`, if present and matching. pub fn attribute(&self, key: &str, index: usize) -> Option<&U> { - self.attributes - .iter() - .find_map(|(k, attribute)| if k == key { attribute.get_any(index)?.downcast_ref::() } else { None }) + self.attributes.get_value(key, index) } } impl From> for ListDyn { fn from(list: List) -> Self { - Self { - attributes: list.attributes.attributes, - len: list.attributes.len, - } - } -} - -impl Clone for ListDyn { - fn clone(&self) -> Self { - Self { - attributes: self.attributes.iter().map(|(key, attribute)| (key.clone(), attribute.clone_box())).collect(), - len: self.len, - } + Self { attributes: list.attributes } } } impl Debug for ListDyn { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let keys: Vec<&str> = self.attributes.iter().map(|(k, _)| k.as_str()).collect(); - f.debug_struct("ListDyn").field("keys", &keys).field("len", &self.len).finish() + let keys: Vec<&str> = self.attributes.keys().collect(); + f.debug_struct("ListDyn").field("keys", &keys).field("len", &self.attributes.len).finish() } } impl PartialEq for ListDyn { fn eq(&self, other: &Self) -> bool { - self.len == other.len - && self.attributes.len() == other.attributes.len() - && self - .attributes - .iter() - .zip(&other.attributes) - .all(|((key_a, attribute_a), (key_b, attribute_b))| key_a == key_b && attribute_a.eq_dyn(&**attribute_b)) + self.attributes.len == other.attributes.len && self.attributes.equals(&other.attributes) } } impl CacheHash for ListDyn { fn cache_hash(&self, state: &mut H) { - self.len.cache_hash(state); - for (key, attribute) in &self.attributes { - key.cache_hash(state); - attribute.cache_hash_dyn(state); - } + self.attributes.len.cache_hash(state); + self.attributes.cache_hash_entries(state); } } @@ -611,32 +727,59 @@ unsafe impl StaticType for ListDyn { type Static = Self; } +// ====== +// Shared +// ====== + +/// A handle on an attribute value that an item also holds, for reading it without copying it. It is not an +/// attribute value itself, so it cannot be stored back onto an item in place of the value it reads. +pub struct Shared(Arc); + +impl std::ops::Deref for Shared { + type Target = T; + + fn deref(&self) -> &T { + &self.0 + } +} + +impl From> for Shared { + fn from(value: Arc) -> Self { + Self(value) + } +} + // =================== // ItemAttributeValues // =================== +/// An item's attribute values with their keys, in insertion order. +type ValueEntries = Vec<(AttributeKey, Arc)>; + /// Scalar attribute storage for a single item. /// /// A small ordered map of type-erased scalar attribute values, keyed by string name. /// Used for individual attribute values in an [`Item`]. /// Linear search preserves insertion order and is likely faster than a HashMap for small attribute counts. +/// Clones share the map and every value in it until one of them is written to. #[derive(Clone, Default)] -pub struct ItemAttributeValues(Vec<(String, Box)>); +pub struct ItemAttributeValues(Option>); impl Debug for ItemAttributeValues { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let keys: Vec<&str> = self.0.iter().map(|(k, _)| k.as_str()).collect(); + let keys: Vec<&str> = self.keys().collect(); f.debug_struct("Attributes").field("keys", &keys).finish() } } impl PartialEq for ItemAttributeValues { fn eq(&self, other: &Self) -> bool { - self.0.len() == other.0.len() - && self - .0 + let (entries, other_entries) = (self.entries(), other.entries()); + + entries.len() == other_entries.len() + && entries .iter() - .zip(&other.0) + .zip(other_entries) .all(|((self_key, self_value), (other_key, other_value))| self_key == other_key && self_value.eq_dyn(other_value.as_ref())) } } @@ -647,86 +790,121 @@ impl ItemAttributeValues { Self::default() } - /// Inserts an attribute with the given key and value, replacing any existing entry with the same key. - pub fn insert(&mut self, key: impl Into, value: T) { - let key = key.into(); + fn from_entries(entries: ValueEntries) -> Self { + Self((!entries.is_empty()).then(|| Arc::new(entries))) + } - for (existing_key, existing_value) in &mut self.0 { - if *existing_key == key { - *existing_value = Box::new(value); - return; - } - } + fn into_entries(self) -> ValueEntries { + self.0.map(Arc::unwrap_or_clone).unwrap_or_default() + } + + fn entries(&self) -> &[(AttributeKey, Arc)] { + self.0.as_deref().map_or(&[], Vec::as_slice) + } + + /// The entries for writing, which copies the map first if clones of the item still share it. + fn entries_mut(&mut self) -> &mut ValueEntries { + Arc::make_mut(self.0.get_or_insert_default()) + } + + fn position(&self, key: &str) -> Option { + self.entries().iter().position(|(existing_key, _)| existing_key.is(key)) + } - self.0.push((key, Box::new(value))); + /// Inserts an attribute with the given key and value, replacing any existing entry with the same key. + pub fn insert(&mut self, key: impl AsRef, value: T) { + let key = key.as_ref(); + let value: Arc = Arc::new(value); + + let position = self.position(key); + let entries = self.entries_mut(); + match position.and_then(|position| entries.get_mut(position)) { + Some((_, existing_value)) => *existing_value = value, + None => entries.push((AttributeKey::new(key), value)), + } } /// Gets a reference to the value of the attribute with the given key, if it exists and can be downcast to the requested type. pub fn get(&self, key: &str) -> Option<&T> { - // Explicit deref `(**value)` reaches `dyn AttributeValue` (which is !Sized and thus dispatches - // through the vtable to the concrete type) rather than resolving to the blanket - // `impl AttributeValue for Box` which would return the wrong TypeId. - self.0 + self.entries() .iter() - .find_map(|(existing_key, value)| if existing_key == key { (**value).as_any().downcast_ref::() } else { None }) + .find_map(|(existing_key, value)| if existing_key.is(key) { (**value).as_any().downcast_ref::() } else { None }) + } + + /// Gets a shared handle to the value of the attribute with the given key, if it exists and can be downcast to the requested type. + pub fn get_shared(&self, key: &str) -> Option> { + let (_, value) = self.entries().iter().find(|(existing_key, _)| existing_key.is(key))?; + + value.clone().into_any_arc().downcast::().ok().map(Shared) } /// Gets a mutable reference to the value of the attribute with the given key, if it exists and can be downcast to the requested type. pub fn get_mut(&mut self, key: &str) -> Option<&mut T> { - self.0 - .iter_mut() - .find_map(|(existing_key, value)| if existing_key == key { (**value).as_any_mut().downcast_mut::() } else { None }) + // Checked before anything is unshared, so a miss leaves the values shared + let position = self.entries().iter().position(|(existing_key, value)| existing_key.is(key) && (**value).as_any().is::())?; + let (_, value) = self.entries_mut().get_mut(position)?; + + unshared_value(value).as_any_mut().downcast_mut::() } /// Gets a mutable reference to the value, inserting a default if it doesn't exist or has the wrong type. pub fn get_or_insert_default_mut(&mut self, key: &str) -> &mut T { - let needs_insert = match self.0.iter().position(|(existing_key, _)| existing_key == key) { - Some(index) => { - if (*self.0[index].1).as_any().downcast_ref::().is_some() { - false - } else { - self.0.remove(index); - true - } - } - None => true, - }; + let position = self.position(key); + let has_type = position.and_then(|position| self.entries().get(position)).is_some_and(|(_, value)| (**value).as_any().is::()); - if needs_insert { - self.0.push((key.to_string(), Box::new(T::default()))); + if !has_type { + let entries = self.entries_mut(); + if let Some(position) = position { + entries.remove(position); + } + entries.push((AttributeKey::new(key), Arc::new(T::default()))); } self.get_mut::(key).expect("Attribute was just ensured to exist with correct type") } /// Removes and returns the value for the given key, if it exists and can be downcast to the requested type. - pub fn remove(&mut self, key: &str) -> Option { - let index = self.0.iter().position(|(existing_key, _)| existing_key == key)?; - let (_, value) = self.0.remove(index); - value.into_any().downcast::().ok().map(|boxed| *boxed) + pub fn remove(&mut self, key: &str) -> Option { + let position = self.position(key)?; + let (_, mut value) = self.entries_mut().remove(position); + if !(*value).as_any().is::() { + return None; + } + + // A value that clones of the item still share is copied, since it can only be moved out of its last owner + if Arc::get_mut(&mut value).is_none() { + value = (*value).clone_arc(); + } + Arc::try_unwrap(value.into_any_arc().downcast::().ok()?).ok() } /// Returns an iterator over the keys of all stored attributes, in insertion order. pub fn keys(&self) -> impl Iterator { - self.0.iter().map(|(key, _)| key.as_str()) + self.entries().iter().map(|(key, _)| key.as_str()) } /// Returns a type-erased reference to the value of the attribute with the given key, if it exists. pub fn get_any(&self, key: &str) -> Option<&dyn std::any::Any> { - self.0.iter().find_map(|(existing_key, value)| if existing_key == key { Some((**value).as_any()) } else { None }) + self.entries() + .iter() + .find_map(|(existing_key, value)| if existing_key.is(key) { Some((**value).as_any()) } else { None }) } /// Returns an iterator over key and type-erased value pairs of all stored attributes, in insertion order. pub fn iter_any(&self) -> impl Iterator { - self.0.iter().map(|(key, value)| (key.as_str(), (**value).as_any())) + self.entries().iter().map(|(key, value)| (key.as_str(), (**value).as_any())) } /// Returns a debug-formatted string representation of the attribute value for the given key, if it exists. /// The `overrides` function can provide custom formatting for specific type. pub fn display_value(&self, key: &str, overrides: fn(&dyn std::any::Any) -> Option) -> Option { - self.0.iter().find_map(|(k, value)| { - if k == key { - if let Some(text) = overrides(value.as_any()) { Some(text) } else { Some(value.display_string()) } + self.entries().iter().find_map(|(k, value)| { + if k.is(key) { + if let Some(text) = overrides((**value).as_any()) { + Some(text) + } else { + Some(value.display_string()) + } } else { None } @@ -735,32 +913,30 @@ impl ItemAttributeValues { /// Moves the attribute at `from_key` to `to_key`. /// Does nothing if `from_key` is absent, overwrites any existing `to_key`. - pub fn rename(&mut self, from_key: &str, to_key: impl Into) { - let Some(pos) = self.0.iter().position(|(k, _)| k == from_key) else { return }; - let (_, value) = self.0.remove(pos); - - let to_key = to_key.into(); - for (existing_key, existing_value) in &mut self.0 { - if *existing_key == to_key { - *existing_value = value; - return; - } + pub fn rename(&mut self, from_key: &str, to_key: impl AsRef) { + let Some(position) = self.position(from_key) else { return }; + let entries = self.entries_mut(); + let (_, value) = entries.remove(position); + + let to_key = to_key.as_ref(); + match entries.iter_mut().find(|(existing_key, _)| existing_key.is(to_key)) { + Some((_, existing_value)) => *existing_value = value, + None => entries.push((AttributeKey::new(to_key), value)), } - self.0.push((to_key, value)); } /// Clones the attribute with `key` from `source`, replacing any existing attribute with the same key. pub fn insert_cloned_from(&mut self, source: &Self, key: &str) { - let Some((_, value)) = source.0.iter().find(|(existing_key, _)| existing_key == key) else { + let Some((_, value)) = source.entries().iter().find(|(existing_key, _)| existing_key.is(key)) else { return; }; let value = value.clone(); - if let Some((_, existing_value)) = self.0.iter_mut().find(|(existing_key, _)| existing_key == key) { - *existing_value = value; - } else { - self.0.push((key.to_string(), value)); + let entries = self.entries_mut(); + match entries.iter_mut().find(|(existing_key, _)| existing_key.is(key)) { + Some((_, existing_value)) => *existing_value = value, + None => entries.push((AttributeKey::new(key), value)), } } } @@ -769,20 +945,94 @@ impl ItemAttributeValues { // Attributes // ========== +/// One of a list's attributes with its key. +type AttributeEntry = (AttributeKey, Arc); + +/// A list's attributes with their keys, in insertion order. A lone attribute is held inline, so the many small lists +/// carrying just one need no collection of their own. +#[derive(Clone, Default)] +enum AttributeEntries { + #[default] + None, + One(AttributeEntry), + Many(Arc>), +} + +impl AttributeEntries { + fn as_slice(&self) -> &[AttributeEntry] { + match self { + Self::None => &[], + Self::One(entry) => std::slice::from_ref(entry), + Self::Many(entries) => entries, + } + } + + /// The entries for writing in place, which copies the collection first if clones of the list still share it. + fn as_mut_slice(&mut self) -> &mut [AttributeEntry] { + match self { + Self::None => &mut [], + Self::One(entry) => std::slice::from_mut(entry), + Self::Many(entries) => Arc::make_mut(entries).as_mut_slice(), + } + } + + fn is_empty(&self) -> bool { + self.as_slice().is_empty() + } + + fn push(&mut self, entry: AttributeEntry) { + *self = match std::mem::take(self) { + Self::None => Self::One(entry), + Self::One(first) => { + let mut entries = Vec::with_capacity(4); + entries.extend([first, entry]); + Self::Many(Arc::new(entries)) + } + Self::Many(mut entries) => { + Arc::make_mut(&mut entries).push(entry); + Self::Many(entries) + } + }; + } + + /// Takes out the entry at `position`, doing nothing when there is none there. + fn remove(&mut self, position: usize) { + *self = match std::mem::take(self) { + // What is left of one or two entries is nothing or a lone entry again, neither of which needs the collection + Self::Many(entries) if position < entries.len() && entries.len() <= 2 => entries.get(1 - position).cloned().map_or(Self::None, Self::One), + Self::Many(mut entries) if position < entries.len() => { + Arc::make_mut(&mut entries).remove(position); + Self::Many(entries) + } + Self::One(_) if position == 0 => Self::None, + unchanged => unchanged, + }; + } + + fn into_vec(self) -> Vec { + match self { + Self::None => Vec::new(), + Self::One(entry) => vec![entry], + Self::Many(entries) => Arc::unwrap_or_clone(entries), + } + } +} + /// The storage data structure for attributes. /// /// A collection of type-erased parallel attributes, keyed by string name. /// All access goes through [`List`] and [`Item`] since internals are private. +/// Clones share every attribute until one of them is written to. /// Invariant: every attribute in `attributes` has exactly `len` elements. #[derive(Clone, Default)] struct Attributes { - attributes: Vec<(String, Box)>, + attributes: AttributeEntries, len: usize, } impl Debug for Attributes { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let keys: Vec<&str> = self.attributes.iter().map(|(k, _)| k.as_str()).collect(); + let keys: Vec<&str> = self.keys().collect(); f.debug_struct("Attributes").field("keys", &keys).field("len", &self.len).finish() } } @@ -795,141 +1045,195 @@ impl Attributes { /// Creates an empty attribute store with no attributes but a pre-set item count. fn with_len(len: usize) -> Self { - Self { attributes: Vec::new(), len } + Self { + attributes: AttributeEntries::None, + len, + } + } + + fn entries(&self) -> &[AttributeEntry] { + self.attributes.as_slice() + } + + fn position(&self, key: &str) -> Option { + self.entries().iter().position(|(existing_key, _)| existing_key.is(key)) + } + + /// The position of the attribute for the given key, if it exists and holds values of type `T`. + fn typed_position(&self, key: &str) -> Option { + self.entries() + .iter() + .position(|(existing_key, attribute)| existing_key.is(key) && (**attribute).as_any().is::>()) + } + + /// The values of the attribute at `position` for writing, if it holds values of type `T`. + fn values_mut(&mut self, position: usize) -> Option<&mut Vec> { + let (_, attribute) = self.attributes.as_mut_slice().get_mut(position)?; + + unshared(attribute).as_any_mut().downcast_mut::>().map(|attribute| &mut attribute.0) } /// Pushes an item's scalar attributes into this attribute store. /// Existing attributes that the item lacks receive a default value. /// New attribute keys create a new attribute padded with defaults for all prior items. fn push_item(&mut self, item: ItemAttributeValues) { - let mut item_entries = item.0; + let len = self.len; + self.len += 1; + + if self.attributes.is_empty() && item.0.is_none() { + return; + } + + let mut item_entries = item.into_entries(); // Push values into existing attributes, or the implicit default if the item lacks that attribute - for (attribute_key, attribute) in &mut self.attributes { - if let Some(position) = item_entries.iter().position(|(k, _)| k == attribute_key) { - let (_, value) = item_entries.swap_remove(position); - attribute.push(value); - } else { - pad_with_implicit_default(attribute_key, attribute, 1); + for (attribute_key, attribute) in self.attributes.as_mut_slice() { + let attribute = unshared(attribute); + + match item_entries.iter().position(|(key, _)| key == attribute_key) { + Some(position) => attribute.push(item_entries.swap_remove(position).1), + None => pad_with_implicit_default(attribute_key.as_str(), attribute, 1), } } // Create new attributes for any remaining item values, padded with implicit defaults for prior items for (key, value) in item_entries { - self.attributes.push((key.clone(), value.into_attribute(&key, self.len))); + let attribute = value.into_attribute(key.as_str(), len); + self.attributes.push((key, attribute)); } - - self.len += 1; } /// Appends all attribute data from another attribute store into this one. /// Attributes present in only one side are padded with defaults for the other side's items. fn extend(&mut self, other: Attributes) { - let other_len = other.len; - let mut other_entries = other.attributes; + let (len, other_len) = (self.len, other.len); + self.len += other_len; + + // An empty store takes on the other's attributes whole, sharing them + if len == 0 && self.entries().is_empty() { + self.attributes = other.attributes; + return; + } + // Nothing to add or pad, so self's attributes can stay shared + if other.attributes.is_empty() && (other_len == 0 || self.attributes.is_empty()) { + return; + } + + let mut other_entries = other.attributes.into_vec(); // Extend matching attributes, or pad self's attributes with implicit defaults for the other's item count - for (key, self_attribute) in &mut self.attributes { - if let Some(position) = other_entries.iter().position(|(k, _)| k == key) { - let (_, other_attribute) = other_entries.swap_remove(position); - self_attribute.extend(other_attribute); - } else { - pad_with_implicit_default(key, self_attribute, other_len); + for (key, self_attribute) in self.attributes.as_mut_slice() { + let self_attribute = unshared(self_attribute); + + match other_entries.iter().position(|(other_key, _)| other_key == key) { + Some(position) => self_attribute.extend(other_entries.swap_remove(position).1), + None => pad_with_implicit_default(key.as_str(), self_attribute, other_len), } } // Remaining other attributes are new, so we pad with implicit defaults for self's existing items for (key, other_attribute) in other_entries { let mut combined = other_attribute.new_with_defaults(0); - pad_with_implicit_default(&key, &mut combined, self.len); - combined.extend(other_attribute); + let writable = unshared(&mut combined); + pad_with_implicit_default(key.as_str(), writable, len); + writable.extend(other_attribute); + self.attributes.push((key, combined)); } - - self.len += other_len; } /// Gets a reference to the value at the given index from the attribute for the given key. fn get_value(&self, key: &str, index: usize) -> Option<&T> { - self.attributes + self.entries() .iter() - .find_map(|(k, attribute)| if k == key { attribute.get_any(index)?.downcast_ref::() } else { None }) + .find_map(|(k, attribute)| if k.is(key) { attribute.get_any(index)?.downcast_ref::() } else { None }) } /// Removes the entire attribute for the given key, if present. fn remove_attribute(&mut self, key: &str) { - if let Some(position) = self.attributes.iter().position(|(k, _)| k == key) { + if let Some(position) = self.position(key) { self.attributes.remove(position); } } /// Creates a new attribute of type `T` filled with `key`'s implicit default for all existing items. - fn new_attribute_padded(&self, key: &str) -> Box { - let mut attribute: Box = Box::new(Attribute::(Vec::with_capacity(self.len))); + fn new_attribute_padded(&self, key: &str) -> Arc { + let mut attribute = Attribute::(Vec::with_capacity(self.len)); pad_with_implicit_default(key, &mut attribute, self.len); - attribute + + Arc::new(attribute) } /// Finds or creates an attribute for the given key and type, returning its position. /// If an attribute with the key exists but has the wrong type, it is removed and replaced with a new attribute of the correct type, padded with implicit defaults. /// A newly created attribute is filled with `key`'s implicit default for all existing items. fn find_or_create_attribute(&mut self, key: &str) -> usize { - match self.attributes.iter().position(|(k, _)| k == key) { - Some(position) => { - if (*self.attributes[position].1).as_any().downcast_ref::>().is_some() { - position - } else { - self.attributes.remove(position); - let attribute = self.new_attribute_padded::(key); - self.attributes.push((key.to_string(), attribute)); - self.attributes.len() - 1 - } - } - None => { - let attribute = self.new_attribute_padded::(key); - self.attributes.push((key.to_string(), attribute)); - self.attributes.len() - 1 - } + if let Some(position) = self.typed_position::(key) { + return position; } + + let mistyped = self.position(key); + let attribute = self.new_attribute_padded::(key); + + if let Some(position) = mistyped { + self.attributes.remove(position); + } + self.attributes.push((AttributeKey::new(key), attribute)); + + self.entries().len() - 1 } /// Gets a mutable reference to the value at the given index, creating the attribute if it doesn't exist or has the wrong type. fn get_or_insert_default_value(&mut self, key: &str, index: usize) -> &mut T { let attribute_position = self.find_or_create_attribute::(key); - let attribute = (*self.attributes[attribute_position].1).as_any_mut().downcast_mut::>().unwrap(); - &mut attribute.0[index] + let values = self.values_mut::(attribute_position).unwrap(); + &mut values[index] } /// Sets the value at the given index in the attribute for the given key. /// Creates the attribute with defaults if it doesn't exist. - fn set_value(&mut self, key: impl Into, index: usize, value: T) { - let key = key.into(); - let attribute_position = self.find_or_create_attribute::(&key); - let attribute = (*self.attributes[attribute_position].1).as_any_mut().downcast_mut::>().unwrap(); - attribute.0[index] = value; + fn set_value(&mut self, key: &str, index: usize, value: T) { + let attribute_position = self.find_or_create_attribute::(key); + let values = self.values_mut::(attribute_position).unwrap(); + values[index] = value; } /// Replaces the attribute for the given key with one value per item, keeping its place unless its type changes. - fn set_attribute_values(&mut self, key: String, values: Vec) { + fn set_attribute_values(&mut self, key: &str, values: Vec) { // An attribute changing type moves to the end, as it does when set one value at a time - let existing = self.attributes.iter().position(|(k, _)| k == &key); - let same_type = existing.is_some_and(|position| (*self.attributes[position].1).as_any().downcast_ref::>().is_some()); - - let attribute: Box = Box::new(Attribute(values)); - match existing { - Some(position) if same_type => self.attributes[position].1 = attribute, - Some(position) => { - self.attributes.remove(position); - self.attributes.push((key, attribute)); - } - None => self.attributes.push((key, attribute)), + let (existing, same_type) = (self.position(key), self.typed_position::(key).is_some()); + + let attribute: Arc = Arc::new(Attribute(values)); + if same_type && let Some((_, existing_attribute)) = existing.and_then(|position| self.attributes.as_mut_slice().get_mut(position)) { + *existing_attribute = attribute; + return; + } + + if let Some(position) = existing { + self.attributes.remove(position); + } + self.attributes.push((AttributeKey::new(key), attribute)); + } + + /// Sets a single type-erased value at the given index of a list of `item_count` items, creating the attribute from the + /// value's type if the key holds none. + fn set_value_dyn(&mut self, key: &str, index: usize, value: Box, item_count: usize) { + let position = self.position(key); + + if let Some((_, attribute)) = position.and_then(|position| self.attributes.as_mut_slice().get_mut(position)) { + unshared(attribute).set_at(index, value); + } else { + let mut new_attribute = Arc::::from(value).into_attribute(key, index); + let trailing_defaults = item_count.saturating_sub(new_attribute.len()); + pad_with_implicit_default(key, unshared(&mut new_attribute), trailing_defaults); + self.attributes.push((AttributeKey::new(key), new_attribute)); } } /// Returns a debug-formatted string for the value at the given index in the attribute for the given key. fn display_value(&self, key: &str, index: usize, overrides: fn(&dyn std::any::Any) -> Option) -> Option { - self.attributes.iter().find_map(|(k, attribute)| { - if k == key { + self.entries().iter().find_map(|(k, attribute)| { + if k.is(key) { if let Some(value) = attribute.get_any(index) && let Some(text) = overrides(value) { @@ -944,19 +1248,19 @@ impl Attributes { /// Returns a type-erased reference to the value at the given index in the attribute for the given key. fn get_any_value(&self, key: &str, index: usize) -> Option<&dyn std::any::Any> { - self.attributes.iter().find_map(|(k, attribute)| if k == key { attribute.get_any(index) } else { None }) + self.entries().iter().find_map(|(k, attribute)| if k.is(key) { attribute.get_any(index) } else { None }) } /// Returns an iterator over the keys of all stored attributes (in insertion order). fn keys(&self) -> impl Iterator { - self.attributes.iter().map(|(key, _)| key.as_str()) + self.entries().iter().map(|(key, _)| key.as_str()) } /// Returns a typed slice of the attribute for the given key, if it exists and can be downcast to `Attribute`. fn get_attribute_slice(&self, key: &str) -> Option<&[T]> { - self.attributes.iter().find_map(|(k, attribute)| { - if k == key { - attribute.as_any().downcast_ref::>().map(|c| c.0.as_slice()) + self.entries().iter().find_map(|(k, attribute)| { + if k.is(key) { + (**attribute).as_any().downcast_ref::>().map(|c| c.0.as_slice()) } else { None } @@ -965,46 +1269,56 @@ impl Attributes { /// Returns a mutable typed slice of the attribute for the given key, if it exists and can be downcast to `Attribute`. fn get_attribute_slice_mut(&mut self, key: &str) -> Option<&mut [T]> { - self.attributes.iter_mut().find_map(|(k, attribute)| { - if k == key { - attribute.as_any_mut().downcast_mut::>().map(|c| c.0.as_mut_slice()) - } else { - None - } - }) + // Checked before anything is unshared, so a miss leaves the attributes shared + let position = self.typed_position::(key)?; + + self.values_mut::(position).map(Vec::as_mut_slice) } /// Returns a mutable typed slice of the attribute for the given key, creating a new attribute filled with defaults if it doesn't exist. fn get_or_create_attribute_slice_mut(&mut self, key: &str) -> &mut [T] { let position = self.find_or_create_attribute::(key); - let attribute = (*self.attributes[position].1).as_any_mut().downcast_mut::>().unwrap(); - &mut attribute.0 + self.values_mut::(position).unwrap() } /// Clones all attribute values at the given item index into a new scalar Attributes. fn clone_item(&self, index: usize) -> ItemAttributeValues { - let mut attributes = ItemAttributeValues::new(); + let entries = self.entries().iter().filter_map(|(key, attribute)| Some((key.clone(), attribute.clone_value(index)?))); - for (key, attribute) in &self.attributes { - if let Some(value) = attribute.clone_value(index) { - attributes.0.push((key.clone(), value)); - } - } - - attributes + ItemAttributeValues::from_entries(entries.collect()) } /// Drains all attribute data into a Vec of per-item scalar Attributes. fn into_item_vec(self) -> Vec { - let mut items: Vec = (0..self.len).map(|_| ItemAttributeValues::new()).collect(); + let attributes = self.attributes.into_vec(); + let mut items: Vec> = (0..self.len).map(|_| Vec::with_capacity(attributes.len())).collect(); - for (key, attribute) in self.attributes { - for (i, value) in attribute.drain().into_iter().enumerate() { - items[i].0.push((key.clone(), value)); + for (key, attribute) in attributes { + for (item, value) in items.iter_mut().zip(attribute.drain()) { + item.push((key.clone(), value)); } } - items + items.into_iter().map(ItemAttributeValues::from_entries).collect() + } + + /// Whether both stores hold the same attributes in the same order with equal values. + fn equals(&self, other: &Self) -> bool { + let (entries, other_entries) = (self.entries(), other.entries()); + + entries.len() == other_entries.len() + && entries + .iter() + .zip(other_entries) + .all(|((self_key, self_attribute), (other_key, other_attribute))| self_key == other_key && self_attribute.eq_dyn(other_attribute.as_ref())) + } + + /// Hashes every attribute's key and values. + fn cache_hash_entries(&self, state: &mut H) { + for (key, attribute) in self.entries() { + std::hash::Hash::hash(key.as_str(), state); + attribute.cache_hash_dyn(state); + } } } @@ -1111,7 +1425,7 @@ impl List { self.attributes.get_attribute_slice::(key).map(|s| s.iter()) } - /// Returns an iterator over mutable references to the values of a typed attribute attribute, or `None` if the attribute doesn't exist or has the wrong type. + /// Returns an iterator over mutable references to the values of a typed attribute, or `None` if the attribute doesn't exist or has the wrong type. pub fn iter_attribute_values_mut(&mut self, key: &str) -> Option> { self.attributes.get_attribute_slice_mut::(key).map(|s| s.iter_mut()) } @@ -1162,14 +1476,14 @@ impl List { } /// Sets the attribute value at the given item index and key, creating the attribute with defaults if it doesn't exist. - pub fn set_attribute(&mut self, key: impl Into, index: usize, value: U) { - self.attributes.set_value(key, index, value); + pub fn set_attribute(&mut self, key: impl AsRef, index: usize, value: U) { + self.attributes.set_value(key.as_ref(), index, value); } /// Sets the attribute's value for every item at once from one value per item, replacing any values the key held (an empty list still gains the key). /// Logs an error and leaves the list unchanged if the number of values differs from the number of items. - pub fn set_attribute_values(&mut self, key: impl Into, values: Vec) { - let key = key.into(); + pub fn set_attribute_values(&mut self, key: impl AsRef, values: Vec) { + let key = key.as_ref(); if values.len() != self.element.len() { log::error!("Attribute '{key}' was given {} values for a list of {} items", values.len(), self.element.len()); return; @@ -1180,16 +1494,8 @@ impl List { /// Sets a single type-erased attribute value at the given index, creating the attribute from the value's underlying type if it doesn't exist (padded with defaults to match the list's length). /// Falls back to default if the value's type doesn't match an existing attribute. - pub fn set_attribute_value_dyn(&mut self, key: impl Into, index: usize, value: AttributeValueDyn) { - let key = key.into(); - if let Some(position) = self.attributes.attributes.iter().position(|(k, _)| k == &key) { - self.attributes.attributes[position].1.set_at(index, value.0); - } else { - let mut new_attribute = value.0.into_attribute(&key, index); - let trailing_defaults = self.element.len().saturating_sub(new_attribute.len()); - pad_with_implicit_default(&key, &mut new_attribute, trailing_defaults); - self.attributes.attributes.push((key, new_attribute)); - } + pub fn set_attribute_value_dyn(&mut self, key: impl AsRef, index: usize, value: AttributeValueDyn) { + self.attributes.set_value_dyn(key.as_ref(), index, value.0, self.element.len()); } /// Removes the entire attribute for the given key, if present. @@ -1221,9 +1527,7 @@ impl List { /// This enables simultaneous mutable access to elements and a single attribute without borrowing conflicts. pub fn element_and_attribute_slices_mut(&mut self, key: &str) -> (&mut [T], &mut [U]) { let Self { element, attributes } = self; - let attribute_position = attributes.find_or_create_attribute::(key); - let attribute = (*attributes.attributes[attribute_position].1).as_any_mut().downcast_mut::>().unwrap(); - (element.as_mut_slice(), &mut attribute.0) + (element.as_mut_slice(), attributes.get_or_create_attribute_slice_mut::(key)) } // ================== @@ -1320,26 +1624,16 @@ impl CacheHash for List { fn cache_hash(&self, state: &mut H) { self.element.cache_hash(state); - // Hash every attribute attribute (key + values) rather than just the well-known ones, so changes to user-defined keys + // Hash every attribute (key + values) rather than just the well-known ones, so changes to user-defined keys // (e.g., gradient_form, gradient_spread) invalidate downstream graph caches as expected - for (key, attribute) in &self.attributes.attributes { - std::hash::Hash::hash(key.as_str(), state); - attribute.cache_hash_dyn(state); - } + self.attributes.cache_hash_entries(state); } } impl PartialEq for List { fn eq(&self, other: &Self) -> bool { // Attributes participate in equality so the `a == b` ⇒ `hash(a) == hash(b)` contract holds with `cache_hash` - self.element == other.element - && self.attributes.attributes.len() == other.attributes.attributes.len() - && self - .attributes - .attributes - .iter() - .zip(&other.attributes.attributes) - .all(|((self_key, self_attribute), (other_key, other_attribute))| self_key == other_key && self_attribute.eq_dyn(other_attribute.as_ref())) + self.element == other.element && self.attributes.equals(&other.attributes) } } @@ -1415,7 +1709,7 @@ impl CacheHash for Item { self.element.cache_hash(state); // Hash every attribute (key + value) so attribute changes invalidate downstream caches, mirroring `List` - for (key, attribute) in &self.attributes.0 { + for (key, attribute) in self.attributes.entries() { std::hash::Hash::hash(key.as_str(), state); attribute.cache_hash_dyn(state); } @@ -1468,6 +1762,13 @@ impl Item { self.attributes.get(key) } + /// Returns a shared handle to the attribute value for the given key, if it exists and is of the requested type. + /// The handle goes on reading the value without copying it, whatever then happens to the item. Writing or removing + /// the item's value while a handle is held copies the value first, so a handle is best dropped before then. + pub fn attribute_shared(&self, key: &str) -> Option> { + self.attributes.get_shared(key) + } + /// Returns the attribute value for the given key, or the provided default if absent or of a different type. pub fn attribute_or<'a, U: 'static>(&'a self, key: &str, default: &'a U) -> &'a U { self.attribute(key).unwrap_or(default) @@ -1494,18 +1795,18 @@ impl Item { } /// Sets the attribute value for the given key, replacing any existing entry with the same key. - pub fn set_attribute(&mut self, key: impl Into, value: U) { + pub fn set_attribute(&mut self, key: impl AsRef, value: U) { self.attributes.insert(key, value); } /// Sets the attribute value for the given key and returns the item, enabling builder-style chaining. - pub fn with_attribute(mut self, key: impl Into, value: U) -> Self { + pub fn with_attribute(mut self, key: impl AsRef, value: U) -> Self { self.set_attribute(key, value); self } /// Removes and returns the attribute value for the given key, if it exists and is of the requested type. - pub fn remove_attribute(&mut self, key: &str) -> Option { + pub fn remove_attribute(&mut self, key: &str) -> Option { self.attributes.remove(key) } } @@ -1640,4 +1941,183 @@ mod tests { assert_eq!(list.attribute_keys().collect::>(), [ATTR_END, ATTR_START]); assert_eq!(list, one_at_a_time); } + + fn hash_of(value: &T) -> u64 { + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + value.cache_hash(&mut hasher); + core::hash::Hasher::finish(&hasher) + } + + // Attribute values are hashed through a buffer, which must not let a change slip by wherever it falls + #[test] + fn attribute_values_all_reach_the_hash() { + let build = |changed: Option| { + let mut list = List::new_from_elements(vec![0_u8; 300]); + let mut values: Vec = (0..300).collect(); + if let Some(index) = changed { + values[index] += 1; + } + list.set_attribute_values(ATTR_START, values); + list.set_attribute_values(ATTR_NAME, vec![String::from("a string long enough to pass straight through the buffer when repeated").repeat(3); 300]); + list + }; + + let hash = hash_of(&build(None)); + assert_eq!(hash, hash_of(&build(None))); + for index in [0, 15, 16, 17, 127, 128, 299] { + assert_ne!(hash, hash_of(&build(Some(index))), "a change at {index} went unnoticed"); + } + } + + /// Keeps every byte it is given, in order, so a buffered stream can be checked against what went into it. + #[derive(Default)] + struct RecordingHasher(Vec); + + impl core::hash::Hasher for RecordingHasher { + fn finish(&self) -> u64 { + 0 + } + + fn write(&mut self, bytes: &[u8]) { + self.0.extend_from_slice(bytes); + } + } + + // Writes of every length pass through the buffer whole and in order, also from a value hashing through a buffer of its own + #[test] + fn buffered_writes_reach_the_inner_hasher_in_order() { + use core::hash::Hasher; + + let lengths = [3, 125, 1, 200, 0, 128, 64, 65, 300, 127, 2]; + let pieces: Vec> = lengths.iter().enumerate().map(|(index, &length)| (0..length).map(|byte| (byte + index * 31) as u8).collect()).collect(); + + let mut recorder = RecordingHasher::default(); + { + let mut outer = DynHasher::new(&mut recorder); + for (index, piece) in pieces.iter().enumerate() { + // Every third piece comes from a nested value, the way a list stored as an attribute value is hashed + if index % 3 == 2 { + DynHasher::new(&mut outer).write(piece); + } else { + outer.write(piece); + } + } + } + + assert_eq!(recorder.0, pieces.concat()); + } + + // A cloned list shares each attribute with its source until one of the two writes to it + #[test] + fn list_clones_share_attributes_until_written() { + let mut original = List::new_from_elements(vec![1_u32, 2, 3]); + original.set_attribute_values(ATTR_START, vec![10_u64, 20, 30]); + original.set_attribute_values(ATTR_END, vec![1_u64, 2, 3]); + let values = |list: &List, key: &str| list.iter_attribute_values::(key).map(|values| values.as_slice().as_ptr()); + + let mut copy = original.clone(); + assert_eq!(values(&original, ATTR_START), values(©, ATTR_START)); + assert_eq!((&original, hash_of(&original)), (©, hash_of(©))); + + copy.set_attribute(ATTR_START, 1, 99_u64); + assert_eq!(original.attribute::(ATTR_START, 1), Some(&20)); + assert_eq!(copy.attribute::(ATTR_START, 1), Some(&99)); + assert_ne!(original, copy); + + // Only the attribute written to stops being shared + assert_ne!(values(&original, ATTR_START), values(©, ATTR_START)); + assert_eq!(values(&original, ATTR_END), values(©, ATTR_END)); + } + + // A list reads the same however many attributes it has held, and a lone attribute is shared with clones like any other + #[test] + fn a_lone_attribute_reads_like_one_among_several() { + let mut lone = List::new_from_elements(vec![1_u32, 2, 3]); + lone.set_attribute_values(ATTR_START, vec![10_u64, 20, 30]); + let values = |list: &List, key: &str| list.iter_attribute_values::(key).map(|values| values.as_slice().as_ptr()); + + let mut copy = lone.clone(); + assert_eq!(values(&lone, ATTR_START), values(©, ATTR_START)); + copy.set_attribute(ATTR_START, 1, 99_u64); + assert_eq!((lone.attribute::(ATTR_START, 1), copy.attribute::(ATTR_START, 1)), (Some(&20), Some(&99))); + assert_ne!(values(&lone, ATTR_START), values(©, ATTR_START)); + + // A clone giving the lone attribute up or another type leaves the source as it was + let mut emptied = lone.clone(); + emptied.remove_attribute(ATTR_START); + assert_eq!(emptied, List::new_from_elements(vec![1_u32, 2, 3])); + let mut retyped = lone.clone(); + retyped.set_attribute_values(ATTR_START, vec![0.5_f64, 1.5, 2.5]); + assert_eq!(retyped.attribute::(ATTR_START, 2), Some(&2.5)); + assert_eq!(lone.attribute::(ATTR_START, 2), Some(&30)); + + // Gaining a second attribute and losing it again leaves a list equal to one that never had it + let mut regrown = lone.clone(); + regrown.set_attribute_values(ATTR_END, vec![1_u64, 2, 3]); + assert_eq!(regrown.attribute_keys().collect::>(), [ATTR_START, ATTR_END]); + assert_eq!(values(&lone, ATTR_START), values(®rown, ATTR_START)); + regrown.remove_attribute(ATTR_END); + assert_eq!((®rown, hash_of(®rown)), (&lone, hash_of(&lone))); + + regrown.remove_attribute(ATTR_START); + assert_eq!(regrown, List::new_from_elements(vec![1_u32, 2, 3])); + assert_eq!(lone.attribute::(ATTR_START, 2), Some(&30)); + } + + // A cloned item shares each attribute value with its source until one of the two writes to it or takes it out + #[test] + fn item_clones_share_values_until_written() { + let original = Item::new_from_element(()).with_attribute(ATTR_NAME, vec![1_u32, 2, 3]).with_attribute(ATTR_OPACITY, 0.5_f64); + let name = |item: &Item<()>| item.attribute::>(ATTR_NAME).map(|name| name.as_ptr()); + + let mut copy = original.clone(); + assert_eq!(name(&original), name(©)); + assert_eq!((&original, hash_of(&original)), (©, hash_of(©))); + + copy.attribute_mut::>(ATTR_NAME).unwrap().push(4); + assert_eq!(original.attribute::>(ATTR_NAME), Some(&vec![1, 2, 3])); + assert_eq!(copy.attribute::>(ATTR_NAME), Some(&vec![1, 2, 3, 4])); + + let mut emptied = original.clone(); + assert_eq!(emptied.remove_attribute::>(ATTR_NAME), Some(vec![1, 2, 3])); + assert_eq!(emptied.attribute::>(ATTR_NAME), None); + assert_eq!(original.attribute::>(ATTR_NAME), Some(&vec![1, 2, 3])); + } + + // Taking a shared list apart into items and building on them leaves the list it was cloned from as it was + #[test] + fn items_from_a_shared_list_leave_its_source_intact() { + let mut original = List::new_from_elements(vec![1_u32, 2]); + original.set_attribute_values(ATTR_START, vec![10_u64, 20]); + let starts = |list: &List| list.iter_attribute_values::(ATTR_START).unwrap().copied().collect::>(); + + let raise = |mut item: Item| { + *item.attribute_mut_or_insert_default::(ATTR_START) += 1; + item + }; + let mut rebuilt: List = original.clone().into_iter().map(raise).collect(); + rebuilt.extend(original.clone()); + rebuilt.push(Item::new_from_element(3)); + + assert_eq!(starts(&original), [10, 20]); + assert_eq!(starts(&rebuilt), [11, 21, 10, 20, 0]); + } + + // Keys are stored inline up to a length limit and shared beyond it, which reads the same either way + #[test] + fn attribute_keys_of_any_length_read_back() { + let long_key = "a_user_defined_attribute_name_longer_than_the_inline_limit"; + let keys = [long_key, ATTR_GRADIENT_HUE_DIRECTION, "", "name with spaces"]; + + let mut list = List::new_from_elements(vec![0_u8]); + for (value, key) in keys.into_iter().enumerate() { + list.set_attribute(key, 0, value as u64); + } + + assert_eq!(list.attribute_keys().collect::>(), keys); + for (value, key) in keys.into_iter().enumerate() { + assert_eq!(list.attribute::(key, 0), Some(&(value as u64))); + } + assert_eq!(list.attribute::("gradient_hue_directio", 0), None); + } } diff --git a/node-graph/libraries/vector-types/src/vector/addressing.rs b/node-graph/libraries/vector-types/src/vector/addressing.rs index 4f25b09f786..e80df3799a5 100644 --- a/node-graph/libraries/vector-types/src/vector/addressing.rs +++ b/node-graph/libraries/vector-types/src/vector/addressing.rs @@ -1,6 +1,6 @@ use super::connectivity::Connectivity; use super::misc::HandleId; -use super::path::{CurveType, Path, curve_type_at}; +use super::path::{CurveType, Path, curve_type_at, curve_types_of}; use super::vector_attributes::{PointId, SegmentId}; use core_types::list::{ATTR_ID, List}; use glam::DVec2; @@ -27,9 +27,46 @@ impl Span { /// Every span of every path, in path order. pub fn spans(paths: &List) -> impl Iterator + '_ { - paths.iter_element_values().enumerate().flat_map(|(path_index, path)| { - let curve_type = curve_type_at(paths, path_index); - (0..path.segment_count(curve_type) as u32).map(move |index| Span { path: path_index as u32, index }) + let paths_with_curve_types = paths.iter_element_values().zip(curve_types_of(paths)).enumerate(); + + paths_with_curve_types.flat_map(|(path_index, (path, curve_type))| (0..path.segment_count(curve_type) as u32).map(move |index| Span { path: path_index as u32, index })) +} + +/// A span together with what its path holds for it. +pub(crate) struct ResolvedSpan<'a> { + pub span: Span, + pub path: &'a Path, + pub curve_type: CurveType, + /// The span's stable ID, absent when its path names no spans. + pub id: Option, + /// The pool points the span runs between. + pub endpoints: (u32, u32), +} + +/// Every span of every path in path order, resolved in O(references) by reading each path's curve type, references, and IDs once. +pub(crate) fn resolved_spans(paths: &List) -> impl Iterator> + '_ { + paths.iter_element_values().zip(curve_types_of(paths)).enumerate().flat_map(move |(path_index, (path, curve_type))| { + let stride = curve_type.stride(); + let references = path.0.iter_element_values().as_slice(); + let ids = path.0.iter_attribute_values::(ATTR_ID).map(|ids| ids.as_slice()); + + (0..path.segment_count(curve_type)).filter_map(move |index| { + let reference = index * stride; + let endpoints = (*references.get(reference)? as u32, *references.get(reference + stride)? as u32); + let span = Span { + path: path_index as u32, + index: index as u32, + }; + let id = ids.and_then(|ids| ids.get(reference).copied()); + + Some(ResolvedSpan { + span, + path, + curve_type, + id, + endpoints, + }) + }) }) } @@ -44,7 +81,7 @@ pub fn segment_id_of(paths: &List, span: Span) -> Option { /// The span a segment ID names, found by scanning every span in O(references). pub fn span_of_segment_id(paths: &List, segment: SegmentId) -> Option { - spans(paths).find(|&span| segment_id_of(paths, span) == Some(segment)) + resolved_spans(paths).find(|resolved| resolved.id == Some(segment)).map(|resolved| resolved.span) } /// The pool points a span runs between. diff --git a/node-graph/libraries/vector-types/src/vector/connectivity.rs b/node-graph/libraries/vector-types/src/vector/connectivity.rs index 9938aa9f76a..9a27795847d 100644 --- a/node-graph/libraries/vector-types/src/vector/connectivity.rs +++ b/node-graph/libraries/vector-types/src/vector/connectivity.rs @@ -1,4 +1,4 @@ -use super::path::{CurveType, Path, curve_type_at}; +use super::path::{CurveType, Path, curve_types_of}; use core_types::list::List; /// One path's reference to a pool point, naming where in the weave that point is used. @@ -49,9 +49,8 @@ impl Connectivity { let mut cursors = starts[..point_count].to_vec(); let mut references = vec![Reference::default(); starts[point_count] as usize]; - for (path_index, path) in paths.iter_element_values().enumerate() { - // Each path's curve type is read once here, so queries never look it up again - let curve_type = curve_type_at(paths, path_index); + // Each path's curve type is read once here, so queries never look it up again + for ((path_index, path), curve_type) in paths.iter_element_values().enumerate().zip(curve_types_of(paths)) { let last = path.0.len().saturating_sub(1); for (reference_index, point) in path.references().enumerate() { diff --git a/node-graph/libraries/vector-types/src/vector/contour.rs b/node-graph/libraries/vector-types/src/vector/contour.rs index b0cf78faf52..5e8355d021c 100644 --- a/node-graph/libraries/vector-types/src/vector/contour.rs +++ b/node-graph/libraries/vector-types/src/vector/contour.rs @@ -1,5 +1,5 @@ use super::misc::dvec2_to_point; -use super::path::{CurveType, Path, append_segment, curve_type_at}; +use super::path::{CurveType, Path, append_segment, curve_types_of}; use core_types::list::List; use glam::DVec2; use kurbo::BezPath; @@ -94,7 +94,7 @@ pub fn contours(point_count: usize, paths: &List) -> Vec { /// The curve type of every path, read once so the stitching below never looks one up per path. fn curve_types(paths: &List) -> Vec { - (0..paths.len()).map(|index| curve_type_at(paths, index)).collect() + curve_types_of(paths).collect() } fn contours_with(point_count: usize, paths: &List, curve_types: &[CurveType]) -> Vec { diff --git a/node-graph/libraries/vector-types/src/vector/flatten.rs b/node-graph/libraries/vector-types/src/vector/flatten.rs index da3cb3a8ff8..5cce94d222f 100644 --- a/node-graph/libraries/vector-types/src/vector/flatten.rs +++ b/node-graph/libraries/vector-types/src/vector/flatten.rs @@ -1,5 +1,5 @@ use super::misc::{BezierHandles, HandleId, HandleType, segment_to_handles}; -use super::path::{CurveType, Path, continues_previous_path, curve_type_at, push_path}; +use super::path::{CurveType, Path, continues_previous_path, curve_types_of, push_paths}; use super::vector_attributes::{PointId, SegmentId}; use super::vector_types::Vector; use core_types::list::{ATTR_ID, List}; @@ -44,8 +44,7 @@ impl WorkingVector { let working_point = |pool_index: u32| pool_to_working.get(pool_index as usize).copied().flatten(); // One pass so a span that fails to resolve leaves every array at the same length - for (path_index, path) in paths.iter_element_values().enumerate() { - let curve_type = curve_type_at(paths, path_index); + for ((path_index, path), curve_type) in paths.iter_element_values().enumerate().zip(curve_types_of(paths)) { let stride = curve_type.stride(); let span_ids = path.0.iter_attribute_values::(ATTR_ID).map(|ids| ids.as_slice()).unwrap_or_default(); @@ -223,9 +222,7 @@ impl WorkingVector { vector.0.set_attribute_values(ATTR_ID, point_ids); } - for (curve_type, references, ids) in runs { - push_path(paths, curve_type, references, ids); - } + push_paths(paths, runs); // G1 pairs narrow to the smooth flag on the anchor both handles face, and other pairings are dropped let anchor = |handle: HandleId| { @@ -331,9 +328,9 @@ pub(crate) fn chain_order(point_count: usize, endpoints: &[(usize, usize)], dire let mut partner = vec![usize::MAX; segment_count * 2]; for ends in ends_at.iter().filter(|ends| ends.len() >= 2) { if ends.len() > MOST_ENDS_PAIRED_BY_STRAIGHTNESS { - for pair in ends.chunks_exact(2) { - partner[pair[0]] = pair[1]; - partner[pair[1]] = pair[0]; + for &[first, second] in ends.as_chunks::<2>().0 { + partner[first] = second; + partner[second] = first; } continue; } @@ -509,6 +506,7 @@ pub fn vector_from_legacy_parts( #[cfg(test)] mod tests { use super::*; + use crate::vector::path::{curve_type_at, push_path}; use core_types::list::{ATTR_CURVE_TYPE, ATTR_ID, ATTR_SMOOTH, Item}; fn working(points: usize, segments: &[(usize, usize, BezierHandles)]) -> WorkingVector { diff --git a/node-graph/libraries/vector-types/src/vector/geometry.rs b/node-graph/libraries/vector-types/src/vector/geometry.rs index 9672efee695..589d4f9e501 100644 --- a/node-graph/libraries/vector-types/src/vector/geometry.rs +++ b/node-graph/libraries/vector-types/src/vector/geometry.rs @@ -222,10 +222,6 @@ impl VectorGeometry { self.vector.bezpath_from_segments_ignore_discontinuities(segments) } - pub fn point_tangent(&self, point_index: usize) -> Option { - self.vector.point_tangent(&self.paths, point_index) - } - /// The manipulator's position in layer space. See [`ManipulatorPointId::get_position`]. pub fn position_of(&self, point: ManipulatorPointId) -> Option { point.get_position(&self.vector, &self.paths) diff --git a/node-graph/libraries/vector-types/src/vector/path.rs b/node-graph/libraries/vector-types/src/vector/path.rs index 22ecd44da35..10f3827528b 100644 --- a/node-graph/libraries/vector-types/src/vector/path.rs +++ b/node-graph/libraries/vector-types/src/vector/path.rs @@ -1,7 +1,7 @@ use super::algorithms::consts::CLOSE_POINT_TOLERANCE; use super::misc::{dvec2_to_point, point_to_dvec2}; -use super::vector_attributes::SegmentId; -use core_types::list::{ATTR_CURVE_TYPE, ATTR_ID, ATTR_PATHS, Item, List}; +use super::vector_attributes::{PointId, SegmentId}; +use core_types::list::{ATTR_CURVE_TYPE, ATTR_ID, ATTR_PATHS, Item, List, Shared}; use glam::DVec2; use graphene_hash::CacheHash; use kurbo::{BezPath, CubicBez, Line, PathEl, PathSeg, Point, QuadBez}; @@ -135,8 +135,8 @@ impl Path { } } -/// Appends one path weaving the given references in the curve type's stride, naming its spans with the given IDs in order. -pub fn push_path(paths: &mut List, curve_type: CurveType, references: impl IntoIterator, ids: impl IntoIterator) { +/// Weaves the given references into a path in the curve type's stride, naming its spans with the given IDs in order. +fn woven_path(curve_type: CurveType, references: impl IntoIterator, ids: impl IntoIterator) -> Path { let mut path = Path::from_references(references); let stride = curve_type.stride(); @@ -149,9 +149,34 @@ pub fn push_path(paths: &mut List, curve_type: CurveType, references: impl path.0.set_attribute_values(ATTR_ID, span_ids); } + path +} + +/// Appends one path weaving the given references in the curve type's stride, naming its spans with the given IDs in order. +pub fn push_path(paths: &mut List, curve_type: CurveType, references: impl IntoIterator, ids: impl IntoIterator) { + let path = woven_path(curve_type, references, ids); paths.push(Item::new_from_element(path).with_attribute(ATTR_CURVE_TYPE, curve_type)); } +/// Appends a path for each run of references, as [`push_path`] would one at a time. +pub(crate) fn push_paths(paths: &mut List, runs: impl IntoIterator, Vec)>) { + let woven = runs.into_iter().map(|(curve_type, references, ids)| (curve_type, woven_path(curve_type, references, ids))); + let (curve_types, woven) = woven.unzip(); + + extend_with_paths(paths, curve_types, woven); +} + +/// Appends paths with their curve types as one piece, which spares each path a pass through an item of its own. +fn extend_with_paths(paths: &mut List, curve_types: Vec, woven: Vec) { + if woven.is_empty() { + return; + } + + let mut appended = List::new_from_elements(woven); + appended.set_attribute_values(ATTR_CURVE_TYPE, curve_types); + paths.extend(appended); +} + /// Appends a segment to a bezpath already positioned at that segment's start point. pub fn append_segment(bezpath: &mut BezPath, segment: PathSeg) { match segment { @@ -166,6 +191,14 @@ pub fn paths_from_item_attributes(item: &Item) -> Option<&List> { item.attribute(ATTR_PATHS) } +/// The paths an item carries beside a vector element, held without copying them and empty when it carries none. +/// The handle goes on reading the paths whatever then happens to the item. +pub fn shared_paths_from_item_attributes(item: &Item) -> Shared> { + static EMPTY: std::sync::LazyLock>> = std::sync::LazyLock::new(Default::default); + + item.attribute_shared(ATTR_PATHS).unwrap_or_else(|| EMPTY.clone().into()) +} + /// The paths a list carries at `index` beside a vector element, absent when the pool is a bare point cloud. pub fn paths_from_list_row_attributes(list: &List, index: usize) -> Option<&List> { list.attribute(ATTR_PATHS, index) @@ -182,6 +215,13 @@ pub fn curve_type_at(paths: &List, index: usize) -> CurveType { paths.attribute_cloned_or_default(ATTR_CURVE_TYPE, index) } +/// The curve type every path weaves in, in path order, found with one lookup for all of them. +pub(crate) fn curve_types_of(paths: &List) -> impl ExactSizeIterator + '_ { + let stored = paths.iter_attribute_values::(ATTR_CURVE_TYPE).map(|curve_types| curve_types.as_slice()).unwrap_or_default(); + + (0..paths.len()).map(move |index| stored.get(index).copied().unwrap_or_default()) +} + /// Whether the path at `index` only carries on the one stored before it in another curve type, setting out from /// that path's last anchor. Such a path begins nothing of its own. pub fn continues_previous_path(paths: &List, index: usize) -> bool { @@ -194,28 +234,48 @@ pub fn continues_previous_path(paths: &List, index: usize) -> bool { previous_last > 0 && path.0.element(0) == previous.0.element(previous_last) && curve_type_at(paths, index) != curve_type_at(paths, previous_index) } +/// The IDs a weave gives the anchors and spans it adds, each bezpath's continuing on from the latest taken before it. +#[derive(Default)] +struct WovenNames { + last_point: Option, + last_segment: Option, + /// The ID of every woven point so far, with `PointId::UNNAMED` for a handle. + point_ids: Vec, + /// The ID of every span of every finished path so far, in order. + segment_ids: Vec, + /// How many finished paths have had their spans named. + named_paths: usize, +} + /// Weaves bezpath contours into pool points and paths, starting a new path at each verb change so every /// path keeps one stride. The runs of a split subpath share the anchor they meet at, welding them. struct PathWeaver { positions: Vec, anchors: Vec, offset: u32, - woven: Vec<(CurveType, Vec)>, + /// The references of every woven path end to end, the last of them belonging to the path being woven. references: Vec, + /// Each finished path's curve type and where its references end. + woven: Vec<(CurveType, usize)>, + /// Where the references of the path being woven begin. + path_start: usize, curve_type: Option, subpath_start: Option, + names: Option, } impl PathWeaver { - fn new(offset: u32) -> Self { + fn new(offset: u32, names: Option) -> Self { Self { positions: Vec::new(), anchors: Vec::new(), offset, - woven: Vec::new(), references: Vec::new(), + woven: Vec::new(), + path_start: 0, curve_type: None, subpath_start: None, + names, } } @@ -230,14 +290,20 @@ impl PathWeaver { self.positions.get(reference.checked_sub(self.offset)? as usize).copied() } + /// The latest reference of the path being woven. + fn last_reference(&self) -> Option { + self.references.get(self.path_start..)?.last().copied() + } + /// Banks the references woven so far as a finished path, dropping a lone anchor that drew nothing. fn flush(&mut self) { - if self.references.len() >= 2 { - let curve_type = self.curve_type.unwrap_or_default(); - self.woven.push((curve_type, std::mem::take(&mut self.references))); + if self.references.len() - self.path_start >= 2 { + self.woven.push((self.curve_type.unwrap_or_default(), self.references.len())); + } else { + self.references.truncate(self.path_start); } - self.references.clear(); + self.path_start = self.references.len(); self.curve_type = None; } @@ -245,8 +311,8 @@ impl PathWeaver { /// Returns whether an anchor exists to draw from. fn begin_segment(&mut self, verb: CurveType) -> bool { // After a close the pen rests on the subpath's first point, so a segment drawn with no new MoveTo departs from there - let weld = match self.references.last() { - Some(&weld) => weld, + let weld = match self.last_reference() { + Some(weld) => weld, None => { let Some(start) = self.subpath_start else { return false }; self.references.push(start); @@ -294,7 +360,7 @@ impl PathWeaver { /// Closes the subpath by naming its first point again, drawing a straight line home when the pen sits elsewhere. fn close(&mut self) { - let (Some(start), Some(&last)) = (self.subpath_start, self.references.last()) else { return }; + let (Some(start), Some(last)) = (self.subpath_start, self.last_reference()) else { return }; if last == start { self.flush(); @@ -317,35 +383,123 @@ impl PathWeaver { self.flush(); } - /// Appends the woven points and paths, returning whether each appended point is an anchor rather than a handle. - fn finish(mut self, pool: &mut List, paths: &mut List) -> Vec { + /// Weaves a whole bezpath, which leaves nothing open for the next one to carry on from. + fn push_bezpath(&mut self, bezpath: &BezPath) { + let elements = bezpath.elements(); + let point_count = |element: &PathEl| match element { + PathEl::MoveTo(_) | PathEl::LineTo(_) => 1, + PathEl::QuadTo(..) => 2, + PathEl::CurveTo(..) => 3, + PathEl::ClosePath => 0, + }; + let points: usize = elements.iter().map(point_count).sum(); + + // Every point is referenced once, and closes and verb changes add a few more + self.positions.reserve(points); + self.anchors.reserve(points); + self.references.reserve(points + elements.len()); + + for &element in elements { + self.push_element(element); + } self.flush(); + self.subpath_start = None; - pool.extend(List::new_from_elements(self.positions)); + self.name_latest(); + } - for (curve_type, references) in self.woven { - let path = Path::from_references(references); - paths.push(Item::new_from_element(path).with_attribute(ATTR_CURVE_TYPE, curve_type)); + /// Names the anchors and spans woven since the last naming, when this weave names what it adds. + fn name_latest(&mut self) { + let Some(names) = &mut self.names else { return }; + + // Pool order is traversal order, so anchors and free points take IDs in the order they were drawn + let new_anchors = self.anchors.get(names.point_ids.len()..).unwrap_or_default(); + if !new_anchors.is_empty() { + let mut point_id = names.last_point.map_or(PointId::ZERO, |mut latest| latest.next_id()); + let ids = new_anchors.iter().map(|&anchor| if anchor { point_id.next_id() } else { PointId::UNNAMED }); + names.point_ids.extend(ids); + names.last_point = Some(point_id); } - self.anchors + let new_paths = self.woven.get(names.named_paths..).unwrap_or_default(); + let mut path_start = names.named_paths.checked_sub(1).and_then(|previous| self.woven.get(previous)).map_or(0, |&(_, end)| end); + let mut segment_id = names.last_segment.map_or(SegmentId::ZERO, |mut latest| latest.next_id()); + for &(curve_type, path_end) in new_paths { + let span_count = (path_end - path_start).saturating_sub(1) / curve_type.stride(); + if span_count > 0 { + names.segment_ids.extend((0..span_count).map(|_| segment_id.next_id())); + names.last_segment = Some(segment_id); + } + + path_start = path_end; + } + names.named_paths = self.woven.len(); + } + + /// Appends the woven points and paths. + fn finish(self, pool: &mut List, paths: &mut List) { + let (point_ids, segment_ids) = self.names.map(|names| (names.point_ids, names.segment_ids)).unzip(); + + if !self.positions.is_empty() { + let mut points = List::new_from_elements(self.positions); + if let Some(point_ids) = point_ids { + // An attribute of another type under the key would keep the IDs out, so it makes way for them + if pool.iter_attribute_values::(ATTR_ID).is_none() { + pool.remove_attribute(ATTR_ID); + } + points.set_attribute_values(ATTR_ID, point_ids); + } + pool.extend(points); + } + + let mut segment_ids = segment_ids.unwrap_or_default().into_iter(); + let mut path_start = 0; + let woven = self.woven.into_iter().map(|(curve_type, path_end)| { + let references = self.references.get(path_start..path_end).unwrap_or_default(); + path_start = path_end; + + (curve_type, woven_path(curve_type, references.iter().copied(), segment_ids.by_ref())) + }); + let (curve_types, woven) = woven.unzip(); + extend_with_paths(paths, curve_types, woven); } } /// Appends a bezpath's contours to a point pool and its paths. pub fn append_bezpath(pool: &mut List, paths: &mut List, bezpath: &BezPath) { - weave_bezpath(pool, paths, bezpath); + let mut weaver = PathWeaver::new(pool.len() as u32, None); + weaver.push_bezpath(bezpath); + weaver.finish(pool, paths); } -/// Appends a bezpath's contours to a point pool and its paths, returning whether each appended point is an anchor. -pub(crate) fn weave_bezpath(pool: &mut List, paths: &mut List, bezpath: &BezPath) -> Vec { - let mut weaver = PathWeaver::new(pool.len() as u32); +/// Appends the contours of each bezpath in turn to a point pool and its paths, naming the anchors and spans they add, +/// and returns how many spans each bezpath added. Each bezpath's IDs skip one after the largest taken before it, as +/// they do when bezpaths are appended one at a time. +pub(crate) fn append_named_bezpaths( + pool: &mut List, + paths: &mut List, + bezpaths: impl IntoIterator>, + last_point: Option, + last_segment: Option, +) -> Vec { + let names = WovenNames { + last_point, + last_segment, + ..Default::default() + }; + let mut weaver = PathWeaver::new(pool.len() as u32, Some(names)); - for &element in bezpath.elements() { - weaver.push_element(element); + // Every span woven is named, so the names taken so far count the spans each bezpath adds + let named_spans = |weaver: &PathWeaver| weaver.names.as_ref().map_or(0, |names| names.segment_ids.len()); + let mut span_counts = Vec::new(); + for bezpath in bezpaths { + let spans_before = named_spans(&weaver); + weaver.push_bezpath(bezpath.borrow()); + span_counts.push(named_spans(&weaver) - spans_before); } - weaver.finish(pool, paths) + weaver.finish(pool, paths); + span_counts } /// Builds a fresh point pool and its paths from a bezpath. diff --git a/node-graph/libraries/vector-types/src/vector/vector_attributes.rs b/node-graph/libraries/vector-types/src/vector/vector_attributes.rs index 42ecbb7a0d5..2cfca094db9 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_attributes.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_attributes.rs @@ -1,7 +1,7 @@ use crate::vector::addressing::{self, Span}; use crate::vector::contour::contours; use crate::vector::misc::{ManipulatorGroup, Tangent, point_to_dvec2, segment_to_handles}; -use crate::vector::path::{CurveType, Path, append_segment, continues_previous_path, curve_type_at}; +use crate::vector::path::{CurveType, Path, append_segment, continues_previous_path, curve_type_at, curve_types_of}; use crate::vector::vector_types::Vector; use core_types::list::{ATTR_ID, Item, List}; use dyn_any::DynAny; @@ -109,17 +109,15 @@ impl SegmentGraph { point_count: vector.point_count(), }; - for span in addressing::spans(paths) { - let Some(path) = paths.element(span.path as usize) else { continue }; - let Some((start, end)) = addressing::span_endpoints(paths, span) else { continue }; - let Some(segment) = path.segment(&vector.0, curve_type_at(paths, span.path as usize), span.index as usize) else { + for resolved in addressing::resolved_spans(paths) { + let Some(segment) = resolved.path.segment(&vector.0, resolved.curve_type, resolved.span.index as usize) else { continue; }; - graph.spans.push(span); - graph.ids.push(addressing::segment_id_of(paths, span).unwrap_or_default()); - graph.start_point.push(start as usize); - graph.end_point.push(end as usize); + graph.spans.push(resolved.span); + graph.ids.push(resolved.id.unwrap_or_default()); + graph.start_point.push(resolved.endpoints.0 as usize); + graph.end_point.push(resolved.endpoints.1 as usize); graph.segments.push(segment); } @@ -300,90 +298,27 @@ impl Faces { } } -impl SegmentGraph { - /// Computes the direction-of-travel tangent at one endpoint of a segment. - /// Uses the "first distinct control point" pattern: iterates through the Bezier control points - /// from the anchor outward, returning the direction to the first one that differs in position. - /// This handles zero-length handles by finding the tangent direction in the limit. - /// Returns `DVec2::ZERO` if all control points coincide (fully degenerate segment). - fn segment_tangent_at_endpoint(&self, segment_index: usize, at_start: bool) -> DVec2 { - let (points, count) = match self.segments[segment_index] { - PathSeg::Line(line) => ([point_to_dvec2(line.p0), point_to_dvec2(line.p1), DVec2::ZERO, DVec2::ZERO], 2), - PathSeg::Quad(quad) => ([point_to_dvec2(quad.p0), point_to_dvec2(quad.p1), point_to_dvec2(quad.p2), DVec2::ZERO], 3), - PathSeg::Cubic(cubic) => ([point_to_dvec2(cubic.p0), point_to_dvec2(cubic.p1), point_to_dvec2(cubic.p2), point_to_dvec2(cubic.p3)], 4), - }; - - let not_near = |a: DVec2, b: DVec2| a.distance_squared(b) > f64::EPSILON * 1e3; - - if at_start { - let anchor = points[0]; - points[1..count].iter().find(|&&p| not_near(p, anchor)).map_or(DVec2::ZERO, |&point| point - anchor) - } else { - let anchor = points[count - 1]; - points[..count - 1].iter().rev().find(|&&p| not_near(p, anchor)).map_or(DVec2::ZERO, |&point| anchor - point) - } - } - - /// Computes the average tangent direction at a point based on its 1 or 2 connected segments. - /// Returns `None` for points with 0 or 3+ connections (ambiguous or undefined tangent), - /// or if the tangent is degenerate (all control points coincide). - pub fn point_tangent(&self, point_index: usize) -> Option { - // Collect connected segments with their relationship to this point (at_start flag) - let mut connections: [(usize, bool); 2] = [(0, false); 2]; - let mut connection_count = 0; - - for (segment_index, (&start, &end)) in self.start_point.iter().zip(&self.end_point).enumerate() { - // Self-loop segments count as two connections (outgoing and incoming) - let is_start = start == point_index; - let is_end = end == point_index; - - if !is_start && !is_end { - continue; - } - - if is_start { - if connection_count >= 2 { - return None; - } - connections[connection_count] = (segment_index, true); - connection_count += 1; - } - if is_end { - if connection_count >= 2 { - return None; - } - connections[connection_count] = (segment_index, false); - connection_count += 1; - } - } - - if connection_count == 0 { - return None; - } - - // Compute the direction-of-travel tangent for the first connection - let (segment_index, at_start) = connections[0]; - let tangent1 = self.segment_tangent_at_endpoint(segment_index, at_start).try_normalize(); - - if connection_count == 1 { - return tangent1; - } - - // Compute the direction-of-travel tangent for the second connection - let (segment_index, at_start) = connections[1]; - let tangent2 = self.segment_tangent_at_endpoint(segment_index, at_start).try_normalize(); - - // Average the two normalized tangents - let average = tangent1? + tangent2?; +/// The direction of travel at one end of a segment, found from the nearest control point that sits apart from that anchor, +/// which looks past zero-length handles to the limit direction. `DVec2::ZERO` if every control point coincides. +fn tangent_at_endpoint(segment: PathSeg, at_start: bool) -> DVec2 { + let (points, count) = match segment { + PathSeg::Line(line) => ([point_to_dvec2(line.p0), point_to_dvec2(line.p1), DVec2::ZERO, DVec2::ZERO], 2), + PathSeg::Quad(quad) => ([point_to_dvec2(quad.p0), point_to_dvec2(quad.p1), point_to_dvec2(quad.p2), DVec2::ZERO], 3), + PathSeg::Cubic(cubic) => ([point_to_dvec2(cubic.p0), point_to_dvec2(cubic.p1), point_to_dvec2(cubic.p2), point_to_dvec2(cubic.p3)], 4), + }; - // If the tangents are nearly opposite (straight-through), use t1 directly - if average.length_squared() < (f64::EPSILON * 1e3).powi(2) { - return tangent1; - } + let not_near = |a: DVec2, b: DVec2| a.distance_squared(b) > f64::EPSILON * 1e3; - average.try_normalize() + if at_start { + let anchor = points[0]; + points[1..count].iter().find(|&&p| not_near(p, anchor)).map_or(DVec2::ZERO, |&point| point - anchor) + } else { + let anchor = points[count - 1]; + points[..count - 1].iter().rev().find(|&&p| not_near(p, anchor)).map_or(DVec2::ZERO, |&point| anchor - point) } +} +impl SegmentGraph { /// Walks every face orbit of the segment graph and traces each one's path and signed area. fn face_orbits(&self) -> FaceOrbits { let mut adjacency: Vec> = vec![Vec::new(); self.point_count]; @@ -592,8 +527,8 @@ impl Vector { pub fn is_branching(&self, paths: &List) -> bool { // Tally the segment ends at each anchor in one pass over the references, stopping at the first junction let mut spokes = vec![0_u8; self.point_count()]; - for (path_index, path) in paths.iter_element_values().enumerate() { - let stride = curve_type_at(paths, path_index).stride(); + for (path, curve_type) in paths.iter_element_values().zip(curve_types_of(paths)) { + let stride = curve_type.stride(); let references = path.0.iter_element_values().as_slice(); let last = references.len().saturating_sub(1); @@ -620,14 +555,59 @@ impl Vector { SegmentGraph::new(self, paths).construct_faces() } - /// Computes the average tangent direction at a point based on its 1 or 2 connected segments. - pub fn point_tangent(&self, paths: &List, point_index: usize) -> Option { - SegmentGraph::new(self, paths).point_tangent(point_index) + /// The direction of travel through every pool point in O(points + references), following the contour that passes + /// it and averaging the segments on either side. A point has none when no contour passes it, when three or more + /// segments meet at it, or when a segment at it is degenerate. + pub fn anchor_tangents(&self, paths: &List) -> Vec> { + let curve_types: Vec = curve_types_of(paths).collect(); + + // What each point gathers: how many segment ends meet it, their tangents summed, the first of them, and whether any was degenerate + let mut gathered = vec![(0_u8, DVec2::ZERO, DVec2::ZERO, false); self.point_count()]; + for contour in contours(self.point_count(), paths) { + for part in &contour.parts { + let Some(path) = paths.element(part.path as usize) else { continue }; + let Some(&curve_type) = curve_types.get(part.path as usize) else { continue }; + let references = path.0.iter_element_values().as_slice(); + let (stride, count) = (curve_type.stride(), path.segment_count(curve_type)); + + for step in 0..count { + let span = if part.reversed { count - 1 - step } else { step }; + let Some(segment) = path.segment(&self.0, curve_type, span) else { continue }; + let (Some(&start), Some(&end)) = (references.get(span * stride), references.get(span * stride + stride)) else { + continue; + }; + + // The segment as the contour crosses it, leaving one anchor and arriving at the other + let (segment, leaving, arriving) = if part.reversed { (segment.reverse(), end, start) } else { (segment, start, end) }; + for (point, at_start) in [(leaving, true), (arriving, false)] { + let Some((ends, sum, first, degenerate)) = gathered.get_mut(point as usize) else { continue }; + + match tangent_at_endpoint(segment, at_start).try_normalize() { + Some(tangent) if *ends == 0 => (*sum, *first) = (tangent, tangent), + Some(tangent) => *sum += tangent, + None => *degenerate = true, + } + *ends = ends.saturating_add(1); + } + } + } + } + + // Tangents that cancel out mark a cusp doubling straight back, which keeps the first of its two directions + let cancels_out = |sum: DVec2| sum.length_squared() < (f64::EPSILON * 1e3).powi(2); + let resolve = |(ends, sum, first, degenerate): (u8, DVec2, DVec2, bool)| match ends { + _ if degenerate => None, + 1 => Some(first), + 2 if cancels_out(sum) => Some(first), + 2 => sum.try_normalize(), + _ => None, + }; + gathered.into_iter().map(resolve).collect() } /// Every segment's stable ID, in span order. pub fn segment_ids(&self, paths: &List) -> Vec { - addressing::spans(paths).map(|span| addressing::segment_id_of(paths, span).unwrap_or_default()).collect() + addressing::resolved_spans(paths).map(|resolved| resolved.id.unwrap_or_default()).collect() } /// The ID of the first segment of every path that begins one, leaving out the paths that only carry on the one before them in another curve type. @@ -759,7 +739,7 @@ impl Vector { pub fn stroke_manipulator_groups(&self, paths: &List) -> impl Iterator, bool)> { let mut groups = Vec::new(); - let curve_types: Vec = (0..paths.len()).map(|index| curve_type_at(paths, index)).collect(); + let curve_types: Vec = curve_types_of(paths).collect(); let ids = self.0.iter_attribute_values::(ATTR_ID).map(|ids| ids.as_slice()).unwrap_or_default(); let id_at = |point: u32| ids.get(point as usize).copied().unwrap_or_default(); @@ -1094,6 +1074,59 @@ mod tests { assert_eq!(bezpath_from_manipulator_groups(manipulators, *closed).elements(), contours[0].elements()); } + // Where two paths weld tail to tail, the tangent follows the contour through the weld instead of the two stored directions + #[test] + fn anchor_tangents_follow_the_contour_through_a_weld() { + let points = [DVec2::new(0., 0.), DVec2::new(-10., 10.), DVec2::new(0., 20.), DVec2::new(10., 10.)]; + let (vector, mut paths) = build_vector(&points, &[]); + for references in [[0, 1, 2], [0, 3, 2]] { + paths.push(Item::new_from_element(Path::from_references(references)).with_attribute(ATTR_CURVE_TYPE, CurveType::Linear)); + } + + let tangents = vector.anchor_tangents(&paths); + let near = |tangent: Option, expected: DVec2| tangent.is_some_and(|tangent| tangent.distance(expected) < 1e-9); + + // The contour runs down the left half and back up the right half + assert!(near(tangents[2], DVec2::X), "{:?}", tangents[2]); + assert!(near(tangents[0], DVec2::NEG_X), "{:?}", tangents[0]); + assert!(near(tangents[1], DVec2::Y), "{:?}", tangents[1]); + assert!(near(tangents[3], DVec2::NEG_Y), "{:?}", tangents[3]); + } + + // An open end takes its one segment's direction and a closed loop's seam averages the two meeting there. A cusp keeps + // the direction arriving at it, and a junction of three ends or a collapsed segment gives no tangent. + #[test] + fn anchor_tangents_at_ends_seams_cusps_junctions_and_collapsed_segments() { + let tangents = |points: &[DVec2], curve_type: CurveType, paths: &[&[u32]]| { + let (vector, mut list) = build_vector(points, &[]); + for &references in paths { + list.push(Item::new_from_element(Path::from_references(references.iter().copied())).with_attribute(ATTR_CURVE_TYPE, curve_type)); + } + vector.anchor_tangents(&list) + }; + let near = |tangent: Option, expected: DVec2| tangent.is_some_and(|tangent| tangent.distance(expected.normalize()) < 1e-9); + + let open = tangents(&[DVec2::ZERO, DVec2::new(10., 0.), DVec2::new(10., 10.)], CurveType::Linear, &[&[0, 1, 2]]); + assert_eq!(open[0], Some(DVec2::X)); + assert!(near(open[1], DVec2::ONE), "{:?}", open[1]); + assert_eq!(open[2], Some(DVec2::Y)); + + let square = [DVec2::ZERO, DVec2::new(10., 0.), DVec2::new(10., 10.), DVec2::new(0., 10.)]; + let closed = tangents(&square, CurveType::Linear, &[&[0, 1, 2, 3, 0]]); + assert!(near(closed[0], DVec2::new(1., -1.)), "{:?}", closed[0]); + + let cusp = tangents(&[DVec2::ZERO, DVec2::new(10., 0.), DVec2::ZERO], CurveType::Linear, &[&[0, 1, 2]]); + assert_eq!(cusp[1], Some(DVec2::X)); + + let junction = tangents(&square, CurveType::Linear, &[&[0, 1], &[0, 2], &[0, 3]]); + assert_eq!(junction[0], None); + assert_eq!(junction[1], Some(DVec2::X)); + + let collapsed = tangents(&[DVec2::ONE; 4], CurveType::Cubic, &[&[0, 1, 2, 3]]); + assert_eq!(collapsed[0], None); + assert_eq!(collapsed[3], None); + } + /// Square wound positively with a reverse-wound triangle welded to its corner: /// the triangle's interior face is deliberate negative space, so only the surrounding face is filled. #[test] diff --git a/node-graph/libraries/vector-types/src/vector/vector_modification.rs b/node-graph/libraries/vector-types/src/vector/vector_modification.rs index 3a483e96521..3b37667e406 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_modification.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_modification.rs @@ -1,7 +1,7 @@ use super::*; use crate::vector::flatten::WorkingVector; use crate::vector::misc::{BezierHandles, HandleId, HandleType}; -use crate::vector::path::{CurveType, Path, curve_type_at}; +use crate::vector::path::{CurveType, Path, curve_types_of}; use core_types::list::{ATTR_ID, List}; use core_types::uuid::generate_uuid; use dyn_any::DynAny; @@ -136,8 +136,7 @@ impl PointModification { } // A segment's first handle follows its start anchor, and a cubic's second handle follows its end anchor - for (path_index, path) in paths.iter_element_values().enumerate() { - let curve_type = curve_type_at(paths, path_index); + for (path, curve_type) in paths.iter_element_values().zip(curve_types_of(paths)) { if curve_type == CurveType::Linear { continue; } @@ -214,8 +213,8 @@ impl SegmentModification { working.segment_begins_path.resize(segment_count, false); let (mut removed_began_path, mut removed_end) = (false, None); - for index in 0..segment_count { - if !keep[index] { + for (index, &kept) in keep.iter().enumerate() { + if !kept { removed_began_path |= std::mem::take(&mut working.segment_begins_path[index]); removed_end = Some(working.segment_end[index]); continue; @@ -389,8 +388,7 @@ impl SegmentModification { /// Calls the visitor with each span a handle edit names, along with its curve type, references, starting reference, and edits. fn for_each_edited_span(&self, paths: &List, mut visit: impl FnMut(CurveType, &Path, usize, Option>, Option>)) { - for (path_index, path) in paths.iter_element_values().enumerate() { - let curve_type = curve_type_at(paths, path_index); + for (path, curve_type) in paths.iter_element_values().zip(curve_types_of(paths)) { let stride = curve_type.stride(); let ids = path.0.iter_attribute_values::(ATTR_ID).map(|ids| ids.as_slice()).unwrap_or_default(); @@ -771,9 +769,17 @@ impl VectorModification { /// Apply this modification to the specified [`Vector`] and its paths. pub fn apply(&self, vector: &mut Vector, paths: &mut List) { + if !self.apply_in_place(vector, paths) { + self.apply_through_working_form(vector, paths); + } + } + + /// Applies this modification if it leaves the paths as they are, returning whether it did. Otherwise nothing + /// changes and [`Self::apply`] has to rewrite the paths. + pub fn apply_in_place(&self, vector: &mut Vector, paths: &List) -> bool { // An untouched vector keeps its incoming path structure instead of being flattened and rewoven if self.is_empty() { - return; + return true; } // Moving points and handles leaves the weave intact, so the pool is edited where it lies in O(points + references), keeping @@ -783,11 +789,11 @@ impl VectorModification { if handles_unshared { self.points.move_in_place(vector, paths, &handles); self.segments.set_handles_in_place(vector, paths); - return; + return true; } } - self.apply_through_working_form(vector, paths); + false } /// Applies this modification by flattening to the legacy working form, editing that, and weaving the result back. diff --git a/node-graph/libraries/vector-types/src/vector/vector_types.rs b/node-graph/libraries/vector-types/src/vector/vector_types.rs index 64dacb7f5b1..65c509f057c 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_types.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_types.rs @@ -2,7 +2,9 @@ use super::addressing; use super::connectivity::Connectivity; use super::contour::{contour_bezpaths, contours}; use super::misc::{HandleId, ManipulatorPointId, dvec2_to_point}; -use super::path::{CurveType, Path, curve_type_at, no_paths, paths_from_item_attributes, paths_from_list_row_attributes, push_path, weave_bezpath}; +use super::path::{ + CurveType, Path, append_named_bezpaths, curve_type_at, curve_types_of, no_paths, paths_from_item_attributes, paths_from_list_row_attributes, push_path, shared_paths_from_item_attributes, +}; use super::style::{Stroke, StrokeAlign, StrokeCap, StrokeJoin}; use super::vector_attributes::{PointId, SegmentId}; use core_types::bounds::{BoundingBox, RenderBoundingBox}; @@ -84,58 +86,38 @@ impl Vector { } } -/// The ID continuing the pool's point ID sequence. -fn next_point_id(pool: &List) -> PointId { - let ids = pool.iter_attribute_values::(ATTR_ID); - ids.and_then(|ids| ids.copied().filter(|id| *id != PointId::UNNAMED).max()) - .map(|mut id| id.next_id()) - .unwrap_or(PointId::ZERO) +/// The largest ID in the pool's point ID sequence, which a pool has once it holds point IDs at all. +fn largest_point_id(pool: &List) -> Option { + let ids = pool.iter_attribute_values::(ATTR_ID)?; + + // An ID attribute that names nothing yet counts as a sequence at zero, which documents saved with edits to such geometry rely on + Some(ids.copied().filter(|id| *id != PointId::UNNAMED).max().unwrap_or_default()) } -/// The ID continuing the paths' segment ID sequence. -fn next_segment_id(paths: &List) -> SegmentId { - let ids = paths - .iter_element_values() - .filter_map(|path| path.0.iter_attribute_values::(ATTR_ID)) - .flatten() - .copied() - .max(); +/// The largest ID in the paths' segment ID sequence. +fn largest_segment_id(paths: &List) -> Option { + let ids = paths.iter_element_values().filter_map(|path| path.0.iter_attribute_values::(ATTR_ID)); - ids.map(|mut id| id.next_id()).unwrap_or(SegmentId::ZERO) + ids.flatten().copied().max() } impl Vector { /// Weaves a bezpath's contours onto this pool and the given paths, naming the new anchors and spans /// by continuing the ID sequences. Handles take `PointId::UNNAMED`. pub fn append_bezpath(&mut self, paths: &mut List, bezpath: &BezPath) { - let (first_new_point, first_new_path) = (self.0.len(), paths.len()); - let anchors = weave_bezpath(&mut self.0, paths, bezpath); - - // Pool order is traversal order, so anchors and free points take IDs in the order they were drawn - if !anchors.is_empty() { - let mut point_id = next_point_id(&self.0); - let ids = self.0.iter_attribute_values_mut_or_default::(ATTR_ID).skip(first_new_point); - for (id, anchor) in ids.zip(anchors) { - *id = if anchor { point_id.next_id() } else { PointId::UNNAMED }; - } - } - - let mut segment_id = next_segment_id(paths); - for path_index in first_new_path..paths.len() { - let curve_type = curve_type_at(paths, path_index); - let Some(path) = paths.element_mut(path_index) else { continue }; + self.append_bezpaths(paths, [bezpath]); + } - let span_count = path.segment_count(curve_type); - if span_count == 0 { - continue; - } + /// Weaves each bezpath in turn as [`Self::append_bezpath`] would, in O(existing + appended) however many there are + /// since the IDs to continue from are found once. + pub fn append_bezpaths(&mut self, paths: &mut List, bezpaths: impl IntoIterator>) { + self.append_bezpaths_counting_spans(paths, bezpaths); + } - let mut span_ids = vec![SegmentId::default(); path.0.len()]; - for span in 0..span_count { - span_ids[span * curve_type.stride()] = segment_id.next_id(); - } - path.0.set_attribute_values(ATTR_ID, span_ids); - } + /// Weaves bezpaths as [`Self::append_bezpaths`] does, returning how many spans each of them added. + pub fn append_bezpaths_counting_spans(&mut self, paths: &mut List, bezpaths: impl IntoIterator>) -> Vec { + let (last_point, last_segment) = (largest_point_id(&self.0), largest_segment_id(paths)); + append_named_bezpaths(&mut self.0, paths, bezpaths, last_point, last_segment) } /// Builds a fresh pool and paths from a single bezpath. @@ -157,13 +139,14 @@ impl Vector { impl Vector { /// Every segment in span order as `(id, segment, start point ID, end point ID)`. pub fn segment_iter<'a>(&'a self, paths: &'a List) -> impl Iterator + 'a { - addressing::spans(paths).filter_map(|span| { - let path = paths.element(span.path as usize)?; - let segment = path.segment(&self.0, curve_type_at(paths, span.path as usize), span.index as usize)?; - let id = addressing::segment_id_of(paths, span).unwrap_or_default(); - let (start, end) = addressing::span_endpoints(paths, span)?; + let point_ids = self.0.iter_attribute_values::(ATTR_ID).map(|ids| ids.as_slice()).unwrap_or_default(); + let point_id = move |point: u32| point_ids.get(point as usize).copied().unwrap_or_default(); - Some((id, segment, self.point_id_at(start).unwrap_or_default(), self.point_id_at(end).unwrap_or_default())) + addressing::resolved_spans(paths).filter_map(move |resolved| { + let segment = resolved.path.segment(&self.0, resolved.curve_type, resolved.span.index as usize)?; + let (start, end) = resolved.endpoints; + + Some((resolved.id.unwrap_or_default(), segment, point_id(start), point_id(end))) }) } @@ -379,8 +362,8 @@ impl Vector { let mut roles = vec![UNREFERENCED; self.0.len()]; let mut handles_unshared = true; - for (path_index, path) in paths.iter_element_values().enumerate() { - let stride = curve_type_at(paths, path_index).stride(); + for (path, curve_type) in paths.iter_element_values().zip(curve_types_of(paths)) { + let stride = curve_type.stride(); for (reference_index, point) in path.references().enumerate() { let Some(role) = roles.get_mut(point as usize) else { continue }; if reference_index.is_multiple_of(stride) { @@ -697,12 +680,12 @@ impl Vector { return; } - // Each closing segment's ID skips one after the previous, as the IDs of a newly appended bezpath do - let mut base = next_segment_id(paths); + // Each closing segment's ID skips one after the latest, as the IDs of a newly appended bezpath do + let mut latest = largest_segment_id(paths); let mut closing_paths = Vec::with_capacity(closings.len()); for (before, backward, references) in closings { - let id = base.next_id(); - base.next_id(); + let id = latest.map_or(SegmentId::ZERO, |mut latest| latest.next_id()).next_id(); + latest = Some(id); let mut single = List::new(); push_path(&mut single, CurveType::Linear, references, [id]); @@ -795,15 +778,32 @@ impl Vector { } } -/// Edits a holder item's pool and paths together, taking the paths attribute out and restoring it. +/// Edits a holder item's pool and paths together, taking the paths attribute out and putting it back where it was, or last if absent. pub fn edit_item_geometry(item: &mut Item, edit: impl FnOnce(&mut Vector, &mut List) -> R) -> R { - let mut paths = item.remove_attribute::>(ATTR_PATHS).unwrap_or_default(); + let mut paths = item.attribute_mut::>(ATTR_PATHS).map(std::mem::take).unwrap_or_default(); let result = edit(item.element_mut(), &mut paths); - item.set_attribute(ATTR_PATHS, paths); + *item.attribute_mut_or_insert_default::>(ATTR_PATHS) = paths; + + result +} + +/// Edits an item's pool while only reading its paths, which stay shared with the item's clones instead of being copied. +pub fn edit_item_points(item: &mut Item, edit: impl FnOnce(&mut Vector, &List) -> R) -> R { + let paths = shared_paths_from_item_attributes(item); + let result = edit(item.element_mut(), &paths); + ensure_paths_attribute(item); result } +/// Gives an item without paths an empty paths attribute, placed last, as [`edit_item_geometry`] gives a point cloud. +pub fn ensure_paths_attribute(item: &mut Item) { + if item.attribute::>(ATTR_PATHS).is_none() { + item.remove_attribute::>(ATTR_PATHS); + item.set_attribute(ATTR_PATHS, List::::new()); + } +} + /// Edits one list row's pool and paths together, writing the paths attribute back afterwards. pub fn edit_list_row_geometry(list: &mut List, index: usize, edit: impl FnOnce(&mut Vector, &mut List) -> R) -> Option { let mut paths = list.attribute::>(ATTR_PATHS, index).cloned().unwrap_or_default(); @@ -863,7 +863,7 @@ impl RenderComplexity for Vector { fn segment_count(paths: Option<&List>) -> usize { let Some(paths) = paths else { return 0 }; - paths.iter_element_values().enumerate().map(|(index, path)| path.segment_count(curve_type_at(paths, index))).sum() + paths.iter_element_values().zip(curve_types_of(paths)).map(|(path, curve_type)| path.segment_count(curve_type)).sum() } // Note: BoundingBox for List is handled by blanket impl in core-types @@ -896,6 +896,54 @@ mod tests { assert_eq!(List::new_from_item(item).render_complexity(), 4); } + // Bezpaths appended together come out as they do appended one at a time, IDs included, with the spans each one added counted + #[test] + fn bezpaths_appended_together_match_one_at_a_time() { + let mut mixed = BezPath::new(); + mixed.move_to((0., 0.)); + mixed.line_to((4., 0.)); + mixed.quad_to((6., 2.), (4., 4.)); + mixed.close_path(); + let bezpaths = [ellipse_bezpath(DVec2::NEG_ONE, DVec2::ONE), BezPath::new(), mixed, open_curve_bezpath()]; + + let (mut together, mut together_paths) = (lone_anchor(DVec2::splat(9.)), List::new()); + let span_counts = together.append_bezpaths_counting_spans(&mut together_paths, &bezpaths); + assert_eq!(span_counts, [4, 0, 3, 1]); + + let (mut in_turn, mut in_turn_paths) = (lone_anchor(DVec2::splat(9.)), List::new()); + for bezpath in &bezpaths { + in_turn.append_bezpath(&mut in_turn_paths, bezpath); + } + + assert_eq!((together, together_paths), (in_turn, in_turn_paths)); + } + + // Each appended bezpath's IDs skip one after the largest before it, which documents saved with edits to such geometry rely on + #[test] + fn appended_bezpaths_continue_the_id_sequences_with_a_gap() { + let mut triangle = BezPath::new(); + triangle.move_to((0., 0.)); + triangle.line_to((4., 0.)); + triangle.line_to((0., 4.)); + triangle.close_path(); + let named = |vector: &Vector, paths: &List| { + let point_ids: Vec = vector.0.iter_attribute_values::(ATTR_ID).into_iter().flatten().map(|id| id.inner()).collect(); + let segment_ids: Vec = vector.segment_iter(paths).map(|(id, ..)| id.inner()).collect(); + (point_ids, segment_ids) + }; + + let (mut vector, mut paths) = (Vector::default(), List::new()); + vector.append_bezpaths(&mut paths, [&triangle, &triangle]); + assert_eq!(named(&vector, &paths), (vec![1, 2, 3, 5, 6, 7], vec![1, 2, 3, 5, 6, 7])); + + // A pool holding IDs that name nothing yet starts its sequence one further on + let mut unnamed = Vector::default(); + unnamed.0.set_attribute_values(ATTR_ID, Vec::::new()); + let mut unnamed_paths = List::new(); + unnamed.append_bezpath(&mut unnamed_paths, &triangle); + assert_eq!(named(&unnamed, &unnamed_paths).0, [2, 3, 4]); + } + // A lone anchor is named zero and only the later collisions are renamed, so stored edits keyed on zero still land #[test] fn lone_anchor_is_named_zero_and_collisions_rename_from_it() { @@ -923,7 +971,7 @@ mod tests { let index = vector.index_of_point_id(PointId::ZERO).unwrap(); assert_eq!(vector.position(index as usize), Some(DVec2::new(5., 5.))); assert!(vector.anchor_indices(&paths).contains(&(index as usize))); - assert_eq!(next_point_id(&vector.0).inner(), 3); + assert_eq!(largest_point_id(&vector.0).map(PointId::inner), Some(2)); } #[test] diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index 70c4216ddff..3af6a11db5b 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -175,9 +175,8 @@ fn boolean_operation_on_vector_list(vector: &List, boolean_operation: Bo }; let contours = top.contours(|winding| winding.is_inside(boolean_operation)); let mut result_paths = List::new(); - for contour in contours.contours() { - row.element_mut().append_bezpath(&mut result_paths, &closed(contour.path.clone())); - } + let closed_contours = contours.contours().map(|contour| closed(contour.path.clone())); + row.element_mut().append_bezpaths(&mut result_paths, closed_contours); row.set_attribute(ATTR_PATHS, result_paths); list.push(row); diff --git a/node-graph/nodes/text/src/path_builder.rs b/node-graph/nodes/text/src/path_builder.rs index cd0720d6039..1a9ba175b3c 100644 --- a/node-graph/nodes/text/src/path_builder.rs +++ b/node-graph/nodes/text/src/path_builder.rs @@ -17,9 +17,8 @@ pub struct PathBuilder { pub vector_list: List, /// Whether each glyph becomes its own list item rather than merging into one vector. per_glyph_items: bool, - /// The single accumulated vector and its paths in merged mode, written to item 0 in `finalize()`. - merged_vector: Vector, - merged_paths: List, + /// Every glyph's outline in merged mode, woven into item 0 together in `finalize()`. + merged_glyphs: Vec, /// Per-glyph AABBs collected in single-item mode, published as `ATTR_EDITOR_CLICK_TARGET` in `finalize()`. merged_click_target_bboxes: Vec<[DVec2; 2]>, /// Per-glyph baselines, parallel to `merged_click_target_bboxes`. Groups glyphs by line for the widening pass. @@ -40,8 +39,7 @@ impl PathBuilder { glyph_bezpath: BezPath::new(), vector_list: if per_glyph_items { List::new() } else { List::new_from_element(Vector::default()) }, per_glyph_items, - merged_vector: Vector::default(), - merged_paths: List::new(), + merged_glyphs: Vec::new(), merged_click_target_bboxes: Vec::new(), merged_click_target_baselines: Vec::new(), per_glyph_bboxes: Vec::new(), @@ -94,7 +92,7 @@ impl PathBuilder { // Defer click target creation to `finalize()` where adjacent AABBs get widened self.per_glyph_bboxes.push(glyph_bbox); } else { - self.merged_vector.append_bezpath(&mut self.merged_paths, &core::mem::take(&mut self.glyph_bezpath)); + self.merged_glyphs.push(core::mem::take(&mut self.glyph_bezpath)); if let Some(bbox) = glyph_bbox { self.merged_click_target_bboxes.push(bbox); @@ -164,12 +162,14 @@ impl PathBuilder { } pub fn finalize(mut self) -> List { - // Merged mode accumulated into side fields, which item 0 takes over here + // Merged mode gathered every glyph's outline, which item 0 takes on here in one pass if !self.per_glyph_items && let Some(element) = self.vector_list.element_mut(0) { - *element = core::mem::take(&mut self.merged_vector); - let paths = core::mem::take(&mut self.merged_paths); + let (mut vector, mut paths) = (Vector::default(), List::::new()); + vector.append_bezpaths(&mut paths, &self.merged_glyphs); + + *element = vector; self.vector_list.set_attribute(core_types::list::ATTR_PATHS, 0, paths); } diff --git a/node-graph/nodes/vector/src/generator_nodes.rs b/node-graph/nodes/vector/src/generator_nodes.rs index e0860b03faa..4d03f9ab88b 100644 --- a/node-graph/nodes/vector/src/generator_nodes.rs +++ b/node-graph/nodes/vector/src/generator_nodes.rs @@ -250,15 +250,13 @@ fn qr_code( true => { let (mut vector, mut paths) = (Vector::default(), List::new()); - let dimension = qr_code.size() as usize; - for y in 0..dimension { - for x in 0..dimension { - if qr_code.get_module(x as i32, y as i32) { - let corner1 = DVec2::new(x as f64, y as f64); - vector.append_bezpath(&mut paths, &shapes::rectangle_bezpath(corner1, corner1 + DVec2::splat(1.))); - } - } - } + let dimension = qr_code.size(); + let cells = (0..dimension).flat_map(|y| (0..dimension).map(move |x| (x, y))); + let squares = cells.filter(|&(x, y)| qr_code.get_module(x, y)).map(|(x, y)| { + let corner1 = DVec2::new(x as f64, y as f64); + shapes::rectangle_bezpath(corner1, corner1 + DVec2::splat(1.)) + }); + vector.append_bezpaths(&mut paths, squares); (vector, paths) } @@ -432,7 +430,7 @@ mod tests { use super::*; use kurbo::ParamCurve; use vector_types::vector::misc::point_to_dvec2; - use vector_types::vector::paths_from_item_attributes; + use vector_types::vector::shared_paths_from_item_attributes; fn item(value: T) -> Item { Item::new_from_element(value) @@ -446,7 +444,7 @@ mod tests { // Works properly let grid = grid((), (), item(GridType::Isometric), item(10.), item(5_i64), item(5_i64), item((30., 30.).into()), item(true)); - let paths = paths_from_item_attributes(&grid).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&grid); assert_eq!(grid.element().anchor_points(&paths).count(), 5 * 5); assert_eq!(grid.element().segment_iter(&paths).count(), 4 * 5 + 4 * 9); for (_, segment, _, _) in grid.element().segment_iter(&paths) { @@ -459,7 +457,7 @@ mod tests { #[test] fn skew_isometric_grid_test() { let grid = grid((), (), item(GridType::Isometric), item(10.), item(5_i64), item(5_i64), item((40., 30.).into()), item(true)); - let paths = paths_from_item_attributes(&grid).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&grid); assert_eq!(grid.element().anchor_points(&paths).count(), 5 * 5); assert_eq!(grid.element().segment_iter(&paths).count(), 4 * 5 + 4 * 9); for (_, segment, _, _) in grid.element().segment_iter(&paths) { @@ -474,7 +472,7 @@ mod tests { #[test] fn grid_joins_segments_only_where_they_follow_each_other() { let grid = grid((), (), item(GridType::Rectangular), item(10.), item(3_i64), item(3_i64), item((30., 30.).into()), item(true)); - let paths = paths_from_item_attributes(&grid).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&grid); let vector = grid.element(); assert_eq!(vector.segment_ids(&paths).len(), 2 * 3 + 2 * 3); @@ -486,7 +484,7 @@ mod tests { fn grid_disconnected_cells_test() { // A 3x3 rectangular grid has a 2x2 arrangement of cells, each its own closed quad subpath. let grid = grid((), (), item(GridType::Rectangular), item(10.), item(3_i64), item(3_i64), item((30., 30.).into()), item(false)); - let paths = paths_from_item_attributes(&grid).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&grid); let vector = grid.element(); assert_eq!(vector.stroke_manipulator_groups(&paths).filter(|(_, closed)| *closed).count(), 4); assert_eq!(vector.anchor_points(&paths).count(), 4 * 4); @@ -512,7 +510,7 @@ mod tests { item(QRCodeErrorCorrectionLevel::Low), item(true), ); - let paths = paths_from_item_attributes(&qr).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&qr); assert!(qr.element().anchor_points(&paths).count() > 0); assert!(!qr.element().segment_ids(&paths).is_empty()); } diff --git a/node-graph/nodes/vector/src/merge_qr_squares.rs b/node-graph/nodes/vector/src/merge_qr_squares.rs index ee02a93426d..65f5a701175 100644 --- a/node-graph/nodes/vector/src/merge_qr_squares.rs +++ b/node-graph/nodes/vector/src/merge_qr_squares.rs @@ -6,8 +6,7 @@ use vector_types::vector::Path; use vector_types::vector::algorithms::shapes; pub fn merge_qr_squares(qr_code: &qrcodegen::QrCode) -> (Vector, List) { - let mut vector = Vector::default(); - let mut paths = List::new(); + let mut outlines = Vec::new(); let size = qr_code.size() as usize; @@ -109,7 +108,7 @@ pub fn merge_qr_squares(qr_code: &qrcodegen::QrCode) -> (Vector, List) { } if !simplified.is_empty() { - vector.append_bezpath(&mut paths, &shapes::polyline_bezpath(simplified, true)); + outlines.push(shapes::polyline_bezpath(simplified, true)); } } } @@ -123,5 +122,8 @@ pub fn merge_qr_squares(qr_code: &qrcodegen::QrCode) -> (Vector, List) { } } + let (mut vector, mut paths) = (Vector::default(), List::new()); + vector.append_bezpaths(&mut paths, &outlines); + (vector, paths) } diff --git a/node-graph/nodes/vector/src/vector_modification_nodes.rs b/node-graph/nodes/vector/src/vector_modification_nodes.rs index f902f9fec8a..c25257d99f9 100644 --- a/node-graph/nodes/vector/src/vector_modification_nodes.rs +++ b/node-graph/nodes/vector/src/vector_modification_nodes.rs @@ -5,22 +5,35 @@ use core_types::{ATTR_EDITOR_CLICK_TARGET, ATTR_EDITOR_LAYER_PATH, ATTR_TRANSFOR use glam::{DAffine2, DVec2}; use graphic_types::Vector; use vector_types::kurbo::BezPath; -use vector_types::vector::{VectorModification, edit_item_geometry}; +use vector_types::vector::{VectorModification, edit_item_geometry, ensure_paths_attribute, shared_paths_from_item_attributes}; /// Applies a differential modification to a vector path, associating changes made by the Pen and Path tools to indices of edited points and segments. #[node_macro::node(category(""))] async fn path_modify(_ctx: impl Ctx, vector: Item, modification: Item>, node_path: Item) -> Item { let mut vector = vector; let modification = modification.into_element(); - edit_item_geometry(&mut vector, |element, paths| { - modification.apply(element, paths); - // Faces tell negative space apart by a loop's winding, which users draw in arbitrary directions. - // Every other fill follows the contours, which keep the direction each path was drawn in. - if element.use_face_fill(paths) { - element.normalize_winding_directions(paths); - } - }); + // Moving points leaves the paths as they are, so they stay shared with the incoming geometry instead of being copied + let paths = shared_paths_from_item_attributes(&vector); + let moved_in_place = modification.apply_in_place(vector.element_mut(), &paths); + let fills_by_face = moved_in_place && vector.element().use_face_fill(&paths); + drop(paths); + + if moved_in_place && !fills_by_face { + ensure_paths_attribute(&mut vector); + } else { + edit_item_geometry(&mut vector, |element, paths| { + if !moved_in_place { + modification.apply(element, paths); + } + + // Faces tell negative space apart by a loop's winding, which users draw in arbitrary directions. + // Every other fill follows the contours, which keep the direction each path was drawn in. + if fills_by_face || (!moved_in_place && element.use_face_fill(paths)) { + element.normalize_winding_directions(paths); + } + }); + } // Drop the stale click-target override so hit testing uses the geometry the user is now editing vector.remove_attribute::(ATTR_EDITOR_CLICK_TARGET); @@ -53,7 +66,8 @@ async fn bake_transform(_ctx: impl Ctx, #[cfg(test)] mod tests { use super::*; - use vector_types::vector::paths_from_item_attributes; + use core_types::list::{ATTR_ID, ATTR_PATHS}; + use vector_types::vector::{CurveType, PointId, SegmentId, VectorModificationType, paths_from_item_attributes, push_path}; // A lone loop fills by its contour alone, so it keeps the direction it was drawn in even when that winds backwards #[tokio::test] @@ -70,4 +84,71 @@ mod tests { assert_eq!(paths_from_item_attributes(&modified), paths_from_item_attributes(&drawn)); } + + // Moving a point of a mesh leaves its paths in place to begin with, and its loops still get their winding set for the face fill + #[tokio::test] + async fn a_mesh_moved_in_place_still_has_its_winding_set() { + // A reverse-wound square beside three spokes meeting at a hub, which makes the geometry branch + let positions = [(0., 0.), (0., 10.), (10., 10.), (10., 0.), (30., 0.), (40., 0.), (30., 10.), (20., 10.)]; + let (mut vector, mut paths) = (Vector::default(), List::new()); + let (mut point_id, mut segment_id) = (PointId::ZERO, SegmentId::ZERO); + for (x, y) in positions { + vector.0.push(Item::new_from_element(DVec2::new(x, y)).with_attribute(ATTR_ID, point_id.next_id())); + } + for references in [vec![0, 1, 2, 3, 0], vec![4, 5], vec![4, 6], vec![4, 7]] { + let ids: Vec = (1..references.len()).map(|_| segment_id.next_id()).collect(); + push_path(&mut paths, CurveType::Linear, references, ids); + } + assert!(vector.use_face_fill(&paths)); + let hub = vector.point_id_at(4).unwrap(); + let drawn = Item::new_from_element(vector).with_attribute(ATTR_PATHS, paths); + + let mut modification = VectorModification::default(); + modification.modify(&VectorModificationType::ApplyPointDelta { point: hub, delta: DVec2::X }); + let modified = path_modify((), drawn.clone(), Item::new_from_element(Box::new(modification)), Item::default()).await; + + let first_path = |item: &Item| paths_from_item_attributes(item).unwrap().element(0).unwrap().0.iter_element_values().copied().collect::>(); + assert_eq!(first_path(&drawn), [0, 1, 2, 3, 0]); + assert_eq!(first_path(&modified), [0, 3, 2, 1, 0]); + } + + // Moving points leaves the paths shared with the incoming geometry, and where they sat among its attributes + #[tokio::test] + async fn moving_points_leaves_the_paths_shared_and_in_place() { + let mut bezpath = BezPath::new(); + bezpath.move_to((0., 0.)); + bezpath.line_to((10., 0.)); + bezpath.line_to((10., 10.)); + let drawn = Vector::item_from_bezpath(&bezpath).with_attribute(ATTR_TRANSFORM, DAffine2::IDENTITY); + let corner = drawn.element().point_id_at(1).unwrap(); + + let mut modification = VectorModification::default(); + modification.modify(&VectorModificationType::ApplyPointDelta { point: corner, delta: DVec2::X }); + let modified = path_modify((), drawn.clone(), Item::new_from_element(Box::new(modification)), Item::default()).await; + + assert_eq!(modified.element().0.element(1), Some(&DVec2::new(11., 0.))); + assert!(std::ptr::eq(paths_from_item_attributes(&drawn).unwrap(), paths_from_item_attributes(&modified).unwrap())); + assert_eq!(modified.attributes().keys().collect::>(), [ATTR_PATHS, ATTR_TRANSFORM, ATTR_EDITOR_LAYER_PATH]); + } + + // A point cloud comes out with an empty paths attribute placed last, whether the edit is made in place or by reweaving + #[tokio::test] + async fn a_point_cloud_gains_an_empty_paths_attribute() { + let mut cloud = Vector::default(); + cloud.0.push(Item::new_from_element(DVec2::ZERO).with_attribute(ATTR_ID, PointId::ZERO)); + let drawn = Item::new_from_element(cloud).with_attribute(ATTR_TRANSFORM, DAffine2::IDENTITY); + + let mut adds_a_point = VectorModification::default(); + adds_a_point.modify(&VectorModificationType::InsertPoint { + id: PointId::generate(), + position: DVec2::ONE, + }); + + for modification in [VectorModification::default(), adds_a_point] { + let modified = path_modify((), drawn.clone(), Item::new_from_element(Box::new(modification)), Item::default()).await; + + assert_eq!(paths_from_item_attributes(&modified).map(List::len), Some(0)); + assert_eq!(modified.attributes().keys().collect::>(), [ATTR_TRANSFORM, ATTR_PATHS, ATTR_EDITOR_LAYER_PATH]); + } + } } diff --git a/node-graph/nodes/vector/src/vector_nodes.rs b/node-graph/nodes/vector/src/vector_nodes.rs index 033a63a51db..4c50ed81a1d 100644 --- a/node-graph/nodes/vector/src/vector_nodes.rs +++ b/node-graph/nodes/vector/src/vector_nodes.rs @@ -34,8 +34,8 @@ use vector_types::vector::misc::{ }; use vector_types::vector::style::{DashPattern, Gradient, GradientSettings, Stroke, StrokeAlign, StrokeCap, StrokeJoin}; use vector_types::vector::{ - CurveType, Path, PointId, SegmentId, WorkingVector, curve_type_at, edit_item_geometry, handle_point, no_paths, paths_from_item_attributes, paths_from_list_row_attributes, push_path, - span_endpoints, spans, + CurveType, Path, PointId, SegmentId, WorkingVector, curve_type_at, edit_item_geometry, edit_item_points, handle_point, no_paths, paths_from_item_attributes, paths_from_list_row_attributes, + push_path, shared_paths_from_item_attributes, span_endpoints, spans, }; /// Appends one point to the working form, returning the index that names it. @@ -129,7 +129,7 @@ trait VectorItemMut { impl VectorItemMut for Item { fn for_each_vector_mut(&mut self, mut f: impl FnMut(&mut Vector, &List, DAffine2)) { - let paths = paths_from_item_attributes(self).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(self); let transform = self.attribute_cloned_or_default::(ATTR_TRANSFORM); f(self.element_mut(), &paths, transform); } @@ -139,7 +139,7 @@ impl VectorItemMut for Item { fn for_each_vector_mut(&mut self, mut f: impl FnMut(&mut Vector, &List, DAffine2)) { match self.element_mut() { Graphic::Vector(item) => { - let paths = paths_from_item_attributes(item).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(item); let transform = item.attribute_cloned_or_default::(ATTR_TRANSFORM); f(item.element_mut(), &paths, transform); } @@ -571,7 +571,7 @@ async fn copy_to_points( let do_rotation = random_rotation.abs() > 1e-6; let points_transform: DAffine2 = row.attribute_cloned_or_default(ATTR_TRANSFORM); - let row_paths = paths_from_item_attributes(&row).cloned().unwrap_or_default(); + let row_paths = shared_paths_from_item_attributes(&row); for point in row.element().anchor_positions(&row_paths) { let translation = points_transform.transform_point2(point); @@ -638,7 +638,7 @@ async fn round_corners( let (radius, roundness, edge_length_limit, min_angle_threshold) = (*radius.element(), *roundness.element(), *edge_length_limit.element(), *min_angle_threshold.element()); let source_transform: DAffine2 = source.attribute_cloned_or_default(ATTR_TRANSFORM); let source_transform_inverse = source_transform.inverse(); - let source_paths = paths_from_item_attributes(&source).cloned().unwrap_or_default(); + let source_paths = shared_paths_from_item_attributes(&source); let (source, mut attributes) = source.into_parts(); // Flip the roundness to help with user intuition @@ -646,7 +646,7 @@ async fn round_corners( // Convert 0-100 to 0-0.5 let edge_length_limit = edge_length_limit * 0.005; - let (mut result, mut result_paths) = (Vector::default(), List::new()); + let mut rounded_subpaths = Vec::new(); for bezpath in source.stroke_bezpath_iter(&source_paths) { // The radius is measured after the transform, so scaling the layer doesn't scale its rounding @@ -655,14 +655,17 @@ async fn round_corners( // A subpath without corners to round passes through as it was let Some(mut rounded_subpath) = round_subpath_corners(&document_bezpath, radius, roundness, edge_length_limit, min_angle_threshold.to_radians()) else { - result.append_bezpath(&mut result_paths, &bezpath); + rounded_subpaths.push(bezpath); continue; }; rounded_subpath.apply_affine(Affine::new(source_transform_inverse.to_cols_array())); - result.append_bezpath(&mut result_paths, &rounded_subpath); + rounded_subpaths.push(rounded_subpath); } + let (mut result, mut result_paths) = (Vector::default(), List::new()); + result.append_bezpaths(&mut result_paths, &rounded_subpaths); + attributes.insert(ATTR_PATHS, result_paths); Item::from_parts(result, attributes) }) @@ -1032,14 +1035,14 @@ async fn extrude(_: impl Ctx, #[implementations(G #[node_macro::node(category("Vector: Modifier"), icon("NodeBoxWarp"), path(core_types::vector))] async fn box_warp(_: impl Ctx, #[implementations(Graphic, Vector)] content: Item, #[expose] rectangle: Item) -> Item { let target_transform: DAffine2 = rectangle.attribute_cloned_or_default(ATTR_TRANSFORM); - let target_paths = paths_from_item_attributes(&rectangle).cloned().unwrap_or_default(); + let target_paths = shared_paths_from_item_attributes(&rectangle); let target = rectangle.into_element(); V::map_vector_items(content, |content| { let mut row = content; { let transform: DAffine2 = row.attribute_cloned_or_default(ATTR_TRANSFORM); - let paths = paths_from_item_attributes(&row).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&row); let vector = std::mem::take(row.element_mut()); // Get the bounding box of the source vector geometry @@ -1240,19 +1243,12 @@ async fn auto_tangents( let (spread, preserve_existing) = (*spread.element(), *preserve_existing.element()); let transform: DAffine2 = source.attribute_cloned_or_default(ATTR_TRANSFORM); - let source_paths = paths_from_item_attributes(&source).cloned().unwrap_or_default(); + let source_paths = shared_paths_from_item_attributes(&source); let (source, mut attributes) = source.into_parts(); - let (mut result, mut result_paths) = (Vector::default(), List::new()); - - // Each auto-tangented anchor is found as the end of its incoming segment, counted across all subpaths - let mut segment_offset = 0; - let mut smooth_segment_ends = Vec::new(); - let segments_after = |paths: &List, first_path: usize| -> usize { - (first_path..paths.len()) - .filter_map(|index| Some(paths.element(index)?.segment_count(curve_type_at(paths, index)))) - .sum() - }; + // Each subpath's softened bezpath, beside which of its manipulators were given auto-tangents + let mut softened_bezpaths = Vec::new(); + let mut auto_tangented_manipulators = Vec::new(); for (mut manipulators_list, is_closed) in source.stroke_manipulator_groups(&source_paths) { for manipulator in &mut manipulators_list { @@ -1266,9 +1262,8 @@ async fn auto_tangents( let mut unchanged = bezpath_from_manipulator_groups(&manipulators_list, is_closed); unchanged.apply_affine(Affine::new(transform.inverse().to_cols_array())); - let first_path = result_paths.len(); - result.append_bezpath(&mut result_paths, &unchanged); - segment_offset += segments_after(&result_paths, first_path); + softened_bezpaths.push(unchanged); + auto_tangented_manipulators.push(Vec::new()); continue; } @@ -1363,12 +1358,20 @@ async fn auto_tangents( let mut softened_bezpath = bezpath_from_manipulator_groups(&new_manipulators_list, is_closed); softened_bezpath.apply_affine(Affine::new(transform.inverse().to_cols_array())); - let first_path = result_paths.len(); - result.append_bezpath(&mut result_paths, &softened_bezpath); - let appended_segments = segments_after(&result_paths, first_path); + softened_bezpaths.push(softened_bezpath); + auto_tangented_manipulators.push(auto_tangented); + } + // Weaving every subpath at once costs O(points) rather than a pass over everything woven so far per subpath + let (mut result, mut result_paths) = (Vector::default(), List::new()); + let span_counts = result.append_bezpaths_counting_spans(&mut result_paths, &softened_bezpaths); + + // Each auto-tangented anchor is found as the end of its incoming segment, counted across all subpaths + let mut segment_offset = 0; + let mut smooth_segment_ends = Vec::new(); + for (auto_tangented, appended_segments) in auto_tangented_manipulators.iter().zip(span_counts) { // Mark auto-tangented points as having colinear handles - let num_manipulators = new_manipulators_list.len(); + let num_manipulators = auto_tangented.len(); for (i, _) in auto_tangented.iter().enumerate().filter(|&(_, &tangented)| tangented) { // For interior point i, the incoming segment is (i - 1) and the outgoing one is i, counted within this subpath. // For closed paths, point 0's incoming segment is the last one. For open paths, endpoints are never @@ -1398,7 +1401,7 @@ async fn auto_tangents( async fn bounding_box(_: impl Ctx, #[implementations(Graphic, Vector)] content: Item) -> Item { V::map_vector_items(content, |content| { let mut content = content; - let paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&content); let (result, result_paths) = content .element() .bounding_box_rect(&paths) @@ -1658,11 +1661,11 @@ async fn offset_path( let transform_attribute: DAffine2 = content.attribute_cloned_or_default(ATTR_TRANSFORM); let transform = Affine::new(transform_attribute.to_cols_array()); - let source_paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); + let source_paths = shared_paths_from_item_attributes(&content); let vector = std::mem::take(content.element_mut()); let bezpaths = vector.stroke_bezpath_iter(&source_paths).collect::>(); - let (mut result, mut result_paths) = (Vector::default(), List::new()); + let mut offset_bezpaths = Vec::with_capacity(bezpaths.len()); // Perform operation on all subpaths in this shape. for mut bezpath in bezpaths { @@ -1683,9 +1686,12 @@ async fn offset_path( bezpath_out.apply_affine(transform.inverse()); // One closed subpath, open path. - result.append_bezpath(&mut result_paths, &bezpath_out); + offset_bezpaths.push(bezpath_out); } + let (mut result, mut result_paths) = (Vector::default(), List::new()); + result.append_bezpaths(&mut result_paths, &offset_bezpaths); + *content.element_mut() = result; content.set_attribute(ATTR_PATHS, result_paths); content @@ -1774,8 +1780,8 @@ fn solidify_stroke_list(content: List) -> List { .first_index_of(Cover::Stroke) .zip(appearance.first_index_of(Cover::Fill)) .is_some_and(|(stroke_index, fill_index)| stroke_index < fill_index); - let paths = attributes.get::>(ATTR_PATHS).cloned().unwrap_or_default(); - let bezpaths = vector.stroke_bezpath_iter(&paths).collect::>(); + let paths = attributes.get::>(ATTR_PATHS).unwrap_or_else(|| no_paths()); + let bezpaths = vector.stroke_bezpath_iter(paths).collect::>(); let (mut solidified_stroke, mut solidified_stroke_paths) = (Vector::default(), List::new()); // Taking the existing stroke data and passing it to kurbo::stroke to generate new fill paths. @@ -1806,7 +1812,7 @@ fn solidify_stroke_list(content: List) -> List { // 0.25 is balanced between performance and accuracy of the curve. const STROKE_TOLERANCE: f64 = 0.25; - for mut path in bezpaths { + let solidified_bezpaths = bezpaths.into_iter().map(|mut path| { path.apply_affine(Affine::new(stroke.transform.to_cols_array())); let is_closed = matches!(path.elements().last(), Some(kurbo::PathEl::ClosePath)); @@ -1817,8 +1823,9 @@ fn solidify_stroke_list(content: List) -> List { solidified.apply_affine(Affine::new(stroke.transform.inverse().to_cols_array())); } - solidified_stroke.append_bezpath(&mut solidified_stroke_paths, &solidified); - } + solidified + }); + solidified_stroke.append_bezpaths(&mut solidified_stroke_paths, solidified_bezpaths); // If the original vector has a fill, preserve it as a separate item with the stroke coverages dropped let fill_row = has_fill.then(|| { @@ -1859,7 +1866,7 @@ async fn solidify_stroke(_: impl Ctx, #[implement #[node_macro::node(category("Vector: Modifier"), icon("NodeSeparateSubpaths"), path(core_types::vector))] async fn separate_subpaths(_: impl Ctx, #[implementations(Graphic, Vector)] content: Item) -> List { V::expand_vector_items(content, |content| { - let content_paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); + let content_paths = shared_paths_from_item_attributes(&content); let bezpaths = content.element().stroke_bezpath_iter(&content_paths).collect::>(); // Pass the original element through unchanged when it has no subpaths, so its attributes @@ -2004,9 +2011,9 @@ async fn sample_polyline( } }; - let (mut result, mut result_paths) = (Vector::default(), List::new()); + let mut sample_bezpaths = Vec::new(); - let content_paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); + let content_paths = shared_paths_from_item_attributes(&content); for local_bezpath in content.element().stroke_bezpath_iter(&content_paths).collect::>() { // Apply the transform to compute sample locations in world space (for correct distance-based spacing) let mut world_bezpath = local_bezpath.clone(); @@ -2042,9 +2049,12 @@ async fn sample_polyline( if was_closed { sample_bezpath.close_path(); } - result.append_bezpath(&mut result_paths, &sample_bezpath); + sample_bezpaths.push(sample_bezpath); } + let (mut result, mut result_paths) = (Vector::default(), List::new()); + result.append_bezpaths(&mut result_paths, &sample_bezpaths); + *content.element_mut() = result; content.set_attribute(ATTR_PATHS, result_paths); content @@ -2077,17 +2087,18 @@ async fn simplify( let transform = Affine::new(transform_attribute.to_cols_array()); let inverse_transform = transform.inverse(); - let (mut result, mut result_paths) = (Vector::default(), List::new()); - - let item_paths = paths_from_item_attributes(&item).cloned().unwrap_or_default(); - for mut bezpath in item.element().stroke_bezpath_iter(&item_paths).collect::>() { + let item_paths = shared_paths_from_item_attributes(&item); + let simplified_bezpaths = item.element().stroke_bezpath_iter(&item_paths).map(|mut bezpath| { bezpath.apply_affine(transform); let mut simplified = simplify_bezpath(bezpath, tolerance, &options); simplified.apply_affine(inverse_transform); - result.append_bezpath(&mut result_paths, &simplified); - } + simplified + }); + + let (mut result, mut result_paths) = (Vector::default(), List::new()); + result.append_bezpaths(&mut result_paths, simplified_bezpaths); *item.element_mut() = result; item.set_attribute(ATTR_PATHS, result_paths); @@ -2174,9 +2185,9 @@ async fn decimate( let transform = Affine::new(transform_attribute.to_cols_array()); let inverse_transform = transform.inverse(); - let (mut result, mut result_paths) = (Vector::default(), List::new()); + let mut decimated_bezpaths = Vec::new(); - let content_paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); + let content_paths = shared_paths_from_item_attributes(&content); for mut bezpath in content.element().stroke_bezpath_iter(&content_paths).collect::>() { bezpath.apply_affine(transform); @@ -2213,9 +2224,12 @@ async fn decimate( } new_bezpath.apply_affine(inverse_transform); - result.append_bezpath(&mut result_paths, &new_bezpath); + decimated_bezpaths.push(new_bezpath); } + let (mut result, mut result_paths) = (Vector::default(), List::new()); + result.append_bezpaths(&mut result_paths, &decimated_bezpaths); + *content.element_mut() = result; content.set_attribute(ATTR_PATHS, result_paths); content @@ -2245,7 +2259,7 @@ async fn cut_path( let euclidean = !parameterized_distance; - let content_paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); + let content_paths = shared_paths_from_item_attributes(&content); let bezpaths = content.element().stroke_bezpath_iter(&content_paths).collect::>(); let bezpath_count = bezpaths.len() as f64; @@ -2254,20 +2268,19 @@ async fn cut_path( let index = if t_value >= bezpath_count { (bezpath_count - 1.) as usize } else { t_value as usize }; if let Some(bezpath) = bezpaths.get(index).cloned() { - let (mut result_vector, mut result_paths) = (Vector::default(), List::new()); - - for (_, bezpath) in bezpaths.iter().enumerate().filter(|&(i, _)| i != index) { - result_vector.append_bezpath(&mut result_paths, bezpath); - } let t = if t_value == bezpath_count { 1. } else { t_value.fract() }; let t = if euclidean { TValue::Euclidean(t) } else { TValue::Parametric(t) }; - if let Some((first, second)) = split_bezpath(&bezpath, t) { - result_vector.append_bezpath(&mut result_paths, &first); - result_vector.append_bezpath(&mut result_paths, &second); - } else { - result_vector.append_bezpath(&mut result_paths, &bezpath); - } + // Every other bezpath comes first, then the two halves of the one being split, or all of it where it cannot be + let halves = split_bezpath(&bezpath, t); + let split = match &halves { + Some((first, second)) => vec![first, second], + None => vec![&bezpath], + }; + let others = bezpaths.iter().enumerate().filter(|&(i, _)| i != index).map(|(_, bezpath)| bezpath); + + let (mut result_vector, mut result_paths) = (Vector::default(), List::new()); + result_vector.append_bezpaths(&mut result_paths, others.chain(split)); *content.element_mut() = result_vector; content.set_attribute(ATTR_PATHS, result_paths); @@ -2437,7 +2450,7 @@ async fn scatter_points( let mut cloud = List::new(); - let content_paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); + let content_paths = shared_paths_from_item_attributes(&content); let path_with_bounding_boxes: Vec<_> = content .element() .stroke_bezpath_iter(&content_paths) @@ -2460,6 +2473,7 @@ async fn scatter_points( } // The scattered points replace the source shape entirely, leaving no paths + drop(content_paths); edit_item_geometry(&mut content, |vector, paths| { *vector = Vector(cloud); *paths = List::new(); @@ -2630,12 +2644,13 @@ async fn jitter_points( let transform_attribute: DAffine2 = content.attribute_cloned_or_default(ATTR_TRANSFORM); let inverse_linear = inverse_linear_or_repair(transform_attribute.matrix2); - edit_item_geometry(&mut content, |vector, paths| { + edit_item_points(&mut content, |vector, paths| { + let tangents = if along_normals { vector.anchor_tangents(paths) } else { Vec::new() }; let anchors = vector.anchor_indices(paths); let deltas: Vec<_> = anchors .iter() .map(|&point_index| { - let normal = if along_normals { vector.point_tangent(paths, point_index).map(|t| -t.perp()) } else { None }; + let normal = tangents.get(point_index).copied().flatten().map(|tangent| -tangent.perp()); let offset = if let Some(normal) = normal { normal * (rng.random::() * 2. - 1.) @@ -2673,12 +2688,13 @@ async fn offset_points( let transform_attribute: DAffine2 = content.attribute_cloned_or_default(ATTR_TRANSFORM); let inverse_linear = inverse_linear_or_repair(transform_attribute.matrix2); - edit_item_geometry(&mut content, |vector, paths| { + edit_item_points(&mut content, |vector, paths| { + let tangents = vector.anchor_tangents(paths); let anchors = vector.anchor_indices(paths); let deltas: Vec<_> = anchors .iter() .map(|&point_index| { - let Some(normal) = vector.point_tangent(paths, point_index).map(|t| -t.perp()) else { + let Some(normal) = tangents.get(point_index).copied().flatten().map(|tangent| -tangent.perp()) else { return DVec2::ZERO; }; @@ -3868,7 +3884,7 @@ mod test { #[test] fn points_to_vector_keeps_positions_in_order_and_unconnected() { let result = vector_item_from_points(&SQUARE_WITH_CENTER); - let paths = paths_from_item_attributes(&result).cloned().unwrap_or_default(); + let paths = shared_paths_from_item_attributes(&result); assert_eq!(result.element().anchor_positions(&paths), SQUARE_WITH_CENTER); assert_eq!(result.element().segment_iter(&paths).count(), 0); }