diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 1bfd30642..682895abc 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -439,6 +439,24 @@ fn a_painted_effect_on_a_fill_and_refused_results() { } } +/// A result refused for a swatch the document doesn't have stays cached: once the document has the +/// swatch, the next check of that result clears the error. +#[test] +fn a_refused_swatch_s_error_clears_once_the_document_has_the_swatch() { + let mut s = session(); + let id = rect(&mut s, 10.0, "#ff0000"); + let mut later = solid("#00ff00"); + later["swatch"] = json!("Later"); + let output = json!({"objects": [{"path": square_at(40.0), "fills": [later]}]}).to_string(); + install_wat(&mut s, &tpl::returning(&painting("test.engine.laterswatch", true), &output)); + run(&mut s, "effect.apply", json!({"ids": [id.0], "effect": "plugin.test.engine.laterswatch"})); + drawn(&s); + assert!(run(&mut s, "plugin.info", json!({"id": "test.engine.laterswatch"}))["lastError"].is_string()); + run(&mut s, "swatch.new", json!({"name": "Later", "color": "#00ff00", "global": true})); + drawn(&s); + assert_eq!(run(&mut s, "plugin.info", json!({"id": "test.engine.laterswatch"}))["lastError"], Value::Null); +} + /// A painted effect on a group may link its pieces to the document's swatches. #[test] fn a_painted_group_effect_links_to_the_document_s_swatches() { diff --git a/crates/plugins/src/effect.rs b/crates/plugins/src/effect.rs index 0eded4070..fee13330e 100644 --- a/crates/plugins/src/effect.rs +++ b/crates/plugins/src/effect.rs @@ -174,7 +174,23 @@ fn weight(key: &Key, out: Option<&Out>) -> usize { }; anchors(&key.path) + out + (key.params.len() + key.paints.len()) / 50 + 1 } -static ERRORS: Mutex>> = Mutex::new(None); +static ERRORS: Mutex>> = Mutex::new(None); + +/// The errors recorded for a live-effect plug-in, one for each kind of outcome that can clear it. +#[derive(Default)] +struct Failures { + /// From a run of the plug-in. + run: Option, + /// From checking the swatches and patterns a painted result names against the document. + references: Option, +} + +/// Which of a plug-in's [`Failures`] an outcome replaces. +#[derive(Clone, Copy)] +enum Origin { + Run, + References, +} /// Where an installation ([`crate::Plugin::serial`]) stands as a live effect in this process. enum Runs { @@ -227,21 +243,30 @@ fn end_run(serial: u64, cancelled: bool) { } } -/// The last error of live-effect plug-in `id` (cleared when it next succeeds). +/// The last error of live-effect plug-in `id`: a refused reference if there is one, else a failed +/// run's. A run that succeeds clears a failed run's error, and a reference check that passes clears +/// a refused reference's. pub fn last_error(id: &str) -> Option { - ERRORS.lock().unwrap_or_else(|e| e.into_inner()).as_ref()?.get(id).cloned() + let errors = ERRORS.lock().unwrap_or_else(|e| e.into_inner()); + let failures = errors.as_ref()?.get(id)?; + failures.references.clone().or_else(|| failures.run.clone()) } -/// Records how a run of plug-in `id` went: a failure as its last error (a painted effect's named -/// after the plug-in, [`painted::named`]), a success clearing it. -fn note(id: &str, plugin: &crate::Plugin, r: std::result::Result<(), &Error>) { +/// Records how a run of plug-in `id`, or a check of what its result names, went: a failure as that +/// origin's error (a painted effect's named after the plug-in, [`painted::named`]), a success +/// clearing that origin's error only. +fn note(id: &str, plugin: &crate::Plugin, origin: Origin, r: std::result::Result<(), &Error>) { + let message = r.err().map(|e| if plugin.manifest().paint_output { painted::named(&plugin.manifest().name, e) } else { e.to_string() }); let mut errors = ERRORS.lock().unwrap_or_else(|e| e.into_inner()); let errors = errors.get_or_insert_with(HashMap::new); - match r { - Ok(()) => errors.remove(id), - Err(e) if plugin.manifest().paint_output => errors.insert(id.to_string(), painted::named(&plugin.manifest().name, e)), - Err(e) => errors.insert(id.to_string(), e.to_string()), - }; + let failures = errors.entry(id.to_string()).or_default(); + match origin { + Origin::Run => failures.run = message, + Origin::References => failures.references = message, + } + if failures.run.is_none() && failures.references.is_none() { + errors.remove(id); + } } /// Does live-effect plug-in `id` return painted pieces (its manifest declares `paintOutput`)? @@ -294,7 +319,7 @@ pub fn apply_painted( fn references_found(id: &str, plugin: &crate::Plugin, pieces: &[Piece], doc: Option<&Document>) -> Option<()> { let checked = painted::check_references(pieces, doc); if doc.is_some() { - note(id, plugin, checked.as_ref().map(|_| ())); + note(id, plugin, Origin::References, checked.as_ref().map(|_| ())); } checked.ok() } @@ -334,7 +359,7 @@ fn evaluate(id: &str, params: &Value, path: &PathData, bounds: Rect, rule: FillR if cancelled { return None; } - note(id, &plugin, r.as_ref().map(|_| ())); + note(id, &plugin, Origin::Run, r.as_ref().map(|_| ())); let out = r.ok(); let size = weight(&key, out.as_ref()); if size > CACHE_ANCHORS / 4 { @@ -377,6 +402,7 @@ fn run(plugin: &crate::Plugin, mut params: Value, path: &PathData, bounds: Rect, mod tests { use vectorcraft_color::{Color, Paint}; use vectorcraft_doc::{FillLayer, StrokeLayer}; + use vectorcraft_geom::{Point, SubPath}; use super::*; @@ -400,6 +426,34 @@ mod tests { assert_eq!((none.fills, none.strokes, none.item), (vec![], vec![], json!("object"))); } + /// A result taken from the cache passes its reference check without clearing the error of a + /// failed run, and the next run that succeeds clears that error. + #[test] + fn a_passed_reference_check_keeps_a_failed_run_s_error() { + let _lock = crate::safe_mode::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + crate::safe_mode::set_active(None); + const ID: &str = "test.effect.lasterror"; + let manifest = format!(r#"{{"id":"{ID}","name":"Test","version":"1","kind":"effect","api":2,"paintOutput":true,"params":{{}}}}"#); + // Traps on a long input. + let first = "(if (i32.gt_u (local.get $il) (i32.const 4096)) (then unreachable))"; + let src = crate::wat::with_imports(&manifest, "", first, r#"{"objects":[]}"#); + registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap()); + let doc = Document::new(100.0, 100.0); + let line = |n: usize| PathData::single(SubPath::polyline(&(0..n).map(|i| Point::new(i as f64, 0.0)).collect::>(), false)); + let draw = + |path: &PathData| apply_painted(ID, &json!({}), path, Rect::new(0.0, 0.0, 1.0, 1.0), FillRule::NonZero, Paints::default(), Some(&doc)); + let short = line(2); + assert!(draw(&short).is_some()); + assert_eq!(last_error(ID), None); + assert!(draw(&line(1000)).is_none()); + let failed = last_error(ID).unwrap(); + assert!(draw(&short).is_some(), "from the cache"); + assert_eq!(last_error(ID), Some(failed)); + assert!(draw(&line(3)).is_some(), "a new run"); + assert_eq!(last_error(ID), None); + registry::remove(ID); + } + /// A first run that is cancelled leaves the next run to be the first: the launch marker names /// the plug-in during that one. #[test] diff --git a/docs/plugins.md b/docs/plugins.md index 351c139cb..d2473ba33 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -233,7 +233,9 @@ Every run gets a **fresh instance**, so no state survives between calls except w [settings and cache](#settings-and-cache). Traps, exhausted budgets, malformed modules, bad manifests, out-of-range pointers, negative return codes and invalid output all become errors, and the document is left untouched: a filter's output is checked completely before one edit applies it. A live effect that -fails draws the object's geometry unchanged; `plugin.info` reports why (`lastError`). +fails draws the object's geometry unchanged; `plugin.info` reports why (`lastError`). A run that succeeds clears the +error of a failed run. A check that finds every swatch and pattern a painted result names clears only the error of a +refused swatch or pattern. wasmi is built with its *portable* (loop) dispatcher: the default tail-call dispatcher relies on LLVM sibling-call optimisation, and when that doesn't happen a long-running module overflows the host stack, which aborts