Merge pull request #6390 from dotCooCoo/fix/hidden-revolve-profile

Revolve: a hidden Revolve's profile is measured before the effects after it

crates/effects/src/revolve.rs

@@ -70,7 +70,8 @@ pub fn revolve_options(p: &Value) -> Revolve {
 pub fn validate_revolve(n: &Node, p: &Value) -> Result<(), String> {
     validate_revolve_params(p)?;
     let effects = &n.appearance.effects;
-    let before = effects.iter().position(|e| e.visible && e.id == REVOLVE).and_then(|i| effects.get(..i)).unwrap_or(effects);
+    // The profile is measured before the object's one Revolve, hidden or shown.
+    let before = effects.iter().position(|e| e.id == REVOLVE).and_then(|i| effects.get(..i)).unwrap_or(effects);
     if is_container(n) {
         const EMPTY: &str = "Revolve requires a group holding a shape with at least two anchors";
         let mut shapes = members(n, "Revolve")?;
@@ -812,6 +813,23 @@ mod tests {
         assert!(validate_revolve(&object, &json!({})).is_err(), "a reshaped object not yet revolved");
     }
 
+    #[test]
+    fn validation_of_a_hidden_revolve_leaves_out_the_geometry_effects_after_it() {
+        let roughen = crate::new_effect("distort.roughen", &json!({"size": 1, "detail": 100})).unwrap();
+        let mut hidden = crate::new_effect(REVOLVE, &json!({})).unwrap();
+        hidden.visible = false;
+        let mut object = (*star(1)).clone();
+        object.appearance.effects = vec![hidden.clone(), roughen.clone()];
+        assert_eq!(validate_revolve(&object, &json!({})), Ok(()), "an object whose Revolve is shown again");
+        let mut group = group_with(json!({}), vec![star(1)]);
+        group.appearance.effects = vec![hidden, roughen];
+        assert_eq!(validate_revolve(&group, &json!({})), Ok(()), "a group whose Revolve is shown again");
+        for n in [&mut object, &mut group] {
+            n.appearance.effects[0].visible = true;
+            assert!(revolve_art(n).is_some(), "shown, the Revolve draws");
+        }
+    }
+
     #[test]
     fn paint_copies_past_the_budget_are_refused_for_the_whole_group() {
         use vectorcraft_doc::color::{Gradient, GradientKind, GradientPaint, GradientStop};

crates/engine/src/cmd/effectcmd/tests.rs

@@ -285,6 +285,22 @@ fn revolve_on_a_fill_is_checked_in_that_fills_paint() {
     assert!(err.contains("simplify the gradient"), "{err}");
 }
 
+/// A hidden Revolve still counts as the object's one Revolve: a second one is refused, and the
+/// hidden one can be shown again over the geometry effects after it.
+#[test]
+fn a_second_revolve_is_refused_over_a_hidden_one() {
+    let (mut s, id) = session_with_rect();
+    s.execute("effect.apply", &json!({"effect": "threeD.revolve", "ids": [id.0], "item": null})).unwrap();
+    s.execute("effect.setParams", &json!({"index": 0, "ids": [id.0], "item": null, "visible": false})).unwrap();
+    s.execute("effect.apply", &json!({"effect": "distort.roughen", "ids": [id.0], "item": null})).unwrap();
+    let err = s.execute("effect.apply", &json!({"effect": "threeD.revolve", "ids": [id.0], "item": null})).unwrap_err().to_string();
+    assert!(err.contains("one Revolve effect"), "{err}");
+    let ids: Vec<_> = node(&s, id).appearance.effects.iter().map(|e| e.id.clone()).collect();
+    assert_eq!(ids, ["threeD.revolve", "distort.roughen"]);
+    s.execute("effect.setParams", &json!({"index": 0, "ids": [id.0], "item": null, "visible": true})).unwrap();
+    assert!(node(&s, id).appearance.effects[0].visible);
+}
+
 /// Revolve on a group sweeps every member about one axis on the left edge of their bounds, keeps
 /// each member's paint, follows an edited member, expands and undoes as one step.
 #[test]