Merge pull request #5599 from dotCooCoo/fix/twist-format-migration

Native files: documents from before 0.21.0 keep their Twist turning as saved

crates/doc/src/lib.rs

@@ -68,6 +68,7 @@ mod symbols;
 pub use symbols::SymbolBoxes;
 pub mod text;
 pub mod trace;
+pub mod twist;
 pub mod variables;
 pub mod xml_id;
 

crates/doc/src/limits.rs

@@ -13,7 +13,9 @@ pub enum DocError {
     Invalid(String),
 }
 
-pub const FORMAT_VERSION: u32 = 1;
+/// The document version new documents get. Version 2: a positive Twist angle turns clockwise
+/// ([`crate::twist`]).
+pub const FORMAT_VERSION: u32 = 2;
 
 /// Node, slice and asset ids are handed out below this: no document gets near it by counting, so
 /// larger ids in a file are damage (or made by hand) and the counter never follows them (#1718).

crates/doc/src/style_libs.rs

@@ -98,6 +98,10 @@ impl GraphicStyle {
 /// The extension (and `format` header) of graphic style library files.
 pub const STYLES_EXT: &str = "vcstyles";
 
+/// The version of the `.vcstyles` files [`write`] writes. Version 2: a positive Twist angle turns
+/// clockwise ([`crate::twist`]).
+const STYLES_VERSION: u32 = crate::twist::TWIST_CLOCKWISE_SINCE;
+
 /// The file: a header around the library.
 #[derive(Serialize, Deserialize)]
 struct NativeFile {
@@ -109,17 +113,21 @@ struct NativeFile {
 
 /// `lib` as a `.vcstyles` file.
 pub fn write(lib: &StyleLibrary) -> String {
-    let file = NativeFile { format: STYLES_EXT.into(), version: 1, library: lib.clone() };
+    let file = NativeFile { format: STYLES_EXT.into(), version: STYLES_VERSION, library: lib.clone() };
     serde_json::to_string_pretty(&file).unwrap_or_default()
 }
 
-/// Read a `.vcstyles` file; an unnamed library is called `name`.
+/// Read a `.vcstyles` file; an unnamed library is called `name`. A version 1 file was written by
+/// an app that turned Twist counterclockwise: its Twist angles are negated.
 pub fn read(text: &str, name: &str) -> Result<StyleLibrary, String> {
     let f: NativeFile = serde_json::from_str(text.trim_start_matches('\u{feff}')).map_err(|e| format!("not a graphic style library: {e}"))?;
     if f.format != STYLES_EXT {
         return Err(format!("not a graphic style library (format `{}`)", f.format));
     }
     let mut lib = f.library;
+    if f.version < STYLES_VERSION {
+        crate::twist::negate_library(&mut lib);
+    }
     if lib.name.trim().is_empty() {
         lib.name = name.into();
     }

crates/doc/src/twist.rs

@@ -0,0 +1,231 @@
+//! The Twist effect's angle in saved documents. A positive angle turns clockwise on the page in
+//! VectorCraft 0.21.0 and later; earlier releases turned it counterclockwise. Documents of version
+//! 2 ([`TWIST_CLOCKWISE_SINCE`]) turn clockwise. Version 1 documents written before 0.21.0 have
+//! their Twist angles negated when they open, so they draw as they were saved
+//! ([`Document::migrate_twist_angles`], [`written_clockwise`]), and a save for an earlier app
+//! negates them back ([`Document::store_legacy_twist_angles`]). Graphic style library files
+//! (`.vcstyles`) of version 1 have their Twist angles negated when they are read
+//! ([`crate::style_libs::read`]); [`crate::style_libs::write`] writes version 2.
+
+use std::sync::Arc;
+
+use serde_json::{Map, Value, json};
+
+use crate::style_libs::StyleLibrary;
+use crate::{Appearance, Document, Effect, Node};
+
+/// The Twist effect's id.
+pub const TWIST: &str = "distort.twist";
+
+/// The first document version whose Twist angles turn clockwise for a positive value.
+pub const TWIST_CLOCKWISE_SINCE: u32 = 2;
+
+/// The first release (major, minor) whose Twist turned clockwise for a positive angle.
+const CLOCKWISE_RELEASE: (u64, u64) = (0, 21);
+
+/// Did the app named by a native file's `generator` (`"VectorCraft 0.21.0"`) turn Twist effects
+/// clockwise for a positive angle? It did from 0.21.0. Any other value, or none, is an earlier
+/// app (DrawCraft included).
+pub fn written_clockwise(generator: Option<&str>) -> bool {
+    let Some(version) = generator.and_then(|g| g.strip_prefix("VectorCraft ")) else { return false };
+    let mut parts = version.split('.').map(|p| p.parse::<u64>().ok());
+    match (parts.next().flatten(), parts.next().flatten()) {
+        (Some(major), Some(minor)) => (major, minor) >= CLOCKWISE_RELEASE,
+        _ => false,
+    }
+}
+
+/// The angle a Twist effect without a readable one draws with (`vectorcraft-effects` reads a
+/// missing or unreadable `angle` as this).
+const DEFAULT_ANGLE: f64 = 10.0;
+
+/// The key of `Document::unknown` that holds the brush definitions (`vectorcraft-brush`'s
+/// `DOC_KEY`), whose art can carry effects.
+const BRUSHES_KEY: &str = "brushes";
+
+/// The angle a Twist effect with `params` draws with, read as `vectorcraft-effects` reads it: a
+/// number, the number a string starts with, or a boolean as 1 or 0; anything else (or a value
+/// that is not finite) is [`DEFAULT_ANGLE`].
+fn angle(params: &Value) -> f64 {
+    let v = match params.get("angle") {
+        Some(Value::Number(n)) => n.as_f64(),
+        Some(Value::String(s)) => {
+            let t: String = s.trim().chars().take_while(|c| c.is_ascii_digit() || matches!(c, '.' | '-' | '+' | 'e' | 'E')).collect();
+            t.parse().ok()
+        }
+        Some(Value::Bool(b)) => Some(if *b { 1.0 } else { 0.0 }),
+        _ => None,
+    };
+    v.filter(|v| v.is_finite()).unwrap_or(DEFAULT_ANGLE)
+}
+
+/// Write the negated angle a Twist effect with `params` draws with into `params`, as a number.
+fn negate_params(params: &mut Value) {
+    // `0.0 - a` rather than `-a`: an angle of 0 stays 0, not -0.
+    let turned = Value::from(0.0 - angle(params));
+    match params {
+        Value::Object(m) => {
+            m.insert("angle".into(), turned);
+        }
+        p => *p = json!({ "angle": turned }),
+    }
+}
+
+fn has_twist(fx: &[Effect]) -> bool {
+    fx.iter().any(|e| e.id == TWIST)
+}
+
+fn appearance_has_twist(a: &Appearance) -> bool {
+    has_twist(&a.effects) || a.items.iter().any(|i| has_twist(i.effects()))
+}
+
+fn negate_appearance(a: &mut Appearance) {
+    for e in a.effects.iter_mut().chain(a.items.iter_mut().flat_map(|i| i.effects_mut().iter_mut())) {
+        if e.id == TWIST {
+            negate_params(&mut e.params);
+        }
+    }
+}
+
+/// The appearances a Shaper Group's face and edge edits paint with.
+fn shaper_appearances(n: &Node) -> impl Iterator<Item = &Appearance> {
+    let s = n.extras().shaper.as_deref();
+    let faces = s.into_iter().flat_map(|s| s.faces.iter().filter_map(|f| f.paint.as_ref()));
+    let edges = s.into_iter().flat_map(|s| s.edges.iter().filter_map(|e| e.paint.as_ref()));
+    faces.chain(edges)
+}
+
+/// Does `n`'s subtree (opacity-mask art and Shaper edits included) hold a Twist effect?
+fn node_has_twist(n: &Node) -> bool {
+    appearance_has_twist(&n.appearance)
+        || n.extras().mask.as_ref().is_some_and(|m| node_has_twist(&m.art))
+        || shaper_appearances(n).any(appearance_has_twist)
+        || n.children().is_some_and(|c| c.iter().any(|c| node_has_twist(c)))
+}
+
+/// Negate the Twist angles of `a`'s subtree, copying only the nodes on the way to them.
+fn negate_node(a: &mut Arc<Node>) {
+    if !node_has_twist(a) {
+        return;
+    }
+    let n = Arc::make_mut(a);
+    if appearance_has_twist(&n.appearance) {
+        negate_appearance(&mut n.appearance);
+    }
+    if let Some(x) = n.extras_if_any() {
+        if let Some(m) = x.mask.as_mut() {
+            negate_node(&mut m.art);
+        }
+        if let Some(s) = x.shaper.as_mut() {
+            let faces = s.faces.iter_mut().filter_map(|f| f.paint.as_mut());
+            for p in faces.chain(s.edges.iter_mut().filter_map(|e| e.paint.as_mut())) {
+                negate_appearance(p);
+            }
+        }
+    }
+    for c in n.children_mut().into_iter().flatten() {
+        negate_node(c);
+    }
+}
+
+fn is_twist_json(m: &Map<String, Value>) -> bool {
+    m.get("id").and_then(Value::as_str) == Some(TWIST)
+}
+
+/// Does JSON `v` (brush definitions) hold a Twist effect?
+fn json_has_twist(v: &Value) -> bool {
+    match v {
+        Value::Object(m) => is_twist_json(m) || m.values().any(json_has_twist),
+        Value::Array(a) => a.iter().any(json_has_twist),
+        _ => false,
+    }
+}
+
+/// Negate the Twist angles in JSON `v`: every object whose `id` is [`TWIST`] is a serialized
+/// [`Effect`] (object ids are numbers).
+fn negate_json(v: &mut Value) {
+    match v {
+        Value::Object(m) if is_twist_json(m) => negate_params(m.entry("params").or_insert(Value::Null)),
+        Value::Object(m) => m.values_mut().for_each(negate_json),
+        Value::Array(a) => a.iter_mut().for_each(negate_json),
+        _ => {}
+    }
+}
+
+/// Negate every Twist angle of a graphic style library: its styles' and its patterns' art.
+pub(crate) fn negate_library(lib: &mut StyleLibrary) {
+    for g in &mut lib.styles {
+        negate_appearance(&mut g.appearance);
+    }
+    for p in &mut lib.patterns {
+        p.art.iter_mut().for_each(negate_node);
+    }
+}
+
+impl Document {
+    /// A document before version [`TWIST_CLOCKWISE_SINCE`] that holds a Twist effect: when
+    /// `written_clockwise` (asked only then; see [`written_clockwise`]) says the app that saved it
+    /// turned Twist counterclockwise, negate its Twist angles, so it draws as it was saved. A later
+    /// version is left as it is, and the document's version is not changed. It recurses through
+    /// the art, once per level of nesting.
+    pub fn migrate_twist_angles(&mut self, written_clockwise: impl FnOnce() -> bool) {
+        if self.version >= TWIST_CLOCKWISE_SINCE || !self.has_twist() || written_clockwise() {
+            return;
+        }
+        self.negate_twist_angles();
+    }
+
+    /// The reverse of [`Document::migrate_twist_angles`], for saving for an app that turned Twist
+    /// counterclockwise: negate the Twist angles and mark the document version 1. Call it on a
+    /// document whose angles turn clockwise, as every opened or new document's do, whatever
+    /// version another conversion for that save has marked it already.
+    pub fn store_legacy_twist_angles(&mut self) {
+        self.version = TWIST_CLOCKWISE_SINCE - 1;
+        if self.has_twist() {
+            self.negate_twist_angles();
+        }
+    }
+
+    /// Does any Twist effect sit where [`Document::negate_twist_angles`] negates them?
+    fn has_twist(&self) -> bool {
+        let mut roots = self
+            .layers
+            .iter()
+            .chain(self.symbols.iter().map(|s| &s.art))
+            .chain(self.graph_designs.iter().map(|g| &g.art))
+            .chain(self.patterns.iter().flat_map(|p| p.art.iter()))
+            .chain(self.pattern_edit.iter().filter_map(|pe| pe.original.as_ref()).flat_map(|p| p.art.iter()));
+        roots.any(|r| node_has_twist(r))
+            || self.graphic_styles.iter().any(|g| appearance_has_twist(&g.appearance))
+            || self.unknown.get(BRUSHES_KEY).is_some_and(json_has_twist)
+    }
+
+    /// Negate every Twist angle: in the art of the layers, symbols, graph designs, pattern
+    /// swatches and the pattern definition that pattern editing's Cancel puts back (opacity-mask
+    /// art and Shaper edits included), in the graphic styles and in the brush definitions.
+    fn negate_twist_angles(&mut self) {
+        let roots = self
+            .layers
+            .iter_mut()
+            .chain(self.symbols.iter_mut().map(|s| &mut s.art))
+            .chain(self.graph_designs.iter_mut().map(|g| &mut g.art))
+            .chain(self.patterns.iter_mut().flat_map(|p| p.art.iter_mut()))
+            .chain(self.pattern_edit.iter_mut().filter_map(|pe| pe.original.as_mut()).flat_map(|p| p.art.iter_mut()));
+        for root in roots {
+            negate_node(root);
+        }
+        if self.graphic_styles.iter().any(|g| appearance_has_twist(&g.appearance)) {
+            for g in self.graphic_styles.iter_mut() {
+                negate_appearance(&mut g.appearance);
+            }
+        }
+        if self.unknown.get(BRUSHES_KEY).is_some_and(json_has_twist)
+            && let Some(b) = self.unknown.get_mut(BRUSHES_KEY)
+        {
+            negate_json(b);
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests;

crates/doc/src/twist/tests.rs

@@ -0,0 +1,186 @@
+use std::sync::Arc;
+
+use serde_json::{Value, json};
+use vectorcraft_geom::{Point, Rect, shapes};
+
+use super::*;
+use crate::shaper::{FaceEdit, FaceSelector, ShaperSpec};
+use crate::{GraphicStyle, OpacityMask, PatternDef, Symbol};
+
+fn fx(id: &str, params: Value) -> Effect {
+    Effect { id: id.into(), params, visible: true }
+}
+
+/// An appearance with a Twist of `params` on the object and on its fill, and a Roughen.
+fn twisted_appearance(params: &Value) -> Appearance {
+    let mut a = Appearance::default_art();
+    a.effects.push(fx(TWIST, params.clone()));
+    a.effects.push(fx("distort.roughen", json!({"size": 5})));
+    a.items[0].effects_mut().push(fx(TWIST, params.clone()));
+    a
+}
+
+fn twisted(d: &mut Document, params: &Value) -> Node {
+    Node::path(d.alloc_id(), shapes::rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)), twisted_appearance(params))
+}
+
+/// A version 1 document with Twist effects of `params` everywhere one is stored: on a path, in a
+/// group, in an opacity mask's art, in a Shaper edit, in a symbol, a pattern swatch and a graphic
+/// style, and `brush` in a brush definition. Its first layer holds the art; its last has no
+/// Twist.
+fn document(params: &Value, brush: &Value) -> Document {
+    let mut d = Document::new(100.0, 100.0);
+    let l = d.layers[0].id;
+    let path = twisted(&mut d, params);
+    d.insert(Some(l), 0, path).unwrap();
+    let inner = twisted(&mut d, params);
+    let group = Node::group(d.alloc_id(), vec![Arc::new(inner)]);
+    d.insert(Some(l), 1, group).unwrap();
+    let mask_art = twisted(&mut d, params);
+    let mut masked = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(0.0, 0.0, 5.0, 5.0)), Appearance::default_art());
+    masked.extras_mut().mask = Some(Box::new(OpacityMask::new(mask_art, true)));
+    d.insert(Some(l), 2, masked).unwrap();
+    let mut shaper = Node::group(d.alloc_id(), vec![]);
+    let face = FaceEdit {
+        selector: FaceSelector { sources: vec![0], anchor: Point::ZERO },
+        erase: false,
+        erase_edges: false,
+        paint_source: None,
+        merge: None,
+        paint: Some(twisted_appearance(params)),
+    };
+    shaper.extras_mut().shaper = Some(Box::new(ShaperSpec { faces: vec![face], edges: vec![] }));
+    d.insert(Some(l), 3, shaper).unwrap();
+    let art = twisted(&mut d, params);
+    d.symbols.push(Symbol { name: "S".into(), art: Arc::new(art), dynamic: false });
+    let art = twisted(&mut d, params);
+    d.patterns.push(PatternDef::new("P", vec![Arc::new(art)]));
+    d.graphic_styles.push(GraphicStyle::new("G", twisted_appearance(params)));
+    d.unknown.insert(BRUSHES_KEY.into(), json!([{"name": "B", "art": {"id": 99, "appearance": {"effects": [brush]}}}]));
+    d.add_layer(None);
+    d.version = 1;
+    d
+}
+
+/// The angle each Twist effect of the saved document draws with, in the order the JSON lists them.
+fn angles(d: &Document) -> Vec<f64> {
+    fn walk(v: &Value, out: &mut Vec<f64>) {
+        match v {
+            Value::Object(m) => {
+                if is_twist_json(m) {
+                    out.push(angle(m.get("params").unwrap_or(&Value::Null)));
+                }
+                m.values().for_each(|c| walk(c, out));
+            }
+            Value::Array(a) => a.iter().for_each(|c| walk(c, out)),
+            _ => {}
+        }
+    }
+    let mut out = vec![];
+    walk(&serde_json::to_value(d).unwrap(), &mut out);
+    out
+}
+
+#[test]
+fn a_version_1_document_from_before_0_21_has_its_twist_angles_negated() {
+    let mut d = document(&json!({"angle": 30}), &json!({"id": TWIST}));
+    let before = angles(&d);
+    // Two per path, shaper edit and style; one in the brush.
+    assert_eq!(before.len(), 15, "{before:?}");
+    assert_eq!(before.iter().filter(|&&a| a == DEFAULT_ANGLE).count(), 1, "{before:?}");
+    d.migrate_twist_angles(|| false);
+    assert_eq!(d.version, 1, "the caller sets the version");
+    assert_eq!(angles(&d), before.iter().map(|a| -a).collect::<Vec<_>>());
+    let roughen = serde_json::to_string(&d).unwrap().matches(r#""id":"distort.roughen","params":{"size":5}"#).count();
+    assert_eq!(roughen, 7, "other effects are left alone");
+}
+
+#[test]
+fn a_document_that_turned_clockwise_is_left_as_it_is() {
+    let mut d = document(&json!({"angle": 30}), &json!({"id": TWIST, "params": {"angle": 30}}));
+    let before = d.clone();
+    d.migrate_twist_angles(|| true);
+    assert_eq!(d, before, "version 1, written by 0.21.0 or later");
+    d.version = 2;
+    let before = d.clone();
+    d.migrate_twist_angles(|| panic!("a version 2 document's writer is not asked about"));
+    assert_eq!(d, before, "version 2");
+}
+
+#[test]
+fn the_writer_is_asked_about_only_when_a_twist_is_stored() {
+    let mut d = Document::new(10.0, 10.0);
+    d.version = 1;
+    let before = d.clone();
+    d.migrate_twist_angles(|| panic!("a document without a Twist needs no answer"));
+    assert_eq!(d, before);
+}
+
+#[test]
+fn angles_are_read_as_the_effect_reads_them() {
+    let read = |params: Value| {
+        let mut e = fx(TWIST, params);
+        negate_params(&mut e.params);
+        e.params
+    };
+    assert_eq!(read(json!({"angle": 30, "x": 1})), json!({"angle": -30.0, "x": 1}));
+    assert_eq!(read(json!({"angle": "45 deg"})), json!({"angle": -45.0}));
+    assert_eq!(read(json!({"angle": true})), json!({"angle": -1.0}));
+    assert_eq!(read(json!({})), json!({"angle": -10.0}));
+    assert_eq!(read(Value::Null), json!({"angle": -10.0}));
+    let zero = read(json!({"angle": 0}));
+    assert!(zero["angle"].as_f64().is_some_and(|a| a == 0.0 && a.is_sign_positive()), "{zero}");
+}
+
+#[test]
+fn only_the_art_holding_a_twist_is_copied() {
+    let mut d = document(&json!({"angle": 30}), &json!({"id": TWIST}));
+    let plain = d.layers.last().cloned().unwrap();
+    let twisted = d.layers[0].clone();
+    d.migrate_twist_angles(|| false);
+    assert!(Arc::ptr_eq(d.layers.last().unwrap(), &plain), "a layer without a Twist is not copied");
+    assert!(!Arc::ptr_eq(&d.layers[0], &twisted));
+}
+
+#[test]
+fn storing_for_an_earlier_app_and_reading_back_round_trips() {
+    let original = document(&json!({"angle": 30.0}), &json!({"id": TWIST, "params": {"angle": -12.5}}));
+    let mut d = original.clone();
+    d.migrate_twist_angles(|| false);
+    d.version = 2;
+    let current = d.clone();
+    assert_eq!(angles(&current), angles(&original).iter().map(|a| -a).collect::<Vec<_>>());
+    d.store_legacy_twist_angles();
+    assert_eq!(d, original);
+    d.migrate_twist_angles(|| false);
+    d.version = 2;
+    assert_eq!(d, current);
+    // Another conversion for the same save may have marked the document version 1 already.
+    d.version = 1;
+    d.store_legacy_twist_angles();
+    assert_eq!(d, original);
+}
+
+#[test]
+fn a_version_1_style_library_has_its_twist_angles_negated() {
+    let lib = StyleLibrary { name: "L".into(), styles: vec![GraphicStyle::new("G", twisted_appearance(&json!({"angle": 30.0})))], patterns: vec![] };
+    let text = crate::style_libs::write(&lib);
+    assert!(text.contains(r#""version": 2"#), "{text}");
+    assert_eq!(crate::style_libs::read(&text, "x").unwrap(), lib, "a version 2 file reads as written");
+    let old = crate::style_libs::read(&text.replacen(r#""version": 2"#, r#""version": 1"#, 1), "x").unwrap();
+    let fx = &old.styles[0].appearance.effects;
+    assert_eq!(fx[0].params, json!({"angle": -30.0}));
+    assert_eq!(fx[1], lib.styles[0].appearance.effects[1]);
+    assert_eq!(old.styles[0].appearance.items[0].effects()[0].params, json!({"angle": -30.0}));
+}
+
+#[test]
+fn releases_from_0_21_turned_twist_clockwise() {
+    for g in ["VectorCraft 0.21.0", "VectorCraft 0.21.4", "VectorCraft 0.22.0-dev", "VectorCraft 1.0.0"] {
+        assert!(written_clockwise(Some(g)), "{g}");
+    }
+    for g in ["VectorCraft 0.20.0", "VectorCraft 0.9.12", "DrawCraft 0.1.0", "VectorCraft", "VectorCraft x.y", "vectorcraft 0.21.0"] {
+        assert!(!written_clockwise(Some(g)), "{g}");
+    }
+    assert!(!written_clockwise(None));
+}

crates/engine/src/tests/nativefile.rs

@@ -186,6 +186,40 @@ fn newer_keys_and_legacy_profiles_survive_open_and_save() {
     assert_eq!(v2["document"]["unknown"]["colorProfiles"]["rgb"], "sRGB IEC61966-2.1");
 }
 
+/// A version 1 document saved before VectorCraft 0.21.0 turned its Twist effects counterclockwise
+/// for a positive angle. It opens with the angles negated, so its art turns as it did; saved
+/// again it is a version 2 document, and saved for an older file version it is version 1 again.
+#[test]
+fn a_version_1_documents_twist_turns_as_it_was_saved() {
+    use vectorcraft_doc::{Appearance, Document, Effect, Node};
+    use vectorcraft_geom::{PathData, Point, Rect, SubPath};
+    let mut d = Document::new(100.0, 100.0);
+    let l = d.layers[0].id;
+    let id = d.alloc_id();
+    let right = PathData::single(SubPath::polyline(&[Point::new(55.0, 50.0), Point::new(56.0, 50.0)], false));
+    let mut a = Appearance::default_art();
+    a.effects.push(Effect { id: "distort.twist".into(), params: json!({"angle": 45}), visible: true });
+    d.insert(Some(l), 0, Node::path(id, right.clone(), a)).unwrap();
+    let mut j = json_of(&vectorcraft_format::save(&d, false));
+    j["document"]["version"] = json!(1);
+    j["generator"] = json!("VectorCraft 0.20.0");
+    let mut s = Session::new();
+    open(&mut s, "old.vectorcraft", &serde_json::to_vec(&j).unwrap());
+    let doc = &s.doc().unwrap().doc;
+    let n = doc.layers[0].children().and_then(|c| c.first()).cloned().unwrap();
+    let turned = vectorcraft_render::effects::apply_geometry(&n.appearance.effects, &right, Rect::new(0.0, 0.0, 100.0, 100.0));
+    // Counterclockwise on the page takes a point right of the center up (y runs down).
+    let p = turned.subpaths[0].anchors[0].p;
+    assert!(p.y < 49.5, "{p:?}");
+    let again = json_of(&saved(&mut s, json!({})));
+    assert_eq!(again["document"]["version"], 2);
+    assert!(again.to_string().contains(r#""angle":-45.0"#), "{again}");
+    // Saved for an older file version, it is version 1 with its angle as 0.20.0 saved it.
+    let older = json_of(&saved(&mut s, json!({"version": 3})));
+    assert_eq!(older["document"]["version"], 1);
+    assert!(older.to_string().contains(r#""angle":45.0"#), "{older}");
+}
+
 /// A save records the date the document was made in, so the same commands twice gave different
 /// bytes (#2880). A pinned date makes them identical — `file.new {created}` here, and
 /// `SOURCE_DATE_EPOCH` for an export that takes no date param.

crates/format/src/lib.rs

@@ -29,6 +29,13 @@
 //! `data`. Every version loads, and [`save_with`] writes any of them for older apps (under the
 //! format name `drawcraft`, which every version reads). Readers reject files whose `version` is
 //! newer than they support.
+//!
+//! The document has a version of its own (`document.version`, [`vectorcraft_doc::FORMAT_VERSION`]),
+//! which readers do not check. A version 2 document's Twist effects turn clockwise for a positive
+//! angle. A version 1 document's Twist angles are negated when it opens, unless the file is of
+//! [`VERSION`] and its `generator` is VectorCraft 0.21.0 or later, which turned them clockwise
+//! already; [`save_with`] writes a version 1 document with them negated back for an older file
+//! version ([`vectorcraft_doc::twist`]).
 #![forbid(unsafe_code)]
 
 #[cfg(not(target_arch = "wasm32"))]
@@ -244,6 +251,10 @@ fn body(doc: &Document, o: &SaveOptions) -> Result<Body, FormatError> {
     if version < 3 {
         d.store_legacy_color_profiles();
     }
+    // Apps that read only older file versions predate 0.21.0 and turn Twist the other way.
+    if version < VERSION {
+        d.store_legacy_twist_angles();
+    }
     let v1 = if version == 1 {
         let mut v = serde_json::to_value(&d).map_err(encode_err)?;
         legacy::anchors_to_v1(&mut v);
@@ -489,7 +500,7 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result
             vectorcraft_doc::node::with_buffered_value_budget(max_values, || Contents::<Document>::read(file.json()))
         })
     };
-    let f = vectorcraft_doc::geom::path::with_anchor_budget(vectorcraft_doc::geom::path::MAX_READ_ANCHORS, read).map_err(|e| match e {
+    let mut f = vectorcraft_doc::geom::path::with_anchor_budget(vectorcraft_doc::geom::path::MAX_READ_ANCHORS, read).map_err(|e| match e {
         ReadError::Json(e) => {
             let why = if e.to_string().contains("recursion limit") { format!("its objects are nested too deep ({e})") } else { e.to_string() };
             FormatError::NotVectorcraft(why)
@@ -498,6 +509,12 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result
         e => FormatError::NotVectorcraft(e.to_string()),
     })?;
     let info = FileInfo { version: f.version, legacy: f.format == LEGACY_EXTENSION };
+    // A version 1 document saved before VectorCraft 0.21.0, or for an app before it (an older file
+    // version): its Twist angles turned the other way. The app that wrote it is read from the JSON
+    // before the data after it is taken; the walk's depth is bounded by the JSON reader's nesting
+    // limit.
+    let current_file = f.version >= VERSION;
+    f.document.migrate_twist_angles(|| current_file && vectorcraft_doc::twist::written_clockwise(generator(file.json()).as_deref()));
     // The bytes after the JSON, read in one pass: the images' first, then the profiles'.
     let entries: Vec<[u64; 2]> = f.images.values().chain(f.profiles.values()).filter_map(|i| i.blob).collect();
     let tail = file.entries(&entries)?;
@@ -559,6 +576,8 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result
     // labels and the character styles, within the ranges `document.setup` takes (after the depth
     // limit: the walk recurses).
     doc.keep_scripts_in_range();
+    // The conversions above bring an older document to today's version.
+    doc.version = doc.version.max(vectorcraft_doc::FORMAT_VERSION);
     let profiles = f
         .profiles
         .into_iter()
@@ -574,6 +593,37 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result
     Ok(NativeFile { doc, depth_notes, info, profiles })
 }
 
+/// The file's first `generator` member, the app that wrote it (`None`: it has none, it is not a
+/// string, or the file is not an object of members). The other members, and any further
+/// `generator`, are skipped without being kept, as reading the file skips them.
+fn generator(json: &[u8]) -> Option<String> {
+    struct Generator(Option<String>);
+    struct Members;
+    impl<'de> serde::de::Visitor<'de> for Members {
+        type Value = Generator;
+        fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+            f.write_str("a file's members")
+        }
+        fn visit_map<A: serde::de::MapAccess<'de>>(self, mut m: A) -> Result<Generator, A::Error> {
+            let mut first: Option<serde_json::Value> = None;
+            while let Some(key) = m.next_key::<std::borrow::Cow<'de, str>>()? {
+                if key == "generator" && first.is_none() {
+                    first = Some(m.next_value()?);
+                } else {
+                    m.next_value::<serde::de::IgnoredAny>()?;
+                }
+            }
+            Ok(Generator(first.and_then(|v| v.as_str().map(str::to_owned))))
+        }
+    }
+    impl<'de> serde::Deserialize<'de> for Generator {
+        fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
+            d.deserialize_map(Members)
+        }
+    }
+    serde_json::from_slice::<Generator>(json).ok()?.0
+}
+
 /// The bytes of the member of the file's head `pick` picks (only that entry of the data after
 /// the JSON is kept).
 fn head_member(bytes: &[u8], pick: impl FnOnce(Head) -> Option<Image>) -> Option<Vec<u8>> {
@@ -853,6 +903,66 @@ mod tests {
         assert!(why.contains("more than 100 values before its type"), "{why}");
     }
 
+    /// A version 1 document written before VectorCraft 0.21.0 (or by an app the file does not
+    /// name) has its Twist angles negated when it opens; one written by 0.21.0 or later, and a
+    /// version 2 document, keep them. Each opens as version 2. A save for an older file version
+    /// negates them back.
+    #[test]
+    fn twist_angles_follow_the_document_version_and_generator() {
+        use vectorcraft_doc::Effect;
+        let mut d = Document::new(100.0, 100.0);
+        let l = d.layers[0].id;
+        let id = d.alloc_id();
+        let mut a = Appearance::default_art();
+        a.effects.push(Effect { id: "distort.twist".into(), params: serde_json::json!({"angle": 30.0}), visible: true });
+        d.insert(Some(l), 0, Node::path(id, shapes::ellipse(Rect::new(0.0, 0.0, 10.0, 10.0)), a)).unwrap();
+        let angle = |d: &Document| d.node(id).and_then(|n| n.appearance.effects[0].params["angle"].as_f64());
+        let current = save(&d, false);
+        let back = load(&current).unwrap();
+        assert_eq!((back.version, angle(&back)), (2, Some(30.0)), "a version 2 document is read as saved");
+        let opened = |generator: Option<&str>| {
+            let mut v: serde_json::Value = serde_json::from_slice(&current).unwrap();
+            v["document"]["version"] = 1.into();
+            match generator {
+                Some(g) => v["generator"] = g.into(),
+                None => {
+                    v.as_object_mut().unwrap().remove("generator");
+                }
+            }
+            let d = load(&serde_json::to_vec(&v).unwrap()).unwrap();
+            (d.version, angle(&d))
+        };
+        for g in [Some("VectorCraft 0.20.0"), Some("VectorCraft 0.9.12"), Some("DrawCraft 0.1.0"), Some("VectorCraft"), None] {
+            assert_eq!(opened(g), (2, Some(-30.0)), "{g:?}");
+        }
+        for g in ["VectorCraft 0.21.0", "VectorCraft 0.21.3", "VectorCraft 1.0.0"] {
+            assert_eq!(opened(Some(g)), (2, Some(30.0)), "{g}");
+        }
+        // A file may repeat `generator`: the first one names the app.
+        let mut v: serde_json::Value = serde_json::from_slice(&current).unwrap();
+        v["document"]["version"] = 1.into();
+        v["generator"] = "VectorCraft 0.21.0".into();
+        let text = serde_json::to_string(&v).unwrap();
+        let first = r#""generator":"VectorCraft 0.21.0""#;
+        let twice = text.replacen(first, &format!(r#"{first},"generator":"DrawCraft 0.1.0""#), 1);
+        assert_ne!(twice, text);
+        assert_eq!(angle(&load(twice.as_bytes()).unwrap()), Some(30.0));
+        v["generator"] = "VectorCraft 0.20.0".into();
+        let old = load(&serde_json::to_vec(&v).unwrap()).unwrap();
+        let again = load(&save(&old, false)).unwrap();
+        assert_eq!((again.version, angle(&again)), (2, Some(-30.0)), "saved again, it is version 2");
+        // Saved for an older file version (apps before 0.21.0): version 1 with the angle negated
+        // back, whatever the generator, and it reopens as it was.
+        for version in MIN_VERSION..VERSION {
+            let bytes = save_with(&d, &SaveOptions { version, ..SaveOptions::default() }).unwrap();
+            let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
+            assert_eq!(v["document"]["version"], 1, "file version {version}");
+            assert!(String::from_utf8_lossy(&bytes).contains(r#""angle":-30.0"#), "file version {version}");
+            let back = load(&bytes).unwrap();
+            assert_eq!((back.version, angle(&back)), (2, Some(30.0)), "file version {version}");
+        }
+    }
+
     #[test]
     fn rejects_foreign_and_future() {
         assert!(load(b"{}").is_err());