Merge pull request #2637 from dotCooCoo/fix/art-on-artboard-edge

Artboards: art centered on an artboard's right or bottom edge is on that artboard

crates/doc/src/lib.rs

@@ -786,6 +786,14 @@ fn yes() -> bool {
     true
 }
 
+/// Is point `p` on the artboard at `rect`, its edges included, within a millionth of a point? This
+/// is the rule [`Document::art_on_artboard`] applies to art flush with an edge (#890), where
+/// `Rect::contains` leaves out the right and bottom edges.
+pub fn artboard_holds(rect: Rect, p: Point) -> bool {
+    let r = rect.inflate(1e-6, 1e-6);
+    r.x0 <= p.x && p.x <= r.x1 && r.y0 <= p.y && p.y <= r.y1
+}
+
 /// Listed nodes by parent: the parent's index path (empty for the top-level layers), then the index
 /// of each listed child in it, in order, with the child's place in the list.
 type Places = BTreeMap<Vec<usize>, Vec<(usize, usize)>>;
@@ -1356,9 +1364,24 @@ impl Document {
         let symbols = self.symbol_boxes();
         self.layers.iter().fold(None, |acc, l| vectorcraft_geom::union_opt(acc, l.visual_bounds_with(&symbols)))
     }
-    /// Artboard index containing point `p` (topmost = last).
+    /// Artboard index containing point `p` (topmost = last), its edges included
+    /// ([`Document::artboard_holding`]).
     pub fn artboard_at(&self, p: Point) -> Option<usize> {
-        self.artboards.iter().rposition(|a| a.rect.contains(p))
+        self.artboard_holding(p, true)
+    }
+    /// Index of the artboard that holds point `p`: the first (the last when `topmost`) that holds it
+    /// inside or on its left or top edge, else the first (the last) that holds it by
+    /// [`artboard_holds`]: on its right or bottom edge, or within a millionth of a point outside an
+    /// edge. On an edge two artboards share, the artboard to the right of it or below it holds the
+    /// point, except at the end of the edge that lies on that artboard's bottom or right edge, such
+    /// as the point where both artboards' bottom edges meet; there the first (the last) holds it.
+    pub fn artboard_holding(&self, p: Point, topmost: bool) -> Option<usize> {
+        let pick = |on: &dyn Fn(Rect) -> bool| {
+            let on = |i: &usize| self.artboards.get(*i).is_some_and(|a| on(a.rect));
+            let mut all = 0..self.artboards.len();
+            if topmost { all.rfind(on) } else { all.find(on) }
+        };
+        pick(&|r| r.contains(p)).or_else(|| pick(&|r| artboard_holds(r, p)))
     }
     /// The art that moves with an artboard at `rect`: top-level objects (children of layers and
     /// sublayers) lying entirely inside it, with symbol instances measured by their art. Locked and
@@ -1839,6 +1862,44 @@ mod tests {
         assert_eq!(d.art_on_artboard(Rect::new(0.0, 0.0, 15.0, 15.0), false), vec![id]);
     }
 
+    #[test]
+    fn artboard_at_counts_the_right_and_bottom_edges() {
+        // Artboard 0 is (0, 0) to (100, 100), artboard 1 is to its right and artboard 2 below it.
+        let mut d = Document::new(100.0, 100.0);
+        for rect in [Rect::new(100.0, 0.0, 200.0, 100.0), Rect::new(0.0, 100.0, 100.0, 200.0)] {
+            let mut a = d.artboards[0].clone();
+            a.rect = rect;
+            d.artboards.push(a);
+        }
+        let held = |d: &Document, x: f64, y: f64| (d.artboard_at(Point::new(x, y)), d.artboard_holding(Point::new(x, y), false));
+        // Each artboard's own right and bottom edges, edges with roundoff, and points beyond them.
+        assert_eq!(held(&d, 200.0, 50.0), (Some(1), Some(1)), "right edge");
+        assert_eq!(held(&d, 150.0, 100.0), (Some(1), Some(1)), "bottom edge");
+        assert_eq!(held(&d, 200.0, 100.0), (Some(1), Some(1)), "bottom right corner");
+        assert_eq!(held(&d, 100.0, 150.0), (Some(2), Some(2)), "right edge");
+        assert_eq!(held(&d, 50.0, 200.0 + 1e-9), (Some(2), Some(2)), "roundoff past the bottom edge");
+        assert_eq!(held(&d, -1e-9, 50.0), (Some(0), Some(0)), "roundoff past the left edge");
+        assert_eq!(held(&d, 200.001, 50.0), (None, None));
+        assert_eq!(held(&d, 50.0, 200.001), (None, None));
+        // On an edge two artboards share, the artboard to the right of it or below it holds the
+        // point, in either list order, except at the end of the edge that lies on that artboard's
+        // bottom or right edge.
+        assert_eq!(held(&d, 100.0, 50.0), (Some(1), Some(1)));
+        assert_eq!(held(&d, 50.0, 100.0), (Some(2), Some(2)));
+        // The bottom edges of artboards 0 and 1 meet at (100, 100), which is also artboard 2's top
+        // right corner: the first holds the point, or the last when `topmost`.
+        assert_eq!(held(&d, 100.0, 100.0), (Some(2), Some(0)), "shared corner");
+        d.artboards.swap(0, 1);
+        assert_eq!(held(&d, 100.0, 50.0), (Some(0), Some(0)));
+        // Where two artboards overlap, the topmost holds the point, or the first one when not
+        // `topmost`.
+        let mut inner = d.artboards[1].clone();
+        inner.rect = Rect::new(10.0, 10.0, 90.0, 90.0);
+        d.artboards.push(inner);
+        assert_eq!(held(&d, 50.0, 50.0), (Some(3), Some(1)));
+        assert_eq!(held(&d, 90.0, 50.0), (Some(1), Some(1)), "inside artboard 1, on the right edge of artboard 3");
+    }
+
     #[test]
     fn find_and_paths() {
         let (d, a, b) = doc_with_rects();

crates/engine/src/cmd/menucmds.rs

@@ -982,7 +982,7 @@ fn rearrange_artboards(s: &mut Session, p: &Value) -> Result<Value> {
             .filter(|n| n.rides_with_artboard(locked_and_hidden))
             .filter_map(|n| {
                 let c = n.geometric_bounds_with(&symbols)?.center();
-                let dl = *rects.iter().position(|r| r.contains(c)).and_then(|i| deltas.get(i))?;
+                let dl = *d.artboard_holding(c, false).and_then(|i| deltas.get(i))?;
                 (dl != Vec2::ZERO).then_some((n.id, dl))
             })
             .collect()

crates/engine/src/cmd/place/mod.rs

@@ -448,9 +448,9 @@ fn place(s: &mut Session, p: &Value) -> Result<Value> {
     )
 }
 
-/// The artboard under `at`, else the first.
+/// The artboard under `at` (its edges included, [`Document::artboard_holding`]), else the first.
 fn board_at(d: &Document, at: Option<Point>) -> Option<Rect> {
-    at.and_then(|c| d.artboards.iter().find(|a| a.rect.contains(c))).or(d.artboards.first()).map(|a| a.rect)
+    at.and_then(|c| d.artboard_holding(c, false)).and_then(|i| d.artboards.get(i)).or(d.artboards.first()).map(|a| a.rect)
 }
 
 /// Placed type's frame without `rect` or Replace: the artboard under `at` (else the first) less a

crates/engine/src/cmd/select.rs

@@ -264,7 +264,10 @@ fn all_on_artboard(s: &mut Session, p: &Value) -> Result<Value> {
     let ids: Vec<NodeId> = selectable(&st.doc, st.isolation)
         .into_iter()
         .filter(|id| {
-            st.doc.node(*id).and_then(|n| n.geometric_bounds_with(&symbols)).is_some_and(|b| b.intersect(r).area() > 0.0 || r.contains(b.center()))
+            st.doc
+                .node(*id)
+                .and_then(|n| n.geometric_bounds_with(&symbols))
+                .is_some_and(|b| b.intersect(r).area() > 0.0 || vectorcraft_doc::artboard_holds(r, b.center()))
         })
         .collect();
     s.select(|_, sel| sel.set(ids.iter().copied()))?;

crates/engine/src/tests_clipboard.rs

@@ -585,6 +585,25 @@ fn in_place_pastes_of_art_off_the_artboards_stay_where_it_was() {
     }
 }
 
+/// Art centered on an artboard's right or bottom edge is copied from that artboard: Paste in Place
+/// puts it on the active artboard where it was on that one.
+#[test]
+fn in_place_pastes_of_art_on_an_artboard_edge_go_onto_the_artboard_named() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 200, "height": 100, "artboards": 2})).unwrap();
+    let boards: Vec<Rect> = doc(&s).artboards.iter().map(|a| a.rect).collect();
+    let b = boards[1];
+    // A line along the right edge of artboard 2, then one along its bottom edge.
+    for (x1, y1, x2, y2) in [(b.x1, b.y0 + 10.0, b.x1, b.y0 + 40.0), (b.x0 + 10.0, b.y1, b.x0 + 40.0, b.y1)] {
+        let line = id_of(&s.execute("shape.line", &json!({"x1": x1, "y1": y1, "x2": x2, "y2": y2})).unwrap());
+        copy(&mut s, &[line]);
+        assert_eq!(s.clipboard.source_artboard, Some(b));
+        let ids = ids_of(&s.execute("edit.pasteInPlace", &json!({"artboard": 0})).unwrap());
+        let moved = doc(&s).bounds_of(&ids, false).unwrap().origin() - doc(&s).bounds_of(&[line], false).unwrap().origin();
+        assert_eq!(moved, boards[0].origin() - b.origin());
+    }
+}
+
 #[test]
 fn paste_remembers_layers_restores_the_source_layers() {
     let mut s = session();

crates/engine/src/tests_menucmds.rs

@@ -1300,6 +1300,23 @@ fn select_all_in_pattern_and_mask_editing_selects_the_art_being_edited() {
     }
 }
 
+/// All on Active Artboard takes a line on the artboard's right or bottom edge, as it takes one on
+/// the left or top edge. A line on the edge two artboards share is on both.
+#[test]
+fn select_all_on_artboard_takes_lines_on_its_right_and_bottom_edges() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 100, "height": 50, "artboards": 2, "artboardLayout": {"layout": "row", "spacing": 0}})).unwrap();
+    let mut line = |x1: f64, y1: f64, x2: f64, y2: f64| id_of(&s.execute("shape.line", &json!({"x1": x1, "y1": y1, "x2": x2, "y2": y2})).unwrap());
+    let (bottom, right, shared) = (line(10.0, 50.0, 40.0, 50.0), line(200.0, 10.0, 200.0, 40.0), line(100.0, 10.0, 100.0, 40.0));
+    line(200.5, 10.0, 200.5, 40.0);
+    let mut on = |i: u64| {
+        s.execute("select.allOnArtboard", &json!({"artboard": i})).unwrap();
+        selected(&s)
+    };
+    assert_eq!(on(0), vec![bottom, shared]);
+    assert_eq!(on(1), vec![right, shared]);
+}
+
 #[test]
 fn save_and_recall_selection() {
     let mut s = session();

crates/engine/src/tests_newdoc.rs

@@ -330,6 +330,23 @@ fn rearrange_lays_out_by_layout_and_order() {
     }
 }
 
+/// Art centered on an artboard's right or bottom edge moves with that artboard, and art centered on
+/// the edge two artboards share, away from its ends, moves with the artboard to the right of it.
+#[test]
+fn rearrange_moves_art_centered_on_an_artboard_edge() {
+    let mut s = new_doc(json!({"width": 100, "height": 50, "artboards": 2, "artboardLayout": {"layout": "row", "spacing": 0}}));
+    assert_eq!(boards(&s), [Rect::new(0.0, 0.0, 100.0, 50.0), Rect::new(100.0, 0.0, 200.0, 50.0)]);
+    let mut line = |x1: f64, y1: f64, x2: f64, y2: f64| {
+        NodeId(s.execute("shape.line", &json!({"x1": x1, "y1": y1, "x2": x2, "y2": y2})).unwrap()["id"].as_u64().unwrap())
+    };
+    let (bottom, right, shared) = (line(10.0, 50.0, 40.0, 50.0), line(200.0, 10.0, 200.0, 40.0), line(100.0, 10.0, 100.0, 40.0));
+    // Right to left, artboard 0 moves 100 pt right and artboard 1 moves 100 pt left.
+    s.execute("artboard.rearrange", &json!({"layout": "row", "order": "rightToLeft", "spacing": 0})).unwrap();
+    assert_eq!(boards(&s), [Rect::new(100.0, 0.0, 200.0, 50.0), Rect::new(0.0, 0.0, 100.0, 50.0)]);
+    let x0 = |id: NodeId| s.doc().unwrap().doc.node(id).unwrap().geometric_bounds().unwrap().x0;
+    assert_eq!((x0(bottom), x0(right), x0(shared)), (110.0, 100.0, 0.0));
+}
+
 /// `byColumn: true` is Grid by Column; without `moveArtwork` the art stays; bad values are errors.
 #[test]
 fn rearrange_aliases_and_rejects_bad_values() {

crates/engine/src/tests_textimport.rs

@@ -51,6 +51,22 @@ fn crlf_with_returns_removed_places_one_paragraph_per_block_as_area_type() {
     assert!(close(frame, Rect::new(10.0, 20.0, 110.0, 70.0)), "{frame:?}");
 }
 
+/// Placed at a point on an artboard's right or bottom edge, the frame takes that artboard's size less
+/// a 36 pt margin on each side; on the edge two artboards share, away from its ends, it takes the
+/// size of the artboard to the right of it.
+#[test]
+fn type_placed_on_an_artboard_edge_takes_that_artboards_frame() {
+    let mut s = session();
+    let mut wide = s.doc().unwrap().doc.artboards[0].clone();
+    wide.rect = Rect::new(400.0, 0.0, 600.0, 200.0);
+    std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).artboards.push(wide);
+    for at in [[600.0, 100.0], [500.0, 200.0], [400.0, 100.0]] {
+        let r = place_text(&mut s, b"Edge", json!({ "at": at }));
+        let frame = placed(&s, &r).1;
+        assert!(close(frame, Rect::from_center_size((at[0], at[1]), (128.0, 128.0))), "{at:?}: {frame:?}");
+    }
+}
+
 #[test]
 fn text_files_report_and_queue_with_their_options() {
     let mut s = session();

crates/svg/src/css.rs

@@ -199,7 +199,7 @@ impl CssUnits {
 pub struct CssOptions {
     pub units: CssUnits,
     /// `position: absolute` with `left` and `top` from the top-left corner of the object's
-    /// artboard (the one its centre is on, else the first).
+    /// artboard (the one its centre is on, its edges included, else the first).
     pub position: bool,
     /// `width` and `height`.
     pub dimensions: bool,
@@ -381,7 +381,7 @@ impl Rules<'_> {
         let Some(b) = b else { return p };
         if self.opts.position {
             let c = b.center();
-            let ab = self.doc.artboards.iter().find(|a| a.rect.contains(c)).or_else(|| self.doc.artboards.first());
+            let ab = self.doc.artboard_holding(c, false).and_then(|i| self.doc.artboards.get(i)).or_else(|| self.doc.artboards.first());
             let origin = ab.map_or(Point::ZERO, |a| Point::new(a.rect.x0, a.rect.y0));
             p.push(("position", "absolute".into()));
             p.push(("left", self.len(b.x0 - origin.x)));

crates/svg/tests/css_rules.rs

@@ -70,6 +70,27 @@ fn rectangle_gives_background_border_and_radius() {
     );
 }
 
+/// An object centered on an artboard's right or bottom edge is positioned from that artboard's
+/// top-left corner; on the edge two artboards share, away from its ends, from the one to the right
+/// of it.
+#[test]
+fn position_is_from_the_artboard_whose_edge_holds_the_center() {
+    let mut d = doc();
+    // Artboard 2 is (500, 0) to (900, 300); artboard 3 starts at its right edge.
+    for rect in [Rect::new(500.0, 0.0, 900.0, 300.0), Rect::new(900.0, 0.0, 1000.0, 300.0)] {
+        let mut a = d.artboards[0].clone();
+        a.rect = rect;
+        d.artboards.push(a);
+    }
+    let opts = CssOptions { position: true, ..CssOptions::default() };
+    for ((x, y), want) in [((980.0, 100.0), ("80px", "100px")), ((600.0, 280.0), ("100px", "280px")), ((880.0, 100.0), ("-20px", "100px"))] {
+        let n = rect(&mut d, (x, y, 40.0, 40.0), 0.0);
+        let id = add(&mut d, n);
+        let r = one(&d, id, &opts);
+        assert_eq!((prop(&r, "left"), prop(&r, "top")), (Some(want.0), Some(want.1)), "at ({x}, {y})");
+    }
+}
+
 #[test]
 fn live_corners_give_per_corner_radii_and_only_round_ones() {
     let mut d = doc();

crates/tools/src/xform/artboard.rs

@@ -484,7 +484,7 @@ impl Tool for ArtboardTool {
             }
             .turned(cx.view_angle());
         }
-        if cx.doc.artboards.iter().any(|a| a.rect.contains(p)) { Cursor::Move } else { Cursor::Crosshair }
+        if cx.doc.artboard_at(p).is_some() { Cursor::Move } else { Cursor::Crosshair }
     }
 
     fn options(&self) -> Value {
@@ -729,6 +729,22 @@ mod tests {
         assert!((v["height"].as_f64().unwrap() - 113.0).abs() < 1e-9);
     }
 
+    /// Over an artboard's right or bottom edge the cursor shows Move and a click selects that
+    /// artboard, as on its left or top edge.
+    #[test]
+    fn a_click_on_an_artboards_right_or_bottom_edge_selects_it() {
+        let d = two_boards();
+        let (s, p) = (Selection::default(), paint());
+        let c = cx(&d, &s, &p);
+        for at in [Point::new(800.0, 100.0), Point::new(700.0, 200.0)] {
+            let mut t = ArtboardTool::default();
+            assert_eq!(t.cursor(&c, at, Mods::default()), Cursor::Move, "{at:?}");
+            t.pointer(&c, &ev(PointerKind::Down, at.x, at.y));
+            t.pointer(&c, &ev(PointerKind::Up, at.x, at.y));
+            assert_eq!(t.options()["selected"], json!([1]), "{at:?}");
+        }
+    }
+
     /// A proportional resize never asks for a side under 1 pt, which the command would lengthen
     /// alone (scaling the art unevenly): both sides grow by one factor instead.
     #[test]