From 05ada26dc143856475deac568794ae85ef116b68 Mon Sep 17 00:00:00 2001 From: Sven Lafebre Date: Mon, 9 Nov 2026 18:52:08 +0100 Subject: [PATCH 1/2] 3D effect dialogs say why a selection is refused and turn OK off (#6140) The 3D effects refuse objects they can't build from, such as clipping groups and blends under Extrude & Bevel. The preview failed, OK stayed on and only repeated the error in a status-bar line that gets cut off, so the dialog looked broken. The dialog now shows the command's reason under Preview and turns OK and Enter off while it holds. With Preview off, it tries the values once when they change and rolls back, so the reason still appears. --- crates/ui-egui/src/dialogs/effect.rs | 33 ++++++++- crates/ui-egui/src/dialogs/form.rs | 23 +++++- .../src/dialogs/tests_revolve_gizmo.rs | 74 +++++++++++++++++++ 3 files changed, 127 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/dialogs/effect.rs b/crates/ui-egui/src/dialogs/effect.rs index 93688542c..13e15c413 100644 --- a/crates/ui-egui/src/dialogs/effect.rs +++ b/crates/ui-egui/src/dialogs/effect.rs @@ -24,9 +24,39 @@ pub(super) const SPEC: DialogSpec = DialogSpec { preview: true, // Load Texture… adds the texture it reads. passes: |d, field| field == vectorcraft_effects::pixel::TEXTURE_IMAGE && matches!(d.str("__effect").as_str(), GLASS | TEXTURIZER), + // OK could only fail the same way: off, beside the reason the dialog shows (#6140). + ok_enabled: Some(|_, d| !(vectorcraft_effects::is_three_d(&d.str("__effect")) && d.fields.contains_key(form::REFUSED))), ..DialogSpec::FORM }; +/// The field holding the values a 3D effect's dialog last tried with Preview off. +const TRIED: &str = "__tried"; + +/// A 3D effect refuses objects it can't build from and values it can't draw, in the dialog as OK +/// would: shows why under Preview (#6140). With Preview off no preview has tried `params`, so they +/// are tried here, when they change, and rolled back at once. +fn three_d_refusal(app: &mut VectorcraftApp, ui: &mut egui::Ui, d: &mut Dialog, cmd: &str, params: Value) { + if d.bool("preview") { + d.fields.remove(TRIED); + } else if d.fields.get(TRIED) != Some(¶ms) { + // A failure to begin leaves no interaction: the preview below then reports it. + let _ = app.session.begin_interaction(&d.str("__label")); + match app.session.preview(cmd, ¶ms) { + Ok(_) => { + d.fields.remove(form::REFUSED); + } + Err(e) => form::note_refusal(d, &e), + } + // Only a try: without an interaction there is nothing to roll back. + let _ = app.session.cancel_interaction(); + d.fields.insert(TRIED.into(), params); + } + if let Some(why) = d.fields.get(form::REFUSED).and_then(Value::as_str) { + ui.add_space(6.0); + ui.add(egui::Label::new(egui::RichText::new(why).color(Tokens::get(ui.ctx()).warning)).wrap()); + } +} + /// The effect's name (`__label`, English) in the UI language: the catalog may carry it with or /// without its trailing ellipsis. A plug-in effect's name is shown as it is. fn heading_label(d: &Dialog) -> String { @@ -170,7 +200,8 @@ fn body(app: &mut VectorcraftApp, ui: &mut egui::Ui, d: &mut Dialog) -> bool { if vectorcraft_effects::is_three_d(&id) { super::revolve::body(app, ui, d); let (cmd, params) = command(d); - form::preview(app, ui, d, &d.str("__label"), cmd, params); + form::preview(app, ui, d, &d.str("__label"), cmd, params.clone()); + three_d_refusal(app, ui, d, cmd, params); return false; } // Plug-in effects get fields from their parameter schema; colour adjustments their sliders. diff --git a/crates/ui-egui/src/dialogs/form.rs b/crates/ui-egui/src/dialogs/form.rs index be4f9e80c..c87cfb7e6 100644 --- a/crates/ui-egui/src/dialogs/form.rs +++ b/crates/ui-egui/src/dialogs/form.rs @@ -966,6 +966,19 @@ fn rail_ends_w(ui: &mut egui::Ui, width: f32, (left, right): (&str, &str)) { /// The field where [`preview`] keeps the parameters it last previewed. pub(super) const PREVIEWED: &str = "__previewed"; +/// The field holding why the dialog's command last refused its values, for the dialog to show. +pub(super) const REFUSED: &str = "__refused"; + +/// Keeps why the command refused the dialog's values in [`REFUSED`]: a refused value's message is +/// written for the user, so it goes without the command's id that the error's text puts before it. +pub(super) fn note_refusal(d: &mut Dialog, e: &vectorcraft_engine::EngineError) { + let why = match e { + vectorcraft_engine::EngineError::BadParams { msg, .. } => msg.clone(), + other => other.to_string(), + }; + d.fields.insert(REFUSED.into(), json!(why)); +} + /// The Preview checkbox of a dialog that previews `cmd` on the canvas: while it is on, `params` run /// again on the interaction's snapshot whenever they differ from the last preview (whether a widget /// or `ui.dialog.set` changed them); turning it off rolls back. @@ -1022,8 +1035,14 @@ pub(super) fn preview_now(app: &mut crate::VectorcraftApp, d: &mut Dialog, label if d.fields.get(PREVIEWED) != Some(¶ms) || !app.session.in_interaction() { let _ = app.session.begin_interaction(label); match app.session.preview(cmd, ¶ms) { - Ok(r) => result = Some(r), - Err(e) => app.status(e.to_string()), + Ok(r) => { + d.fields.remove(REFUSED); + result = Some(r); + } + Err(e) => { + note_refusal(d, &e); + app.status(e.to_string()); + } } d.fields.insert(PREVIEWED.into(), params); } diff --git a/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs b/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs index 0249bb199..87f58b12e 100644 --- a/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs +++ b/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs @@ -2,6 +2,7 @@ use super::*; use crate::{dialogs, theme}; use egui::{Event, Modifiers, PointerButton}; +use serde_json::Value; use vectorcraft_effects::revolve_options; use vectorcraft_engine::Session; @@ -284,3 +285,76 @@ fn selected_light_gizmo_edits_only_that_light_and_escape_restores_the_list() { h.release(p + vec2(80.0, 0.0), Modifiers::NONE); assert_eq!(h.app.ui.dialog.as_ref().unwrap().fields["lights"], before); } + +impl Harness { + /// A fresh document where `build` runs and Extrude & Bevel's dialog opens, its Preview `on`. + fn extrude_dialog(build: &[(&str, Value)], on: bool) -> Self { + let mut h = Self::new(); + dialogs::cancel(&mut h.app); + h.app.run("file.new", json!({"width":300,"height":300})).unwrap(); + for (id, p) in build { + h.app.run(id, p.clone()).unwrap(); + } + h.app.run("effect.dialog", json!({"effect": vectorcraft_effects::EXTRUDE})).unwrap(); + h.app.ui.dialog.as_mut().unwrap().fields.insert("preview".into(), json!(on)); + h.settle(); + h + } + + fn ok_is_on(&self) -> bool { + let d = self.app.ui.dialog.as_ref().unwrap(); + dialogs::spec(&d.kind).ok_enabled.is_none_or(|f| f(&self.app, d)) + } + + /// Does the next frame of the dialog draw a text holding `text`? + fn shows(&mut self, text: &str) -> bool { + let input = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(1280.0, 900.0))), ..Default::default() }; + let mut out = self.ctx.run_ui(input, |ui| dialogs::show(&mut self.app, ui.ctx())); + out.textures_delta.clear(); + out.shapes.iter().any(|c| matches!(&c.shape, egui::Shape::Text(t) if t.galley.job.text.contains(text))) + } +} + +fn clipping_group() -> Vec<(&'static str, Value)> { + vec![ + ("shape.ellipse", json!({"x":20,"y":20,"width":100,"height":100})), + ("shape.rectangle", json!({"x":60,"y":60,"width":100,"height":100})), + ("select.all", json!({})), + ("object.clippingMask.make", json!({})), + ] +} + +/// #6140: on an object a 3D effect can't take, the dialog says why in its own words, and OK, which +/// could only fail, is off, where it used to look as if OK did nothing. +#[test] +fn a_refused_3d_effect_says_why_and_turns_ok_off() { + let mut h = Harness::extrude_dialog(&clipping_group(), true); + assert!(!h.ok_is_on()); + assert!(h.shows("requires a path")); + assert!(!h.shows("effect.apply"), "the reason is written without the command's id"); + let enter = Event::Key { key: egui::Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }; + h.frame(vec![enter], Modifiers::NONE); + assert!(h.app.ui.dialog.is_some()); +} + +/// With Preview off from the start, nothing has tried the effect: the dialog still finds out, +/// and trying it leaves no preview open. +#[test] +fn a_refused_3d_effect_is_found_with_preview_off() { + let mut h = Harness::extrude_dialog(&clipping_group(), false); + assert!(!h.ok_is_on()); + assert!(h.shows("requires a path")); + assert!(!h.app.session.in_interaction()); +} + +/// An object the effect can take keeps OK on, with Preview on or off, and shows no reason. +#[test] +fn an_accepted_3d_effect_keeps_ok_on() { + let build = [("shape.rectangle", json!({"x":60,"y":60,"width":100,"height":100}))]; + for on in [true, false] { + let mut h = Harness::extrude_dialog(&build, on); + assert!(h.ok_is_on(), "Preview {on}"); + assert!(!h.shows("requires a path"), "Preview {on}"); + assert_eq!(h.app.session.in_interaction(), on); + } +} From 619cd7aa0c1d3da2dfad5a56a4a27043feec5ee7 Mon Sep 17 00:00:00 2001 From: Sven Lafebre Date: Mon, 9 Nov 2026 20:58:11 +0100 Subject: [PATCH 2/2] A 3D dialog's Preview-off try waits for another interaction to end (#6140) begin_interaction joins an interaction already in progress, so the try replaced that interaction's preview and then cancelled it. --- crates/ui-egui/src/dialogs/effect.rs | 5 +++-- .../ui-egui/src/dialogs/tests_revolve_gizmo.rs | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/dialogs/effect.rs b/crates/ui-egui/src/dialogs/effect.rs index 13e15c413..febbc6488 100644 --- a/crates/ui-egui/src/dialogs/effect.rs +++ b/crates/ui-egui/src/dialogs/effect.rs @@ -38,8 +38,9 @@ const TRIED: &str = "__tried"; fn three_d_refusal(app: &mut VectorcraftApp, ui: &mut egui::Ui, d: &mut Dialog, cmd: &str, params: Value) { if d.bool("preview") { d.fields.remove(TRIED); - } else if d.fields.get(TRIED) != Some(¶ms) { - // A failure to begin leaves no interaction: the preview below then reports it. + } else if !app.session.in_interaction() && d.fields.get(TRIED) != Some(¶ms) { + // Waits for another interaction to end: trying inside it would replace its preview and + // cancel it. A failure to begin leaves no interaction: the preview below then reports it. let _ = app.session.begin_interaction(&d.str("__label")); match app.session.preview(cmd, ¶ms) { Ok(_) => { diff --git a/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs b/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs index 87f58b12e..75aecde46 100644 --- a/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs +++ b/crates/ui-egui/src/dialogs/tests_revolve_gizmo.rs @@ -347,6 +347,24 @@ fn a_refused_3d_effect_is_found_with_preview_off() { assert!(!h.app.session.in_interaction()); } +/// With Preview off, trying the values waits for another interaction to end: trying them during +/// one would replace its preview and cancel it. +#[test] +fn a_3d_effect_with_preview_off_leaves_another_interaction_alone() { + let mut h = Harness::extrude_dialog(&clipping_group(), false); + h.app.session.begin_interaction("Drag").unwrap(); + let d = h.app.ui.dialog.as_mut().unwrap(); + d.fields.remove("__tried"); + d.fields.remove(dialogs::form::REFUSED); + h.settle(); + assert!(h.app.session.in_interaction(), "the other interaction goes on"); + assert!(h.ok_is_on(), "nothing has tried the values yet"); + h.app.session.cancel_interaction().unwrap(); + assert!(h.shows("requires a path")); + assert!(!h.ok_is_on()); + assert!(!h.app.session.in_interaction()); +} + /// An object the effect can take keeps OK on, with Preview on or off, and shows no reason. #[test] fn an_accepted_3d_effect_keeps_ok_on() {