Merge pull request #3272 from FractalEncrypt/fix/outline-stroke-open-fills

Keep Group Selection drags on the selected outlined component
This commit is contained in:
BFlat
2026-11-05 20:18:22 +07:00
committed by GitHub
9 changed files with 286 additions and 8 deletions
+2
View File
@@ -897,6 +897,8 @@ pub(crate) fn outline_strokes(d: &mut Document, brushes: &[vectorcraft_brush::Br
return None;
}
let (path, rule) = node_path(l)?;
// A fill belongs to the source path, whether open or closed. Converting its stroke
// preserves that fill on a separate copy (including compound member geometry).
let mut parts: Vec<Node> = vec![];
let mut fills: Vec<AppearanceItem> = vec![];
for item in &l.appearance.items {
+221
View File
@@ -459,6 +459,26 @@ fn outline_stroke_of_line() {
assert!((area_of(&s, r[0]) - 1000.0).abs() < 1.0);
}
#[test]
fn outline_stroke_preserves_a_zero_area_fill_without_retaining_a_live_stroke() {
let mut s = session();
s.execute("shape.line", &json!({"x1": 10, "y1": 20, "x2": 60, "y2": 90})).unwrap();
s.execute("paint.setFill", &json!({"color":"#ffffff"})).unwrap();
let before = s.doc().unwrap().doc.clone();
let result = ids(&s.execute("object.path.outlineStroke", &json!({})).unwrap());
let n = node(&s, result[0]);
let children = n.children().expect("the original fill and the outlined stroke");
assert_eq!(children.len(), 2);
assert!(children.iter().all(|n| n.appearance.stroke().is_none()));
assert!(children.iter().any(|n| n.appearance.fill_paint() == Paint::solid(Color::WHITE)), "an invisible fill is not a live stroke");
assert!(n.appearance.stroke().is_none());
assert!(children.iter().any(|n| n.appearance.fill_paint() == Paint::solid(Color::BLACK)));
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.doc().unwrap().doc.layers, before.layers);
s.execute("edit.redo", &json!({})).unwrap();
assert!(node(&s, result[0]).appearance.stroke().is_none());
}
#[test]
fn outline_stroke_with_fill_makes_group() {
let mut s = session();
@@ -1076,6 +1096,207 @@ fn trim_merge_and_crop_remove_strokes() {
}
}
#[test]
fn outline_stroke_preserves_tiny_and_signed_area_canceling_closed_fills() {
for points in [vec![[0.0, 0.0], [100.0, 100.0], [0.0, 100.0], [100.0, 0.0]], vec![[0.0, 0.0], [1e-8, 0.0], [1e-8, 1e-8]]] {
let mut s = session();
let anchors: Vec<Value> = points.into_iter().map(|[x, y]| json!({"x":x,"y":y})).collect();
s.execute("path.create", &json!({"anchors":anchors,"closed":true})).unwrap();
let result = ids(&s.execute("object.path.outlineStroke", &json!({})).unwrap());
let n = node(&s, result[0]);
let retains_fill = n.children().is_some_and(|ch| ch.iter().any(|c| c.appearance.fill_paint() == Paint::solid(Color::WHITE)))
|| n.appearance.fill_paint() == Paint::solid(Color::WHITE);
assert!(retains_fill, "a real fill survives independently of its signed area or scale");
}
}
#[test]
fn outline_stroke_preserves_bent_open_fill_and_history() {
let mut s = session();
s.execute("path.create", &json!({"anchors":[{"x":0,"y":0},{"x":60,"y":0},{"x":75,"y":-15},{"x":110,"y":-15}],"closed":false})).unwrap();
let before = s.doc().unwrap().doc.clone();
let result = ids(&s.execute("object.path.outlineStroke", &json!({})).unwrap());
let n = node(&s, result[0]);
let children = n.children().expect("original fill beside the outlined stroke");
assert_eq!(children.len(), 2);
let fill = children.iter().find(|n| n.appearance.fill_paint() == Paint::solid(Color::WHITE)).unwrap();
assert!(!fill.path_data().unwrap().subpaths[0].closed);
assert!(children.iter().all(|n| n.appearance.stroke().is_none()));
let after = s.doc().unwrap().doc.layers.clone();
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.doc().unwrap().doc.layers, before.layers);
s.execute("edit.redo", &json!({})).unwrap();
assert_eq!(s.doc().unwrap().doc.layers, after);
}
#[test]
fn outline_stroke_mixed_contours_preserve_all_fill() {
use vectorcraft_geom::{Anchor, Point, SubPath};
let mut s = session();
let id = rect(&mut s, 0.0, 0.0, 20.0, 20.0);
s.edit("mixed contours", |d, _| {
let n = d.node_mut(id).unwrap();
if let NodeKind::Path { path, .. } = &mut n.kind {
path.subpaths.push(SubPath::new(
vec![Anchor::corner(Point::new(50.0, 0.0)), Anchor::corner(Point::new(80.0, 0.0)), Anchor::corner(Point::new(80.0, 20.0))],
false,
));
}
Ok(())
})
.unwrap();
let result = ids(&s.execute("object.path.outlineStroke", &json!({})).unwrap());
let outlined = node(&s, result[0]);
let children = outlined.children().unwrap();
let fill = children.iter().find(|n| n.appearance.fill_paint() == Paint::solid(Color::WHITE)).unwrap();
let NodeKind::Path { path, .. } = &fill.kind else { panic!("fill is a path") };
assert_eq!(path.subpaths.len(), 2);
assert!(path.subpaths[0].closed);
assert!(!path.subpaths[1].closed);
}
#[test]
fn outline_stroke_keeps_closed_compound_members_editable() {
check_outlined_compound_members(false);
}
#[test]
fn outline_stroke_keeps_open_compound_contours_and_members() {
check_outlined_compound_members(true);
}
fn check_outlined_compound_members(mixed: bool) {
use vectorcraft_geom::{Anchor, Point, SubPath};
let mut s = session();
let outer = rect(&mut s, 10.0, 10.0, 100.0, 100.0);
let inner = rect(&mut s, 35.0, 35.0, 30.0, 30.0);
let mut members = vec![outer.0, inner.0];
if mixed {
let open = s.execute("path.create", &json!({"anchors":[{"x":140,"y":10},{"x":170,"y":30},{"x":140,"y":50}]})).unwrap();
members.push(open["id"].as_u64().unwrap());
s.edit("Mixed compound member", |d, _| {
let NodeKind::Path { path, .. } = &mut d.node_mut(inner).unwrap().kind else { panic!("path") };
path.subpaths.push(SubPath::new(vec![Anchor::corner(Point::new(45.0, 80.0)), Anchor::corner(Point::new(70.0, 90.0))], false));
Ok(())
})
.unwrap();
}
s.execute("select.set", &json!({"ids":members})).unwrap();
let compound = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap());
s.edit("Compound member names and rule", |d, _| {
d.node_mut(outer).unwrap().name = Some("outer-member".into());
d.node_mut(inner).unwrap().name = Some("inner-member".into());
let NodeKind::Compound { rule, .. } = &mut d.node_mut(compound).unwrap().kind else { panic!("compound") };
*rule = FillRule::EvenOdd;
Ok(())
})
.unwrap();
let original = node(&s, compound);
let expected: Vec<_> = original
.children()
.unwrap()
.iter()
.map(|child| {
let path = child.path_data().unwrap().clone();
(child.name.clone(), path)
})
.collect();
let before = s.doc().unwrap().doc.layers.clone();
let result = ids(&s.execute("object.path.outlineStroke", &json!({})).unwrap())[0];
let group = node(&s, result);
let fill = group.children().unwrap().iter().find(|child| child.appearance.fill_paint() == Paint::solid(Color::WHITE)).unwrap();
let NodeKind::Compound { children, rule } = &fill.kind else { panic!("closed fill must retain editable compound members") };
assert_eq!(*rule, FillRule::EvenOdd);
assert_eq!(children.len(), if mixed { 3 } else { 2 });
for (child, (name, path)) in children.iter().zip(expected) {
assert_eq!(child.name, name);
assert_eq!(child.path_data().unwrap(), &path);
assert_ne!(child.id, outer);
assert_ne!(child.id, inner);
}
let member = children[0].id;
let sibling = children[1].id;
let member_bounds = children[0].geometric_bounds().unwrap();
let sibling_before = children[1].clone();
let after = s.doc().unwrap().doc.layers.clone();
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.doc().unwrap().doc.layers, before);
s.execute("edit.redo", &json!({})).unwrap();
assert_eq!(s.doc().unwrap().doc.layers, after);
// Group Selection can still address one retained child without moving its sibling.
s.execute("select.set", &json!({"ids":[member.0]})).unwrap();
s.execute("object.move", &json!({"dx":7,"dy":3})).unwrap();
assert!(close(
node(&s, member).geometric_bounds().unwrap(),
Rect::new(member_bounds.x0 + 7.0, member_bounds.y0 + 3.0, member_bounds.x1 + 7.0, member_bounds.y1 + 3.0),
1e-6
));
assert_eq!(node(&s, sibling), *sibling_before);
}
/// Exercise the real Group Selection pointer route, not select.set by id.
#[test]
fn outline_fill_and_stroke_move_independently_with_group_selection() {
use vectorcraft_tools::{PointerEvent, PointerKind};
for closed in [true, false] {
for move_fill in [false, true] {
for click_first in [false, true] {
let mut s = session();
s.execute(
"path.create",
&json!({"anchors":[{"x":100,"y":100},{"x":200,"y":100},{"x":200,"y":200},{"x":100,"y":200}],"closed":closed}),
)
.unwrap();
s.execute("paint.setFill", &json!({"color":"#ff0000"})).unwrap();
s.execute("paint.setStroke", &json!({"color":"#000000"})).unwrap();
s.execute("stroke.set", &json!({"weight":10})).unwrap();
let root = ids(&s.execute("object.path.outlineStroke", &json!({})).unwrap())[0];
let group = node(&s, root);
let children = group.children().expect("fill and outlined stroke stay separate");
assert_eq!(children.len(), 2);
let fill = children.iter().find(|c| c.appearance.fill_paint().color().is_some_and(|c| c.to_hex() == "#ff0000")).unwrap().id;
let stroke = children.iter().find(|c| c.id != fill).unwrap().id;
group.walk(&mut |n| assert!(n.appearance.stroke().is_none(), "no retained live stroke"));
let moved = if move_fill { fill } else { stroke };
let other = if move_fill { stroke } else { fill };
let other_before = node(&s, other);
let bounds_before = bounds(&s, moved);
let layers_before_move = s.doc().unwrap().doc.layers.clone();
let v = ViewInfo { smart_guides: false, snap_to_point: false, ..ViewInfo::default() };
s.select_tool("groupSelection", v).unwrap();
// Start empty, as clicking the art with Group Selection does after deselection.
s.execute("select.none", &json!({})).unwrap();
let (x, y) = if move_fill { (150.0, 150.0) } else { (150.0, 100.0) };
let compound_stroke = !move_fill && matches!(node(&s, stroke).kind, NodeKind::Compound { .. });
for _ in 0..if compound_stroke {
2
} else if click_first {
1
} else {
0
} {
s.pointer(&PointerEvent::new(PointerKind::Down, x, y), v).unwrap();
s.pointer(&PointerEvent::new(PointerKind::Up, x, y), v).unwrap();
}
for (kind, x, y) in [(PointerKind::Down, x, y), (PointerKind::Drag, x + 30.0, y + 15.0), (PointerKind::Up, x + 30.0, y + 15.0)] {
s.pointer(&PointerEvent::new(kind, x, y), v).unwrap();
}
assert_eq!(node(&s, other), other_before, "sibling unchanged: closed={closed}, fill={move_fill}, click_first={click_first}");
let b = bounds(&s, moved);
assert!(
close(b, Rect::new(bounds_before.x0 + 30.0, bounds_before.y0 + 15.0, bounds_before.x1 + 30.0, bounds_before.y1 + 15.0), 1e-6),
"moved leaf: {b:?}"
);
let layers_after_move = s.doc().unwrap().doc.layers.clone();
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.doc().unwrap().doc.layers, layers_before_move);
s.execute("edit.redo", &json!({})).unwrap();
assert_eq!(s.doc().unwrap().doc.layers, layers_after_move);
}
}
}
}
/// #1590: an inset larger than the shape leaves nothing, and the error says so rather than asking
/// for paths that are selected.
#[test]
+10 -3
View File
@@ -1126,8 +1126,8 @@ fn a_drag_from_a_fill_moves_only_the_selected_anchors_of_partly_selected_paths()
}
/// A path selected whole inside a selected group moves once, with the group, when the selection
/// also holds a direct-selected anchor: a Group Selection drag from the path's fill (which adds
/// the group) and an arrow key move both by 10 pt.
/// also holds a direct-selected anchor: a completed Group Selection click adds the group, then
/// a drag and an arrow key move both by 10 pt. A leaf drag before that click keeps its sibling still.
#[test]
fn a_path_in_a_selected_group_moves_once_with_selected_anchors() {
use vectorcraft_tools::PointerKind::{Down, Drag, Up};
@@ -1142,7 +1142,14 @@ fn a_path_in_a_selected_group_moves_once_with_selected_anchors() {
s.execute("select.set", &json!({"ids": [a.0]})).unwrap();
s.execute("select.anchors", &json!({"id": other.0, "anchors": [[0, 0]], "mode": "add"})).unwrap();
gesture(&mut s, v, &[(Down, 125.0, 125.0), (Drag, 130.0, 125.0), (Drag, 135.0, 125.0), (Up, 135.0, 125.0)]);
assert!(s.doc().unwrap().selection.contains(g), "the press added the group");
assert!(!s.doc().unwrap().selection.contains(g), "the leaf drag keeps its selection");
assert_eq!((top_left(&s, a), top_left(&s, b), first_x(&s)), ((110.0, 100.0), (160.0, 100.0), 310.0), "the leaf and selected anchor move");
s.execute("edit.undo", &json!({})).unwrap();
s.execute("select.set", &json!({"ids": [a.0]})).unwrap();
s.execute("select.anchors", &json!({"id": other.0, "anchors": [[0, 0]], "mode": "add"})).unwrap();
gesture(&mut s, v, &[(Down, 125.0, 125.0), (Up, 125.0, 125.0)]);
assert!(s.doc().unwrap().selection.contains(g), "the completed click added the group");
gesture(&mut s, v, &[(Down, 125.0, 125.0), (Drag, 130.0, 125.0), (Drag, 135.0, 125.0), (Up, 135.0, 125.0)]);
assert_eq!((top_left(&s, a), top_left(&s, b), first_x(&s)), ((110.0, 100.0), (170.0, 100.0), 310.0), "the drag");
s.execute("edit.undo", &json!({})).unwrap();
s.execute("select.set", &json!({"ids": [g.0, a.0]})).unwrap();
+5 -3
View File
@@ -215,8 +215,8 @@ pub struct DirectSelectionTool {
click: Option<(NodeId, usize, usize)>,
/// A click on a whole path picks the segment on release; a drag still moves the whole path.
click_segment: Option<(NodeId, usize, usize)>,
/// An Alt-click on a path selected whole adds this enclosing group on release, as a Group
/// Selection click does; an Alt-drag still copies the selection.
/// A Group Selection click, or an Alt-click on a path selected whole, adds this enclosing
/// group on release. A drag keeps the selection; an Alt-drag still copies it.
click_group: Option<NodeId>,
/// The selected anchors of area type's frames, picked by a click or a marquee: they show
/// filled, a drag of one moves them all, and the arrow keys nudge them. They stay selected
@@ -1159,6 +1159,7 @@ impl DirectSelectionTool {
Some(vec![])
}
fn press_group(&mut self, cx: &ToolContext, ev: &PointerEvent, p: Point, tol: f64) -> Vec<Action> {
self.click_group = None;
if let Some(out) = self.guide.press(cx, ev) {
self.state = State::Idle;
return out;
@@ -1188,7 +1189,8 @@ impl DirectSelectionTool {
}
} else if !cx.selection.contains(target) {
if cx.selection.contains(h.leaf) {
out.push(Action::Exec("select.add".into(), json!({"ids": [target.0]})));
// A completed click climbs the group; a drag moves only the selected art.
self.click_group = Some(target);
} else {
out.push(Action::Exec("select.set".into(), json!({"ids": [target.0]})));
}
+32 -2
View File
@@ -755,11 +755,17 @@ fn group_selection_picks_a_path_of_a_compound_path() {
d.insert(Some(l), 0, c).unwrap();
let p = paint();
let none = Selection::default();
let click = |s: &Selection, x, y| DirectSelectionTool::new(true).pointer(&cx(&d, s, &p), &PointerEvent::new(PointerKind::Down, x, y));
let click = |s: &Selection, x, y| {
let mut t = DirectSelectionTool::new(true);
let context = cx(&d, s, &p);
let mut actions = t.pointer(&context, &PointerEvent::new(PointerKind::Down, x, y));
actions.extend(t.pointer(&context, &PointerEvent::new(PointerKind::Up, x, y)));
actions
};
let set = |id: NodeId| vec![Action::Exec("select.set".into(), json!({"ids": [id.0]}))];
assert_eq!(click(&none, 25.0, 100.0), set(outer), "the fill between the outline and the hole");
assert_eq!(click(&none, 50.0, 100.0), set(hole), "the hole's outline");
assert!(click(&none, 100.0, 100.0).is_empty(), "inside the hole nothing is hit");
assert_eq!(click(&none, 100.0, 100.0), vec![Action::Exec("select.none".into(), json!({}))], "a click inside the hole clears selection");
let mut s = Selection::default();
s.set([outer]);
assert_eq!(click(&s, 25.0, 100.0), vec![Action::Exec("select.add".into(), json!({"ids": [compound.0]}))]);
@@ -813,6 +819,30 @@ fn nested_group_doc() -> (vectorcraft_doc::Document, NodeId, NodeId, NodeId, Nod
(d, inside, outside, inner, outer)
}
/// A selected component drags independently; only a completed click adds its group.
#[test]
fn group_selection_distinguishes_selected_leaf_drags_from_clicks() {
let (d, inside, _, inner, _) = nested_group_doc();
let paint = paint();
let mut selected = Selection::default();
selected.set([inside]);
let context = cx(&d, &selected, &paint);
let mut t = DirectSelectionTool::new(true);
let down = PointerEvent::new(PointerKind::Down, 150.0, 150.0);
assert!(t.pointer(&context, &down).is_empty());
let actions = t.pointer(&context, &PointerEvent::new(PointerKind::Drag, 180.0, 165.0));
assert!(matches!(actions.as_slice(), [Action::Begin(_), Action::Preview(command, _)] if command=="object.transform"));
assert_eq!(t.pointer(&context, &PointerEvent::new(PointerKind::Up, 180.0, 165.0)), vec![Action::Commit]);
assert!(t.pointer(&context, &down).is_empty());
assert_eq!(
t.pointer(&context, &PointerEvent::new(PointerKind::Up, 150.0, 150.0)),
vec![Action::Exec("select.add".into(), json!({"ids":[inner.0]}))]
);
assert!(t.pointer(&context, &down).is_empty());
assert_eq!(t.key(&context, ToolKey::Escape, Mods::default()), vec![Action::Cancel]);
assert!(t.pointer(&context, &PointerEvent::new(PointerKind::Up, 150.0, 150.0)).is_empty());
}
/// Isolation mode: a marquee and an anchor click take the path inside the isolated group and
/// leave the path beside it. Group Selection stops at the isolated group instead of adding
/// the group around it.
+4
View File
@@ -4,6 +4,10 @@
## Feature submission evidence
Group Selection keeps an already selected component selected during a drag; a completed click still adds its next enclosing group. The current Direct Selection click/release machinery is shared, preserving compound-member picks, Shift deselection, isolation and masked-art dragging. Outline Stroke continues to preserve all original fills, including open contours; the change does not remove fills. Regressions: `group_selection_distinguishes_selected_leaf_drags_from_clicks` and `outline_fill_and_stroke_move_independently_with_group_selection` cover tool events, independent component movement and Undo/Redo. Scores and estimates unchanged.
**PR #3272 revision — 2026-11-04:** Group Selection component drags defer ancestor selection until a completed click, using current Direct Selection release handling; original Outline Stroke fills and newer selection behavior retained. Scores and estimates unchanged.
Creating a gradient from solid or empty paint uses the document's color model for its default stops, including drawing its vector with the Gradient tool. Explicit `keepModel: true` retains RGB defaults, and existing gradients keep their own colors. Scores and estimates are unchanged.
The closed font-family fields in Character, Properties and Control now accept the
+4
View File
@@ -4,6 +4,10 @@
## Feature submission evidence
Group Selection keeps an already selected component selected during a drag; a completed click still adds its next enclosing group. The current Direct Selection click/release machinery is shared, preserving compound-member picks, Shift deselection, isolation and masked-art dragging. Outline Stroke continues to preserve all original fills, including open contours; the change does not remove fills. Regressions: `group_selection_distinguishes_selected_leaf_drags_from_clicks` and `outline_fill_and_stroke_move_independently_with_group_selection` cover tool events, independent component movement and Undo/Redo. Scores and estimates unchanged.
**PR #3272 revision — 2026-11-04:** Group Selection component drags defer ancestor selection until a completed click, using current Direct Selection release handling; original Outline Stroke fills and newer selection behavior retained. Scores and estimates unchanged.
Creating a gradient from solid or empty paint uses the document's color model for its default stops, including drawing its vector with the Gradient tool. Explicit `keepModel: true` retains RGB defaults, and existing gradients keep their own colors. Scores and estimates are unchanged.
**Feature evidence last updated:** 2026-11-03.
+4
View File
@@ -4,6 +4,10 @@
## Feature submission evidence
Group Selection keeps an already selected component selected during a drag; a completed click still adds its next enclosing group. The current Direct Selection click/release machinery is shared, preserving compound-member picks, Shift deselection, isolation and masked-art dragging. Outline Stroke continues to preserve all original fills, including open contours; the change does not remove fills. Regressions: `group_selection_distinguishes_selected_leaf_drags_from_clicks` and `outline_fill_and_stroke_move_independently_with_group_selection` cover tool events, independent component movement and Undo/Redo. Scores and estimates unchanged.
**PR #3272 revision — 2026-11-04:** Group Selection component drags defer ancestor selection until a completed click, using current Direct Selection release handling; original Outline Stroke fills and newer selection behavior retained. Scores and estimates unchanged.
Ligature caret stops use a font's GDEF contour-point positions in horizontal and vertical text. They use unhinted outlines at the default variation location; invalid point indices use the existing evenly divided fallback. Scores and estimates are unchanged.
Creating a gradient from solid or empty paint uses the document's color model for its default stops, including drawing its vector with the Gradient tool. Explicit `keepModel: true` retains RGB defaults, and existing gradients keep their own colors. Scores and estimates are unchanged.
+4
View File
@@ -4,6 +4,10 @@
## Feature submission evidence
Group Selection keeps an already selected component selected during a drag; a completed click still adds its next enclosing group. The current Direct Selection click/release machinery is shared, preserving compound-member picks, Shift deselection, isolation and masked-art dragging. Outline Stroke continues to preserve all original fills, including open contours; the change does not remove fills. Regressions: `group_selection_distinguishes_selected_leaf_drags_from_clicks` and `outline_fill_and_stroke_move_independently_with_group_selection` cover tool events, independent component movement and Undo/Redo. Scores and estimates unchanged.
**PR #3272 revision — 2026-11-04:** Group Selection component drags defer ancestor selection until a completed click, using current Direct Selection release handling; original Outline Stroke fills and newer selection behavior retained. Scores and estimates unchanged.
Creating a gradient from solid or empty paint uses the document's color model for its default stops, including drawing its vector with the Gradient tool. Explicit `keepModel: true` retains RGB defaults, and existing gradients keep their own colors. Scores and estimates are unchanged.
**Feature evidence last updated:** 2026-11-03.