collective
Browse / github.com/storytold/photocraft / patch
feat: add Indexed Color Matte options
feat: add Indexed Color Matte options ## Scope Small Photoshop-parity improvement to the stock Indexed Color dialog, independent of the palette backend/filter. Rebased onto current `main` (`a85d79167`), including the Indexed sRGB correctness fix #9882. [Scoped diff](https://github.com/Firstbober/photocraft/compare/fix/indexed-color-srgb...feat/indexed-color-matte). Adds the existing Photoshop Matte choices — None, Foreground Color, Background Color, White, Black, Other… (shared color picker) — to the stock dialog. No palette browser, dithering editor, Before/After redesign, or bundled palettes. - `None` keeps the existing quant
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Firstbober (imported from pull request #9887, open)
- Base
a85d79167ef95b7348872ff6542e011f15b9ec3b- Size
- +277 −2 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
8a947170c7a84bbe8da6a09b
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/mode_cmds.rs crates/engine/src/cmd/mode_cmds/tests.rs crates/ui-egui/src/dialogs/filter_dialog.rs crates/ui-egui/src/dialogs/filter_dialog/tests.rs docs/development.md
@@ -9,6 +9,8 @@ use photocraft_algo::quantize::{self, BitmapMethod, Dither, Forced, HalftoneShape, PaletteKind}; use photocraft_algo::transform::{Homography, Interp};+use photocraft_cms::{Builtin, TransformOptions};+ use photocraft_color::{ColorMode, PixelFormat, SampleType}; use photocraft_doc::adjust::CurvePoint; use photocraft_doc::{ColorTable, Document, Duotone, DuotoneInk, Layer, LayerContent, LayerId, Size};@@ -189,6 +191,32 @@ fn composite(doc: &Document, opaque: bool) -> Vec<[f32; 4]> { if opaque { buf.over_background([1.0, 1.0, 1.0]).px } else { buf.px } } +/// Indexed storage and its palette are sRGB. Convert RGB/gray composites before quantisation so+/// wide-gamut document samples are not interpreted as sRGB numbers by the quantizer.+fn composite_in_srgb(doc: &Document, opaque: bool) -> Result<Vec<[f32; 4]>> {+ const CMD: &str = "image.mode.indexedColor";+ let mut pixels = composite(doc, opaque);+ let channels = match doc.mode {+ ColorMode::Grayscale | ColorMode::Bitmap | ColorMode::Duotone => 1,+ ColorMode::Rgb | ColorMode::Indexed => 3,+ _ => return Ok(pixels),+ };+ let source = crate::color_cmds::document_profile(doc);+ let srgb = Builtin::Srgb.profile();+ if source.same_colors(srgb) {+ return Ok(pixels);+ }+ let transform =+ photocraft_cms::cached(&source, srgb, TransformOptions::default()).map_err(|e| bad(CMD, format!("could not convert the composite to sRGB: {e}")))?;+ let mut converted = [0.0f32; 16];+ for pixel in &mut pixels {+ let input = if channels == 1 { &pixel[..1] } else { &pixel[..3] };+ transform.eval(input, &mut converted);+ pixel[..3].copy_from_slice(&converted[..3]);+ }+ Ok(pixels)+}+ /// Replace the document's layers by one 8-bit layer holding `px` in `mode`'s storage format. fn single_layer(doc: &mut Document, active: &mut Option<LayerId>, mode: ColorMode, px: &[[f32; 4]], name: &str, background: bool) { doc.mode = mode;@@ -243,6 +271,32 @@ fn parse_dither(s: &str) -> Option<Dither> { }) } +fn indexed_matte(s: &Session, p: &Value) -> Result<Option<[f32; 3]>> {+ const CMD: &str = "image.mode.indexedColor";+ let rgba = |color: [f32; 4]| [color[0], color[1], color[2]];+ let matte = match p.get("matte") {+ None | Some(Value::Null) => return Ok(None),+ Some(Value::String(value)) => match value.as_str() {+ "none" => return Ok(None),+ "white" => [1.0; 3],+ "black" => [0.0; 3],+ "foreground" => rgba(s.tools.foreground),+ "background" => rgba(s.tools.background),+ value if value.len() == 7 && value.starts_with('#') => p+ .get("matte")+ .and_then(parse_color)+ .ok_or_else(|| bad(CMD, "matte must be none, foreground, background, white, black or #rrggbb"))?+ .map(|v| f32::from(v) / 255.0),+ _ => return Err(bad(CMD, "matte must be none, foreground, background, white, black or #rrggbb")),+ },+ _ => return Err(bad(CMD, "matte must be a colour string")),+ };+ if matte.iter().any(|v| !v.is_finite() || !(0.0..=1.0).contains(v)) {+ return Err(bad(CMD, "matte channels must be finite and in 0..=1"));+ }+ Ok(Some(matte))+}+ fn indexed_color(s: &mut Session, p: &Value) -> Result<Value> { const CMD: &str = "image.mode.indexedColor"; let kind = PaletteKind::from_id(str_or(p, "palette", "selective"))@@ -261,8 +315,23 @@ fn indexed_color(s: &mut Session, p: &Value) -> Result<Value> { }; let amount = num(p, "amount", 75.0).clamp(0.0, 100.0) / 100.0; let transparency = p.get("transparency").and_then(Value::as_bool).unwrap_or(true);+ let matte = indexed_matte(s, p)?; let (pal_len, transparent) = s.edit("Indexed Color", |doc, active| {- let mut px = composite(doc, !transparency);+ let mut px = composite_in_srgb(doc, !transparency && matte.is_none())?;+ if let Some(matte) = matte {+ for pixel in &mut px {+ let alpha = pixel[3].clamp(0.0, 1.0);+ if alpha == 1.0 {+ continue;+ }+ if !transparency || alpha > 0.0 {+ for (channel, background) in pixel[..3].iter_mut().zip(matte) {+ *channel = *channel * alpha + background * (1.0 - alpha);+ }+ pixel[3] = 1.0;+ }+ }+ } let has_clear = transparency && px.iter().any(|q| q[3] < 0.5); let mut pal = quantize::build_palette(&px, kind, if has_clear { colors.saturating_sub(1).max(2) } else { colors }, forced).map_err(|e| bad(CMD, e))?; let transparent = if has_clear && pal.len() < 256 {@@ -274,6 +343,7 @@ fn indexed_color(s: &mut Session, p: &Value) -> Result<Value> { let w = doc.size.width as usize; quantize::quantize(&mut px, w, &pal, dither, amount, transparent); single_layer(doc, active, ColorMode::Indexed, &px, if transparent.is_some() { "Index" } else { "Background" }, transparent.is_none());+ doc.icc_profile = Some(Builtin::Srgb.profile().to_bytes()); doc.color_table = Some(ColorTable { colors: pal.clone(), transparent: transparent.map(|t| t as u8) }); Ok((pal.len(), transparent)) })?;@@ -606,7 +676,7 @@ pub fn specs() -> Vec<CommandSpec> { "image.mode.indexedColor", "Indexed Color…", ["Image", "Mode"],- r##"{"palette":"selective|perceptual|adaptive|exact|systemMac|systemWindows|web|uniform"="selective","colors":2..256=256,"forced":"blackWhite|none|primaries|web"="blackWhite","transparency":bool=true,"dither":"diffusion|none|pattern|noise"="diffusion","amount":0..100=75} (flattens)"##,+ r##"{"palette":"selective|perceptual|adaptive|exact|systemMac|systemWindows|web|uniform"="selective","colors":2..256=256,"forced":"blackWhite|none|primaries|web"="blackWhite","transparency":bool=true,"matte":color,"dither":"diffusion|none|pattern|noise"="diffusion","amount":0..100=75} (flattens; matte: none|foreground|background|white|black|#rrggbb)"##, |s| mode_is( s, &[ColorMode::Rgb, ColorMode::Grayscale, ColorMode::Indexed, ColorMode::Duotone, ColorMode::Cmyk, ColorMode::Lab],@@ -247,6 +247,112 @@ fn indexed_color_palettes_and_dithers() { } } +#[test]+fn indexed_wide_gamut_composites_in_srgb_before_quantizing() {+ for (profile_id, source) in [("display-p3", photocraft_cms::Builtin::DisplayP3), ("adobe-rgb-compat", photocraft_cms::Builtin::AdobeRgbCompat)] {+ for depth in [8, 16, 32] {+ let mut s = Session::new();+ s.execute("file.new", json!({"width": 2, "height": 1, "depth": depth, "mode": "rgb", "background": "transparent"})).unwrap();+ paint(&mut s, |x, _| if x == 0 { [0.13, 0.41, 0.79, 1.0] } else { [0.71, 0.27, 0.19, 0.6] });+ s.execute("edit.assignProfile", json!({"profile": profile_id})).unwrap();+ let source_bytes = source.profile().to_bytes();+ let source_profile = photocraft_cms::Profile::parse(&source_bytes).unwrap();+ let to_srgb =+ photocraft_cms::cached(&source_profile, photocraft_cms::Builtin::Srgb.profile(), photocraft_cms::TransformOptions::default()).unwrap();+ let expected = |rgb: [f32; 3]| {+ let mut out = [0.0f32; 16];+ to_srgb.eval(&rgb, &mut out);+ [0, 1, 2].map(|i| (out[i].clamp(0.0, 1.0) * 255.0).round() as u8)+ };+ let surface = doc(&s).layers[0].surface().unwrap();+ let stored_a = surface.rgba(0, 0);+ let stored_b = surface.rgba(1, 0);+ let expected_a = expected([stored_a[0], stored_a[1], stored_a[2]]);+ let expected_b = expected([stored_b[0], stored_b[1], stored_b[2]]);+ assert_ne!(expected_a, [33, 105, 201], "{profile_id}: the regression colours must move");+ assert_ne!(expected_b, [181, 69, 48], "{profile_id}: the regression colours must move");++ s.execute("image.mode.indexedColor", json!({"palette": "exact", "forced": "none", "dither": "none"})).unwrap();+ let d = doc(&s);+ assert_eq!((d.mode, d.depth), (ColorMode::Indexed, SampleType::U8), "{profile_id} {depth}");+ assert_eq!(d.icc_profile.as_deref(), Some(&*photocraft_cms::Builtin::Srgb.profile().to_bytes()), "{profile_id} {depth}");+ let table = d.color_table.as_ref().unwrap();+ assert!(table.colors.contains(&expected_a), "{profile_id} {depth}: {expected_a:?} not in {:?}", table.colors);+ assert!(table.colors.contains(&expected_b), "{profile_id} {depth}: {expected_b:?} not in {:?}", table.colors);+ let surface = d.layers[0].surface().unwrap();+ for (x, (want, alpha)) in [(0, (expected_a, 1.0)), (1, (expected_b, 0.6))] {+ let got = surface.rgba(x, 0);+ let got8 = [0, 1, 2].map(|i| (got[i].clamp(0.0, 1.0) * 255.0).round() as u8);+ assert_eq!(got8, want, "{profile_id} {depth} x={x}");+ assert!((got[3] - alpha).abs() <= 1.0 / 255.0, "{profile_id} {depth} x={x}: {got:?}");+ }++ s.execute("edit.undo", json!({})).unwrap();+ assert_eq!(doc(&s).mode, ColorMode::Rgb);+ assert_eq!(doc(&s).icc_profile.as_deref(), Some(&*source_bytes));+ s.execute("edit.redo", json!({})).unwrap();+ assert_eq!(doc(&s).icc_profile.as_deref(), Some(&*photocraft_cms::Builtin::Srgb.profile().to_bytes()));+ assert_eq!(doc(&s).layers[0].surface().unwrap().rgba(0, 0)[3], 1.0);+ assert!((doc(&s).layers[0].surface().unwrap().rgba(1, 0)[3] - 0.6).abs() <= 1.0 / 255.0);+ }+ }+}++#[test]+fn indexed_matte_blends_edges_without_replacing_clear_pixels() {+ for depth in [8, 16, 32] {+ for (matte, color) in [+ ("black", [0.0, 0.0, 0.0]),+ ("white", [1.0, 1.0, 1.0]),+ ("#0000ff", [0.0, 0.0, 1.0]),+ ("foreground", [0.0, 0.0, 1.0]),+ ("background", [0.0, 1.0, 0.0]),+ ] {+ let mut s = Session::new();+ s.execute("file.new", json!({"width": 4, "height": 1, "depth": depth, "background": "transparent"})).unwrap();+ s.execute("tools.setColors", json!({"foreground": "#0000ff", "background": "#00ff00"})).unwrap();+ paint(&mut s, |x, _| match x {+ 0 => [0.0, 1.0, 0.0, 1.0],+ 1 => [1.0, 0.0, 0.0, 0.75],+ 2 => [1.0, 0.0, 0.0, 0.25],+ _ => [0.0; 4],+ });+ let source = [1, 2].map(|x| doc(&s).layers[0].surface().unwrap().rgba(x, 0));+ let expected = source.map(|pixel| [0, 1, 2].map(|i| pixel[i] * pixel[3] + color[i] * (1.0 - pixel[3])));+ s.execute("image.mode.indexedColor", json!({"palette": "exact", "forced": "none", "dither": "none", "matte": matte})).unwrap();+ let surface = doc(&s).layers[0].surface().unwrap();+ let edges = [1, 2].map(|x| surface.rgba(x, 0));+ for (edge, expected) in edges.iter().zip(expected) {+ for i in 0..3 {+ assert!((edge[i] - expected[i]).abs() <= 1.0 / 255.0, "{depth}-bit {matte}: {edge:?} vs {expected:?}");+ }+ assert_eq!(edge[3], 1.0);+ }+ assert_eq!(surface.rgba(3, 0)[3], 0.0, "{matte}: fully transparent pixels stay clear");+ assert_eq!(surface.rgba(0, 0), [0.0, 1.0, 0.0, 1.0]);+ s.execute("edit.undo", json!({})).unwrap();+ assert_eq!([1, 2].map(|x| doc(&s).layers[0].surface().unwrap().rgba(x, 0)), source);+ s.execute("edit.redo", json!({})).unwrap();+ assert_eq!([1, 2].map(|x| doc(&s).layers[0].surface().unwrap().rgba(x, 0)), edges);+ }+ }+}++#[test]+fn indexed_matte_without_transparency_fills_clear_pixels_and_rejects_bad_input_atomically() {+ let mut s = Session::new();+ s.execute("file.new", json!({"width": 2, "height": 1, "background": "transparent"})).unwrap();+ let before = s.active().unwrap().revision;+ for matte in [json!(false), json!(12), json!("#gg0000"), json!("#000000ff"), json!("unknown")] {+ assert!(s.execute("image.mode.indexedColor", json!({"matte": matte})).is_err(), "{matte}");+ assert_eq!(s.active().unwrap().revision, before);+ assert_eq!(doc(&s).mode, ColorMode::Rgb);+ }+ s.execute("image.mode.indexedColor", json!({"palette": "exact", "forced": "none", "dither": "none", "transparency": false, "matte": "#0000ff"})).unwrap();+ assert_eq!(doc(&s).layers[0].surface().unwrap().rgba(0, 0), [0.0, 0.0, 1.0, 1.0]);+ assert_eq!(doc(&s).color_table.as_ref().unwrap().transparent, None);+}+ #[test] fn indexed_exact_transparency_and_sources() { // Two colours + transparency: exact palette with a transparent entry.@@ -776,6 +776,44 @@ fn replace_color_result(ui: &mut egui::Ui, f: &mut Map<String, Value>) { } } +fn indexed_matte_row(ui: &mut egui::Ui, fields: &mut Map<String, Value>) {+ let value = fields.get("matte").and_then(Value::as_str).unwrap_or("none");+ let mut choice = if value.starts_with('#') { "custom" } else { value };+ let color = crate::color_picker_ui::parse_hex(value).unwrap_or([1.0; 3]);+ let mut changed = false;+ let mut pick = false;+ ui.horizontal(|ui| {+ ui.label(egui::RichText::new(tl!("Matte")).color(Tokens::get(ui.ctx()).text_dim));+ changed = crate::widgets::dropdown(+ ui,+ "indexed-matte",+ "Matte",+ &mut choice,+ &[+ ("none", "None"),+ ("foreground", "Foreground Color"),+ ("background", "Background Color"),+ ("white", "White"),+ ("black", "Black"),+ ("custom", "Other…"),+ ],+ 170.0,+ );+ if choice == "custom" {+ let bytes = color.map(|v| (v.clamp(0.0, 1.0) * 255.0).round() as u8);+ pick = crate::widgets::color_swatch_button(ui, egui::Color32::from_rgb(bytes[0], bytes[1], bytes[2]), tl!("Matte")).clicked();+ }+ });+ let custom = choice == "custom";+ if changed {+ let next = if custom { crate::color_picker_ui::hex(color) } else { choice.to_owned() };+ fields.insert("matte".into(), json!(next));+ }+ if custom && (changed || pick) {+ crate::color_picker_ui::request_field(fields, "matte", &crate::color_picker_ui::title_for("Matte"));+ }+}+ /// Dialog body for filter commands. pub fn body(ui: &mut egui::Ui, f: &mut Map<String, Value>) { let t = Tokens::get(ui.ctx());@@ -792,6 +830,10 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map<String, Value>) { crate::select_mask_view::controls(ui, f); } for p in parse_spec(&spec) {+ if cmd == "image.mode.indexedColor" && p.key == "matte" {+ indexed_matte_row(ui, f);+ continue;+ } if zoom_center::owns(&cmd, f) && matches!(p.key.as_str(), "centerX" | "centerY") { if p.key == "centerX" { zoom_center::row(ui, f);@@ -1546,3 +1546,47 @@ fn replace_color_localized_color_clusters() { assert!(px(2)[1] > 0.5, "near the point: shifted to green {:?}", px(2)); assert!(px(38)[0] > 0.9 && px(38)[1] < 0.1, "far away: still red {:?}", px(38)); }++#[test]+fn indexed_matte_dropdown_and_picker_change_the_applied_edge_color() {+ use egui::accesskit::Role;+ use egui_kittest::{Harness, kittest::Queryable};++ for (choice, blue) in [("Black", 0.0), ("Other…", 0.25)] {+ let mut session = photocraft_engine::Session::new();+ session.execute("file.new", json!({"width": 2, "height": 1, "depth": 32, "background": "transparent"})).unwrap();+ session+ .edit("fixture", |doc, active| {+ let bounds = doc.bounds();+ doc.layer_mut(active.unwrap()).unwrap().surface_mut().unwrap().write_region(bounds, &[1.0, 0.0, 0.0, 0.75, 0.0, 0.0, 0.0, 0.0]);+ Ok(())+ })+ .unwrap();+ let mut app = PhotocraftApp::new(session, crate::Services::default());+ let mut harness = Harness::builder().with_size(egui::vec2(900.0, 1000.0)).build_ui_state(|ui, app| crate::dialogs::show(app, ui.ctx()), &mut app);+ PhotocraftApp::setup_context(&harness.ctx, crate::theme::ThemeKind::ALL[0]);+ let id = open(harness.state_mut(), "image.mode.indexedColor").unwrap();+ harness.state_mut().ui.dialog_mut(id).unwrap().fields.insert("palette".into(), json!("exact"));+ harness.run_steps(3);+ harness.query_all_by_role(Role::ComboBox).find(|node| node.value().as_deref() == Some("None")).expect("Matte dropdown").click();+ harness.run_steps(2);+ harness.get_by_label(choice).click();+ harness.run_steps(3);+ if choice == "Other…" {+ let picker = harness.state().ui.dialogs.last().unwrap().id;+ assert_ne!(picker, id);+ harness.state_mut().ui.dialog_mut(picker).unwrap().fields.insert("color".into(), json!("#0000ff"));+ crate::dialogs::confirm(harness.state_mut(), picker).unwrap();+ harness.state_mut().ui.dialogs.retain(|dialog| dialog.id != picker);+ harness.run_steps(3);+ }+ harness.get_by_label("OK").click();+ harness.run_steps(3);+ let doc = &harness.state().session.active().unwrap().doc;+ assert_eq!(doc.mode, photocraft_color::ColorMode::Indexed);+ let surface = doc.layers[0].surface().unwrap();+ let edge = surface.rgba(0, 0);+ assert!((edge[0] - 0.75).abs() <= 1.0 / 255.0 && edge[1] == 0.0 && (edge[2] - blue).abs() <= 1.0 / 255.0 && edge[3] == 1.0, "{choice}: {edge:?}");+ assert_eq!(surface.rgba(1, 0)[3], 0.0);+ }+}@@ -523,6 +523,19 @@ How the web shell (`apps/photocraft-web/src/web.rs`) differs from desktop: ## Indexed Color timings +Indexed Color converts the composited image from its composite ICC profile to encoded sRGB+before palette construction and quantization, preserving alpha. The resulting expanded RGB+samples and color table are tagged sRGB. Untagged/sRGB input retains the identity fast path;+the quantizer retains its stock path.++The stock Indexed Color dialog exposes Matte: None, Foreground Color, Background Color,+White, Black and Other… (the shared Color Picker). Automation accepts `matte` as `none`,+`foreground`, `background`, `white`, `black` or `#rrggbb`. A selected matte blends all+partially transparent edges; with Transparency on, fully clear pixels stay transparent.+With Transparency off, the selected matte also fills clear pixels. None retains the existing+hard-edged transparency/default flattening behavior. This follows Adobe's documented+[Indexed Color conversion options](https://helpx.adobe.com/photoshop/desktop/adjust-color/color-modes/conversion-options-for-indexed-color-images.html).+ `cargo run --release -p photocraft-engine --example indexed_color_perf -- image.png` measures palette construction, Floyd–Steinberg diffusion, and the complete full-resolution CPU preview path (proxy copy, engine command, result composition; excludes GPU upload/presentation).