Merge pull request #7030 from dotCooCoo/fix/followup-6960

3D: mapped graphics keep paint order and hide handles on hidden fills

crates/effects/src/solids.rs

@@ -866,7 +866,8 @@ fn masks_surface(paint: &Paint, colored: bool) -> bool {
 }
 
 /// The graphics of the solids `filled` ([`graphic_art`]), each with the index of the surface it is
-/// drawn right after (the last of those solids holding its face) and its face. With `per_member`
+/// drawn right after (the last of those solids holding its face) and its face. Solids in a run
+/// with the same opacity and blending mode share their graphics. With `per_member`
 /// (a member's opacity mask or blending mode applies to its surfaces) each member's solids get
 /// graphics of their own, clipped to that member's caps, so they stay in that member's scopes.
 /// All the groups share one limit on the path elements drawn: once it is reached, later groups
@@ -883,11 +884,12 @@ fn placed_graphics(
     for f in filled {
         // Solids whose fills or strokes differ in opacity or blending mode get graphics of their
         // own, each drawn as its fill or stroke is. A solid whose surface carries an opacity mask
-        // gets its own too, which takes that mask ([`with_graphics`]).
+        // gets its own too, which takes that mask ([`with_graphics`]). A solid joins only the
+        // group of the solid before it, so a graphic is never drawn past a solid painted between.
         let joins = |first: &Filled| {
             (!per_member || first.member == f.member) && first.opacity == f.opacity && first.blend == f.blend && !first.masked && !f.masked
         };
-        match groups.iter_mut().find(|g| g.first().is_some_and(|first| joins(first))) {
+        match groups.last_mut().filter(|g| g.first().is_some_and(|first| joins(first))) {
             Some(g) => g.push(f),
             None => groups.push(vec![f]),
         }

crates/effects/src/solids/tests_graphics.rs

@@ -365,6 +365,41 @@ fn members_with_their_own_blending_get_graphics_of_their_own() {
     assert_eq!(blended, 1);
 }
 
+#[test]
+fn a_graphic_stays_under_a_solid_painted_after_its_own() {
+    let d = document();
+    let member = |id: u64, r: Rect, color: Color, opacity: f32| {
+        let mut n = square(id, r, Paint::solid(color));
+        if let Some(AppearanceItem::Fill(f)) = n.appearance.items.first_mut() {
+            f.opacity = opacity;
+        }
+        Arc::new(n)
+    };
+    let gray = Color::rgb(0.5, 0.5, 0.5);
+    // A and C share their fill's opacity; B, painted between them, has its own and covers part of A.
+    let a = member(1, Rect::new(0.0, 0.0, 60.0, 100.0), gray, 1.0);
+    let b = member(2, Rect::new(40.0, 0.0, 100.0, 100.0), Color::rgb(0.0, 0.0, 1.0), 0.5);
+    let c = member(4, Rect::new(200.0, 0.0, 260.0, 100.0), gray, 1.0);
+    let mut group = Node::group(NodeId(3), vec![a, b, c]);
+    let p = json!({"rotationX": 0.0, "rotationY": 0.0, "rotationZ": 0.0, "shade": false, "graphics": [{"symbol": "Mark", "scale": 3000.0}]});
+    group.appearance.effects.push(crate::new_effect(EXTRUDE, &p).unwrap());
+    let art = solid_art_in(Some(&d), &group).unwrap();
+    // In drawing order: 'G' for a graphic, 'B' for a face of B's solid.
+    let mut order = String::new();
+    art.walk(&mut |n| {
+        if matches!(n.kind, NodeKind::Group { clip: true, .. }) && !graphics_in(n).is_empty() {
+            order.push('G');
+        } else if let Some(AppearanceItem::Fill(f)) = n.appearance.items.first()
+            && f.paint.color().is_some_and(|c| c.to_rgb()[2] > 0.9 && c.to_rgb()[0] < 0.1)
+        {
+            order.push('B');
+        }
+    });
+    // A's graphic is drawn right after A, under B; C's is drawn after B.
+    assert_eq!(graphics_in(&art).len(), 3, "{order}");
+    assert!(order.starts_with('G'), "{order}");
+}
+
 /// [`document`] with symbol "Dense" too: a red polygon of 30,000 sides, straight or `curved`.
 fn dense_document(curved: bool) -> Document {
     let mut d = document();

crates/ui-egui/src/panels/tests_three_d.rs

@@ -612,6 +612,42 @@ fn a_geometry_effect_after_the_3d_effect_hides_the_graphics_handles() {
     handles_frame(&mut app, &ctx, vec![primary(handle, false)]);
 }
 
+#[test]
+fn a_hidden_fill_shows_no_handles_for_its_graphics() {
+    let (mut app, ctx) = materials_app();
+    app.run("threeD.mapGraphic", json!({"symbol": "Graphic"})).unwrap();
+    // The 3D effect, with its graphic, moves onto the square's fill.
+    let fill = {
+        let st = app.session.active_mut().unwrap();
+        let id = st.selection.objects[0];
+        let n = std::sync::Arc::make_mut(&mut st.doc).node_mut(id).unwrap();
+        let fx = n.appearance.effects.remove(0);
+        let fill = n.appearance.items.iter().position(|i| matches!(i, vectorcraft_doc::AppearanceItem::Fill(_))).unwrap();
+        n.appearance.effects_mut(Some(fill)).unwrap().push(fx);
+        fill
+    };
+    let shown = |app: &VectorcraftApp| {
+        let mut shown = false;
+        ctx.run_ui(egui::RawInput::default(), |ui| {
+            crate::panels::set_pstate(ui.ctx(), super::DRAWN, Some(super::root_pass(ui.ctx())));
+            shown = super::graphic_handles(app, ui.ctx()).is_some_and(|(_, _, handles)| !handles.is_empty());
+        })
+        .textures_delta
+        .clear();
+        shown
+    };
+    assert!(shown(&app), "the visible fill's graphic has a handle");
+    {
+        let st = app.session.active_mut().unwrap();
+        let id = st.selection.objects[0];
+        let n = std::sync::Arc::make_mut(&mut st.doc).node_mut(id).unwrap();
+        if let Some(vectorcraft_doc::AppearanceItem::Fill(f)) = n.appearance.items.get_mut(fill) {
+            f.visible = false;
+        }
+    }
+    assert!(!shown(&app), "the hidden fill draws no graphic, so it has no handle");
+}
+
 #[test]
 fn a_press_that_closes_the_context_menu_does_not_grab_a_graphics_handle() {
     let (mut app, ctx) = materials_app();

crates/ui-egui/src/panels/three_d.rs

@@ -324,6 +324,10 @@ fn graphic_handles(app: &VectorcraftApp, ctx: &egui::Context) -> Option<Handles>
     let st = app.session.active()?;
     let n = st.selection.objects.first().and_then(|id| st.doc.node(*id))?;
     let t = find(n)?;
+    // A hidden fill or stroke draws no solid and no graphics, so its graphics get no handles.
+    if t.item.is_some_and(|k| n.appearance.items.get(k).is_none_or(|it| !it.visible())) {
+        return None;
+    }
     // The frame of the effect whose graphics are listed: none while that effect is hidden.
     let frame = vectorcraft_effects::graphic_frame_at(Some(&st.doc), n, t.item, t.index)?;
     // A geometry effect after the 3D effect of any selected object moves or bends its drawn

docs/three-d.md

@@ -326,7 +326,8 @@ These rules are VectorCraft's own:
 
 - A graphic is drawn on the visible face of each of the object's solids (a fill's, and a stroke's outline),
   turned, foreshortened by the perspective and twisted and tapered with the face, and clipped to it. It is shaded
-  with the face's light: each color is multiplied by the light that falls on the face, and drawn with the
+  with the face's light: each color is multiplied by the light that falls on the face, except in pattern tiles,
+  which keep their own colors. It is drawn with the
   opacity and blending mode of the fill or stroke the face is painted with. A graphic on a face turned away from
   the viewer is not drawn.
 - On the back cap, X runs to the right as the back is seen from behind, so the graphic reads the right way round