From 7ecf5d894944b58ce0d43b33766cb4803f9b919f Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Tue, 14 Oct 2025 00:07:01 +0200 Subject: [PATCH 1/4] Cache calculation of bounding boxes with transforms --- .../utility_types/document_metadata.rs | 5 +-- node-graph/gcore/src/vector/click_target.rs | 34 +++++++++++++++++-- 2 files changed, 33 insertions(+), 6 deletions(-) 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/vector/click_target.rs b/node-graph/gcore/src/vector/click_target.rs index 6ab7c1860a..bc363adfac 100644 --- a/node-graph/gcore/src/vector/click_target.rs +++ b/node-graph/gcore/src/vector/click_target.rs @@ -31,11 +31,18 @@ pub enum ClickTargetType { } /// 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_cache: std::sync::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 +52,7 @@ impl ClickTarget { target_type: ClickTargetType::Subpath(subpath), stroke_width, bounding_box, + bounding_box_cache: std::sync::Arc::new(Vec::with_capacity(10).into()), } } @@ -60,6 +68,7 @@ impl ClickTarget { target_type: ClickTargetType::FreePoint(point), stroke_width, bounding_box, + bounding_box_cache: std::sync::Arc::new(Vec::with_capacity(10).into()), } } @@ -76,7 +85,28 @@ impl ClickTarget { } 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)]) + match self.target_type { + ClickTargetType::Subpath(ref subpath) => + { + let read = self.bounding_box_cache.read().unwrap(); + if let Some((_, bounds)) = read.iter().find(|(t, _)| t.to_cols_array().iter().zip(transform.to_cols_array().iter()).all(|(a, b)|a.to_bits() == b.to_bits())){ + *bounds + } else { + std::mem::drop(read); + let mut write = self.bounding_box_cache.write().unwrap(); + if write.len() >= 10 { + write.clear(); + } + let bounds = subpath.bounding_box_with_transform(transform); + + write.push((transform, bounds)); + bounds + } + } + , + // 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) { From 5e3e09a5c401f753fcbb9c12e2fd81d519f61e63 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 15 Oct 2025 21:01:19 +0200 Subject: [PATCH 2/4] Reuse bounding boxes across for the same rotation --- node-graph/gcore/src/transform.rs | 11 ++++ node-graph/gcore/src/vector/click_target.rs | 73 ++++++++++++++++----- 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/node-graph/gcore/src/transform.rs b/node-graph/gcore/src/transform.rs index 141fa69c6c..edc31deb11 100644 --- a/node-graph/gcore/src/transform.rs +++ b/node-graph/gcore/src/transform.rs @@ -21,6 +21,17 @@ pub trait Transform { let rotation = -rotation_matrix.mul_vec2(DVec2::X).angle_to(DVec2::X); if rotation == -0. { 0. } else { rotation } } + + 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 orthogonal and equal length (no skew) + 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 bc363adfac..398d7b02ed 100644 --- a/node-graph/gcore/src/vector/click_target.rs +++ b/node-graph/gcore/src/vector/click_target.rs @@ -1,8 +1,11 @@ +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}; @@ -30,13 +33,21 @@ pub enum ClickTargetType { FreePoint(FreePoint), } +#[derive(Clone, Debug, Default)] +struct BoundingBoxCache { + fingerprints: u64, + elements: [(f64, Option<[DVec2; 2]>); 8], + write_ptr: usize, +} + /// Represents a clickable target for the layer #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct ClickTarget { target_type: ClickTargetType, stroke_width: f64, bounding_box: Option<[DVec2; 2]>, - bounding_box_cache: std::sync::Arc)>>>, + #[serde(skip)] + bounding_box_cache: Arc>, } impl PartialEq for ClickTarget { @@ -52,7 +63,7 @@ impl ClickTarget { target_type: ClickTargetType::Subpath(subpath), stroke_width, bounding_box, - bounding_box_cache: std::sync::Arc::new(Vec::with_capacity(10).into()), + bounding_box_cache: Default::default(), } } @@ -68,7 +79,7 @@ impl ClickTarget { target_type: ClickTargetType::FreePoint(point), stroke_width, bounding_box, - bounding_box_cache: std::sync::Arc::new(Vec::with_capacity(10).into()), + bounding_box_cache: Default::default(), } } @@ -86,24 +97,54 @@ impl ClickTarget { pub fn bounding_box_with_transform(&self, transform: DAffine2) -> Option<[DVec2; 2]> { match self.target_type { - ClickTargetType::Subpath(ref subpath) => - { - let read = self.bounding_box_cache.read().unwrap(); - if let Some((_, bounds)) = read.iter().find(|(t, _)| t.to_cols_array().iter().zip(transform.to_cols_array().iter()).all(|(a, b)|a.to_bits() == b.to_bits())){ - *bounds - } else { + ClickTargetType::Subpath(ref subpath) => { + if transform.has_skew() { + return subpath.bounding_box_with_transform(transform); + } + let rotation = transform.decompose_rotation(); + let fingerprint = (rotation.to_bits() % 265) as u8; + + let read = self.bounding_box_cache.read().unwrap(); + + let mut mask: u8 = 0; + // Split out mask construction into dedicated loop to allow for vectorization + for (i, fp) in (0..8).zip(read.fingerprints.to_le_bytes()) { + if fp == fingerprint { + mask |= 1 << i; + } + } + let scale = transform.decompose_scale(); + + while mask != 0 { + let pos = mask.trailing_zeros() as usize; + + if rotation == read.elements[pos].0 && read.elements[pos].1.is_some() { + let transform = DAffine2::from_scale_angle_translation(scale, 0., transform.translation); + let new_bounds = read.elements[pos].1.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]); + + return new_bounds; + } + mask &= !(1 << pos); + } + std::mem::drop(read); let mut write = self.bounding_box_cache.write().unwrap(); - if write.len() >= 10 { - write.clear(); + let bounds = subpath.bounding_box_with_transform(DAffine2::from_angle(rotation)); + + if bounds.is_none() { + return bounds; } - let bounds = subpath.bounding_box_with_transform(transform); - write.push((transform, bounds)); - bounds - } + let write_ptr = write.write_ptr; + write.elements[write_ptr] = (rotation, bounds); + let mut bytes = write.fingerprints.to_le_bytes(); + bytes[write_ptr] = fingerprint; + write.fingerprints = u64::from_le_bytes(bytes); + write.write_ptr = (write_ptr + 1) % write.elements.len(); + + let transform = DAffine2::from_scale_angle_translation(scale, 0., transform.translation); + bounds.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]) } - , // TODO: use point for calculation of bbox ClickTargetType::FreePoint(_) => self.bounding_box.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]), } From 7bef7163a0003c0009832cb630a68af501f558e0 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Thu, 16 Oct 2025 15:36:27 +0200 Subject: [PATCH 3/4] Cleanup code and add documentation --- node-graph/gcore/src/transform.rs | 8 +- node-graph/gcore/src/vector/click_target.rs | 133 ++++++++++++++------ 2 files changed, 99 insertions(+), 42 deletions(-) diff --git a/node-graph/gcore/src/transform.rs b/node-graph/gcore/src/transform.rs index edc31deb11..8dbb75fd92 100644 --- a/node-graph/gcore/src/transform.rs +++ b/node-graph/gcore/src/transform.rs @@ -22,6 +22,11 @@ pub trait Transform { 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; @@ -29,7 +34,8 @@ pub trait Transform { const EPSILON: f64 = 1e-10; - // Check orthogonal and equal length (no skew) + // 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 } } diff --git a/node-graph/gcore/src/vector/click_target.rs b/node-graph/gcore/src/vector/click_target.rs index 398d7b02ed..576e1c8995 100644 --- a/node-graph/gcore/src/vector/click_target.rs +++ b/node-graph/gcore/src/vector/click_target.rs @@ -11,6 +11,8 @@ 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, @@ -33,19 +35,91 @@ 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, - elements: [(f64, Option<[DVec2; 2]>); 8], + /// (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, 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>, } @@ -87,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 } @@ -95,55 +169,32 @@ 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]> { + 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); } - let rotation = transform.decompose_rotation(); - let fingerprint = (rotation.to_bits() % 265) as u8; - - let read = self.bounding_box_cache.read().unwrap(); - let mut mask: u8 = 0; - // Split out mask construction into dedicated loop to allow for vectorization - for (i, fp) in (0..8).zip(read.fingerprints.to_le_bytes()) { - if fp == fingerprint { - mask |= 1 << i; - } - } + // Decompose transform into rotation, scale, translation for caching strategy + let rotation = transform.decompose_rotation(); let scale = transform.decompose_scale(); + let translation = transform.translation; - while mask != 0 { - let pos = mask.trailing_zeros() as usize; - - if rotation == read.elements[pos].0 && read.elements[pos].1.is_some() { - let transform = DAffine2::from_scale_angle_translation(scale, 0., transform.translation); - let new_bounds = read.elements[pos].1.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]); - - return new_bounds; - } - mask &= !(1 << pos); - } - - std::mem::drop(read); - let mut write = self.bounding_box_cache.write().unwrap(); - let bounds = subpath.bounding_box_with_transform(DAffine2::from_angle(rotation)); + // Generate fingerprint for cache lookup + let fingerprint = BoundingBoxCache::rotation_fingerprint(rotation); - if bounds.is_none() { - return bounds; + // 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); - let write_ptr = write.write_ptr; - write.elements[write_ptr] = (rotation, bounds); - let mut bytes = write.fingerprints.to_le_bytes(); - bytes[write_ptr] = fingerprint; - write.fingerprints = u64::from_le_bytes(bytes); - write.write_ptr = (write_ptr + 1) % write.elements.len(); - - let transform = DAffine2::from_scale_angle_translation(scale, 0., transform.translation); - bounds.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]) + // 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)]), From 251a6c205f0304bc827744f0d28473abf376a989 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Thu, 16 Oct 2025 16:33:20 +0200 Subject: [PATCH 4/4] Add tests --- node-graph/gcore/src/vector/click_target.rs | 162 ++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/node-graph/gcore/src/vector/click_target.rs b/node-graph/gcore/src/vector/click_target.rs index 576e1c8995..da55297cca 100644 --- a/node-graph/gcore/src/vector/click_target.rs +++ b/node-graph/gcore/src/vector/click_target.rs @@ -292,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()); + } + } +}