From dfa8403ed09b21276d5c7fc147f9b99c542d653d Mon Sep 17 00:00:00 2001 From: makesomethingshit Date: Thu, 12 Nov 2026 13:46:51 +0900 Subject: [PATCH] Fix #6373: Keep painting through long Live Paint drags --- crates/engine/src/cmd/buildcmds.rs | 2 +- crates/engine/src/cmd/buildcmds/live_paint.rs | 50 +++++++++++++------ crates/engine/src/tests/build.rs | 43 ++++++++++++++++ crates/tools/src/builder.rs | 4 +- crates/tools/src/builder/live_paint.rs | 29 ++++++++--- crates/tools/src/builder/tests.rs | 19 +++++++ 6 files changed, 123 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index b6a0c9c9e..60a6b8137 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -117,7 +117,7 @@ pub fn specs() -> Vec { "Live Paint Bucket", [], None, - "{group?: id | ids?|id?: [id…] (paths to make live first, as livePaint.make does), point: [x,y], points?: [[x,y]…] (a drag polyline starting at point; 1 to 4096 finite canvas points; fills every crossed face of the same group), color?|swatch?|gradient?|none? (default: current fill; a color in #rrggbbaa or #rgba is refused), tint?: 0..100, keepModel?: bool (both as paint.setFill)} fill the face containing point, skipping hidden faces → {group, face}", + "{group?: id | ids?|id?: [id…] (paths to make live first, as livePaint.make does), point: [x,y], points?: [[x,y]…] (a drag polyline starting at point; 1 to 4096 finite canvas points; fills every crossed face of the same group), continuation?: [[[x,y]…]…] (further path chunks of 1 to 4096 finite canvas points each; each starts at the preceding chunk's last point; requires points), color?|swatch?|gradient?|none? (default: current fill; a color in #rrggbbaa or #rgba is refused), tint?: 0..100, keepModel?: bool (both as paint.setFill)} fill the face containing point, skipping hidden faces → {group, face}", has_doc, lp_fill ), diff --git a/crates/engine/src/cmd/buildcmds/live_paint.rs b/crates/engine/src/cmd/buildcmds/live_paint.rs index 5f4b1205f..3f9b62df2 100644 --- a/crates/engine/src/cmd/buildcmds/live_paint.rs +++ b/crates/engine/src/cmd/buildcmds/live_paint.rs @@ -272,17 +272,34 @@ pub(super) fn lp_gap_options(s: &mut Session, p: &Value) -> Result { pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result { const C: &str = "livePaint.fill"; let pt = point_param(p, "point").filter(|q| finite(*q)).ok_or_else(|| bad(C, "missing point [x,y]"))?; - let points = if let Some(input) = p.get("points") { - if input.as_array().is_none_or(|points| points.is_empty() || points.len() > 4096) { - return Err(bad(C, "points must contain 1 to 4096 points")); + let parse_chunk = |input: &Value, start: Point| -> Result> { + let values = input + .as_array() + .filter(|points| !points.is_empty() && points.len() <= b::LP_FILL_CHUNK_POINTS) + .ok_or_else(|| bad(C, "each path chunk must contain 1 to 4096 points"))?; + let points: Vec<_> = values + .iter() + .map(|point| { + let pair = point.as_array().filter(|pair| pair.len() == 2).ok_or_else(|| bad(C, "points must be [x,y] pairs"))?; + let x = pair.first().and_then(Value::as_f64).ok_or_else(|| bad(C, "point x must be a number"))?; + let y = pair.get(1).and_then(Value::as_f64).ok_or_else(|| bad(C, "point y must be a number"))?; + let point = Point::new(x, y); + if !finite(point) { + return Err(bad(C, "points must be finite canvas points")); + } + Ok(point) + }) + .collect::>()?; + if points.first() != Some(&start) { + return Err(bad(C, "path chunks must share their boundary point")); } - let points = super::points_param(p, "points").ok_or_else(|| bad(C, "points must be [[x,y]...]"))?; - if points.first() != Some(&pt) || !points.iter().all(|q| finite(*q)) { - return Err(bad(C, "points must start at point and contain 1 to 4096 finite canvas points")); - } - Some(points) - } else { - None + Ok(points) + }; + let points = p.get("points"); + let continuation = match p.get("continuation") { + None => &[][..], + Some(input) if points.is_some() => input.as_array().ok_or_else(|| bad(C, "continuation must be an array of path chunks"))?.as_slice(), + Some(_) => return Err(bad(C, "continuation requires points")), }; crate::cmd::paint::check_keep_model(p, C)?; let paint = paint_from(s, p)?.unwrap_or_else(|| s.paint.fill.clone()); @@ -314,8 +331,8 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result { .and_then(|n| b::visible_face_at(n, pt)) .map(|f| f.id) .ok_or_else(|| EngineError::Other("no Live Paint face at that point".into()))?; - let mut targets = vec![fid]; - if let Some(points) = &points { + let mut targets = std::collections::BTreeSet::from([fid]); + if let Some(points) = points { let n = d.node(gid).ok_or(EngineError::NoNode(gid))?; // Use the existing region-crossing probes, so even a face narrower than the distance // between pointer events is filled. Reversing matches visible_face_at's front first. @@ -331,9 +348,12 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result { } } let map = b::BuilderMap { regions, ..Default::default() }; - targets.extend(map.hits_along(points, None).regions.iter().filter_map(|i| face_ids.get(*i).copied())); - targets.sort(); - targets.dedup(); + let mut last = pt; + for input in std::iter::once(points).chain(continuation) { + let chunk = parse_chunk(input, last)?; + last = chunk.last().copied().unwrap_or(last); + targets.extend(map.hits_along(&chunk, None).regions.iter().filter_map(|i| face_ids.get(*i).copied())); + } } for target in targets { d.node_mut(target).ok_or(EngineError::NoNode(target))?.appearance.set_fill(paint.clone()); diff --git a/crates/engine/src/tests/build.rs b/crates/engine/src/tests/build.rs index b5b4f35e7..0fc2bcb47 100644 --- a/crates/engine/src/tests/build.rs +++ b/crates/engine/src/tests/build.rs @@ -3003,6 +3003,42 @@ fn live_paint_bucket_drag_crosses_thin_grid_cells_and_can_cancel() { } } +#[test] +fn live_paint_bucket_long_drag_keeps_late_turns_and_release_as_one_undo() { + for cancel in [false, true] { + let mut s = session(); + let ids = [rect(&mut s, 0.0, 0.0, 150.0, 100.0), line(&mut s, (50.0, 0.0), (50.0, 100.0)), line(&mut s, (100.0, 0.0), (100.0, 100.0))]; + s.execute("select.set", &json!({"ids": ids.map(|id| id.0)})).unwrap(); + let before = s.doc().unwrap().doc.clone(); + let v = ViewInfo::default(); + s.select_tool("livePaintBucket", v).unwrap(); + s.prefs.real_time_drawing = false; + s.paint.fill = Paint::solid(Color::rgb(0.0, 1.0, 1.0)); + s.pointer(&PointerEvent::new(PointerKind::Down, 25.0, 50.0), v).unwrap(); + for i in 1..4096 { + s.pointer(&PointerEvent::new(PointerKind::Drag, 25.0 + f64::from(i) / 10000.0, 50.0), v).unwrap(); + } + // Turn around the middle face after the chunk limit, then enter the right face on Up. + for (x, y) in [(25.0, 150.0), (125.0, 150.0)] { + s.pointer(&PointerEvent::new(PointerKind::Drag, x, y), v).unwrap(); + } + if cancel { + s.tool_key(ToolKey::Escape, Mods::default(), v).unwrap(); + } + s.pointer(&PointerEvent::new(PointerKind::Up, 125.0, 50.0), v).unwrap(); + if !cancel { + let g = s.doc().unwrap().selection.objects[0]; + let gn = node(&s, g); + for x in [25.0, 125.0] { + assert_eq!(vectorcraft_tools::builder::face_at(&gn, Point::new(x, 50.0)).unwrap().appearance.fill_paint(), s.paint.fill); + } + assert_eq!(vectorcraft_tools::builder::face_at(&gn, Point::new(75.0, 50.0)).unwrap().appearance.fill_paint(), Paint::solid(Color::WHITE)); + s.execute("edit.undo", &json!({})).unwrap(); + } + assert!(s.doc().unwrap().doc == before, "long drag is one cancellable undo step"); + } +} + #[test] fn live_paint_bucket_drag_rejects_invalid_points_atomically() { let mut s = session(); @@ -3014,6 +3050,13 @@ fn live_paint_bucket_drag_rejects_invalid_points_atomically() { assert!(s.execute("livePaint.fill", &json!({"group": g.0, "point": [25.0, 50.0], "points": points})).is_err()); assert_eq!(s.doc().unwrap().doc, before); } + for continuation in [json!(null), json!([[[26.0, 50.0], [125.0, 50.0]]]), json!([vec![[25.0, 50.0]; 4097]])] { + assert!( + s.execute("livePaint.fill", &json!({"group": g.0, "point": [25.0, 50.0], "points": [[25.0, 50.0]], "continuation": continuation})) + .is_err() + ); + assert_eq!(s.doc().unwrap().doc, before); + } } #[test] diff --git a/crates/tools/src/builder.rs b/crates/tools/src/builder.rs index b8e5840c2..1c5a89516 100644 --- a/crates/tools/src/builder.rs +++ b/crates/tools/src/builder.rs @@ -37,8 +37,8 @@ use cache::{BACKGROUND_SEGMENTS, CacheKey, RegionCache}; use live_paint::LpHover; use live_paint::lp_color; pub use live_paint::{ - LP_BUCKET_EDGE_FRACTION, LP_BUCKET_WIDTH, LP_BUCKET_WIDTH_RANGE, LP_SELECT_WIDTH, LP_SELECT_WIDTH_RANGE, LivePaintBucketTool, - LivePaintSelectionTool, + LP_BUCKET_EDGE_FRACTION, LP_BUCKET_WIDTH, LP_BUCKET_WIDTH_RANGE, LP_FILL_CHUNK_POINTS, LP_SELECT_WIDTH, LP_SELECT_WIDTH_RANGE, + LivePaintBucketTool, LivePaintSelectionTool, }; pub use lookup::{ EDGE_NAME, FACE_NAME, Face, LIVE_PAINT_NAME, LIVE_PAINT_PREFIX, SOURCES_NAME, builder_members, drawn_outlines, edge_near, edges, face_at, faces, diff --git a/crates/tools/src/builder/live_paint.rs b/crates/tools/src/builder/live_paint.rs index 4822617ad..010e42374 100644 --- a/crates/tools/src/builder/live_paint.rs +++ b/crates/tools/src/builder/live_paint.rs @@ -132,7 +132,7 @@ pub struct LivePaintBucketTool { /// Where the pointer last moved, and whether Shift and Alt were held. at: Option<(Point, bool, bool)>, /// The fill command and polyline previewed from the press's snapshot. - fill_drag: Option<(Value, Vec)>, + fill_drag: Option<(Value, Vec>)>, fills: bool, strokes: bool, highlight: bool, @@ -140,6 +140,9 @@ pub struct LivePaintBucketTool { width: f64, } +/// Maximum points in one Live Paint fill path chunk. +pub const LP_FILL_CHUNK_POINTS: usize = 4096; + impl Default for LivePaintBucketTool { fn default() -> Self { Self { @@ -158,18 +161,32 @@ impl Default for LivePaintBucketTool { impl LivePaintBucketTool { fn begin_fill(&mut self, params: Value, p: Point) -> Vec { - self.fill_drag = Some((params.clone(), vec![p])); + self.fill_drag = Some((params.clone(), vec![vec![p]])); vec![Action::Begin("Live Paint Bucket".into()), Action::Preview("livePaint.fill".into(), params)] } fn drag_fill(&mut self, p: Point) -> Vec { - let Some((params, points)) = &mut self.fill_drag else { return vec![] }; - if points.last() == Some(&p) || points.len() >= 4096 { + let Some((params, chunks)) = &mut self.fill_drag else { return vec![] }; + let Some(points) = chunks.last_mut() else { return vec![] }; + let Some(&last) = points.last() else { return vec![] }; + if last == p { return vec![]; } - points.push(p); + if points.len() == LP_FILL_CHUNK_POINTS { + // Share the boundary point so no segment is lost between chunks. + chunks.push(vec![last, p]); + } else { + points.push(p); + } let mut params = params.clone(); - params["points"] = json!(points.iter().map(|p| [p.x, p.y]).collect::>()); + let mut chunks = chunks.iter().map(|chunk| json!(chunk.iter().map(|p| [p.x, p.y]).collect::>())); + if let Some(first) = chunks.next() { + params["points"] = first; + } + let continuation: Vec<_> = chunks.collect(); + if !continuation.is_empty() { + params["continuation"] = Value::Array(continuation); + } vec![Action::Preview("livePaint.fill".into(), params)] } diff --git a/crates/tools/src/builder/tests.rs b/crates/tools/src/builder/tests.rs index 35ef46e62..eeaf8f0ea 100644 --- a/crates/tools/src/builder/tests.rs +++ b/crates/tools/src/builder/tests.rs @@ -1045,6 +1045,25 @@ fn live_square(d: &mut Document, r: Rect) -> (NodeId, [NodeId; 4]) { (gid, ids) } +#[test] +fn bucket_long_drag_keeps_turns_and_release_after_the_point_limit() { + let (d, gid, ..) = live_group_doc(); + let s = Selection::default(); + let paint = paint(); + let cx = cx(&d, &s, &paint); + let mut t = LivePaintBucketTool::default(); + t.pointer(&cx, &ev(PointerKind::Down, 25.0, 25.0)); + for i in 1..4096 { + t.pointer(&cx, &ev(PointerKind::Drag, 25.0 + f64::from(i) / 10000.0, 25.0)); + } + t.pointer(&cx, &ev(PointerKind::Drag, 25.0, 100.0)); + t.pointer(&cx, &ev(PointerKind::Drag, 75.0, 100.0)); + let acts = t.pointer(&cx, &ev(PointerKind::Up, 75.0, 25.0)); + let [Action::Preview(id, params), Action::Commit] = acts.as_slice() else { panic!("release must extend the drag before committing") }; + assert_eq!(id, "livePaint.fill"); + assert_eq!(params["group"], gid.0); +} + #[test] fn bucket_click_fills_face_and_shift_click_strokes_edge() { let (d, gid, f2, _e) = live_group_doc();