Merge pull request #6346 from slafebre/fix/6140-extrude-refusal-reason

3D effect dialogs say why a selection is refused and turn OK off (#6140)

crates/ui-egui/src/dialogs/effect.rs

@@ -24,9 +24,40 @@ 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 !app.session.in_interaction() && d.fields.get(TRIED) != Some(&params) {
+        // 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, &params) {
+            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 +201,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.

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(&params) || !app.session.in_interaction() {
         let _ = app.session.begin_interaction(label);
         match app.session.preview(cmd, &params) {
-            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);
     }

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,94 @@ 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());
+}
+
+/// 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() {
+    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);
+    }
+}