From c348b384aa77f34e08fe69d087d87ebc57fd93ef Mon Sep 17 00:00:00 2001 From: bflatastic Date: Wed, 14 Oct 2026 08:46:46 +0700 Subject: [PATCH] Fix #1723: a pattern's tile is drawn again when what its art shows changes A tile was kept while the pattern's own art, tile and tiling were the ones it was drawn from. Art that shows a symbol, is painted with another pattern or holds an image or type doesn't change when those do, so the pattern kept drawing the old symbol (in every renderer on the thread: the tiles are shared). Such a tile now also remembers the document's pattern and symbol art, image pixels and fonts, and is drawn again when they differ. A raster effect on an object painted with a pattern reads all of that too, as one on a symbol instance does. --- crates/render/src/fx.rs | 18 +++++++++-- crates/render/src/pattern.rs | 52 ++++++++++++++++++++++++++++--- crates/render/src/tests_memory.rs | 43 +++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/crates/render/src/fx.rs b/crates/render/src/fx.rs index 0109bb20..483ac77a 100644 --- a/crates/render/src/fx.rs +++ b/crates/render/src/fx.rs @@ -532,7 +532,8 @@ impl Drawn { /// objects in it ([`Reads::of`]): which parts of [`Inputs`] can change what its rasters show. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub(crate) struct Reads { - /// Pattern paints: the pattern definitions. + /// Pattern paints: the pattern definitions. Their art can hold anything, so content painted + /// with a pattern reads everything else too. patterns: bool, /// Symbol instances and type with inline graphics: the symbols' art. Symbol art can hold /// anything, so content that shows a symbol reads everything else too. @@ -550,14 +551,25 @@ pub(crate) struct Reads { impl Reads { /// What the content of `n` reads. pub(crate) fn of(n: &Node) -> Self { + Self::of_all(std::iter::once(n)) + } + + /// What the content of `nodes` reads together. A pattern's art can hold anything, as a + /// symbol's can, so content painted with a pattern reads everything else too (#1723). + pub(crate) fn of_all<'a>(nodes: impl Iterator) -> Self { let mut reads = Self::default(); - reads.add(n); - if reads.symbols { + nodes.for_each(|n| reads.add(n)); + if reads.symbols || reads.patterns { return Self { patterns: true, symbols: true, images: true, brushes: true, text: true }; } reads } + /// Does the content read anything outside its own nodes? + pub(crate) fn any(self) -> bool { + self != Self::default() + } + fn add(&mut self, n: &Node) { let pattern = |p: &vectorcraft_color::Paint| matches!(p, vectorcraft_color::Paint::Pattern { .. }); n.walk(&mut |c| { diff --git a/crates/render/src/pattern.rs b/crates/render/src/pattern.rs index c61759dc..711730a5 100644 --- a/crates/render/src/pattern.rs +++ b/crates/render/src/pattern.rs @@ -6,14 +6,16 @@ //! with the paint transform mapping pixels → pattern space → document. Tiles are cached per //! pattern name and raster scale (and ink plane, see [`crate::ink`]) within //! [`lru::PATTERN_BUDGET`]. A tile is used while the pattern's definition is the one it was -//! rasterized from (art `Arc` identity, tile, tile type, overlap): the pattern's "revision". +//! rasterized from (art `Arc` identity, tile, tile type, overlap): the pattern's "revision", and, +//! when its art shows more than itself (a symbol, another pattern, an image, type), while what +//! the document holds of those is what it held then ([`Deps`]). //! //! Pattern editing mode (`Document::pattern_edit`) draws only the temporary tile layer, with //! dimmed copies of it at the neighbouring tile positions and the tile edge. use std::cell::RefCell; use std::hash::{Hash, Hasher}; -use std::sync::Arc; +use std::sync::{Arc, Weak}; use vectorcraft_doc::pattern::{Overlap, PatternDef, TileType}; use vectorcraft_doc::{Document, Node}; @@ -32,8 +34,46 @@ const MAX_TILE_PX: f64 = 2048.0; /// ([`raster_scale`]). type TileKey = (String, u64); +/// What a tile was drawn from besides its pattern's own art, for art that shows more than itself: +/// the art of every pattern and symbol and the pixels of every image in the document, and the +/// fonts. Editing a symbol leaves the art of a pattern that shows it as it was, so the art alone +/// kept the tile from before (#1723). Held weakly: a weak reference keeps the allocation, so no +/// other value gets one of these addresses while the tile is kept. +struct Deps { + art: Vec>, + pixels: Vec>>, + fonts: u64, +} + +impl Deps { + fn art(doc: &Document) -> impl Iterator> { + doc.patterns.iter().flat_map(|p| &p.art).chain(doc.symbols.iter().map(|s| &s.art)) + } + + fn pixels(doc: &Document) -> impl Iterator>> { + doc.images.values().flat_map(|b| std::iter::once(&b.bytes).chain(&b.proxy)) + } + + fn of(doc: &Document) -> Self { + Self { + art: Self::art(doc).map(Arc::downgrade).collect(), + pixels: Self::pixels(doc).map(Arc::downgrade).collect(), + fonts: vectorcraft_text::FontDb::global().generation(), + } + } + + /// Does `doc` hold what the tile was drawn from? + fn current(&self, doc: &Document) -> bool { + self.fonts == vectorcraft_text::FontDb::global().generation() + && self.art.iter().map(Weak::as_ptr).eq(Self::art(doc).map(Arc::as_ptr)) + && self.pixels.iter().map(Weak::as_ptr).eq(Self::pixels(doc).map(Arc::as_ptr)) + } +} + struct TileEntry { art: Vec>, + /// `None` for art that shows only itself. + deps: Option, params: (Rect, TileType, Overlap), sx: f64, sy: f64, @@ -163,7 +203,10 @@ pub(crate) fn set_pattern_paint(ctx: &mut RenderContext, name: &str, xf: Affine, let mut c = c.borrow_mut(); let tiles = &mut c.tiles[ink as usize]; let t = tiles.tick(); - tiles.get(&key, t).filter(|e| e.params == params && same_art(&e.art, &def.art)).map(|e| (e.pixmap.clone(), e.sx, e.sy)) + tiles + .get(&key, t) + .filter(|e| e.params == params && same_art(&e.art, &def.art) && e.deps.as_ref().is_none_or(|d| d.current(doc))) + .map(|e| (e.pixmap.clone(), e.sx, e.sy)) }); let (pm, sx, sy) = match hit { Some(h) => h, @@ -176,11 +219,12 @@ pub(crate) fn set_pattern_paint(ctx: &mut RenderContext, name: &str, xf: Affine, }; let pm = Arc::new(pm); let (sx, sy) = (w as f64 / pw, h as f64 / ph); + let deps = crate::fx::Reads::of_all(def.art.iter().map(|a| &**a)).any().then(|| Deps::of(doc)); CACHE.with(|c| { let mut c = c.borrow_mut(); let tiles = &mut c.tiles[ink as usize]; let (t, bytes) = (tiles.tick(), size_of::() + w as usize * h as usize * 4); - tiles.insert(key, TileEntry { art: def.art.clone(), params, sx, sy, pixmap: pm.clone() }, bytes, t); + tiles.insert(key, TileEntry { art: def.art.clone(), deps, params, sx, sy, pixmap: pm.clone() }, bytes, t); }); (pm, sx, sy) } diff --git a/crates/render/src/tests_memory.rs b/crates/render/src/tests_memory.rs index 031344ff..5d178601 100644 --- a/crates/render/src/tests_memory.rs +++ b/crates/render/src/tests_memory.rs @@ -940,6 +940,49 @@ fn rasters_draw_pattern_edits() { }); } +/// #1723: a pattern's tile is drawn again when what its art shows changes while the art itself +/// doesn't: a symbol an instance in it shows, or another pattern it is painted with. So is a +/// raster effect on an object painted with such a pattern. +#[test] +fn pattern_tiles_draw_edits_to_what_their_art_shows() { + let _fonts = fonts_steady(); + use vectorcraft_doc::pattern::{PatternDef, pattern_paint}; + use vectorcraft_geom::shapes::rectangle; + let tile = |name: &str, art: Node| { + let mut def = PatternDef::new(name, vec![Arc::new(art)]); + def.tile = Rect::new(0.0, 0.0, 20.0, 20.0); + def + }; + // Pattern `Stamps` shows symbol `S`; pattern `Outer` is a square painted with pattern `Checks`. + let doc = |pattern: &str, effects: Vec| { + let mut d = Document::new(200.0, 100.0); + let art = symbol_art(&mut d); + d.symbols.push(vectorcraft_doc::Symbol { name: "S".into(), art: Arc::new(art) }); + let shown = Node::new(d.alloc_id(), NodeKind::SymbolInstance { symbol: "S".into(), xf: Affine::translate((10.0, 10.0)) }); + d.patterns.push(tile("Stamps", shown)); + d.patterns.push(checks(Color::rgb(1.0, 0.0, 0.0))); + let square = filled(&mut d, rectangle(Rect::new(0.0, 0.0, 20.0, 20.0)), pattern_paint("Checks"), vec![]); + d.patterns.push(tile("Outer", square)); + let painted = filled(&mut d, rectangle(Rect::new(10.0, 10.0, 190.0, 90.0)), pattern_paint(pattern), effects); + with_art(d, vec![painted]) + }; + let new_symbol = |d: &mut Document| { + let red = vectorcraft_doc::Appearance::basic(Paint::solid(Color::rgb(1.0, 0.0, 0.0)), Paint::None, 0.0); + let bar = Node::path(d.alloc_id(), rectangle(Rect::new(-8.0, -2.0, 8.0, 2.0)), red); + d.symbols[0].art = Arc::new(bar); + }; + let new_checks = |d: &mut Document| { + if let Some(def) = d.pattern_mut("Checks") { + def.art = checks(Color::rgb(0.0, 0.0, 1.0)).art; + } + }; + let blur = || vec![effect("blur.gaussian", serde_json::json!({"radius": 2.0}))]; + edit_draws_as_new(doc("Stamps", vec![]), new_symbol); + edit_draws_as_new(doc("Stamps", blur()), new_symbol); + edit_draws_as_new(doc("Outer", vec![]), new_checks); + edit_draws_as_new(doc("Outer", blur()), new_checks); +} + /// Raster effects are made again when a symbol shown inside another symbol changes (a group's drop /// shadow around an instance of the other symbol), when a brush definition changes (a brushed /// stroke's drop shadow), and when an image's pixels change under the same key, as when a linked