From e22a9baedceefd97daffe84403b23c99c56d6919 Mon Sep 17 00:00:00 2001 From: Skyfuller1303 Date: Thu, 15 Oct 2026 11:47:12 +0200 Subject: [PATCH] fix: preserve tiny pattern scales and account for cached tiles --- crates/compose/src/shape_split.rs | 54 +++++++++++++++++++-------- crates/vector/src/pattern.rs | 2 +- crates/vector/tests/shape_patterns.rs | 13 +++++++ 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/crates/compose/src/shape_split.rs b/crates/compose/src/shape_split.rs index f0e2ecb6e5..d541de7ec2 100644 --- a/crates/compose/src/shape_split.rs +++ b/crates/compose/src/shape_split.rs @@ -22,24 +22,26 @@ fn cache() -> &'static Mutex<(HashMap, u64)> { } const CAPACITY: usize = 32; -/// Most pixel bytes the split parts may hold (#4530): each entry is two canvas-sized surfaces, -/// so 32 of them at 24 MP could hold several GB outside every memory budget. +/// Most pixel bytes held by split parts and their pinned patterns (#4530). #[cfg(not(test))] const BUDGET: usize = 256 << 20; /// Small under test, so a few 512² shapes exercise the eviction. #[cfg(test)] const BUDGET: usize = 8 << 20; -/// Pixel bytes of one entry's (fill, stroke) parts (the pinned shape pixels are the document's). -fn entry_bytes(parts: &(Surface, Surface)) -> usize { - [&parts.0, &parts.1].iter().map(|s| s.tiles().map(|(_, t)| t.bytes().len()).sum::()).sum() +/// Pixel bytes held by one entry, including pinned source tiles. Shared pattern tiles count once +/// per entry, conservatively matching their retained lifetime without cross-entry bookkeeping. +fn entry_bytes(parts: &(Surface, Surface), patterns: &[Pattern]) -> usize { + let part_bytes = [&parts.0, &parts.1].iter().flat_map(|s| s.tiles().map(|(_, t)| t.bytes().len())).fold(0usize, usize::saturating_add); + let pattern_bytes = patterns.iter().flat_map(|p| p.surface.tiles().map(|(_, t)| t.bytes().len())).fold(0usize, usize::saturating_add); + part_bytes.saturating_add(pattern_bytes) } -/// Drop every cached split (Edit › Purge › All), returning the parts' pixel bytes. Callers may -/// still hold returned surfaces, so this is not a measure of freed process memory. +/// Drop every cached split (Edit › Purge › All), returning accounted pixel bytes. Callers may +/// still hold returned surfaces and patterns, so this is not a measure of freed process memory. pub fn purge_cache() -> usize { let mut c = cache().lock().unwrap_or_else(|e| e.into_inner()); - let bytes = c.0.values().map(|e| entry_bytes(&e.0)).sum(); + let bytes = c.0.values().map(|e| entry_bytes(&e.0, &e.3)).sum(); c.0.clear(); bytes } @@ -100,19 +102,19 @@ pub fn split_with_patterns(sh: &ShapeLayer, canvas: Rect, depth: SampleType, pat } let mut c = cache().lock().unwrap_or_else(|e| e.into_inner()); // Least recently used first, until the new entry fits both the count and the byte budget. - let new_bytes = entry_bytes(&parts); + let new_bytes = entry_bytes(&parts, &used_patterns); + if new_bytes > BUDGET { + // Larger than the whole budget on its own: use it, don't keep it. + return Some(parts); + } loop { - let held: usize = c.0.values().map(|e| entry_bytes(&e.0)).sum(); + let held: usize = c.0.values().map(|e| entry_bytes(&e.0, &e.3)).sum(); if c.0.len() < CAPACITY && held.saturating_add(new_bytes) <= BUDGET { break; } let Some(old) = c.0.iter().min_by_key(|e| (e.1).1).map(|e| *e.0) else { break }; c.0.remove(&old); } - if new_bytes > BUDGET { - // Larger than the whole budget on its own: use it, don't keep it. - return Some(parts); - } let tick = c.1; c.0.insert(key, (parts.clone(), tick, sh.cache.clone(), used_patterns)); Some(parts) @@ -189,9 +191,29 @@ mod tests { } fn held() -> usize { - cache().lock().unwrap_or_else(|e| e.into_inner()).0.values().map(|e| entry_bytes(&e.0)).sum() + cache().lock().unwrap_or_else(|e| e.into_inner()).0.values().map(|e| entry_bytes(&e.0, &e.3)).sum() } + #[test] + fn split_cache_does_not_pin_pattern_tiles_over_its_budget() { + let fmt = PixelFormat::RGBA32F; + let mut pixels = Surface::new(fmt); + pixels.fill_rect(Rect::new(0, 0, 1024, 1024), &[0.2, 0.4, 0.6, 1.0]); + let pattern = Pattern::new("dense", pixels, 1024, 1024); + assert!(pattern.surface.tiles().map(|(_, tile)| tile.bytes().len()).sum::() > BUDGET); + let mut sh = shape(0.0); + sh.fill = Some(Fill::Pattern { id: pattern.id.clone(), name: pattern.name.clone(), scale: 1.0, angle: 0.0, link: false, phase: (0.0, 0.0) }); + split_with_patterns(&sh, Rect::new(0, 0, 32, 32), SampleType::U8, std::slice::from_ref(&pattern), ColorMode::Rgb).expect("stroked"); + let pattern_tiles: Vec<_> = pattern.surface.tiles().map(|(_, tile)| std::sync::Arc::as_ptr(tile) as usize).collect(); + let c = cache().lock().unwrap_or_else(|e| e.into_inner()); + assert!( + c.0.values().all(|entry| entry + .3 + .iter() + .all(|held| held.surface.tiles().all(|(_, tile)| !pattern_tiles.contains(&(std::sync::Arc::as_ptr(tile) as usize))))), + "oversized pattern was pinned by split cache" + ); + } /// The split cache stays within its byte budget, evicting the least recently used, and /// Edit › Purge › All empties it (#4530). #[test] @@ -199,7 +221,7 @@ mod tests { let canvas = Rect::new(0, 0, 1024, 512); for i in 0..6 { let parts = split(&shape(f64::from(i) * 10.0), canvas, SampleType::U8).expect("stroked"); - assert!(entry_bytes(&parts) > 0); + assert!(entry_bytes(&parts, &[]) > 0); assert!(held() <= BUDGET, "shape {i}: {} bytes held", held()); } // Other tests share the cache, so only what this test can rely on: a purge frees bytes. diff --git a/crates/vector/src/pattern.rs b/crates/vector/src/pattern.rs index 53763ea4eb..82fa589674 100644 --- a/crates/vector/src/pattern.rs +++ b/crates/vector/src/pattern.rs @@ -147,7 +147,7 @@ impl Placement { let base = if link { anchor } else { (0.0, 0.0) }; let origin = (base.0 + f64::from(phase.0), base.1 + f64::from(phase.1)); let s = f64::from(scale); - let inv_scale = if s.is_finite() && s > 1e-3 { 1.0 / s } else { 1.0 }; + let inv_scale = if s.is_finite() && s > 0.0 { 1.0 / s } else { 1.0 }; let a = f64::from(angle).to_radians(); Placement { origin, cs: (a.cos(), a.sin()), inv_scale } } diff --git a/crates/vector/tests/shape_patterns.rs b/crates/vector/tests/shape_patterns.rs index e32c6f54c4..402356149f 100644 --- a/crates/vector/tests/shape_patterns.rs +++ b/crates/vector/tests/shape_patterns.rs @@ -89,6 +89,19 @@ fn pattern_lookup_placement_and_premultiplied_sampling() { assert!(tile.sample(f64::MAX, -f64::MAX).iter().all(|v| v.is_finite())); } +#[test] +fn tiny_positive_pattern_scale_is_preserved_in_sampling_and_shape_rendering() { + let p = pattern(SampleType::F32, ColorMode::Rgb); + let scale = 0.0005; + let placement = photocraft_vector::pattern::Placement::anchored((8.0, 8.0), true, (0.0, 0.0), scale, 0.0); + let mapped = placement.map(10.5, 10.5); + assert!((mapped.0 - 5000.0).abs() < 0.001 && (mapped.1 - 5000.0).abs() < 0.001, "{mapped:?}"); + let expected = Tile::new(&p).unwrap().sample(mapped.0, mapped.1); + let paint = Fill::Pattern { id: p.id.clone(), name: p.name.clone(), scale, angle: 0.0, link: true, phase: (0.0, 0.0) }; + let shape = ShapeLayer { path: photocraft_vector::shapes::rounded_rect(8.0, 8.0, 16.0, 16.0, [0.0; 4]), fill: Some(paint), ..Default::default() }; + let rendered = render_shape_with_patterns(&shape, PixelFormat::RGBA32F, Rect::new(0, 0, 32, 32), std::slice::from_ref(&p)); + close(rendered.rgba(10, 10), expected, 0.001); +} #[test] fn oversized_sparse_pattern_samples_without_a_second_full_surface() { let p = Pattern {