diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 0b8aeb7681..fc9629b06b 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -478,9 +478,12 @@ fn display_doc(app: &mut PhotocraftApp, idx: usize) -> (std::sync::Arc let key = (crate::layer_style::preview_hash(&d.fields) ^ st.revision.wrapping_mul(0x9e37_79b9_7f4a_7c15)) | 1 << 63; if app.style_preview.as_ref().map(|p| p.0) != Some(key) { let shown = crate::layer_style::preview_document(&st.doc, &app.session.patterns, &d.fields).map(std::sync::Arc::new); + if let Err(error) = &shown { + log::warn!("Layer Style preview: {error}"); + } app.style_preview = Some((key, shown)); } - if let Some((_, Some(doc))) = &app.style_preview { + if let Some((_, Ok(doc))) = &app.style_preview { return (doc.clone(), key); } } diff --git a/crates/ui-egui/src/layer_style.rs b/crates/ui-egui/src/layer_style.rs index 276a62b85e..2a3748bb56 100644 --- a/crates/ui-egui/src/layer_style.rs +++ b/crates/ui-egui/src/layer_style.rs @@ -326,13 +326,30 @@ pub fn confirm(app: &mut PhotocraftApp, f: &Map) -> Result) -> Result<(), String> { + for kind in std::iter::once(BLENDING).chain(KINDS.iter().map(|(kind, _)| *kind)) { + let key = format!("p:{kind}"); + if let Some(value) = f.get(&key) + && !value.is_object() + { + return Err(format!("invalid layer style parameters: `{key}` must be an object")); + } + } + Ok(()) +} + /// Runs the dialog's commands through `run`: blending options, clear, then each enabled effect. fn apply(f: &Map, mut run: impl FnMut(&str, Value) -> Result) -> Result { + validate_params(f)?; let layer = f.get("layer").cloned().unwrap_or(Value::Null); let initial_light_angle = f.get("globalLight").and_then(Value::as_f64); - if let Some(Value::Object(bo)) = f.get(&format!("p:{BLENDING}")) { + if let Some(bo) = f.get(&format!("p:{BLENDING}")).and_then(Value::as_object) { let mut p = Value::Object(bo.clone()); - p["layer"] = layer.clone(); + if let Some(params) = p.as_object_mut() { + params.insert("layer".into(), layer.clone()); + } run("layer.layerStyle.blendingOptions", p)?; } let _ = run("layer.layerStyle.clear", json!({"layer": layer})); @@ -342,7 +359,9 @@ fn apply(f: &Map, mut run: impl FnMut(&str, Value) -> Result, -) -> Option { +) -> Result { let mut s = photocraft_engine::Session::new(); s.patterns = patterns.clone(); s.add_document(doc.clone(), None); - apply(f, |id, p| s.execute(id, p).map_err(|e| e.to_string())).ok()?; - s.active().map(|d| (*d.doc).clone()) + apply(f, |id, p| s.execute(id, p).map_err(|e| e.to_string()))?; + s.active().map(|d| (*d.doc).clone()).ok_or_else(|| "no preview document".into()) } /// Dialog body (left list, right parameters). pub fn body(ui: &mut egui::Ui, f: &mut Map) { let t = Tokens::get(ui.ctx()); let selected = f.get("selected").and_then(Value::as_str).unwrap_or("dropShadow").to_string(); + let invalid = validate_params(f).err(); ui.horizontal_top(|ui| { // Left: effect list. ui.vertical(|ui| { @@ -440,79 +460,85 @@ pub fn body(ui: &mut egui::Ui, f: &mut Map) { ui.label(RichText::new(tl!(&label)).font(crate::theme::semibold(14.0)).color(t.text)); ui.add_space(6.0); let pkey = format!("p:{selected}"); - let mut p = f.get(&pkey).cloned().unwrap_or_else(|| defaults(&selected)); - for &(key, label, kind) in spec(&selected) { - match kind { - P::Slider(min, max, unit) => { - let mut v = p.get(key).and_then(Value::as_f64).unwrap_or(min as f64) as f32; - if widgets::slider_row(ui, label, &mut v, min..=max, unit, None).changed() { - p[key] = json!(v.round()); + // Only reachable through `ui.dialog.set`: say what is wrong once, keep the bad value. + if let Some(error) = &invalid { + ui.colored_label(t.danger, error); + } + let params = f.get(&pkey).cloned().unwrap_or_else(|| defaults(&selected)); + if let Some(mut p) = params.as_object().map(|o| Value::Object(o.clone())) { + for &(key, label, kind) in spec(&selected) { + match kind { + P::Slider(min, max, unit) => { + let mut v = p.get(key).and_then(Value::as_f64).unwrap_or(min as f64) as f32; + if widgets::slider_row(ui, label, &mut v, min..=max, unit, None).changed() { + p[key] = json!(v.round()); + } + } + P::Color => { + ui.horizontal(|ui| { + ui.label(RichText::new(tl!(&label)).color(t.text_dim)); + let hexs = p.get(key).and_then(Value::as_str).unwrap_or("#000000").to_string(); + let mut c = parse_hex(&hexs); + if ui.color_edit_button_srgba(&mut c).changed() { + p[key] = json!(format!("#{:02x}{:02x}{:02x}", c.r(), c.g(), c.b())); + } + }); + } + P::Blend => { + ui.horizontal(|ui| { + ui.label(RichText::new(tl!(&label)).color(t.text_dim)); + let mut cur = p.get(key).and_then(Value::as_str).unwrap_or("Normal").to_string(); + let opts: Vec<(String, &str)> = + photocraft_color::BlendMode::LAYER_MODES.iter().map(|m| (m.label().to_string(), m.label())).collect(); + if widgets::dropdown(ui, &format!("fx-blend-{selected}"), &mut cur, &opts, 150.0) { + p[key] = json!(cur); + } + }); + } + P::Choice(options) => { + ui.horizontal(|ui| { + ui.label(RichText::new(tl!(&label)).color(t.text_dim)); + let mut cur = p.get(key).and_then(Value::as_str).unwrap_or(options[0].0).to_string(); + let opts: Vec<(String, &str)> = options.iter().map(|(v, l)| (v.to_string(), *l)).collect(); + if widgets::dropdown(ui, &format!("fx-{selected}-{key}"), &mut cur, &opts, 150.0) { + p[key] = json!(cur); + } + }); } - } - P::Color => { - ui.horizontal(|ui| { - ui.label(RichText::new(tl!(&label)).color(t.text_dim)); - let hexs = p.get(key).and_then(Value::as_str).unwrap_or("#000000").to_string(); - let mut c = parse_hex(&hexs); - if ui.color_edit_button_srgba(&mut c).changed() { - p[key] = json!(format!("#{:02x}{:02x}{:02x}", c.r(), c.g(), c.b())); - } - }); - } - P::Blend => { - ui.horizontal(|ui| { - ui.label(RichText::new(tl!(&label)).color(t.text_dim)); - let mut cur = p.get(key).and_then(Value::as_str).unwrap_or("Normal").to_string(); - let opts: Vec<(String, &str)> = - photocraft_color::BlendMode::LAYER_MODES.iter().map(|m| (m.label().to_string(), m.label())).collect(); - if widgets::dropdown(ui, &format!("fx-blend-{selected}"), &mut cur, &opts, 150.0) { - p[key] = json!(cur); - } - }); - } - P::Choice(options) => { - ui.horizontal(|ui| { - ui.label(RichText::new(tl!(&label)).color(t.text_dim)); - let mut cur = p.get(key).and_then(Value::as_str).unwrap_or(options[0].0).to_string(); - let opts: Vec<(String, &str)> = options.iter().map(|(v, l)| (v.to_string(), *l)).collect(); - if widgets::dropdown(ui, &format!("fx-{selected}-{key}"), &mut cur, &opts, 150.0) { - p[key] = json!(cur); - } - }); - } - P::Pattern => { - let list: Vec<(String, String)> = f - .get("patternList") - .and_then(Value::as_array) - .map(|a| a.iter().filter_map(|e| Some((e.get(0)?.as_str()?.to_string(), e.get(1)?.as_str()?.to_string()))).collect()) - .unwrap_or_default(); - ui.horizontal(|ui| { - ui.label(RichText::new(tl!(&label)).color(t.text_dim)); - let mut cur = p - .get(key) - .and_then(Value::as_str) - .filter(|c| !c.is_empty()) - .map(str::to_string) - .or_else(|| list.first().map(|l| l.0.clone())) + P::Pattern => { + let list: Vec<(String, String)> = f + .get("patternList") + .and_then(Value::as_array) + .map(|a| a.iter().filter_map(|e| Some((e.get(0)?.as_str()?.to_string(), e.get(1)?.as_str()?.to_string()))).collect()) .unwrap_or_default(); - let opts: Vec<(String, &str)> = list.iter().map(|(id, n)| (id.clone(), n.as_str())).collect(); - if widgets::dropdown(ui, &format!("fx-{selected}-{key}"), &mut cur, &opts, 180.0) - || p.get(key).and_then(Value::as_str).is_none_or(str::is_empty) - { - p[key] = json!(cur); + ui.horizontal(|ui| { + ui.label(RichText::new(tl!(&label)).color(t.text_dim)); + let mut cur = p + .get(key) + .and_then(Value::as_str) + .filter(|c| !c.is_empty()) + .map(str::to_string) + .or_else(|| list.first().map(|l| l.0.clone())) + .unwrap_or_default(); + let opts: Vec<(String, &str)> = list.iter().map(|(id, n)| (id.clone(), n.as_str())).collect(); + if widgets::dropdown(ui, &format!("fx-{selected}-{key}"), &mut cur, &opts, 180.0) + || p.get(key).and_then(Value::as_str).is_none_or(str::is_empty) + { + p[key] = json!(cur); + } + }); + } + P::Check => { + let mut b = p.get(key).and_then(Value::as_bool).unwrap_or(false); + if widgets::checkbox(ui, &mut b, label).changed() { + p[key] = json!(b); } - }); - } - P::Check => { - let mut b = p.get(key).and_then(Value::as_bool).unwrap_or(false); - if widgets::checkbox(ui, &mut b, label).changed() { - p[key] = json!(b); } } + ui.add_space(2.0); } - ui.add_space(2.0); + f.insert(pkey, p); } - f.insert(pkey, p); }); }); } @@ -576,6 +602,64 @@ mod tests { assert_eq!((fx(&shown), fx(&st.doc)), (2, 0)); } + #[test] + fn preview_rejects_non_object_effect_params() { + let mut s = photocraft_engine::Session::new(); + s.execute("file.new", json!({"width": 16, "height": 16})).unwrap(); + s.execute("layer.new.layer", json!({})).unwrap(); + let st = s.active().unwrap(); + let mut f = initial_fields(st.doc.layer(st.active_layer.unwrap()).unwrap(), Some("colorOverlay"), st.doc.global_light.angle); + f.insert("p:colorOverlay".into(), json!("invalid")); + + assert!(preview_document(&st.doc, &s.patterns, &f).is_err()); + } + + #[test] + fn apply_rejects_non_object_params_before_running_commands() { + let mut f = Map::new(); + f.insert("p:stroke".into(), json!([1, 2])); + f.insert("on:stroke".into(), json!(true)); + let mut calls = 0; + + let result = apply(&f, |_, _| { + calls += 1; + Ok(Value::Null) + }); + + assert!(result.is_err()); + assert_eq!(calls, 0, "invalid params must not partially apply the style"); + } + + #[test] + fn body_keeps_invalid_params_and_renders_without_panicking() { + let ctx = egui::Context::default(); + PhotocraftApp::setup_context(&ctx, crate::theme::ThemeKind::ALL[0]); + let mut fields = Map::new(); + fields.insert("selected".into(), json!("dropShadow")); + fields.insert("p:dropShadow".into(), json!(7)); + + let mut out = ctx.run_ui(Default::default(), |ui| body(ui, &mut fields)); + out.textures_delta.clear(); + + assert_eq!(fields.get("p:dropShadow"), Some(&json!(7))); + } + + #[test] + fn confirm_rejects_invalid_params_and_applies_valid_effects() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); + app.run("file.new", json!({"width": 16, "height": 16})).unwrap(); + app.run("layer.new.layer", json!({})).unwrap(); + let layer = app.session.active().unwrap().doc.layer(app.session.active().unwrap().active_layer.unwrap()).unwrap().clone(); + let mut invalid = initial_fields(&layer, Some("colorOverlay"), 0.0); + invalid.insert("p:colorOverlay".into(), json!(null)); + assert!(confirm(&mut app, &invalid).is_err()); + assert!(app.session.active().unwrap().doc.layer(layer.id).unwrap().effects.items.is_empty()); + + let valid = initial_fields(&layer, Some("colorOverlay"), 0.0); + confirm(&mut app, &valid).unwrap(); + assert_eq!(app.session.active().unwrap().doc.layer(layer.id).unwrap().effects.items.len(), 1); + } + #[test] fn percent_fields_round_trip_through_the_engine() { let mut s = photocraft_engine::Session::new(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index a4ef366123..d3fbb97fa3 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -340,8 +340,8 @@ pub struct PhotocraftApp { pub(crate) transform_preview: Option, /// Move-tool ⇧/⌥ drag state (move_mods). pub(crate) move_mods: move_mods::MoveDrag, - /// Live Layer Style dialog preview: (key over revision + style fields, document with the style applied). - pub(crate) style_preview: Option<(u64, Option>)>, + /// Live Layer Style dialog preview: (key over revision + style fields, preview or validation error). + pub(crate) style_preview: Option<(u64, Result, String>)>, /// Liquify dialog, Puppet Warp and Perspective Warp sessions (distort_ui). pub(crate) distort: distort_ui::Distort, /// Gradient tool live-mode drags and previews (gradient_ui).