Merge pull request #6495 from dotCooCoo/fix/canvas-redraws-on-plugin-change

Canvas: installing or removing a plug-in draws its effects again

crates/render/src/lib.rs

@@ -283,6 +283,7 @@ use frame::{
 #[cfg(test)]
 use frame::{OFFSCREEN_PIXELS, Slices};
 pub use geometry::region_pixels;
+pub use node_checks::draws_plugin_effects;
 use node_checks::holds_plugin_effect;
 #[cfg(test)]
 use node_checks::painted_items;

crates/render/src/node_checks.rs

@@ -1,4 +1,4 @@
-use super::{Affine, AppearanceItem, Arc, BezPath, Node, NodeKind, StrokeLayer, brush_fx, effects};
+use super::{Affine, AppearanceItem, Arc, BezPath, Document, Node, NodeKind, StrokeLayer, brush_fx, effects};
 
 /// Most pixels from the origin that a shape handed to the rasterizer may reach, or a stroke be
 /// wide. A document's art stays within 4 million points, 2.6 billion pixels at the closest zoom.
@@ -142,6 +142,16 @@ pub(crate) fn holds_plugin_effect(n: &Node) -> bool {
     holds
 }
 
+/// Does `doc` draw a plug-in effect, so that installing or removing a plug-in
+/// ([`effects::plugin_revision`]) can change how it draws? It does when an object in its layers, in
+/// the art of its symbols or patterns, or in the art of its brushes holds one ([`holds_plugin_effect`]).
+pub fn draws_plugin_effects(doc: &Document) -> bool {
+    let symbols = doc.symbols.iter().map(|s| &s.art);
+    let patterns = doc.patterns.iter().flat_map(|p| &p.art);
+    doc.layers.iter().chain(symbols).chain(patterns).any(|a| holds_plugin_effect(a))
+        || vectorcraft_brush::library(doc).iter().any(|b| b.art().into_iter().any(holds_plugin_effect))
+}
+
 /// A symbol instance's stain ([`vectorcraft_brush::stain`]) as bits for a cache key: its color
 /// model, the color's components and the amount (all 0 without a stain).
 pub(crate) fn stain_bits(stain: Option<(vectorcraft_color::Color, f32)>) -> [u32; 6] {

crates/render/src/tests/memory.rs

@@ -1824,6 +1824,44 @@ fn pattern_and_symbol_art_with_plugin_effects_follow_the_plugins_installed() {
     plugin_change_draws_as_new(with_art(d, vec![group]), ID, Some(triangle(82.0, 32.0, 10.0)), Some(triangle(82.0, 32.0, 30.0)));
 }
 
+/// A document draws plug-in effects ([`draws_plugin_effects`]) when an object in its layers, in a
+/// symbol's or a pattern's art, or in a brush's art has a visible plug-in effect, and not when its
+/// objects have no effect, another effect or a hidden plug-in effect.
+#[test]
+fn a_document_draws_plugin_effects_where_any_of_its_art_has_one() {
+    use vectorcraft_brush::{ArtBrush, Brush, BrushKind};
+    use vectorcraft_geom::shapes::rectangle;
+    let plugin = || effect("plugin.test.render.draws", serde_json::json!({}));
+    let square = |d: &mut Document, effects: Vec<vectorcraft_doc::Effect>| {
+        filled(d, rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)), Paint::solid(Color::BLACK), effects)
+    };
+    let mut cases: Vec<(&str, Document, bool)> = vec![];
+    for (name, effects, draws) in [
+        ("no effect", vec![], false),
+        ("another effect", vec![drop_shadow()], false),
+        ("hidden plug-in effect", vec![vectorcraft_doc::Effect { visible: false, ..plugin() }], false),
+        ("plug-in effect", vec![plugin()], true),
+    ] {
+        let mut d = Document::new(200.0, 100.0);
+        let n = square(&mut d, effects);
+        cases.push((name, with_art(d, vec![n]), draws));
+    }
+    let mut d = Document::new(200.0, 100.0);
+    let art = square(&mut d, vec![plugin()]);
+    d.symbols.push(vectorcraft_doc::Symbol { dynamic: false, name: "Shaped".into(), art: Arc::new(art) });
+    cases.push(("symbol art", d, true));
+    let mut d = Document::new(200.0, 100.0);
+    let art = square(&mut d, vec![plugin()]);
+    d.patterns.push(vectorcraft_doc::pattern::PatternDef::new("Shaped", vec![Arc::new(art)]));
+    cases.push(("pattern art", d, true));
+    let mut d = Document::new(200.0, 100.0);
+    let art = square(&mut d, vec![plugin()]);
+    vectorcraft_brush::store(&mut d, &[Brush { name: "Shaped".into(), kind: BrushKind::Art(ArtBrush { art, ..Default::default() }) }]);
+    cases.push(("brush art", d, true));
+    let wrong: Vec<&str> = cases.iter().filter(|(_, d, draws)| draws_plugin_effects(d) != *draws).map(|(name, ..)| *name).collect();
+    assert!(wrong.is_empty(), "{wrong:?}");
+}
+
 fn effect(id: &str, params: serde_json::Value) -> vectorcraft_doc::Effect {
     vectorcraft_doc::Effect { id: id.into(), params, visible: true }
 }

crates/ui-egui/src/app.rs

@@ -88,6 +88,7 @@ impl VectorcraftApp {
                 selection_box: None,
                 selection_bounds: None,
                 content_bounds: None,
+                plugin_keys: Default::default(),
                 cursors: Default::default(),
                 view_moved: None,
                 hover_cursor: None,

crates/ui-egui/src/caches.rs

@@ -58,6 +58,9 @@ pub struct CanvasCache {
     pub selection_bounds: Option<((u64, u64), Option<vectorcraft_geom::Rect>)>,
     /// [`VectorcraftApp::content_bounds`] for (document uid, revision).
     pub content_bounds: Option<((u64, u64), Option<vectorcraft_geom::Rect>)>,
+    /// [`CacheKey::plugins`] by document uid, with the plug-in revision
+    /// ([`vectorcraft_render::effects::plugin_revision`]) it was last checked at.
+    pub plugin_keys: std::collections::HashMap<u64, (u64, u64)>,
     /// The tools' cursors as OS cursor bitmaps.
     pub cursors: cursors::Images,
     /// The view the canvas last drew and when it last changed (egui's input time): while it moves,
@@ -119,6 +122,11 @@ pub struct CacheKey {
     pub highlight_substituted_fonts: bool,
     /// [`vectorcraft_render::placed_document::generation`]: placed documents' bitmaps made since.
     pub placed: u64,
+    /// The plug-in revision ([`vectorcraft_render::effects::plugin_revision`]) of the last plug-in
+    /// install or removal after which the document drew a plug-in effect
+    /// ([`vectorcraft_render::draws_plugin_effects`]): a frame drawn before it draws those effects
+    /// as the plug-ins installed then did. 0 until then.
+    pub plugins: u64,
     /// View › Pixel Preview: the document pixels rendered (x0, y0, x1, y1), one per point, shown
     /// with hard edges. None: the art is rendered for the screen.
     pub pixel: Option<[i64; 4]>,

crates/ui-egui/src/canvas.rs

@@ -316,7 +316,8 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
     // File Handling › Display Bitmaps as Anti-aliased Images in Pixel Preview: off, images show
     // their pixels as they rasterize, hard-edged.
     let smooth_images = pixel.is_none() || app.session.prefs.anti_aliased_bitmaps;
-    let key = cache_key(app, ui, st, &t, &v, (w, h, ppp), (pixel, smooth_images));
+    let plugins = plugin_key(&mut app.canvas.plugin_keys, &app.session, st);
+    let key = cache_key(app, ui, st, &t, &v, (w, h, ppp), (pixel, smooth_images, plugins));
     // Placed documents' bitmaps are being made: draw again when they are ready.
     if vectorcraft_render::placed_document::busy() {
         ui.ctx().request_repaint_after(std::time::Duration::from_millis(30));
@@ -565,7 +566,7 @@ fn cache_key(
     t: &Tokens,
     v: &View,
     (w, h, ppp): (u32, u32, f32),
-    (pixel, smooth_images): (Option<[i64; 4]>, bool),
+    (pixel, smooth_images, plugins): (Option<[i64; 4]>, bool, u64),
 ) -> CacheKey {
     // Paths made into guides are drawn as the ruler guides are (Preferences › Guides & Grid).
     let guide_look = LineLook::guides(&app.session.prefs, t);
@@ -590,6 +591,7 @@ fn cache_key(
         highlight_alternate_glyphs: app.session.prefs.highlight_alternate_glyphs,
         highlight_substituted_fonts: app.session.prefs.highlight_substituted_fonts,
         placed: vectorcraft_render::placed_document::generation(),
+        plugins,
         pixel,
         smooth_images,
         isolated: st.isolation.map(|id| id.0),
@@ -602,6 +604,26 @@ fn cache_key(
     }
 }
 
+/// [`CacheKey::plugins`] of document `st`, one of the documents open in `session`. `keys` holds each
+/// document's value and the plug-in revision it was checked at, so a document is looked through only
+/// once after each plug-in install or removal.
+fn plugin_key(
+    keys: &mut std::collections::HashMap<u64, (u64, u64)>,
+    session: &vectorcraft_engine::Session,
+    st: &vectorcraft_engine::DocState,
+) -> u64 {
+    let revision = vectorcraft_render::effects::plugin_revision();
+    match keys.get(&st.uid) {
+        Some(&(checked, key)) if checked == revision => key,
+        seen => {
+            let key = if vectorcraft_render::draws_plugin_effects(&st.doc) { revision } else { seen.map_or(0, |&(_, key)| key) };
+            keys.retain(|uid, _| session.documents().iter().any(|d| d.uid == *uid));
+            keys.insert(st.uid, (revision, key));
+            key
+        }
+    }
+}
+
 /// Start the render worker, and upload the frame it finished.
 fn poll_worker(app: &mut VectorcraftApp, ui: &Ui, key_doc: usize) {
     if !app.canvas.worker_started {

crates/ui-egui/src/canvas/panes.rs

@@ -12,8 +12,8 @@ use super::outline::{c32, node_outline, stroke_path, too_many_anchors};
 use super::view::fit_zoom;
 use super::{
     Affine, CacheKey, Point, Rect, RulerScale, Sense, Stroke, StrokeKind, Tokens, Ui, VectorcraftApp, View, Xf, artboard_edges, cache_key,
-    dim_on_canvas, frame_options, frame_view, grid, guides_of, json, paint_paper, pixel_region, pos2, rulers, shows_artboards, textured_quad, vec2,
-    view_rects,
+    dim_on_canvas, frame_options, frame_view, grid, guides_of, json, paint_paper, pixel_region, plugin_key, pos2, rulers, shows_artboards,
+    textured_quad, vec2, view_rects,
 };
 use crate::doc_windows::Win;
 use crate::thumb_worker::ThumbWorker;
@@ -303,7 +303,8 @@ pub(crate) fn pane(app: &mut VectorcraftApp, ui: &Ui, w: Win, cell: egui::Rect)
     // View › Pixel Preview, as the canvas draws it.
     let pixel = (app.ui.view.pixel_preview && v.zoom * ppp as f64 > 1.0).then(|| pixel_region(&xf)).flatten();
     let smooth_images = pixel.is_none() || app.session.prefs.anti_aliased_bitmaps;
-    let mut key = cache_key(app, ui, st, &t, &v, (pw, ph, ppp), (pixel, smooth_images));
+    let plugins = plugin_key(&mut app.canvas.plugin_keys, &app.session, st);
+    let mut key = cache_key(app, ui, st, &t, &v, (pw, ph, ppp), (pixel, smooth_images, plugins));
     // The current document's drag and Image Trace hold are not this document's, and this view has
     // its own Outline or Preview.
     (key.dragging, key.trace_source, key.outline) = (false, None, v.outline);

crates/ui-egui/src/canvas/tests.rs

@@ -10590,6 +10590,48 @@ fn a_whole_pixel_pan_copies_the_pixels_that_stay_on_screen() {
     }
 }
 
+/// Installing, replacing or removing a plug-in draws a document with a plug-in effect again, and a
+/// pan from the frame drawn before the change renders whole instead of copying its pixels. The key
+/// of a document without a plug-in effect does not follow the plug-ins. The other tests share the
+/// plug-in registry, and the plug-in is removed after.
+#[test]
+fn a_plug_in_change_draws_the_document_s_plug_in_effects_again() {
+    use vectorcraft_plugins::wat as tpl;
+    let id = "test.canvas.redraw";
+    let module = vectorcraft_format::base64_encode(&wat::parse_str(tpl::module(&tpl::manifest(id, "effect", "{}"), tpl::ECHO, "", 1)).unwrap());
+    let mut app = VectorcraftApp::new(Session::new(), Default::default());
+    app.run("plugin.install", json!({"dataBase64": module})).unwrap();
+    app.session.execute("file.new", &json!({"width": 256, "height": 128})).unwrap();
+    let rect = app.run("shape.rectangle", json!({"x": 16, "y": 16, "width": 64, "height": 96})).unwrap()["id"].clone();
+    app.canvas.worker_started = true; // render on this thread
+    let ctx = egui::Context::default();
+    shapes(&mut app, &ctx);
+    // Other keys' fields (placed documents' bitmaps) follow what other tests do; this one's don't.
+    let plugins = |app: &VectorcraftApp| app.canvas.key.as_ref().expect("a frame was rendered").plugins;
+    let plain = plugins(&app);
+    app.run("plugin.install", json!({"dataBase64": module})).unwrap();
+    shapes(&mut app, &ctx);
+    assert_eq!(plugins(&app), plain, "no plug-in effect, so nothing to draw again");
+    app.run("effect.apply", json!({"effect": vectorcraft_plugins::effect::effect_id(id), "ids": [rect]})).unwrap();
+    shapes(&mut app, &ctx);
+    let before = app.canvas.key.clone().expect("a frame was rendered");
+    app.run("plugin.install", json!({"dataBase64": module})).unwrap();
+    shapes(&mut app, &ctx);
+    let after = plugins(&app);
+    assert_ne!(after, before.plugins, "drawn again once the plug-in was replaced");
+    // The worker's frame from before the change gives no pixels to the next pan.
+    let doc = app.session.active().unwrap().doc.clone();
+    let at_100 = CacheKey { zoom: 1.0, rot: 0.0, ppp: 1.0, pixel: None, ..before };
+    let mut r = vectorcraft_render::Renderer::new();
+    let mut kept = None;
+    render_job(&mut r, &mut kept, &pan_job(&doc, &at_100, (0.0, 0.0)));
+    let pan = pan_job(&doc, &CacheKey { plugins: after, ..at_100.clone() }, (10.0, 0.0));
+    assert!(!render_job(&mut r, &mut kept, &pan).reused, "copied from the frame before the change");
+    app.run("plugin.remove", json!({"id": id})).unwrap();
+    shapes(&mut app, &ctx);
+    assert_ne!(plugins(&app), after, "drawn again once the plug-in was removed");
+}
+
 /// A frame does not copy from the last one when the view moves by part of a pixel or over half the
 /// frame, when copying is not allowed, or when a listed key field, render option, zoom, rotation,
 /// canvas size or device scale differs. Each such frame has the job's size. The session test
@@ -10643,6 +10685,7 @@ fn a_frame_renders_whole_when_anything_but_a_whole_pixel_pan_changed() {
         ("outline key", other_key(&|k| k.outline = true)),
         ("Pixel Preview", other_key(&|k| k.pixel = Some([0, 0, 256, 128]))),
         ("placed bitmaps", other_key(&|k| k.placed += 1)),
+        ("plug-ins", other_key(&|k| k.plugins += 1)),
         ("outline", other_opts(&|o| o.outline = true)),
         ("proof", other_opts(&|o| o.proof = Some(Default::default()))),
         ("overprint preview", other_opts(&|o| o.overprint_preview = true)),