Merge pull request #7443 from dotCooCoo/fix/wireframe-outline-stroke-width

3D: Expand as Wireframes outlines each face's own stroke under a later Outline Stroke

crates/effects/src/revolve.rs

@@ -8,6 +8,7 @@ use vectorcraft_doc::{Appearance, AppearanceItem, Document, Effect, FillLayer, N
 use vectorcraft_geom::{PathData, PathEl, Point, Rect, Vec2};
 use vectorcraft_three_d::{Edge, Revolve};
 
+use crate::solids::wireframe::WireFace;
 use crate::solids::{
     Profile, Scope, is_container, members, object_profile, past_effect, profile, reshape_members, resolve_for, solid_paints, union_bounds,
 };
@@ -663,10 +664,16 @@ pub(crate) fn revolve_meshes(n: &Node, total: &mut usize, limit: usize) -> Resul
 /// effect on the object, the surface in its top visible fill (else its stroke); with it on fills and strokes, one surface for each of
 /// them, reshaped by the geometry effects after each one's Revolve; on a group or layer, one for
 /// each member, about one shared axis. The swatches and patterns named by a live-effect plug-in
-/// after a fill's or stroke's Revolve are looked up in `doc`.
-pub(crate) fn revolve_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec<PathData>, String> {
-    let faces = |path: &PathData, b: Rect, options: Revolve| -> Result<Vec<PathData>, String> {
-        Ok(vectorcraft_three_d::revolve(path, b, options)?.iter().map(vectorcraft_three_d::Face::path).collect())
+/// after a fill's or stroke's Revolve are looked up in `doc`. Each face carries the width of the
+/// stroke the canvas draws around it ([`surface`]).
+pub(crate) fn revolve_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec<WireFace>, String> {
+    let faces = |path: &PathData, b: Rect, fx: &Effect, options: Revolve| -> Result<Vec<WireFace>, String> {
+        if fx.id == CLASSIC_REVOLVE {
+            let p = merged_params(&fx.id, &fx.params);
+            let facets = vectorcraft_three_d::revolve_facets(path, b, options)?;
+            return Ok(facets.iter().map(|f| WireFace { path: f.path(), stroke: crate::solids::classic::stroke_width(f, &p) }).collect());
+        }
+        Ok(vectorcraft_three_d::revolve(path, b, options)?.iter().map(|f| WireFace { path: f.path(), stroke: SEAM_WIDTH }).collect())
     };
     if is_container(n) {
         let index = n
@@ -682,7 +689,7 @@ pub(crate) fn revolve_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec
         let options = shared_options(&shapes, fx_options(fx))?;
         let mut out = vec![];
         for s in shapes.iter().filter(|s| surface_item(&s.items).is_some()) {
-            out.extend(faces(&s.path, b, options)?);
+            out.extend(faces(&s.path, b, fx, options)?);
         }
         return Ok(out);
     }
@@ -695,7 +702,7 @@ pub(crate) fn revolve_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec
             return Err("the revolved object has no fill or stroke".into());
         }
         let b = path.bounds().ok_or("Revolve requires a shape with at least two anchors")?;
-        return faces(&path, b, fx_options(fx));
+        return faces(&path, b, fx, fx_options(fx));
     }
     let path = geometry(&path, &n.appearance.effects, &ctx);
     let mut out = vec![];
@@ -705,7 +712,7 @@ pub(crate) fn revolve_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec
         {
             let p = geometry(&path, item.effects().get(..index).unwrap_or_default(), &ctx.item(item));
             let b = p.bounds().ok_or("Revolve requires a shape with at least two anchors")?;
-            let surface = faces(&p, b, fx_options(fx))?;
+            let surface = faces(&p, b, fx, fx_options(fx))?;
             out.extend(crate::solids::wireframe::reshaped(surface, item.effects().get(index + 1..).unwrap_or_default(), &GeomContext { doc, ..ctx }));
         }
     }

crates/effects/src/solids.rs

@@ -575,10 +575,8 @@ fn surface(n: &Node, facets: Vec<Facet>, paint: &Paint, opacity: f32, blend: Ble
         .into_iter()
         .map(|face| {
             // Adjacent facets, antialiased on their own, would leave hairline seams: a narrow
-            // stroke of the same paint closes them. No wider than the facet is thick, though: seen
-            // at a grazing angle a facet can be thinner than the stroke, and its stroke would then
-            // paint over its neighbours.
-            let seam = (face.thickness() / 2.0).clamp(0.05, 0.75);
+            // stroke of the same paint closes them ([`seam_width`]).
+            let seam = seam_width(&face);
             if let Some(g) = face.rgb_gradient
                 && (paint.color().is_some() || matches!(paint, Paint::Gradient(_)))
             {
@@ -608,6 +606,13 @@ fn surface(n: &Node, facets: Vec<Facet>, paint: &Paint, opacity: f32, blend: Ble
     group
 }
 
+/// The width of the stroke of a facet's own paint that closes the seams between `face` and its
+/// neighbors. No wider than the facet is thick: seen at a grazing angle a facet can be thinner than
+/// the stroke, and its stroke would then paint over its neighbors.
+pub(crate) fn seam_width(face: &Facet) -> f64 {
+    (face.thickness() / 2.0).clamp(0.05, 0.75)
+}
+
 /// Is a surface of `facets` in `paint` one composite of all its facets when [`surface`] draws it
 /// whole: a solid color under colored lights with smooth shading?
 fn composites_facets(facets: &[Facet], paint: &Paint, colored: bool) -> bool {

crates/effects/src/solids/classic.rs

@@ -192,6 +192,12 @@ fn shaded(c: Color, tone: f64, spec: f64, shade: Option<Color>) -> Color {
     Color::rgb(r, g, b)
 }
 
+/// The width of the stroke [`surface`] draws around `face` with parameters `p`: a wire edge on
+/// Wireframe, else the seam between faces.
+pub(crate) fn stroke_width(face: &Facet, p: &Value) -> f64 {
+    if classic_shading(p).surface == ClassicSurface::Wireframe { WIRE } else { super::seam_width(face) }
+}
+
 /// One solid's faces in paint `paint`, as a group with the paint's opacity and blend mode, one child
 /// for each facet: the edges of the faces on Wireframe, else each face filled in its paint shaded by
 /// its tone and highlight. A gradient with transparent stops is shaded opaque and then masked by
@@ -213,7 +219,7 @@ pub(crate) fn surface(n: &Node, facets: Vec<Facet>, source: &Paint, opacity: f32
             }
             let (tone, spec) = shading.tone(face.normal, &lights);
             // A narrow stroke of the face's own paint closes the hairline seams between faces.
-            let seam = (face.thickness() / 2.0).clamp(0.05, 0.75);
+            let seam = super::seam_width(&face);
             let items = match paint.color() {
                 // Unshaded (No Shading, no shading color, or a face in full light) with no
                 // highlight: the paint itself, its color space and swatch kept.

crates/effects/src/solids/wireframe.rs

@@ -4,13 +4,13 @@ use std::collections::{HashMap, HashSet, VecDeque};
 use std::sync::Arc;
 
 use vectorcraft_doc::color::{Color, Paint};
-use vectorcraft_doc::{Appearance, Document, Effect, Node, NodeId};
+use vectorcraft_doc::{Appearance, AppearanceItem, Document, Effect, FillLayer, Node, NodeId};
 use vectorcraft_geom::{Anchor, CubicBez, PathData, Point, SubPath};
 use vectorcraft_three_d::Facet;
 
 use super::{
-    Profile, ROTATE, built_solids, facets, geometry, has_solid, is_container, is_plane, is_three_d, label_of, members, object_profile, region,
-    reshape_members, resolve, resolve_for, solid, solid_paint_sources, solid_parts, union_bounds,
+    CLASSIC_EXTRUDE, CLASSIC_ROTATE, Profile, ROTATE, built_solids, facets, geometry, has_solid, is_container, is_plane, is_three_d, label_of,
+    members, object_profile, region, reshape_members, resolve, resolve_for, solid, solid_paint_sources, solid_parts, union_bounds,
 };
 use crate::{GeomContext, is_geometry};
 
@@ -54,7 +54,7 @@ pub fn three_d_wireframe_in(doc: Option<&Document>, n: &Node) -> Result<Vec<Path
     // cuts it ([`crate::revolve_art_in`]).
     let n = &*if revolve { resolve_for(doc, n, label, false)? } else { resolve(doc, n, label)? };
     let faces = if revolve { crate::revolve::revolve_face_paths(doc, n)? } else { solid_face_paths(doc, n)? };
-    let faces = after_geometry(doc, n, faces, revolve);
+    let faces: Vec<PathData> = after_geometry(doc, n, faces, revolve).into_iter().map(|f| f.path).collect();
     let wire = edges(&faces)?;
     if wire.is_empty() {
         return Err(format!("{label} draws no faces for this object"));
@@ -65,29 +65,46 @@ pub fn three_d_wireframe_in(doc: Option<&Document>, n: &Node) -> Result<Vec<Path
 /// `faces` reshaped by the visible geometry effects after the 3D effect of `n`'s own that drew them
 /// ([`reshaped`]): its first visible Revolve when `revolve` is set, else its first visible Extrude &
 /// Bevel, Inflate or Rotate.
-fn after_geometry(doc: Option<&Document>, n: &Node, faces: Vec<PathData>, revolve: bool) -> Vec<PathData> {
+fn after_geometry(doc: Option<&Document>, n: &Node, faces: Vec<WireFace>, revolve: bool) -> Vec<WireFace> {
     let fx = &n.appearance.effects;
     let drawn = if revolve { fx.iter().position(|e| e.visible && crate::revolve::is_revolve(&e.id)) } else { solid(fx).map(|(i, _)| i) };
     let Some(index) = drawn else { return faces };
     reshaped(faces, fx.get(index + 1..).unwrap_or_default(), &GeomContext { doc, ..GeomContext::of(n) })
 }
 
+/// One face of a drawn 3D surface: its outline and the width of the stroke the canvas draws around
+/// it (the seam between faces, or a wire edge), which a later Outline Stroke outlines.
+pub(crate) struct WireFace {
+    pub(crate) path: PathData,
+    pub(crate) stroke: f64,
+}
+
 /// `faces` reshaped by the visible geometry effects in `later` (the effects after a 3D effect), as
 /// they reshape the faces of the drawn surface ([`crate::reshape::reshape_leaves`]): each effect
 /// reshapes each face on its own, about the bounds all the faces have before it, and the faces keep
-/// to one budget of anchors together. `ctx` gives the object's id, the seed of a random Transform,
-/// and the document.
-pub(crate) fn reshaped(faces: Vec<PathData>, later: &[Effect], ctx: &GeomContext) -> Vec<PathData> {
+/// to one budget of anchors together. Each face is filled and carries its round-joined stroke, as on
+/// the canvas, so an Outline Stroke outlines that stroke. `ctx` gives the object's id, the seed of a
+/// random Transform, and the document.
+pub(crate) fn reshaped(faces: Vec<WireFace>, later: &[Effect], ctx: &GeomContext) -> Vec<WireFace> {
     let after: Vec<&Effect> = later.iter().filter(|e| e.visible && is_geometry(&e.id)).collect();
     if after.is_empty() {
         return faces;
     }
     let id = NodeId(ctx.seed);
-    let fill = Appearance::basic(Paint::solid(Color::BLACK), Paint::None, 0.0);
-    let mut art = Node::group(id, faces.into_iter().map(|f| Arc::new(Node::path(id, f, fill.clone()))).collect());
+    let paint = Paint::solid(Color::BLACK);
+    let face = |f: WireFace| {
+        let items =
+            vec![AppearanceItem::Fill(FillLayer::new(paint.clone())), AppearanceItem::Stroke(crate::lighting::seam_stroke(paint.clone(), f.stroke))];
+        Arc::new(Node::path(id, f.path, Appearance { items, ..Default::default() }))
+    };
+    let mut art = Node::group(id, faces.into_iter().map(face).collect());
     crate::reshape::reshape_leaves(&mut art, &after, later, ctx.doc);
     let mut out = vec![];
-    art.walk(&mut |c| out.extend(c.path_data().cloned()));
+    art.walk(&mut |c| {
+        if let Some(path) = c.path_data() {
+            out.push(WireFace { path: path.clone(), stroke: c.appearance.stroke().map_or(0.0, |s| s.width) });
+        }
+    });
     out
 }
 
@@ -95,7 +112,7 @@ pub(crate) fn reshaped(faces: Vec<PathData>, later: &[Effect], ctx: &GeomContext
 /// on the object, its solids; with it on fills and strokes, the solid of each; on a group or layer,
 /// the solids of its members, about one shared axis. The swatches and patterns named by a live-effect
 /// plug-in after a fill's or stroke's 3D effect are looked up in `doc`.
-fn solid_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec<PathData>, String> {
+fn solid_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec<WireFace>, String> {
     if is_container(n) {
         let (index, fx) = solid(&n.appearance.effects).ok_or("a 3D effect on a group's fill or stroke draws nothing")?;
         let mut shapes = members(n, label_of(&fx.id))?;
@@ -126,17 +143,20 @@ fn solid_face_paths(doc: Option<&Document>, n: &Node) -> Result<Vec<PathData>, S
 /// Rotate (Classic) turn every fill and stroke; Extrude & Bevel and Inflate build a solid from each shape's
 /// top fill and from the outline of each stroke. A stroke whose solid is past the limits is left out,
 /// as it is left out of the drawn art; past the limits of the object's solids together, the result
-/// is an error, as the canvas draws the source art.
-fn facet_paths(fx: &Effect, shapes: &[Profile]) -> Result<Vec<PathData>, String> {
+/// is an error, as the canvas draws the source art. Each facet carries the width of the stroke the
+/// canvas draws around it.
+fn facet_paths(fx: &Effect, shapes: &[Profile]) -> Result<Vec<WireFace>, String> {
     let label = label_of(&fx.id);
     let b = union_bounds(shapes).ok_or_else(|| format!("{label} requires a shape with at least two anchors"))?;
     let p = crate::merged_params(&fx.id, &fx.params);
+    let classic = fx.id == CLASSIC_EXTRUDE || fx.id == CLASSIC_ROTATE;
+    let face = |f: &Facet| WireFace { path: f.path(), stroke: if classic { super::classic::stroke_width(f, &p) } else { super::seam_width(f) } };
     let mut out = vec![];
     if is_plane(&fx.id) {
         for s in shapes {
             for item in &s.items {
                 let (area, rule) = region(&s.path, s.rule, item);
-                out.extend(facets(ROTATE, &area, rule, b, &p)?.iter().map(Facet::path));
+                out.extend(facets(ROTATE, &area, rule, b, &p)?.iter().map(face));
             }
         }
         return Ok(out);
@@ -144,7 +164,7 @@ fn facet_paths(fx: &Effect, shapes: &[Profile]) -> Result<Vec<PathData>, String>
     let (parts, bounds) = solid_parts(shapes, b);
     // The solids the art draws, each past the limits of the object's solids together as it is.
     for f in built_solids(&fx.id, &p, &parts, bounds, vectorcraft_three_d::MAX_OBJECT_FACES) {
-        out.extend(f?.iter().flatten().map(Facet::path));
+        out.extend(f?.iter().flatten().map(face));
     }
     Ok(out)
 }

crates/effects/src/solids/wireframe/tests.rs

@@ -163,13 +163,43 @@ fn revolve_classic_and_rotate_classic_expand_too() {
 fn later_geometry_effects_reshape_each_face_on_its_own() {
     let n = solid_square(super::super::EXTRUDE, json!({}));
     let fx = [crate::new_effect("path.offsetPath", &json!({"offset": 1.0})).unwrap()];
-    let out = reshaped(vec![square(0.0, 0.0, 10.0), square(10.0, 0.0, 10.0)], &fx, &GeomContext::of(&n));
-    assert_eq!(out.len(), 2, "{out:?}");
-    let (a, b) = (out[0].bounds().unwrap(), out[1].bounds().unwrap());
+    let faces = [square(0.0, 0.0, 10.0), square(10.0, 0.0, 10.0)].map(|path| WireFace { path, stroke: 0.75 });
+    let out = reshaped(faces.into(), &fx, &GeomContext::of(&n));
+    assert_eq!(out.len(), 2);
+    let (a, b) = (out[0].path.bounds().unwrap(), out[1].path.bounds().unwrap());
     assert!((a.x0 + 1.0).abs() < 1e-3 && (a.x1 - 11.0).abs() < 1e-3, "{a:?}");
     assert!((b.x0 - 9.0).abs() < 1e-3 && (b.x1 - 21.0).abs() < 1e-3, "{b:?}");
 }
 
+/// An Outline Stroke after a 3D effect outlines the stroke each face is drawn with (the seam between
+/// faces, or a Classic wire edge), not a plain 1 pt stroke: the wireframe covers the outlined faces
+/// the canvas draws, in the same place.
+#[test]
+fn a_later_outline_stroke_outlines_the_stroke_each_face_is_drawn_with() {
+    let outline = || crate::new_effect("path.outlineStroke", &json!({})).unwrap();
+    let cases = [
+        (super::super::EXTRUDE, json!({})),
+        (super::super::CLASSIC_EXTRUDE, json!({})),
+        (super::super::CLASSIC_EXTRUDE, json!({"surface": "wireframe"})),
+        (super::super::INFLATE, json!({})),
+        (ROTATE, json!({})),
+        (crate::REVOLVE, json!({})),
+        (crate::CLASSIC_REVOLVE, json!({})),
+        (crate::CLASSIC_REVOLVE, json!({"surface": "wireframe"})),
+    ];
+    for (id, params) in cases {
+        let mut n = solid_square(id, params.clone());
+        n.appearance.effects.push(outline());
+        let wire = three_d_wireframe_in(None, &n).unwrap_or_else(|e| panic!("{id} {params}: {e}"));
+        let art = if crate::revolve::is_revolve(id) { crate::revolve_art(&n) } else { super::super::solid_art(&n) }.unwrap();
+        let drawn = crate::group::shape_art(None, &art, None).unwrap();
+        let (w, a) = (bounds(&wire), bounds(&face_paths(&drawn)));
+        for (x, y) in [(w.x0, a.x0), (w.y0, a.y0), (w.x1, a.x1), (w.y1, a.y1)] {
+            assert!((x - y).abs() < 1e-6, "{id} {params}: {w:?} {a:?}");
+        }
+    }
+}
+
 /// A geometry effect after a fill's 3D effect moves that fill's edges, as it moves its drawn surface.
 #[test]
 fn a_geometry_effect_after_a_fill_s_3d_effect_reshapes_its_edges() {