diff --git a/apps/vectorcraft/src/main.rs b/apps/vectorcraft/src/main.rs index 7594bc2344..fdbec59bfc 100644 --- a/apps/vectorcraft/src/main.rs +++ b/apps/vectorcraft/src/main.rs @@ -107,7 +107,7 @@ use titlebar::opted_into_system_title_bar; #[cfg(windows)] use titlebar::set_taskbar_icon; #[cfg(all(test, feature = "wgpu"))] -use titlebar::{app_draws_frame, opted_into_app_title_bar, start_frame, system_title_bar_applies}; +use titlebar::{app_draws_frame, caption_layout_from, opted_into_app_title_bar, start_frame, system_title_bar_applies}; use titlebar::{custom_titlebar, native_options, titlebar_from_env}; struct App { @@ -367,7 +367,7 @@ fn main() -> std::process::ExitCode { }; #[cfg(feature = "wgpu")] let crash = left_out.crash.clone(); - let custom_titlebar = custom_titlebar(saved.as_ref()); + let (custom_titlebar, caption_layout) = custom_titlebar(saved.as_ref()); let options = native_options(custom_titlebar); // winit picks Wayland whenever `WAYLAND_DISPLAY` is set; Preferences › Performance › Linux // Display Server › X11 opens the window through XWayland instead, where file drops work (#1151), @@ -593,6 +593,7 @@ fn main() -> std::process::ExitCode { } app.titlebar_from_env = titlebar_from_env(); app.custom_titlebar = custom_titlebar; + app.caption_layout = caption_layout; app.desktop_frame = !custom_titlebar; if let Some(port) = control_port { let rx = control_server::start(port, cc.egui_ctx.clone()); diff --git a/apps/vectorcraft/src/tests.rs b/apps/vectorcraft/src/tests.rs index 6ae80944a6..99ca880bc9 100644 --- a/apps/vectorcraft/src/tests.rs +++ b/apps/vectorcraft/src/tests.rs @@ -220,8 +220,10 @@ fn the_app_title_bar_preference_gives_the_app_frame_on_desktop_frames() { } /// The frame a start uses: the preference turns the desktop frame the probe chose into the app frame -/// with the default window buttons, a probed app frame keeps its own buttons, and the desktop frame -/// stays where the app frame is not drawn. System Title Bar takes precedence over the preference. +/// with the default window buttons, an app frame the desktop left to the app keeps its own buttons, +/// one that `VECTORCRAFT_TITLEBAR=app` forced or the preference kept through XWayland gets the +/// default buttons, and the desktop frame stays +/// where the app frame is not drawn. System Title Bar takes precedence over the preference. #[test] fn the_start_frame_applies_the_app_title_bar_preference() { use craft_window::{Button, Frame, Layout}; @@ -233,9 +235,10 @@ fn the_start_frame_applies_the_app_title_bar_preference() { ((Frame::Desktop, None, true, true), Frame::App(Layout::default())), ((Frame::App(gnome.clone()), None, false, false), Frame::App(gnome.clone())), ((Frame::App(gnome.clone()), None, true, false), Frame::Desktop), - ((Frame::App(gnome.clone()), None, true, true), Frame::App(gnome.clone())), + ((Frame::App(gnome.clone()), None, true, true), Frame::App(Layout::default())), ((Frame::Desktop, Some(false), false, true), Frame::Desktop), - ((Frame::App(gnome.clone()), Some(true), true, false), Frame::App(gnome.clone())), + ((Frame::App(gnome.clone()), Some(true), true, false), Frame::App(Layout::default())), + ((Frame::App(gnome.clone()), Some(true), false, false), Frame::App(Layout::default())), ]; for ((probed, overridden, forced_to_x11, opted_into_app), expected) in cases { let label = format!("{probed:?} {overridden:?} {forced_to_x11} {opted_into_app}"); @@ -246,6 +249,30 @@ fn the_start_frame_applies_the_app_title_bar_preference() { assert!(!super::custom_titlebar_from(&both, true), "System Title Bar takes precedence over the app title bar preference"); } +/// The app bar's caption buttons follow the window-button layout of the app frame on Linux and the +/// BSDs (Close alone at the right, or all three at the left with Close outermost); the desktop frame, +/// macOS and Windows keep all three at the right. +#[test] +fn the_caption_buttons_follow_the_frame_layout() { + use craft_window::{Button, Frame, Layout}; + use vectorcraft_ui_egui::titlebar::{Caption, CaptionLayout}; + let close_only = Layout::parse("appmenu:close"); + let at_left = Layout { left: vec![Button::Close, Button::Minimize, Button::Maximize], right: vec![] }; + let split = Layout { left: vec![Button::Close], right: vec![Button::Minimize, Button::Maximize] }; + let probed = [ + (close_only, CaptionLayout { left: vec![], right: vec![Caption::Close] }), + (at_left, CaptionLayout { left: vec![Caption::Close, Caption::Minimize, Caption::Maximize], right: vec![] }), + (split, CaptionLayout { left: vec![Caption::Close], right: vec![Caption::Minimize, Caption::Maximize] }), + ]; + for (layout, expected) in probed { + let shown = super::caption_layout_from(&Frame::App(layout.clone())); + let expected = if cfg!(any(target_os = "macos", target_os = "windows")) { CaptionLayout::default() } else { expected }; + assert_eq!(shown, expected, "{layout:?}"); + } + assert_eq!(super::caption_layout_from(&Frame::Desktop), CaptionLayout::default()); + assert_eq!(CaptionLayout::default().right, vec![Caption::Minimize, Caption::Maximize, Caption::Close]); +} + /// `VECTORCRAFT_TITLEBAR` chooses the frame for the run it is set in, with System Title Bar off and /// on. The saved preference still decides while the variable is unset. #[test] @@ -303,7 +330,7 @@ fn the_titlebar_variable_decides_the_frame_before_the_window_opens() { let off = ui_state_with_prefs(serde_json::json!({"systemTitleBar": false})); let on = ui_state_with_prefs(serde_json::json!({"systemTitleBar": true})); for (saved, opted_into_system) in [(None, false), (Some(&off), false), (Some(&on), true)] { - let custom = super::custom_titlebar(saved); + let (custom, _) = super::custom_titlebar(saved); let expected = match value.as_str() { _ if not_read => !cfg!(target_os = "macos") && !opted_into_system, "app" => true, @@ -312,7 +339,7 @@ fn the_titlebar_variable_decides_the_frame_before_the_window_opens() { // `craft-window` probe decides, the same with System Title Bar off as with no saved // preferences. _ if opted_into_system => false, - _ => super::custom_titlebar(None), + _ => super::custom_titlebar(None).0, }; assert_eq!(custom, expected, "{value}, preference {opted_into_system}"); assert_eq!(super::native_options(custom).viewport.decorations, Some(!custom), "{value}, preference {opted_into_system}"); diff --git a/apps/vectorcraft/src/titlebar.rs b/apps/vectorcraft/src/titlebar.rs index fc6e395927..88dce26897 100644 --- a/apps/vectorcraft/src/titlebar.rs +++ b/apps/vectorcraft/src/titlebar.rs @@ -31,7 +31,7 @@ pub(crate) fn adapter_summary(info: &eframe::wgpu::AdapterInfo) -> String { format!("{} ({:?}, {:?})", info.name.trim(), info.backend, info.device_type) } -/// Windows and Linux: no OS title bar; the app bar is the title bar (`vectorcraft_ui_egui::titlebar`). +/// Windows: no OS title bar; the app bar is the title bar (`vectorcraft_ui_egui::titlebar`). /// macOS keeps its traffic lights over the integrated title strip. /// /// Used on macOS and Windows (where VectorCraft keeps today's frame regardless of the @@ -68,18 +68,45 @@ pub(crate) fn app_draws_frame(probed_app: bool, overridden: Option, forced } /// The frame this start uses, before the System Title Bar preference is applied -/// ([`custom_titlebar_from`]): [`app_draws_frame`] applied to the frame the probe chose. A probed -/// app frame keeps its window-button layout; an app frame the probe did not choose gets the default -/// layout, since `custom_titlebar_from` reads only which frame it is. +/// ([`custom_titlebar_from`]): [`app_draws_frame`] applied to the frame the probe chose. An app frame +/// the desktop left to the app (no `VECTORCRAFT_TITLEBAR` override, not forced onto XWayland) keeps +/// its window-button layout. An app frame turned on by `VECTORCRAFT_TITLEBAR=app` or by VectorCraft +/// Title Bar on Every Desktop gets the default layout: on those desktops the `button-layout` key that `craft-window` reads can +/// hold its schema default (`appmenu:close`) rather than the desktop's own buttons. pub(crate) fn start_frame(probed: craft_window::Frame, overridden: Option, forced_to_x11: bool, opted_into_app: bool) -> craft_window::Frame { let probed_app = matches!(probed, craft_window::Frame::App(_)); match (app_draws_frame(probed_app, overridden, forced_to_x11, opted_into_app), probed) { - (true, frame @ craft_window::Frame::App(_)) => frame, - (true, craft_window::Frame::Desktop) => craft_window::Frame::App(craft_window::Layout::default()), + (true, frame @ craft_window::Frame::App(_)) if overridden.is_none() && !forced_to_x11 => frame, + (true, _) => craft_window::Frame::App(craft_window::Layout::default()), (false, _) => craft_window::Frame::Desktop, } } +/// Where the app bar puts its caption buttons for `frame`: on Linux and the BSDs, the window-button +/// layout of an app frame, which `craft-window` reads from the desktop (the `button-layout` key of +/// `org.gnome.desktop.wm.preferences`); otherwise all three at the right. Each list is copied in the +/// key's order, first item leftmost, so `close,minimize,maximize:` puts Close at the left edge. +#[cfg(not(any(target_os = "macos", target_os = "windows")))] +pub(crate) fn caption_layout_from(frame: &craft_window::Frame) -> vectorcraft_ui_egui::titlebar::CaptionLayout { + use vectorcraft_ui_egui::titlebar::{Caption, CaptionLayout}; + let caption = |b: &craft_window::Button| match b { + craft_window::Button::Minimize => Caption::Minimize, + craft_window::Button::Maximize => Caption::Maximize, + craft_window::Button::Close => Caption::Close, + }; + match frame.layout() { + Some(layout) => CaptionLayout { left: layout.left.iter().map(caption).collect(), right: layout.right.iter().map(caption).collect() }, + None => CaptionLayout::default(), + } +} + +/// macOS and Windows: all three caption buttons at the right, whatever the probe says (see +/// [`custom_titlebar_from`]). +#[cfg(any(target_os = "macos", target_os = "windows"))] +pub(crate) fn caption_layout_from(_frame: &craft_window::Frame) -> vectorcraft_ui_egui::titlebar::CaptionLayout { + vectorcraft_ui_egui::titlebar::CaptionLayout::default() +} + /// The `craft-window` probe's answer for this process (its override variable is /// `VECTORCRAFT_TITLEBAR`). The probe runs once; on a Wayland session it opens a connection. fn probed_choice() -> &'static craft_window::Choice { @@ -131,7 +158,8 @@ pub(crate) fn custom_titlebar_from(_frame: &craft_window::Frame, opted_into_syst CUSTOM_TITLEBAR && !opted_into_system } -/// Whether this start draws its own title bar. Probes the shared `craft-window` crate once (its +/// Whether this start draws its own title bar, and where its caption buttons go +/// ([`caption_layout_from`]). Probes the shared `craft-window` crate once (its /// override variable is `VECTORCRAFT_TITLEBAR`) and combines its decision with the saved System /// Title Bar preference, which does not apply while the variable is set to `app`, `system` or /// `desktop`. Call once at startup, @@ -142,7 +170,7 @@ pub(crate) fn custom_titlebar_from(_frame: &craft_window::Frame, opted_into_syst /// on X11, whose window manager draws the frame — so when XWayland is being forced, keep the native /// frame, the same as a real X11 session. A recognized `VECTORCRAFT_TITLEBAR` override still wins, /// and so does VectorCraft Title Bar on Every Desktop ([`app_draws_frame`]). -pub(crate) fn custom_titlebar(saved: Option<&vectorcraft_ui_egui::UiState>) -> bool { +pub(crate) fn custom_titlebar(saved: Option<&vectorcraft_ui_egui::UiState>) -> (bool, vectorcraft_ui_egui::titlebar::CaptionLayout) { let choice = probed_choice(); let opted_into_system = system_title_bar_applies(opted_into_system_title_bar(saved), titlebar_from_env()); // macOS and Windows never read VectorCraft Title Bar on Every Desktop (`custom_titlebar_from` @@ -160,7 +188,7 @@ pub(crate) fn custom_titlebar(saved: Option<&vectorcraft_ui_egui::UiState>) -> b #[cfg(not(target_os = "linux"))] let forced_to_x11 = false; let frame = start_frame(choice.frame().clone(), choice.overridden(), forced_to_x11, opted_into_app); - custom_titlebar_from(&frame, opted_into_system) + (custom_titlebar_from(&frame, opted_into_system), caption_layout_from(&frame)) } pub(crate) fn native_options(custom_titlebar: bool) -> eframe::NativeOptions { diff --git a/crates/brush/src/calli.rs b/crates/brush/src/calli.rs index 0b679497ce..888294356c 100644 --- a/crates/brush/src/calli.rs +++ b/crates/brush/src/calli.rs @@ -78,7 +78,11 @@ fn samples(t: &Track, step: Option) -> Vec<(Point, Vec2, f64)> { let pieces = (seg / step).ceil().clamp(1.0, MAX_STEPS) as usize; for k in 1..pieces { let u = k as f64 / pieces as f64; - out.push((p.lerp(t.verts[i + 1], u), t.dirs[i], (t.cum[i] + seg * u) / len)); + let s = t.cum[i] + seg * u; + // On a chord of a flattened curve the samples follow the smooth frame through the + // vertices. + let (q, tg) = if t.curved.get(i).copied().unwrap_or(false) { t.frame_in(i, u) } else { (p.lerp(t.verts[i + 1], u), t.dirs[i]) }; + out.push((q, tg, s / len)); } } } diff --git a/crates/brush/src/tests.rs b/crates/brush/src/tests.rs index 2d5156d162..2d62a1a993 100644 --- a/crates/brush/src/tests.rs +++ b/crates/brush/src/tests.rs @@ -1037,6 +1037,26 @@ fn pressed(points: Vec<(f64, f64)>) -> StrokeLayer { StrokeLayer { pressure: Some(vectorcraft_doc::PressureProfile { points }), ..stroke(1.0) } } +/// A nib that follows the pen is sampled between the vertices of the flattened path too. Along a +/// curve those samples sit on the curve, and the outline keeps its distance from it. +#[test] +fn pen_driven_nib_outline_follows_the_curve_between_vertices() { + // The top half of a circle of radius 50; a round nib 20 pt across, the same at any pressure. + let (r, half) = (50.0, 10.0); + let c = Point::new(r, r); + let ring = shapes::ellipse(Rect::new(0.0, 0.0, 2.0 * r, 2.0 * r)).to_bezpath(); + let half_circle: BezPath = ring.elements().iter().take(3).copied().collect(); + let out = stroke_pieces(&Document::new(1.0, 1.0), &pressure_calli(20.0, 0.0), &half_circle, &pressed(vec![(0.0, 0.5), (1.0, 0.5)])); + let mut pts = vec![]; + for n in &out { + all_points(n, &mut pts); + } + // Away from the round caps at the two ends, every outline point is `half` from the curve. + let middle = pts.iter().filter(|p| ((p.y - c.y).atan2(p.x - c.x).abs() - std::f64::consts::FRAC_PI_2).abs() <= 1.2); + let worst = middle.map(|p| ((p.distance(c) - (r + half)).abs()).min((p.distance(c) - (r - half)).abs())).fold(0.0, f64::max); + assert!(worst < 0.05, "an outline point strays {worst:.4} pt from the nib's reach"); +} + /// #852: with Size on Pressure the nib goes from size − variation at the lightest pressure to /// size + variation at the heaviest, along the stroke where the pen pressed. #[test] @@ -1686,6 +1706,35 @@ fn an_ordinary_stroke_is_unaffected_by_the_budget() { assert!(anchors(&out) < 2_000_000); } +/// A straight-sided art brush on a smooth curve draws smooth edges: they do not turn at the smooth +/// anchors the fit puts along them. +#[test] +fn art_brush_edges_stay_smooth_along_a_curve() { + for &(r, w, h, sw) in &[(100.0, 100.0, 10.0, 1.0), (300.0, 100.0, 10.0, 2.0), (30.0, 100.0, 10.0, 1.0), (2000.0, 100.0, 10.0, 1.0)] { + // The top half of a circle, as one open path of two curves. + let ring = shapes::ellipse(Rect::new(0.0, 0.0, 2.0 * r, 2.0 * r)).to_bezpath(); + let half_circle: BezPath = ring.elements().iter().take(3).copied().collect(); + let out = stroke_pieces(&Document::new(1.0, 1.0), &art_brush(rect_art(w, h), ArtScale::Stretch), &half_circle, &stroke(sw)); + let NodeKind::Path { path, .. } = &out[0].kind else { panic!("an art brush on a path draws a path") }; + + // The turn at each smooth anchor of the edges: the angle between the incoming and the + // outgoing direction. Real corners (the ends of the art) turn by far more than 60°. + let mut kink = 0.0f64; + for sp in &path.subpaths { + for (k, a) in sp.anchors.iter().enumerate().skip(1).take(sp.anchors.len().saturating_sub(2)) { + let h_in = if a.h_in != a.p { a.h_in } else { sp.anchors[k - 1].p }; + let h_out = if a.h_out != a.p { a.h_out } else { sp.anchors[k + 1].p }; + let (u, v) = (a.p - h_in, h_out - a.p); + if u.hypot() < 1e-9 || v.hypot() < 1e-9 || u.dot(v) < 0.5 * u.hypot() * v.hypot() { + continue; + } + kink = kink.max(u.cross(v).atan2(u.dot(v)).abs().to_degrees()); + } + } + assert!(kink < 0.5, "r={r} art={w}x{h} weight {sw}: an edge turns by {kink:.3}° at a smooth anchor"); + } +} + /// Anchors under `n`, and how many of them keep a handle (so the segment is a curve). fn anchors_and_curved(n: &Node) -> (usize, usize) { let NodeKind::Path { path, .. } = &n.kind else { return (0, 0) }; diff --git a/crates/brush/src/track.rs b/crates/brush/src/track.rs index 6c65246176..3b5fb1add3 100644 --- a/crates/brush/src/track.rs +++ b/crates/brush/src/track.rs @@ -1,8 +1,9 @@ //! Arc-length parameterised polylines ("tracks") that brushes lay art along. //! //! A track is one flattened subpath. `frame(s)` gives the point and unit tangent at arc length -//! `s`; tangents are blended across the gentle vertices of flattened curves (so warped art bends -//! smoothly) but stay discontinuous at real corners (so art folds there, like Illustrator). +//! `s`. Along a flattened curve the frame runs on a smooth curve through the vertices, so warped +//! art bends smoothly; along a line of the path it stays on the line. At real corners the tangent +//! stays discontinuous, so art folds there. use vectorcraft_geom::{BezPath, PathEl, Point, Vec2}; @@ -22,6 +23,11 @@ pub struct Track { pub tangents: Vec, /// Is the vertex a corner (turn > [`CORNER_DEG`])? Open ends are never corners. pub corner: Vec, + /// Does the segment come from flattening a curve (rather than from a line of the path)? + pub curved: Vec, + /// How far (radians) a curved segment's end tangents may turn from its direction for `frame` + /// to follow a curve through them; 0 for a line. + turn: Vec, } fn unit(v: Vec2) -> Vec2 { @@ -35,16 +41,53 @@ fn aligned(a: Vec2, b: Vec2) -> bool { a.dot(b) > 1.0 - 1e-12 } +/// How far a vertex tangent may turn from a chord of a flattened curve, as a multiple of the +/// flattening tolerance over the chord's length. A circular arc needs 4; the rest is slack for +/// flattening that lands a little inside its tolerance. +const TURN_PER_TOL: f64 = 6.0; + +/// `t` turned by the smallest angle that brings it within `limit` radians of each `dir`. When no +/// angle satisfies every limit, `t` turns to the middle of the gap between them. +fn held_within(t: Vec2, limits: [Option<(Vec2, f64)>; 2]) -> Vec2 { + let (mut lo, mut hi) = (f64::NEG_INFINITY, f64::INFINITY); + for (d, limit) in limits.into_iter().flatten() { + let to = t.cross(d).atan2(t.dot(d)); + lo = lo.max(to - limit); + hi = hi.min(to + limit); + } + let turn = if lo <= hi { 0f64.max(lo).min(hi) } else { (lo + hi) / 2.0 }; + if turn == 0.0 || !turn.is_finite() { + return t; + } + let (sin, cos) = turn.sin_cos(); + Vec2::new(t.x * cos - t.y * sin, t.x * sin + t.y * cos) +} + /// Left-hand normal in y-down document space (for a path running +x it points +y). pub fn normal(t: Vec2) -> Vec2 { Vec2::new(-t.y, t.x) } impl Track { - fn new(mut pts: Vec, closed: bool) -> Option { - pts.dedup_by(|a, b| a.distance(*b) < 1e-9); + /// `raw` pairs each vertex with the tolerance the segment ending at it was flattened with + /// when it comes from a curve, and `None` when it is a line of the path. + fn new(raw: Vec<(Point, Option)>, closed: bool) -> Option { + let mut pts: Vec = Vec::with_capacity(raw.len()); + // `ends_curve[k]`: the flattening tolerance of the segment ending at vertex `k`, when it + // comes from a curve. + let mut ends_curve: Vec> = Vec::with_capacity(raw.len()); + for (p, c) in raw { + // A duplicate vertex adds no segment, so it is dropped with its tolerance. + if pts.last().is_some_and(|q| q.distance(p) < 1e-9) { + continue; + } + pts.push(p); + ends_curve.push(c); + } + let mut closing_curve = None; if closed && pts.len() > 2 && pts[0].distance(pts[pts.len() - 1]) < 1e-9 { pts.pop(); + closing_curve = ends_curve.pop().flatten(); } if pts.len() < 2 { return None; @@ -52,8 +95,11 @@ impl Track { let closed = closed && pts.len() > 2; if closed { pts.push(pts[0]); + ends_curve.push(closing_curve); } let nseg = pts.len() - 1; + let curve_tol: Vec> = (0..nseg).map(|i| ends_curve.get(i + 1).copied().flatten()).collect(); + let curved: Vec = curve_tol.iter().map(Option::is_some).collect(); let dirs: Vec = (0..nseg).map(|i| unit(pts[i + 1] - pts[i])).collect(); let mut cum = vec![0.0]; for i in 0..nseg { @@ -62,34 +108,78 @@ impl Track { let mut tangents = Vec::with_capacity(nseg + 1); let mut corner = Vec::with_capacity(nseg + 1); let cos_limit = CORNER_DEG.to_radians().cos(); + let seg = |i: usize| (dirs[i], cum[i + 1] - cum[i], curved[i]); for i in 0..=nseg { let incoming = if i > 0 { - Some(dirs[i - 1]) + Some(seg(i - 1)) } else if closed { - Some(dirs[nseg - 1]) + Some(seg(nseg - 1)) } else { None }; let outgoing = if i < nseg { - Some(dirs[i]) + Some(seg(i)) } else if closed { - Some(dirs[0]) + Some(seg(0)) } else { None }; match (incoming, outgoing) { - (Some(a), Some(b)) => { + (Some((a, la, ca)), Some((b, lb, cb))) => { corner.push(a.dot(b) < cos_limit); - tangents.push(unit(a + b)); + tangents.push(match (ca, cb) { + // Between two chords of a flattened curve: each direction weighted by the + // length of the other chord, which is the tangent of the circle through + // the vertex and its two neighbors. + (true, true) => unit(a * lb + b * la), + // Where a line of the path meets a curve, the line's direction. + (false, true) => a, + (true, false) => b, + (false, false) => unit(a + b), + }); } (a, b) => { // An end of an open track (both `None` only for a track without segments). corner.push(false); - tangents.push(a.or(b).unwrap_or(Vec2::new(1.0, 0.0))); + tangents.push(a.or(b).map_or(Vec2::new(1.0, 0.0), |(d, _, _)| d)); } } } - Some(Track { verts: pts, cum, closed, dirs, tangents, corner }) + // An open track's end on a curve continues the bend of its last chord: the tangent there + // is the neighboring vertex's tangent mirrored about the chord, as on the circle through + // the last three vertices. + if !closed && nseg >= 2 { + for (end, inner, d) in [(0, 1, dirs[0]), (nseg, nseg - 1, dirs[nseg - 1])] { + let (s_end, s_inner) = if end == 0 { (0, 1) } else { (nseg - 1, nseg - 2) }; + if !corner[inner] && curved[s_end] && curved[s_inner] { + let t = tangents[inner]; + tangents[end] = unit(d * (2.0 * d.dot(t)) - t); + } + } + } + // A curve lies within its flattening tolerance `tol` of each of its chords, so its tangent + // at a vertex turns only a little from a chord: at most about `4 * tol / len` on a + // circular arc. Next to a long chord the estimates above can turn much further (a + // straight curve element beside a tight bend), and `frame` would then bow off the chord. + // Between curved chords, each vertex tangent is held within `TURN_PER_TOL * tol / len` of + // both chords. Where a line meets a curve the tangent stays the line's direction, and a + // curved chord whose end tangents turn further than that from it keeps to the chord. + let turn: Vec = (0..nseg) + .map(|j| match curve_tol.get(j) { + Some(Some(tol)) => (TURN_PER_TOL * tol / (cum[j + 1] - cum[j]).max(1e-12)).min(1.0).asin(), + _ => 0.0, + }) + .collect(); + for (i, t) in tangents.iter_mut().enumerate() { + let incoming = if i > 0 { Some(i - 1) } else { closed.then_some(nseg - 1) }; + let outgoing = if i < nseg { Some(i) } else { closed.then_some(0) }; + let sides = [incoming, outgoing]; + if corner[i] || sides.iter().flatten().any(|&j| !curved[j]) { + continue; + } + *t = held_within(*t, sides.map(|j| j.map(|j| (dirs[j], turn[j])))); + } + Some(Track { verts: pts, cum, closed, dirs, tangents, corner, curved, turn }) } pub fn len(&self) -> f64 { @@ -135,11 +225,11 @@ impl Track { let n = self.segments(); let s = if self.closed && l > 0.0 { s.rem_euclid(l) } else { s }; if s <= 0.0 && !self.closed { - let t = self.dirs[0]; + let t = self.tangents[0]; return (self.verts[0] + t * s, t); } if s >= l && !self.closed { - let t = self.dirs[n - 1]; + let t = self.tangents[n]; return (self.verts[n] + t * (s - l), t); } // Binary search the segment. @@ -149,10 +239,44 @@ impl Track { }; let seg = self.cum[i + 1] - self.cum[i]; let u = if seg > 1e-12 { ((s - self.cum[i]) / seg).clamp(0.0, 1.0) } else { 0.0 }; - let p = self.verts[i].lerp(self.verts[i + 1], u); - let t0 = if self.corner[i] { self.dirs[i] } else { self.tangents[i] }; - let t1 = if self.corner[i + 1] { self.dirs[i] } else { self.tangents[i + 1] }; - (p, unit(t0 + (t1 - t0) * u)) + self.frame_in(i, u) + } + + /// Segment `i`'s direction and the tangents [`Self::frame`] uses at its two ends: the vertex + /// tangents, or the segment's direction at a corner. + fn end_tangents(&self, i: usize) -> Option<(Vec2, Vec2, Vec2)> { + let (&d, &c0, &c1, &t0, &t1) = + (self.dirs.get(i)?, self.corner.get(i)?, self.corner.get(i + 1)?, self.tangents.get(i)?, self.tangents.get(i + 1)?); + Some((d, if c0 { d } else { t0 }, if c1 { d } else { t1 })) + } + + /// Point and unit tangent at the fraction `u` (0 to 1) of segment `i`, as [`Self::frame`] + /// gives them. `i` must be below [`Self::segments`]. + pub fn frame_in(&self, i: usize, u: f64) -> (Point, Vec2) { + let (Some(&p0), Some(&p1), Some((d, t0, t1))) = (self.verts.get(i), self.verts.get(i + 1), self.end_tangents(i)) else { + return (self.verts.last().copied().unwrap_or(Point::ZERO), Vec2::new(1.0, 0.0)); + }; + let seg = self.cum.get(i + 1).zip(self.cum.get(i)).map_or(0.0, |(b, a)| b - a); + if aligned(t0, d) && aligned(t1, d) { + return (p0.lerp(p1, u), d); + } + // The tangents may turn this far from a curved chord, a little beyond it for rounding. + let within = self.turn.get(i).map_or(-1.0, |a| a.sin() + 1e-9); + let bends = |t: Vec2| t.dot(d) <= 0.0 || t.cross(d).abs() > within; + if !self.curved.get(i).copied().unwrap_or(false) || bends(t0) || bends(t1) { + // A line of the path, or a curved chord at a sharper turn than the curve can make + // there: the point stays on it, and the tangent turns across it toward the vertex + // tangents. + return (p0.lerp(p1, u), unit(t0 + (t1 - t0) * u)); + } + // On a chord of a flattened curve the frame follows the cubic through both vertices with + // their tangents, at the chord's length per unit of `u`. Its position and tangent then + // meet those of the next segment at each vertex. + let (u2, u3) = (u * u, u * u * u); + let (m0, m1) = (t0 * seg, t1 * seg); + let p = p0 + (p1 - p0) * (3.0 * u2 - 2.0 * u3) + m0 * (u3 - 2.0 * u2 + u) + m1 * (u3 - u2); + let dp = (p1 - p0) * (6.0 * u - 6.0 * u2) + m0 * (3.0 * u2 - 4.0 * u + 1.0) + m1 * (3.0 * u2 - 2.0 * u); + (p, unit(dp)) } /// The point at arc length `s`, offset `d` along the left normal. @@ -183,8 +307,8 @@ impl Track { if self.closed && (lo < -1e-9 || hi > l + 1e-9) { return false; } - // Past an open track's ends `frame` extrapolates along the end segment's direction, so - // those segments are part of the span too. + // Past an open track's ends `frame` extrapolates along the end tangent, which is checked + // against the end segment's direction below, so those segments are part of the span too. let ends = [(!self.closed && lo < 0.0).then_some(0), (!self.closed && hi > l).then(|| n - 1)]; let touched = (0..n).filter(|&i| { let (a, b) = (self.cum.get(i), self.cum.get(i + 1)); @@ -193,9 +317,7 @@ impl Track { let mut dir: Option = None; let mut any = false; for i in touched.chain(ends.into_iter().flatten()) { - let (Some(&d), Some(&t0), Some(&t1), Some(&c0), Some(&c1)) = - (self.dirs.get(i), self.tangents.get(i), self.tangents.get(i + 1), self.corner.get(i), self.corner.get(i + 1)) - else { + let Some((d, t0, t1)) = self.end_tangents(i) else { return false; }; if dir.is_some_and(|first: Vec2| !aligned(first, d)) { @@ -203,11 +325,11 @@ impl Track { } dir = Some(d); any = true; - // `frame` blends the two vertex tangents across the segment unless the vertex is a - // corner, where it uses the segment's own direction. So a neighbour outside the span - // can still bend the tangent inside it, and checking the directions alone would miss - // that: segments d, d, e leave vertex 2's tangent at unit(d + e). - if !(c0 || aligned(t0, d)) || !(c1 || aligned(t1, d)) { + // `frame` blends the two end tangents across a line, and follows a curve through them + // on a curved chord. So a neighbour outside the span can still bend the tangent + // inside it, and checking the directions alone would miss that: segments d, d, e + // leave vertex 2's tangent at unit(d + e). + if !aligned(t0, d) || !aligned(t1, d) { return false; } } @@ -220,9 +342,10 @@ const MAX_FLATTENED: usize = 4_000_000; /// Flatten `bp` into tracks (one per subpath) with `tol` accuracy. pub fn tracks(bp: &BezPath, tol: f64) -> Vec { + let tol = tol.max(1e-4); let mut out = vec![]; - let mut cur: Vec = vec![]; - let flush = |cur: &mut Vec, closed: bool, out: &mut Vec| { + let mut cur: Vec<(Point, Option)> = vec![]; + let flush = |cur: &mut Vec<(Point, Option)>, closed: bool, out: &mut Vec| { if let Some(t) = Track::new(std::mem::take(cur), closed) { out.push(t); } @@ -236,19 +359,40 @@ pub fn tracks(bp: &BezPath, tol: f64) -> Vec { return out; } let mut els = vec![]; + // When the element being flattened is a curve, the tolerance `flatten` uses for it: `flatten` + // pulls each element before it reports that element's lines, and raises the tolerance of a + // long curve as `vectorcraft_geom::flatten` describes. + let in_curve = std::cell::Cell::new(None); + let (last, start) = (std::cell::Cell::new(Point::ZERO), std::cell::Cell::new(Point::ZERO)); + let source = bp.iter().inspect(|el| { + let from = last.get(); + let (curve, to) = match *el { + PathEl::MoveTo(p) => { + start.set(p); + (None, p) + } + PathEl::LineTo(p) => (None, p), + PathEl::QuadTo(a, p) => (Some(from.distance(a) + a.distance(p)), p), + PathEl::CurveTo(a, b, p) => (Some(from.distance(a) + a.distance(b) + b.distance(p)), p), + PathEl::ClosePath => (None, start.get()), + }; + last.set(to); + let per = vectorcraft_geom::MAX_FLATTEN_PER_CURVE * vectorcraft_geom::MAX_FLATTEN_PER_CURVE; + in_curve.set(curve.map(|length| if length.is_finite() { tol.max(length / per) } else { f64::INFINITY })); + }); // Guarded: kurbo works out every subdivision of a curve even when the callback drops them. - vectorcraft_geom::flatten(bp.iter(), tol.max(1e-4), |el| { + vectorcraft_geom::flatten(source, tol, |el| { if els.len() < MAX_FLATTENED { - els.push(el); + els.push((el, in_curve.get())); } }); - for el in els { + for (el, from_curve) in els { match el { PathEl::MoveTo(p) => { flush(&mut cur, false, &mut out); - cur.push(p); + cur.push((p, None)); } - PathEl::LineTo(p) => cur.push(p), + PathEl::LineTo(p) => cur.push((p, from_curve)), PathEl::ClosePath => flush(&mut cur, true, &mut out), _ => {} } @@ -368,6 +512,173 @@ mod tests { assert_eq!(t.map(110.0, 0.0), Point::new(110.0, 0.0)); } + /// Between two lines that meet at a gentle turn, the frame stays on the lines: only the chords + /// of a flattened curve are smoothed. Where a line meets a curve at a gentle turn, the frame + /// stays on the line with the line's direction, and its tangent does not jump at the join. + #[test] + fn a_polylines_frame_stays_on_its_lines() { + // A line, then a line or a nearly straight curve at a 20° turn, below the corner angle, so + // the vertex is not a corner. + let turn = 20f64.to_radians(); + let (c, s) = (100.0 * turn.cos(), 100.0 * turn.sin()); + for curve in [false, true] { + let mut bp = BezPath::new(); + bp.move_to((0.0, 0.0)); + bp.line_to((100.0, 0.0)); + if curve { + bp.curve_to((100.0 + c / 3.0, s / 3.0 + 0.5), (100.0 + 2.0 * c / 3.0, 2.0 * s / 3.0 + 0.5), (100.0 + c, s)); + } else { + bp.line_to((100.0 + c, s)); + } + let t = &tracks(&bp, 0.05)[0]; + for at in [10.0, 30.0, 50.0, 70.0, 90.0] { + let (p, tg) = t.frame(at); + assert!(p.y.abs() < 1e-9 && (p.x - at).abs() < 1e-9, "curve {curve}: at {at} the frame left the first line: {p:?}"); + if curve { + assert!(tg.y.abs() < 1e-12, "at {at} the line's tangent turned toward the curve: {tg:?}"); + } + } + if curve { + // The curve's first chord keeps to the chord rather than bending toward the line. + let (a, b) = (t.cum[1], t.cum[2]); + let (p, q) = (t.verts[1], t.verts[2]); + let mid = t.frame((a + b) / 2.0).0; + assert!((mid - p.midpoint(q)).hypot() < 1e-9, "the first curve chord bows: {mid:?}"); + } + let (before, after) = (t.frame(100.0 - 1e-7).1, t.frame(100.0 + 1e-7).1); + assert!(before.dot(after) > 1.0 - 1e-9, "curve {curve}: the tangent jumps at the join: {before:?} {after:?}"); + } + } + + /// A curve of no length between two lines leaves them lines. + #[test] + fn a_curve_of_no_length_leaves_a_line_a_line() { + let turn = 20f64.to_radians(); + let mut bp = BezPath::new(); + bp.move_to((0.0, 0.0)); + bp.line_to((100.0, 0.0)); + bp.curve_to((100.0, 0.0), (100.0, 0.0), (100.0, 0.0)); + bp.line_to((100.0 + 100.0 * turn.cos(), 100.0 * turn.sin())); + let t = &tracks(&bp, 0.05)[0]; + assert!(t.curved.iter().all(|c| !c), "no segment is a curve: {:?}", t.curved); + for at in [10.0, 50.0, 90.0] { + let (p, _) = t.frame(at); + assert!(p.y.abs() < 1e-9 && (p.x - at).abs() < 1e-9, "at {at} the frame left the first line: {p:?}"); + } + } + + /// Where a line joins a curve smoothly, the line keeps its direction up to the join. + #[test] + fn a_line_into_a_smooth_curve_keeps_its_direction() { + // A line along y = 0 into a quarter circle of radius 50 that starts along +x. + let r = 50.0; + let mut bp = BezPath::new(); + bp.move_to((-100.0, 0.0)); + bp.line_to((0.0, 0.0)); + bp.curve_to((r * K, 0.0), (r, r - r * K), (r, r)); + let t = &tracks(&bp, 0.05)[0]; + for s in [10.0, 50.0, 90.0, 99.0] { + let (p, tg) = t.frame(s); + assert!(p.y.abs() < 1e-9 && tg.y.abs() < 1e-9, "at {s} the line's frame turned: {p:?} {tg:?}"); + } + } + + /// The handle length of a cubic quarter circle. + const K: f64 = 0.552_284_749_8; + + /// A curve element whose handles lie on its chord is a line drawn as a curve. Next to a tight + /// bend, at either end of a track, the frame along it stays within the flattening tolerance + /// of the line. + #[test] + fn a_straight_curve_element_beside_a_tight_bend_stays_on_its_line() { + let tol = 0.05; + for bend_first in [false, true] { + // A 200 pt line along y = 0 drawn as a cubic, joined smoothly to a quarter circle of + // radius 5 that turns toward +y. + let mut bp = BezPath::new(); + if bend_first { + bp.move_to((-5.0, -5.0)); + bp.curve_to((-5.0, -5.0 + 5.0 * K), (-5.0 * K, 0.0), (0.0, 0.0)); + bp.curve_to((50.0, 0.0), (150.0, 0.0), (200.0, 0.0)); + } else { + bp.move_to((0.0, 0.0)); + bp.curve_to((50.0, 0.0), (150.0, 0.0), (200.0, 0.0)); + bp.curve_to((200.0 + 5.0 * K, 0.0), (205.0, 5.0 - 5.0 * K), (205.0, 5.0)); + } + let t = &tracks(&bp, tol)[0]; + // The bend lies outside 0 <= x <= 200, so every frame point in that range is on the + // line's stretch of the track. + let on_line: Vec<(Point, Vec2)> = + (0..=2000).map(|k| t.frame(t.len() * k as f64 / 2000.0)).filter(|(p, _)| (0.0..=200.0).contains(&p.x)).collect(); + let worst = on_line.iter().map(|(p, _)| p.y.abs()).fold(0.0, f64::max); + assert!(worst <= 2.0 * tol, "bend first {bend_first}: the frame strays {worst:.4} pt off the line"); + // Along the line the tangent turns by no more than the line's flattening allows. + let lean = on_line.iter().map(|(_, tg)| tg.y.abs()).fold(0.0, f64::max); + assert!(lean <= 0.01, "bend first {bend_first}: the tangent turns {lean:.4} off the line"); + } + } + + /// A curve long enough that flattening raises its tolerance still has its frame follow the + /// curve between the vertices, not the chords. + #[test] + fn a_long_curve_flattened_coarser_is_still_followed() { + // A line along y = 0 into a cubic quarter circle of radius 5,000,000 that starts along +x. + let r = 5.0e6; + let curve = kurbo::CubicBez::new((0.0, 0.0), (r * K, 0.0), (r, r - r * K), (r, r)); + let mut bp = BezPath::new(); + bp.move_to((-100.0, 0.0)); + bp.line_to((0.0, 0.0)); + bp.curve_to(curve.p1, curve.p2, curve.p3); + let t = &tracks(&bp, 0.05)[0]; + let worst = (1..400) + .map(|k| t.frame(100.0 + (t.len() - 100.0) * k as f64 / 400.0).0) + .map(|p| kurbo::ParamCurveNearest::nearest(&curve, p, 1e-9).distance_sq.sqrt()) + .fold(0.0, f64::max); + assert!(worst < 0.05, "the frame strays {worst:.4} pt from the curve"); + } + + /// A closed curve is smooth across its seam: the frame stays on the circle and at right angles + /// to its radius all the way round, the start included. + #[test] + fn a_closed_curves_frame_is_smooth_across_its_seam() { + let (r, tol) = (50.0, 0.05); + let mut bp = BezPath::new(); + bp.move_to((r, 0.0)); + bp.curve_to((r, r * K), (r * K, r), (0.0, r)); + bp.curve_to((-r * K, r), (-r, r * K), (-r, 0.0)); + bp.curve_to((-r, -r * K), (-r * K, -r), (0.0, -r)); + bp.curve_to((r * K, -r), (r, -r * K), (r, 0.0)); + bp.close_path(); + let t = &tracks(&bp, tol)[0]; + assert!(t.closed && t.curved.iter().all(|c| *c), "every chord of the circle is a chord of a curve"); + for k in -20..=20 { + let (p, tg) = t.frame(k as f64 * 0.37); + let radial = p.to_vec2() / p.to_vec2().hypot(); + assert!((p.to_vec2().hypot() - r).abs() <= tol, "at {k} the frame is off the circle: {p:?}"); + assert!(tg.dot(radial).abs() < 0.5f64.to_radians().sin(), "at {k} the tangent leans off the circle: {tg:?}"); + } + } + + /// A closed path whose last side is the implicit line back to the start keeps that line + /// straight. + #[test] + fn an_implicit_closing_line_is_not_a_curve() { + let r = 50.0; + let mut bp = BezPath::new(); + bp.move_to((r, 0.0)); + bp.curve_to((r, r * K), (r * K, r), (0.0, r)); + bp.curve_to((-r * K, r), (-r, r * K), (-r, 0.0)); + bp.close_path(); + let t = &tracks(&bp, 0.05)[0]; + assert!(t.closed); + assert_eq!(t.curved.last(), Some(&false), "the closing line is a line"); + let n = t.segments(); + for u in [0.1, 0.5, 0.9] { + let (p, _) = t.frame(t.cum[n - 1] + (t.cum[n] - t.cum[n - 1]) * u); + assert!(p.y.abs() < 1e-9, "the frame leaves the closing line: {p:?}"); + } + } + #[test] fn closed_square_has_four_corners() { let mut bp = BezPath::new(); diff --git a/crates/doc/src/document_guides.rs b/crates/doc/src/document_guides.rs index 14d0404bfc..df336fe09e 100644 --- a/crates/doc/src/document_guides.rs +++ b/crates/doc/src/document_guides.rs @@ -183,8 +183,8 @@ impl Document { self.layers.retain(|l| !l.is_ruler_guide()); gone -= self.layers.len(); for l in &mut self.layers { - if layer_untidy(l) { - gone += tidy_layer(Arc::make_mut(l)); + if let Some(found) = untidy(l, &Node::is_ruler_guide) { + gone += tidy_layer(Arc::make_mut(l), found); } } let others = self @@ -198,8 +198,8 @@ impl Document { if root.is_ruler_guide() { *root = Arc::new(Node::group(root.id, vec![])); gone += 1; - } else if any_guide(root) { - gone += drop_guides_below(Arc::make_mut(root)); + } else if let Some(found) = guides_below(root, &Node::is_ruler_guide) { + gone += drop_guides_below(Arc::make_mut(root), found); } } gone @@ -272,8 +272,8 @@ impl Document { return; } for l in &mut self.layers { - if l.holds_ruler_guides() { - Arc::make_mut(l).strip_ruler_guides(); + if let Some(held) = picked_in_layers(l, &Node::is_ruler_guide) { + drop_picked(l, held); } } self.saved_guides.extend(guides); @@ -347,16 +347,14 @@ impl Document { /// saved guide whose layer is gone opens. Returns the removed node, without the guides. pub fn remove_edit_layer(&mut self, id: NodeId) -> Result, crate::DocError> { let mut layer = self.remove(id)?; - if !layer.holds_ruler_guides() { - return Ok(layer); - } + let Some(held) = picked_in_layers(&layer, &Node::is_ruler_guide) else { return Ok(layer) }; let mut guides = vec![]; layer.walk(&mut |c| { if c.is_ruler_guide() { guides.push(Arc::new(c.clone())); } }); - Arc::make_mut(&mut layer).strip_ruler_guides(); + drop_picked(&mut layer, held); let home = self.default_layer().unwrap_or_else(|| self.add_layer(None)); self.children_mut(Some(home))?.extend(guides); Ok(layer) @@ -418,21 +416,11 @@ impl Document { if gone.is_empty() { return 0; } - fn drop(layer: &mut Arc, gone: &HashSet) { - fn holds(n: &Node, gone: &HashSet) -> bool { - n.is_layer() && n.children().is_some_and(|ch| ch.iter().any(|c| gone.contains(&c.id) || holds(c, gone))) - } - if !holds(layer, gone) { - return; - } - let Some(ch) = Arc::make_mut(layer).children_mut() else { return }; - ch.retain(|c| !(c.is_ruler_guide() && gone.contains(&c.id))); - for c in ch.iter_mut().filter(|c| c.is_layer()) { - drop(c, gone); - } - } + let pick = |n: &Node| n.is_ruler_guide() && gone.contains(&n.id); for l in &mut self.layers { - drop(l, &gone); + if let Some(held) = picked_in_layers(l, &pick) { + drop_picked(l, held); + } } sel.guides.retain(|id| !gone.contains(id)); if sel.key_guide.is_some_and(|k| gone.contains(&k)) { @@ -444,29 +432,11 @@ impl Document { /// Change the line of every ruler guide on artboard `id` with `f` (whether it is vertical, its /// position). fn edit_artboard_guides(&mut self, id: u32, f: &mut impl FnMut(bool, &mut f64)) { - fn visit(layer: &mut Arc, id: u32, f: &mut impl FnMut(bool, &mut f64)) { - fn holds(n: &Node, id: u32) -> bool { - n.is_layer() - && n.children().is_some_and(|ch| { - ch.iter().any(|c| matches!(c.kind, NodeKind::RulerGuide { artboard: Some(a), .. } if a == id) || holds(c, id)) - }) - } - if !holds(layer, id) { - return; - } - let Some(ch) = Arc::make_mut(layer).children_mut() else { return }; - for c in ch.iter_mut() { - if matches!(c.kind, NodeKind::RulerGuide { artboard: Some(a), .. } if a == id) { - if let NodeKind::RulerGuide { vertical, pos, .. } = &mut Arc::make_mut(c).kind { - f(*vertical, pos); - } - } else if c.is_layer() { - visit(c, id, f); - } - } - } + let pick = |n: &Node| matches!(n.kind, NodeKind::RulerGuide { artboard: Some(a), .. } if a == id); for l in &mut self.layers { - visit(l, id, f); + if let Some(held) = picked_in_layers(l, &pick) { + edit_picked(l, held, f); + } } } @@ -580,65 +550,172 @@ impl Document { } } -/// Does `n`, or anything below it (opacity-mask art included), hold a ruler guide? -fn any_guide(n: &Node) -> bool { - let mut any = false; - crate::swatches::walk_with_masks(n, &mut |m| any |= m.is_ruler_guide()); - any +/// What a walk picked in a node: its children by index in order, each with what was picked +/// inside it, and what was picked in its opacity-mask art. Empty for a node picked itself. +#[derive(Debug, Default)] +struct Picked { + children: Vec<(usize, Picked)>, + mask: Option>, } -/// Remove every ruler guide below `n`, in its members and its opacity-mask art (`n` itself -/// stays). Mask art that is a ruler guide becomes an empty group. Returns how many went. -fn drop_guides_below(n: &mut Node) -> usize { - let mut gone = drop_mask_guides(n); - if let Some(ch) = n.children_mut() { - let before = ch.len(); - ch.retain(|c| !c.is_ruler_guide()); - gone += before - ch.len(); - for c in ch.iter_mut().filter(|c| any_guide(c)) { - gone += drop_guides_below(Arc::make_mut(c)); +impl Picked { + fn is_empty(&self) -> bool { + self.children.is_empty() && self.mask.is_none() + } + + /// `self`, or `None` when nothing was picked. + fn some(self) -> Option { + (!self.is_empty()).then_some(self) + } +} + +/// The children of `n`, if it is a layer, that `pick` picks (with nothing inside), and its +/// sublayers holding one (with what they hold), or `None` when it holds none. `pick` looks at +/// each child of the layers once. +fn picked_in_layers(n: &Node, pick: &impl Fn(&Node) -> bool) -> Option { + if !n.is_layer() { + return None; + } + let children = n + .children() + .into_iter() + .flatten() + .enumerate() + .filter_map(|(i, c)| if pick(c) { Some((i, Picked::default())) } else { picked_in_layers(c, pick).map(|p| (i, p)) }) + .collect(); + Picked { children, mask: None }.some() +} + +/// Remove the children of `layer` and of its sublayers that [`picked_in_layers`] picked (`held`). +/// Only the layers holding one are copied, and only those are visited. +fn drop_picked(layer: &mut Arc, held: Picked) { + let Some(ch) = Arc::make_mut(layer).children_mut() else { return }; + let mut held = held.children.into_iter().peekable(); + let mut next = 0; + ch.retain_mut(|c| { + let at = next; + next += 1; + let Some((_, inner)) = held.next_if(|(i, _)| *i == at) else { return true }; + // A sublayer holds something; a child picked itself has nothing inside. + if inner.is_empty() { + return false; } + drop_picked(c, inner); + true + }); +} + +/// Change with `f` (whether it is vertical, its position) the line of each ruler guide of `layer` +/// and of its sublayers that [`picked_in_layers`] picked (`held`), in the order they lie in. Only +/// the layers holding one, and the guides, are copied, and only those layers are visited. +fn edit_picked(layer: &mut Arc, held: Picked, f: &mut impl FnMut(bool, &mut f64)) { + let Some(ch) = Arc::make_mut(layer).children_mut() else { return }; + for (i, inner) in held.children { + let Some(c) = ch.get_mut(i) else { continue }; + if !inner.is_empty() { + edit_picked(c, inner, f); + } else if let NodeKind::RulerGuide { vertical, pos, .. } = &mut Arc::make_mut(c).kind { + f(*vertical, pos); + } + } +} + +/// `n` picked itself when it is a ruler guide by `is_guide`, else the ruler guides below it +/// ([`guides_below`]). +fn guide_or_below(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option { + if is_guide(n) { Some(Picked::default()) } else { guides_below(n, is_guide) } +} + +/// The ruler guides below `n`, by `is_guide`: those among its members and its opacity-mask art +/// (each with nothing inside), and the members and mask art holding one further down (with what +/// they hold), or `None` when there is none. `is_guide` looks at each node below `n` once, and at +/// nothing inside a ruler guide. +fn guides_below(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option { + let mask = n.extras().mask.as_deref().and_then(|m| guide_or_below(&m.art, is_guide)).map(Box::new); + let children = n.children().into_iter().flatten().enumerate().filter_map(|(i, c)| guide_or_below(c, is_guide).map(|p| (i, p))).collect(); + Picked { children, mask }.some() +} + +/// Remove the ruler guides below `n` that [`guides_below`] `found`, in its members and its +/// opacity-mask art (`n` itself stays). Mask art that is a ruler guide becomes an empty group. +/// Only the nodes holding one are copied, and only those are visited. Returns how many went. +fn drop_guides_below(n: &mut Node, found: Picked) -> usize { + let mut gone = drop_mask_guides(n, found.mask); + if let Some(ch) = n.children_mut() { + let mut found = found.children.into_iter().peekable(); + let mut next = 0; + ch.retain_mut(|c| { + let at = next; + next += 1; + let Some((_, inner)) = found.next_if(|(i, _)| *i == at) else { return true }; + if inner.is_empty() { + gone += 1; + return false; + } + gone += drop_guides_below(Arc::make_mut(c), inner); + true + }); } gone } -/// Remove every ruler guide in the opacity-mask art of `n`. Mask art that is a ruler guide -/// becomes an empty group. Returns how many went. -fn drop_mask_guides(n: &mut Node) -> usize { +/// Remove the ruler guides `found` in the opacity-mask art of `n` ([`guides_below`]'s mask +/// part). Mask art that is a ruler guide becomes an empty group. Returns how many went. +fn drop_mask_guides(n: &mut Node, found: Option>) -> usize { + let Some(found) = found else { return 0 }; let Some(mask) = n.extras_if_any().and_then(|e| e.mask.as_deref_mut()) else { return 0 }; - if mask.art.is_ruler_guide() { + if found.is_empty() { mask.art = Arc::new(Node::group(mask.art.id, vec![])); 1 - } else if any_guide(&mask.art) { - drop_guides_below(Arc::make_mut(&mut mask.art)) } else { - 0 + drop_guides_below(Arc::make_mut(&mut mask.art), *found) } } -/// Does layer `l` hold a ruler guide out of place: below art, in an object or in mask art, or so -/// in one of its sublayers? -fn layer_untidy(l: &Node) -> bool { - let Some(ch) = l.children() else { return false }; - let start = Node::guide_run_start(ch); - l.extras().mask.as_deref().is_some_and(|m| any_guide(&m.art)) - || ch.iter().take(start).any(|c| c.is_ruler_guide() || if c.is_layer() { layer_untidy(c) } else { any_guide(c) }) +/// The ruler guides, by `is_guide`, that layer `l` holds out of place, or `None` when it holds +/// none: those below art (each with nothing inside), those in its mask art and in its objects +/// ([`guides_below`]), and its sublayers holding one out of place (with what they hold). +/// `is_guide` looks at each node once; the guides at the end of a layer are in place and are +/// not looked at. +fn untidy(l: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option { + let ch = l.children()?; + let mask = l.extras().mask.as_deref().and_then(|m| guide_or_below(&m.art, is_guide)).map(Box::new); + let children = ch + .iter() + .enumerate() + .take(Node::guide_run_start(ch)) + .filter_map(|(i, c)| { + let found = if is_guide(c) { + Some(Picked::default()) + } else if c.is_layer() { + untidy(c, is_guide) + } else { + guides_below(c, is_guide) + }; + found.map(|p| (i, p)) + }) + .collect(); + Picked { children, mask }.some() } -/// [`Document::tidy_ruler_guides`] for layer `l`. Returns how many guides were removed. -fn tidy_layer(l: &mut Node) -> usize { - let mut gone = drop_mask_guides(l); +/// [`Document::tidy_ruler_guides`] for layer `l`, whose guides out of place [`untidy`] `found`. +/// Only the nodes holding one are copied, and only those are visited. Returns how many guides +/// were removed. +fn tidy_layer(l: &mut Node, found: Picked) -> usize { + let mut gone = drop_mask_guides(l, found.mask); let Some(ch) = l.children_mut() else { return gone }; - for c in ch.iter_mut() { - if c.is_layer() { - if layer_untidy(c) { - gone += tidy_layer(Arc::make_mut(c)); - } - } else if !c.is_ruler_guide() && any_guide(c) { - gone += drop_guides_below(Arc::make_mut(c)); + let mut below_art = false; + for (i, inner) in found.children { + let Some(c) = ch.get_mut(i) else { continue }; + if inner.is_empty() { + below_art = true; + } else if c.is_layer() { + gone += tidy_layer(Arc::make_mut(c), inner); + } else { + gone += drop_guides_below(Arc::make_mut(c), inner); } } - if ch.iter().take(Node::guide_run_start(ch)).any(|c| c.is_ruler_guide()) { + if below_art { let (guides, art): (Vec<_>, Vec<_>) = std::mem::take(ch).into_iter().partition(|c| c.is_ruler_guide()); *ch = art; ch.extend(guides); @@ -698,3 +775,286 @@ impl std::fmt::Debug for GuideCache { f.write_str("GuideCache") } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Appearance, OpacityMask, Symbol}; + use std::cell::Cell; + use vectorcraft_geom::shapes; + + /// The sublayers in [`nested`]'s chain: with the top-level layer around them and the objects in + /// the innermost, the deepest a document goes ([`crate::MAX_DEPTH`]). + const CHAIN: u64 = 30; + + fn rect(id: u64) -> Arc { + Arc::new(Node::path(NodeId(id), shapes::rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)), Appearance::default_art())) + } + + fn guide(id: u64, artboard: Option) -> Arc { + Arc::new(Node::new(NodeId(id), NodeKind::RulerGuide { vertical: false, pos: id as f64, artboard })) + } + + fn group(id: u64, children: Vec>) -> Arc { + Arc::new(Node::group(NodeId(id), children)) + } + + /// `n` with `art` as its opacity-mask art. + fn masked(n: Arc, art: Arc) -> Arc { + let mut n = (*n).clone(); + n.extras_mut().mask = Some(Box::new(OpacityMask { art, clip: true, invert: false, disabled: false, linked: true })); + Arc::new(n) + } + + /// A sublayer like a new document's first layer, holding `children`. + fn sublayer(id: u64, children: Vec>) -> Arc { + let mut l = (*Document::new(1.0, 1.0).layers[0]).clone(); + l.id = NodeId(id); + if let Some(ch) = l.children_mut() { + *ch = children; + } + Arc::new(l) + } + + /// A document whose first layer holds `children`. + fn doc(children: Vec>) -> Document { + let mut d = Document::new(100.0, 100.0); + if let Some(ch) = d.layers.first_mut().and_then(|l| Arc::make_mut(l).children_mut()) { + *ch = children; + } + d + } + + /// The ids of the nodes in the layers, in tree order. + fn ids(d: &Document) -> Vec { + fn walk(n: &Node, out: &mut Vec) { + out.push(n.id.0); + for c in n.children().into_iter().flatten() { + walk(c, out); + } + } + let mut out = vec![]; + for l in &d.layers { + walk(l, &mut out); + } + out + } + + /// [`CHAIN`] sublayers, each inside the one before and holding an object, a canvas guide and a + /// guide of artboard 7 (the innermost: object 150, guides 200 and 201). Beside the chain, a + /// sublayer with a canvas guide only. Returns the document and that sublayer. + fn nested() -> (Document, Arc) { + let mut chain = sublayer(100, vec![rect(150), guide(200, None), guide(201, Some(7))]); + for depth in 1..CHAIN { + chain = sublayer(100 + depth, vec![rect(600 + depth), chain, guide(300 + depth * 2, None), guide(301 + depth * 2, Some(7))]); + } + let still = sublayer(500, vec![rect(502), guide(501, None)]); + (doc(vec![chain, Arc::clone(&still)]), still) + } + + /// How many children the layers of [`nested`] hold: the first layer's 2, the untouched + /// sublayer's 2, the innermost sublayer's 3 and 4 in each sublayer around it. + const NESTED_CHILDREN: usize = 2 + 2 + 3 + (CHAIN as usize - 1) * 4; + + fn on_artboard_7(n: &Node) -> bool { + matches!(n.kind, NodeKind::RulerGuide { artboard: Some(7), .. }) + } + + #[test] + fn removing_guides_looks_at_each_child_of_nested_layers_once() { + let (mut d, still) = nested(); + assert!(!d.clone().limit_depth(), "the chain is as deep as a document goes"); + let mut deeper = doc(vec![sublayer(999, d.layers[0].children().cloned().unwrap_or_default())]); + assert!(deeper.limit_depth(), "a document nested deeper is brought within the cap when it is read"); + let mut whole = d.clone(); + let looked = Cell::new(0_usize); + let pick = |n: &Node| { + looked.set(looked.get() + 1); + on_artboard_7(n) + }; + let held = picked_in_layers(&d.layers[0], &pick).unwrap(); + assert_eq!(looked.get(), NESTED_CHILDREN, "each child of the layers is looked at once"); + drop_picked(&mut d.layers[0], held); + assert_eq!(looked.get(), NESTED_CHILDREN, "removing looks at nothing again"); + assert!(d.layers[0].children().is_some_and(|c| Arc::ptr_eq(&c[1], &still)), "a sublayer with nothing to remove is not copied"); + + // The command's own entry point removes the same guides. + let before = whole.clone(); + let mut sel = Selection::default(); + sel.set_guides([NodeId(200), NodeId(201), NodeId(501)]); + sel.objects.push(NodeId(150)); + sel.key_guide = Some(NodeId(201)); + assert_eq!(whole.retain_guides(&mut sel, |g| g.artboard != Some(7)), CHAIN as usize); + assert_eq!(ids(&whole), ids(&d)); + assert_eq!(ids(&whole).len(), ids(&before).len() - CHAIN as usize); + assert!(whole.ruler_guides().iter().all(|g| g.artboard.is_none())); + assert_eq!(whole.ruler_guides().len(), CHAIN as usize + 1); + assert_eq!((sel.objects.as_slice(), sel.guides.as_slice(), sel.key_guide), (&[NodeId(150)][..], &[NodeId(200), NodeId(501)][..], None)); + assert!(whole.layers[0].children().is_some_and(|c| Arc::ptr_eq(&c[1], &still))); + // The document as it was (an undo step's copy) still holds every guide. + assert_eq!(before.ruler_guides().len(), CHAIN as usize * 2 + 1); + + // Saving the guides as a list, and removing a layer but not its guides, take the guides + // out of the nested layers too. + let mut saved = before.clone(); + saved.guides_to_saved(); + assert_eq!(saved.saved_guides, before.ruler_guides()); + assert_eq!(ids(&saved).len(), 1 + CHAIN as usize * 2 + 2); + assert!(saved.layers[0].children().is_some_and(|c| !Arc::ptr_eq(&c[1], &still) && c[1].children().is_some_and(|c| c.len() == 1))); + let mut edited = before.clone(); + let chain = edited.remove_edit_layer(NodeId(100 + CHAIN - 1)).unwrap(); + assert!(!chain.holds_ruler_guides()); + assert_eq!(edited.ruler_guides().len(), before.ruler_guides().len(), "the removed layer's guides stay in the document"); + assert_eq!(edited.guides_on(edited.layers[0].id).len(), CHAIN as usize * 2); + + // Keeping every guide changes nothing and copies no layer. + let kept = whole.clone(); + assert_eq!(whole.retain_guides(&mut sel, |_| true), 0); + assert!(Arc::ptr_eq(&whole.layers[0], &kept.layers[0])); + // Removing every guide leaves the objects. + assert_eq!(whole.retain_guides(&mut sel, |_| false), CHAIN as usize + 1); + assert!(whole.ruler_guides().is_empty() && sel.guides.is_empty()); + assert_eq!(ids(&whole).len(), 1 + CHAIN as usize * 2 + 2); + assert_eq!(sel.objects, [NodeId(150)]); + // A document without layers, and one without guides, have nothing to remove. + let mut empty = Document::new(10.0, 10.0); + assert_eq!(empty.retain_guides(&mut sel, |_| false), 0); + empty.layers.clear(); + assert_eq!(empty.retain_guides(&mut sel, |_| false), 0); + } + + #[test] + fn moving_an_artboards_guides_looks_at_each_child_of_nested_layers_once() { + let (mut d, still) = nested(); + let mut whole = d.clone(); + let looked = Cell::new(0_usize); + let pick = |n: &Node| { + looked.set(looked.get() + 1); + on_artboard_7(n) + }; + let held = picked_in_layers(&d.layers[0], &pick).unwrap(); + assert_eq!(looked.get(), NESTED_CHILDREN, "each child of the layers is looked at once"); + let mut seen = vec![]; + edit_picked(&mut d.layers[0], held, &mut |vertical, pos: &mut f64| { + seen.push((vertical, *pos)); + *pos += 9.0; + }); + assert_eq!(looked.get(), NESTED_CHILDREN, "changing looks at nothing again"); + // A sublayer comes before its layer's own guides, so the innermost guide is changed first. + let order: Vec<(bool, f64)> = [201.0].into_iter().chain((1..CHAIN).map(|depth| (301 + depth * 2) as f64)).map(|pos| (false, pos)).collect(); + assert_eq!(seen, order, "each guide of the artboard is changed once, in the order the guides lie in"); + assert!(d.layers[0].children().is_some_and(|c| Arc::ptr_eq(&c[1], &still)), "a sublayer with no guide of the artboard is not copied"); + + // The public moves change the same guides by the same amount. + let before = whole.clone(); + whole.move_artboard_guides(7, Vec2::new(5.0, 9.0)); + assert_eq!(whole, d); + assert_eq!(ids(&whole), ids(&before)); + for g in whole.ruler_guides() { + let id = g.id.map_or(0, |id| id.0) as f64; + assert_eq!(g.pos, if g.artboard == Some(7) { id + 9.0 } else { id }, "guide {id}"); + } + assert!(whole.layers[0].children().is_some_and(|c| Arc::ptr_eq(&c[1], &still))); + assert_eq!(before.ruler_guide(NodeId(201)).map(|g| g.pos), Some(201.0), "the document as it was keeps its guides"); + whole.map_artboard_guides(7, Affine::scale(2.0)); + assert_eq!(whole.ruler_guide(NodeId(201)).map(|g| g.pos), Some(420.0)); + assert_eq!(whole.ruler_guide(NodeId(200)).map(|g| g.pos), Some(200.0), "a canvas guide stays"); + + // An artboard without guides, and a document without layers, change nothing and copy no layer. + let kept = whole.clone(); + whole.move_artboard_guides(99, Vec2::new(5.0, 9.0)); + assert!(Arc::ptr_eq(&whole.layers[0], &kept.layers[0])); + let mut empty = Document::new(10.0, 10.0); + empty.layers.clear(); + empty.move_artboard_guides(7, Vec2::new(5.0, 9.0)); + assert!(empty.layers.is_empty()); + } + + #[test] + fn guides_in_objects_are_found_by_looking_at_each_node_once() { + // Groups, each inside the one before and holding an object and a guide. Beside them, an + // object whose mask art holds a guide, one whose mask art is a guide, and a group with none. + let mut chain = group(100, vec![rect(150), guide(200, None)]); + for depth in 1..CHAIN { + chain = group(100 + depth, vec![rect(600 + depth), chain, guide(300 + depth, None)]); + } + let still = group(700, vec![rect(701)]); + let root = group( + 1, + vec![chain, masked(rect(800), group(801, vec![guide(802, None), rect(803)])), masked(rect(810), guide(811, None)), Arc::clone(&still)], + ); + let looked = Cell::new(0_usize); + let is_guide = |n: &Node| { + looked.set(looked.get() + 1); + n.is_ruler_guide() + }; + // The chain's groups, objects and guides; a masked object, its mask group and the two in + // it; a masked object and its mask guide; the last group and its object. + let nodes = CHAIN as usize * 3 + 4 + 2 + 2; + let found = guides_below(&root, &is_guide).unwrap(); + assert_eq!(looked.get(), nodes, "each node below the root is looked at once"); + let mut cleared = (*root).clone(); + assert_eq!(drop_guides_below(&mut cleared, found), CHAIN as usize + 2); + assert_eq!(looked.get(), nodes, "removing looks at nothing again"); + assert!(guides_below(&cleared, &Node::is_ruler_guide).is_none()); + let ch = cleared.children().unwrap(); + assert!(Arc::ptr_eq(&ch[3], &still), "a group without guides is not copied"); + assert_eq!( + ch[1].extras().mask.as_deref().map(|m| m.art.children().map(|c| c.iter().map(|c| c.id.0).collect::>())), + Some(Some(vec![803])) + ); + assert_eq!(ch[2].extras().mask.as_deref().map(|m| (m.art.id, m.art.children().map(Vec::len))), Some((NodeId(811), Some(0)))); + + // Opening a file removes the same guides from an object on a layer and from a symbol's art. + let mut d = doc(vec![Arc::clone(&root), guide(900, None)]); + d.symbols = vec![Symbol { name: "s".into(), art: Arc::clone(&root), dynamic: false }]; + assert_eq!(d.tidy_ruler_guides(), (CHAIN as usize + 2) * 2); + assert_eq!(d.layers[0].children().map(|c| (*c[0]).clone()), Some(cleared.clone())); + assert_eq!(*d.symbols[0].art, cleared); + assert_eq!(d.ruler_guides().iter().map(|g| g.id).collect::>(), [Some(NodeId(900))]); + } + + #[test] + fn guides_out_of_place_are_found_by_looking_at_each_node_once() { + // Sublayers, each inside the one before, with their guides in place; the innermost also + // holds a guide below its art. Beside them, a group holding a guide, a sublayer with its + // guide in place, and a sublayer whose mask art holds a guide. + let mut chain = sublayer(100, vec![guide(200, None), rect(150), guide(201, None)]); + for depth in 1..CHAIN { + chain = sublayer(100 + depth, vec![rect(600 + depth), chain, guide(300 + depth, None)]); + } + let still = sublayer(500, vec![rect(502), guide(501, None)]); + let with_mask = masked(sublayer(510, vec![rect(513)]), group(511, vec![guide(512, None)])); + let mut d = doc(vec![chain, group(700, vec![rect(701), guide(702, None)]), Arc::clone(&still), with_mask, guide(900, None)]); + let mut whole = d.clone(); + let looked = Cell::new(0_usize); + let is_guide = |n: &Node| { + looked.set(looked.get() + 1); + n.is_ruler_guide() + }; + // Below the guides at the end of each layer: the first layer's 4, the group's 2, the + // untouched sublayer's 1, the masked sublayer's mask group, its guide and the object, 2 in + // the innermost sublayer and 2 in each sublayer around it. + let nodes = 4 + 2 + 1 + 3 + 2 + (CHAIN as usize - 1) * 2; + let found = untidy(&d.layers[0], &is_guide).unwrap(); + assert_eq!(looked.get(), nodes, "each node below the guides in place is looked at once"); + assert_eq!(tidy_layer(Arc::make_mut(&mut d.layers[0]), found), 2); + assert_eq!(looked.get(), nodes, "tidying looks at nothing again"); + assert!(d.layers[0].children().is_some_and(|c| Arc::ptr_eq(&c[2], &still)), "a sublayer with its guides in place is not copied"); + assert!(untidy(&d.layers[0], &Node::is_ruler_guide).is_none()); + + // Opening a file does the same. + let before = whole.clone(); + assert_eq!(whole.tidy_ruler_guides(), 2); + assert_eq!(whole, d); + let all = ids(&whole); + assert!(!all.contains(&702) && all.len() == ids(&before).len() - 1); + let innermost = whole.node(NodeId(100)).and_then(|n| n.children()).map(|c| c.iter().map(|c| c.id.0).collect::>()); + assert_eq!(innermost, Some(vec![150, 200, 201]), "a guide below art moves up to the other guides"); + assert!(whole.node(NodeId(510)).is_some_and(|n| n.extras().mask.as_deref().is_some_and(|m| m.art.children().is_some_and(Vec::is_empty)))); + // A tidy document is left alone, and no layer is copied. + let kept = whole.clone(); + assert_eq!(whole.tidy_ruler_guides(), 0); + assert!(Arc::ptr_eq(&whole.layers[0], &kept.layers[0])); + } +} diff --git a/crates/doc/src/node/ops.rs b/crates/doc/src/node/ops.rs index 2bb655236a..fb6e8de8d9 100644 --- a/crates/doc/src/node/ops.rs +++ b/crates/doc/src/node/ops.rs @@ -763,33 +763,19 @@ impl Node { children.iter().rposition(|c| !c.is_ruler_guide()).map_or(0, |i| i + 1) } - /// Remove the ruler guides of this layer and of its sublayers. Other nodes hold none. + /// Remove the ruler guides of this layer and of its sublayers. Other nodes hold none. Only + /// layers that hold one are copied. pub fn strip_ruler_guides(&mut self) { - if !self.holds_ruler_guides() { - return; - } - if let Some(ch) = self.children_mut() { - ch.retain(|c| !c.is_ruler_guide()); - for c in ch.iter_mut().filter(|c| c.holds_ruler_guides()) { - Arc::make_mut(c).strip_ruler_guides(); - } + if let Some(held) = held_guides(self, &Self::is_ruler_guide) { + strip_held(self, held); } } /// Call `f` on the position (along the axis it crosses) of each ruler guide of this layer and /// of its sublayers, with whether the guide is vertical. Only layers that hold one are copied. pub fn map_ruler_guide_positions(&mut self, f: &mut impl FnMut(bool, &mut f64)) { - if !self.holds_ruler_guides() { - return; - } - if let Some(ch) = self.children_mut() { - for c in ch.iter_mut().filter(|c| c.is_ruler_guide() || c.holds_ruler_guides()) { - let c = Arc::make_mut(c); - match &mut c.kind { - NodeKind::RulerGuide { vertical, pos, .. } => f(*vertical, pos), - _ => c.map_ruler_guide_positions(f), - } - } + if let Some(held) = held_guides(self, &Self::is_ruler_guide) { + map_held(self, held, f); } } @@ -834,6 +820,61 @@ impl Node { } } +/// The ruler guides a layer holds: its children that are one, by index in order (each with +/// nothing inside), and its sublayers holding one (with what they hold). +#[derive(Debug, Default)] +pub(super) struct HeldGuides(Vec<(usize, HeldGuides)>); + +/// The ruler guides, by `is_guide`, that `n` holds if it is a layer, or `None` when it holds +/// none. `is_guide` looks at each child of `n` and of its sublayers once. +pub(super) fn held_guides(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option { + if !n.is_layer() { + return None; + } + let held: Vec<_> = n + .children() + .into_iter() + .flatten() + .enumerate() + .filter_map(|(i, c)| if is_guide(c) { Some((i, HeldGuides::default())) } else { held_guides(c, is_guide).map(|h| (i, h)) }) + .collect(); + (!held.is_empty()).then_some(HeldGuides(held)) +} + +/// Remove the ruler guides of `layer` and of its sublayers that [`held_guides`] found (`held`). +/// Only the sublayers holding one are copied, and only those are visited. +pub(super) fn strip_held(layer: &mut Node, held: HeldGuides) { + let Some(ch) = layer.children_mut() else { return }; + let mut held = held.0.into_iter().peekable(); + let mut next = 0; + ch.retain_mut(|c| { + let at = next; + next += 1; + let Some((_, inner)) = held.next_if(|(i, _)| *i == at) else { return true }; + // A sublayer holds something; a ruler guide has nothing inside. + if inner.0.is_empty() { + return false; + } + strip_held(Arc::make_mut(c), inner); + true + }); +} + +/// Call `f` (whether it is vertical, its position) on each ruler guide of `layer` and of its +/// sublayers that [`held_guides`] found (`held`), in the order they lie in. Only the sublayers +/// holding one, and the guides, are copied, and only those sublayers are visited. +pub(super) fn map_held(layer: &mut Node, held: HeldGuides, f: &mut impl FnMut(bool, &mut f64)) { + let Some(ch) = layer.children_mut() else { return }; + for (i, inner) in held.0 { + let Some(c) = ch.get_mut(i) else { continue }; + if !inner.0.is_empty() { + map_held(Arc::make_mut(c), inner, f); + } else if let NodeKind::RulerGuide { vertical, pos, .. } = &mut Arc::make_mut(c).kind { + f(*vertical, pos); + } + } +} + impl Node { /// The Appearance panel's row for what this object's own fills and strokes paint around: /// "Contents" for a group or layer (its members) and for an image (its pixels), "Characters" diff --git a/crates/doc/src/node/tests.rs b/crates/doc/src/node/tests.rs index 9981514879..06e68a79d0 100644 --- a/crates/doc/src/node/tests.rs +++ b/crates/doc/src/node/tests.rs @@ -598,3 +598,207 @@ fn compound_stroke_path_is_its_members_in_order() { assert_eq!(bp.elements(), expected.elements()); assert!(!bp.elements().is_empty()); } + +/// The sublayers in [`guide_layers`]'s chain, each inside the one before. +const GUIDE_CHAIN: u64 = 30; + +/// How many children the layers of [`guide_layers`] hold: the layer's 6, 1 in the sublayer +/// without a guide, 2 at each of the two places the shared sublayer lies, the innermost +/// sublayer's 2 and 3 in each sublayer around it. +const GUIDE_LAYER_CHILDREN: usize = 6 + 1 + 2 * 2 + 2 + (GUIDE_CHAIN as usize - 1) * 3; + +/// How many ruler guides the layers of [`guide_layers`] hold: one in each sublayer of the chain, +/// one at each place the shared sublayer lies and the layer's own. +const GUIDE_LAYER_GUIDES: usize = GUIDE_CHAIN as usize + 2 + 1; + +/// A layer holding, in order: [`GUIDE_CHAIN`] sublayers, each inside the one before and holding an +/// object and a ruler guide; a sublayer with an object only; a sublayer with an object and a +/// guide; an object; that last sublayer again; and a guide. Guides differ in direction, artboard +/// and lock. Returns the layer, the sublayer without a guide and the one that lies twice. +fn guide_layers() -> (Node, Arc, Arc) { + let rect = |id| Arc::new(Node::path(NodeId(id), shapes::rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)), Appearance::default_art())); + let guide = |id: u64| { + let kind = NodeKind::RulerGuide { vertical: id.is_multiple_of(2), pos: id as f64, artboard: id.is_multiple_of(3).then_some(7) }; + let mut g = Node::new(NodeId(id), kind); + g.locked = id.is_multiple_of(5); + Arc::new(g) + }; + let sublayer = |id, children: Vec>| { + let mut l = Node::layer(NodeId(id), "Sub", LayerColor::Preset(1)); + if let Some(ch) = l.children_mut() { + *ch = children; + } + Arc::new(l) + }; + let mut chain = sublayer(100, vec![rect(150), guide(200)]); + for depth in 1..GUIDE_CHAIN { + chain = sublayer(100 + depth, vec![rect(600 + depth), chain, guide(300 + depth)]); + } + let still = sublayer(500, vec![rect(501)]); + let twice = sublayer(510, vec![rect(511), guide(512)]); + let mut layer = Node::layer(NodeId(1), "Layer", LayerColor::Preset(0)); + if let Some(ch) = layer.children_mut() { + *ch = vec![chain, Arc::clone(&still), Arc::clone(&twice), rect(2), Arc::clone(&twice), guide(3)]; + } + (layer, still, twice) +} + +/// Whether `a` and `b` hold the same children, none of them copied. +fn same_children(a: &Node, b: &Node) -> bool { + let (a, b) = (a.children().unwrap(), b.children().unwrap()); + a.len() == b.len() && a.iter().zip(b).all(|(a, b)| Arc::ptr_eq(a, b)) +} + +/// Stripping the ruler guides of a layer looks at each child of the layer and of its sublayers +/// once, however deep the sublayers nest. The art and its order stay, a sublayer without a guide is +/// not copied, and a copy of the layer held elsewhere keeps its guides. +#[test] +fn stripping_ruler_guides_looks_at_each_child_of_nested_layers_once() { + use std::cell::Cell; + let (whole, still, twice) = guide_layers(); + let ids = |n: &Node| { + let mut out = vec![]; + n.walk(&mut |c| out.push(c.id)); + out + }; + let mut art = vec![]; + whole.walk(&mut |c| { + if !c.is_ruler_guide() { + art.push(c.id); + } + }); + assert_eq!(ids(&whole).len(), art.len() + GUIDE_LAYER_GUIDES); + + let mut n = whole.clone(); + let looked = Cell::new(0_usize); + let pick = |c: &Node| { + looked.set(looked.get() + 1); + c.is_ruler_guide() + }; + let held = super::ops::held_guides(&n, &pick).unwrap(); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "each child of the layers is looked at once"); + super::ops::strip_held(&mut n, held); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "removing looks at nothing again"); + assert_eq!(ids(&n), art, "the art stays, in its order"); + let ch = n.children().unwrap(); + assert!(Arc::ptr_eq(&ch[1], &still), "a sublayer without a guide is not copied"); + assert!(Arc::ptr_eq(&ch[3], &whole.children().unwrap()[3]), "an object is not copied"); + assert!(!Arc::ptr_eq(&ch[2], &twice) && !Arc::ptr_eq(&ch[4], &twice) && !Arc::ptr_eq(&ch[2], &ch[4])); + assert_eq!( + (ch[2].children().map(Vec::len), ch[4].children().map(Vec::len)), + (Some(1), Some(1)), + "a sublayer lying twice is stripped at both places" + ); + // The copies held elsewhere (an undo step's, say) keep their guides. + assert_eq!(twice.children().map(Vec::len), Some(2)); + assert_eq!(ids(&whole).len(), art.len() + GUIDE_LAYER_GUIDES); + + // The method strips the same guides. + let mut same = whole.clone(); + same.strip_ruler_guides(); + assert_eq!(same, n); + assert!(Arc::ptr_eq(&same.children().unwrap()[1], &still)); + // With no guide left there is nothing to remove, and nothing is copied. + let before = n.clone(); + n.strip_ruler_guides(); + assert!(same_children(&n, &before)); + let mut empty = Node::layer(NodeId(900), "Empty", LayerColor::Preset(0)); + empty.strip_ruler_guides(); + assert_eq!(empty.children().map(Vec::len), Some(0)); + // Only a layer holds ruler guides: a group is left as it is, on its own and in a layer. + let mut group = Node::group(NodeId(901), vec![Arc::clone(&twice)]); + assert!(super::ops::held_guides(&group, &Node::is_ruler_guide).is_none()); + group.strip_ruler_guides(); + assert!(Arc::ptr_eq(&group.children().unwrap()[0], &twice)); + let group = Arc::new(group); + let mut around = Node::layer(NodeId(902), "Around", LayerColor::Preset(0)); + around.children_mut().unwrap().push(Arc::clone(&group)); + around.strip_ruler_guides(); + assert!(Arc::ptr_eq(&around.children().unwrap()[0], &group)); +} + +/// Changing the positions of the ruler guides of a layer looks at each child of the layer and of +/// its sublayers once, however deep the sublayers nest. Each guide is changed once, in the order +/// the guides lie in, and keeps everything else about it; the art is unchanged, a sublayer without +/// a guide is not copied, and a copy of the layer held elsewhere keeps its positions. +#[test] +fn mapping_ruler_guide_positions_looks_at_each_child_of_nested_layers_once() { + use std::cell::Cell; + let (whole, still, twice) = guide_layers(); + let mut lines = vec![]; + whole.walk(&mut |c| { + if let NodeKind::RulerGuide { vertical, pos, .. } = c.kind { + lines.push((vertical, pos)); + } + }); + assert_eq!(lines.len(), GUIDE_LAYER_GUIDES); + assert!(lines.iter().any(|l| l.0) && lines.iter().any(|l| !l.0), "guides in both directions"); + + let mut n = whole.clone(); + let looked = Cell::new(0_usize); + let pick = |c: &Node| { + looked.set(looked.get() + 1); + c.is_ruler_guide() + }; + let held = super::ops::held_guides(&n, &pick).unwrap(); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "each child of the layers is looked at once"); + let mut seen = vec![]; + super::ops::map_held(&mut n, held, &mut |vertical, pos| { + seen.push((vertical, *pos)); + *pos = *pos * 2.0 + 1.0; + }); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "changing looks at nothing again"); + assert_eq!(seen, lines, "each guide once, in the order the guides lie in"); + + // The same nodes in the same order: a guide differs in its position alone, and the art not + // at all. + let (mut was, mut now) = (vec![], vec![]); + whole.walk(&mut |c| was.push(c)); + n.walk(&mut |c| now.push(c)); + assert_eq!(was.len(), now.len()); + for (a, b) in was.into_iter().zip(now) { + if a.is_layer() { + assert_eq!((b.id, b.is_layer(), b.children().map(Vec::len)), (a.id, true, a.children().map(Vec::len))); + continue; + } + let mut want = a.clone(); + if let NodeKind::RulerGuide { pos, .. } = &mut want.kind { + *pos = *pos * 2.0 + 1.0; + } + assert_eq!(b, &want); + } + let ch = n.children().unwrap(); + assert!(Arc::ptr_eq(&ch[1], &still), "a sublayer without a guide is not copied"); + assert!(Arc::ptr_eq(&ch[3], &whole.children().unwrap()[3]), "an object is not copied"); + assert!(!Arc::ptr_eq(&ch[2], &twice) && !Arc::ptr_eq(&ch[4], &twice) && !Arc::ptr_eq(&ch[2], &ch[4])); + // The copy held elsewhere keeps its position. + assert!(matches!(twice.children().unwrap()[1].kind, NodeKind::RulerGuide { pos, .. } if pos == 512.0)); + + // The method changes the same guides, each once. + let mut same = whole.clone(); + let mut calls = 0; + same.map_ruler_guide_positions(&mut |_, pos| { + calls += 1; + *pos = *pos * 2.0 + 1.0; + }); + assert_eq!((calls, &same), (GUIDE_LAYER_GUIDES, &n)); + assert!(Arc::ptr_eq(&same.children().unwrap()[1], &still)); + // A function that leaves the positions as they are is still called on each guide. + let mut kept = whole.clone(); + let mut calls = 0; + kept.map_ruler_guide_positions(&mut |_, _| calls += 1); + assert_eq!((calls, &kept), (GUIDE_LAYER_GUIDES, &whole)); + // A layer without a guide, an empty layer and a group: nothing to change, nothing copied. + let mut bare = whole.clone(); + bare.strip_ruler_guides(); + let before = bare.clone(); + let mut calls = 0; + bare.map_ruler_guide_positions(&mut |_, _| calls += 1); + assert!(same_children(&bare, &before)); + let mut empty = Node::layer(NodeId(900), "Empty", LayerColor::Preset(0)); + empty.map_ruler_guide_positions(&mut |_, _| calls += 1); + let mut group = Node::group(NodeId(901), vec![Arc::clone(&twice)]); + group.map_ruler_guide_positions(&mut |_, _| calls += 1); + assert!(Arc::ptr_eq(&group.children().unwrap()[0], &twice)); + assert_eq!(calls, 0); +} diff --git a/crates/effects/src/solids.rs b/crates/effects/src/solids.rs index a68ff9529f..7500af0617 100644 --- a/crates/effects/src/solids.rs +++ b/crates/effects/src/solids.rs @@ -429,6 +429,12 @@ const MAX_MEMBERS: usize = 4096; /// normals): past it the object is too complex to export. const MAX_MESH_TRIANGLES: usize = 2_000_000; +/// The most facets of solid colors under colored lights that one 3D effect draws as faces of their +/// own, to go in one order with other solids' faces. Each such face is about 16 nodes, a composite +/// of a whole surface about 6 per facet, so at this count the faces hold about as many nodes as the +/// composites do at the paint-copy allowance. Past it those solids draw as one composite each. +const MAX_FACE_COMPOSITES: usize = 30_720; + /// The shapes a 3D effect on group or layer `n` builds its solids from: every visible path, /// compound path and type in it (after [`resolve`] gives it the art of its symbol instances, blends /// and clipping groups), at any depth of its groups, each through its own geometry @@ -535,9 +541,12 @@ pub fn validate_three_d_params(id: &str, p: &Value) -> Result<(), String> { } /// One surface: each facet a path shaded from `paint`, as a group with the paint's opacity and -/// blend mode (so overlapping facets don't add up their transparency). -fn surface(n: &Node, facets: Vec, paint: &Paint, opacity: f32, blend: BlendMode, colored: bool) -> Node { - if colored && paint.color().is_some() && facets.iter().any(|face| face.rgb_gradient.is_some()) { +/// blend mode (so overlapping facets don't add up their transparency). Under colored lights a solid +/// color with smooth facets is one composite of all its facets ([`composites_facets`]) unless +/// `faces`: then each facet is a face of its own, as in a gradient, so the faces can go in one order +/// with the faces of other solids. +fn surface(n: &Node, facets: Vec, paint: &Paint, opacity: f32, blend: BlendMode, colored: bool, faces: bool) -> Node { + if !faces && composites_facets(&facets, paint, colored) { let mut group = crate::lighting::solid_surface(n.id, &facets, paint); group.opacity = opacity; group.blend = blend; @@ -583,6 +592,12 @@ fn surface(n: &Node, facets: Vec, paint: &Paint, opacity: f32, blend: Ble group } +/// 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 { + colored && paint.color().is_some() && facets.iter().any(|face| face.rgb_gradient.is_some()) +} + /// `item`'s paint, its opacity times `member` (the opacity of the objects it is in) and its blend. fn paint_of(item: &AppearanceItem, member: f32) -> (Paint, f32, BlendMode) { match item { @@ -600,10 +615,10 @@ fn paint_ref(item: &AppearanceItem) -> &Paint { } /// Paint copies `surface` makes for `facets`: two per facet (fill and seam stroke), six when colored -/// lights give each facet derived paints (`lighting::solid_surface`). +/// lights give each facet derived paints (`lighting::solid_surface`, or `lighting::facet_art` for +/// each face). fn facet_copies(facets: &[Facet], paint: &Paint, colored: bool) -> usize { - let derived = colored && paint.color().is_some() && facets.iter().any(|f| f.rgb_gradient.is_some()); - facets.len().saturating_mul(if derived { 6 } else { 2 }) + facets.len().saturating_mul(if composites_facets(facets, paint, colored) { 6 } else { 2 }) } /// The area `item` paints on `path`, and the rule that fills it: the path itself for a fill, the @@ -1265,8 +1280,9 @@ type ShadowAndSurfaces = (Vec>, Vec>); /// The 3D art of one effect `fx` on `path` (already through the effects before it). /// Rotate turns every fill and stroke, each in its own paint. Extrude & Bevel and Inflate build a /// solid from the path for the top visible fill and one from the outline of each visible stroke, -/// each in its own paint. When every one is opaque with normal blending and no colored light makes -/// a surface one composite, their faces are drawn in one back-to-front order (where depths tie, as +/// each in its own paint. When every one is opaque with normal blending, their faces are drawn in +/// one back-to-front order, a solid color under colored lights included up to +/// [`MAX_FACE_COMPOSITES`] facets (where depths tie, as /// for the caps turned toward the viewer, in the order of the appearance, and with caps, a shape's /// fill walls before the faces of its covering strokes that overlap them, and its fill's faces and /// its other strokes' faces, cut where they pass through each other ([`cut_stroke_faces`]), in the @@ -1521,7 +1537,8 @@ fn surfaces_for(doc: Option<&Document>, n: &Node, fx: &Effect, shapes: &[Profile filled.push(Filled { at: out.len(), member, caps, opacity, blend, masked: masks_surface(&paint, colored) }); } } - let art = if classic { classic::surface(n, f, &paint, opacity, blend, &p) } else { surface(n, f, &paint, opacity, blend, colored) }; + let art = + if classic { classic::surface(n, f, &paint, opacity, blend, &p) } else { surface(n, f, &paint, opacity, blend, colored, false) }; out.push((s.scopes.as_slice(), Arc::new(art))); } } @@ -1554,22 +1571,31 @@ fn surfaces_for(doc: Option<&Document>, n: &Node, fx: &Effect, shapes: &[Profile opacity >= 1.0 && blend == BlendMode::Normal }); let closed = |shape: usize| shapes.get(shape).is_some_and(|s| holds_edge(&fx.id, &s.path)); - // A solid whose surface is one composite of all its facets ([`surface`]: a solid color under - // colored lights with smooth shading) has no faces to order. - let composite = |item: &AppearanceItem, f: &[Facet]| colored && paint_ref(item).color().is_some() && f.iter().any(|f| f.rgb_gradient.is_some()); - // The faces are drawn in one order (below) unless a composite draws the surfaces whole. Under - // a member's opacity mask or blending, each member's faces go in one order of their own when its - // fills and strokes are opaque, blend normally, carry no opacity mask and are not composites. - let one_order = opaque && !solids.iter().any(|(item, _, _, f, _)| composite(item, f)); + // A solid color under colored lights with smooth shading draws as one composite of its facets + // ([`surface`]) unless its facets go in one order with other solids' faces. They do while all + // such facets together number at most MAX_FACE_COMPOSITES. + let composite = |item: &AppearanceItem, f: &[Facet]| composites_facets(f, paint_ref(item), colored); + let colored_faces: usize = solids.iter().filter(|(item, _, _, f, _)| composite(item, f)).map(|(.., f, _)| f.len()).sum(); + let split = colored_faces <= MAX_FACE_COMPOSITES; + // The faces are drawn in one order (below) when every solid is opaque and blends normally and + // no composite draws a surface whole. Under a member's opacity mask or blending, each member's + // faces go in one order of their own when its fills and strokes are opaque, blend normally, + // carry no opacity mask and are not composites. + let one_order = opaque && (split || !solids.iter().any(|(item, _, _, f, _)| composite(item, f))); let shape_order: Vec = (0..shapes.len()) .map(|shape| { solids.iter().filter(|s| s.4 == shape).all(|(item, member, _, f, _)| { let (paint, opacity, blend) = paint_of(item, *member); - opacity >= 1.0 && blend == BlendMode::Normal && !masks_surface(&paint, colored) && !composite(item, f) + opacity >= 1.0 && blend == BlendMode::Normal && !masks_surface(&paint, colored) && (split || !composite(item, f)) }) }) .collect(); let ordered = |shape: usize| if masks { shape_order.get(shape).copied().unwrap_or(false) } else { one_order }; + // The solids whose faces share one order with another solid's: every solid in the one order of + // the object, or of its member under a mask. A lone solid draws its facets in their own order, + // so a solid color under colored lights stays one composite there ([`surface`]). + let shared: Vec = + (0..shapes.len()).map(|shape| ordered(shape) && solids.iter().filter(|s| !masks || s.4 == shape).nth(1).is_some()).collect(); if solids.iter().any(|s| ordered(s.4)) { // A stroke face that passes through a fill wall or another stroke's face is cut where it // does. The pieces count against the limits as the faces did: cutting stops before the @@ -1605,6 +1631,10 @@ fn surfaces_for(doc: Option<&Document>, n: &Node, fx: &Effect, shapes: &[Profile .collect(); placed_graphics(doc, n.id, (&fx.id, &p), bounds, &filled, masks) }; + // With the pieces of the cuts, solid colors under colored lights past MAX_FACE_COMPOSITES draw + // as one composite each, so their surfaces draw whole (below). + let split = + split && solids.iter().filter(|(item, _, _, f, _)| composite(item, f)).map(|(.., f, _)| f.len()).sum::() <= MAX_FACE_COMPOSITES; let members: Vec = solids.iter().map(|s| s.4).collect(); let surfaces: Vec = solids .into_iter() @@ -1613,7 +1643,11 @@ fn surfaces_for(doc: Option<&Document>, n: &Node, fx: &Effect, shapes: &[Profile let depths: Vec = f.iter().map(|f| Placed::of(f, shape, paints)).collect(); let ends = f.iter().map(|f| f.end.filter(|_| !f.away)).collect(); let (paint, opacity, blend) = paint_of(item, member); - let art = if classic { classic::surface(n, f, &paint, opacity, blend, &p) } else { surface(n, f, &paint, opacity, blend, colored) }; + let art = if classic { + classic::surface(n, f, &paint, opacity, blend, &p) + } else { + surface(n, f, &paint, opacity, blend, colored, split && shared.get(shape).copied().unwrap_or(false)) + }; (depths, art, scopes, ends) }) .collect(); @@ -1622,8 +1656,8 @@ fn surfaces_for(doc: Option<&Document>, n: &Node, fx: &Effect, shapes: &[Profile CLASSIC_EXTRUDE => classic_extrude_options(&p).cap, _ => extrude_options(&p).cap, }; - // One order takes a plain group of faces from each solid, one face for each facet. With - // colored lights a surface can instead be one clipped composite of all its facets. A composite + // One order takes a plain group of faces from each solid, one face for each facet. A lone + // solid's surface can instead be one clipped composite of all its facets ([`surface`]), which // has no faces to order, so it draws whole. let plain = |(depths, s, _, _): &BuiltSurface| matches!(&s.kind, NodeKind::Group { children, clip: false } if children.len() == depths.len()); if masks { diff --git a/crates/effects/src/solids/tests.rs b/crates/effects/src/solids/tests.rs index c825159dda..eb9e81bbc0 100644 --- a/crates/effects/src/solids/tests.rs +++ b/crates/effects/src/solids/tests.rs @@ -491,8 +491,8 @@ fn a_masked_members_fill_and_stroke_faces_go_in_one_order() { } /// Under a light that is not white, Inflate in gradients, whose faces are drawn one by one, draws the -/// fill's and the dashed stroke's faces in one order, as it does under a white light. A solid color, -/// drawn as one composite of each surface's facets, still draws each surface whole. +/// fill's and the dashed stroke's faces in one order, as it does under a white light, and so does +/// Inflate in solid colors. #[test] fn a_colored_inflate_in_gradients_draws_its_faces_in_one_order() { use vectorcraft_doc::color::{Gradient, GradientKind, GradientPaint, GradientStop}; @@ -530,9 +530,111 @@ fn a_colored_inflate_in_gradients_draws_its_faces_in_one_order() { let cut = drawn_parts(&n, INFLATE, &json!({"rotationX": -20, "rotationY": -40, "rotationZ": 0, "depth": 60})).len(); assert_eq!(white.1, cut, "the white light's faces, cut"); assert_eq!(drawn(&n, Some(colored.clone())).0, 1, "one order under the colored light"); - // A solid color under the colored light: a composite for each of the fill and the stroke. + // A solid color under the colored light: the fill's and the stroke's faces in one order too. let solid = stroked(&letter_c()); - assert_eq!(drawn(&solid, Some(colored)).0, 2); + assert_eq!(drawn(&solid, Some(colored)).0, 1); +} + +/// The bounds of face `f` of 3D art: its path's, or for a clipped colored composite +/// ([`crate::lighting::facet_art`]) its color cover's, which is the face's bounds grown by 1 pt. +fn drawn_face_bounds(f: &Node) -> Rect { + match &f.kind { + NodeKind::Path { .. } => f.path_data().and_then(PathData::bounds).unwrap(), + NodeKind::Group { clip: true, children } => { + let cover = &children[1].children().unwrap()[0]; + cover.path_data().and_then(PathData::bounds).unwrap().inflate(-1.0, -1.0) + } + _ => panic!("a face is a path or a clipped composite"), + } +} + +/// Under a light that is not white, a solid-color fill and a solid-color stroke, plain, dashed or +/// with a width profile, draw their faces in one order, the faces of both solids in the order of +/// their depths where they overlap, in Inflate and in Extrude & Bevel, with and without perspective +/// and in a member with an opacity mask. Drawing each solid whole, the fill's faces before the +/// stroke's, shows farther faces on top. +#[test] +fn colored_solid_fills_and_strokes_share_face_depth_order() { + use vectorcraft_doc::{Dash, WidthProfile}; + type Style<'a> = (&'a str, &'a dyn Fn(&mut StrokeLayer)); + let styles: [Style; 3] = [ + ("plain", &|_| {}), + ("dashed", &|s| s.dash = Some(Dash { pattern: vec![12.0, 12.0], offset: 0.0, align_corners: false })), + ("profiled", &|s| s.profile = Some(WidthProfile::lens())), + ]; + let views = [ + json!({"rotationX": -20, "rotationY": -40, "rotationZ": 0}), + json!({"rotationX": -30, "rotationY": 20, "rotationZ": 5, "perspectiveAngle": 40}), + ]; + let mut whole_misdrawn = 0; + for id in [INFLATE, EXTRUDE] { + for view in &views { + for (style, set) in &styles { + let mut params = view.clone(); + params["depth"] = json!(60); + params["lights"] = json!([{"color": "#ff8080"}, {"color": "#4060ff", "rotation": 120}]); + let mut n = stroked(&letter_c()); + if let Some(AppearanceItem::Stroke(s)) = n.appearance.items.iter_mut().find(|i| matches!(i, AppearanceItem::Stroke(_))) { + set(s); + } + let what = format!("{id} {view} {style}"); + let parts = drawn_parts(&n, id, ¶ms); + if id == INFLATE { + assert!(parts.iter().any(|(f, ..)| f.rgb_gradient.is_some()), "{what}: smooth colored facets"); + } + let mut with = n.clone(); + with.appearance.effects.push(fx(id, params.clone())); + let art = solid_art(&with).unwrap(); + let surfaces = art.children().unwrap(); + assert_eq!(surfaces.len(), 1, "{what}: one order"); + let faces = surfaces[0].children().unwrap(); + assert_eq!(faces.len(), parts.len(), "{what}: one face per facet"); + for (k, (face, (facet, ..))) in faces.iter().zip(&parts).enumerate() { + let (a, b) = (drawn_face_bounds(face), facet.path().bounds().unwrap()); + let close = [a.x0 - b.x0, a.y0 - b.y0, a.x1 - b.x1, a.y1 - b.y1].iter().all(|d| d.abs() < 1e-6); + assert!(close, "{what}: face {k} drawn out of the order of the depths"); + } + let allowed = if id == INFLATE { 5 } else { 0 }; + let wrong = misdrawn(&parts); + assert!(wrong.len() <= allowed, "{what}: {} points show a farther face, first {:?}", wrong.len(), wrong.first()); + // The same faces drawn as two whole surfaces, the fill's then the stroke's. + let mut whole = parts.clone(); + whole.sort_by_key(|(_, part, _)| *part); + whole_misdrawn += misdrawn(&whole).len(); + // In a group, under the member's opacity mask, the faces keep the same order. + if id == INFLATE && *style == "plain" { + let cover = Node::path( + NodeId(900), + vectorcraft_geom::shapes::rectangle(Rect::new(0.0, 0.0, 400.0, 400.0)), + Appearance::basic(Paint::solid(Color::WHITE), Paint::None, 0.0), + ); + let mut masked = n.clone(); + masked.extras_mut().mask = Some(Box::new(OpacityMask::new(cover, true))); + let mut plain = Node::group(NodeId(2), vec![Arc::new(n.clone())]); + plain.appearance.effects.push(fx(id, params.clone())); + let mut g = Node::group(NodeId(2), vec![Arc::new(masked)]); + g.appearance.effects.push(fx(id, params.clone())); + let bounds = |art: &Node| { + let mut out = vec![]; + art.walk(&mut |n| { + if let NodeKind::Group { clip: true, .. } = &n.kind { + out.push(drawn_face_bounds(n)); + } + }); + out + }; + let (plain, scoped) = (solid_art(&plain).unwrap(), solid_art(&g).unwrap()); + let mut masks = 0; + scoped.walk(&mut |n| masks += usize::from(n.extras().mask.is_some())); + assert!(masks > 0, "{what}: drawn in the member's mask"); + let ordered = bounds(&plain); + assert!(ordered.len() > 100, "{what}: clipped faces"); + assert_eq!(bounds(&scoped), ordered, "{what}: the member's faces in one order"); + } + } + } + } + assert!(whole_misdrawn > 0, "whole surfaces show farther faces"); } /// A face of a stroke that leaves gaps passing through the plane of its fill's wall where they @@ -1322,11 +1424,12 @@ fn legacy_white_inflate_keeps_polygon_faces_and_its_seam_range() { assert!(faces(&art).iter().all(|face| matches!(face.kind, NodeKind::Path { .. }))); } +/// A solid color under colored lights is one composite of all its facets, with one clip and a fixed +/// number of groups, where its solid's faces draw in no order shared with another solid's: a lone +/// fill, or a fill and a translucent stroke. A fill and an opaque stroke draw each face on its own, +/// in one order. #[test] fn colored_solid_inflate_batches_channels_and_clips_once_per_surface() { - let mut n = square_node(); - n.appearance.effects.push(fx(INFLATE, json!({"lights":[{"color":"#ff0000"},{"color":"#0000ff"}]}))); - let art = solid_art(&n).unwrap(); fn count(n: &Node) -> (usize, usize, usize) { let (mut groups, mut clips, mut paths) = match &n.kind { NodeKind::Group { clip, .. } => (1, usize::from(*clip), 0), @@ -1341,13 +1444,125 @@ fn colored_solid_inflate_batches_channels_and_clips_once_per_surface() { } (groups, clips, paths) } - let (groups, clips, paths) = count(&art); - assert!(paths > 1000, "exercise a tessellated surface"); - // The fill's solid and the stroke's, each one composite with its own clip. - let surfaces = art.children().unwrap().len(); - assert_eq!(surfaces, 2); - assert_eq!(clips, surfaces); - assert_eq!(groups, 1 + 6 * surfaces, "group count must not grow with triangle count"); + let lights = json!({"lights":[{"color":"#ff0000"},{"color":"#0000ff"}]}); + let lone = |n: &mut Node| n.appearance.items.retain(|i| matches!(i, AppearanceItem::Fill(_))); + let translucent = |n: &mut Node| { + if let Some(AppearanceItem::Stroke(s)) = n.appearance.items.iter_mut().find(|i| matches!(i, AppearanceItem::Stroke(_))) { + s.opacity = 0.5; + } + }; + for (what, set, surfaces) in [("a lone fill", &lone as &dyn Fn(&mut Node), 1), ("a translucent stroke", &translucent, 2)] { + let mut n = square_node(); + set(&mut n); + n.appearance.effects.push(fx(INFLATE, lights.clone())); + let art = solid_art(&n).unwrap(); + let (groups, clips, paths) = count(&art); + assert!(paths > 1000, "{what}: exercise a tessellated surface"); + // Each solid one composite with its own clip. + assert_eq!(art.children().unwrap().len(), surfaces, "{what}"); + assert_eq!(clips, surfaces, "{what}"); + assert_eq!(groups, 1 + 6 * surfaces, "{what}: group count must not grow with triangle count"); + } + // An opaque fill and stroke: one order of faces, each its own clipped composite or shaded path. + let mut n = square_node(); + n.appearance.effects.push(fx(INFLATE, lights)); + let art = solid_art(&n).unwrap(); + let surfaces = art.children().unwrap(); + assert_eq!(surfaces.len(), 1, "one order"); + let faces = surfaces[0].children().unwrap(); + assert!(faces.len() > 100, "one face per facet"); + assert!(faces.iter().all(|f| matches!(&f.kind, NodeKind::Path { .. } | NodeKind::Group { clip: true, .. }))); + assert!(faces.iter().any(|f| matches!(&f.kind, NodeKind::Group { clip: true, .. })), "colored ramps"); +} + +/// Solid colors under colored lights go in one order of faces while their facets together, with the +/// pieces of the cuts, number at most `MAX_FACE_COMPOSITES`. Past it each solid draws whole, as one +/// composite. +#[test] +fn colored_solids_past_the_face_limit_draw_as_composites() { + let lights = json!({"lights":[{"color":"#ff0000"},{"color":"#0000ff","rotation":90}]}); + let mut base = square_node(); + base.appearance.effects.push(fx(INFLATE, lights.clone())); + let stroke = base.appearance.items.iter().find(|i| matches!(i, AppearanceItem::Stroke(_))).cloned().unwrap(); + // The facets each solid draws under these lights. Strokes alike each build the same solid. + let shapes = [Profile { path: profile(&base).unwrap().0, rule: FillRule::NonZero, items: solid_paints(&base), opacity: 1.0, scopes: vec![] }]; + let (parts, bounds) = solid_parts(&shapes, union_bounds(&shapes).unwrap()); + let counts: Vec = built_solids(INFLATE, &crate::merged_params(INFLATE, &lights), &parts, bounds, MAX_OBJECT_FACES) + .map(|f| f.unwrap().map_or(0, |f| f.len())) + .collect(); + let [fill, each] = counts[..] else { panic!("a fill's solid and a stroke's") }; + let fits = (MAX_FACE_COMPOSITES - fill) / each; + assert!(fits >= 4, "{fill} + {fits} × {each}"); + // Under the limit; under it before the cuts but past it with their pieces; past it before them. + let mut seen = vec![]; + for strokes in [fits / 2, fits, fits + 1] { + let mut n = base.clone(); + n.appearance.items.extend(std::iter::repeat_n(stroke.clone(), strokes - 1)); + let uncut = fill + strokes * each; + let cut = drawn_parts(&n, INFLATE, &lights).len(); + let one_order = cut <= MAX_FACE_COMPOSITES; + seen.push((uncut <= MAX_FACE_COMPOSITES, one_order)); + let art = solid_art(&n).unwrap(); + let surfaces = art.children().unwrap(); + if one_order { + assert_eq!(surfaces.len(), 1, "{uncut} faces, {cut} cut: one order"); + } else { + assert_eq!(surfaces.len(), 1 + strokes, "{uncut} faces, {cut} cut: each solid whole"); + assert!(surfaces.iter().all(|s| s.children().is_some_and(|c| c.len() < 10)), "each solid one composite"); + } + } + assert_eq!(seen, [(true, true), (true, false), (false, false)], "every case"); +} + +/// A solid-color face drawn on its own under colored lights takes the same colors as in the +/// surface's one composite: the paint, multiplied by the same red, green and blue light ramps, and +/// a seam of the same width. +#[test] +fn a_colored_solid_face_on_its_own_keeps_the_composites_colors() { + let n = square_node(); + let p = crate::merged_params(INFLATE, &json!({"lights":[{"color":"#ff0000"},{"color":"#0000ff","rotation":90}]})); + let (path, rule) = profile(&n).unwrap(); + let all = facets(INFLATE, &path, rule, path.bounds().unwrap(), &p).unwrap(); + let face = all + .iter() + .find(|f| f.rgb_gradient.is_some_and(|g| g.channels[0] != g.channels[1] || g.channels[1] != g.channels[2])) + .expect("a facet with colored ramps") + .clone(); + let paint = Paint::solid(RED); + // Each node's own fill paint, and its seam stroke's width, for every node under `n`. + let paints = |n: &Node| { + let mut out = vec![]; + n.walk(&mut |n| { + for item in &n.appearance.items { + match item { + AppearanceItem::Fill(f) => out.push((Some(f.paint.clone()), None)), + AppearanceItem::Stroke(s) => out.push((None, Some(s.width))), + } + } + }); + out + }; + let alone = surface(&n, vec![face.clone()], &paint, 1.0, BlendMode::Normal, true, true); + let batch = surface(&n, vec![face.clone()], &paint, 1.0, BlendMode::Normal, true, false); + let [single] = alone.children().unwrap().as_slice() else { panic!("one face") }; + assert!(matches!(&single.kind, NodeKind::Group { clip: true, .. }), "a clipped face"); + // The face's paint cover and its three channel ramps. + let fills = |n: &Node| paints(n).into_iter().filter_map(|(f, _)| f).collect::>(); + let one = fills(single); + assert_eq!(one.len(), 4); + assert_eq!(one[0], paint, "the paint"); + // The composite: its paint cover, then each channel's black backdrop and the face's ramp. + let all_fills = fills(&batch); + let ramps: Vec<&Paint> = all_fills.iter().filter(|p| **p != Paint::solid(Color::BLACK)).skip(1).collect(); + assert_eq!(all_fills.first(), Some(&paint)); + assert_eq!(ramps, one.iter().skip(1).collect::>(), "the same light ramps"); + // The single face's clip holds its seam: the clip reaches past the facet by half the seam. + let seam = (face.thickness() / 2.0).clamp(0.05, 0.75); + let widths: Vec = paints(&batch).into_iter().filter_map(|(_, w)| w).filter(|w| *w > 0.0).collect(); + assert!(widths.len() == 3 && widths.iter().all(|w| (w - seam).abs() < 1e-12), "{widths:?}"); + let NodeKind::Group { children, .. } = &single.kind else { panic!("a clip group") }; + let (clip, facet) = (children[0].path_data().and_then(PathData::bounds).unwrap(), face.path().bounds().unwrap()); + assert!((clip.x0 - (facet.x0 - seam / 2.0)).abs() < 0.1 && (clip.x1 - (facet.x1 + seam / 2.0)).abs() < 0.1, "{clip:?} {facet:?}"); } /// `square_node` moved right by `dx`, with id `id` and fill `fill`. diff --git a/crates/engine/src/cmd/addons/color.rs b/crates/engine/src/cmd/addons/color.rs index 8701d379f7..6eb21c9a63 100644 --- a/crates/engine/src/cmd/addons/color.rs +++ b/crates/engine/src/cmd/addons/color.rs @@ -43,7 +43,7 @@ pub fn specs() -> Vec { "Apply as Filter", ["Object", "Plug-ins", "Color Effects"], None, - "{effect: a colour adjustment's effect id (adjust.brightnessContrast, adjust.curves, adjust.hueSaturation, adjust.levels, adjust.shiftToColor, adjust.temperatureTint, addon.color.exposure, addon.color.desaturate, addon.color.duotone or addon.color.invert), params?: {…} (as effect.apply; missing keys take the dialog defaults), fill?: true, stroke?: true, includeImages?: true, includePatterns?: true, ids?: [id…] (default: the selection)} recolour the objects and everything inside them as the live effect would draw them, but in their colours, as edit.colors.invert does (fills, strokes, type, gradient meshes, embedded images and pattern tiles), as one undo step; their effects stay. Duotone links a colour to its one tone's swatch as the live effect does. On a path directly in a graph series, the graph also stores the new paint of the topmost visible fill and stroke, and on a graph label the new paint of its characters and rows for every label of its kind (docs/mcp.md) → {changed}", + "{effect: a colour adjustment's effect id (adjust.brightnessContrast, adjust.curves, adjust.hueSaturation, adjust.levels, adjust.shiftToColor, adjust.temperatureTint, addon.color.exposure, addon.color.desaturate, addon.color.duotone or addon.color.invert), params?: {…} (as effect.apply; missing keys take the dialog defaults), fill?: true, stroke?: true, includeImages?: true, includePatterns?: true, ids?: [id…] (default: the selection)} recolour the objects and everything inside them as the live effect would draw them, but in their colours, as edit.colors.invert does (fills, strokes, type, gradient meshes, embedded images and pattern tiles), as one undo step (none when nothing changes); their effects stay. Duotone links a colour to its one tone's swatch as the live effect does. On a path directly in a graph series, the graph also stores the new paint of the topmost visible fill and stroke, and on a graph label the new paint of its characters and rows for every label of its kind (docs/mcp.md) → {changed}", has_selection, filter ), @@ -70,7 +70,7 @@ pub fn specs() -> Vec { "Apply Nearest Swatch", ["Object", "Plug-ins", "Color Effects"], None, - "{library?: as addon.color.nearestSwatch (default: \"document\"), fill?: true, stroke?: true, ids?: [id…] (default: the selection)} give each solid colour and gradient stop of the selected objects' fills and strokes (their type's and those of the objects inside them; gradient meshes too) the colour of its nearest swatch in the library, as addon.color.nearestSwatch finds it, as one undo step: a global or spot swatch's colour links to it. A library's swatches the colours take are added to the document's first, as swatch.library.add adds them (a spot colour stays spot); images and patterns keep their colours. On a path directly in a graph series, the graph also stores the new paint of the topmost visible fill and stroke, and on a graph label the new paint of its characters and rows for every label of its kind (docs/mcp.md) → {changed: paints changed, swatches: [the document swatches the colours took], added: [swatches added to the document]}", + "{library?: as addon.color.nearestSwatch (default: \"document\"), fill?: true, stroke?: true, ids?: [id…] (default: the selection)} give each solid colour and gradient stop of the selected objects' fills and strokes (their type's and those of the objects inside them; gradient meshes too) the colour of its nearest swatch in the library, as addon.color.nearestSwatch finds it, as one undo step (none when nothing changes): a global or spot swatch's colour links to it. A library's swatches the colours take are added to the document's first, as swatch.library.add adds them (a spot colour stays spot); images and patterns keep their colours. On a path directly in a graph series, the graph also stores the new paint of the topmost visible fill and stroke, and on a graph label the new paint of its characters and rows for every label of its kind (docs/mcp.md) → {changed: paints changed, swatches: [the document swatches the colours took], added: [swatches added to the document]}", has_selection, nearest::apply ), diff --git a/crates/engine/src/cmd/addons/pathtools/tests.rs b/crates/engine/src/cmd/addons/pathtools/tests.rs index f98880e204..a72075d233 100644 --- a/crates/engine/src/cmd/addons/pathtools/tests.rs +++ b/crates/engine/src/cmd/addons/pathtools/tests.rs @@ -713,6 +713,71 @@ fn corner_commands_refuse_bad_params() { } } +#[test] +fn corner_commands_refuse_an_unknown_item_key() { + let mut s = session(); + let id = square(&mut s); + let i = id.0; + let set = json!({"items": [{"id": i, "anchor": [0]}], "radius": 5}); + let e = s.execute("addon.pathtools.cornerSet", &set).unwrap_err().to_string(); + assert!(e.contains("unknown parameter `items[0].anchor`; did you mean `items[0].anchors`?"), "{e}"); + let set_late = json!({"items": [{"id": i, "anchors": [0]}, {"id": i, "radius": 3}], "radius": 5}); + let e = s.execute("addon.pathtools.cornerSet", &set_late).unwrap_err().to_string(); + assert!(e.contains("unknown parameter `items[1].radius`; the command takes: items[1].id, items[1].anchors"), "{e}"); + refuses(&mut s, "addon.pathtools.cornerSet", &[set, set_late]); + s.execute("addon.pathtools.cornerSet", &json!({"radius": 10})).unwrap(); + let remove = json!({"items": [{"id": i, "anchor": [0]}]}); + let e = s.execute("addon.pathtools.cornerRemove", &remove).unwrap_err().to_string(); + assert!(e.contains("unknown parameter `items[0].anchor`; did you mean `items[0].anchors`?"), "{e}"); + refuses(&mut s, "addon.pathtools.cornerRemove", &[remove]); + assert_eq!(drawn(&s, id, 0), 10.0); +} + +#[test] +fn corner_items_take_their_documented_keys() { + let mut s = session(); + let id = square(&mut s); + let i = id.0; + let r = s.execute("addon.pathtools.cornerSet", &json!({"items": [{"id": i, "anchors": [0]}], "radius": 5})).unwrap(); + assert_eq!((r["corners"].clone(), drawn(&s, id, 0), drawn(&s, id, 1)), (json!(1), 5.0, 0.0)); + // A member without `anchors` takes the request's. + s.execute("addon.pathtools.cornerSet", &json!({"items": [{"id": i}], "anchors": [2], "radius": 6})).unwrap(); + assert_eq!(drawn(&s, id, 2), 6.0); + let r = s.execute("addon.pathtools.cornerRemove", &json!({"items": [{"id": i, "anchors": [0]}]})).unwrap(); + assert_eq!((r["corners"].clone(), drawn(&s, id, 0), drawn(&s, id, 2)), (json!(1), 0.0, 6.0)); + // A null `ids` beside `items` counts as not given. + s.execute("addon.pathtools.cornerRemove", &json!({"items": [{"id": i}], "ids": null})).unwrap(); + assert_eq!(drawn(&s, id, 2), 0.0); +} + +#[test] +fn corner_commands_refuse_items_with_ids() { + let mut s = session(); + let id = square(&mut s); + let other = line(&mut s, &[(350.0, 10.0), (390.0, 10.0), (390.0, 50.0)], false); + let both = json!({"items": [{"id": id.0, "anchors": [0]}], "ids": [other.0], "radius": 5}); + let e = s.execute("addon.pathtools.cornerSet", &both).unwrap_err().to_string(); + assert!(e.contains("give `items` or `ids`, not both"), "{e}"); + refuses(&mut s, "addon.pathtools.cornerSet", &[both]); + s.execute("addon.pathtools.cornerSet", &json!({"ids": [id.0], "radius": 5})).unwrap(); + let both = json!({"items": [{"id": id.0}], "ids": [id.0]}); + let e = s.execute("addon.pathtools.cornerRemove", &both).unwrap_err().to_string(); + assert!(e.contains("give `items` or `ids`, not both"), "{e}"); + refuses(&mut s, "addon.pathtools.cornerRemove", &[both]); +} + +#[test] +fn saved_corner_steps_with_older_item_keys_still_play() { + let mut s = session(); + let id = square(&mut s); + let other = line(&mut s, &[(350.0, 10.0), (390.0, 10.0), (390.0, 50.0)], false); + let saved = json!({"items": [{"id": id.0, "anchors": [0], "old": 1}], "ids": [other.0], "radius": 5}); + s.replay("addon.pathtools.cornerSet", &saved).unwrap(); + assert_eq!((drawn(&s, id, 0), drawn(&s, id, 1)), (5.0, 0.0)); + s.replay("addon.pathtools.cornerRemove", &json!({"items": [{"id": id.0, "old": 1}]})).unwrap(); + assert_eq!(drawn(&s, id, 0), 0.0); +} + /// A closed five-pointed star about (200, 150): its points at the even indices 0, 2… (the odd /// numbers counting from 1), its inner corners at the odd ones. fn star_path(s: &mut Session) -> NodeId { diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 94b35e9f93..b35d9993cc 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -419,6 +419,27 @@ pub(crate) fn appearance_targets(s: &Session, p: &Value) -> Result> if targets_contents(p)? { leaf_targets(s, &roots) } else { Ok(roots) } } +/// The refusal of an appearance edit whose targets are all the art of graph designs drawn in graph series. +const DESIGN_ART: &str = "the art of a graph design in a graph series is drawn again from the design: edit the design to change it"; + +/// The [`appearance_targets`] whose rows and effects a command edits: the art of graph designs drawn in graph series +/// ([`super::graph::is_design_art`]) is left out, as the graph draws it again from the design. A command whose targets +/// are all such art is refused. +pub(crate) fn edit_targets(s: &Session, p: &Value) -> Result> { + let ids = appearance_targets(s, p)?; + without_design_art(&s.doc()?.doc, ids, "appearance") +} + +/// `ids` without the art of graph designs drawn in graph series; `cmd` is refused when `ids` held nothing else. +pub(crate) fn without_design_art(d: &Document, mut ids: Vec, cmd: &str) -> Result> { + let given = ids.len(); + ids.retain(|id| !super::graph::is_design_art(d, *id)); + if given > 0 && ids.is_empty() { + return Err(bad(cmd, DESIGN_ART)); + } + Ok(ids) +} + /// `ids` with each graph series group that has no fill or stroke row of its own replaced by the paths directly inside /// it ([`super::graph::series_group_marks`]). A series group with rows, as a document saved earlier can hold, stays: /// the Appearance panel lists those rows and edits them by index. @@ -453,17 +474,17 @@ pub(crate) fn look_holder<'a>(d: &'a Document, n: &'a Node) -> &'a Node { look_holders(d, n.id).last().and_then(|id| d.node(*id)).unwrap_or(n) } -/// Objects whose fill and stroke rows a command edits: the [`appearance_targets`], with a graph series group standing -/// for the paths directly inside it ([`rowless_series_marks`]), so the graph can store the edit. +/// Objects whose fill and stroke rows a command edits: the [`edit_targets`], with a graph series group standing for the +/// paths directly inside it ([`rowless_series_marks`]), so the graph can store the edit. pub(crate) fn row_targets(s: &Session, p: &Value) -> Result> { - let ids = appearance_targets(s, p)?; + let ids = edit_targets(s, p)?; Ok(rowless_series_marks(&s.doc()?.doc, &ids)) } /// Objects whose whole appearance a command edits: the [`row_targets`], then each graph series group they stand for, /// which can hold effects and a graphic style link of its own. pub(crate) fn whole_targets(s: &Session, p: &Value) -> Result> { - let own = appearance_targets(s, p)?; + let own = edit_targets(s, p)?; let mut ids = rowless_series_marks(&s.doc()?.doc, &own); let groups = stood_for(&own, &ids); ids.extend(groups); @@ -876,6 +897,11 @@ fn transfer(s: &mut Session, p: &Value) -> Result { return Err(bad(C, "`source` and `target` are the same object")); } let copy = switch_or(p, "copy", false, C)?; + // The art of a graph design in a series is read as a source, but neither takes an appearance nor gives one up. + without_design_art(&s.doc()?.doc, vec![target], C)?; + if !copy { + without_design_art(&s.doc()?.doc, vec![source], C)?; + } // A graph series group stands for the paths directly inside it: as the source, the topmost of them is read and a // move clears them all; as the target, each of them other than a source takes the appearance. let sources = look_holders(&s.doc()?.doc, source); @@ -1252,7 +1278,7 @@ fn move_item(s: &mut Session, p: &Value) -> Result { let aligned = aligned(p, C)?; if p.get("from").and_then(Value::as_str) == Some("contents") { // A graph series group's own Contents row, not its paths, which have none. - let ids = appearance_targets(s, p)?; + let ids = edit_targets(s, p)?; if ids.is_empty() { return Err(bad(C, "select objects or give ids")); } @@ -1454,6 +1480,7 @@ fn copy_from(s: &mut Session, p: &Value) -> Result { (Some(clicked), leaf_targets(s, &selected)?) }; targets.retain(|id| Some(*id) != source); + let mut targets = without_design_art(&s.doc()?.doc, targets, C)?; let node = match source { Some(id) => Arc::new(s.doc()?.doc.node(id).ok_or(EngineError::NoNode(id))?.clone()), None => s.eyedropper_sample.clone().ok_or_else(|| bad(C, "select the object to copy from"))?, diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index 94fe55c119..3fc5953619 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -72,7 +72,7 @@ pub fn specs() -> Vec { "Make", ["Object", "Live Paint"], Some("Cmd+Alt+X"), - "{ids?|id?} build a Live Paint group from the selected paths (paths, compound paths and groups of them; type, images and other objects are refused, nothing changes); each covered face starts with its front-most covering source's fill; with a Live Paint group among them, add the others to it as livePaint.merge does → {id, faces, edges}", + "{ids?|id?} build a Live Paint group from the selected paths (paths, compound paths and groups of them; type, images and other objects are refused, nothing changes); each covered face starts with its front-most covering source's fill; with a Live Paint group among them, add the others to it as livePaint.merge does; a Live Paint group alone has nothing to add, so it is selected and nothing changes (no undo step) → {id, faces, edges}", has_selection_or_ids, lp_make ), @@ -81,7 +81,7 @@ pub fn specs() -> Vec { "Merge", ["Object", "Live Paint"], None, - "{ids?|id?} add the other selected paths (compound paths, groups of paths and Live Paint groups too; type, images and other objects are refused, nothing changes) to the bottom-most Live Paint group, keeping its id, its paint and its own properties (name, lock, visibility, transparency, appearance, opacity mask, graphic style link) → {id, faces, edges}", + "{ids?|id?} add the other selected paths (compound paths, groups of paths and Live Paint groups too; type, images and other objects are refused, nothing changes) to the bottom-most Live Paint group, keeping its id, its paint and its own properties (name, lock, visibility, transparency, appearance, opacity mask, graphic style link); a Live Paint group alone has nothing to add, so it is selected and nothing changes (no undo step) → {id, faces, edges}", has_multi_or_ids, lp_merge ), @@ -108,7 +108,7 @@ pub fn specs() -> Vec { "Gap Options", [], None, - "{id?, ids?: [id…] (instead of id; default: the selected Live Paint groups), detect?: bool (Gap Detection: paint stops at gaps up to size wide), size?: \"small\" (3 pt) | \"medium\" (6 pt) | \"large\" (12 pt) | pt (custom, over 0 up to 72) (Paint Stops At), color?: #rrggbb or a layer color's name such as \"Red\" (Gap Preview Color; a hex color with an alpha part, #rrggbbaa or #rgba, is an error), closeGaps?: bool (add an unpainted path across each gap up to size wide to the group's paths)} set the Gap Options of Live Paint groups (what is not given stays as it was; a group starts with detect false, small and Red) and make their faces again, keeping their paint → {ids, faces, gaps: [[x0,y0,x1,y1]…] (the gaps up to size wide that no path closes, from a loose end of a path to where it closes to; detect closes them for paint), closed (paths added)}", + "{id?, ids?: [id…] (instead of id; default: the selected Live Paint groups), detect?: bool (Gap Detection: paint stops at gaps up to size wide), size?: \"small\" (3 pt) | \"medium\" (6 pt) | \"large\" (12 pt) | pt (custom, over 0 up to 72) (Paint Stops At), color?: #rrggbb or a layer color's name such as \"Red\" (Gap Preview Color; a hex color with an alpha part, #rrggbbaa or #rgba, is an error), closeGaps?: bool (add an unpainted path across each gap up to size wide to the group's paths)} set the Gap Options of Live Paint groups (what is not given stays as it was; a group starts with detect false, small and Red) and make their faces again, keeping their paint; when every group already has the options and closeGaps finds no gap, nothing changes (no undo step) → {ids, faces, gaps: [[x0,y0,x1,y1]…] (the gaps up to size wide that no path closes, from a loose end of a path to where it closes to; detect closes them for paint), closed (paths added)}", has_selection_or_ids, lp_gap_options ), @@ -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), 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}", + "{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; when every face it reaches already has the paint, nothing changes (no undo step, and the recent colors stay as they are) → {group, face}", has_doc, lp_fill ), @@ -126,7 +126,7 @@ pub fn specs() -> Vec { "Live Paint Bucket (Stroke)", [], None, - "{group: id, point: [x,y], color?|swatch?|none? (default: current stroke), width?: pt (default: current weight), tolerance?: pt (4), tint?: 0..100, keepModel?: bool (both as paint.setFill)} paint the edge nearest point → {group, edge}", + "{group: id, point: [x,y], color?|swatch?|none? (default: current stroke), width?: pt (default: current weight), tolerance?: pt (4), tint?: 0..100, keepModel?: bool (both as paint.setFill)} paint the edge nearest point; when it already has the paint and the width, nothing changes (no undo step, and the recent colors stay as they are) → {group, edge}", has_doc, lp_stroke_edge ), diff --git a/crates/engine/src/cmd/buildcmds/live_paint.rs b/crates/engine/src/cmd/buildcmds/live_paint.rs index e5e7757486..fb9e6c32ac 100644 --- a/crates/engine/src/cmd/buildcmds/live_paint.rs +++ b/crates/engine/src/cmd/buildcmds/live_paint.rs @@ -52,8 +52,34 @@ fn make_live(d: &mut Document, sel: &mut Selection, roots: &[NodeId]) -> Result< Ok((gid, nf, ne)) } +/// Select `ids` alone, without an undo step, when they are not selected that way already. +fn select_only(s: &mut Session, ids: &[NodeId]) -> Result<()> { + let st = s.doc()?; + let mut sel = st.selection.clone(); + sel.set(ids.iter().copied()); + if sel != st.selection { + s.select(|_, selection| *selection = sel)?; + } + Ok(()) +} + +/// Make or Merge given one Live Paint group and nothing else has nothing to add to it, so it leaves +/// the document as it is, records no undo step and selects the group: → its reply. `None` for any +/// other `roots`. +fn lone_live_group(s: &mut Session, roots: &[NodeId]) -> Result> { + let d = &s.doc()?.doc; + let Some(&g) = roots.first().filter(|g| roots.iter().all(|r| r == *g)) else { return Ok(None) }; + let Some(n) = d.node(g).filter(|n| is_live_paint(n)) else { return Ok(None) }; + let reply = json!({ "id": g.0, "faces": b::faces(n).len(), "edges": b::edges(n).len() }); + select_only(s, &[g])?; + Ok(Some(reply)) +} + pub(super) fn lp_make(s: &mut Session, p: &Value) -> Result { let roots = root_ids(s, p)?; + if let Some(reply) = lone_live_group(s, &roots)? { + return Ok(reply); + } let (id, faces, edges) = s.edit("Make Live Paint", |d, sel| make_live(d, sel, &roots))?; Ok(json!({ "id": id.0, "faces": faces, "edges": edges })) } @@ -130,6 +156,9 @@ fn merge_live(d: &mut Document, sel: &mut Selection, roots: &[NodeId]) -> Result pub(super) fn lp_merge(s: &mut Session, p: &Value) -> Result { let roots = root_ids(s, p)?; + if let Some(reply) = lone_live_group(s, &roots)? { + return Ok(reply); + } let (id, faces, edges) = s.edit("Merge Live Paint", |d, sel| merge_live(d, sel, &roots))?; Ok(json!({ "id": id.0, "faces": faces, "edges": edges })) } @@ -230,27 +259,58 @@ pub(super) fn lp_gap_options(s: &mut Session, p: &Value) -> Result { }; let close = switch_or(p, "closeGaps", false, C)?; let targets = live_targets(s, p)?; + let with_params = |mut options: GapOptions| { + options.detect = detect.unwrap_or(options.detect); + options.size = size.unwrap_or(options.size); + options.color = color.unwrap_or(options.color); + options + }; + // When every group already has these options and Close Gaps With Paths has no gap to close, + // nothing changes: no undo step, and the reply is the same. + let unchanged = { + let d = &s.doc()?.doc; + let (mut faces, mut gaps, mut same) = (0, vec![], true); + for g in &targets { + let old = d.node(*g).ok_or(EngineError::NoNode(*g))?; + let options = old.gap_options(); + if with_params(options) != options { + same = false; + break; + } + let open = open_gaps(old, options); + if close && !open.is_empty() { + same = false; + break; + } + faces += b::faces(old).len(); + gaps.extend(open); + } + same.then_some((faces, gaps)) + }; + if let Some((faces, gaps)) = unchanged { + select_only(s, &targets)?; + return Ok(json!({ "ids": ids_json(&targets), "faces": faces, "gaps": gaps_json(&gaps), "closed": 0 })); + } let (faces, gaps, closed) = s.edit("Gap Options", |d, sel| { let (mut faces, mut gaps, mut closed) = (0, vec![], 0); let mut changed = Beside::replacing(d, &targets); for g in &targets { let old = d.node(*g).cloned().ok_or(EngineError::NoNode(*g))?; - let mut options = old.gap_options(); - options.detect = detect.unwrap_or(options.detect); - options.size = size.unwrap_or(options.size); - options.color = color.unwrap_or(options.color); + let options = with_params(old.gap_options()); let mut sources = source_nodes(&old); + let mut added = 0; if close { // Close Gaps With Paths adds an unpainted line across each gap to the group's paths. for gap in po::find_gaps(&source_shapes(&sources).0, options.size.points()) { let line = PathData::single(SubPath::polyline(&gap, false)); sources.push(Arc::new(Node::path(d.alloc_id(), line, Appearance::default()))); - closed += 1; + added += 1; } } + closed += added; let mut n = old.clone(); // The faces change only with what paint stops at, or with new paths. - if close || options.max_gap() != old.gap_options().max_gap() { + if added > 0 || options.max_gap() != old.gap_options().max_gap() { let (children, _, _) = build_children(d, sources, options.max_gap())?; if let Some(ch) = n.children_mut() { *ch = children; @@ -305,27 +365,9 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result { let paint = paint_from(s, p)?.unwrap_or_else(|| s.paint.fill.clone()); let group = object_param(p, "group", C)?; let roots = if group.is_none() { root_ids(s, p)? } else { vec![] }; - let (g, face) = s.edit("Live Paint Bucket", |d, sel| { - let gid = match group { - Some(g) => { - if !d.node(g).is_some_and(is_live_paint) { - return Err(bad(C, format!("{g} is not a Live Paint group"))); - } - g - } - None => { - let live_at = |face: fn(&Node, Point) -> Option<&Arc>| { - roots.iter().copied().find(|r| d.node(*r).is_some_and(|n| is_live_paint(n) && face(n, pt).is_some())) - }; - match (live_at(b::visible_face_at), live_at(face_at)) { - (Some(g), _) => g, - // At a point on a hidden face of a Live Paint group among the objects, the command returns an error - // and does not merge the objects. - (None, Some(_)) => return Err(EngineError::Other("no Live Paint face at that point".into())), - (None, None) => make_live(d, sel, &roots)?.0, - } - } - }; + // The faces the click or drag paints in Live Paint group `gid`: the visible face at the point, + // and every visible face the drag crosses; → (the face at the point, the faces). + let targets_in = |d: &Document, gid: NodeId| -> Result<(NodeId, std::collections::BTreeSet)> { let fid = d .node(gid) .and_then(|n| b::visible_face_at(n, pt)) @@ -355,11 +397,58 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result { 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()); + Ok((fid, targets)) + }; + // The Live Paint group the fill paints; `None` when the objects are made into one first. + let live = { + let d = &s.doc()?.doc; + match group { + Some(g) => { + if !d.node(g).is_some_and(is_live_paint) { + return Err(bad(C, format!("{g} is not a Live Paint group"))); + } + Some(g) + } + None => { + let live_at = |face: fn(&Node, Point) -> Option<&Arc>| { + roots.iter().copied().find(|r| d.node(*r).is_some_and(|n| is_live_paint(n) && face(n, pt).is_some())) + }; + match (live_at(b::visible_face_at), live_at(face_at)) { + (Some(g), _) => Some(g), + // At a point on a hidden face of a Live Paint group among the objects, the command returns an error + // and does not merge the objects. + (None, Some(_)) => return Err(EngineError::Other("no Live Paint face at that point".into())), + (None, None) => None, + } + } } - Ok((gid, fid)) - })?; + }; + let (g, face) = match live { + Some(gid) => { + let (fid, targets) = targets_in(&s.doc()?.doc, gid)?; + // Faces that already have the paint stay as they are: when every face has it, nothing + // changes and no undo step is recorded. + let d = &s.doc()?.doc; + let unpainted: Vec = targets.into_iter().filter(|t| d.node(*t).is_none_or(|f| f.appearance.fill_paint() != paint)).collect(); + if unpainted.is_empty() { + return Ok(json!({ "group": gid.0, "face": fid.0 })); + } + s.edit("Live Paint Bucket", |d, _| { + for target in unpainted { + d.node_mut(target).ok_or(EngineError::NoNode(target))?.appearance.set_fill(paint.clone()); + } + Ok((gid, fid)) + })? + } + None => s.edit("Live Paint Bucket", |d, sel| { + let gid = make_live(d, sel, &roots)?.0; + let (fid, targets) = targets_in(d, gid)?; + for target in targets { + d.node_mut(target).ok_or(EngineError::NoNode(target))?.appearance.set_fill(paint.clone()); + } + Ok((gid, fid)) + })?, + }; s.remember_paint(&paint); Ok(json!({ "group": g.0, "face": face.0 })) } @@ -375,9 +464,22 @@ pub(super) fn lp_stroke_edge(s: &mut Session, p: &Value) -> Result { return Err(bad(C, "width out of range")); } let tol = number_or(p, "tolerance", 4.0, C)?.clamp(0.0, 1000.0); - let edge = s.edit("Live Paint Bucket", |d, _| { + let (eid, unchanged) = { + let d = &s.doc()?.doc; let n = d.node(g).filter(|n| is_live_paint(n)).ok_or_else(|| bad(C, format!("{g} is not a Live Paint group")))?; - let eid = b::visible_edge_near(n, pt, tol).map(|e| e.id).ok_or_else(|| EngineError::Other("no Live Paint edge near that point".into()))?; + let e = b::visible_edge_near(n, pt, tol).ok_or_else(|| EngineError::Other("no Live Paint edge near that point".into()))?; + // An edge that already has the paint and the width (or has no stroke and gets none) stays as + // it is, and no undo step is recorded. + let unchanged = match e.appearance.stroke() { + Some(st) => st.paint == paint && st.width == width, + None => paint.is_none(), + }; + (e.id, unchanged) + }; + if unchanged { + return Ok(json!({ "group": g.0, "edge": eid.0 })); + } + let edge = s.edit("Live Paint Bucket", |d, _| { let e = d.node_mut(eid).ok_or(EngineError::NoNode(eid))?; e.appearance.set_stroke(paint.clone()); if let Some(st) = e.appearance.stroke_mut() { diff --git a/crates/engine/src/cmd/colorcmds.rs b/crates/engine/src/cmd/colorcmds.rs index 7ad53d5bf5..8377822094 100644 --- a/crates/engine/src/cmd/colorcmds.rs +++ b/crates/engine/src/cmd/colorcmds.rs @@ -36,7 +36,7 @@ pub fn specs() -> Vec { "Invert Colors", ["Edit", "Edit Colors"], None, - "{fill?: true, stroke?: true, includeImages?: true, includePatterns?: true, ids?} replace each colour by its RGB inverse (keeps the colour model) in fills, strokes, text, gradient meshes (with fills), colorized image fills (their pixels stay unchanged), other embedded images (a recoloured copy) and pattern tiles (a new pattern swatch; the original stays), as one undo step. On a path directly in a graph series, the graph also stores the new paint of the topmost visible fill and stroke, and on a graph label the new paint of its characters and rows for every label of its kind (docs/mcp.md) → {changed, notChanged: {locked, hidden} (when the selection is everything select.all takes: the locked and hidden objects it left out whose colors would have changed; an object both hidden and locked counts as hidden), warning?: set when notChanged counts any object}", + "{fill?: true, stroke?: true, includeImages?: true, includePatterns?: true, ids?} replace each colour by its RGB inverse (keeps the colour model) in fills, strokes, text, gradient meshes (with fills), colorized image fills (their pixels stay unchanged), other embedded images (a recoloured copy) and pattern tiles (a new pattern swatch; the original stays), as one undo step (none when nothing changes). On a path directly in a graph series, the graph also stores the new paint of the topmost visible fill and stroke, and on a graph label the new paint of its characters and rows for every label of its kind (docs/mcp.md) → {changed, notChanged: {locked, hidden} (when the selection is everything select.all takes: the locked and hidden objects it left out whose colors would have changed; an object both hidden and locked counts as hidden), warning?: set when notChanged counts any object}", has_selection, |s, p| recolor(s, p, "edit.colors.invert", "Invert Colors", &invert) ), @@ -104,7 +104,7 @@ pub fn specs() -> Vec { "Blend Front to Back", ["Edit", "Edit Colors"], None, - "{} ≥3 filled objects (solid fills or gradient meshes, which keep their shading): intermediate fills graded between the frontmost and backmost fill. On a path directly in a graph series, the graph also stores the new fill, as for the other Blend commands (docs/mcp.md) → {changed}", + "{} ≥3 filled objects (solid fills or gradient meshes, which keep their shading): intermediate fills graded between the frontmost and backmost fill. On a path directly in a graph series, the graph also stores the new fill, as for the other Blend commands (docs/mcp.md); no undo step when no fill changes → {changed}", has_selection, |s, _| blend(s, BlendOrder::Stack) ), @@ -113,7 +113,7 @@ pub fn specs() -> Vec { "Blend Horizontally", ["Edit", "Edit Colors"], None, - "{} ≥3 filled objects or meshes: fills graded between the leftmost and rightmost → {changed}", + "{} ≥3 filled objects or meshes: fills graded between the leftmost and rightmost; no undo step when no fill changes → {changed}", has_selection, |s, _| blend(s, BlendOrder::Horizontal) ), @@ -122,7 +122,7 @@ pub fn specs() -> Vec { "Blend Vertically", ["Edit", "Edit Colors"], None, - "{} ≥3 filled objects or meshes: fills graded between the topmost and bottommost → {changed}", + "{} ≥3 filled objects or meshes: fills graded between the topmost and bottommost; no undo step when no fill changes → {changed}", has_selection, |s, _| blend(s, BlendOrder::Vertical) ), @@ -457,8 +457,8 @@ pub(crate) fn recolor_links(s: &mut Session, p: &Value, cmd: &str, label: &str, run_pass(s, p, cmd, label, Recolor::with_links(Scope::of(p, cmd)?, f, links)) } -/// Run `pass` on the `ids` param's objects (default: the selection) as one undo step named `label`; -/// errors name the command `cmd`. The reply's `notChanged` counts the locked and hidden objects the +/// Run `pass` on the `ids` param's objects (default: the selection) as one undo step named `label`, +/// or none when the pass changes nothing; errors name the command `cmd`. The reply's `notChanged` counts the locked and hidden objects the /// selection left out ([`left_out`]) that the pass would have changed. fn run_pass(s: &mut Session, p: &Value, cmd: &str, label: &str, pass: Recolor) -> Result { let (ids, left) = match ids_param(p, "ids", cmd)? { @@ -471,20 +471,47 @@ fn run_pass(s: &mut Session, p: &Value, cmd: &str, label: &str, pass: Recolor) - } }; let colorized_ok = super::paint::check_colorized(&s.doc()?.doc).is_ok(); - let (n, not_changed) = s.edit(label, |d, _| { + let (n, not_changed) = edit_if_changed(s, label, |_, d| { let not_changed = NotChanged::of(pass.again().would_change(d, &left).into_iter().map(|(_, hidden)| hidden)); let n = keeping_series_paint(d, &ids, |d| { let n = pass.run(d, &ids); super::paint::check_colorized_kept(colorized_ok, d).map_err(|e| bad(cmd, e))?; Ok(n) })?; - Ok((n, not_changed)) + Ok(((n, not_changed), n > 0)) })?; let mut out = json!({ "changed": n }); not_changed.report(&mut out); Ok(out) } +/// [`Session::edit`] for an Edit Colors pass, which may change nothing: `f` runs on a copy of the +/// document (it gets the document as it is too) and returns its result with whether it changed the +/// copy. Only a changed copy becomes the document, as one undo step named `label`; otherwise the +/// undo and redo steps and the saved state stay as they were. +pub(crate) fn edit_if_changed(s: &mut Session, label: &str, f: impl FnOnce(&Document, &mut Document) -> Result<(T, bool)>) -> Result { + if s.script_running() { + return Err(EngineError::Busy); + } + // A typing session ends here as `s.edit` would end it, before the copy is taken, so that the + // copy holds what was typed. + if !s.previewing && super::typing_in_progress(s) { + super::settle_interaction(s)?; + } + let before = s.doc()?.doc.clone(); + let mut doc = (*before).clone(); + let (v, changed) = f(&before, &mut doc)?; + // The document is shared with `before` no longer, so `s.edit` does not copy it again. + drop(before); + if changed { + s.edit(label, |d, _| { + *d = doc; + Ok(()) + })?; + } + Ok(v) +} + /// Run `recolor` on `d` (it returns how many objects it changed) over `roots`, then store on their graphs what it changed /// on the graph series marks and graph labels among `roots` and inside them, so the marks and every label of a kind keep /// their new paint when the graph is drawn again. @@ -599,11 +626,6 @@ fn pass_targets(s: &Session, p: &Value, cmd: &str) -> Result { fn to_indexed(s: &mut Session, p: &Value) -> Result { const C: &str = "edit.colors.toIndexed"; let indexing = Indexing::of(s, p)?; - // A typing session ends here as `s.edit` would end it, before the copy is taken, so that the - // copy holds what was typed. - if !s.previewing && super::typing_in_progress(s) { - super::settle_interaction(s)?; - } let remake = |b: &ImageBlob| indexing.convert(b); let images = Scope { fill: false, stroke: false, images: true, patterns: false }; let pass = Recolor { remake: Some(&remake), ..Recolor::new(images, &same) }; @@ -611,22 +633,18 @@ fn to_indexed(s: &mut Session, p: &Value) -> Result { // indexed PNG yet; it is not converted to find out. The empty PNG only gives it a new key. let would = |b: &ImageBlob| (!b.indexed_png() && b.pixel_size().is_some()).then(|| ImageBlob::png(vec![])); let (ids, left) = pass_targets(s, p, C)?; - let st = s.doc()?; - let colorized_ok = super::paint::check_colorized(&st.doc).is_ok(); - let not_changed = NotChanged::of( - Recolor { remake: Some(&would), ..Recolor::new(images, &same) }.would_change(&st.doc, &left).into_iter().map(|(_, hidden)| hidden), - ); - let mut doc = (*st.doc).clone(); - let n = pass.run(&mut doc, &ids); - if let Some((w, h)) = indexing.too_large.get() { - return Err(bad(C, format!("an image of {w} × {h} pixels is too large to convert (at most {} pixels)", Indexing::MAX_PIXELS))); - } - if n > 0 { - s.edit("Convert to Indexed Color", |d, _| { - *d = doc; - super::paint::check_colorized_kept(colorized_ok, d).map_err(|e| bad(C, e)) - })?; - } + let (n, not_changed) = edit_if_changed(s, "Convert to Indexed Color", |now, doc| { + let colorized_ok = super::paint::check_colorized(now).is_ok(); + let not_changed = NotChanged::of( + Recolor { remake: Some(&would), ..Recolor::new(images, &same) }.would_change(now, &left).into_iter().map(|(_, hidden)| hidden), + ); + let n = pass.run(doc, &ids); + if let Some((w, h)) = indexing.too_large.get() { + return Err(bad(C, format!("an image of {w} × {h} pixels is too large to convert (at most {} pixels)", Indexing::MAX_PIXELS))); + } + super::paint::check_colorized_kept(colorized_ok, doc).map_err(|e| bad(C, e))?; + Ok(((n, not_changed), n > 0)) + })?; let mut out = json!({ "changed": n }); not_changed.report(&mut out); Ok(out) @@ -925,7 +943,7 @@ fn blend(s: &mut Session, order: BlendOrder) -> Result { super::paint::check_colorized_kept(colorized_ok, d).map_err(|e| bad(label, e))?; Ok(changed) }; - let changed = s.edit(label, |d, _| keeping_series_paint(d, &ids, run))?; + let changed = edit_if_changed(s, label, |_, d| keeping_series_paint(d, &ids, run).map(|n| (n, n > 0)))?; Ok(json!({ "changed": changed })) } diff --git a/crates/engine/src/cmd/compositefonts/tests.rs b/crates/engine/src/cmd/compositefonts/tests.rs index 7997ca8f64..434f2e383a 100644 --- a/crates/engine/src/cmd/compositefonts/tests.rs +++ b/crates/engine/src/cmd/compositefonts/tests.rs @@ -536,6 +536,80 @@ fn a_document_replaced_in_the_back_is_measured_when_the_front_one_closes() { assert_eq!(actual.cached_bounds, Some(expected)); } +/// The type of document `index` with id `id`, wherever its tab is. +fn text_in(s: &Session, index: usize, id: u64) -> vectorcraft_doc::TextObject { + match &s.documents()[index].doc.node(NodeId(id)).unwrap().kind { + NodeKind::Text(t) => (**t).clone(), + _ => panic!("not text"), + } +} + +/// A document replaced while in the back has its type measured in its own composite fonts at once, +/// while the front document stays in front with its own definition registered; coming to the front +/// does not measure that type again. +#[test] +fn a_document_replaced_in_the_back_is_measured_before_it_comes_to_the_front() { + let (bytes, id, width) = full_roman_file("CF Pane"); + let mut s = session(); + s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Pane")]})).unwrap(); + assert!(s.replace_document(0, vectorcraft_format::load(&bytes).unwrap())); + assert_eq!(s.active_index(), Some(1)); + let front = vectorcraft_text::FontDb::global().composite_font("CF Pane").unwrap(); + assert_eq!(front.parts.get(vectorcraft_doc::CharClass::Roman).size, 50.0); + let measured = text_in(&s, 0, id); + assert!((measured.cached_bounds.unwrap().width() - width).abs() < 1e-6); + let node = s.documents()[0].doc.node(NodeId(id)).unwrap().geometric_bounds().unwrap(); + assert!((node.width() - width).abs() < 1e-6); + let doc = std::sync::Arc::as_ptr(&s.documents()[0].doc); + assert!(s.set_active(0)); + assert!(std::ptr::eq(doc, std::sync::Arc::as_ptr(&s.documents()[0].doc))); + assert_eq!(text(&s, id).cached_bounds, measured.cached_bounds); +} + +/// Type a document replaced in the back sets in a composite font that only the front document +/// defines is measured in that font while the front one is registered, and again, as missing, when +/// the replaced document comes to the front. +#[test] +fn a_document_replaced_in_the_back_measures_a_font_it_lacks_again_in_front() { + let mut s = session(); + let id = s.execute("text.create", &json!({"x": 10, "y": 50, "text": "Ab12", "size": 20, "font": "CF Lacked"})).unwrap()["id"].as_u64().unwrap(); + let bytes = vectorcraft_format::save_file(&s.doc().unwrap().doc); + s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Lacked")]})).unwrap(); + assert!(s.replace_document(0, vectorcraft_format::load(&bytes).unwrap())); + let back = text_in(&s, 0, id); + let expected = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &back).bounds; + assert_eq!(back.cached_bounds, Some(expected)); + assert!(s.set_active(0)); + assert!(vectorcraft_text::FontDb::global().composite_font("CF Lacked").is_none()); + let actual = text(&s, id); + let expected = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &actual).bounds; + assert_eq!(actual.cached_bounds, Some(expected)); +} + +/// Text holding an inline graphic whose symbol's art is type in a composite font, in a document +/// replaced in the back, is measured with the graphic at the size the document's own definition +/// gives it before it comes to the front. +#[test] +fn a_document_replaced_in_the_back_measures_inline_graphics_in_its_own_definition() { + let mut s = session(); + let mut full = font("CF Back Inline"); + full["parts"]["roman"]["size"] = json!(100); + s.execute("type.compositeFonts.set", &json!({"fonts": [full]})).unwrap(); + let art = + s.execute("text.create", &json!({"x": 10, "y": 50, "text": "Ab12", "size": 20, "font": "CF Back Inline"})).unwrap()["id"].as_u64().unwrap(); + s.execute("symbol.new", &json!({"name": "CF Back Inline Mark", "ids": [art]})).unwrap(); + let id = s.execute("text.create", &json!({"x": 10, "y": 150, "text": "x", "size": 20})).unwrap()["id"].as_u64().unwrap(); + s.execute("text.insertInline", &json!({"id": id, "at": 0, "symbol": "CF Back Inline Mark"})).unwrap(); + let width = text(&s, id).cached_bounds.unwrap().width(); + let bytes = vectorcraft_format::save_file(&s.doc().unwrap().doc); + s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Back Inline")]})).unwrap(); + assert!(s.replace_document(0, vectorcraft_format::load(&bytes).unwrap())); + assert!((text_in(&s, 0, id).cached_bounds.unwrap().width() - width).abs() < 1e-6); +} + /// A failed batch takes back the composite fonts it set, in the font database too. #[test] fn a_failed_batch_takes_back_its_composite_fonts() { diff --git a/crates/engine/src/cmd/docinfo.rs b/crates/engine/src/cmd/docinfo.rs index 54da0562b1..4068c2506c 100644 --- a/crates/engine/src/cmd/docinfo.rs +++ b/crates/engine/src/cmd/docinfo.rs @@ -781,7 +781,10 @@ fn info(s: &mut Session, p: &Value) -> Result { // Each brush the art uses with its type, found by name in the Brushes panel as the renderer finds it (the first of two // brushes with one name); a brush the panel does not hold draws as a plain stroke and has no type. A row names the type // as the New Brush dialog does (`Art Brush`), and the panel translates these values when it shows them. - let kinds: HashMap<&str, &vectorcraft_brush::BrushKind> = library.iter().map(|b| (b.name.as_str(), &b.kind)).collect(); + let mut kinds: HashMap<&str, &vectorcraft_brush::BrushKind> = HashMap::new(); + for b in &library { + kinds.entry(b.name.as_str()).or_insert(&b.kind); + } let brush_entries: Vec = brushes.iter().map(|name| json!({ "name": name, "type": kinds.get(name.as_str()).map(|k| k.type_id()) })).collect(); let brush_rows: Vec<(String, String)> = brushes diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index 17fcb63fe2..6ee3905d95 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -13,7 +13,7 @@ use vectorcraft_doc::{Document, Effect, Node, NodeKind, StrokeLayer}; use vectorcraft_geom::{FillRule, PathData}; use vectorcraft_render::effects; -use super::appearance::{ItemTarget, aligned, appearance_targets, index_param, item_indices, item_target, item_target_at}; +use super::appearance::{ItemTarget, aligned, appearance_targets, edit_targets, index_param, item_indices, item_target, item_target_at}; use super::expand::has_seeded; use super::rasterfx; use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, check_switches, cmd, fileio, has_doc, str_param, switch_or}; @@ -49,7 +49,7 @@ fn effect_specs() -> Vec { "Apply Effect", [], None, - "{effect: id (see effect.list, e.g. \"stylize.dropShadow\", \"distort.roughen\", \"warp.arc\", or \"plugin.\" for an installed effect plug-in), params?: {…} (missing keys take the dialog defaults), item?: appearance item index|null (apply to that fill/stroke only; omitted: the Appearance panel's active item, else the whole object), ids?|id?: [..] (layers too), target?: \"object\"|\"contents\" (contents: the objects inside groups and layers)} append a live effect to each selected object's appearance (a group's or layer's apply to its members as one piece: one combined shadow; a Pathfinder effect, which combines a group's contents, groups several loose selected objects first, in the same undo step) → {ids, index, item, grouped?}", + "{effect: id (see effect.list, e.g. \"stylize.dropShadow\", \"distort.roughen\", \"warp.arc\", or \"plugin.\" for an installed effect plug-in), params?: {…} (missing keys take the dialog defaults), item?: appearance item index|null (apply to that fill/stroke only; omitted: the Appearance panel's active item, else the whole object), ids?|id?: [..] (layers too), target?: \"object\"|\"contents\" (contents: the objects inside groups and layers)} append a live effect to each selected object's appearance (a group's or layer's apply to its members as one piece: one combined shadow; a Pathfinder effect, which combines a group's contents, groups several loose selected objects first, in the same undo step); the art of a graph design in a graph series is left out, and objects that are all such art are an error (docs/mcp.md) → {ids, index, item, grouped?}", has_doc, apply ), @@ -298,7 +298,7 @@ fn ids_json(ids: &[NodeId]) -> Value { /// edits its equal item, or its equal object-level effect, each target matched on its own whatever /// the other targets have; in a target without them, `item` and `index` stand for themselves. pub(crate) fn effect_targets(s: &Session, p: &Value, item: ItemTarget, index: usize, cmd: &str) -> Result, usize)>> { - let roots = appearance_targets(s, p)?; + let roots = edit_targets(s, p)?; let d = &s.doc()?.doc; let per = item.per_target(s, &roots); let first = roots.first().and_then(|f| d.node(*f)); @@ -453,7 +453,7 @@ pub(crate) fn apply(s: &mut Session, p: &Value) -> Result { } let item = item_target(s, p, C)?; if effects::is_three_d(id) { - let targets = appearance_targets(s, p)?; + let targets = edit_targets(s, p)?; for (target, at) in targets.iter().copied().zip(item.per_target(s, &targets)) { let d = &s.doc()?.doc; let n = d.node(target).ok_or(EngineError::NoNode(target))?; @@ -492,8 +492,9 @@ fn loose_for_pathfinder(s: &Session, p: &Value, id: &str) -> Result= 2 && !grouped).then_some(roots)) } @@ -606,7 +607,7 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { let src = item_target_at(s, p, "fromItem", C)?; let dst = if p.get("toItem").is_some() { item_target_at(s, p, "toItem", C)? } else { src }; let copy = switch_or(p, "copy", false, C)?; - let roots = appearance_targets(s, p)?; + let roots = edit_targets(s, p)?; // Where an aligned effect would go in each of several different stacks isn't one place. if aligned(p, C)? && super::appearance::appearances_differ(&s.doc()?.doc, &roots) { return Err(bad(C, "effects of objects whose appearances differ can't be moved together")); diff --git a/crates/engine/src/cmd/effectcmd/graphics.rs b/crates/engine/src/cmd/effectcmd/graphics.rs index 3f95ae0518..9f4b89d2f4 100644 --- a/crates/engine/src/cmd/effectcmd/graphics.rs +++ b/crates/engine/src/cmd/effectcmd/graphics.rs @@ -7,7 +7,7 @@ use serde_json::{Map, Value, json}; use vectorcraft_doc::{Effect, Node}; use vectorcraft_render::effects; -use super::super::appearance::{appearance_targets, index_param, item_target}; +use super::super::appearance::{edit_targets, index_param, item_target}; use super::super::brushsym::{normalize, selection_art, store_symbol}; use super::super::{CommandSpec, EngineError, NodeId, Result, Session, bad, cmd, has_doc, has_selection, str_param}; use super::{effect_targets, ids_json}; @@ -95,7 +95,7 @@ fn map_graphic(s: &mut Session, p: &Value) -> Result { effect_targets(s, p, item, index, C)? } else { let mut out = vec![]; - for id in appearance_targets(s, p)? { + for id in edit_targets(s, p)? { let (item, index) = d.node(id).and_then(first_three_d).ok_or_else(|| bad(C, format!("object {} needs a 3D effect", id.0)))?; out.push((id, item, index)); } diff --git a/crates/engine/src/cmd/expand.rs b/crates/engine/src/cmd/expand.rs index d4a6c60802..02dbad5d23 100644 --- a/crates/engine/src/cmd/expand.rs +++ b/crates/engine/src/cmd/expand.rs @@ -25,7 +25,7 @@ use super::dimension::EndedLink; use super::edit::selected_roots; use super::menucmds::{run_raw, squash}; use super::object::as_clipping_path; -use super::pathops::{assemble, node_path, outline_strokes, replace_leaves, styled_copy, with_transparency}; +use super::pathops::{assemble, node_path, outlinable, outline_strokes, replace_leaves, styled_copy, with_transparency}; use super::{CommandSpec, EngineError, Result, Session, bad, cmd, has_selection, number_or, switch_or, text_param}; const C: &str = "object.expand"; @@ -47,7 +47,7 @@ pub fn specs() -> Vec { "Expand…", ["Object"], None, - "{object?: true (symbol instances without effects (their fills' and strokes' included) and not in perspective → their art, as Break Link to Symbol places it (the instances that art holds stay instances), type → outlines, live shapes → paths, effects baked, blends → groups of their steps, repeats → groups of their instances, Intertwine groups → groups of their objects and of the parts drawn over the others (clip groups), envelopes → their distorted content, placed documents → editable copies of their art, as placing the file without link gives, its symbols, patterns, swatches and images joining the document), fill?: true (gradient fills → `gradient` art, pattern fills → their tiles in a clip group shaped like the object), stroke?: true (strokes → filled outlines, the stroked paths that baking effects leaves included), gradient?: \"objects\" (default: `steps` solid strips, concentric ellipses for radial gradients, wedges around the center for conic ones, nested squares for diamond ones) | \"mesh\" (a gradient mesh); both in a clip group shaped like the object (freeform gradients always become a mesh shaped like it), steps?: 1..1000 (255; a gradient that repeats or reflects gets the same whole number of them in each run, at least one, so a run starts on an object's edge; one that needs more than 256 runs across the box stays a gradient)} one undo step; a dimension whose art an option changes becomes a plain group of its art, except one selected with art it measures, directly or in groups without fills, strokes or effects of their own: its lines and label stay as they are, and if the new art no longer fits its link it stays where it was, no longer linked → {ids}", + "{object?: true (symbol instances without effects (their fills' and strokes' included) and not in perspective → their art, as Break Link to Symbol places it (the instances that art holds stay instances), type → outlines (its own fills and strokes on paths of its whole outline, which Fill and Stroke expand), live shapes → paths, effects baked, blends → groups of their steps, repeats → groups of their instances, Intertwine groups → groups of their objects and of the parts drawn over the others (clip groups), envelopes → their distorted content, placed documents → editable copies of their art, as placing the file without link gives, its symbols, patterns, swatches and images joining the document), fill?: true (gradient fills → `gradient` art, pattern fills → their tiles in a clip group shaped like the object), stroke?: true (strokes → filled outlines, the stroked paths that baking effects leaves included), gradient?: \"objects\" (default: `steps` solid strips, concentric ellipses for radial gradients, wedges around the center for conic ones, nested squares for diamond ones) | \"mesh\" (a gradient mesh); both in a clip group shaped like the object (freeform gradients always become a mesh shaped like it), steps?: 1..1000 (255; a gradient that repeats or reflects gets the same whole number of them in each run, at least one, so a run starts on an object's edge; one that needs more than 256 runs across the box stays a gradient)} one undo step; a dimension whose art an option changes becomes a plain group of its art, except one selected with art it measures, directly or in groups without fills, strokes or effects of their own: its lines and label stay as they are, and if the new art no longer fits its link it stays where it was, no longer linked → {ids}", has_selection, expand ), @@ -56,7 +56,7 @@ pub fn specs() -> Vec { "Expand Options", [], None, - "{} what each Expand option would change in the selection → {object, fill, stroke, gradient, appearance, symbolArt?: {fill, stroke, gradient}} (true: something to expand; gradient: a gradient fill that Fill expands (in type, on the outline Object makes of a character), which Expand Gradient To applies to; appearance: appearance attributes Expand Appearance expands first, for which Object › Expand… is dimmed in the menus; symbolArt, when the selection holds symbol instances that Object expands: true when only their art holds it, which Fill and Stroke reach only with Object on)", + "{} what each Expand option would change in the selection → {object, fill, stroke, gradient, appearance, symbolArt?: {fill, stroke, gradient}} (true: something to expand; gradient: a gradient fill that Fill expands (in type, on the outline Object makes of a character, or of the whole type for the type object's own fills), which Expand Gradient To applies to; appearance: appearance attributes Expand Appearance expands first, for which Object › Expand… is dimmed in the menus; symbolArt, when the selection holds symbol instances that Object expands: true when only their art holds it, which Fill and Stroke reach only with Object on)", has_selection, |s, _| { let roots = selected_roots(s)?; @@ -189,21 +189,37 @@ impl Expandable { e.placed |= matches!(c.kind, NodeKind::PlacedDocument(_)); e.repeats |= matches!(c.kind, NodeKind::Repeat(_)); e.intertwines |= matches!(c.kind, NodeKind::Intertwine { .. }); + // Type's outlines take its characters' stroke and fill, and a path of its whole outline + // takes the type object's own fills and strokes (see `type.createOutlines`). + // Any stroke Outline Stroke outlines, not only the topmost one. + let own_stroke = c.appearance.items.iter().any(|i| matches!(i, AppearanceItem::Stroke(st) if outlinable(st))); + let own_pattern = c.appearance.items.iter().any(|i| pattern_fill(d, i).is_some()); + let own_gradient = c.appearance.items.iter().any(|i| gradient_fill(i).is_some()); + // Type without an outline (empty, or only spaces) gets no path for its own fills and strokes. + let whole = match &c.kind { + NodeKind::Text(t) if own_stroke || own_pattern || own_gradient => text_whole_outline(t), + _ => None, + }; e.stroke |= match &c.kind { - // Type's outlines take its characters' stroke (see `type.createOutlines`). - NodeKind::Text(t) => t.runs.iter().any(|r| r.style.has_stroke()), - _ => shape && !c.appearance.stroke_paint().is_none(), + NodeKind::Text(t) => (own_stroke && whole.is_some()) || t.runs.iter().any(|r| r.style.has_stroke()), + _ => shape && own_stroke, }; // Once one is found, the next path is not measured. e.gradient = e.gradient || match &c.kind { - NodeKind::Text(t) => text_gradient_expands(d, t), + NodeKind::Text(t) => { + text_gradient_expands(d, t) + || (own_gradient && whole.as_ref().is_some_and(|(path, tb)| text_object_gradient_expands(d, c, path, *tb))) + } _ => shape && gradient_expands(d, c), }; e.fill |= e.gradient; e.fill |= match &c.kind { - NodeKind::Text(t) => t.runs.iter().any(|r| matches!(&r.style.fill, Paint::Pattern { pattern, .. } if d.pattern(pattern).is_some())), - _ => shape && c.appearance.items.iter().any(|i| pattern_fill(d, i).is_some()), + NodeKind::Text(t) => { + (own_pattern && whole.is_some()) + || t.runs.iter().any(|r| matches!(&r.style.fill, Paint::Pattern { pattern, .. } if d.pattern(pattern).is_some())) + } + _ => shape && own_pattern, }; }); } @@ -275,6 +291,33 @@ fn text_gradient_expands(d: &Document, t: &TextObject) -> bool { || vectorcraft_text::decorations(&lay, vectorcraft_text::FontDb::global(), t).iter().any(|(run, bar)| expands(*run, bar)) } +/// The path of type `t`'s whole outline in the document (glyphs, ruby glyphs and underline and +/// strikethrough bars), which `type.createOutlines` paints with the type object's own fills and +/// strokes, and the layout's bounds there; `None` when the type has no outline. +fn text_whole_outline(t: &TextObject) -> Option<(PathData, Rect)> { + let db = vectorcraft_text::FontDb::global(); + let lay = vectorcraft_text::layout(db, t); + let mut all = BezPath::new(); + for g in lay.glyphs.iter().chain(&lay.ruby) { + all.extend(g.outline.iter()); + } + for (_, bar) in vectorcraft_text::decorations(&lay, db, t) { + all.extend(bar.iter()); + } + let path = PathData::from_bezpath(&all).transformed(t.xf); + (!path.is_empty()).then(|| (path, t.xf.transform_rect_bbox(lay.bounds))) +} + +/// Whether type node `n` has a gradient fill of its own (not its characters') that Fill expands +/// once Object has outlined it: [`gradient_expands`] on `path`, the type's whole outline +/// ([`text_whole_outline`]), with its unplaced gradients fixed to `tb`, the layout's bounds in the +/// document. Leaves the document as it is. +fn text_object_gradient_expands(d: &Document, n: &Node, path: &PathData, tb: Rect) -> bool { + let mut appearance = Appearance { items: n.appearance.items.clone(), ..Default::default() }; + appearance.pin_gradients(tb); + gradient_expands(d, &Node::path(NodeId(u64::MAX), path.clone(), appearance)) +} + /// [`gradient_expands`] for a path that no live-effect plug-in paints ([`expand_piece_fills`]). fn piece_gradient_expands(d: &Document, l: &Node) -> bool { if !l.appearance.items.iter().filter_map(gradient_fill).any(|(_, g)| g.gradient.is_spread()) { diff --git a/crates/engine/src/cmd/fileio.rs b/crates/engine/src/cmd/fileio.rs index 9154ce7ee5..d20c6c46b2 100644 --- a/crates/engine/src/cmd/fileio.rs +++ b/crates/engine/src/cmd/fileio.rs @@ -123,7 +123,7 @@ mod formats; #[cfg(not(target_arch = "wasm32"))] pub(crate) use access::list_dir; -pub use access::{write_file_with, write_save_file, write_save_file_with}; +pub use access::{check_linked_scripts, write_file_with, write_save_file, write_save_file_with}; #[cfg(all(test, not(target_arch = "wasm32")))] mod tests_readonly; diff --git a/crates/engine/src/cmd/fileio/access.rs b/crates/engine/src/cmd/fileio/access.rs index 6488972d31..203a2262b0 100644 --- a/crates/engine/src/cmd/fileio/access.rs +++ b/crates/engine/src/cmd/fileio/access.rs @@ -121,6 +121,24 @@ pub(crate) fn naming_in_message(msg: &str, path: &str, name: &str) -> String { out } +/// The error for an SVG of a document that links a JavaScript file outside the read roots. +pub(crate) const SCRIPT_OUTSIDE: &str = "a linked JavaScript file is outside the read roots"; + +/// `cmd` may write `st`'s document in format `format` (a format id): any format but SVG and SVGZ, +/// and those too unless read roots are in force and the document links a JavaScript file outside +/// them (`svg_scripts`, a relative path taken from the document's folder; a URL is not checked), +/// which the file's `