diff --git a/editor/src/messages/portfolio/document/utility_types/document_metadata.rs b/editor/src/messages/portfolio/document/utility_types/document_metadata.rs index acef970228..63b2d865b7 100644 --- a/editor/src/messages/portfolio/document/utility_types/document_metadata.rs +++ b/editor/src/messages/portfolio/document/utility_types/document_metadata.rs @@ -154,10 +154,7 @@ impl DocumentMetadata { pub fn bounding_box_with_transform(&self, layer: LayerNodeIdentifier, transform: DAffine2) -> Option<[DVec2; 2]> { self.click_targets(layer)? .iter() - .filter_map(|click_target| match click_target.target_type() { - ClickTargetType::Subpath(subpath) => subpath.bounding_box_with_transform(transform), - ClickTargetType::FreePoint(_) => click_target.bounding_box_with_transform(transform), - }) + .filter_map(|click_target| click_target.bounding_box_with_transform(transform)) .reduce(Quad::combine_bounds) } diff --git a/node-graph/gcore/src/transform.rs b/node-graph/gcore/src/transform.rs index 141fa69c6c..8dbb75fd92 100644 --- a/node-graph/gcore/src/transform.rs +++ b/node-graph/gcore/src/transform.rs @@ -21,6 +21,23 @@ pub trait Transform { let rotation = -rotation_matrix.mul_vec2(DVec2::X).angle_to(DVec2::X); if rotation == -0. { 0. } else { rotation } } + + /// Detects if the transform contains skew by checking if the transformation matrix + /// deviates from a pure rotation + uniform scale + translation. + /// + /// Returns true if the matrix columns are not orthogonal or have different lengths, + /// indicating the presence of skew or non-uniform scaling. + fn has_skew(&self) -> bool { + let mat2 = self.transform().matrix2; + let col0 = mat2.x_axis; + let col1 = mat2.y_axis; + + const EPSILON: f64 = 1e-10; + + // Check if columns are orthogonal (dot product should be ~0) and equal length + // Non-orthogonal columns or different lengths indicate skew/non-uniform scaling + col0.dot(col1).abs() > EPSILON || (col0.length() - col1.length()).abs() > EPSILON + } } pub trait TransformMut: Transform { diff --git a/node-graph/gcore/src/vector/click_target.rs b/node-graph/gcore/src/vector/click_target.rs index 6ab7c1860a..da55297cca 100644 --- a/node-graph/gcore/src/vector/click_target.rs +++ b/node-graph/gcore/src/vector/click_target.rs @@ -1,13 +1,18 @@ +use std::sync::{Arc, RwLock}; + use super::algorithms::{bezpath_algorithms::bezpath_is_inside_bezpath, intersection::filtered_segment_intersections}; use super::misc::dvec2_to_point; use crate::math::math_ext::QuadExt; use crate::math::quad::Quad; use crate::subpath::Subpath; +use crate::transform::Transform; use crate::vector::PointId; use crate::vector::misc::point_to_dvec2; use glam::{DAffine2, DMat2, DVec2}; use kurbo::{Affine, BezPath, ParamCurve, PathSeg, Shape}; +type BoundingBox = Option<[DVec2; 2]>; + #[derive(Copy, Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize)] pub struct FreePoint { pub id: PointId, @@ -30,12 +35,99 @@ pub enum ClickTargetType { FreePoint(FreePoint), } +/// Fixed-size ring buffer cache for rotated bounding boxes. +/// +/// Stores up to 8 rotation angles and their corresponding bounding boxes to avoid +/// recomputing expensive bezier curve bounds for repeated rotations. Uses 7-bit +/// fingerprint hashing with MSB as presence flag for fast lookup. +#[derive(Clone, Debug, Default)] +struct BoundingBoxCache { + /// Packed 7-bit fingerprints with MSB presence flags for cache lookup + fingerprints: u64, + /// (rotation_angle, cached_bounds) pairs + elements: [(f64, BoundingBox); Self::CACHE_SIZE], + /// Next position to write in ring buffer + write_ptr: usize, +} + +impl BoundingBoxCache { + /// Cache size - must be ≤ 8 since fingerprints is u64 (8 bytes, 1 byte per element) + const CACHE_SIZE: usize = 8; + const FINGERPRINT_BITS: u32 = 7; + const PRESENCE_FLAG: u8 = 1 << Self::FINGERPRINT_BITS; + + /// Generates a 7-bit fingerprint from rotation with MSB as presence flag + fn rotation_fingerprint(rotation: f64) -> u8 { + (rotation.to_bits() % (1 << Self::FINGERPRINT_BITS)) as u8 | Self::PRESENCE_FLAG + } + /// Attempts to find cached bounding box for the given rotation. + /// Returns Some(bounds) if found, None if not cached. + fn try_read(&self, rotation: f64, scale: DVec2, translation: DVec2, fingerprint: u8) -> Option { + // Build bitmask of positions with matching fingerprints for vectorized comparison + let mut mask: u8 = 0; + for (i, fp) in (0..Self::CACHE_SIZE).zip(self.fingerprints.to_le_bytes()) { + // Check MSB for presence and lower 7 bits for fingerprint match + if fp == fingerprint { + mask |= 1 << i; + } + } + // Check each position with matching fingerprint for exact rotation match + while mask != 0 { + let pos = mask.trailing_zeros() as usize; + + if rotation == self.elements[pos].0 { + // Found cached rotation - apply scale and translation to cached bounds + let transform = DAffine2::from_scale_angle_translation(scale, 0., translation); + let new_bounds = self.elements[pos].1.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]); + + return Some(new_bounds); + } + mask &= !(1 << pos); + } + None + } + /// Computes and caches bounding box for the given rotation, then applies scale/translation. + /// Returns the final transformed bounds. + fn add_to_cache(&mut self, subpath: &Subpath, rotation: f64, scale: DVec2, translation: DVec2, fingerprint: u8) -> BoundingBox { + // Compute bounds for pure rotation (expensive operation we want to cache) + let bounds = subpath.bounding_box_with_transform(DAffine2::from_angle(rotation)); + + if bounds.is_none() { + return bounds; + } + + // Store in ring buffer at current write position + let write_ptr = self.write_ptr; + self.elements[write_ptr] = (rotation, bounds); + + // Update fingerprint byte for this position + let mut bytes = self.fingerprints.to_le_bytes(); + bytes[write_ptr] = fingerprint; + self.fingerprints = u64::from_le_bytes(bytes); + + // Advance write pointer (ring buffer behavior) + self.write_ptr = (write_ptr + 1) % Self::CACHE_SIZE; + + // Apply scale and translation to cached rotated bounds + let transform = DAffine2::from_scale_angle_translation(scale, 0., translation); + bounds.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]) + } +} + /// Represents a clickable target for the layer -#[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize)] +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct ClickTarget { target_type: ClickTargetType, stroke_width: f64, - bounding_box: Option<[DVec2; 2]>, + bounding_box: BoundingBox, + #[serde(skip)] + bounding_box_cache: Arc>, +} + +impl PartialEq for ClickTarget { + fn eq(&self, other: &Self) -> bool { + self.target_type == other.target_type && self.stroke_width == other.stroke_width && self.bounding_box == other.bounding_box + } } impl ClickTarget { @@ -45,6 +137,7 @@ impl ClickTarget { target_type: ClickTargetType::Subpath(subpath), stroke_width, bounding_box, + bounding_box_cache: Default::default(), } } @@ -60,6 +153,7 @@ impl ClickTarget { target_type: ClickTargetType::FreePoint(point), stroke_width, bounding_box, + bounding_box_cache: Default::default(), } } @@ -67,7 +161,7 @@ impl ClickTarget { &self.target_type } - pub fn bounding_box(&self) -> Option<[DVec2; 2]> { + pub fn bounding_box(&self) -> BoundingBox { self.bounding_box } @@ -75,8 +169,36 @@ impl ClickTarget { self.bounding_box.map(|bbox| bbox[0] + (bbox[1] - bbox[0]) / 2.) } - pub fn bounding_box_with_transform(&self, transform: DAffine2) -> Option<[DVec2; 2]> { - self.bounding_box.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]) + pub fn bounding_box_with_transform(&self, transform: DAffine2) -> BoundingBox { + match self.target_type { + ClickTargetType::Subpath(ref subpath) => { + // Bypass cache for skewed transforms since rotation decomposition isn't valid + if transform.has_skew() { + return subpath.bounding_box_with_transform(transform); + } + + // Decompose transform into rotation, scale, translation for caching strategy + let rotation = transform.decompose_rotation(); + let scale = transform.decompose_scale(); + let translation = transform.translation; + + // Generate fingerprint for cache lookup + let fingerprint = BoundingBoxCache::rotation_fingerprint(rotation); + + // Try to read from cache first + let read_lock = self.bounding_box_cache.read().unwrap(); + if let Some(value) = read_lock.try_read(rotation, scale, translation, fingerprint) { + return value; + } + std::mem::drop(read_lock); + + // Cache miss - compute and store new entry + let mut write_lock = self.bounding_box_cache.write().unwrap(); + write_lock.add_to_cache(subpath, rotation, scale, translation, fingerprint) + } + // TODO: use point for calculation of bbox + ClickTargetType::FreePoint(_) => self.bounding_box.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]), + } } pub fn apply_transform(&mut self, affine_transform: DAffine2) { @@ -170,3 +292,165 @@ impl ClickTarget { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::subpath::Subpath; + use glam::DVec2; + use std::f64::consts::PI; + + #[test] + fn test_bounding_box_cache_fingerprint_generation() { + // Test that fingerprints have MSB set and use only 7 bits for data + let rotation1 = 0.0; + let rotation2 = PI / 3.0; + let rotation3 = PI / 2.0; + + let fp1 = BoundingBoxCache::rotation_fingerprint(rotation1); + let fp2 = BoundingBoxCache::rotation_fingerprint(rotation2); + let fp3 = BoundingBoxCache::rotation_fingerprint(rotation3); + + // All fingerprints should have MSB set (presence flag) + assert_eq!(fp1 & BoundingBoxCache::PRESENCE_FLAG, BoundingBoxCache::PRESENCE_FLAG); + assert_eq!(fp2 & BoundingBoxCache::PRESENCE_FLAG, BoundingBoxCache::PRESENCE_FLAG); + assert_eq!(fp3 & BoundingBoxCache::PRESENCE_FLAG, BoundingBoxCache::PRESENCE_FLAG); + + // Lower 7 bits should contain the actual fingerprint data + let data1 = fp1 & !BoundingBoxCache::PRESENCE_FLAG; + let data2 = fp2 & !BoundingBoxCache::PRESENCE_FLAG; + let data3 = fp3 & !BoundingBoxCache::PRESENCE_FLAG; + + // Data portions should be different (unless collision) + assert!(data1 != data2 && data2 != data3 && data3 != data1); + } + + #[test] + fn test_bounding_box_cache_basic_operations() { + let mut cache = BoundingBoxCache::default(); + + // Create a simple rectangle subpath for testing + let subpath = Subpath::new_rect(DVec2::ZERO, DVec2::new(100.0, 50.0)); + + let rotation = PI / 4.0; + let scale = DVec2::new(2.0, 2.0); + let translation = DVec2::new(10.0, 20.0); + let fingerprint = BoundingBoxCache::rotation_fingerprint(rotation); + + // Cache should be empty initially + assert!(cache.try_read(rotation, scale, translation, fingerprint).is_none()); + + // Add to cache + let result = cache.add_to_cache(&subpath, rotation, scale, translation, fingerprint); + assert!(result.is_some()); + + // Should now be able to read from cache + let cached = cache.try_read(rotation, scale, translation, fingerprint); + assert!(cached.is_some()); + assert_eq!(cached.unwrap(), result); + } + + #[test] + fn test_bounding_box_cache_ring_buffer_behavior() { + let mut cache = BoundingBoxCache::default(); + let subpath = Subpath::new_rect(DVec2::ZERO, DVec2::new(10.0, 10.0)); + let scale = DVec2::ONE; + let translation = DVec2::ZERO; + + // Fill cache beyond capacity to test ring buffer behavior + let rotations: Vec = (0..10).map(|i| i as f64 * PI / 8.0).collect(); + + for rotation in &rotations { + let fingerprint = BoundingBoxCache::rotation_fingerprint(*rotation); + cache.add_to_cache(&subpath, *rotation, scale, translation, fingerprint); + } + + // First two entries should be overwritten (cache size is 8) + let first_fp = BoundingBoxCache::rotation_fingerprint(rotations[0]); + let second_fp = BoundingBoxCache::rotation_fingerprint(rotations[1]); + let last_fp = BoundingBoxCache::rotation_fingerprint(rotations[9]); + + assert!(cache.try_read(rotations[0], scale, translation, first_fp).is_none()); + assert!(cache.try_read(rotations[1], scale, translation, second_fp).is_none()); + assert!(cache.try_read(rotations[9], scale, translation, last_fp).is_some()); + } + + #[test] + fn test_click_target_bounding_box_caching() { + // Create a click target with a simple rectangle + let subpath = Subpath::new_rect(DVec2::ZERO, DVec2::new(100.0, 50.0)); + let click_target = ClickTarget::new_with_subpath(subpath, 1.0); + + let rotation = PI / 6.0; + let scale = DVec2::new(1.5, 1.5); + let translation = DVec2::new(20.0, 30.0); + let transform = DAffine2::from_scale_angle_translation(scale, rotation, translation); + + // Helper function to count present values in cache + let count_present_values = || { + let cache = click_target.bounding_box_cache.read().unwrap(); + cache.fingerprints.to_le_bytes().iter().filter(|&&fp| fp & BoundingBoxCache::PRESENCE_FLAG != 0).count() + }; + + // Initially cache should be empty + assert_eq!(count_present_values(), 0); + + // First call should compute and cache + let result1 = click_target.bounding_box_with_transform(transform); + assert!(result1.is_some()); + assert_eq!(count_present_values(), 1); + + // Second call with same transform should use cache, not add new entry + let result2 = click_target.bounding_box_with_transform(transform); + assert_eq!(result1, result2); + assert_eq!(count_present_values(), 1); // Should still be 1, not 2 + + // Different scale/translation but same rotation should use cached rotation + let transform2 = DAffine2::from_scale_angle_translation(DVec2::new(2.0, 2.0), rotation, DVec2::new(50.0, 60.0)); + let result3 = click_target.bounding_box_with_transform(transform2); + assert!(result3.is_some()); + assert_ne!(result1, result3); // Different due to different scale/translation + assert_eq!(count_present_values(), 1); // Should still be 1, reused same rotation + } + + #[test] + fn test_click_target_skew_bypass_cache() { + let subpath = Subpath::new_rect(DVec2::ZERO, DVec2::new(100.0, 50.0)); + let click_target = ClickTarget::new_with_subpath(subpath.clone(), 1.0); + + // Create a transform with skew (non-uniform scaling in different directions) + let skew_transform = DAffine2::from_cols_array(&[2.0, 0.5, 0.0, 1.0, 10.0, 20.0]); + assert!(skew_transform.has_skew()); + + // Should bypass cache and compute directly + let result = click_target.bounding_box_with_transform(skew_transform); + let expected = subpath.bounding_box_with_transform(skew_transform); + assert_eq!(result, expected); + } + + #[test] + fn test_cache_fingerprint_collision_handling() { + let mut cache = BoundingBoxCache::default(); + let subpath = Subpath::new_rect(DVec2::ZERO, DVec2::new(10.0, 10.0)); + let scale = DVec2::ONE; + let translation = DVec2::ZERO; + + // Find two rotations that produce the same fingerprint (collision) + let rotation1 = 0.0; + let rotation2 = 0.25; + let fp1 = BoundingBoxCache::rotation_fingerprint(rotation1); + let fp2 = BoundingBoxCache::rotation_fingerprint(rotation2); + + // If we found a collision, test that exact rotation matching still works + if fp1 == fp2 && rotation1 != rotation2 { + // Add first rotation + cache.add_to_cache(&subpath, rotation1, scale, translation, fp1); + + // Should find the exact rotation + assert!(cache.try_read(rotation1, scale, translation, fp1).is_some()); + + // Should not find the colliding rotation (different exact value) + assert!(cache.try_read(rotation2, scale, translation, fp2).is_none()); + } + } +}