Revolve: the fills and strokes of one object share the paint-copy budget

Each fill or stroke with its own Revolve counted the copies of its paint from zero, so an object with several of them could draw past the 256 MiB budget. They now share one count, and past it the source path draws, as for the members of a group.

effect.apply and effect.setParams on a fill or stroke now check the Revolve in that item's paint, after the item's geometry effects before it, together with the object's other items that carry a Revolve. Before, they checked the object's top fill.

docs/revolve.md marks the axis position, type handling and member paint as VectorCraft's own rules, and docs/mcp.md says how the budget is counted.

crates/effects/src/lib.rs

@@ -87,7 +87,7 @@ pub use revolve::{REVOLVE, has_revolve, revolve_art, revolve_options, validate_r
 pub use solids::{
     CLASSIC_EXTRUDE, EXTRUDE, INFLATE, ROTATE, ROTATION_PRESETS, SHADING_COLORS, SOLIDS, SolidMesh, THREE_D_TYPES, classic_extrude_options,
     classic_shading, extrude_options, has_solid, inflate_options, is_three_d, solid_art, three_d_meshes, three_d_meshes_of, three_d_params_as,
-    validate_solid, validate_three_d, validate_three_d_params, view_options,
+    validate_solid, validate_three_d, validate_three_d_at, validate_three_d_params, view_options,
 };
 pub use stylize::{ScribbleSetting, scribble_setting_id, scribble_settings};
 pub use svg_filter::SVG_FILTER;

crates/effects/src/revolve.rs

@@ -93,6 +93,38 @@ pub fn validate_revolve(n: &Node, p: &Value) -> Result<(), String> {
     check_surface(&path, b, revolve_options(p), surface_item(&paints), &mut 0)
 }
 
+/// [`validate_revolve`] for a Revolve with parameters `p` at index `index` of the effects of
+/// appearance item `target` of `n`. The surfaces of every fill and stroke that carries a Revolve,
+/// `target`'s included, share one budget of paint copies, as in [`art`]. A visible object-level
+/// Revolve draws in place of those surfaces.
+pub(crate) fn validate_item_revolve(n: &Node, p: &Value, target: usize, index: usize) -> Result<(), String> {
+    if is_container(n) || visible(&n.appearance.effects) || target >= n.appearance.items.len() {
+        return validate_revolve(n, p);
+    }
+    validate_revolve_params(p)?;
+    const EMPTY: &str = "Revolve requires a shape with at least two anchors";
+    let (path, _) = object_profile(n, "Revolve")?;
+    if !has_two_anchors(&path) {
+        return Err(EMPTY.into());
+    }
+    let ctx = GeomContext::of(n);
+    let path = geometry(&path, &n.appearance.effects, &ctx);
+    let mut copies = 0;
+    for (i, item) in n.appearance.items.iter().enumerate() {
+        let effects = item.effects();
+        let at = if i == target { Some(index) } else { effects.iter().position(|e| e.visible && e.id == REVOLVE) };
+        let options = match at.and_then(|at| effects.get(at)) {
+            _ if i == target => revolve_options(p),
+            Some(fx) if crate::paints(item) => revolve_options(&merged_params(REVOLVE, &fx.params)),
+            _ => continue,
+        };
+        let path = geometry(&path, at.and_then(|at| effects.get(..at)).unwrap_or(effects), &ctx.item(item));
+        let b = path.bounds().ok_or(EMPTY)?;
+        check_surface(&path, b, options, crate::paints(item).then_some(item), &mut copies)?;
+    }
+    Ok(())
+}
+
 fn has_two_anchors(path: &PathData) -> bool {
     path.subpaths.iter().any(|sp| sp.anchors.len() >= 2)
 }
@@ -250,12 +282,13 @@ fn surface(
     Some((depths, group))
 }
 
-/// A surface of one object: `shape`'s own, built with options derived from `fx`, about `b`.
-fn object_surface(n: &Node, path: &PathData, fx: &Effect, item: &AppearanceItem, expand: bool) -> Option<Node> {
+/// A surface of one object about the bounds of `path`, built with options derived from `fx`.
+/// `copies` is the count [`surface`] adds to.
+fn object_surface(n: &Node, path: &PathData, fx: &Effect, item: &AppearanceItem, expand: bool, copies: &mut usize) -> Option<Node> {
     let b = path.bounds()?;
     let (paint, opacity, blend) = paint_of(item);
     let options = revolve_options(&merged_params(REVOLVE, &fx.params));
-    surface(n, path, b, options, fx, paint, opacity, blend, expand, &mut 0).map(|(_, art)| art)
+    surface(n, path, b, options, fx, paint, opacity, blend, expand, copies).map(|(_, art)| art)
 }
 
 fn shading_paint(g: vectorcraft_three_d::ShadeGradient, color: impl Fn(f64) -> Color, opacity: impl Fn(f64) -> f32) -> Paint {
@@ -357,6 +390,7 @@ fn art(n: &Node, expand: bool) -> Option<Node> {
     let (path, _) = profile(n)?;
     let ctx = GeomContext::of(n);
     let mut children = vec![];
+    let mut copies = 0;
     let mut remaining = n.appearance.effects.clone();
     if let Some(index) = n.appearance.effects.iter().position(|e| e.visible && e.id == REVOLVE) {
         let fx = n.appearance.effects.get(index)?;
@@ -365,7 +399,7 @@ fn art(n: &Node, expand: bool) -> Option<Node> {
         let paints = solid_paints(n);
         let item = surface_item(&paints)?;
         let expand = expand && !n.appearance.effects.iter().skip(index + 1).any(|e| e.visible && is_geometry(&e.id));
-        children.push(Arc::new(object_surface(n, &path, fx, item, expand)?));
+        children.push(Arc::new(object_surface(n, &path, fx, item, expand, &mut copies)?));
         // Later geometry acts on the projected art as a whole through container evaluation.
         remaining = n
             .appearance
@@ -383,7 +417,7 @@ fn art(n: &Node, expand: bool) -> Option<Node> {
                 let fx = item.effects().get(index)?;
                 let p = geometry(&path, item.effects().get(..index)?, &ctx.item(item));
                 let expand = expand && !item.effects().iter().skip(index + 1).any(|e| e.visible && is_geometry(&e.id));
-                let mut art = object_surface(n, &p, fx, item, expand)?;
+                let mut art = object_surface(n, &p, fx, item, expand, &mut copies)?;
                 art.appearance.effects = item.effects().iter().skip(index + 1).filter(|e| e.id != REVOLVE).cloned().collect();
                 children.push(Arc::new(art));
             } else {
@@ -721,6 +755,55 @@ mod tests {
         assert!(validate_revolve(&object, &json!({})).is_err(), "one object past the budget");
     }
 
+    #[test]
+    fn the_fills_of_one_object_share_the_budget_of_paint_copies() {
+        use vectorcraft_doc::color::{Gradient, GradientKind, GradientPaint, GradientStop};
+        let source = profile_node();
+        let NodeKind::Path { path: profile, .. } = &source.kind else { panic!("a path") };
+        let faces = vectorcraft_three_d::revolve(profile, profile.bounds().unwrap(), revolve_options(&json!({}))).unwrap().len();
+        let stops = MAX_PAINT_COPIES * 6 / 10 / (faces * 4) / std::mem::size_of::<GradientStop>();
+        let fill = |stops: usize, revolved: bool| {
+            let stops = (0..stops).map(|i| GradientStop::new(i as f32 / stops as f32, Color::BLACK)).collect();
+            let paint = Paint::Gradient(Box::new(GradientPaint::new(Gradient::new(GradientKind::Linear, stops))));
+            let mut item = AppearanceItem::Fill(FillLayer::new(paint));
+            if revolved {
+                item.effects_mut().push(fx());
+            }
+            item
+        };
+        let with = |items: Vec<AppearanceItem>| {
+            let mut n = source.clone();
+            n.appearance = Appearance { items, ..Default::default() }.into();
+            n
+        };
+        let one = with(vec![fill(stops, true)]);
+        assert!(revolve_art(&one).is_some());
+        assert_eq!(validate_item_revolve(&one, &json!({}), 0, 0), Ok(()));
+        let two = with(vec![fill(stops, true), fill(stops, true)]);
+        assert_eq!(revolve_art(&two), None, "past the budget the source draws");
+        let refused = validate_item_revolve(&two, &json!({}), 1, 0).unwrap_err();
+        assert!(refused.contains("simplify the gradient"), "{refused}");
+        let second = with(vec![fill(stops, true), fill(stops, false)]);
+        assert!(validate_item_revolve(&second, &json!({}), 1, 0).is_err(), "the item the effect goes on counts");
+        assert_eq!(validate_item_revolve(&second, &json!({}), 0, 0), Ok(()), "an item without a Revolve does not");
+        let below = with(vec![fill(stops * 2, false), fill(1, false)]);
+        assert!(validate_item_revolve(&below, &json!({}), 0, 0).is_err(), "the item's own paint, not the top fill");
+        assert_eq!(validate_item_revolve(&below, &json!({}), 1, 0), Ok(()));
+    }
+
+    #[test]
+    fn an_items_revolve_is_validated_after_only_the_geometry_effects_before_it() {
+        let roughen = crate::new_effect("distort.roughen", &json!({"size": 1, "detail": 100})).unwrap();
+        let mut object = (*star(1)).clone();
+        let mut hidden = fx();
+        hidden.visible = false;
+        let item = object.appearance.items.first_mut().unwrap();
+        item.effects_mut().push(hidden);
+        item.effects_mut().push(roughen);
+        assert_eq!(validate_item_revolve(&object, &json!({}), 0, 0), Ok(()), "the roughen after it is not swept");
+        assert!(validate_item_revolve(&object, &json!({}), 0, 2).is_err(), "a Revolve added after the roughen");
+    }
+
     #[test]
     fn transparency_unknown_paints_and_later_distortion_keep_complete_surfaces() {
         let mut n = profile_node();

crates/effects/src/solids.rs

@@ -298,6 +298,17 @@ pub fn validate_three_d(n: &Node, id: &str, p: &Value) -> Result<(), String> {
     if id == crate::REVOLVE { crate::validate_revolve(n, p) } else { validate_solid(n, id, p) }
 }
 
+/// [`validate_three_d`] for an effect at `(item, index)`, index `index` of the effects of
+/// appearance item `item` of `n` (the effects' length for a new effect), or on the object itself
+/// when `at` is `None`. A Revolve on a fill or stroke is checked in that item's paint, after the
+/// item's geometry effects before it, together with the object's other items that carry a Revolve.
+pub fn validate_three_d_at(n: &Node, id: &str, p: &Value, at: Option<(usize, usize)>) -> Result<(), String> {
+    match at {
+        Some((item, index)) if id == crate::REVOLVE => crate::revolve::validate_item_revolve(n, p, item, index),
+        _ => validate_three_d(n, id, p),
+    }
+}
+
 /// Are 3D effect `id`'s parameter values `p` valid? The checks of [`validate_three_d`] that do not
 /// build the art, for an edit that hides the effect: hidden, it draws nothing, so its object does
 /// not need to fit.

crates/engine/src/cmd/effectcmd.rs

@@ -280,9 +280,13 @@ pub(crate) fn apply(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let item = item_target(s, p, C)?;
     if effects::is_three_d(id) {
-        for target in appearance_targets(s, p)? {
+        let targets = appearance_targets(s, p)?;
+        for (target, at) in targets.iter().copied().zip(item.per_target(s, &targets)) {
             let n = s.doc()?.doc.node(target).ok_or(EngineError::NoNode(target))?;
-            effects::validate_three_d(n, id, &effect.params).map_err(|e| bad(C, e))?;
+            // A new effect goes after the item's others.
+            let on = at.and_then(|at| at.effects_item(&n.appearance, C).ok()).flatten();
+            let on = on.and_then(|i| Some((i, n.appearance.items.get(i)?.effects().len())));
+            effects::validate_three_d_at(n, id, &effect.params, on).map_err(|e| bad(C, e))?;
             if id == effects::REVOLVE && effects::has_revolve(n) {
                 return Err(bad(C, "A profile supports one Revolve effect. Edit the existing effect's options instead."));
             }
@@ -589,7 +593,7 @@ fn set_params(s: &mut Session, p: &Value) -> Result<Value> {
                 cur.extend(new);
             }
             if visible != Some(false) {
-                effects::validate_three_d(n, to, &prospective).map_err(|e| bad(C, e))?;
+                effects::validate_three_d_at(n, to, &prospective, target.map(|item| (item, index))).map_err(|e| bad(C, e))?;
             } else {
                 // Hidden, the effect draws nothing, so its object need not fit. The values given are
                 // checked, and after a change of 3D type so is every value it keeps.

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

@@ -260,6 +260,31 @@ fn revolve_refuses_type_and_groups_without_a_shape() {
     assert!(node(&s, g).appearance.effects.is_empty());
 }
 
+/// A Revolve on one fill is checked in that fill's paint, and `effect.setParams` on it as well.
+#[test]
+fn revolve_on_a_fill_is_checked_in_that_fills_paint() {
+    use vectorcraft_color::{Color, Gradient, GradientKind, GradientPaint, GradientStop, Paint};
+    use vectorcraft_doc::{AppearanceItem, FillLayer};
+    let (mut s, id) = session_with_rect();
+    // The paint copies of 100 faces reach the 256 MiB budget: 8 segments stay under, 64 pass it.
+    let count = (256 << 20) / (4 * 100 * std::mem::size_of::<GradientStop>());
+    let stops = (0..count).map(|i| GradientStop::new(i as f32 / count as f32, Color::rgb(1.0, 0.0, 0.0))).collect();
+    let ramp = Paint::Gradient(Box::new(GradientPaint::new(Gradient::new(GradientKind::Linear, stops))));
+    s.edit("fills", |d, _| {
+        let n = d.node_mut(id).unwrap();
+        n.appearance.items =
+            vec![AppearanceItem::Fill(FillLayer::new(ramp)), AppearanceItem::Fill(FillLayer::new(Paint::solid(Color::rgb(0.0, 0.0, 1.0))))];
+        Ok(())
+    })
+    .unwrap();
+    let err = s.execute("effect.apply", &json!({"effect": "threeD.revolve", "ids": [id.0], "item": 0})).unwrap_err().to_string();
+    assert!(err.contains("simplify the gradient"), "{err}");
+    assert!(node(&s, id).appearance.items[0].effects().is_empty());
+    s.execute("effect.apply", &json!({"effect": "threeD.revolve", "ids": [id.0], "item": 0, "params": {"segments": 8}})).unwrap();
+    let err = s.execute("effect.setParams", &json!({"index": 0, "ids": [id.0], "item": 0, "params": {"segments": 128}})).unwrap_err().to_string();
+    assert!(err.contains("simplify the gradient"), "{err}");
+}
+
 /// 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]

crates/three-d/src/mesh/tests.rs

@@ -297,13 +297,15 @@ fn the_wall_of_a_counter_faces_into_the_hole() {
     };
     for (clockwise, rule) in [(true, FillRule::NonZero), (true, FillRule::EvenOdd), (false, FillRule::EvenOdd)] {
         let m = ring(clockwise, rule);
-        assert!(wall_directions(&m, 110.0).iter().all(|s| *s > 0.0), "outer wall, {clockwise} {rule:?}");
+        let outer = wall_directions(&m, 110.0);
+        assert!(!outer.is_empty() && outer.iter().all(|s| *s > 0.0), "outer wall, {clockwise} {rule:?}");
         let hole = wall_directions(&m, 90.0);
         assert!(!hole.is_empty() && hole.iter().all(|s| *s < 0.0), "the hole's far wall faces the axis, {clockwise} {rule:?}");
     }
     // Wound the same way under the nonzero rule, the inner square is not a hole.
     let m = ring(false, FillRule::NonZero);
-    assert!(wall_directions(&m, 90.0).iter().all(|s| *s > 0.0));
+    let inner = wall_directions(&m, 90.0);
+    assert!(!inner.is_empty() && inner.iter().all(|s| *s > 0.0));
 }
 
 #[test]

docs/mcp.md

@@ -2352,7 +2352,7 @@ compound path, type, or a group or layer of them. `effect.dialog` opens compact
 `effect.expandAppearance` bakes it into shaded vector faces. Native saves keep the editable source;
 SVG/PDF exports bake the same faces the canvas draws. Only one Revolve is supported per profile.
 Type sweeps as its glyph outlines and stays type. On a group or layer, every visible member sweeps about one axis on the chosen edge of the members' bounds together, in its own paint, and the members stay editable. A group holding a clipping group, an image, a symbol instance or another object that can't be swept is refused, and so is type with no outlines or a group with no shape in it.
-A surface takes at most 16,384 profile points and 163,840 faces, the members of a group together, counted after any geometry effects before the Revolve. A straight segment takes one point, a curve takes at least four, and each contour takes one. Each curved segment takes 16 samples; on a surface over those limits every curve takes as many as fit, down to 4, and the call is refused below that with "reduce Segments or simplify the profile". The call is also refused with "reduce Segments or simplify the gradient" when the faces' copies of the paint would take more than 256 MiB. Mesh export refuses a profile as too complex when its closed contours nest so deeply that finding the holes would take more than 16,000,000 path element tests.
+A surface takes at most 16,384 profile points and 163,840 faces, the members of a group together, counted after any geometry effects before the Revolve. A straight segment takes one point, a curve takes at least four, and each contour takes one. Each curved segment takes 16 samples; on a surface over those limits every curve takes as many as fit, down to 4, and the call is refused below that with "reduce Segments or simplify the profile". The call is also refused with "reduce Segments or simplify the gradient" when the faces' copies of the paint would take more than 256 MiB, counted together for the members of a group and for the fills and strokes of one object that carry a Revolve. Mesh export refuses a profile as too complex when its closed contours nest so deeply that finding the holes would take more than 16,000,000 path element tests.
 
 Parameters: `angle` (degrees, 0..360, default 360), `offset` (points, 0..100000, default 0),
 `edge` (`"left"` or `"right"`), `rotationX/Y/Z` (degrees, defaults 0/0/0), `perspectiveAngle`

docs/revolve.md

@@ -3,9 +3,9 @@
 This initial implementation follows the scope agreed in [issue #605](https://github.com/storytold/vectorcraft/issues/605#issuecomment-6070543567).
 
 Select a path, compound path, type, or a group or layer of them, then choose **Effect > 3D and Materials > Revolve**.
-Type stays editable and sweeps as its glyph outlines. On a group or layer, every visible member sweeps about one axis
-on the chosen edge of the members' bounds together, in its own paint, as [the help page](https://helpx.adobe.com/illustrator/desktop/special-effects-styles/create-3d-graphics/create-3d-objects.html)
-describes for a group.
+On a group or layer, every visible member sweeps about one axis, as [the help page](https://helpx.adobe.com/illustrator/desktop/special-effects-styles/create-3d-graphics/create-3d-objects.html)
+describes for a group. The following rules are VectorCraft's own: type stays editable and sweeps as its glyph outlines,
+the shared axis lies on the chosen edge of the members' bounds together, and each member keeps its own paint.
 The profile's horizontal distance from a vertical axis is its radius. **Axis side** chooses the left or right
 edge of the profile's bounds; **Offset** moves the axis outward. For a cylinder made from a vertical
 line, give it a nonzero offset. A vertical line at zero offset lies on the axis and has no surface.