diff --git a/apps/photocraft/Cargo.toml b/apps/photocraft/Cargo.toml index 505947e222..61c43ae9da 100644 --- a/apps/photocraft/Cargo.toml +++ b/apps/photocraft/Cargo.toml @@ -50,11 +50,12 @@ core-graphics2 = { version = "=0.6.1", default-features = false, features = ["di # safe API. Pinned exactly: review the diff before bumping. fmv-macos-events = "=0.1.0" # Safe AppKit calls only: turn off AppKit's title bar dragging (mac_window.rs) and find where the -# pointer is when files are dropped (cursor.rs; winit 0.30 drops carry no position). Already in +# pointer is when files are dropped (cursor.rs; winit 0.30 drops carry no position), and play the +# system alert sound for Beep When Done (services.rs, NSBeep). Already in # Cargo.lock (winit, photocraft-tablet); pinned exactly to the same versions. objc2 = "=0.6.4" objc2-foundation = { version = "=0.3.2", default-features = false, features = ["std", "NSArray", "NSEnumerator", "NSGeometry"] } -objc2-app-kit = { version = "=0.3.2", default-features = false, features = ["std", "NSApplication", "NSApplicationScripting", "NSEvent", "NSResponder", "NSView", "NSWindow"] } +objc2-app-kit = { version = "=0.3.2", default-features = false, features = ["std", "NSApplication", "NSApplicationScripting", "NSEvent", "NSGraphics", "NSResponder", "NSView", "NSWindow"] } # The macOS menu bar (mac_menu.rs): a standalone native menu crate (from the tauri-apps org, but # not Tauri: no webview, no JS), AppKit menus behind a safe API (no GTK with default features off). Pinned exactly: review the diff before bumping. muda = { version = "=0.21.1", default-features = false } diff --git a/apps/photocraft/src/services.rs b/apps/photocraft/src/services.rs index ba70d6e1df..96af7f9e25 100644 --- a/apps/photocraft/src/services.rs +++ b/apps/photocraft/src/services.rs @@ -692,10 +692,14 @@ fn platform_beep() { { let _ = winsafe::MessageBeep(winsafe::co::MBP::OK); } - #[cfg(not(target_os = "windows"))] + // The user's alert sound, at the alert volume set in System Settings ▸ Sound. The terminal + // bell below is silent for an app started from the Finder or the Dock. + #[cfg(target_os = "macos")] + objc2_app_kit::NSBeep(); + #[cfg(not(any(target_os = "windows", target_os = "macos")))] { - // No beep API among this app's other platforms' dependencies: write the terminal bell - // (audible when launched from a terminal, which is the same cue macOS terminals use). + // No beep API among the other platforms' dependencies: write the terminal bell (audible + // when launched from a terminal). use std::io::Write as _; let _ = std::io::stdout().write_all(b"\x07").and_then(|()| std::io::stdout().flush()); } diff --git a/contributors/people.toml b/contributors/people.toml index f8a49ee90e..08325ad358 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -217,3 +217,6 @@ sync_github_name = false [people.theshubh77] real_name = "Shubham Bhamare" sync_github_name = true + +[people.ksaxena149] +sync_github_name = true diff --git a/crates/automation/src/rpc.rs b/crates/automation/src/rpc.rs index 83d91238f4..c0369c917d 100644 --- a/crates/automation/src/rpc.rs +++ b/crates/automation/src/rpc.rs @@ -351,6 +351,19 @@ mod tests { Mutex::new(Headless::trusted_local()) } + #[test] + fn document_activation_reply_survives_engine_rpc() { + let h = session(); + let mut g = h.lock().unwrap(); + for _ in 0..2 { + g.handle("engine.execute", json!({"command": "file.new", "params": {"width": 4, "height": 4}})).unwrap(); + } + let reply = g.handle("engine.execute", json!({"command": "document.activate", "params": {"offset": -1}})).unwrap(); + assert_eq!(reply, json!({"document": 0})); + let reply = g.handle("engine.execute", json!({"command": "document.activate", "params": {"document": 1}})).unwrap(); + assert_eq!(reply, json!({"document": 1})); + } + #[test] fn execute_reports_invalid_filter_parameters_without_changing_history() { let h = session(); diff --git a/crates/engine/src/actions_cmds.rs b/crates/engine/src/actions_cmds.rs index 7685c43c4e..f831d63878 100644 --- a/crates/engine/src/actions_cmds.rs +++ b/crates/engine/src/actions_cmds.rs @@ -1146,6 +1146,43 @@ mod tests { } } + /// An Alt-drag copy records `inPlace` (#7200), which only matters for an artboard: a plain + /// layer's recorded copy must still export to .atn, while an artboard's must be refused rather + /// than exported as a copy beside the original. + #[test] + fn recorded_in_place_duplicate_keeps_in_place_only_for_artboards() { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + s.execute("actions.record", json!({"name": "Layer copy"})).unwrap(); + s.execute("layer.duplicate", json!({"inPlace": true})).unwrap(); + s.execute("actions.stop", json!({})).unwrap(); + s.execute("layer.new.artboard", json!({"name": "Board", "rect": [0, 0, 8, 8]})).unwrap(); + s.execute("actions.record", json!({"name": "Board copy"})).unwrap(); + s.execute("layer.duplicate", json!({"inPlace": true})).unwrap(); + s.execute("actions.stop", json!({})).unwrap(); + assert_eq!(s.actions.list[0].steps, vec![("layer.duplicate".to_string(), json!({}))]); + assert_eq!(s.actions.list[1].steps, vec![("layer.duplicate".to_string(), json!({"inPlace": true}))]); + assert!(crate::atn_cmds::export("Audit", &[&s.actions.list[0]], &[], false).is_ok()); + assert!(crate::atn_cmds::export("Audit", &[&s.actions.list[1]], &[], false).is_err()); + // Several selected layers are copied together: one artboard among them keeps `inPlace`. + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + let photo = s.active().unwrap().active_layer.unwrap(); + s.execute("layer.new.layer", json!({"name": "Logo"})).unwrap(); + s.execute("layer.select", json!({"layer": photo.0, "mode": "add"})).unwrap(); + s.execute("actions.record", json!({"name": "Layers copy"})).unwrap(); + s.execute("layer.duplicate", json!({"inPlace": true})).unwrap(); + s.execute("actions.stop", json!({})).unwrap(); + assert_eq!(s.active().unwrap().selected_layers().len(), 2, "both layers were copied"); + s.execute("layer.new.artboard", json!({"name": "Board", "rect": [0, 0, 8, 8]})).unwrap(); + s.execute("layer.select", json!({"layer": photo.0, "mode": "add"})).unwrap(); + s.execute("actions.record", json!({"name": "Mixed copy"})).unwrap(); + s.execute("layer.duplicate", json!({"inPlace": true})).unwrap(); + s.execute("actions.stop", json!({})).unwrap(); + assert_eq!(s.actions.list[0].steps, vec![("layer.duplicate".to_string(), json!({}))]); + assert_eq!(s.actions.list[1].steps, vec![("layer.duplicate".to_string(), json!({"inPlace": true}))]); + } + #[test] fn named_selection_preserves_modes_and_duplicate_occurrences() { let mut s = Session::new(); diff --git a/crates/engine/src/artboard_cmds.rs b/crates/engine/src/artboard_cmds.rs index 7d5d7bbb0e..d9d002ee84 100644 --- a/crates/engine/src/artboard_cmds.rs +++ b/crates/engine/src/artboard_cmds.rs @@ -183,6 +183,20 @@ pub(crate) fn place_copy(doc: &mut Document, id: LayerId) -> Result<()> { Ok(()) } +/// Record a Duplicate's `inPlace` only when a copy (the selection after it) is an artboard: for +/// any other layer it changes nothing, and Photoshop's Duplicate can't express it, so a recorded +/// Alt-drag copy of a plain layer must stay exportable to .atn (#7200). +pub(crate) fn recorded_duplicate(s: &Session, mut p: Value) -> Value { + let Some(st) = s.active() else { return p }; + if st.selected_layers().into_iter().any(|id| st.doc.layer(id).is_some_and(|l| l.artboard().is_some())) { + return p; + } + if let Some(fields) = p.as_object_mut() { + fields.remove("inPlace"); + } + p +} + /// Grow the canvas so every artboard fits. Photoshop's artboard documents grow in every /// direction: a board left of or above the origin shifts the whole document (boards, their /// contents, guides, paths…) right/down by its overhang, then the canvas grows to show it. An diff --git a/crates/engine/src/atn_cmds.rs b/crates/engine/src/atn_cmds.rs index bdfd7411ae..3b974a34a5 100644 --- a/crates/engine/src/atn_cmds.rs +++ b/crates/engine/src/atn_cmds.rs @@ -13,7 +13,9 @@ //! work; tool selections and other steps with no document effect are imported switched off. //! //! Layer references by name (`select` of layer "Shadow") become `{"layer": "Shadow"}`, which -//! `commands::layer_param` resolves in whatever document the action plays on (#219). +//! `commands::layer_param` resolves in whatever document the action plays on (#219). A switch +//! to another open document (`select` of `Dcmn` by offset) becomes `document.activate +//! {"offset"}`, the tab that many places from the active one (#7103). use photocraft_psd::atn::{self, AtnAction, AtnEvent, AtnFile, AtnItem}; use photocraft_psd::descriptor::{Class, Descriptor, Id, ReferenceItem, UnicodeString, Value as D}; @@ -590,6 +592,9 @@ fn curves_to(p: &Value) -> Option { Some(desc("Crvs").with("presetKind", enm("presetKindType", "presetKindCustom")).with("Adjs", D::List(entries))) } +/// The hue key of a Hue/Saturation adjustment (`Hst2`): Photoshop's `H ` (#8302). +const HUE_KEY: &str = "H "; + const HUE_RANGES: [&str; 6] = ["reds", "yellows", "greens", "cyans", "blues", "magentas"]; fn hue_sat_from(d: &Descriptor) -> Option { @@ -600,8 +605,11 @@ fn hue_sat_from(d: &Descriptor) -> Option { for e in get_list(d, "Adjs").unwrap_or(&[]) { let D::Descriptor(e) = e else { return None }; let mut m = Map::new(); - for (key, param) in [("Hue ", "hue"), ("Strt", "saturation"), ("Lght", "lightness")] { - if let Some(v) = get_num(e, key) { + // Photoshop's hue key is `H ` (stringID "hue", #8302); PhotoCraft wrote `Hue ` before, + // so its own older files still read. + let hue = get_num(e, HUE_KEY).or_else(|| get_num(e, "Hue ")); + for (param, v) in [("hue", hue), ("saturation", get_num(e, "Strt")), ("lightness", get_num(e, "Lght"))] { + if let Some(v) = v { m.insert(param.into(), v.into()); } } @@ -631,7 +639,7 @@ fn hue_entry(p: &Value, range: Option<(usize, &Value)>) -> Option { } } } - for (key, param) in [("Hue ", "hue"), ("Strt", "saturation"), ("Lght", "lightness")] { + for (key, param) in [(HUE_KEY, "hue"), ("Strt", "saturation"), ("Lght", "lightness")] { e = e.with(key, long(jnum(p, param).unwrap_or(0.0))); } Some(e) @@ -856,7 +864,8 @@ fn gradient_map_to(p: &Value) -> Option { Some(d) } -/// An adjustment: its Photoshop class (also the direct event's code), the command suffix (`image.adjustments.`, `layer.newAdjustmentLayer.`), and +/// An adjustment: its Photoshop class (usually also the direct event's code; Black & White and +/// Photo Filter run by name), the command suffix (`image.adjustments.`, `layer.newAdjustmentLayer.`), and /// the converters between the adjustment descriptor and the command params. struct Adjustment { class: &'static str, @@ -1672,6 +1681,8 @@ fn select_from(d: &Descriptor) -> Mapped { match get_enum(d, "selectionModifier").as_deref() { Some("addToSelection") => p["mode"] = json!("add"), Some("removeFromSelection") => p["mode"] = json!("remove"), + // Shift-click: select everything between the active layer and this one. + Some("addToSelectionContinuous") => p["mode"] = json!("range"), _ => {} } step("layer.select", p) @@ -1706,6 +1717,9 @@ fn select_from(d: &Descriptor) -> Mapped { Some(id) => none(id), None => Mapped::Unsupported { harmless: false }, }, + // Switching to another open document: its offset from the active one, -1 for the previous + // tab and 1 for the next (a template action copies from one and pastes into the other, #7103). + (Some("Dcmn"), Target::Offset(o)) => step("document.activate", json!({"offset": o})), _ => Mapped::Unsupported { harmless: false }, } } @@ -1727,6 +1741,7 @@ fn select_to(id: &str, p: &Value) -> Option { None | Some("replace") => {} Some("add") => d = d.with("selectionModifier", enm("selectionModifierType", "addToSelection")), Some("remove") => d = d.with("selectionModifier", enm("selectionModifierType", "removeFromSelection")), + Some("range") => d = d.with("selectionModifier", enm("selectionModifierType", "addToSelectionContinuous")), Some(_) => return None, } d.with("MkVs", D::Boolean(false)) @@ -1747,6 +1762,11 @@ fn select_to(id: &str, p: &Value) -> Option { "layer.addBelowToSelection" => desc("null").with("null", ord("Bckw")).with("selectionModifier", enm("selectionModifierType", "addToSelection")), "layer.selectTop" => desc("null").with("null", ord("Frnt")), "layer.selectBottom" => desc("null").with("null", ord("Back")), + // An absolute tab index has no Photoshop equivalent; an offset is what Photoshop records. + "document.activate" if p.get("document").is_none_or(Value::is_null) => { + let offset = i32::try_from(p.get("offset")?.as_i64()?).ok()?; + desc("null").with("null", D::Reference(vec![ReferenceItem::Offset { class: class("Dcmn"), offset }])) + } _ => { let item = lookup_back(MENU_ITEMS, id)?; desc("null").with("null", D::Reference(vec![ReferenceItem::Enumerated { class: class("Mn "), type_id: code("MnIt"), value: code(item) }])) @@ -2272,6 +2292,11 @@ fn duplicate_to(id: &str, p: &Value) -> Option { if id == "layer.duplicate" && jstr(p, "document") == Some("new") { return None; } + // Photoshop's Duplicate has no in-place flag: a plain one would place the artboard copy beside + // the original on playback. + if id == "layer.duplicate" && jbool(p, "inPlace") == Some(true) { + return None; + } let mut d = match id { "layer.duplicate" => desc("null").with("null", layer_ref(p)?), "image.duplicate" => { @@ -2806,13 +2831,21 @@ fn image_size_to(_: &str, p: &Value) -> Option { return None; } let mut d = desc("null"); - if let Some(pc) = jnum(p, "percent") { + // `resample: "none"` changes the resolution only and the command ignores any size then. The + // descriptor has no "none" (it is the absence of `Intr`), so writing the ignored size would + // turn the step into a pixel resize on playback (#7189). + let resolution_only = jstr(p, "resample") == Some("none"); + if resolution_only && jnum(p, "resolution").is_none() { + // Nothing left to record: the step changes nothing. + return None; + } + if let Some(pc) = jnum(p, "percent").filter(|_| !resolution_only) { d = d.with("Wdth", pct(pc)).with("Hght", pct(pc)).with("CnsP", D::Boolean(true)); } - if let Some(w) = jnum(p, "width") { + if let Some(w) = jnum(p, "width").filter(|_| !resolution_only) { d = d.with("Wdth", px(w)); } - if let Some(h) = jnum(p, "height") { + if let Some(h) = jnum(p, "height").filter(|_| !resolution_only) { d = d.with("Hght", px(h)); } if let Some(r) = jnum(p, "resolution") { @@ -2851,6 +2884,11 @@ fn crop_from(d: &Descriptor) -> Mapped { } fn crop_to(_: &str, p: &Value) -> Option { + // The W x H x Resolution output options have no field here: refuse rather than export a plain + // rectangle crop that keeps the old size and resolution on replay. + if ["targetWidth", "targetHeight", "resolution"].iter().any(|k| p.get(k).is_some_and(|v| !v.is_null())) { + return None; + } let mut d = desc("null"); if let (Some(x), Some(y), Some(w), Some(h)) = (jnum(p, "x"), jnum(p, "y"), jnum(p, "width"), jnum(p, "height")) { d = d.with("To ", D::Descriptor(desc("Rctn").with("Top ", px(y)).with("Left", px(x)).with("Btom", px(y + h)).with("Rght", px(x + w)))); @@ -3314,7 +3352,8 @@ const EVENTS: &[Event] = &[ adjustment_event!("ClrB", "ClrB", "colorBalance", "Color Balance"), adjustment_event!("Vbrn", "Vbrn", "vibrance", "Vibrance"), adjustment_event!("Exps", "Exps", "exposure", "Exposure"), - adjustment_event!("PhtF", "PhtF", "photoFilter", "Photo Filter"), + // Photoshop runs Photo Filter by name only: `PhtF` is the class, not an event (#8301). + adjustment_event!("", "PhtF", "photoFilter", "Photo Filter"), adjustment_event!("", "BlnW", "blackAndWhite", "Black & White", "image.adjustments.blackWhite"), adjustment_event!("ChnM", "ChnM", "channelMixer", "Channel Mixer"), adjustment_event!("GdMp", "GdMp", "gradientMapEvent", "Gradient Map", "image.adjustments.gradientMap"), @@ -3469,6 +3508,7 @@ const EVENTS: &[Event] = &[ "view.actualPixels", "view.zoomIn", "view.zoomOut", + "document.activate", ], from: select_from, to: select_to, @@ -3638,7 +3678,8 @@ const EVENTS: &[Event] = &[ /// these codes fails ("the command is not currently available"), and `Fltn` is the /// `flatness` property, not the flatten event. Import keeps accepting them so actions saved /// before still load; export never writes them. -const LEGACY_CODES: &[(&str, &str)] = &[("BoxB", "boxblur"), ("SrfB", "surfaceBlur"), ("Fltn", "flattenImage"), ("BlnW", "blackAndWhite")]; +const LEGACY_CODES: &[(&str, &str)] = + &[("BoxB", "boxblur"), ("SrfB", "surfaceBlur"), ("Fltn", "flattenImage"), ("BlnW", "blackAndWhite"), ("PhtF", "photoFilter")]; fn event_for(item: &AtnEvent) -> Option<&'static Event> { let id = item.id(); @@ -3782,6 +3823,29 @@ fn export_item(id: &str, params: &Value, set: &str) -> Option { Some(AtnItem { event, dictionary: e.label.as_bytes().to_vec(), descriptor: Some(descriptor), ..Default::default() }) } +/// Photoshop applies only the rename when a Set mixes it with opacity or blend mode (#8304). +/// Write each layer property separately, renaming last so named references still resolve. +fn export_items(id: &str, params: &Value, set: &str) -> Option> { + let item = export_item(id, params, set)?; + if id != "layer.setProps" { + return Some(vec![item]); + } + let descriptor = item.descriptor.as_ref()?; + let properties = get_obj(descriptor, "T ")?; + if properties.items.len() <= 1 { + return Some(vec![item]); + } + let target = descriptor.get("null")?; + let mut items = Vec::with_capacity(properties.items.len()); + for (key, value) in properties.items.iter().filter(|(k, _)| !k.is("Nm ")).chain(properties.items.iter().filter(|(k, _)| k.is("Nm "))) { + let mut property = properties.clone(); + property.items = vec![(key.clone(), value.clone())]; + let descriptor = desc("null").with("null", target.clone()).with("T ", D::Descriptor(property)); + items.push(AtnItem { descriptor: Some(descriptor), ..item.clone() }); + } + Some(items) +} + /// An action's key as a shortcut string: `F5`, with Photoshop's Shift and Command options /// `Shift+F5`, `Cmd+F5` or `Cmd+Shift+F5` (#7646). pub fn function_key_name(key: u16, shift: bool, command: bool) -> String { @@ -3822,7 +3886,7 @@ pub fn export( (out.function_key, out.shift, out.command) = (n, shift, command); } for (i, (id, params)) in a.steps.iter().enumerate() { - let Some(mut item) = export_item(id, params, set) else { + let Some(items) = export_items(id, params, set) else { if skip_unsupported { skipped.push(Skipped { action: a.name.clone(), step: i, id: id.clone() }); continue; @@ -3837,10 +3901,12 @@ pub fn export( a.name )); }; - item.enabled = a.step_enabled(i); - item.with_dialog = a.dialogs.contains(&i); - item.dialog_options = if item.with_dialog { 2 } else { 0 }; - out.items.push(item); + for mut item in items { + item.enabled = a.step_enabled(i); + item.with_dialog = a.dialogs.contains(&i); + item.dialog_options = if item.with_dialog { 2 } else { 0 }; + out.items.push(item); + } } file.actions.push(out); } @@ -4059,3 +4125,6 @@ pub fn specs() -> Vec { #[cfg(test)] mod tests; + +#[cfg(test)] +mod layer_props_tests; diff --git a/crates/engine/src/atn_cmds/layer_props_tests.rs b/crates/engine/src/atn_cmds/layer_props_tests.rs new file mode 100644 index 0000000000..424af7bd55 --- /dev/null +++ b/crates/engine/src/atn_cmds/layer_props_tests.rs @@ -0,0 +1,151 @@ +use super::*; + +/// PhotoCraft's own roundtrip accepts a combined Set, but Photoshop applies only its rename. +#[test] +fn compound_properties_export_as_separate_sets_with_rename_last() { + for target in [None, Some("Original")] { + let mut params = json!({"name": "Renamed", "opacity": 0.37, "blend": "Multiply"}); + if let Some(name) = target { + params["layer"] = json!(name); + } + let action = Action::new("Properties", vec![("layer.setProps".into(), params.clone())]); + let (bytes, skipped) = export("Set", &[&action], &[], false).unwrap(); + assert!(skipped.is_empty()); + let file = atn::parse(&bytes).unwrap(); + let items = &file.actions[0].items; + assert_eq!(items.len(), 3); + let expected = [("Opct", pct(37.0)), ("Md ", enm("BlnM", "Mltp")), ("Nm ", text("Renamed"))]; + for (item, (key, value)) in items.iter().zip(expected) { + assert_eq!(item.event.id(), "setd"); + let descriptor = item.descriptor.as_ref().unwrap(); + assert_eq!(descriptor.get("null"), Some(&layer_ref(¶ms))); + let properties = get_obj(descriptor, "T ").unwrap(); + assert!(properties.class_id.is("Lyr ")); + assert_eq!(properties.items.len(), 1, "Photoshop must receive each property independently"); + assert_eq!(properties.get(key), Some(&value)); + } + } +} + +#[test] +fn expanded_properties_keep_flags_and_original_skip_indices() { + let mut action = Action::new( + "Flags", + vec![ + ("layer.setProps".into(), json!({"name": "Renamed", "opacity": 0.37, "blend": "Multiply"})), + ("filter.blur.gaussianBlur".into(), json!({"radius": 2})), + ("filter.render.clouds".into(), json!({})), + ], + ); + action.disabled.insert(0); + action.dialogs.insert(0); + let (bytes, skipped) = export("Set", &[&action], &[], true).unwrap(); + assert_eq!(skipped, [Skipped { action: "Flags".into(), step: 2, id: "filter.render.clouds".into() }]); + let file = atn::parse(&bytes).unwrap(); + let items = &file.actions[0].items; + assert_eq!(items.len(), 4); + for item in &items[..3] { + assert!(!item.enabled); + assert!(item.with_dialog); + assert_eq!(item.dialog_options, 2); + } + assert_eq!(items[3].event.id(), "GsnB"); + assert!(items[3].enabled); + assert!(!items[3].with_dialog); + assert_eq!(items[3].dialog_options, 0); + let back = import(&bytes).unwrap(); + assert_eq!(back.actions[0].disabled.iter().copied().collect::>(), [0, 1, 2]); + assert_eq!(back.actions[0].dialogs.iter().copied().collect::>(), [0, 1, 2]); +} + +#[test] +fn invalid_compound_properties_fail_or_skip_the_whole_source_step() { + let action = Action::new( + "Atomic", + vec![ + ("filter.blur.gaussianBlur".into(), json!({"radius": 2})), + ("layer.setProps".into(), json!({"name": "Renamed", "opacity": 0.37, "blend": "invalid blend"})), + ("select.inverse".into(), json!({})), + ], + ); + let error = export("Set", &[&action], &[], false).unwrap_err(); + assert!(error.contains("step 2"), "{error}"); + let (bytes, skipped) = export("Set", &[&action], &[], true).unwrap(); + assert_eq!(skipped, [Skipped { action: "Atomic".into(), step: 1, id: "layer.setProps".into() }]); + let file = atn::parse(&bytes).unwrap(); + let events: Vec<_> = file.actions[0].items.iter().map(|item| item.event.id()).collect(); + assert_eq!(events, ["GsnB", "Invs"]); +} + +#[test] +fn single_layer_properties_keep_their_existing_descriptor() { + for params in [json!({"name": "Renamed"}), json!({"opacity": 0.37}), json!({"blend": "Multiply"}), json!({"fill": 0.5}), json!({"visible": false})] { + let action = Action::new("Single", vec![("layer.setProps".into(), params.clone())]); + let expected = export_item("layer.setProps", ¶ms, "Set").unwrap(); + let (bytes, skipped) = export("Set", &[&action], &[], false).unwrap(); + assert!(skipped.is_empty()); + let file = atn::parse(&bytes).unwrap(); + assert_eq!(file.actions[0].items.len(), 1); + assert_eq!(file.actions[0].items[0].event, expected.event); + assert_eq!(file.actions[0].items[0].descriptor, expected.descriptor); + } +} + +#[test] +fn fill_and_visibility_also_export_separately_before_rename() { + let action = Action::new("Other properties", vec![("layer.setProps".into(), json!({"name": "Renamed", "fill": 0.5, "visible": false}))]); + let (bytes, _) = export("Set", &[&action], &[], false).unwrap(); + let file = atn::parse(&bytes).unwrap(); + let keys: Vec<_> = file.actions[0] + .items + .iter() + .map(|item| { + let properties = get_obj(item.descriptor.as_ref().unwrap(), "T ").unwrap(); + assert_eq!(properties.items.len(), 1); + trimmed(&properties.items[0].0) + }) + .collect(); + assert_eq!(keys, ["fillOpacity", "Vsbl", "Nm"]); +} + +#[test] +fn expanded_properties_replay_like_direct_edits_at_every_depth() { + for depth in [8, 16, 32] { + for named in [false, true] { + let prepare = || { + let mut session = Session::new(); + session.execute("file.new", json!({"width": 2, "height": 2, "depth": depth})).unwrap(); + session.execute("layer.new.layer", json!({"name": "Original"})).unwrap(); + session.execute("layer.new.layer", json!({"name": "Other"})).unwrap(); + if !named { + session.execute("layer.select", json!({"layer": "Original"})).unwrap(); + } + session + }; + let mut params = json!({"name": "Renamed", "opacity": 0.37, "blend": "Multiply"}); + if named { + params["layer"] = json!("Original"); + } + let mut direct = prepare(); + direct.execute("layer.setProps", params.clone()).unwrap(); + let action = Action::new("Properties", vec![("layer.setProps".into(), params)]); + let (bytes, _) = export("Set", &[&action], &[], false).unwrap(); + let mut replay = prepare(); + replay.actions.list = import(&bytes).unwrap().actions; + let result = replay.execute("actions.play", json!({"action": "Properties"})).unwrap(); + assert!(result["failed"].is_null(), "{result}"); + assert_eq!(result["ran"], 3); + let state = |session: &Session| { + session + .active() + .unwrap() + .doc + .walk() + .iter() + .map(|(_, _, layer)| (layer.name.clone(), layer.opacity, layer.blend, layer.visible)) + .collect::>() + }; + assert_eq!(state(&direct), state(&replay), "depth={depth}, named={named}"); + } + } +} diff --git a/crates/engine/src/atn_cmds/tests.rs b/crates/engine/src/atn_cmds/tests.rs index bd08c8bb42..a50e4ceebd 100644 --- a/crates/engine/src/atn_cmds/tests.rs +++ b/crates/engine/src/atn_cmds/tests.rs @@ -479,6 +479,7 @@ fn every_mapped_command_exports_and_imports_back() { ("layer.setProps", json!({"layer": "Shadow", "visible": false})), ("layer.setLabelColor", json!({"color": "red"})), ("layer.select", json!({"layer": "Shadow", "mode": "add"})), + ("layer.select", json!({"layer": "Shadow", "mode": "range"})), ("layer.selectTop", json!({})), ("layer.selectBelow", json!({})), ("layer.addBelowToSelection", json!({})), @@ -553,6 +554,7 @@ fn every_mapped_command_exports_and_imports_back() { ("layerComp.apply", json!({"comp": "Original"})), ("actions.play", json!({"action": "Other", "set": "S"})), (STOP_MESSAGE, json!({"message": "Hi", "allowContinue": false})), + ("document.activate", json!({"offset": -1})), // The TrueGrit Texture Supply sets (Debaser, Distress Press, Halftone Zine Machine). ("filter.distort.ripple", json!({"amount": 50, "size": "medium"})), ("filter.pixelate.crystallize", json!({"cellSize": 5})), @@ -1388,6 +1390,74 @@ fn fsel(t: D) -> Descriptor { desc("null").with("null", D::Reference(vec![ReferenceItem::Property { class: class("Chnl"), key: Id::new("fsel") }])).with("T ", t) } +/// #7103: Photoshop records switching to another open document as a `select` of `Dcmn` by +/// offset (previous -1, next +1). The template action of the report, which copies from the +/// source image and pastes into the template opened after it, imports with no Photoshop steps, +/// plays across the two documents and saves back the same shape. With no previous document +/// open, playback stops at the switch instead of pasting into the wrong document. +#[test] +fn select_document_by_offset_imports_plays_and_saves_back() { + let doc = |offset: i32| desc("null").with("null", D::Reference(vec![ReferenceItem::Offset { class: class("Dcmn"), offset }])); + // A recorded document id (`DocI`) means nothing in another session: ignored, not written back. + let copy_in = vec![ + item("slct", "Select", Some(doc(-1).with("DocI", long(35.0)))), + item("setd", "Set", Some(fsel(enm("Ordn", "Al ")))), + item("copy", "Copy", None), + item("slct", "Select", Some(doc(1))), + item("past", "Paste", None), + ]; + let data = set_of("Template", vec![("Copy In", copy_in)]); + + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8, "name": "source"})).unwrap(); + s.execute("edit.fill", json!({"contents": "color", "color": "#ff0000"})).unwrap(); + s.execute("file.new", json!({"width": 8, "height": 8, "name": "template", "background": "transparent"})).unwrap(); + let r = s.execute("actions.load", json!({"data": data})).unwrap(); + assert_eq!(r["unsupported"], json!([]), "{r}"); + let steps = &s.actions.list[0].steps; + assert_eq!(steps[0], ("document.activate".to_string(), json!({"offset": -1}))); + assert_eq!(steps[3], ("document.activate".to_string(), json!({"offset": 1}))); + + let pixel = |s: &mut Session| -> Vec { serde_json::from_value(s.execute("document.pixel", json!({"x": 4, "y": 4})).unwrap()).unwrap() }; + assert_eq!(pixel(&mut s), [0.0; 4]); + let r = s.execute("actions.play", json!({"action": "Copy In"})).unwrap(); + assert!(r.get("failed").is_none(), "{r}"); + assert_eq!(s.active().unwrap().doc.name, "template"); + assert_eq!(s.active().unwrap().doc.layers.len(), 2, "the source's pixels are pasted on a new layer"); + assert_eq!(pixel(&mut s), [1.0, 0.0, 0.0, 1.0]); + assert_eq!(s.documents()[0].doc.layers.len(), 1, "the source is only copied from"); + + let first = s.actions.list.clone(); + let r = s.execute("actions.save", json!({"set": "Template"})).unwrap(); + assert_eq!(r["skipped"], json!([])); + let saved = photocraft_paint::tile::b64_decode(r["data"].as_str().unwrap()).unwrap(); + let file = atn::parse(&saved).unwrap(); + let offsets: Vec = file.actions[0] + .items + .iter() + .filter_map(|i| match get_ref(i.descriptor.as_ref()?, "null")? { + [ReferenceItem::Offset { class, offset }] if trimmed(&class.class_id) == "Dcmn" => Some(*offset), + _ => None, + }) + .collect(); + assert_eq!(offsets, [-1, 1]); + assert!(file.actions[0].items[0].descriptor.as_ref().unwrap().get("DocI").is_none()); + let mut t = Session::new(); + t.execute("actions.load", json!({"data": b64(&saved)})).unwrap(); + assert_eq!(t.actions.list, first); + // An absolute tab index has no Photoshop equivalent: it stays out of a strict save. + assert!(export_item("document.activate", &json!({"document": 0}), "S").is_none()); + assert!(export_item("document.activate", &json!({"document": 0, "offset": 1}), "S").is_none()); + + let mut one = Session::new(); + one.execute("file.new", json!({"width": 8, "height": 8, "name": "template"})).unwrap(); + one.execute("actions.load", json!({"data": data})).unwrap(); + let r = one.execute("actions.play", json!({"action": "Copy In"})).unwrap(); + assert_eq!(r["failed"]["step"], 0, "{r}"); + assert_eq!(r["failed"]["error"], "no document 1 before the active one, which is 1 of 1", "{r}"); + assert_eq!(one.active().unwrap().doc.layers.len(), 1); +} + fn set_of(name: &str, actions: Vec<(&str, Vec)>) -> String { let file = AtnFile { name: UnicodeString::new_nul(name), @@ -1556,17 +1626,19 @@ fn remove_white_clears_the_targeted_layer_not_the_converted_background() { assert!(bottom.surface().unwrap().sample_channel(2, 2, 1) > 0.99, "and kept its pixels"); } -/// Measured in Photoshop: `executeAction` fails for the codes `BoxB`, `SrfB`, `Fltn` and -/// `BlnW` ("the command is not currently available"), while the names `boxblur`, -/// `surfaceBlur`, `flattenImage`, `blackAndWhite` and the code `FltI` run. +/// Measured in Photoshop: `executeAction` fails for the codes `BoxB`, `SrfB`, `Fltn`, `BlnW` +/// and `PhtF` ("the command is not currently available"), while the names `boxblur`, +/// `surfaceBlur`, `flattenImage`, `blackAndWhite`, `photoFilter` and the code `FltI` run. #[test] fn events_photoshop_only_runs_by_name_export_by_name_and_old_codes_still_import() { let bw = json!({"reds": 40, "yellows": 60, "greens": 40, "cyans": 60, "blues": 20, "magentas": 80, "tint": false, "tintColor": "#e4d5b0"}); + let photo_filter = json!({"color": "#0022cd", "density": 67, "preserveLuminosity": false}); let cases = [ ("filter.blur.boxBlur", json!({"radius": 3}), "boxblur", "BoxB"), ("filter.blur.surfaceBlur", json!({"radius": 5, "threshold": 15}), "surfaceBlur", "SrfB"), ("layer.flattenImage", json!({}), "FltI", "Fltn"), ("image.adjustments.blackWhite", bw, "blackAndWhite", "BlnW"), + ("image.adjustments.photoFilter", photo_filter, "photoFilter", "PhtF"), ]; for (id, params, written, legacy) in &cases { let exported = export_item(id, params, "S").unwrap_or_else(|| panic!("{id} does not export")); @@ -1613,7 +1685,7 @@ fn events_photoshop_only_runs_by_name_export_by_name_and_old_codes_still_import( // Unknown ids, near misses and the legacy codes written as names stay Photoshop steps, // never a panic or a wrong command. - for event in ["BoxBlur", "boxBlur", "BOXB", "Fltn ", "FLTI", "", "\u{0}\u{0}\u{0}\u{0}", "BlnWx"] { + for event in ["BoxBlur", "boxBlur", "BOXB", "Fltn ", "FLTI", "", "\u{0}\u{0}\u{0}\u{0}", "BlnWx", "PHTF", "PhtF ", "photofilter"] { let e = event_for(&item(event, "", None).event); assert!(e.is_none(), "{event:?} maps to {:?}", e.map(|e| e.ids)); } @@ -1627,6 +1699,72 @@ fn events_photoshop_only_runs_by_name_export_by_name_and_old_codes_still_import( } } +/// #8301: Photoshop stops on a Photo Filter step saved as `PhtF` ("not currently available") +/// and runs the same descriptor under the name `photoFilter`. A recorded step saves by name, +/// a Photoshop-authored step re-saves by name, and actions saved before with `PhtF` still load +/// and play the same. +#[test] +fn photo_filter_saves_under_the_name_photoshop_runs() { + let params = json!({"color": "#0022cd", "density": 67, "preserveLuminosity": false}); + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8, "background": "#808080"})).unwrap(); + s.execute("actions.record", json!({"name": "Filter"})).unwrap(); + s.execute("image.adjustments.photoFilter", params.clone()).unwrap(); + s.execute("actions.stop", json!({})).unwrap(); + let saved = s.execute("actions.save", json!({"action": "Filter"})).unwrap(); + assert_eq!(saved["skipped"], json!([]), "{saved}"); + let file = atn::parse(&photocraft_paint::tile::b64_decode(saved["data"].as_str().unwrap()).unwrap()).unwrap(); + let step = &file.actions[0].items[0]; + assert_eq!(step.event, AtnEvent::Name("photoFilter".into()), "never the code PhtF"); + assert_eq!(step.dictionary, "Photo Filter".as_bytes()); + let d = step.descriptor.clone().unwrap(); + for key in ["Clr ", "Dnst", "PrsL"] { + assert!(d.get(key).is_some(), "the descriptor keeps {key}: {d:?}"); + } + + // A Photoshop-authored step (by name) and an old PhotoCraft one (`PhtF`) both load to the + // command, and saving them again writes the name. + let data = + set_of("S", vec![("Photoshop", vec![item("photoFilter", "Photo Filter", Some(d.clone()))]), ("Legacy", vec![item("PhtF", "Photo Filter", Some(d))])]); + let mut t = Session::new(); + let r = t.execute("actions.load", json!({"data": data})).unwrap(); + assert_eq!(r["unsupported"], json!([]), "{r}"); + for a in &t.actions.list { + assert_eq!(a.steps.len(), 1, "{}", a.name); + assert_eq!(a.steps[0].0, "image.adjustments.photoFilter", "{}", a.name); + let mut p = a.steps[0].1.clone(); + normalise(&mut p); + assert_eq!(p, params, "{}", a.name); + } + let resaved = t.execute("actions.save", json!({"set": "S"})).unwrap(); + assert_eq!(resaved["skipped"], json!([]), "{resaved}"); + let file = atn::parse(&photocraft_paint::tile::b64_decode(resaved["data"].as_str().unwrap()).unwrap()).unwrap(); + let events: Vec = file.actions.iter().flat_map(|a| a.items.iter().map(|i| i.event.id())).collect(); + assert_eq!(events, ["photoFilter", "photoFilter"]); + + // Both play like the command run directly, at 8 and 16 bits. + for depth in [8, 16] { + let pixel = |s: &Session| { + let st = s.active().unwrap(); + let surface = st.doc.layers.first().unwrap().surface().unwrap(); + [0, 1, 2].map(|c| surface.sample_channel(3, 3, c)) + }; + let new_doc = |t: &mut Session| { + t.execute("file.new", json!({"width": 8, "height": 8, "background": "#808080", "depth": depth})).unwrap(); + }; + new_doc(&mut t); + t.execute("image.adjustments.photoFilter", params.clone()).unwrap(); + let direct = pixel(&t); + assert_ne!(direct, [128.0 / 255.0; 3], "the filter changes the grey"); + for action in ["Photoshop", "Legacy"] { + new_doc(&mut t); + let r = t.execute("actions.play", json!({"action": action})).unwrap(); + assert!(r.get("failed").is_none(), "{action}: {r}"); + assert_eq!(pixel(&t), direct, "{action} at {depth} bits"); + } + } +} + /// Smart Blur's quality uses Photoshop's `SmBQ` codes (`SBQL`/`SBQM`/`SBQH`): measured in /// Photoshop, the old `Hgh `/`Mdm `/`Low ` play back as its fallback quality. Old codes still read. #[test] @@ -2103,6 +2241,24 @@ fn add_noise_with_mask_target_is_not_exported_as_pixel_filter() { } } +/// Photoshop's Duplicate Layer has no in-place flag: an in-place duplicate (the artboard copy +/// that stays on the original) must not be exported as a plain duplicate, which places the +/// copy beside the original on playback. +#[test] +fn duplicate_in_place_is_not_exported_as_plain_duplicate() { + let in_place = Action { name: "Probe".into(), steps: vec![("layer.duplicate".into(), json!({"inPlace": true}))], ..Default::default() }; + assert!(export("Audit5", &[&in_place], &[], false).is_err()); + let (_, skipped) = export("Audit5", &[&in_place], &[], true).unwrap(); + assert_eq!(skipped, vec![Skipped { action: "Probe".into(), step: 0, id: "layer.duplicate".into() }]); + for params in [json!({}), json!({"inPlace": false})] { + let plain = Action { name: "Plain".into(), steps: vec![("layer.duplicate".into(), params)], ..Default::default() }; + let (bytes, skipped) = export("Audit5", &[&plain], &[], false).unwrap(); + assert!(skipped.is_empty()); + let imported = import(&bytes).unwrap(); + assert_eq!(imported.actions[0].steps, vec![("layer.duplicate".to_string(), json!({}))]); + } +} + /// A Play Action that starts the called action at a later step must not export as a call of the /// whole action: that would replay the steps the recording deliberately skipped. #[test] @@ -2408,3 +2564,91 @@ fn radial_blur_off_center_strict_export_refuses_silent_recentering() { let saved_pixels = s.execute("actions.save", json!({"action": "PixelsTarget"})).unwrap(); assert_eq!(saved_pixels["steps"], 1); } + +/// A crop with W x H x Resolution output options must not export as a plain rectangle crop +/// that keeps the old size and resolution on replay: the mapper has no field for them. +#[test] +fn crop_with_output_size_or_resolution_is_not_exported_as_plain_crop() { + for params in [ + json!({"x": 1, "y": 1, "width": 4, "height": 3, "targetWidth": 8, "targetHeight": 6, "resolution": 144, "resample": "nearest"}), + json!({"x": 1, "y": 1, "width": 4, "height": 3, "resolution": 144}), + json!({"x": 1, "y": 1, "width": 4, "height": 3, "targetWidth": 8, "targetHeight": 6}), + ] { + let sized = Action { name: "Probe".into(), steps: vec![("image.crop".into(), params)], ..Default::default() }; + assert!(export("Audit", &[&sized], &[], false).is_err()); + let (_, skipped) = export("Audit", &[&sized], &[], true).unwrap(); + assert_eq!(skipped, vec![Skipped { action: "Probe".into(), step: 0, id: "image.crop".into() }]); + } + let plain = Action { name: "Plain".into(), steps: vec![("image.crop".into(), json!({"x": 1, "y": 1, "width": 4, "height": 3}))], ..Default::default() }; + let (bytes, skipped) = export("Audit", &[&plain], &[], false).unwrap(); + assert!(skipped.is_empty()); + let imported = import(&bytes).unwrap(); + assert_eq!(imported.actions[0].steps, vec![("image.crop".to_string(), json!({"x": 1, "y": 1, "width": 4, "height": 3, "deleteCroppedPixels": true}))]); +} + +/// `image.imageSize` with `resample: "none"` ignores width, height and percent; the `.atn` +/// round trip must not turn them into a pixel resize (#7189). +#[test] +fn image_size_resample_none_keeps_ignoring_its_size_through_atn_export() { + let event = event_for(&export_item("image.imageSize", &json!({"resolution": 72}), "S").expect("exports").event).expect("a known event"); + let back = |p: Value| { + let item = export_item("image.imageSize", &p, "S")?; + let Mapped::Steps(steps) = (event.from)(item.descriptor.as_ref().unwrap()) else { panic!("does not import back") }; + let mut back = steps[0].1.clone(); + normalise(&mut back); + Some(back) + }; + for sized in [json!({"width": 12, "height": 10}), json!({"width": 12}), json!({"percent": 200}), json!({"width": 12, "percent": 50, "height": 3})] { + let mut p = sized.clone(); + p["resolution"] = json!(144); + p["resample"] = json!("none"); + assert_eq!(back(p.clone()), Some(json!({"resolution": 144, "resample": "none"})), "{p}"); + } + // A step with nothing left to record is refused rather than exported as an empty (unsupported) event. + assert_eq!(back(json!({"width": 12, "height": 10, "resample": "none"})), None); + // Real resizes are untouched. + assert_eq!(back(json!({"width": 12, "height": 10, "resample": "bicubic"})), Some(json!({"width": 12, "height": 10, "resample": "bicubic"}))); +} + +/// #8302, #8308: Hue/Saturation's hue is `H ` in Photoshop's `Hst2`, both ways. It was written +/// and read as `Hue `: exported actions failed in Photoshop, and Photoshop-recorded ones lost +/// their hue on import with no warning. PhotoCraft's own older files (`Hue `) still read. +#[test] +fn hue_saturation_uses_photoshops_hue_key() { + let params = json!({"colorize": false, "hue": 30, "saturation": 20, "lightness": -7, "greens": {"hue": -12, "saturation": 0, "lightness": 0, "range": [75, 105, 135, 165]}}); + let a = Action::new("H", vec![("image.adjustments.hueSaturation".into(), params.clone())]); + let (bytes, skipped) = export("S", &[&a], &[], false).unwrap(); + assert!(skipped.is_empty()); + let file = atn::parse(&bytes).unwrap(); + let d = file.actions[0].items[0].descriptor.clone().expect("a descriptor"); + let entries: Vec = + get_list(&d, "Adjs").expect("Adjs").iter().filter_map(|e| if let D::Descriptor(e) = e { Some(e.clone()) } else { None }).collect(); + assert_eq!(entries.len(), 2); + assert_eq!(get_num(&entries[0], "H "), Some(30.0), "the master hue under H"); + assert_eq!(get_num(&entries[1], "H "), Some(-12.0), "a range's hue under H"); + assert!(entries.iter().all(|e| get_num(e, "Hue ").is_none()), "no Hue key"); + + // A Photoshop-recorded step (H) and an older PhotoCraft one (Hue ) both import with the hue. + let step = |key: &str| { + desc("null") + .with("presetKind", enm("presetKindType", "presetKindCustom")) + .with("Clrz", D::Boolean(false)) + .with("Adjs", D::List(vec![D::Descriptor(desc("Hst2").with(key, long(30.0)).with("Strt", long(20.0)).with("Lght", long(-7.0)))])) + }; + for key in ["H ", "Hue "] { + let e = event_for(&item("HStr", "", None).event).expect("HStr imports"); + match (e.from)(&step(key)) { + Mapped::Steps(steps) => { + assert_eq!(steps[0].0, "image.adjustments.hueSaturation"); + assert_eq!(steps[0].1["hue"], json!(30.0), "{key:?}: {}", steps[0].1); + assert_eq!(steps[0].1["saturation"], json!(20.0)); + } + Mapped::Unsupported { .. } => panic!("{key:?} is unsupported"), + } + } + // Through a whole file, saved and loaded back: the hue survives. + let back = import(&bytes).unwrap(); + let mut p = back.actions[0].steps[0].1.clone(); + normalise(&mut p); + assert_eq!(p["hue"], json!(30), "{p}"); +} diff --git a/crates/engine/src/automate_cmds.rs b/crates/engine/src/automate_cmds.rs index 38ad1f55d0..2256a76cda 100644 --- a/crates/engine/src/automate_cmds.rs +++ b/crates/engine/src/automate_cmds.rs @@ -314,9 +314,11 @@ fn create_droplet(s: &mut Session, p: &Value) -> Result { let path = p.get("path").and_then(Value::as_str).filter(|v| !v.is_empty()).ok_or_else(|| bad(cmd, "missing \"path\" (where to save the droplet)"))?; let steps_v = p.get("steps").or_else(|| p.get("action")).ok_or_else(|| bad(cmd, "missing \"steps\" (the action to run)"))?; let mut steps = parse_steps(steps_v, cmd)?; - // The droplet's output folder replaces recorded saves, and view steps don't apply, as in batch. - steps.retain(|(id, _)| !crate::actions_cmds::shell_save_command(id) && !crate::actions_cmds::shell_view_command(id)); - check_known(&steps, cmd)?; + // As in batch, the droplet's output folder replaces recorded saves, which are kept to mark the + // result, and view steps don't apply. + steps.retain(|(id, _)| crate::file_cmds::batch_keeps_step(id)); + let commands: Vec<_> = steps.iter().filter(|(id, _)| !crate::file_cmds::batch_save_point(id)).cloned().collect(); + check_known(&commands, cmd)?; // Play Action steps need their actions when the droplet runs on its own (#2794). Not under // `actions`: the CLI reads that key as an action set. let called = crate::actions_cmds::called_actions(&s.actions.list, &mut steps); diff --git a/crates/engine/src/automate_cmds/tests.rs b/crates/engine/src/automate_cmds/tests.rs index 322e6fb5b3..8bea15c69a 100644 --- a/crates/engine/src/automate_cmds/tests.rs +++ b/crates/engine/src/automate_cmds/tests.rs @@ -166,6 +166,24 @@ fn droplets_are_written_and_run() { assert!(s.execute("file.automate.createDroplet", json!({"path": path, "steps": [["bogus", {}]]})).is_err()); } +/// A droplet keeps the action's recorded saves, which mark the result as in Batch, so one made from +/// an action ending in Save As and Close writes its files instead of failing on every one. +#[test] +fn droplets_keep_the_recorded_save_that_marks_the_result() { + let dir = tmp("droplet-save-close"); + let inputs = images(&dir, 1, 4, 2); + let mut s = Session::new(); + let steps = json!([["image.adjustments.invert", {}], ["file.saveAs", {}], ["file.close", {}]]); + let p = json!({"path": format!("{dir}/Invert"), "steps": steps, "format": "png", "output": format!("{dir}/out"), "shim": false}); + let dp = s.execute("file.automate.createDroplet", p).unwrap()["path"].as_str().unwrap().to_string(); + let r = s.execute("file.automate.runDroplet", json!({"droplet": dp, "input": [dir.clone()]})).unwrap(); + assert_eq!(r["errors"], json!([]), "{r}"); + assert_eq!(r["files"].as_array().unwrap().len(), inputs.len(), "{r}"); + let out = photocraft_io::import("img0.png", &std::fs::read(format!("{dir}/out/img0.png")).unwrap()).unwrap().document; + let px = photocraft_compose::render(&out, photocraft_geom::Rect::new(1, 1, 2, 2)).px[0]; + assert!((px[0] - 235.0 / 255.0).abs() < 1e-3, "img0's gray 20 inverted: {px:?}"); +} + #[cfg(unix)] #[test] fn droplet_shim_passes_shell_metacharacters_as_a_literal_path() { diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 58c7282b36..843efa2e06 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -1272,12 +1272,33 @@ fn build() -> Vec { label: "Activate Document", menu: &[], shortcut: None, - params: r##"{"document":index}"##, + params: r##"{"document":index} or {"offset":int} (the open document that many tabs after the active one, before it when negative: a Photoshop action's switch to another document) → {document}"##, enabled: has_doc, run: |s, p| { - let i = p.get("document").and_then(Value::as_u64).ok_or_else(|| bad("document.activate", "missing `document`"))?; - let idx = usize::try_from(i).map_err(|_| bad("document.activate", "`document` out of range"))?; - if s.set_active(idx) { Ok(Value::Null) } else { Err(EngineError::Other(format!("no document at index {idx}"))) } + // A null key counts as omitted. + let given = |k: &str| p.get(k).filter(|v| !v.is_null()); + let idx = match (given("document"), given("offset")) { + (Some(_), Some(_)) => return Err(bad("document.activate", "give `document` or `offset`, not both")), + (None, None) => return Err(bad("document.activate", "needs `document` (a tab index) or `offset`")), + // Past the first or last tab is refused rather than wrapped, so an action that + // copies from one document never silently pastes into the wrong one (#7103). + (None, Some(o)) => { + let o = o.as_i64().ok_or_else(|| bad("document.activate", "`offset` must be an integer"))?; + let (active, len) = (s.active_index().ok_or(EngineError::NoDocument)?, s.documents().len()); + let target = i64::try_from(active).ok().and_then(|a| a.checked_add(o)).and_then(|i| usize::try_from(i).ok()); + let Some(i) = target.filter(|&i| i < len) else { + let side = if o < 0 { "before" } else { "after" }; + let msg = format!("no document {} {side} the active one, which is {} of {len}", o.unsigned_abs(), active + 1); + return Err(EngineError::Other(msg)); + }; + i + } + (Some(d), None) => { + let i = d.as_u64().ok_or_else(|| bad("document.activate", "`document` must be a tab index"))?; + usize::try_from(i).map_err(|_| bad("document.activate", "`document` out of range"))? + } + }; + if s.set_active(idx) { Ok(json!({"document": idx})) } else { Err(EngineError::Other(format!("no document at index {idx}"))) } }, journal: false, }, diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index c5e9d464ce..b473210fc1 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -1281,12 +1281,21 @@ pub(crate) fn list_images_tree(dir: &str, _skip: &str, _cmd: &str) -> Result Result<()>) -> Value { let inputs: Vec = inputs.iter().map(|path| BatchInput { path: path.clone(), rel_dir: String::new() }).collect(); - process_tree(&inputs, output, format, save, suffix, f) + process_tree(&inputs, output, format, save, suffix, &|s, _| f(s).map(|()| false)) } /// [`process_files`] for inputs from subfolders: each result goes to its input's relative folder -/// under `output` (created as needed), so `a/x.png` and `b/x.png` don't collide. -pub(crate) fn process_tree(inputs: &[BatchInput], output: &str, format: &str, save: SaveOpts, suffix: &str, f: &dyn Fn(&mut Session) -> Result<()>) -> Value { +/// under `output` (created as needed), so `a/x.png` and `b/x.png` don't collide. `f` gets the +/// output path and returns `true` when it wrote the result there itself (Batch writes what a +/// recorded save marked); otherwise the active document is saved after it. +pub(crate) fn process_tree( + inputs: &[BatchInput], + output: &str, + format: &str, + save: SaveOpts, + suffix: &str, + f: &dyn Fn(&mut Session, &str) -> Result, +) -> Value { let outputs = inputs.iter().map(|BatchInput { path, rel_dir }| { let ext = if format == "same" { same_format(path) } else { format.trim_start_matches('.').to_ascii_lowercase() }; (path.as_str(), join(&join(output, rel_dir), &format!("{}{suffix}.{ext}", stem(path)))) @@ -1319,7 +1328,7 @@ fn same_format_takes_the_file_names_extension() { assert_eq!(same_format("/in/.hidden"), "png"); } -fn process_outputs<'a>(outputs: impl IntoIterator, save: SaveOpts, f: &dyn Fn(&mut Session) -> Result<()>) -> Value { +fn process_outputs<'a>(outputs: impl IntoIterator, save: SaveOpts, f: &dyn Fn(&mut Session, &str) -> Result) -> Value { let mut files = Vec::new(); let mut errors = Vec::new(); let mut written = OutputClaims::default(); @@ -1330,9 +1339,10 @@ fn process_outputs<'a>(outputs: impl IntoIterator, sav let mut scratch = Session::new(); let doc = import(&file_name(path), &bytes)?; scratch.add_document(doc, Some(path.to_string())); - f(&mut scratch)?; - let d = scratch.active().ok_or(EngineError::NoDocument)?; - save_doc(&d.doc, &out, save)?; + if !f(&mut scratch, &out)? { + let d = scratch.active().ok_or(EngineError::NoDocument)?; + save_doc(&d.doc, &out, save)?; + } Ok(()) })(); match r { @@ -1367,15 +1377,26 @@ impl OutputClaims { } } +/// A recorded Save, Save As or Save a Copy, which marks the result Batch writes to its destination +/// (Photoshop's "Override Action 'Save As' Commands"). +pub(crate) fn batch_save_point(id: &str) -> bool { + matches!(id, "file.save" | "file.saveAs" | "file.saveACopy") +} + +/// Whether Batch and droplets keep a recorded step: save points mark the result, while other +/// shell-only saves and view steps (zoom, fit) don't change the output (#2752). +pub(crate) fn batch_keeps_step(id: &str) -> bool { + (batch_save_point(id) || !crate::actions_cmds::shell_save_command(id)) && !crate::actions_cmds::shell_view_command(id) +} + fn batch(s: &mut Session, p: &Value) -> Result { let cmd = "file.automate.batch"; let mut steps = parse_steps(p.get("steps").or_else(|| p.get("action")).ok_or_else(|| EngineError::BadParams { cmd: cmd.into(), msg: "missing \"steps\"".into() })?)?; // Each result goes to the destination folder, which overrides the action's recorded Save / - // Save As steps (Photoshop's "Override Action 'Save As' Commands"). Recorded view steps - // (zoom, fit) don't change the output, so they are skipped (#2752). - steps.retain(|(id, _)| !crate::actions_cmds::shell_save_command(id) && !crate::actions_cmds::shell_view_command(id) && id != "file.export.quickExport"); - if let Some((id, _)) = steps.iter().find(|(id, _)| crate::commands::find(id).is_none()) { + // Save As steps: they only mark the result. + steps.retain(|(id, _)| batch_keeps_step(id) && id != "file.export.quickExport"); + if let Some((id, _)) = steps.iter().find(|(id, _)| !batch_save_point(id) && crate::commands::find(id).is_none()) { return Err(EngineError::BadParams { cmd: cmd.into(), msg: format!("unknown command `{id}` in the action") }); } let output = str_param(p, "output", cmd)?.to_string(); @@ -1398,16 +1419,32 @@ fn batch(s: &mut Session, p: &Value) -> Result { let actions = crate::actions_cmds::ActionState { list, playing: s.actions.playing, playback_stack: s.actions.playback_stack.clone(), ..Default::default() }; let names = batch_naming::plan(&inputs, &output, &format, p)?; let save = SaveOpts::from_params(s, p, cmd)?; - let run = |scratch: &mut Session| { + let run = |scratch: &mut Session, out: &str| { scratch.actions = actions.clone(); + // The document as the last recorded save found it, written once the action has run so a + // failing step still writes nothing. + let mut saved: Option> = None; for (id, params) in &steps { + if batch_save_point(id) { + saved = Some(scratch.active().ok_or(EngineError::NoDocument)?.doc.clone()); + continue; + } let r = scratch.execute(id, params.clone())?; // A failed step inside a called action makes this file an error, not a saved result. if let Some(error) = crate::actions_cmds::nested_failure(id, &r) { return Err(EngineError::Other(error)); } } - Ok(()) + // While the saved document is still open the steps after the save reach the result too + // (#3058), so it is saved after the action as usual. Closed after its save, or with another + // document left active, it is written as the save found it. + match saved { + Some(doc) if scratch.active().is_none_or(|d| d.doc.id != doc.id) => { + save_doc(&doc, out, save)?; + Ok(true) + } + _ => Ok(false), + } }; let r = match names { Some(names) => process_outputs(inputs.iter().map(|i| i.path.as_str()).zip(names), save, &run), @@ -1426,14 +1463,14 @@ fn image_processor(s: &mut Session, p: &Value) -> Result { (w, h) => Some(json!({"width": w.unwrap_or(1e9), "height": h.unwrap_or(1e9), "dontEnlarge": true})), }; let to_srgb = p.get("convertToSrgb").and_then(Value::as_bool).unwrap_or(false); - let r = process_tree(&inputs, &output, &format, SaveOpts::from_params(s, p, cmd)?.or_quality(8.0), "", &|scratch| { + let r = process_tree(&inputs, &output, &format, SaveOpts::from_params(s, p, cmd)?.or_quality(8.0), "", &|scratch, _| { if to_srgb && scratch.active().is_some_and(|d| d.doc.mode != ColorMode::Rgb) { scratch.execute("image.mode.rgb", json!({}))?; } if let Some(fp) = &fit { fit_image(scratch, fp)?; } - Ok(()) + Ok(false) }); Ok(r) } @@ -1925,7 +1962,7 @@ pub fn specs() -> Vec { "Batch…", &["File", "Automate"], None, - r##"{"steps":[[commandId,params]|{"command":id,"params":{}}…] (a recorded action),"input":folder|[paths],"includeSubfolders":bool=false (a folder input also takes the images in its subfolders, up to 32 levels and 100000 files; each result keeps its subfolder under output),"output":folder,"format":"same|png|jpg|psd|tiff|…"="same","quality":0..12?,"jpegFormat":"baseline|optimized|progressive"?,"naming":string? (blank keeps source names; {name}, {serial} padded to 3 digits, {date} as YYYY-MM-DD UTC; extension added automatically),"startingSerial":1..4294967295=1 (input position, including failed inputs),"tiffLayers":bool=false,"tiffCompression":"none|lzw|zip"?,"tgaBits":24|32?,"dpxBits":8|10|12|16? (DPX: bits per sample; unset, 8 for an 8-bit document, else 16),"ddsCompression":"none|bc1|bc2|bc3"="none" (DDS: uncompressed, or block compression; BC1 keeps alpha on or off),"ddsMipmaps":bool=false (DDS: also write the full mip chain),"calledActions":[{"name","steps"}…]? (the actions Play Action steps call; default: the session's)} → {files, errors} (an input whose output name was already written in the run goes to errors)"##, + r##"{"steps":[[commandId,params]|{"command":id,"params":{}}…] (a recorded action; a Save, Save As or Save a Copy step marks the document written to output, in this command's format and options: as the save found it if it was closed or another document is active at the end, else as the action leaves it; with no such step, the active document at the end),"input":folder|[paths],"includeSubfolders":bool=false (a folder input also takes the images in its subfolders, up to 32 levels and 100000 files; each result keeps its subfolder under output),"output":folder,"format":"same|png|jpg|psd|tiff|…"="same","quality":0..12?,"jpegFormat":"baseline|optimized|progressive"?,"naming":string? (blank keeps source names; {name}, {serial} padded to 3 digits, {date} as YYYY-MM-DD UTC; extension added automatically),"startingSerial":1..4294967295=1 (input position, including failed inputs),"tiffLayers":bool=false,"tiffCompression":"none|lzw|zip"?,"tgaBits":24|32?,"dpxBits":8|10|12|16? (DPX: bits per sample; unset, 8 for an 8-bit document, else 16),"ddsCompression":"none|bc1|bc2|bc3"="none" (DDS: uncompressed, or block compression; BC1 keeps alpha on or off),"ddsMipmaps":bool=false (DDS: also write the full mip chain),"calledActions":[{"name","steps"}…]? (the actions Play Action steps call; default: the session's)} → {files, errors} (an input whose output name was already written in the run goes to errors)"##, native, batch ), diff --git a/crates/engine/src/file_cmds/tests.rs b/crates/engine/src/file_cmds/tests.rs index 2a52155ca2..24bcd262dd 100644 --- a/crates/engine/src/file_cmds/tests.rs +++ b/crates/engine/src/file_cmds/tests.rs @@ -1587,6 +1587,83 @@ fn batch_naming_rejects_outputs_that_replace_inputs_before_processing() { assert_eq!(std::fs::read(&second).unwrap(), before[1]); } +/// Batch takes the result at the action's recorded Save, Save As or Save a Copy, as Photoshop's +/// "Override Action 'Save As' Commands" does: a document closed after its save is written as the +/// save found it. The usual "…, Save As, Close" ending failed on every file: the Save was dropped, +/// and nothing was left open to save after the action. +#[test] +fn batch_saves_at_the_recorded_save_so_a_close_after_it_keeps_the_result() { + let dir = tmp("batch-save-close"); + let red = png(&dir, "red.png", 8, 8, "#ff0000"); + let saved = |out: &str| photocraft_io::import("red.png", &std::fs::read(join(out, "red.png")).unwrap()).unwrap().document; + let mut s = Session::new(); + let out = join(&dir, "close"); + let steps = json!([["image.adjustments.invert", {}], ["file.saveAs", {}], ["file.close", {}]]); + let r = s.execute("file.automate.batch", json!({"input": [red], "output": out, "format": "png", "steps": steps})).unwrap(); + assert_eq!(r["errors"], json!([]), "{r}"); + assert_eq!(r["files"], json!([join(&out, "red.png")])); + assert_eq!(photocraft_compose::render(&saved(&out), Rect::new(1, 1, 2, 2)).px[0], [0.0, 1.0, 1.0, 1.0], "the inverted red"); + + // Save a Copy marks the result the same way, and its recorded path is never written. + let out = join(&dir, "copy"); + let elsewhere = join(&dir, "elsewhere.png"); + let steps = json!([["image.adjustments.invert", {}], ["file.saveACopy", {"path": elsewhere}], ["file.close", {}]]); + let r = s.execute("file.automate.batch", json!({"input": [red], "output": out, "format": "png", "steps": steps})).unwrap(); + assert_eq!(r["errors"], json!([]), "{r}"); + assert_eq!(photocraft_compose::render(&saved(&out), Rect::new(1, 1, 2, 2)).px[0], [0.0, 1.0, 1.0, 1.0]); + assert!(!std::path::Path::new(&elsewhere).exists()); + + // With no Save step the result is still saved after the action; with nothing left open then, + // the file is an error, not a silent skip. + let out = join(&dir, "closed"); + let r = s.execute("file.automate.batch", json!({"input": [red], "output": out, "format": "png", "steps": [["file.close", {}]]})).unwrap(); + assert_eq!(r["files"], json!([]), "{r}"); + assert!(r["errors"][0]["error"].as_str().unwrap().contains("no active document"), "{r}"); +} + +/// The document a recorded save marked is the result, not the one left active after it closed. +/// Here the action saves and closes a second document it made, then inverts the input; the output +/// used to be the inverted input, written in place of the result without an error. +#[test] +fn batch_saves_the_document_active_at_the_save_not_the_one_left_after_it() { + let dir = tmp("batch-save-second"); + let red = png(&dir, "red.png", 8, 8, "#ff0000"); + let out = join(&dir, "out"); + let steps = json!([ + ["file.new", {"width": 4, "height": 4, "background": "#00ff00"}], + ["file.save", {}], + ["file.close", {}], + ["image.adjustments.invert", {}] + ]); + let r = Session::new().execute("file.automate.batch", json!({"input": [red], "output": out, "format": "png", "steps": steps})).unwrap(); + assert_eq!(r["errors"], json!([]), "{r}"); + let saved = photocraft_io::import("red.png", &std::fs::read(join(&out, "red.png")).unwrap()).unwrap().document; + assert_eq!((saved.size.width, saved.size.height), (4, 4)); + assert_eq!(photocraft_compose::render(&saved, Rect::new(1, 1, 2, 2)).px[0], [0.0, 1.0, 0.0, 1.0]); +} + +/// While the saved document is open, the steps after its save still reach the result (#3058: a +/// recorded Save then Rotate writes the rotated image). A step failing after the save writes +/// nothing, as when Batch saved only after the action. +#[test] +fn batch_applies_the_steps_after_a_save_while_its_document_is_open() { + let dir = tmp("batch-save-then-edit"); + let red = png(&dir, "red.png", 8, 8, "#ff0000"); + let mut s = Session::new(); + let out = join(&dir, "edited"); + let steps = json!([["file.saveAs", {}], ["image.adjustments.invert", {}]]); + let r = s.execute("file.automate.batch", json!({"input": [red], "output": out, "format": "png", "steps": steps})).unwrap(); + assert_eq!(r["errors"], json!([]), "{r}"); + let saved = photocraft_io::import("red.png", &std::fs::read(join(&out, "red.png")).unwrap()).unwrap().document; + assert_eq!(photocraft_compose::render(&saved, Rect::new(1, 1, 2, 2)).px[0], [0.0, 1.0, 1.0, 1.0]); + + let out = join(&dir, "failed"); + let steps = json!([["image.adjustments.invert", {}], ["file.saveAs", {}], ["layer.select", {"layer": "missing"}]]); + let r = s.execute("file.automate.batch", json!({"input": [red], "output": out, "format": "png", "steps": steps})).unwrap(); + assert_eq!(r["errors"].as_array().unwrap().len(), 1, "{r}"); + assert!(!std::path::Path::new(&join(&out, "red.png")).exists(), "{r}"); +} + #[test] fn exposure_text_near_one_second_is_not_one_over_one() { assert_eq!(exposure_text(0.996).as_deref(), Some("0.996 s"), "not \"1/1 s\"; three significant digits"); diff --git a/crates/engine/src/jobs.rs b/crates/engine/src/jobs.rs index 6d98a6bc0f..57846617dd 100644 --- a/crates/engine/src/jobs.rs +++ b/crates/engine/src/jobs.rs @@ -906,6 +906,7 @@ impl Session { if !crate::brush_cmds::coalesce_journal(self, id, ¶ms) { let params = match id { _ if self.actions.recording.is_none() => params, + "layer.duplicate" => crate::artboard_cmds::recorded_duplicate(self, crate::layer_multi_cmds::recorded_step(self, id, params, pre_selected)), "layerComp.apply" => crate::comps_cmds::recorded_apply(self, params), _ => crate::layer_multi_cmds::recorded_step(self, id, params, pre_selected), }; diff --git a/crates/engine/src/presets/tests.rs b/crates/engine/src/presets/tests.rs index 3906db577c..89ba8a5855 100644 --- a/crates/engine/src/presets/tests.rs +++ b/crates/engine/src/presets/tests.rs @@ -823,7 +823,9 @@ fn a_damaged_preset_set_keeps_the_other_sets() { }); s.execute("file.export.presets.save", json!({"name": "Web JPEG", "settings": export})).unwrap(); let saved = s.presets.to_json(&s); - for key in ["gradients", "gradient", "styles", "shapes", "pattern_groups", "custom_shapes", "layer_defaults", "swatches", "export_presets"] { + for key in + ["gradients", "gradient", "styles", "shapes", "pattern_groups", "recent_documents", "custom_shapes", "layer_defaults", "swatches", "export_presets"] + { let mut blob = saved.clone(); blob[key] = json!("not a preset set"); let mut next = Session::new(); @@ -834,12 +836,27 @@ fn a_damaged_preset_set_keeps_the_other_sets() { assert_eq!(next.presets.export_presets, s.presets.export_presets, "damaged `{key}`"); } } + let mut blob = saved.clone(); + blob["export_presets"] = json!("not a preset set"); + let mut next = Session::new(); + next.load_presets_json(blob); + assert!(next.presets.export_presets.is_empty()); + assert_eq!(next.presets.documents, s.presets.documents); + let mut blob = saved.clone(); + blob["documents"] = json!("not a preset set"); + let mut next = Session::new(); + next.load_presets_json(blob); + assert!(next.presets.documents.is_empty()); + assert_eq!(next.presets.export_presets, s.presets.export_presets); // Null sets keep their defaults, as before. let mut next = Session::new(); - next.load_presets_json(json!({"tool_presets": null, "gradients": null, "documents": saved["documents"].clone()})); + next.load_presets_json( + json!({"tool_presets": null, "gradients": null, "documents": saved["documents"].clone(), "export_presets": saved["export_presets"].clone()}), + ); assert_eq!(next.presets.tool_presets.len(), Session::new().presets.tool_presets.len()); assert_eq!(next.presets.gradients.len(), Session::new().presets.gradients.len()); assert_eq!(next.presets.documents, s.presets.documents); + assert_eq!(next.presets.export_presets, s.presets.export_presets); } // ------------------------------------------------------------------ export presets diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index de40f8662c..0b1b48aa3b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1573,6 +1573,53 @@ fn document_inspect_and_activate_invalid_index() { assert_eq!(err_act.to_string(), "no document at index 9"); } +/// #7103: Photoshop's Select Document by offset (previous is -1, next +1) walks the open +/// documents in tab order from the active one. Past either end it is refused, not wrapped onto +/// another document, and bad params leave the active document as it was. +#[test] +fn document_activate_by_offset_walks_the_tabs() { + let mut s = Session::new(); + for name in ["a", "b", "c"] { + s.execute("file.new", json!({"width": 2, "height": 2, "name": name})).unwrap(); + } + let active = |s: &Session| s.active().unwrap().doc.name.clone(); + assert_eq!(active(&s), "c"); + assert_eq!(s.execute("document.activate", json!({"offset": -2})).unwrap(), json!({"document": 0})); + assert_eq!(active(&s), "a"); + assert_eq!(s.execute("document.activate", json!({"offset": 1})).unwrap(), json!({"document": 1})); + assert_eq!(active(&s), "b"); + s.execute("document.activate", json!({"offset": 0})).unwrap(); + assert_eq!(active(&s), "b"); + assert_eq!(s.execute("document.activate", json!({"document": 2})).unwrap(), json!({"document": 2})); + // A null key counts as omitted. + s.execute("document.activate", json!({"document": null, "offset": -1})).unwrap(); + assert_eq!(active(&s), "b"); + + let err = s.execute("document.activate", json!({"offset": 2})).unwrap_err().to_string(); + assert_eq!(err, "no document 2 after the active one, which is 2 of 3"); + let err = s.execute("document.activate", json!({"offset": -2})).unwrap_err().to_string(); + assert_eq!(err, "no document 2 before the active one, which is 2 of 3"); + for bad in [ + json!({}), + json!({"offset": null}), + json!({"offset": 1.5}), + json!({"offset": "1"}), + json!({"offset": i64::MAX}), + json!({"offset": i64::MIN}), + json!({"offset": u64::MAX}), + json!({"document": 0, "offset": 1}), + json!({"document": -1}), + ] { + assert!(s.execute("document.activate", bad.clone()).is_err(), "{bad}"); + assert_eq!(active(&s), "b", "{bad}"); + } + // Tabs, not the order the documents were opened in: with "c" dragged to the front, two tabs + // before "b" is "c". + s.execute("document.move", json!({"document": 2, "to": 0})).unwrap(); + s.execute("document.activate", json!({"offset": -2})).unwrap(); + assert_eq!(active(&s), "c"); +} + /// #2166, measured in Photoshop 25.1: a layer mask and the layer pixels keep separate /// foreground/background pairs, swapped when the edit target changes. #[test] diff --git a/crates/ui-egui/src/browse_field_tests.rs b/crates/ui-egui/src/browse_field_tests.rs new file mode 100644 index 0000000000..2aa3ef44f5 --- /dev/null +++ b/crates/ui-egui/src/browse_field_tests.rs @@ -0,0 +1,65 @@ +//! #7942 / #7531: path fields of command dialogs get a Browse… button (Artboards to Files' +//! folder, Install Plug-in…'s .wasm file) that fills the field from a file dialog. +use crate::PhotocraftApp; +use crate::file_dialog::{FileDialogAnswer, FileDialogRequest}; +use egui::vec2; +use egui_kittest::{Harness, kittest::Queryable}; +use serde_json::{Value, json}; + +fn harness(answers: Vec>) -> (Harness<'static, PhotocraftApp>, std::rc::Rc>>) { + let (show, asked) = crate::file_dialog::fake(answers); + let h = Harness::builder().with_size(vec2(1440.0, 900.0)).with_max_steps(64).build_eframe(move |cc| { + PhotocraftApp::setup_context(&cc.egui_ctx, Default::default()); + PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services { file_dialog: Some(show), ..Default::default() }) + }); + (h, asked) +} + +fn field(h: &Harness<'_, PhotocraftApp>, key: &str) -> Option { + h.state().ui.dialogs.last().and_then(|d| d.fields.get(key).cloned()) +} + +#[test] +fn artboards_to_files_folder_is_chosen_with_browse() { + let (mut h, asked) = harness(vec![Some(FileDialogAnswer::FolderTo("/tmp/exports".into()))]); + h.state_mut().run("file.new", json!({"width": 64, "height": 64})).unwrap(); + h.state_mut().sync_views(); + let ctx = h.ctx.clone(); + crate::menus::invoke(h.state_mut(), &ctx, "file.export.artboardsToFiles", json!({})).unwrap(); + h.run_steps(4); + assert!(field(&h, "dir").is_some(), "the dialog has a Dir field"); + h.get_by_label("Browse…").click(); + h.run_steps(4); + assert!(matches!(asked.borrow().as_slice(), [FileDialogRequest::Folder { .. }])); + assert_eq!(field(&h, "dir"), Some(json!("/tmp/exports"))); + assert!(field(&h, crate::dialogs::BROWSE_FIELD).is_none(), "the request is consumed"); +} + +#[test] +fn cancelling_browse_keeps_the_typed_folder() { + let (mut h, _) = harness(vec![None]); + h.state_mut().run("file.new", json!({"width": 64, "height": 64})).unwrap(); + h.state_mut().sync_views(); + let ctx = h.ctx.clone(); + crate::menus::invoke(h.state_mut(), &ctx, "file.export.layersToFiles", json!({})).unwrap(); + h.run_steps(4); + let before = field(&h, "dir"); + h.get_by_label("Browse…").click(); + h.run_steps(4); + assert_eq!(field(&h, "dir"), before); +} + +#[test] +fn install_plugin_path_is_chosen_with_browse() { + let (mut h, asked) = harness(vec![Some(FileDialogAnswer::Paths(vec!["/tmp/blur.wasm".into()]))]); + let ctx = h.ctx.clone(); + crate::menus::invoke(h.state_mut(), &ctx, "plugin.install", json!({})).unwrap(); + h.run_steps(4); + h.get_by_label("Browse…").click(); + h.run_steps(4); + match asked.borrow().as_slice() { + [FileDialogRequest::Open { multiple: false, extensions: Some(e), .. }] => assert_eq!(e, &["wasm".to_string()]), + other => panic!("unexpected requests {other:?}"), + } + assert_eq!(field(&h, "path"), Some(json!("/tmp/blur.wasm"))); +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index f9a87af2bc..7e051ed9ca 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -7,6 +7,37 @@ use crate::PhotocraftApp; use crate::state::{Dialog, DialogKind}; use crate::widgets::{ButtonRole, DialogButton, dialog_buttons}; +/// A dialog body sets this field to the key of the text field whose Browse… button was clicked; +/// [`show`] takes it out again and opens the matching file or folder dialog. +pub(crate) const BROWSE_FIELD: &str = "__browse"; + +/// Browse… beside a path field of dialog `id`: a folder picker for `dir` fields, a `.wasm` picker +/// for Install Plug-in…'s `path`. The answer fills the field. +fn browse_field(app: &mut PhotocraftApp, id: u64, fields: &serde_json::Map, key: &str) { + let key = key.to_string(); + let r = if key == "dir" { + app.ask_folder(move |app, path| fill_field(app, id, &key, path)) + } else if fields.get("__command").and_then(Value::as_str) == Some("plugin.install") { + let initial_dir = crate::file_dialog::last_used_dir(&app.ui.recent_files); + let request = crate::file_dialog::FileDialogRequest::Open { multiple: false, initial_dir, extensions: Some(vec!["wasm".into()]) }; + app.ask_file(request, move |app, answer| match answer { + crate::file_dialog::FileDialogAnswer::Paths(mut paths) if !paths.is_empty() => fill_field(app, id, &key, paths.remove(0)), + _ => Err("Install Plug-in reads a .wasm file from disk".into()), + }) + } else { + return; + }; + if let Err(e) = r.map(|_| ()).or_else(|e| if e == crate::file_dialog::CANCELLED { Ok(()) } else { Err(e) }) { + app.ui.status = e; + } +} + +fn fill_field(app: &mut PhotocraftApp, id: u64, key: &str, path: String) -> Result { + let d = app.ui.dialog_mut(id).ok_or("the dialog was closed")?; + d.fields.insert(key.into(), json!(path)); + Ok(Value::Null) +} + /// Where this frame's dialogs are on screen (the canvas reads last frame's: it draws first). const RECTS: &str = "pc-dialog-rects"; @@ -130,7 +161,15 @@ fn show_modal( } if let Some(size) = resizable { let max = (ctx.content_rect().size() - egui::vec2(40.0, 40.0)).max(egui::vec2(560.0, 400.0)); - egui::Resize::default().id(id.with("resize")).default_size(size.min(max)).min_size(egui::vec2(560.0, 400.0)).max_size(max).show(ui, contents) + // No stroke: Resize outlines its contents 2 points out, an inner box inside the + // dialog's frame that left the title and buttons flush against it (#8199). + egui::Resize::default() + .id(id.with("resize")) + .with_stroke(false) + .default_size(size.min(max)) + .min_size(egui::vec2(560.0, 400.0)) + .max_size(max) + .show(ui, contents) } else { contents(ui) } @@ -389,6 +428,9 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { } } crate::field_tab::end(ui.ctx(), tab); + if let Some(Value::String(key)) = fields.remove(BROWSE_FIELD) { + browse_field(app, d.id, &fields, &key); + } if crate::guide_layout_ui::owns(&fields) || crate::brush_preset_dialog::owns(&fields) { // These dialogs own their right-side actions and validate Enter themselves. } else if crate::color_picker_ui::owns(&fields) { @@ -1224,4 +1266,45 @@ mod preference_resize_tests { h.run_steps(10); assert_eq!(frame(&h), smaller); } + + /// #8199: no outline is drawn just around the contents, which left the title flush against a + /// second, inner box instead of the frame's margin like the other dialogs. + #[test] + fn preferences_has_no_inner_outline_hugging_its_title() { + fn find<'a>(shape: &'a egui::Shape, out: &mut Vec<&'a egui::Shape>) { + match shape { + egui::Shape::Vec(shapes) => shapes.iter().for_each(|s| find(s, out)), + s => out.push(s), + } + } + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + crate::prefs_ui::open_preferences(&mut app, "general"); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_ui_state( + |ui, app| { + if ui.ctx().fonts(|f| f.families().contains(&egui::FontFamily::Name("semibold".into()))) { + show(app, ui.ctx()); + } + }, + app, + ); + PhotocraftApp::setup_context(&h.ctx, Default::default()); + h.run_steps(5); + let mut shapes = Vec::new(); + h.output().shapes.iter().for_each(|s| find(&s.shape, &mut shapes)); + let title = shapes + .iter() + .find_map(|s| match s { + egui::Shape::Text(t) if t.galley.job.text == "Preferences" => Some(t.pos), + _ => None, + }) + .expect("the title is painted"); + let hugging: Vec = shapes + .iter() + .filter_map(|s| match s { + egui::Shape::Rect(r) if r.stroke.width > 0.0 && r.rect.contains(title) && (0.0..4.0).contains(&(title.x - r.rect.left())) => Some(r.rect), + _ => None, + }) + .collect(); + assert!(hugging.is_empty(), "an outline within 4 points of the title at {title:?}: {hugging:?}"); + } } diff --git a/crates/ui-egui/src/filter_dialog.rs b/crates/ui-egui/src/filter_dialog.rs index 95ce581083..59ec8a20f0 100644 --- a/crates/ui-egui/src/filter_dialog.rs +++ b/crates/ui-egui/src/filter_dialog.rs @@ -907,6 +907,10 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map) { ui.horizontal(|ui| { ui.label(egui::RichText::new(label(&p.key)).color(t.text_dim)); ui.add(egui::TextEdit::singleline(&mut v).desired_width(200.0)); + // Install Plug-in…: pick the .wasm file instead of typing its path (#7531). + if cmd == "plugin.install" && p.key == "path" && crate::widgets::secondary_button(ui, tl!("Browse…"), 0.0).clicked() { + f.insert(crate::dialogs::BROWSE_FIELD.into(), json!(p.key)); + } }); if v.is_empty() { f.remove(&p.key); diff --git a/crates/ui-egui/src/i18n/cs.tsv b/crates/ui-egui/src/i18n/cs.tsv index e4449aee33..73edddc364 100644 --- a/crates/ui-egui/src/i18n/cs.tsv +++ b/crates/ui-egui/src/i18n/cs.tsv @@ -2850,6 +2850,9 @@ cameraData Lens Objektiv Cooling80 Ochlazovací 80 Cooling82 Ochlazovací 82 Copyright Status Stav autorských práv + Copyright Info URL URL informace o autorských právech + Copyright Notice Oznámení autorských práv + Document Title Název dokumentu Copyrighted Chráněno autorským právem Correct C A Korigovat chromatickou aberaci Correct Distortion Korigovat zkreslení diff --git a/crates/ui-egui/src/i18n/de.tsv b/crates/ui-egui/src/i18n/de.tsv index 85f9a1defb..87a78b03d5 100644 --- a/crates/ui-egui/src/i18n/de.tsv +++ b/crates/ui-egui/src/i18n/de.tsv @@ -2877,6 +2877,9 @@ curves Smooth Glätten # Document Info and color profiles Author Title Position des Urhebers Copyright Status Urheberrechtsstatus + Copyright Info URL URL für Urheberrechtsinfo + Copyright Notice Urheberrechtsvermerk + Document Title Dokumenttitel Unknown Unbekannt Copyrighted Urheberrechtlich geschützt Public Domain Gemeinfrei diff --git a/crates/ui-egui/src/i18n/el.tsv b/crates/ui-egui/src/i18n/el.tsv index 36f75f107c..c3426ee1e9 100644 --- a/crates/ui-egui/src/i18n/el.tsv +++ b/crates/ui-egui/src/i18n/el.tsv @@ -999,6 +999,8 @@ Keywords Λέξεις-κλειδιά Copyright Info URL Σύνδεσμος πληροφοριών πνευματικών δικαιωμάτων Copyright Status Κατάσταση πνευματικών δικαιωμάτων + Copyright Notice Σημείωση πνευματικών δικαιωμάτων + Document Title Τίτλος εγγράφου Copyrighted Προστατεύεται από πνευματικά δικαιώματα Public Domain Κοινό κτήμα Unknown Άγνωστο diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index d2ca52f285..9eb97b30f0 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -3054,6 +3054,8 @@ curves Smooth Suavizar Keywords Palabras clave Copyright Info URL URL de información de copyright Copyright Status Estado del copyright + Copyright Notice Aviso de copyright + Document Title Título del documento Copyrighted Con copyright Public Domain Dominio público Unknown Desconocido diff --git a/crates/ui-egui/src/i18n/fr.tsv b/crates/ui-egui/src/i18n/fr.tsv index 9a622b88be..dea14f6389 100644 --- a/crates/ui-egui/src/i18n/fr.tsv +++ b/crates/ui-egui/src/i18n/fr.tsv @@ -2443,6 +2443,8 @@ Keywords Mots-clés Copyright Info URL URL des informations de copyright Copyright Status Statut du copyright + Copyright Notice Mention de copyright + Document Title Titre du document Copyrighted Protégé par copyright Public Domain Domaine public Unknown Inconnu diff --git a/crates/ui-egui/src/i18n/hu.tsv b/crates/ui-egui/src/i18n/hu.tsv index 078d2e8c6c..c45763955c 100644 --- a/crates/ui-egui/src/i18n/hu.tsv +++ b/crates/ui-egui/src/i18n/hu.tsv @@ -3154,6 +3154,8 @@ cameraRaw Yellow Sárga Convert to Profile Átalakítás profilra Convert to srgb Átalakítás sRGB-re Copyright Status Szerzői jogi állapot + Copyright Notice Szerzői jogi értesítés + Document Title Dokumentum címe Corner radius Sarokkerekítés sugara Correct C A Kromatikus aberráció javítása Correct Distortion Torzítás javítása diff --git a/crates/ui-egui/src/i18n/id.tsv b/crates/ui-egui/src/i18n/id.tsv index 04610863d7..0c46c3d09f 100644 --- a/crates/ui-egui/src/i18n/id.tsv +++ b/crates/ui-egui/src/i18n/id.tsv @@ -2450,6 +2450,8 @@ Keywords Kata Kunci Copyright Info URL URL Info Hak Cipta Copyright Status Status Hak Cipta + Copyright Notice Pemberitahuan Hak Cipta + Document Title Judul Dokumen Copyrighted Berhak Cipta Public Domain Domain Publik Unknown Tidak Diketahui diff --git a/crates/ui-egui/src/i18n/it.tsv b/crates/ui-egui/src/i18n/it.tsv index 412d628568..3fe2306732 100644 --- a/crates/ui-egui/src/i18n/it.tsv +++ b/crates/ui-egui/src/i18n/it.tsv @@ -3038,6 +3038,8 @@ curves Smooth Leviga Keywords Parole chiave Copyright Info URL URL info copyright Copyright Status Stato copyright + Copyright Notice Avviso sul copyright + Document Title Titolo documento Copyrighted Protetto da copyright Public Domain Pubblico dominio Unknown Sconosciuto diff --git a/crates/ui-egui/src/i18n/ja.tsv b/crates/ui-egui/src/i18n/ja.tsv index fa773fcfe8..e91e385ae2 100644 --- a/crates/ui-egui/src/i18n/ja.tsv +++ b/crates/ui-egui/src/i18n/ja.tsv @@ -3062,6 +3062,8 @@ fontWeight Italic 斜体 Cooling80 寒色フィルター (80) Cooling82 寒色フィルター (82) Copyright Status 著作権の状態 + Copyright Notice 著作権表示 + Document Title ドキュメントタイトル Copyrighted 著作権あり Corners 角 Correct C A 色収差を補正 diff --git a/crates/ui-egui/src/i18n/ko.tsv b/crates/ui-egui/src/i18n/ko.tsv index 75badef1b8..4dc994bbd8 100644 --- a/crates/ui-egui/src/i18n/ko.tsv +++ b/crates/ui-egui/src/i18n/ko.tsv @@ -990,6 +990,9 @@ Copper 구리 Copyright 저작권 Copyright and Contact Info 저작권 및 연락처 정보 + Copyright Info URL 저작권 정보 URL + Copyright Notice 저작권 고지 + Document Title 문서 제목 Corner Crop Marks 모서리 자르기 표시 Corner radius 모서리 반경 Correction 보정 diff --git a/crates/ui-egui/src/i18n/nl.tsv b/crates/ui-egui/src/i18n/nl.tsv index 2e9bc6297e..82beb1c87c 100644 --- a/crates/ui-egui/src/i18n/nl.tsv +++ b/crates/ui-egui/src/i18n/nl.tsv @@ -2849,6 +2849,8 @@ cameraRaw Color Kleur Keywords Trefwoorden Copyright Info URL URL copyrightinformatie Copyright Status Copyrightstatus + Copyright Notice Copyrightvermelding + Document Title Documenttitel Copyrighted Auteursrechtelijk beschermd Public Domain Publiek domein Unknown Onbekend diff --git a/crates/ui-egui/src/i18n/pl.tsv b/crates/ui-egui/src/i18n/pl.tsv index d53b306f25..4d5cf5cc5f 100644 --- a/crates/ui-egui/src/i18n/pl.tsv +++ b/crates/ui-egui/src/i18n/pl.tsv @@ -2483,6 +2483,8 @@ Keywords Słowa kluczowe Copyright Info URL Adres URL informacji o prawach autorskich Copyright Status Status praw autorskich + Copyright Notice Informacja o prawach autorskich + Document Title Tytuł dokumentu Copyrighted Chronione prawem autorskim Public Domain Domena publiczna Unknown Nieznany diff --git a/crates/ui-egui/src/i18n/pt-br.tsv b/crates/ui-egui/src/i18n/pt-br.tsv index f852a5b466..012c2d3e71 100644 --- a/crates/ui-egui/src/i18n/pt-br.tsv +++ b/crates/ui-egui/src/i18n/pt-br.tsv @@ -2438,6 +2438,8 @@ Keywords Palavras-chave Copyright Info URL URL das informações de direitos autorais Copyright Status Status dos direitos autorais + Copyright Notice Aviso de copyright + Document Title Título do documento Copyrighted Protegido por direitos autorais Public Domain Domínio público Unknown Desconhecido diff --git a/crates/ui-egui/src/i18n/ru.tsv b/crates/ui-egui/src/i18n/ru.tsv index 147a9ffab8..409be356fc 100644 --- a/crates/ui-egui/src/i18n/ru.tsv +++ b/crates/ui-egui/src/i18n/ru.tsv @@ -1352,6 +1352,8 @@ Keywords Ключевые слова Copyright Info URL URL сведений об авторских правах Copyright Status Статус авторских прав + Copyright Notice Уведомление об авторских правах + Document Title Название документа Copyrighted Охраняется авторским правом Public Domain Общественное достояние Unknown Неизвестно diff --git a/crates/ui-egui/src/i18n/sr.tsv b/crates/ui-egui/src/i18n/sr.tsv index 5df879a0ed..c7c02dacb9 100644 --- a/crates/ui-egui/src/i18n/sr.tsv +++ b/crates/ui-egui/src/i18n/sr.tsv @@ -2420,6 +2420,8 @@ Keywords Ključne reči Copyright Info URL URL informacija o autorskim pravima Copyright Status Status autorskih prava + Copyright Notice Obaveštenje o autorskim pravima + Document Title Naslov dokumenta Copyrighted Zaštićeno autorskim pravima Public Domain Javno dobro Unknown Nepoznato diff --git a/crates/ui-egui/src/i18n/tr.tsv b/crates/ui-egui/src/i18n/tr.tsv index 09fb45ed70..9d7c338f32 100644 --- a/crates/ui-egui/src/i18n/tr.tsv +++ b/crates/ui-egui/src/i18n/tr.tsv @@ -2822,6 +2822,8 @@ cameraRaw Color Renk Keywords Anahtar Kelimeler Copyright Info URL Telif Hakkı Bilgisi URL'si Copyright Status Telif Hakkı Durumu + Copyright Notice Telif hakkı bildirimi + Document Title Belge başlığı Copyrighted Telif hakkıyla korunuyor Public Domain Kamu Alanı Unknown Bilinmiyor diff --git a/crates/ui-egui/src/i18n/uk.tsv b/crates/ui-egui/src/i18n/uk.tsv index 914cd20bb4..63f86dcd21 100644 --- a/crates/ui-egui/src/i18n/uk.tsv +++ b/crates/ui-egui/src/i18n/uk.tsv @@ -1326,6 +1326,8 @@ Keywords Ключові слова Copyright Info URL URL інформації про авторське право Copyright Status Статус авторського права + Copyright Notice Застереження про авторські права + Document Title Назва документа Copyrighted Захищено авторським правом Public Domain Суспільне надбання Unknown Невідомо diff --git a/crates/ui-egui/src/i18n/vi.tsv b/crates/ui-egui/src/i18n/vi.tsv index badd43b975..178199788c 100644 --- a/crates/ui-egui/src/i18n/vi.tsv +++ b/crates/ui-egui/src/i18n/vi.tsv @@ -3000,6 +3000,9 @@ cameraData Lens Ống kính Cooling80 Lạnh80 Cooling82 Lạnh82 Copyright Status Trạng thái bản quyền + Copyright Info URL URL thông tin bản quyền + Copyright Notice Thông báo bản quyền + Document Title Tiêu đề tài liệu Copyrighted Được bảo vệ bản quyền Correct C A Hiệu chỉnh C A Correct Distortion Hiệu chỉnh độ méo diff --git a/crates/ui-egui/src/i18n/zh-hans.tsv b/crates/ui-egui/src/i18n/zh-hans.tsv index 7f559e8598..951d343aa0 100644 --- a/crates/ui-egui/src/i18n/zh-hans.tsv +++ b/crates/ui-egui/src/i18n/zh-hans.tsv @@ -1019,6 +1019,9 @@ Copper 铜色 Copyright 版权 Copyright and Contact Info 版权与联系信息 + Copyright Info URL 版权信息 URL + Copyright Notice 版权声明 + Document Title 文档标题 Corner Crop Marks 角落裁剪标记 Corner radius 圆角半径 Correction 校正 diff --git a/crates/ui-egui/src/i18n/zh-hant.tsv b/crates/ui-egui/src/i18n/zh-hant.tsv index d40fe28512..794a7ee777 100644 --- a/crates/ui-egui/src/i18n/zh-hant.tsv +++ b/crates/ui-egui/src/i18n/zh-hant.tsv @@ -1007,6 +1007,9 @@ Copper 銅色 Copyright 版權 Copyright and Contact Info 版權與聯絡資訊 + Copyright Info URL 版權資訊 URL + Copyright Notice 版權聲明 + Document Title 文件標題 Corner Crop Marks 角落裁切標記 Corner radius 圓角半徑 Correction 校正 diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c7123df81d..8674393117 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3767,6 +3767,8 @@ mod channel_tint_tests { } } +#[cfg(test)] +mod browse_field_tests; #[cfg(test)] mod options_hit_area_tests; #[cfg(test)] diff --git a/crates/ui-egui/src/new_doc_ui.rs b/crates/ui-egui/src/new_doc_ui.rs index ca836b61f5..4f09d6a028 100644 --- a/crates/ui-egui/src/new_doc_ui.rs +++ b/crates/ui-egui/src/new_doc_ui.rs @@ -453,7 +453,8 @@ fn saved_presets(app: &mut crate::PhotocraftApp, ui: &mut egui::Ui, f: &mut Map< let response = ui.add_sized(vec2(422.0, 32.0), label).on_hover_text(&preset.name); if response.clicked() { apply_saved_preset(f, preset); - create |= response.double_clicked() || response.triple_clicked(); + // Touch-tolerant double-tap (finger jitter > egui's 6 px); see `widgets::second_click`. + create |= widgets::second_click(ui, &response); } if widgets::secondary_button(ui, tl!("Delete"), 76.0).clicked() { match app.run("document.presets.delete", json!({"name":preset.name})) { @@ -517,7 +518,8 @@ fn save_preset(app: &mut crate::PhotocraftApp, ui: &mut egui::Ui, f: &mut Map) -> bool { let t = Tokens::get(ui.ctx()); + // Headings and field labels are static, as in Photoshop: no text cursor over them, no + // drag-selecting them (#8331). Text fields keep theirs. + ui.style_mut().interaction.selectable_labels = false; let cat = get_s(f, "__category", "Recent"); let mut create = false; // Category tabs. @@ -623,7 +628,8 @@ pub fn body(app: &mut crate::PhotocraftApp, ui: &mut egui::Ui, f: &mut Map apply_preset(f, p, &units), Card::Recent(settings) => apply_settings(f, settings), } - create |= resp.double_clicked() || resp.triple_clicked(); + // Touch-tolerant double-tap (finger jitter > egui's 6 px); see `widgets::second_click`. + create |= widgets::second_click(ui, &resp); } } }); @@ -1315,6 +1321,17 @@ mod tests { h.run_steps(2); } + /// A finger tap at `at`. Events for one phase share a frame (`input_mut`), because `Harness::step` would otherwise run one frame per queued event and drop Touch before the click release (so `second_click` would not see touch). + fn touch_tap_at(h: &mut Harness<'static, PhotocraftApp>, at: egui::Pos2) { + let touch = |phase| egui::Event::Touch { device_id: egui::TouchDeviceId(1), id: egui::TouchId(1), phase, pos: at, force: None }; + let button = |pressed| egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }; + h.input_mut().events.extend([egui::Event::PointerMoved(at), touch(egui::TouchPhase::Start), button(true)]); + h.step(); + h.input_mut().events.extend([touch(egui::TouchPhase::End), button(false)]); + h.step(); + h.run_steps(1); + } + /// The Width (0), Height (1) and Resolution (2) fields. fn field(h: &Harness<'static, PhotocraftApp>, i: usize) -> egui::Rect { h.query_all_by_role(Role::SpinButton).nth(i).map(|n| n.rect()).expect("a size field") @@ -1380,6 +1397,37 @@ mod tests { } } + fn cursor_over(h: &mut Harness<'static, PhotocraftApp>, at: egui::Pos2) -> egui::CursorIcon { + h.hover_at(at); + h.run_steps(2); + h.output().platform_output.cursor_icon + } + + /// #8331: headings and field labels keep the arrow; only the text fields show the I-beam. + #[test] + fn labels_show_the_arrow_and_only_text_fields_the_text_cursor() { + use egui_kittest::kittest::NodeT; + let mut h = harness(); + let label_at = |h: &Harness<'static, PhotocraftApp>, text: &str| { + h.query_all_by_label(text) + .find(|n| n.accesskit_node().role() == Role::Label) + .map(|n| n.rect().center()) + .unwrap_or_else(|| panic!("a {text:?} label")) + }; + for text in ["PRESET DETAILS", "Width", "Height", "Resolution", "Color Mode", "Background Contents"] { + let at = label_at(&h, text); + assert_eq!(cursor_over(&mut h, at), egui::CursorIcon::Default, "over {text:?}"); + } + // The name field still shows the text cursor, so the check above can tell them apart. + let name = h.query_all_by_role(Role::TextInput).next().map(|n| n.rect().center()).expect("the name field"); + assert_eq!(cursor_over(&mut h, name), egui::CursorIcon::Text, "over the name field"); + // The Saved tab's labels too. + h.get_by_label("Saved").click(); + h.run_steps(2); + let at = label_at(&h, "No saved presets yet."); + assert_eq!(cursor_over(&mut h, at), egui::CursorIcon::Default, "over the empty Saved list"); + } + #[test] fn double_click_builtin_preset_creates_its_size_and_current_details() { let mut h = harness(); @@ -1404,6 +1452,33 @@ mod tests { assert_pixel(&mut h, [0.0; 4]); } + /// Framework-style touch: the second tap lands ~48 px off (past egui's 6 px and the old 32 px touch slop) and still creates. + #[test] + fn touch_double_tap_with_jitter_creates_from_builtin_preset() { + let mut h = harness(); + h.get_by_label("Photo").click(); + h.run_steps(2); + let at = preset_title_at(&h, "Portrait, 4 x 6"); + touch_tap_at(&mut h, at); + assert_eq!(fields(&h)["__preset"], "Portrait, 4 x 6"); + assert!(h.state().session.documents().is_empty(), "first tap only selects"); + touch_tap_at(&mut h, egui::pos2(at.x + 48.0, at.y)); + assert_eq!(created(&h), (1200, 1800, 300.0)); + } + + /// Mouse keeps egui's tight distance: 20 px between clicks is two singles, not a create. + #[test] + fn mouse_double_click_with_large_jitter_does_not_create() { + let mut h = harness(); + h.get_by_label("Photo").click(); + h.run_steps(2); + let at = preset_title_at(&h, "Portrait, 4 x 6"); + click_at(&mut h, at); + click_at(&mut h, at + egui::vec2(20.0, 0.0)); + assert!(h.state().session.documents().is_empty()); + assert_eq!(h.state().ui.dialogs.len(), 1, "dialog stays open"); + } + #[test] fn double_click_clipboard_preset_creates_its_size_and_current_details() { let mut h = harness(); diff --git a/crates/ui-egui/src/prefs_ui.rs b/crates/ui-egui/src/prefs_ui.rs index ed1bf82021..aa96553c75 100644 --- a/crates/ui-egui/src/prefs_ui.rs +++ b/crates/ui-egui/src/prefs_ui.rs @@ -1497,6 +1497,9 @@ fn rendering_mode_row(ui: &mut egui::Ui, obj: &mut Map) { } }); ui.label(tl!("Automatic uses GPU acceleration when available and falls back to CPU rendering on errors.")); + // Next to the setting, as for System Title Bar (#1677): the renderer is picked when the + // window opens, so a change otherwise looks like it did nothing (#7939). + ui.label(RichText::new(tl!("Applies at next launch.")).color(Tokens::get(ui.ctx()).text_dim)); ui.add_space(8.0); } @@ -1570,8 +1573,11 @@ fn theme_swatch(ui: &mut egui::Ui, kind: ThemeKind, id: &str, selected: &mut Str ui.painter().circle(center, BADGE, t.card, egui::Stroke::new(1.0, t.card_border)); let (icon, tint) = if favorite { ("star-filled", t.accent) } else { ("star", t.text_dim) }; let action = if favorite { tl!("Remove from favorites") } else { tl!("Add to favorites") }; - let star = egui::Button::image(crate::icons::image(icon, 10.0, tint)).frame(false); - let response = ui.put(badge, star).on_hover_text(action); + // Interact and paint without allocating: a widget placed at `badge` would grow the row by the + // 4 px the badge overhangs the swatch, re-centring the swatches after this one whenever the + // pointer arrives, and oscillating when that moves the swatch out from under it. + let response = ui.interact(badge, ui.id().with((id, "favorite")), Sense::click()).on_hover_text(action); + crate::icons::paint(ui, badge, icon, 10.0, tint); response.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, true, format!("{action}: {label}"))); if response.clicked() { if favorite { @@ -3132,6 +3138,27 @@ mod tests { assert_eq!(dark, vec![ThemeKind::Studio]); } + /// The favourite star badge overhangs its swatch; showing it on hover must not move the swatches + /// after it, or the row would shift under the pointer. + #[test] + fn theme_star_badge_on_hover_does_not_move_the_next_swatch() { + use egui_kittest::{Harness, kittest::Queryable}; + let (mut app, _) = app_with_store(); + open_preferences(&mut app, "interface"); + let mut h = Harness::builder().with_size(vec2(1280.0, 800.0)).build_eframe(move |cc| { + PhotocraftApp::setup_context(&cc.egui_ctx, Default::default()); + app + }); + h.run_steps(4); + let first = h.get_by_label("Classic").rect(); + let next = h.get_by_label("Adwaita").rect(); + h.hover_at(first.right_top() + vec2(-3.0, 3.0)); + h.run_steps(3); + assert!(h.query_by_label("Add to favorites: Classic").is_some(), "the badge shows while the swatch is hovered"); + assert_eq!(h.get_by_label("Classic").rect(), first); + assert_eq!(h.get_by_label("Adwaita").rect(), next); + } + #[test] fn preferences_interface_shows_language_without_scrolling() { // #2532: in a small window the theme cards filled the page and Language sat out of view @@ -4143,6 +4170,28 @@ mod tests { assert!(h.state().custom_titlebar); } + /// #7939: switching Rendering Mode waits for the next launch, and says so next to the setting. + #[test] + fn rendering_mode_explains_next_launch() { + use egui_kittest::{Harness, kittest::Queryable}; + let obj = json!({"renderingMode": "auto", "historyStates": 50}).as_object().unwrap().clone(); + let mut h = Harness::new_ui_state( + |ui, obj: &mut Map| section_fields(ui, "performance", obj, &[], crate::i18n::Lang::from_pref("en"), None, ""), + obj, + ); + h.run_steps(4); + h.get_by_label("Rendering Mode"); + h.get_by_label("Applies at next launch."); + // A search that leaves Rendering Mode out leaves its notice out too. + let obj = json!({"renderingMode": "auto", "historyStates": 50}).as_object().unwrap().clone(); + let mut h = Harness::new_ui_state( + |ui, obj: &mut Map| section_fields(ui, "performance", obj, &[], crate::i18n::Lang::from_pref("en"), None, "history"), + obj, + ); + h.run_steps(4); + assert!(h.query_by_label("Applies at next launch.").is_none()); + } + #[test] fn next_launch_notice_does_not_mark_unrelated_interface_preferences() { use egui_kittest::{Harness, kittest::Queryable}; diff --git a/crates/ui-egui/src/view_cmds.rs b/crates/ui-egui/src/view_cmds.rs index 0938a9c715..6b37768740 100644 --- a/crates/ui-egui/src/view_cmds.rs +++ b/crates/ui-egui/src/view_cmds.rs @@ -1090,6 +1090,23 @@ mod form_reset_tests { } } +/// Display order of a `__form` dialog's field keys. `__order` lists them the way Photoshop +/// shows them (e.g. File Info); keys it omits keep their catalogue order after it. +pub fn form_keys(f: &Map) -> Vec { + let mut keys: Vec = f + .get("__order") + .and_then(Value::as_array) + .into_iter() + .flatten() + .filter_map(Value::as_str) + .filter(|k| !k.starts_with("__") && f.contains_key(*k)) + .map(str::to_string) + .collect(); + let rest: Vec = f.keys().filter(|k| !k.starts_with("__") && !keys.contains(k)).cloned().collect(); + keys.extend(rest); + keys +} + /// Body of a `__form` dialog: text fields, number fields, checkboxes and `__choices` dropdowns. pub fn form_body(ui: &mut egui::Ui, f: &mut Map) { let choices = f.get("__choices").cloned().unwrap_or(Value::Null); @@ -1097,7 +1114,8 @@ pub fn form_body(ui: &mut egui::Ui, f: &mut Map) { // come from `__labels` verbatim; everything else goes through the catalogue. let labels = f.get("__labels").cloned().unwrap_or(Value::Null); let label = |k: &str| labels.get(k).and_then(Value::as_str).map_or_else(|| label_of(k), str::to_string); - let keys: Vec = f.keys().filter(|k| !k.starts_with("__")).cloned().collect(); + let keys = form_keys(f); + let mut browse: Option = None; egui::Grid::new("form-dialog").num_columns(2).spacing([12.0, 6.0]).show(ui, |ui| { for k in keys { let v = f.get(&k).cloned().unwrap_or(Value::Null); @@ -1144,7 +1162,13 @@ pub fn form_body(ui: &mut egui::Ui, f: &mut Map) { crate::widgets::dropdown(ui, &format!("form-{k}"), &mut s, &refs, 200.0); } None => { - ui.add(egui::TextEdit::singleline(&mut s).desired_width(260.0)); + ui.horizontal(|ui| { + ui.add(egui::TextEdit::singleline(&mut s).desired_width(260.0)); + // A folder field gets the system folder picker (#7942). + if k == "dir" && crate::widgets::secondary_button(ui, tl!("Browse…"), 0.0).clicked() { + browse = Some(k.clone()); + } + }); } } json!(s) @@ -1165,6 +1189,9 @@ pub fn form_body(ui: &mut egui::Ui, f: &mut Map) { ui.end_row(); } }); + if let Some(key) = browse { + f.insert(crate::dialogs::BROWSE_FIELD.into(), json!(key)); + } camera_data(ui, f); } @@ -1311,7 +1338,23 @@ fn front(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, params: &Value) let kw = info["keywords"].as_array().map(|a| a.iter().filter_map(Value::as_str).collect::>().join("; ")).unwrap_or_default(); dialog( app, - json!({"title": info["title"], "author": info["author"], "authorTitle": info["authorTitle"], "description": info["description"], "keywords": kw, "copyright": info["copyright"], "copyrightStatus": info["copyrightStatus"], "copyrightUrl": info["copyrightUrl"], "__camera": info["camera"]}), + json!({ + "title": info["title"], + "author": info["author"], + "authorTitle": info["authorTitle"], + "description": info["description"], + "keywords": kw, + "copyrightStatus": info["copyrightStatus"], + "copyright": info["copyright"], + "copyrightUrl": info["copyrightUrl"], + "__order": ["title", "author", "authorTitle", "description", "keywords", "copyrightStatus", "copyright", "copyrightUrl"], + "__labels": { + "title": tl!("Document Title"), + "copyright": tl!("Copyright Notice"), + "copyrightUrl": tl!("Copyright Info URL"), + }, + "__camera": info["camera"] + }), json!({"copyrightStatus": ["unknown", "copyrighted", "publicDomain"]}), ) } diff --git a/crates/ui-egui/src/view_cmds/tests.rs b/crates/ui-egui/src/view_cmds/tests.rs index 3e579e0c65..05c1f1bb60 100644 --- a/crates/ui-egui/src/view_cmds/tests.rs +++ b/crates/ui-egui/src/view_cmds/tests.rs @@ -753,3 +753,28 @@ fn every_arranged_tile_has_rulers() { } assert!(labels[0] > 0 && labels[1] > 0, "ruler labels per tile: {labels:?}"); } + +#[test] +fn file_info_dialog_matches_photoshop_field_order_and_labels() { + crate::i18n::with_language(crate::i18n::Lang::EN, || { + let (mut app, ctx) = app_with(1); + let d = menu(&mut app, &ctx, "file.fileInfo", json!({})).unwrap()["dialog"].as_u64().unwrap(); + let fields = app.ui.dialog_mut(d).unwrap().fields.clone(); + assert_eq!( + form_keys(&fields).iter().map(String::as_str).collect::>(), + ["title", "author", "authorTitle", "description", "keywords", "copyrightStatus", "copyright", "copyrightUrl"] + ); + let labels = fields["__labels"].as_object().unwrap(); + assert_eq!(labels["title"], "Document Title"); + assert_eq!(labels["copyright"], "Copyright Notice"); + assert_eq!(labels["copyrightUrl"], "Copyright Info URL"); + }); +} + +#[test] +fn form_keys_orders_by_order_then_catalogue() { + let f = json!({"zeta": "", "alpha": "", "mid": "", "__order": ["mid", "zeta", "ghost"]}).as_object().unwrap().clone(); + assert_eq!(form_keys(&f).iter().map(String::as_str).collect::>(), ["mid", "zeta", "alpha"]); + let f = json!({"zeta": "", "alpha": ""}).as_object().unwrap().clone(); + assert_eq!(form_keys(&f).iter().map(String::as_str).collect::>(), ["alpha", "zeta"]); +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index b9c4ec6afd..5e2aad8a9e 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -1085,26 +1085,39 @@ pub(crate) fn count_notches(ui: &Ui, acc_id: egui::Id, wheel: &[(egui::MouseWhee /// Windows' default double-click time (Photoshop's; egui's own default is 0.3 s). const DOUBLE_CLICK_S: f64 = 0.5; -/// Whether this click on `resp` completes a double-click, counted the Windows way (Photoshop): -/// a click soon after a lone click at the same spot; the click after a double-click starts over. -/// (egui would call a third quick click a triple-click even when the first two weren't a double.) -fn second_click(ui: &Ui, resp: &Response) -> bool { +/// Finger jitter between taps easily exceeds egui's 6 px `max_click_dist`; 64 px is about two-thirds of an inch at 96 DPI (roughly a fingertip pad). 32 px still rejected casual Framework double-taps; Windows' mouse SM_CXDOUBLECLK is only 4 px and is not used for touch. +const TOUCH_MAX_CLICK_DIST: f32 = 64.0; + +/// True when this frame's pointer work came from a touch contact (still down, or a Touch event including End on the click frame). +fn pointer_from_touch(ui: &Ui) -> bool { + ui.input(|i| i.any_touches() || i.events.iter().any(|e| matches!(e, egui::Event::Touch { .. }))) +} + +/// Whether this click on `resp` completes a double-click / double-tap, counted the Windows way (Photoshop): a click soon after a lone click near the same spot; the click after a double starts over. +/// +/// Time window is 0.5 s (Windows / Photoshop; egui's default is 0.3 s). Mouse uses egui's `max_click_dist` (6 px); touch uses [`TOUCH_MAX_CLICK_DIST`] so finger jitter between taps still counts. Call on every click for that widget so the first tap is remembered. +pub fn second_click(ui: &Ui, resp: &Response) -> bool { if !resp.clicked() { return false; } let key = resp.id.with("pc-last-click"); - let (time, pos) = ui.input(|i| (i.time, i.pointer.interact_pos())); - let dist = ui.ctx().options(|o| o.input_options.max_click_dist); - let last: Option<(f64, Pos2)> = ui.data(|d| d.get_temp(key)); + // Prefer the response's hit pos: raw `pointer.interact_pos` can be unset on a release-only frame. + let pos = resp.interact_pointer_pos().or_else(|| ui.input(|i| i.pointer.interact_pos())); + let time = ui.input(|i| i.time); + let touch = pointer_from_touch(ui); + let mouse_dist = ui.ctx().options(|o| o.input_options.max_click_dist); + let last: Option<(f64, Pos2, bool)> = ui.data(|d| d.get_temp(key)); + // Looser distance if either tap in the pair was touch (End clears `any_touches` but leaves Touch in `events`). + let dist = if touch || last.is_some_and(|(_, _, t)| t) { TOUCH_MAX_CLICK_DIST.max(mouse_dist) } else { mouse_dist }; let double = match (last, pos) { - (Some((t, p)), Some(q)) => time - t < DOUBLE_CLICK_S && p.distance(q) < dist, + (Some((t, p, _)), Some(q)) => time - t < DOUBLE_CLICK_S && p.distance(q) < dist, _ => false, }; ui.data_mut(|d| { if let (false, Some(q)) = (double, pos) { - d.insert_temp(key, (time, q)); + d.insert_temp(key, (time, q, touch)); } else { - d.remove::<(f64, Pos2)>(key); + d.remove::<(f64, Pos2, bool)>(key); } }); double @@ -2065,6 +2078,81 @@ mod tests { out.textures_delta.clear(); } } + + /// One primary press+release at `pos`. When `touch`, emit Touch Start/End with the pointer events (as egui-winit does) so [`super::second_click`] uses the touch distance. + fn pointer_click(ctx: &egui::Context, time: f64, pos: egui::Pos2, touch: bool) -> bool { + let area = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(200.0, 200.0)); + let button = |pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }; + let touch_ev = |phase| egui::Event::Touch { device_id: egui::TouchDeviceId(1), id: egui::TouchId(1), phase, pos, force: None }; + let mut saw = false; + for (t, mut events) in [ + (time, { + let mut v = vec![egui::Event::PointerMoved(pos), button(true)]; + if touch { + v.insert(0, touch_ev(egui::TouchPhase::Start)); + } + v + }), + (time + 0.02, { + let mut v = vec![button(false)]; + if touch { + v.insert(0, touch_ev(egui::TouchPhase::End)); + } + v + }), + ] { + let mut raw = egui::RawInput { time: Some(t), screen_rect: Some(area), ..Default::default() }; + raw.events.append(&mut events); + let mut out = ctx.run_ui(raw, |ui| { + let r = ui.allocate_rect(area, egui::Sense::click()); + saw |= super::second_click(ui, &r); + }); + out.textures_delta.clear(); + } + saw + } + + /// #2274 touch: taps past egui's 6 px mouse limit (and past the old 32 px touch slop) within 0.5 s still count as a double; the same mouse path does not. + #[test] + fn second_click_tolerates_touch_jitter_but_keeps_mouse_tight() { + let area = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(200.0, 200.0)); + let a = egui::pos2(50.0, 50.0); + let mild = egui::pos2(70.0, 50.0); // 20 px — past mouse max_click_dist (6) + let wide = egui::pos2(50.0 + 48.0, 50.0); // past the old 32 px slop, inside TOUCH_MAX_CLICK_DIST (64) + for touch in [false, true] { + let ctx = egui::Context::default(); + let mut out = ctx.run_ui(egui::RawInput { time: Some(0.0), screen_rect: Some(area), ..Default::default() }, |ui| { + ui.allocate_rect(area, egui::Sense::click()); + }); + out.textures_delta.clear(); + assert!(!pointer_click(&ctx, 0.1, a, touch), "first click only arms the pair"); + let second = pointer_click(&ctx, 0.25, mild, touch); + assert_eq!(second, touch, "touch={touch}: 20 px jitter is a double only for touch"); + } + // Touch-only: casual Framework-style drift beyond the previous 32 px budget. + let ctx = egui::Context::default(); + let mut out = ctx.run_ui(egui::RawInput { time: Some(0.0), screen_rect: Some(area), ..Default::default() }, |ui| { + ui.allocate_rect(area, egui::Sense::click()); + }); + out.textures_delta.clear(); + assert!(!pointer_click(&ctx, 0.1, a, true)); + assert!(pointer_click(&ctx, 0.25, wide, true), "48 px touch jitter is still a double"); + } + + /// A mouse double past the time window is not a double, even at the same pixel. + #[test] + fn second_click_respects_the_half_second_window() { + let area = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(200.0, 200.0)); + let p = egui::pos2(40.0, 40.0); + let ctx = egui::Context::default(); + let mut out = ctx.run_ui(egui::RawInput { time: Some(0.0), screen_rect: Some(area), ..Default::default() }, |ui| { + ui.allocate_rect(area, egui::Sense::click()); + }); + out.textures_delta.clear(); + assert!(!pointer_click(&ctx, 0.1, p, false)); + assert!(!pointer_click(&ctx, 0.1 + super::DOUBLE_CLICK_S + 0.05, p, false), "past Windows' 0.5 s window"); + } + use egui_kittest::{Harness, kittest::Queryable}; /// #3789: a plain text field is one boxed, outline-free input whose text-selection colour stays diff --git a/docs/gaps.md b/docs/gaps.md index eeb1b11e68..c93fc048d2 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Where PhotoCraft falls short of Photoshop -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** minor (G14: the Nikon D4 white level follows the sample depth, so 12-bit D4 NEFs no longer open 2.6 stops dark, #5978; G14: a Nikon NEF's Adobe RGB preview is read as Adobe RGB (maker-note ColorSpace 0x001E), #5987; Apply Layer Comp action mapping, #7101; G10: JPEG Options dialog landed, FILE-215-19; G10: Cineon write; G10: DPX read and write, #1049, Cineon and Amiga IFF read; Classical Object Finder improvement; learned segmentation remains open; G9: Preferences › File Handling › Save in Background takes effect, #6207, 24 of 169 preferences do nothing; G7: Content-Aware Move previews every selection the command takes, coarse then exact, TOOL-213-4 done, #6680; G4: Pattern Stamp and Background Eraser draw live, TOOL-213-7; G9: Animated Zoom pref read: holding the Zoom tool zooms continuously, #6210, prefs that do nothing 25 of 166; G2: a cancelled 24 MP Gaussian Blur job stops in at most 44 ms, P25 under its 200 ms budget in local runs, baseline not re-run, #6205; G1: generated large-file fixtures, a 4.38 GB PSB opens on Windows through io, FILE-216-2 done, #5107; G9: fill and adjustment layers copy and paste whole, #2344; G1: smart filter masks show on PSD open, Photoshop-oracle floor 188/258; G10: DPX read and write, #1049, Cineon read; G1: CMYK Hue/Saturation and Color Balance keep the black ink; G1: CMYK Posterize on the stored values, psd-tools oracle floor 242/309; G1: CMYK adjustment layers composite on the document's inks, Photoshop-oracle floor 165/258; G1: CMYK Brightness/Contrast on the inks, CMYK Threshold's luma; G1: modern Brightness/Contrast's brightness curve matches Photoshop, Photoshop-oracle floor 151/258; G1: Lab documents composite on their stored L, a, b, Photoshop-oracle floor 152/258; G1: Levels on whole levels at every step, Photoshop-oracle floor 146/258, psd-tools 238/309; G1: 32-bit Brightness/Contrast legacy, Channel Mixer, Photo Filter and Black & White tint unclipped as in Photoshop, Photoshop-oracle floor 149/258; G1: mask feathers blur with their exact variance, psd-tools oracle floor 240/309; G1: Threshold splits on Photoshop's luminosity, psd-tools oracle floor 241/309; G1: pass-through clipping base with an adjustment renders isolated, psd-tools oracle floor 240/309; G1: 16-bit Posterize as Photoshop bins it, psd-tools oracle floor 241/309; G1: Gradient Map driven by the W3C luminosity, Photoshop-oracle floor 147/258; G7: Field Blur's on-canvas pins, #3373; G4: Hue Jitter amount track, #3825 partial; G4: minimum field interaction, #3825 partial; G1: CMYK Invert, Posterize and Threshold on the inks, 16-bit Lab Invert; Photoshop-oracle floor 145/258; G11: `.atn` import maps 112 events: Select and Mask, Filter Gallery Spatter, Ripple, Crystallize, Pattern Fill layers, mask targeting, Step Backward, nudges in points, Bitmap mode; G5: Free Transform H/V skew fields; G9: toolbar tool shortcut editing; refreshed against main: G2, G5, G7, G9, G10, G13, G14, G17 items that landed; G10: DPX read and write, #1049; G4: checkbox-only Brush Settings rows; G11: Load/Save Actions `.atn` landed, AUTO-219-2 partial; G5: hold-a-key spring-loaded tools landed, #2672; G1: #2465 fixed; stale missing-tool rows corrected: Color Replacement, Red Eye and Add/Delete Anchor Point are live tools, so only Art History Brush is missing; no estimate changed) · **Target:** Adobe Photoshop 2026 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-21 · **Change:** minor (G11: compound layer properties export as separate Photoshop Set events, rename last, #8304; G11: Batch and droplets take the result at the action's Save step, so actions ending in Save As and Close work; G11: `.atn` steps that switch to another open document import, play and save back, #7103; G14: the Nikon D4 white level follows the sample depth, so 12-bit D4 NEFs no longer open 2.6 stops dark, #5978; G14: a Nikon NEF's Adobe RGB preview is read as Adobe RGB (maker-note ColorSpace 0x001E), #5987; Apply Layer Comp action mapping, #7101; G10: JPEG Options dialog landed, FILE-215-19; G10: Cineon write; G10: DPX read and write, #1049, Cineon and Amiga IFF read; Classical Object Finder improvement; learned segmentation remains open; G9: Preferences › File Handling › Save in Background takes effect, #6207, 24 of 169 preferences do nothing; G7: Content-Aware Move previews every selection the command takes, coarse then exact, TOOL-213-4 done, #6680; G4: Pattern Stamp and Background Eraser draw live, TOOL-213-7; G9: Animated Zoom pref read: holding the Zoom tool zooms continuously, #6210, prefs that do nothing 25 of 166; G2: a cancelled 24 MP Gaussian Blur job stops in at most 44 ms, P25 under its 200 ms budget in local runs, baseline not re-run, #6205; G1: generated large-file fixtures, a 4.38 GB PSB opens on Windows through io, FILE-216-2 done, #5107; G9: fill and adjustment layers copy and paste whole, #2344; G1: smart filter masks show on PSD open, Photoshop-oracle floor 188/258; G10: DPX read and write, #1049, Cineon read; G1: CMYK Hue/Saturation and Color Balance keep the black ink; G1: CMYK Posterize on the stored values, psd-tools oracle floor 242/309; G1: CMYK adjustment layers composite on the document's inks, Photoshop-oracle floor 165/258; G1: CMYK Brightness/Contrast on the inks, CMYK Threshold's luma; G1: modern Brightness/Contrast's brightness curve matches Photoshop, Photoshop-oracle floor 151/258; G1: Lab documents composite on their stored L, a, b, Photoshop-oracle floor 152/258; G1: Levels on whole levels at every step, Photoshop-oracle floor 146/258, psd-tools 238/309; G1: 32-bit Brightness/Contrast legacy, Channel Mixer, Photo Filter and Black & White tint unclipped as in Photoshop, Photoshop-oracle floor 149/258; G1: mask feathers blur with their exact variance, psd-tools oracle floor 240/309; G1: Threshold splits on Photoshop's luminosity, psd-tools oracle floor 241/309; G1: pass-through clipping base with an adjustment renders isolated, psd-tools oracle floor 240/309; G1: 16-bit Posterize as Photoshop bins it, psd-tools oracle floor 241/309; G1: Gradient Map driven by the W3C luminosity, Photoshop-oracle floor 147/258; G7: Field Blur's on-canvas pins, #3373; G4: Hue Jitter amount track, #3825 partial; G4: minimum field interaction, #3825 partial; G1: CMYK Invert, Posterize and Threshold on the inks, 16-bit Lab Invert; Photoshop-oracle floor 145/258; G11: `.atn` import maps 112 events: Select and Mask, Filter Gallery Spatter, Ripple, Crystallize, Pattern Fill layers, mask targeting, Step Backward, nudges in points, Bitmap mode; G5: Free Transform H/V skew fields; G9: toolbar tool shortcut editing; refreshed against main: G2, G5, G7, G9, G10, G13, G14, G17 items that landed; G10: DPX read and write, #1049; G4: checkbox-only Brush Settings rows; G11: Load/Save Actions `.atn` landed, AUTO-219-2 partial; G5: hold-a-key spring-loaded tools landed, #2672; G1: #2465 fixed; stale missing-tool rows corrected: Color Replacement, Red Eye and Add/Delete Anchor Point are live tools, so only Art History Brush is missing; no estimate changed) · **Target:** Adobe Photoshop 2026 Every known shortfall against Photoshop 2026 (27.11.0), one entry each, ranked by user impact on the way to beta. This is the work list agents pick from: take the highest-ranked entry that @@ -98,6 +98,10 @@ what an agent can't finish alone. - **Estimate:** 120–200 h. ### G11. Actions and scripting +- **2026-10-21 evidence (#8304):** Save Actions expands compound layer-property edits into + separate Photoshop Set events, with rename last so named layer references remain valid. + Exported descriptors and native playback at RGB 8/16/32-bit have [regression coverage](../crates/engine/src/atn_cmds/layer_props_tests.rs); + independent Photoshop replay of the fix is still pending. No readiness percentage or estimate changed. - **Done 2026-10-17 (#7101):** `.atn` Apply Layer Comp steps with a named `compsClass` reference import as `layerComp.apply` and export to the same Photoshop event. Names resolve in the document being played; missing comps stop playback. Other reference forms stay kept @@ -110,6 +114,18 @@ what an agent can't finish alone. mapped events (paths, scripts, Generative Fill, Filter Gallery effects other than Spatter, Duplicate Layers into a new document, Bitmap halftone screens…) load as kept Photoshop steps that fail at playback by name. +- **Done 2026-10-21:** Batch and droplets take each result at the action's recorded Save, Save As + or Save a Copy step, written to the destination folder (Photoshop's "Override Action 'Save As' + Commands"). A document closed after its save is written as the save found it, so actions ending + in Save As and Close no longer fail on every file, and a Close that leaves another document + active no longer saves that one instead. While the saved document stays open, the steps after + the save still apply (#3058). Saves inside a played action, and the CLI's `batch` loop, still + save after the action. +- **Done 2026-10-19:** `.atn` steps that switch to another open document (a `select` of `Dcmn` by + offset: the previous, next or Nth tab from the active one) import as `document.activate {"offset"}`, + play and save back in the same shape, so actions that copy from one open document and paste into + another no longer stop at the switch (#7103). An offset past the first or last open document is + refused, not wrapped. - **Done 2026-10-12:** Load/Save Actions `.atn` in the panel menu, `batch --actions set.atn` and `actions.load`/`actions.save`; 112 Photoshop events map to commands both ways (Transform, Apply Image, Save for Web, smart-object edits, Gradient Map, Quick Mask, polygon selections, @@ -316,8 +332,11 @@ what an agent can't finish alone. | Date | Change | Summary | |---|---|---| +| 2026-10-21 | minor | #8304: compound layer properties export as separate Set events, rename last; independent Photoshop replay pending; percentages and estimates unchanged | +| 2026-10-21 | minor | G11: Batch and droplets take the result at the action's recorded Save / Save As step (Photoshop's "Override Action 'Save As' Commands"); estimate unchanged | | 2026-10-20 | minor | G8: Auto / Left-to-right / Right-to-left paragraph direction buttons in the Paragraph panel, Properties and the Type options bar; TYPE-218-2 partial (the visual RTL caret is still missing) | | 2026-10-19 | minor | G10: JPEG Options dialog landed (FILE-215-19); no readiness percentage or estimate changed | +| 2026-10-19 | minor | G11: `.atn` steps that select another open document by offset import as `document.activate {"offset"}`, play and save back (#7103); estimate unchanged | | 2026-10-18 | minor | G13: Russian gained the 1,229 dynamic UI labels other catalogs already had (filter and adjustment parameters, Camera Raw, lighting, History/Layers/paths menus, metadata, System Info, status templates, dialogs); engine messages and native review remain open | | 2026-10-17 | minor | Classical Object Finder improvement; learned segmentation remains open | | 2026-10-15 | minor | G10: Cineon write landed (10-bit log gray/RGB, Save As `.cin`) | diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 76c56c2999..1e6cd36bb5 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,6 +1,6 @@ # PhotoCraft vs Adobe Photoshop: target-app parity -> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-19 · **Change:** minor (JPEG Options dialog landed, FILE-215-19; Content-Aware Move live preview for every selection size, TOOL-213-4 done, #6680; retouching evidence: Pattern Stamp and Background Eraser draw live, only the Mixer Brush is left of TOOL-213-7; fill and adjustment layers copy and paste whole, UI-217-23 partial, #2344; shape layers take Density and Feather, TOOL-214-12 done, #5846; on-canvas corner radius widgets, TOOL-214-12; `.atn` import maps 112 events; Free Transform H/V skew fields; stale rows corrected against `main` and the scorecard, estimates unchanged; Photoshop Actions `.atn` load and save; 2026-10-10: Tilt-Shift measurements added; earlier the same day: major, full re-measure against Photoshop 2026 27.11.0; merged `parity-estimate.md` and the roadmap's honest assessment; stale missing-tool rows corrected: Color Replacement and Red Eye are live toolbar tools, Frame landed; percentages unchanged) · **Target:** Adobe Photoshop 2026 +> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-21 · **Change:** minor (compound layer properties export as separate Photoshop Set events, #8304; JPEG Options dialog landed, FILE-215-19; Content-Aware Move live preview for every selection size, TOOL-213-4 done, #6680; retouching evidence: Pattern Stamp and Background Eraser draw live, only the Mixer Brush is left of TOOL-213-7; fill and adjustment layers copy and paste whole, UI-217-23 partial, #2344; shape layers take Density and Feather, TOOL-214-12 done, #5846; on-canvas corner radius widgets, TOOL-214-12; `.atn` import maps 112 events; Free Transform H/V skew fields; stale rows corrected against `main` and the scorecard, estimates unchanged; Photoshop Actions `.atn` load and save; 2026-10-10: Tilt-Shift measurements added; earlier the same day: major, full re-measure against Photoshop 2026 27.11.0; merged `parity-estimate.md` and the roadmap's honest assessment; stale missing-tool rows corrected: Color Replacement and Red Eye are live toolbar tools, Frame landed; percentages unchanged) · **Target:** Adobe Photoshop 2026 The authoritative answer to "how close is PhotoCraft to Photoshop, and how much work is left". The one-page summary is [`ROADMAP.md`](../ROADMAP.md); the ranked work list is [`gaps.md`](gaps.md); @@ -177,7 +177,7 @@ parity for the area and are already contained in the dimension rows above. | Vector shapes and paths | 55% | 80–140 | Pen, Direct/Path Selection, shapes, shape strokes. Freeform and Curvature Pen (#1054). Add/Delete Anchor Point and Convert Point tools exist (`Tool` enum); on-canvas corner radius widgets on live rectangles (TOOL-214-12, 2026-10-13); shape Density and Feather, the path as the fill's soft vector mask, matching Photoshop's 10 px feather ramp within 1 level (#5846, 2026-10-15); missing path-point nudge; shape parity tracking issue #2496 | | Colour management and modes | 70% | 60–100 | Own ICC engine, soft proof, CMYK/Lab/Indexed/Duotone/Multichannel; no Adobe CMYK profiles, no spot plates (FILE-215-17), monitor profile only at launch | | Camera Raw and computational photo (Photomerge, HDR, lens) | 45% | 120–200 | Raw for DNG/CR2/NEF/ARW/RW2/ORF/RAF(uncompressed)/PEF; no CR3; ACR dialog partial (no masking, calibration); generic lens profiles only | -| Automation, actions, batch, scripting | 45% | 120–200 | CLI/MCP/control channel ahead of Photoshop; Actions panel and recording not document-independent (AUTO-219-1/2); `.atn` load/save since 2026-10-12 (112 events mapped, the rest kept verbatim); no ExtendScript/UXP | +| Automation, actions, batch, scripting | 45% | 120–200 | CLI/MCP/control channel ahead of Photoshop; Actions panel and recording not document-independent (AUTO-219-1/2); `.atn` load/save since 2026-10-12 (112 events mapped, the rest kept verbatim); compound layer properties export as separate Set events with rename last (#8304, 2026-10-21; independent Photoshop replay pending); no ExtendScript/UXP | | File I/O | 60% | 300–500 | See [file-format-parity.md](file-format-parity.md) | | Print, export, Save for Web | 50% | 60–100 | no Photoshop PDF, no separations, Save for Web settings not remembered | | Video / timeline, data-driven graphics | 30% | 100–200 | Menu items live; frame model persists in `.pcraft`; no PSD video-layer mapping | @@ -565,6 +565,7 @@ See [method, raw measurements and reproduction](five-algorithm-performance.md). | Date | Change | Summary | |---|---|---| +| 2026-10-21 | minor | #8304: compound layer properties export as separate Set events, rename last; independent Photoshop replay pending; percentages and estimates unchanged | | 2026-10-19 | minor | JPEG Options dialog landed (FILE-215-19); percentages unchanged | | 2026-10-18 | minor | Layers: with no selection, Copy takes fill and adjustment layers whole and Paste recreates them (#2344); UI-217-23 stays partial; percentages unchanged | | 2026-10-16 | minor | Content-Aware Move: live result preview for every selection the command takes (TOOL-213-4 done, #6680); percentages unchanged |