From 55e6c0a5b1530d8e39d8c7d04b9364a646c81728 Mon Sep 17 00:00:00 2001
From: dotCooCoo <RobertLeeLW@gmail.com>
Date: Mon, 9 Nov 2026 19:47:40 -0800
Subject: [PATCH] Revolve: a hidden Revolve's profile is measured before the
 effects after it

effect.setParams checks a Revolve before the edit, so turning a hidden Revolve back on measured
its profile after every geometry effect on the object, those after the Revolve included, and could
refuse a profile that draws. The check now measures the profile after only the geometry effects
before the Revolve, hidden or shown, as drawing does. An object holds at most one Revolve, hidden
ones included, so the check finds the same one that draws.
---
 crates/effects/src/revolve.rs            | 20 +++++++++++++++++++-
 crates/engine/src/cmd/effectcmd/tests.rs | 16 ++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/crates/effects/src/revolve.rs b/crates/effects/src/revolve.rs
index 99a6e053e..a081070a6 100644
--- a/crates/effects/src/revolve.rs
+++ b/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};
diff --git a/crates/engine/src/cmd/effectcmd/tests.rs b/crates/engine/src/cmd/effectcmd/tests.rs
index add1f3b79..6604c99a2 100644
--- a/crates/engine/src/cmd/effectcmd/tests.rs
+++ b/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]