Replace an adjustment smart filter's params when its dialog edits them

layer.smartFilter.setParams merges, so a key the dialog removed stayed
stored. A Channel Mixer filter with Monochrome turned off kept its gray
row, which is read over red: the Red sliders had no effect. The dialog
holds all of an adjustment's settings, so it now passes replace: true;
other callers keep merging.
This commit is contained in:
Khawar Zamman Wani committed 2026-11-06 11:52:38 +05:30
1 parent a259993118
commit 2e13ec280b
6 files changed
+115 -4

No files matched your search

+1 -1
View File
@@ -340,7 +340,7 @@ pub fn specs() -> Vec<CommandSpec> {
"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
),
+5 -1
View File
@@ -170,7 +170,9 @@ fn edited_shape_params(s: &Session, previous: &Value, patch: &Value) -> Result<V
pub(super) fn set_filter_params(s: &mut Session, p: &Value) -> Result<Value> {
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<Value> {
}
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),
+21
View File
@@ -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]}));
}
+1 -1
View File
@@ -246,7 +246,7 @@ pub fn owns(fields: &Map<String, Value>) -> 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))
}
@@ -134,7 +134,14 @@ pub(crate) fn edit_command(app: &PhotocraftApp, fields: &Map<String, Value>) ->
}
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}),
))
}
}
@@ -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::<egui::Rect>(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");
}