Merge fix/635

crates/format/src/fig/image_filter_tests.rs

@@ -0,0 +1,104 @@
+//! Image paint filters read from `.fig` files, on fixtures built in code with the parent module's
+//! helpers.
+
+use super::*;
+
+/// The shared schema plus the image filter containers only these tests use.
+fn image_filter_schema() -> Schema {
+    let text = SCHEMA
+        .replace(
+            "message Paint ",
+            "message FilterColorAdjust float tint, float shadows, float highlights, float detail, float exposure, float vignette, float temperature, float vibrance\n\
+             message PaintFilterMessage float tint, float shadows, float highlights, float detail, float exposure, float vignette, float temperature, float vibrance, float contrast, float brightness, float mystery\n\
+             message Paint ",
+        )
+        .replace("message Paint PaintType type,", "message Paint PaintFilterMessage paintFilter, FilterColorAdjust filterColorAdjust, PaintType type,");
+    Schema::parse(&text).unwrap()
+}
+
+#[test]
+fn image_filters_import_from_current_container_and_skip_ambiguous_data() {
+    let filter = |fields: Vec<(&'static str, Kv<'static>)>| o(fields);
+    let image_paint = |id: u64, name: &'static str, mut extra: Vec<(&'static str, Kv<'static>)>| {
+        let mut paint = vec![("type", e("IMAGE")), ("image", o(vec![("hash", Kv::Bytes(HASH.to_vec()))]))];
+        paint.append(&mut extra);
+        node(id, Some((1, "!")), "RECTANGLE", name, vec![("size", xy(2.0, 1.0)), ("fillPaints", arr(vec![o(paint)]))])
+    };
+    let nodes = vec![
+        node(0, None, "DOCUMENT", "Document", vec![]),
+        node(1, Some((0, "!")), "CANVAS", "Page", vec![]),
+        image_paint(
+            2,
+            "Current",
+            vec![(
+                "paintFilter",
+                filter(vec![
+                    ("exposure", f(0.25)),
+                    ("contrast", f(0.3)),
+                    ("vibrance", f(-0.4)),
+                    ("temperature", f(0.5)),
+                    ("tint", f(1.0)),
+                    ("highlights", f(-0.6)),
+                    ("shadows", f(0.7)),
+                ]),
+            )],
+        ),
+        image_paint(3, "Negative tint", vec![("paintFilter", filter(vec![("tint", f(-1.0)), ("contrast", f(-0.3))]))]),
+        image_paint(4, "Neutral", vec![]),
+        image_paint(5, "Explicit neutral", vec![("paintFilter", filter(vec![("exposure", f(0.0)), ("vibrance", f(0.0))]))]),
+        image_paint(6, "Legacy", vec![("filterColorAdjust", filter(vec![("exposure", f(0.8))]))]),
+        image_paint(7, "Both", vec![("filterColorAdjust", filter(vec![("exposure", f(0.8))])), ("paintFilter", filter(vec![("contrast", f(0.2))]))]),
+        image_paint(
+            8,
+            "Unsupported",
+            vec![("paintFilter", filter(vec![("brightness", f(0.4)), ("detail", f(-0.2)), ("vignette", f(0.3)), ("mystery", f(0.1))]))],
+        ),
+    ];
+    let bytes = zip_file(&canvas(&image_filter_schema(), &o(vec![("nodeChanges", arr(nodes))])));
+    let (doc, warnings) = import(&bytes, "ImageFilters.fig").unwrap();
+    let get = |name: &str| {
+        let node = doc.nodes().find(|node| node.name == name).unwrap();
+        let figcraft_doc::color::PaintKind::Image(image) = &node.fills[0].kind else { panic!("{name} is not an image fill") };
+        image
+    };
+    let current = get("Current");
+    assert!((current.filters.exposure - 0.25).abs() < 1e-6);
+    assert!((current.filters.contrast - 0.3).abs() < 1e-6, "observed raw contrast remains 0.3");
+    assert!((current.filters.saturation + 0.4).abs() < 1e-6, "vibrance maps to saturation");
+    assert!((current.filters.temperature - 0.5).abs() < 1e-6);
+    assert!((current.filters.tint + 1.0).abs() < 1e-6, "a positive (magenta) tint in the file is a negative native tint");
+    assert!((current.filters.highlights + 0.6).abs() < 1e-6);
+    assert!((current.filters.shadows - 0.7).abs() < 1e-6);
+    assert!((get("Negative tint").filters.tint - 1.0).abs() < 1e-6);
+    assert!((get("Negative tint").filters.contrast + 0.3).abs() < 1e-6);
+    assert_eq!(get("Neutral").filters, Default::default());
+    assert_eq!(get("Explicit neutral").filters, Default::default());
+    assert_eq!(doc.images.get(&current.image).map(|image| image.bytes.as_slice()), Some(png().as_slice()));
+    assert!(warnings.iter().any(|w| w.contains("legacy image filter container is unsupported")), "{warnings:?}");
+    assert!(warnings.iter().any(|w| w.contains("both filter containers are present")), "{warnings:?}");
+    for field in ["brightness", "detail", "vignette", "mystery"] {
+        assert!(warnings.iter().any(|w| w.contains(field)), "missing {field} warning: {warnings:?}");
+    }
+    assert_eq!(warnings.iter().filter(|w| w.contains("approximate renderer")).count(), 1, "deduplicated approximation warning: {warnings:?}");
+}
+
+#[test]
+fn warnings_naming_unknown_filter_fields_are_capped() {
+    // A schema can name any number of fields; each nonzero unknown one is a warning of its own.
+    let names: Vec<&'static str> = (0..300).map(|i| &*Box::leak(format!("unknown{i}").into_boxed_str())).collect();
+    let fields: Vec<String> = names.iter().map(|n| format!("float {n}")).collect();
+    let text = SCHEMA
+        .replace("message Paint ", &format!("message ManyFilters {}\nmessage Paint ", fields.join(", ")))
+        .replace("message Paint PaintType type,", "message Paint ManyFilters paintFilter, PaintType type,");
+    let filters = o(names.iter().map(|n| (*n, f(0.5))).collect());
+    let paint = o(vec![("type", e("IMAGE")), ("image", o(vec![("hash", Kv::Bytes(HASH.to_vec()))])), ("paintFilter", filters)]);
+    let nodes = vec![
+        node(0, None, "DOCUMENT", "Document", vec![]),
+        node(1, Some((0, "!")), "CANVAS", "Page", vec![]),
+        node(2, Some((1, "!")), "RECTANGLE", "Photo", vec![("size", xy(2.0, 1.0)), ("fillPaints", arr(vec![paint]))]),
+    ];
+    let bytes = zip_file(&canvas(&Schema::parse(&text).unwrap(), &o(vec![("nodeChanges", arr(nodes))])));
+    let (_, warnings) = import(&bytes, "ManyFilters.fig").unwrap();
+    let named = warnings.iter().filter(|w| w.contains("unsupported image filter")).count();
+    assert!((100..=crate::rest::MAX_WARNINGS).contains(&named), "{named} of {} warnings", warnings.len());
+}

crates/format/src/fig/map.rs

@@ -57,6 +57,10 @@ pub(super) fn to_rest(msg: &Kv, name: &str) -> (Value, Vec<String>) {
     (out, m.warnings)
 }
 
+#[cfg(test)]
+#[path = "map_image_filter_tests.rs"]
+mod image_filter_tests;
+
 /// REST importer warnings reworded for a `.fig` import (`None`: drop it).
 pub(super) fn adapt_warning(w: String) -> Option<String> {
     if w.contains("geometry=paths") {
@@ -75,6 +79,16 @@ fn num(n: &Kv, k: &str) -> Option<f64> {
     n.get(k).and_then(Kv::f64)
 }
 
+fn filter_number(value: &Kv) -> Option<f64> {
+    let number = match value {
+        Kv::Float(value) => f64::from(*value),
+        Kv::Int(value) => *value as f64,
+        Kv::Uint(value) => *value as f64,
+        _ => return None,
+    };
+    number.is_finite().then_some(number)
+}
+
 fn guid(v: &Kv) -> Option<String> {
     Some(format!("{}:{}", num(v, "sessionID")? as u64, num(v, "localID")? as u64))
 }
@@ -462,7 +476,8 @@ impl<'a> Mapper<'a> {
 
     fn warn(&mut self, w: impl Into<String>) {
         let w = w.into();
-        if !self.warnings.contains(&w) {
+        // Capped like the REST importer's list: some warnings name fields of the file (image filters).
+        if self.warnings.len() < crate::rest::MAX_WARNINGS && !self.warnings.contains(&w) {
             self.warnings.push(w);
         }
     }
@@ -1262,11 +1277,11 @@ impl<'a> Mapper<'a> {
         let fill_style = self.style(v, "styleIdForFill", "inheritFillStyleID", "fill", &mut styles);
         let fills = own("fillPaints")
             .unwrap_or_else(|| fill_style.map_or_else(|| v.list("fillPaints"), |s| s.get("fillPaints").map(Kv::list).unwrap_or_default()));
-        o.insert("fills".into(), Value::Array(fills.iter().filter_map(paint).collect()));
+        o.insert("fills".into(), Value::Array(fills.iter().filter_map(|p| paint(p, &mut |w| self.warn(w))).collect()));
         let stroke_style = self.style(v, "styleIdForStrokeFill", "inheritFillStyleIDForStroke", "stroke", &mut styles);
         let strokes = own("strokePaints")
             .unwrap_or_else(|| stroke_style.map_or_else(|| v.list("strokePaints"), |s| s.get("fillPaints").map(Kv::list).unwrap_or_default()));
-        o.insert("strokes".into(), Value::Array(strokes.iter().filter_map(paint).collect()));
+        o.insert("strokes".into(), Value::Array(strokes.iter().filter_map(|p| paint(p, &mut |w| self.warn(w))).collect()));
         let effect_style = self.style(v, "styleIdForEffect", "inheritEffectStyleID", "effect", &mut styles);
         let effects = effect_style.map_or_else(|| v.list("effects"), |s| s.get("effects").map(Kv::list).unwrap_or_default());
         if !effects.is_empty() {
@@ -1494,7 +1509,7 @@ impl<'a> Mapper<'a> {
         }
         o.insert("style".into(), Value::Object(style));
         let Some(td) = td else { return };
-        char_styles(o, td, size);
+        char_styles(o, td, size, &mut |w| self.warn(w));
         let lines: Vec<&str> = td
             .get("lines")
             .map(Kv::list)
@@ -1828,7 +1843,7 @@ impl<'a> Mapper<'a> {
                             o.remove("styleOverrideTable");
                             if let Some(tv) = val.get("textValue") {
                                 let size = o.get("style").and_then(|s| s.get("fontSize")).and_then(Value::as_f64);
-                                char_styles(o, tv, size);
+                                char_styles(o, tv, size, &mut |w| self.warn(w));
                             }
                         }
                     }
@@ -2474,7 +2489,7 @@ fn paint_bound(p: &Kv) -> bool {
     alias(p.get("colorVar")).is_some() || p.get("stopsVar").map(Kv::list).unwrap_or_default().iter().any(|s| alias(s.get("colorVar")).is_some())
 }
 
-fn paint(p: &Kv) -> Option<Value> {
+fn paint(p: &Kv, warn: &mut dyn FnMut(String)) -> Option<Value> {
     let ty = p.get("type").and_then(Kv::str)?;
     let mut o = Obj::new();
     o.insert("type".into(), json!(ty));
@@ -2543,6 +2558,52 @@ fn paint(p: &Kv) -> Option<Value> {
             if let Some(r) = num(p, "rotation") {
                 o.insert("rotation".into(), json!(r));
             }
+            match (p.get("filterColorAdjust"), p.get("paintFilter")) {
+                (Some(_), Some(_)) => {
+                    warn("image filters are ambiguous because both filter containers are present; filter conversion was skipped".into())
+                }
+                (Some(_), None) => warn("legacy image filter container is unsupported; filter conversion was skipped".into()),
+                (None, Some(filters)) => {
+                    let mut mapped = Obj::new();
+                    let fields = [
+                        ("exposure", "exposure"),
+                        ("contrast", "contrast"),
+                        ("vibrance", "saturation"),
+                        ("temperature", "temperature"),
+                        ("tint", "tint"),
+                        ("highlights", "highlights"),
+                        ("shadows", "shadows"),
+                    ];
+                    if let Kv::Object(values) = filters {
+                        for (source, target) in fields {
+                            if let Some(value) = filters.get(source) {
+                                match filter_number(value) {
+                                    Some(value) => {
+                                        mapped.insert(target.into(), json!(value));
+                                    }
+                                    None => warn(format!("malformed image filter `{source}` was skipped")),
+                                }
+                            }
+                        }
+                        for (name, value) in values {
+                            if fields.iter().any(|(known, _)| *known == *name) {
+                                continue;
+                            }
+                            match filter_number(value) {
+                                Some(0.0) => {}
+                                Some(_) => warn(format!("nonzero unsupported image filter `{}` was skipped", crate::rest::props::clip_str(name))),
+                                None => warn(format!("malformed unsupported image filter `{}` was skipped", crate::rest::props::clip_str(name))),
+                            }
+                        }
+                    } else {
+                        warn("malformed image paintFilter container was skipped".into());
+                    }
+                    if !mapped.is_empty() {
+                        o.insert("filters".into(), Value::Object(mapped));
+                    }
+                }
+                (None, None) => {}
+            }
         }
         _ => {}
     }
@@ -2626,7 +2687,7 @@ fn variable_value(d: &Kv) -> Option<Value> {
 
 /// Per-character styles of text data `td` (`characterStyleIDs` + `styleOverrideTable`) as REST
 /// `characterStyleOverrides` + `styleOverrideTable`; `size`: the base font size.
-fn char_styles(o: &mut Obj, td: &Kv, size: Option<f64>) {
+fn char_styles(o: &mut Obj, td: &Kv, size: Option<f64>, warn: &mut dyn FnMut(String)) {
     let ids: Vec<u64> = td.get("characterStyleIDs").map(Kv::list).unwrap_or_default().iter().filter_map(Kv::f64).map(|i| i as u64).collect();
     if ids.iter().all(|i| *i == 0) {
         return;
@@ -2636,7 +2697,7 @@ fn char_styles(o: &mut Obj, td: &Kv, size: Option<f64>) {
         let Some(sid) = num(e, "styleID") else { continue };
         let mut s = text_style(&View::of(e), size);
         if let Some(fills) = e.get("fillPaints") {
-            s.insert("fills".into(), Value::Array(fills.list().iter().filter_map(paint).collect()));
+            s.insert("fills".into(), Value::Array(fills.list().iter().filter_map(|p| paint(p, warn)).collect()));
         }
         table.insert((sid as u64).to_string(), Value::Object(s));
     }

crates/format/src/fig/map_image_filter_tests.rs

@@ -0,0 +1,27 @@
+//! Malformed image paint filters are skipped with a warning.
+
+use super::*;
+
+#[test]
+fn malformed_non_finite_filter_is_skipped_with_warning() {
+    let paint = Kv::Object(vec![("type", Kv::Enum("IMAGE")), ("paintFilter", Kv::Object(vec![("exposure", Kv::Float(f32::NAN))]))]);
+    let mut warnings = Vec::new();
+    let value = super::paint(&paint, &mut |warning| warnings.push(warning)).unwrap();
+    assert!(value.get("filters").is_none(), "{value}");
+    assert!(warnings.iter().any(|warning| warning.contains("malformed image filter `exposure`")), "{warnings:?}");
+}
+
+#[test]
+fn boolean_filter_values_are_malformed_even_when_false() {
+    for (field, value, expected_warning) in [
+        ("exposure", Kv::Bool(true), "malformed image filter `exposure`"),
+        ("exposure", Kv::Bool(false), "malformed image filter `exposure`"),
+        ("mystery", Kv::Bool(false), "malformed unsupported image filter `mystery`"),
+    ] {
+        let paint = Kv::Object(vec![("type", Kv::Enum("IMAGE")), ("paintFilter", Kv::Object(vec![(field, value)]))]);
+        let mut warnings = Vec::new();
+        let value = super::paint(&paint, &mut |warning| warnings.push(warning)).unwrap();
+        assert!(value.get("filters").is_none(), "{value}");
+        assert!(warnings.iter().any(|warning| warning.contains(expected_warning)), "{warnings:?}");
+    }
+}

crates/format/src/fig/tests.rs

@@ -1042,6 +1042,9 @@ fn nested_instances_resized_again_drop_the_layout_derived_for_their_old_size() {
 #[path = "tail_tests.rs"]
 mod tail;
 
+#[path = "image_filter_tests.rs"]
+mod image_filters;
+
 /// `.fig` files in the folder `FIGCRAFT_FIG_SAMPLES` names (none when it's unset).
 pub(super) fn samples() -> Vec<std::path::PathBuf> {
     let Some(dir) = std::env::var_os("FIGCRAFT_FIG_SAMPLES") else { return Vec::new() };

crates/format/src/rest/mod.rs

@@ -66,7 +66,7 @@ const MAX_PATH_BYTES: usize = 4 << 20;
 /// Most path elements in one vector.
 const MAX_PATH_ELEMENTS: usize = 200_000;
 /// Most distinct warnings reported.
-const MAX_WARNINGS: usize = 200;
+pub(crate) const MAX_WARNINGS: usize = 200;
 /// Most shared style definitions read.
 const MAX_STYLES: usize = 10_000;
 /// Image key used for image fills without an `imageRef`.

crates/format/src/rest/props.rs

@@ -178,6 +178,24 @@ fn image(cx: &mut Ctx, p: &Value) -> ImagePaint {
         .filter(|c| c[0].abs() > 1e-9 && c[3].abs() > 1e-9);
     let f = p.get("filters");
     let filter = |k: &str| f.and_then(|f| num(f, k)).unwrap_or(0.0).clamp(-1.0, 1.0) as f32;
+    let filter_fields = ["exposure", "contrast", "saturation", "temperature", "tint", "highlights", "shadows"];
+    let mut has_filters = false;
+    if let Some(filters) = f.and_then(Value::as_object) {
+        for (key, value) in filters {
+            match value.as_f64().filter(|value| value.is_finite()) {
+                Some(value) if filter_fields.contains(&key.as_str()) => has_filters |= value != 0.0,
+                Some(0.0) => {}
+                Some(_) => cx.warn(format!("nonzero unsupported image filter `{}` was skipped", clip_str(key))),
+                None => cx.warn(format!("malformed image filter `{}` was skipped", clip_str(key))),
+            }
+        }
+    } else if f.is_some_and(|filters| !filters.is_null()) {
+        cx.warn("malformed image filters container was skipped");
+    }
+    let fig = cx.fig_sources.is_some();
+    if has_filters && fig {
+        cx.warn("image filters use FigCraft's approximate renderer");
+    }
     ImagePaint {
         image: cx.image_key(text(p, "imageRef").unwrap_or("")),
         scale_mode,
@@ -189,7 +207,9 @@ fn image(cx: &mut Ctx, p: &Value) -> ImagePaint {
             contrast: filter("contrast"),
             saturation: filter("saturation"),
             temperature: filter("temperature"),
-            tint: filter("tint"),
+            // The files' tint runs from green (negative) to magenta (positive), in REST file JSON and
+            // .fig alike; a positive native tint adds green.
+            tint: -filter("tint"),
             highlights: filter("highlights"),
             shadows: filter("shadows"),
         },
@@ -571,6 +591,6 @@ pub(super) fn export_settings(v: &Value) -> Vec<ExportSetting> {
 }
 
 /// At most 200 chars of `s` (names and labels from the file stay readable in warnings).
-pub(super) fn clip_str(s: &str) -> &str {
+pub(crate) fn clip_str(s: &str) -> &str {
     s.char_indices().nth(200).and_then(|(i, _)| s.get(..i)).unwrap_or(s)
 }

crates/format/src/rest/tests.rs

@@ -186,6 +186,25 @@ fn paints_gradients_and_image_placeholders() {
     assert!(warnings.iter().any(|w| w.starts_with("1 image without image data shows")), "{warnings:?}");
 }
 
+#[test]
+fn image_filter_tint_turns_magenta_positive_into_native_green_positive() {
+    let rect = |name: &str, tint: f64| {
+        json!({
+            "id": name, "name": name, "type": "RECTANGLE", "relativeTransform": at(0.0, 0.0), "size": {"x": 10, "y": 10},
+            "fills": [{"type": "IMAGE", "imageRef": "tint", "filters": {"tint": tint}}]
+        })
+    };
+    let (doc, warnings) = one(json!([rect("Magenta", 0.75), rect("Green", -0.5)]));
+    let image = |name: &str| {
+        let PaintKind::Image(image) = &named(&doc, name).fills[0].kind else { panic!("{name} is not an image fill") };
+        image
+    };
+    // The same sign as a `.fig` import (`fig::tests::image_filters`).
+    assert!(close(f64::from(image("Magenta").filters.tint), -0.75));
+    assert!(close(f64::from(image("Green").filters.tint), 0.5));
+    assert!(!warnings.iter().any(|w| w.contains("approximate renderer")), "{warnings:?}");
+}
+
 #[test]
 fn image_bytes_come_from_the_resolver() {
     let png = {