Merge pull request #6366 from dotCooCoo/fix/issue-5632

Part of #5632: Convert to Grayscale stores an image with one gray channel

crates/doc/src/pixels.rs

@@ -60,6 +60,38 @@ impl ImageBlob {
         img.write_to(&mut Cursor::new(&mut png), image::ImageFormat::Png).ok()?;
         Some(Self::png(png))
     }
+
+    /// The image as a grayscale PNG: `f` gives each pixel's gray level from its (straight) RGB, and
+    /// the PNG has an alpha channel only when some pixel isn't opaque. `f` runs once per distinct
+    /// color. `None` when the image can't be decoded, or when its file stores grayscale already
+    /// and no visible pixel changed.
+    pub fn map_gray(&self, mut f: impl FnMut([u8; 3]) -> u8) -> Option<ImageBlob> {
+        let img = rgba_within(&self.bytes, MAX_RASTER_ALLOC)?;
+        let (width, height) = img.dimensions();
+        let mut memo: HashMap<[u8; 3], u8> = HashMap::new();
+        let mut changed = self.gray_source().is_none();
+        let mut opaque = true;
+        let mut gray_alpha = Vec::with_capacity(img.len() / 2);
+        for px in img.pixels() {
+            let rgb = [px.0[0], px.0[1], px.0[2]];
+            let level = *memo.entry(rgb).or_insert_with(|| f(rgb));
+            changed |= px.0[3] > 0 && [level; 3] != rgb;
+            opaque &= px.0[3] == 255;
+            gray_alpha.extend_from_slice(&[level, px.0[3]]);
+        }
+        if !changed {
+            return None;
+        }
+        let mut png = Vec::new();
+        let mut out = Cursor::new(&mut png);
+        if opaque {
+            let levels = gray_alpha.iter().step_by(2).copied().collect();
+            image::GrayImage::from_raw(width, height, levels)?.write_to(&mut out, image::ImageFormat::Png).ok()?;
+        } else {
+            image::GrayAlphaImage::from_raw(width, height, gray_alpha)?.write_to(&mut out, image::ImageFormat::Png).ok()?;
+        }
+        Some(Self::png(png))
+    }
 }
 
 /// How a grayscale image's file stores its pixels ([`ImageBlob::gray_source`]).

crates/engine/src/cmd/colorcmds.rs

@@ -48,9 +48,9 @@ pub fn specs() -> Vec<CommandSpec> {
             "Convert to Grayscale",
             ["Edit", "Edit Colors"],
             None,
-            "{fill?, stroke?, includeImages?, includePatterns?, ids?} (as edit.colors.invert) → {changed, notChanged, warning?}",
+            "{fill?, stroke?, includeImages?, includePatterns?, ids?} (as edit.colors.invert; an embedded image is stored with one gray channel) → {changed, notChanged, warning?}",
             has_selection,
-            |s, p| recolor(s, p, "edit.colors.toGrayscale", "Convert to Grayscale", &to_gray)
+            |s, p| recolor_gray(s, p, "edit.colors.toGrayscale", "Convert to Grayscale", &to_gray)
         ),
         cmd!(
             "edit.colors.toRGB",
@@ -187,6 +187,8 @@ pub(crate) struct Recolor<'a> {
     /// Maps solid colours and gradient stops with their link and tint in place of `f`.
     links: Option<&'a LinkMap<'a>>,
     scope: Scope,
+    /// Recolored images are stored with one gray channel ([`ImageBlob::map_gray`]).
+    gray_images: bool,
     /// Image key → the key of its recoloured copy (`None`: unchanged).
     images: HashMap<String, Option<String>>,
     /// Pattern name → the name of its recoloured copy (`None`: unchanged, or being recoloured).
@@ -198,7 +200,17 @@ pub(crate) struct Recolor<'a> {
 
 impl<'a> Recolor<'a> {
     pub(crate) fn new(scope: Scope, f: &'a dyn Fn(Color) -> Color) -> Self {
-        Self { f, links: None, scope, images: HashMap::new(), patterns: HashMap::new(), new_images: vec![], new_patterns: vec![], changed: 0 }
+        Self {
+            f,
+            links: None,
+            scope,
+            gray_images: false,
+            images: HashMap::new(),
+            patterns: HashMap::new(),
+            new_images: vec![],
+            new_patterns: vec![],
+            changed: 0,
+        }
     }
 
     /// A pass that maps only solid colours and gradient stops, with their swatch link and tint
@@ -215,7 +227,7 @@ impl<'a> Recolor<'a> {
 
     /// A pass that maps colors as this one does, with nothing recolored yet.
     fn again(&self) -> Self {
-        Self { links: self.links, ..Self::new(self.scope, self.f) }
+        Self { links: self.links, gray_images: self.gray_images, ..Self::new(self.scope, self.f) }
     }
 
     /// The entries of `objects` (ids, each with a flag) whose object this pass would change, other
@@ -313,10 +325,15 @@ impl<'a> Recolor<'a> {
             return done.clone();
         }
         let f = self.f;
-        let new = d.images.get(key)?.map_rgb(|[r, g, b]| {
-            let [r, g, b, _] = f(Color::rgb8(r, g, b)).to_rgba8(1.0);
-            [r, g, b]
-        });
+        let blob = d.images.get(key)?;
+        let new = if self.gray_images {
+            blob.map_gray(|[r, g, b]| f(Color::rgb8(r, g, b)).to_rgba8(1.0)[0])
+        } else {
+            blob.map_rgb(|[r, g, b]| {
+                let [r, g, b, _] = f(Color::rgb8(r, g, b)).to_rgba8(1.0);
+                [r, g, b]
+            })
+        };
         let new = new.map(|blob| {
             let k = blob.content_key();
             self.new_images.push((k.clone(), blob));
@@ -400,6 +417,11 @@ pub(crate) fn recolor(s: &mut Session, p: &Value, cmd: &str, label: &str, f: &dy
     run_pass(s, p, cmd, label, Recolor::new(Scope::of(p), f))
 }
 
+/// [`recolor`] where `f` gives gray colors, so embedded images are stored with one gray channel.
+fn recolor_gray(s: &mut Session, p: &Value, cmd: &str, label: &str, f: &dyn Fn(Color) -> Color) -> Result<Value> {
+    run_pass(s, p, cmd, label, Recolor { gray_images: true, ..Recolor::new(Scope::of(p), f) })
+}
+
 /// [`recolor`] with solid colours and gradient stops mapped through `links`, with their swatch
 /// link and tint.
 pub(crate) fn recolor_links(s: &mut Session, p: &Value, cmd: &str, label: &str, f: &dyn Fn(Color) -> Color, links: &LinkMap) -> Result<Value> {
@@ -575,10 +597,11 @@ fn adjust_balance(s: &mut Session, p: &Value) -> Result<Value> {
         other => return Err(bad(C, format!("unknown mode `{other}` (rgb|cmyk|gray|global)"))),
     };
     let convert = bool_or(p, "convert", false);
-    recolor(s, p, C, "Adjust Colors", &move |c| {
+    let f = move |c| {
         let out = adjust(c);
         if convert { out } else { keep_model(c, out) }
-    })
+    };
+    if convert && mode == "gray" { recolor_gray(s, p, C, "Adjust Colors", &f) } else { recolor(s, p, C, "Adjust Colors", &f) }
 }
 
 enum BlendOrder {

crates/engine/src/tests/editcolors.rs

@@ -342,7 +342,12 @@ fn pixels(s: &Session, id: NodeId) -> Vec<[u8; 4]> {
 
 /// A selected embedded 2×2 image.
 fn image_node(s: &mut Session) -> NodeId {
-    let bytes = png([[255, 0, 0, 255], [0, 255, 0, 255], [0, 0, 255, 255], [255, 255, 255, 128]]);
+    image_of(s, [[255, 0, 0, 255], [0, 255, 0, 255], [0, 0, 255, 255], [255, 255, 255, 128]])
+}
+
+/// A selected embedded 2×2 image of the pixels `px`.
+fn image_of(s: &mut Session, px: [[u8; 4]; 4]) -> NodeId {
+    let bytes = png(px);
     s.edit("Place", |d, sel| {
         d.images.insert("pic".into(), vectorcraft_doc::ImageBlob::png(bytes));
         let id = d.alloc_id();
@@ -383,6 +388,85 @@ fn invert_recolours_an_embedded_image() {
     assert_eq!(pixels(&s, id)[..2], [[0, 0, 0, 255], [0, 255, 0, 255]]);
 }
 
+/// The key of image object `id`'s blob.
+fn image_key(s: &Session, id: NodeId) -> String {
+    let NodeKind::Image(im) = &s.doc().unwrap().doc.node(id).unwrap().kind else { panic!("not an image") };
+    im.key.clone()
+}
+
+/// Convert to Grayscale stores an image with one color channel, alpha kept (#5632).
+#[test]
+fn convert_to_grayscale_stores_an_image_as_grayscale() {
+    let mut s = session();
+    let id = image_node(&mut s);
+    let r = s.execute("edit.colors.toGrayscale", &json!({})).unwrap();
+    assert_eq!(r["changed"], 1);
+    let key = image_key(&s, id);
+    let source = s.doc().unwrap().doc.images[&key].gray_source();
+    assert!(source.is_some_and(|g| g.alpha && !g.one_bit), "a grayscale file with alpha: {source:?}");
+    let gray = |hex: &str| Color::from_hex(hex).unwrap().in_model(vectorcraft_color::cms::Model::Gray).to_rgba8(1.0)[0];
+    let (r_, g_, b_, w) = (gray("#ff0000"), gray("#00ff00"), gray("#0000ff"), gray("#ffffff"));
+    assert_eq!(pixels(&s, id), [[r_, r_, r_, 255], [g_, g_, g_, 255], [b_, b_, b_, 255], [w, w, w, 128]]);
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert_eq!(pixels(&s, id)[0], [255, 0, 0, 255], "one undo step restores the color image");
+    s.execute("edit.redo", &json!({})).unwrap();
+    // An image that is already grayscale has nothing to convert.
+    let again = s.execute("edit.colors.toGrayscale", &json!({})).unwrap();
+    assert_eq!(again["changed"], 0);
+    assert_eq!(image_key(&s, id), key);
+}
+
+/// An RGB image whose pixels are all neutral still becomes a one-channel image (#5632).
+#[test]
+fn convert_to_grayscale_stores_a_neutral_rgb_image_as_grayscale() {
+    let mut s = session();
+    let id = image_of(&mut s, [[255, 255, 255, 255], [0, 0, 0, 255], [255, 255, 255, 255], [0, 0, 0, 255]]);
+    let r = s.execute("edit.colors.toGrayscale", &json!({})).unwrap();
+    assert_eq!(r["changed"], 1);
+    let source = s.doc().unwrap().doc.images[&image_key(&s, id)].gray_source();
+    assert!(source.is_some_and(|g| !g.alpha), "an opaque image has no alpha channel: {source:?}");
+}
+
+/// Select All leaves a hidden neutral RGB image out of Convert to Grayscale, which would change it.
+#[test]
+fn a_hidden_neutral_rgb_image_counts_as_not_converted() {
+    let mut s = session();
+    let hidden = image_of(&mut s, [[255, 255, 255, 255], [0, 0, 0, 255], [255, 255, 255, 255], [0, 0, 0, 255]]);
+    s.execute("layer.setProps", &json!({"ids": [hidden.0], "visible": false})).unwrap();
+    rect(&mut s, json!("#ff0000"));
+    s.execute("select.all", &json!({})).unwrap();
+    let r = s.execute("edit.colors.toGrayscale", &json!({})).unwrap();
+    assert_eq!(r["notChanged"], json!({"locked": 0, "hidden": 1}), "{r}");
+}
+
+/// Adjust Colors converting to gray stores an image with one channel; without Convert it keeps
+/// the image's RGB (#5632).
+#[test]
+fn adjust_colors_converting_to_gray_stores_an_image_as_grayscale() {
+    let source = |convert: bool| {
+        let mut s = session();
+        let id = image_node(&mut s);
+        s.execute("edit.colors.adjustBalance", &json!({"mode": "gray", "gray": 10, "convert": convert})).unwrap();
+        s.doc().unwrap().doc.images[&image_key(&s, id)].gray_source()
+    };
+    assert!(source(true).is_some(), "converted to gray");
+    assert!(source(false).is_none(), "the colors keep their model");
+}
+
+/// A PDF of a CMYK document writes an image Convert to Grayscale made with one channel (#5632).
+#[test]
+fn a_converted_image_is_written_to_pdf_as_gray_in_a_cmyk_document() {
+    let mut s = session();
+    s.execute("file.documentColorMode", &json!({"mode": "cmyk"})).unwrap();
+    image_of(&mut s, [[255, 0, 0, 255], [0, 255, 0, 255], [0, 0, 255, 255], [10, 200, 30, 255]]);
+    s.execute("edit.colors.toGrayscale", &json!({})).unwrap();
+    let opts = vectorcraft_pdf::PdfOptions { created: Some(0), ..vectorcraft_pdf::PdfOptions::uncompressed() };
+    let pdf = String::from_utf8_lossy(&vectorcraft_pdf::export(&s.doc().unwrap().doc, &opts).unwrap()).into_owned();
+    let image = pdf.split("/Subtype/Image").nth(1).or_else(|| pdf.split("/Subtype /Image").nth(1)).expect("an image in the PDF");
+    let image = &image[..image.find("stream").unwrap_or(image.len())];
+    assert!(image.contains("/DeviceGray") && !image.contains("/DeviceRGB"), "one color channel: {image}");
+}
+
 /// "Dots": a red square tile; a selected rectangle filled with it.
 fn pattern_fill(s: &mut Session) -> NodeId {
     rect(s, json!("#ff0000"));

docs/mcp.md

@@ -2507,6 +2507,8 @@ the image; linked images are left alone) and the tiles of pattern fills and stro
 pattern swatch, e.g. "Dots 2"; the original pattern is untouched). Colorized image fills are recolored when fills are in
 scope, but their pixels are never recolored. `includeImages: false` leaves other images' pixels unchanged.
 For `edit.colors.*`, `fill: false` leaves colorized image fills unchanged. `recolor.apply` always includes fills.
+`edit.colors.toGrayscale` and `edit.colors.adjustBalance` with `mode: "gray"` and `convert: true` store an embedded image
+with one gray channel, so a PDF writes it in gray.
 `includePatterns: false` leaves patterns out. The Blend commands also grade gradient meshes, which keep their shading.
 `edit.colors.saturate` needs `intensity`, a number from -100 to 100 (a number outside the range counts as its nearest
 end). Without one, or with a value that is not a number, it answers `` missing number `intensity` `` and changes nothing.