diff --git a/crates/engine/src/cmd/smart_cmds.rs b/crates/engine/src/cmd/smart_cmds.rs index fd9234fc3e..c80b93ce1f 100644 --- a/crates/engine/src/cmd/smart_cmds.rs +++ b/crates/engine/src/cmd/smart_cmds.rs @@ -340,7 +340,7 @@ pub fn specs() -> Vec { "layer.smartFilter.setParams", "Edit Smart Filter", &[], - r##"{"layer":id?,"index":u32?,"params":{…} (merged)}"##, + r##"{"layer":id?,"index":u32?,"params":{…} (merged),"replace":bool=false (params replace the stored ones instead)}"##, has_smart_filters, set_filter_params ), diff --git a/crates/engine/src/cmd/smart_cmds/filters.rs b/crates/engine/src/cmd/smart_cmds/filters.rs index af9d08b9a6..c718848c36 100644 --- a/crates/engine/src/cmd/smart_cmds/filters.rs +++ b/crates/engine/src/cmd/smart_cmds/filters.rs @@ -170,7 +170,9 @@ fn edited_shape_params(s: &Session, previous: &Value, patch: &Value) -> Result Result { const CMD: &str = "layer.smartFilter.setParams"; - let new = p.get("params").cloned().ok_or_else(|| bad(CMD, "pass `params` (merged into the filter's parameters)"))?; + let new = p.get("params").cloned().ok_or_else(|| bad(CMD, "pass `params` (merged into the filter's parameters, or replacing them with `replace`)"))?; + // An editor holding all of a filter's settings replaces them, so a key it removed stops applying. + let replace = p.get("replace").and_then(Value::as_bool).unwrap_or(false); let authorize = s.authorize; // A Displace given another open document as its map embeds that one in place of the old // (#7423). It is read here, before the edit, from the session; the request is authorized @@ -209,6 +211,8 @@ pub(super) fn set_filter_params(s: &mut Session, p: &Value) -> Result { } if let Some(params) = shape_params { f.params = params; + } else if replace { + f.params = new; } else { match (&mut f.params, new) { (Value::Object(old), Value::Object(n)) => old.extend(n), diff --git a/crates/engine/src/cmd/smart_cmds/tests.rs b/crates/engine/src/cmd/smart_cmds/tests.rs index 6e8ce37f9c..ca20a49a19 100644 --- a/crates/engine/src/cmd/smart_cmds/tests.rs +++ b/crates/engine/src/cmd/smart_cmds/tests.rs @@ -1397,3 +1397,24 @@ fn native_digest_cache_misses_after_copy_on_write_and_keeps_recently_used_source drop(newest); assert!(weak.upgrade().is_none()); } + +/// `replace` makes `params` the filter's whole settings, so a key the edit left out stops +/// applying; without it the params merge, as partial edits rely on. Either way it is one undo. +#[test] +fn set_params_replace_drops_the_keys_it_leaves_out() { + let mut s = session(8); + paint(&mut s); + convert(&mut s); + s.execute("image.adjustments.channelMixer", json!({"monochrome": true, "gray": [30, 30, 40, 0]})).unwrap(); + let params = |s: &Session| active_smart(s).smart_filters[0].params.clone(); + + s.execute("layer.smartFilter.setParams", json!({"params": {"monochrome": false}})).unwrap(); + assert_eq!(params(&s), json!({"monochrome": false, "gray": [30, 30, 40, 0]}), "merged by default"); + + let edit = json!({"monochrome": false, "red": [70, 30, 0, 0]}); + s.execute("layer.smartFilter.setParams", json!({"params": edit, "replace": true})).unwrap(); + assert_eq!(params(&s), edit, "replaced"); + + s.undo(); + assert_eq!(params(&s), json!({"monochrome": false, "gray": [30, 30, 40, 0]})); +} diff --git a/crates/ui-egui/src/dialogs/adjust_dialog.rs b/crates/ui-egui/src/dialogs/adjust_dialog.rs index 715c0860bb..0665c4fbaf 100644 --- a/crates/ui-egui/src/dialogs/adjust_dialog.rs +++ b/crates/ui-egui/src/dialogs/adjust_dialog.rs @@ -246,7 +246,7 @@ pub fn owns(fields: &Map) -> bool { } /// Root of the dialog editor's view state in egui memory. -fn dialog_mem(kind: &str) -> egui::Id { +pub(crate) fn dialog_mem(kind: &str) -> egui::Id { egui::Id::unique(("adjust-dialog", kind)) } diff --git a/crates/ui-egui/src/dialogs/smart_filter_dialog.rs b/crates/ui-egui/src/dialogs/smart_filter_dialog.rs index dbe00fdf9b..d1eb305ca0 100644 --- a/crates/ui-egui/src/dialogs/smart_filter_dialog.rs +++ b/crates/ui-egui/src/dialogs/smart_filter_dialog.rs @@ -134,7 +134,14 @@ pub(crate) fn edit_command(app: &PhotocraftApp, fields: &Map) -> } Ok((COMMAND.into(), params)) } else { - Ok(("layer.smartFilter.setParams".into(), json!({"layer": layer.0, "index": index, "params": crate::filter_dialog::params_of(fields)}))) + // An adjustment's editor opens on all the recorded settings and may remove keys, so OK + // replaces them: merged, a removed one stays stored and can override the edit (a stale + // Channel Mixer `gray` is read over `red`). + let replace = fields.contains_key("__adjust"); + Ok(( + "layer.smartFilter.setParams".into(), + json!({"layer": layer.0, "index": index, "params": crate::filter_dialog::params_of(fields), "replace": replace}), + )) } } diff --git a/crates/ui-egui/src/panels/smart_ui/editor_tests.rs b/crates/ui-egui/src/panels/smart_ui/editor_tests.rs index 6e0b9d14f8..c51169c235 100644 --- a/crates/ui-egui/src/panels/smart_ui/editor_tests.rs +++ b/crates/ui-egui/src/panels/smart_ui/editor_tests.rs @@ -185,3 +185,82 @@ fn filter_editor_starts_missing_parameters_from_defaults_not_remembered_values() let id = crate::filter_dialog::open(&mut app, command).unwrap(); assert_eq!(app.ui.dialog_mut(id).unwrap().fields["reduceNoise"].as_f64(), Some(60.0)); } + +/// A smart object in a `mode` document with `command` recorded as its one smart filter, and its layer id. +fn adjustment_filter(mode: &str, command: &str, params: serde_json::Value) -> (PhotocraftApp, u64) { + let mut s = photocraft_engine::Session::new(); + s.execute("file.new", json!({"width": 64, "height": 48, "mode": mode})).unwrap(); + s.execute("layer.new.layer", json!({})).unwrap(); + s.execute("edit.fill", json!({"contents": "black"})).unwrap(); + let id = s.execute("layer.smartObjects.convertToSmartObject", json!({})).unwrap()["layer"].as_u64().unwrap(); + s.execute(command, params).unwrap(); + let mut app = PhotocraftApp::new(s, crate::Services::default()); + app.background_jobs = false; + (app, id) +} + +fn recorded(app: &PhotocraftApp, id: u64) -> photocraft_doc::SmartObject { + match &app.session.active().unwrap().doc.layer(photocraft_doc::LayerId(id)).unwrap().content { + photocraft_doc::LayerContent::Smart(sm) => sm.clone(), + other => panic!("expected a smart object, got {}", other.kind_name()), + } +} + +/// The app in a harness with the editor of the layer's first smart filter open, and the dialog's id. +fn edit_first_filter(app: PhotocraftApp, id: u64) -> (Harness<'static, PhotocraftApp>, u64) { + let sm = recorded(&app, id); + let mut h = Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state(|ui, app: &mut PhotocraftApp| crate::dialogs::show(app, ui.ctx()), app); + PhotocraftApp::setup_context(&h.ctx, crate::theme::ThemeKind::ALL[0]); + let ctx = h.ctx.clone(); + super::open_editor(h.state_mut(), &ctx, id, &sm, 0); + let dialog = h.state().ui.dialogs.last().unwrap().id; + h.run_steps(3); + (h, dialog) +} + +/// Monochrome off removes `gray` from the dialog's values; a stored one read over the Red row +/// would keep the old mix, whatever the Red sliders say, on OK and on every later edit. +#[test] +fn a_channel_mixer_smart_filter_turned_off_monochrome_keeps_its_red_row() { + let (app, id) = adjustment_filter("rgb", "image.adjustments.channelMixer", json!({"monochrome": true, "gray": [30, 30, 40, 0]})); + let (mut h, dialog) = edit_first_filter(app, id); + h.get_by_label("Monochrome").click(); + h.run_steps(3); + let fields = &mut h.state_mut().ui.dialog_mut(dialog).unwrap().fields; + assert_eq!(fields.get("monochrome"), Some(&json!(false))); + assert!(!fields.contains_key("gray"), "the editor removed the gray row: {fields:?}"); + fields.insert("red".into(), json!([70.0, 30.0, 0.0, 0.0])); + h.run_steps(2); + crate::dialogs::confirm(h.state_mut(), dialog).unwrap(); + + let sm = recorded(h.state(), id); + assert_eq!(sm.smart_filters.len(), 1, "no filter was stacked"); + let adjustment = photocraft_engine::adjust_params::from_params("channelMixer", &sm.smart_filters[0].params, None, photocraft_doc::ColorMode::Rgb).unwrap(); + let photocraft_doc::Adjustment::ChannelMixer { matrix, monochrome } = adjustment else { panic!("Channel Mixer params") }; + assert!(!monochrome); + assert!(matrix[0].iter().zip([0.7, 0.3, 0.0, 0.0]).all(|(a, b)| (a - b).abs() < 1e-5), "the edited Red row renders: {:?}", matrix[0]); +} + +/// In Lab, top-level `points` aliases Lightness and the editor removes it on an edit. A stored +/// one must not override the edit: the Lightness the editor sends is what the filter renders. +#[test] +fn a_lab_curves_smart_filter_keeps_a_lightness_edit() { + let old = json!([[0, 0], [128, 64], [255, 255]]); + let (app, id) = adjustment_filter("lab", "image.adjustments.curves", json!({"points": old, "lightness": old})); + let (mut h, dialog) = edit_first_filter(app, id); + // Add a point on the Lightness curve, the channel the dialog shows first. + let graph = h.ctx.data(|d| d.get_temp::(crate::adjust_dialog::dialog_mem("curves").with("curves-graph"))).unwrap(); + click(&mut h, egui::pos2(graph.left() + graph.width() * 0.75, graph.top() + graph.height() * 0.1)); + let fields = &h.state_mut().ui.dialog_mut(dialog).unwrap().fields; + assert!(!fields.contains_key("points"), "the editor removed the alias: {fields:?}"); + let edited = fields["lightness"].clone(); + assert_ne!(edited, old, "the click edited Lightness"); + crate::dialogs::confirm(h.state_mut(), dialog).unwrap(); + + let sm = recorded(h.state(), id); + assert_eq!(sm.smart_filters.len(), 1, "no filter was stacked"); + let read = |params: &serde_json::Value| photocraft_engine::adjust_params::from_params("curves", params, None, photocraft_doc::ColorMode::Lab).unwrap(); + let photocraft_doc::Adjustment::Curves { per_channel, .. } = read(&sm.smart_filters[0].params) else { panic!("Curves params") }; + let photocraft_doc::Adjustment::Curves { per_channel: wanted, .. } = read(&json!({"lightness": edited})) else { panic!("Curves params") }; + assert_eq!(per_channel[0], wanted[0], "the edited Lightness renders"); +}