Merge pull request #1896 from storytold/fix/pattern-tiles-brush-cms

Pattern tiles after brush edits and Color Settings; MCP docs on failed batches

crates/doc/src/shared.rs

@@ -19,6 +19,11 @@ impl<T> Shared<T> {
     pub fn ptr_eq(a: &Self, b: &Self) -> bool {
         Arc::ptr_eq(&a.0, &b.0)
     }
+    /// A weak reference to the value: it keeps the allocation, so the value's address names it
+    /// while the reference is kept, but not the value.
+    pub fn downgrade(this: &Self) -> std::sync::Weak<T> {
+        Arc::downgrade(&this.0)
+    }
 }
 
 impl<T> std::ops::Deref for Shared<T> {

crates/engine/tests/color_settings.rs

@@ -127,6 +127,41 @@ fn raster_effects_draw_through_new_color_settings() {
     cms::set_active(&old).unwrap();
 }
 
+/// Pattern tiles follow the Color Settings: a fill of a pattern in a CMYK color is drawn through
+/// the new working CMYK profile in the next frame, as a renderer on a new thread (with no tiles of
+/// its own yet) draws it.
+#[test]
+fn pattern_tiles_draw_through_new_color_settings() {
+    use std::sync::Arc;
+    use vectorcraft_color::{Color, Paint};
+    use vectorcraft_doc::pattern::{PatternDef, pattern_paint};
+    use vectorcraft_doc::{Appearance, Document, Node};
+    use vectorcraft_geom::Rect;
+    use vectorcraft_geom::shapes::rectangle;
+    let _g = settings();
+    let old = cms::active_settings();
+    let mut d = Document::new(200.0, 100.0);
+    let cyan = Appearance::basic(Paint::solid(Color::cmyk(1.0, 0.0, 0.0, 0.0)), Paint::None, 0.0);
+    let square = Node::path(d.alloc_id(), rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)), cyan);
+    let mut def = PatternDef::new("Checks", vec![Arc::new(square)]);
+    def.tile = Rect::new(0.0, 0.0, 20.0, 20.0);
+    d.patterns.push(def);
+    let checks = Appearance::basic(pattern_paint("Checks"), Paint::None, 0.0);
+    let filled = Node::path(d.alloc_id(), rectangle(Rect::new(10.0, 10.0, 190.0, 90.0)), checks);
+    let layer = d.layers[0].id;
+    d.insert(Some(layer), 0, filled).unwrap();
+    let opts = RenderOptions { background: Some([255, 255, 255, 255]), ..Default::default() };
+    let frame = |r: &mut Renderer, d: &Document| r.render(d, 200, 100, Affine::IDENTITY, &opts).pixels;
+    let mut r = Renderer::new();
+    let before = frame(&mut r, &d);
+    session().execute("edit.colorSettings", &json!({"cmyk": cms::DEVICE_CMYK})).unwrap();
+    let warm = frame(&mut r, &d);
+    let fresh = std::thread::scope(|t| t.spawn(|| frame(&mut Renderer::new(), &d)).join().unwrap());
+    cms::set_active(&old).unwrap();
+    assert!(fresh != before, "the new profile changes the frame");
+    assert!(warm == fresh, "drawn as a renderer with no tiles draws it");
+}
+
 /// Live objects follow the Color Settings: in the next frame, a blend of an RGB red and a CMYK cyan,
 /// the same blend in an envelope, an envelope that distorts the appearance of a CMYK cyan rectangle
 /// with Revolve, and a gradient mesh in CMYK colors draw through the new working CMYK profile, and

crates/render/src/pattern.rs

@@ -7,8 +7,9 @@
 //! 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", 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`]).
+//! when its art shows more than itself (a symbol, another pattern, an image, type, a brush), while
+//! what the document holds of those is what it held then ([`Deps`]), and while the color settings
+//! are the ones it was drawn through.
 //!
 //! 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.
@@ -17,6 +18,7 @@ use std::cell::RefCell;
 use std::hash::{Hash, Hasher};
 use std::sync::{Arc, Weak};
 
+use vectorcraft_color::cms::Cms;
 use vectorcraft_doc::pattern::{Overlap, PatternDef, TileType};
 use vectorcraft_doc::{Document, Node};
 use vectorcraft_geom::{Affine, Rect, Shape};
@@ -35,13 +37,17 @@ const MAX_TILE_PX: f64 = 2048.0;
 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.
+/// the art of every pattern and symbol and the pixels of every image in the document, the
+/// document's other data (which holds its brushes), 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<Weak<Node>>,
     pixels: Vec<Weak<Vec<u8>>>,
+    /// shortcut: any change to the document's other data draws the tile again, not only a brush
+    /// edit; compare the brush library alone if that ever costs a frame.
+    unknown: Weak<vectorcraft_brush::serde_json::Map<String, vectorcraft_brush::serde_json::Value>>,
     fonts: u64,
 }
 
@@ -58,6 +64,7 @@ impl Deps {
         Self {
             art: Self::art(doc).map(Arc::downgrade).collect(),
             pixels: Self::pixels(doc).map(Arc::downgrade).collect(),
+            unknown: vectorcraft_doc::Shared::downgrade(&doc.unknown),
             fonts: vectorcraft_text::FontDb::global().generation(),
         }
     }
@@ -65,6 +72,7 @@ impl Deps {
     /// Does `doc` hold what the tile was drawn from?
     fn current(&self, doc: &Document) -> bool {
         self.fonts == vectorcraft_text::FontDb::global().generation()
+            && std::ptr::eq(self.unknown.as_ptr(), &*doc.unknown)
             && 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))
     }
@@ -74,6 +82,8 @@ struct TileEntry {
     art: Vec<Arc<Node>>,
     /// `None` for art that shows only itself.
     deps: Option<Deps>,
+    /// The color settings the tile was drawn through, held weakly as [`Deps`] holds its values.
+    cms: Weak<Cms>,
     params: (Rect, TileType, Overlap),
     sx: f64,
     sy: f64,
@@ -199,13 +209,19 @@ pub(crate) fn set_pattern_paint(ctx: &mut RenderContext, name: &str, xf: Affine,
     let scale = raster_scale(s, (pw, ph));
     let params = (def.tile, def.tile_type, def.overlap);
     let key = (name.to_string(), scale.to_bits());
+    let cms = vectorcraft_color::cms::active();
     let hit = CACHE.with(|c| {
         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) && e.deps.as_ref().is_none_or(|d| d.current(doc)))
+            .filter(|e| {
+                e.params == params
+                    && same_art(&e.art, &def.art)
+                    && std::ptr::eq(e.cms.as_ptr(), Arc::as_ptr(&cms))
+                    && 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 {
@@ -224,7 +240,7 @@ 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, bytes) = (tiles.tick(), size_of::<TileEntry>() + w as usize * h as usize * 4);
-                tiles.insert(key, TileEntry { art: def.art.clone(), deps, params, sx, sy, pixmap: pm.clone() }, bytes, t);
+                tiles.insert(key, TileEntry { art: def.art.clone(), deps, cms: Arc::downgrade(&cms), params, sx, sy, pixmap: pm.clone() }, bytes, t);
             });
             (pm, sx, sy)
         }

crates/render/src/tests_memory.rs

@@ -941,8 +941,8 @@ 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.
+/// doesn't: a symbol an instance in it shows, another pattern it is painted with, or a brush it is
+/// stroked 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();
@@ -981,6 +981,20 @@ fn pattern_tiles_draw_edits_to_what_their_art_shows() {
     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);
+    // Pattern `Brushed` is a line stroked with a calligraphic brush; the brush gets wider.
+    let mut d = Document::new(200.0, 100.0);
+    let mut stroke = vectorcraft_doc::Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 1.0);
+    stroke.stroke_mut().unwrap().brush = Some("3 pt. Round".into());
+    let brushed = Node::path(d.alloc_id(), vectorcraft_geom::shapes::line((2.0, 10.0).into(), (18.0, 10.0).into()), stroke);
+    d.patterns.push(tile("Brushed", brushed));
+    let painted = filled(&mut d, rectangle(Rect::new(10.0, 10.0, 190.0, 90.0)), pattern_paint("Brushed"), vec![]);
+    edit_draws_as_new(with_art(d, vec![painted]), |d| {
+        let mut lib = vectorcraft_brush::library(d);
+        if let vectorcraft_brush::BrushKind::Calligraphic(c) = &mut lib[0].kind {
+            c.size = 12.0;
+        }
+        vectorcraft_brush::store(d, &lib);
+    });
 }
 
 /// Raster effects are made again when a symbol shown inside another symbol changes (a group's drop

docs/mcp.md

@@ -2124,7 +2124,9 @@ the Actions panel leaves the date out: playing it later dates the document then)
 command records the date it used (and any value it takes from the preferences) in its step of the batch's journal entry.
 A batch may open, switch, close or revert documents: each document it edits gets one undo step, the batch is
 journaled whichever document it ends in, and an error rolls every document back (the ones it opened close, the ones
-it closed or reverted come back).
+it closed or reverted come back). The rollback puts the documents back, not the disk: files its steps wrote (a save, a
+Save As, an export) stay written, new files included, as Undo leaves a saved file on disk. A caller that wants nothing
+written unless the whole batch succeeds puts the saves and exports last, or runs them after the batch.
 
 ```json
 {"name":"run_command","arguments":{"command":"file.info","params":{"author":"Ada","keywords":"poster, fair","rating":4,"copyrightStatus":"copyrighted","copyrightNotice":"© 2026 Ada"}}}