mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-08 10:40:45 +00:00
Merge pull request #5748 from Silvertree2010/fix/5729-bug-pencil-tool-options-wrong-fi
Keep Pencil and Paintbrush Fidelity in screen pixels at any zoom
This commit is contained in:
@@ -12,7 +12,7 @@ use super::{
|
||||
/// Fit a freehand polyline: drop jitter, build a spline through the samples (sharp turns stay
|
||||
/// corners) and simplify it to `tol`.
|
||||
fn fit_freehand(pts: &[Point], tol: f64, closed: bool) -> SubPath {
|
||||
let min = (tol * 0.3).max(vectorcraft_tools::draw2::MIN_FIDELITY * 0.3);
|
||||
let min = (tol * 0.3).max(vectorcraft_tools::draw2::MIN_FIT_TOLERANCE * 0.3);
|
||||
let (Some(&p0), Some(&pn)) = (pts.first(), pts.last()) else { return SubPath::new(vec![], closed) };
|
||||
let mut kept: Vec<Point> = vec![p0];
|
||||
for &p in pts.iter().skip(1) {
|
||||
@@ -150,7 +150,7 @@ pub(super) fn freehand(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
}
|
||||
let pen = PenRecord::of_samples(&samples);
|
||||
let mut pts: Vec<Point> = samples.into_iter().map(|s| s.pt).collect();
|
||||
let tol = number_or(p, "fidelity", 1.5, C)?.clamp(vectorcraft_tools::draw2::MIN_FIDELITY, 100.0);
|
||||
let tol = number_or(p, "fidelity", 1.5, C)?.clamp(vectorcraft_tools::draw2::MIN_FIT_TOLERANCE, 100.0);
|
||||
// The tolerance the stroke is fitted at; with `fit: false` it is not fitted.
|
||||
let fit_tol = switch_or(p, "fit", true, C)?.then_some(tol);
|
||||
let closed = switch_or(p, "closed", false, C)?;
|
||||
|
||||
@@ -3999,11 +3999,17 @@ fn pen_continue_at_a_first_anchor_in_a_compound_reverses_only_that_subpath() {
|
||||
/// The anchors of a closed, slightly wobbly blob drawn at `fidelity`: samples half a point apart,
|
||||
/// with a small tremor on top of the outline.
|
||||
fn blob_anchors(s: &mut Session, fidelity: f64) -> usize {
|
||||
scaled_blob_anchors(s, fidelity, 1.0)
|
||||
}
|
||||
|
||||
/// [`blob_anchors`] with the blob scaled by `scale`: drawn the same on screen zoomed in 1 / `scale`
|
||||
/// times.
|
||||
fn scaled_blob_anchors(s: &mut Session, fidelity: f64, scale: f64) -> usize {
|
||||
let pts: Vec<_> = (0..1200)
|
||||
.map(|i| {
|
||||
let a = i as f64 / 1200.0 * std::f64::consts::TAU;
|
||||
let r = 90.0 + 12.0 * (3.0 * a).sin() + 0.15 * (97.0 * a).sin();
|
||||
json!([400.0 + r * a.cos(), 300.0 + r * a.sin()])
|
||||
json!([(400.0 + r * a.cos()) * scale, (300.0 + r * a.sin()) * scale])
|
||||
})
|
||||
.collect();
|
||||
let r = s.execute("path.freehand", &json!({"points": pts, "closed": true, "fidelity": fidelity, "keepSelected": false})).unwrap();
|
||||
@@ -4018,9 +4024,26 @@ fn pencil_fidelity_goes_down_to_a_hundredth_and_lower_keeps_more() {
|
||||
assert!(counts.windows(2).all(|w| w[0] <= w[1]), "{counts:?}");
|
||||
// Below the old floor of 0.05 the stroke keeps more of itself.
|
||||
assert!(counts[4] > counts[2], "{counts:?}");
|
||||
// Below 0.01 is read as 0.01.
|
||||
assert_eq!(blob_anchors(&mut s, 0.001), counts[4]);
|
||||
assert_eq!(blob_anchors(&mut s, 0.0), counts[4]);
|
||||
// Below the floor, the lowest Fidelity (0.01 px) at the highest zoom (64000 %), is read as the
|
||||
// floor (#5729).
|
||||
let floor = vectorcraft_tools::draw2::MIN_FIDELITY / 640.0;
|
||||
let at_floor = blob_anchors(&mut s, floor);
|
||||
assert!(at_floor >= counts[4], "{counts:?}");
|
||||
assert_eq!(blob_anchors(&mut s, floor / 10.0), at_floor);
|
||||
assert_eq!(blob_anchors(&mut s, 0.0), at_floor);
|
||||
}
|
||||
|
||||
/// #5729: the tools send Fidelity / zoom, so a stroke drawn the same on screen zoomed in keeps as
|
||||
/// many anchors, down to the lowest Fidelity at the highest zoom.
|
||||
#[test]
|
||||
fn pencil_fit_keeps_its_anchors_on_a_stroke_drawn_zoomed_in() {
|
||||
let mut s = session();
|
||||
for fidelity in [1.5, 0.1, 0.01] {
|
||||
let want = blob_anchors(&mut s, fidelity);
|
||||
for zoom in [4.0, 64.0, 640.0] {
|
||||
assert_eq!(scaled_blob_anchors(&mut s, fidelity / zoom, 1.0 / zoom), want, "{fidelity} at {zoom}x");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// At Fidelity 0.01 short, one-point and repeated-point strokes still draw or are refused, never
|
||||
|
||||
@@ -26,10 +26,14 @@ pub use gesture::GestureTool;
|
||||
/// Colour of tool feedback (Illustrator's selection blue).
|
||||
pub const FEEDBACK: [u8; 3] = [0x4f, 0x9d, 0xff];
|
||||
|
||||
/// The lowest Fidelity (points) the Pencil and Paintbrush take (#3328); the Smooth tool keeps
|
||||
/// [`SMOOTH_MIN_FIDELITY`].
|
||||
/// The lowest Fidelity the Pencil and Paintbrush take (#3328; screen pixels, points for the Blob
|
||||
/// Brush); the Smooth tool keeps [`SMOOTH_MIN_FIDELITY`].
|
||||
pub const MIN_FIDELITY: f64 = 0.01;
|
||||
|
||||
/// The finest tolerance (points) `path.freehand` fits a Pencil or Paintbrush stroke at: the lowest
|
||||
/// Fidelity at the highest zoom (64000 %), whose screen pixels the tools send in points (#5729).
|
||||
pub const MIN_FIT_TOLERANCE: f64 = MIN_FIDELITY / 640.0;
|
||||
|
||||
/// The lowest Fidelity (points) the Smooth tool takes.
|
||||
pub const SMOOTH_MIN_FIDELITY: f64 = 0.05;
|
||||
|
||||
|
||||
@@ -66,7 +66,9 @@ pub struct GestureTool {
|
||||
/// Pencil/Paintbrush starting on a selected path away from its ends: (path, Edit Selected
|
||||
/// Paths' distance in points); the stroke redraws the section it ends on (#2850).
|
||||
redraw: Option<(NodeId, f64)>,
|
||||
/// Fit tolerance in points (Pencil/Paintbrush/Smooth/Blob Brush "Fidelity").
|
||||
/// Fit tolerance (Pencil/Paintbrush/Smooth/Blob Brush "Fidelity"): in screen pixels for the
|
||||
/// Pencil and the Paintbrush, so a stroke keeps as many points at any zoom (#5729), in points
|
||||
/// for the others.
|
||||
pub fidelity: f64,
|
||||
/// Brush / eraser diameter in points (the Eraser's across its tip's long axis).
|
||||
pub size: f64,
|
||||
@@ -453,11 +455,12 @@ impl GestureTool {
|
||||
self.draws_path() && !self.smoothing && !self.straight && self.extend.is_none() && self.redraw.is_none() && self.ends_near_start(cx, end)
|
||||
}
|
||||
|
||||
/// Parameters of `path.freehand` for the current stroke.
|
||||
pub fn freehand_params(&self, closed: bool) -> Value {
|
||||
/// Parameters of `path.freehand` for the current stroke, fitted at Fidelity on screen. Only the
|
||||
/// Pencil and the Paintbrush (`draws_path`) send it; the Blob Brush's Fidelity stays in points.
|
||||
pub fn freehand_params(&self, cx: &ToolContext, closed: bool) -> Value {
|
||||
let mut v = json!({
|
||||
"points": self.stroke_json(),
|
||||
"fidelity": self.fidelity,
|
||||
"fidelity": cx.tol(self.fidelity),
|
||||
"closed": closed,
|
||||
"style": if self.id == "paintbrush" { "brush" } else { "pencil" },
|
||||
"fill": self.fill,
|
||||
@@ -494,7 +497,7 @@ impl GestureTool {
|
||||
let near = pts.last().is_some_and(|&end| self.ends_near_start(cx, end));
|
||||
// A straight line has two points: closing it would draw it back over itself.
|
||||
let closed = !self.straight && ((m.alt && (self.id == "paintbrush" || !self.alt_smooth)) || (self.extend.is_none() && near));
|
||||
let mut params = self.freehand_params(closed);
|
||||
let mut params = self.freehand_params(cx, closed);
|
||||
// A Pencil stroke that continues a selected path and ends on an open end of another
|
||||
// path selected at the press connects the two.
|
||||
let connect = pts.last().and_then(|&end| self.find_end(cx, end, &self.connect_to));
|
||||
@@ -649,9 +652,9 @@ impl Tool for GestureTool {
|
||||
}
|
||||
// With the Pencil's Live preview off a thin line shows instead (`overlays`).
|
||||
if self.previews() && crate::previews_at(self.points.len()) {
|
||||
let mut v = self.freehand_params(false);
|
||||
let fidelity = self.fidelity.max(cx.tol(PREVIEW_FIDELITY_PX));
|
||||
v["fidelity"] = json!(fidelity);
|
||||
let mut v = self.freehand_params(cx, false);
|
||||
let fidelity = self.fidelity.max(PREVIEW_FIDELITY_PX);
|
||||
v["fidelity"] = json!(cx.tol(fidelity));
|
||||
// A preview fitted coarser than the stroke kept is stale (`deactivate`).
|
||||
self.previewed = if fidelity > self.fidelity { 0 } else { self.points.len() };
|
||||
if !self.live_fit {
|
||||
@@ -748,13 +751,13 @@ impl Tool for GestureTool {
|
||||
fn hover_ended(&mut self, _cx: &ToolContext) {
|
||||
self.tip_at = None;
|
||||
}
|
||||
fn deactivate(&mut self, _cx: &ToolContext) -> Vec<Action> {
|
||||
fn deactivate(&mut self, cx: &ToolContext) -> Vec<Action> {
|
||||
let began = self.began;
|
||||
self.tip_at = None;
|
||||
// Drags past the last preview are in the stroke kept, too, and with Live curve fitting
|
||||
// off the stroke kept is fitted.
|
||||
let stale = (began && (self.previewed != self.points.len() || !self.live_fit) && self.draws_path() && !self.smoothing)
|
||||
.then(|| Action::Preview("path.freehand".into(), self.freehand_params(false)));
|
||||
.then(|| Action::Preview("path.freehand".into(), self.freehand_params(cx, false)));
|
||||
self.snap.clear();
|
||||
self.active = false;
|
||||
self.near_start = false;
|
||||
|
||||
@@ -154,7 +154,8 @@ fn pencil_with_live_curve_fitting_off_previews_the_stroke_unfitted() {
|
||||
|
||||
/// A stroke whose Fidelity is finer than half a screen pixel previews fitted at half a pixel, which
|
||||
/// looks the same at a fraction of the fit's cost (#4565); the release, and another tool chosen
|
||||
/// while it is drawn, fit it at its own Fidelity. A coarser Fidelity previews as it is kept.
|
||||
/// while it is drawn, fit it at its own Fidelity. A coarser Fidelity previews as it is kept. Fidelity
|
||||
/// is in screen pixels, so the stroke is fitted in points at Fidelity / zoom (#5729).
|
||||
#[test]
|
||||
fn fine_fidelity_previews_at_half_a_pixel() {
|
||||
let (d, _) = doc_with_rect();
|
||||
@@ -166,26 +167,83 @@ fn fine_fidelity_previews_at_half_a_pixel() {
|
||||
acts.iter().filter_map(|a| if let Action::Preview(_, v) = a { v["fidelity"].as_f64() } else { None }).collect()
|
||||
};
|
||||
for id in ["pencil", "paintbrush"] {
|
||||
for (zoom, fidelity, drawn) in [(1.0, 0.01, 0.5), (4.0, 0.01, 0.125), (1.0, 1.5, 1.5)] {
|
||||
for (zoom, fidelity, drawn, kept) in [(1.0, 0.01, 0.5, 0.01), (4.0, 0.01, 0.125, 0.0025), (1.0, 1.5, 1.5, 1.5), (4.0, 1.5, 0.375, 0.375)] {
|
||||
cx.zoom = zoom;
|
||||
let mut t = GestureTool::new(id);
|
||||
t.set_option("fidelity", &json!(fidelity));
|
||||
let acts = drag(&mut t, &cx, &stroke, Mods::default());
|
||||
let (release, during) = acts.split_last().unwrap();
|
||||
assert_eq!(fidelities(&during.concat()), [drawn; 3], "{id} at {zoom}x, {fidelity} pt");
|
||||
assert_eq!(fidelities(release), [fidelity], "{id} at {zoom}x, {fidelity} pt: the release");
|
||||
assert_eq!(fidelities(&during.concat()), [drawn; 3], "{id} at {zoom}x, {fidelity} px");
|
||||
assert_eq!(fidelities(release), [kept], "{id} at {zoom}x, {fidelity} px: the release");
|
||||
t.pointer(&cx, &PointerEvent::new(PointerKind::Down, 0.0, 0.0));
|
||||
for &(x, y) in &stroke[1..] {
|
||||
t.pointer(&cx, &PointerEvent::new(PointerKind::Drag, x, y));
|
||||
}
|
||||
let acts = t.deactivate(&cx);
|
||||
let kept = if drawn > fidelity { [fidelity].to_vec() } else { vec![] };
|
||||
assert_eq!(fidelities(&acts), kept, "{id} at {zoom}x, {fidelity} pt: another tool chosen");
|
||||
let stale = if drawn > kept { [kept].to_vec() } else { vec![] };
|
||||
assert_eq!(fidelities(&acts), stale, "{id} at {zoom}x, {fidelity} px: another tool chosen");
|
||||
assert_eq!(acts.last(), Some(&Action::Commit));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// #5729: the Pencil's and the Paintbrush's Fidelity is in screen pixels. The same stroke drawn on
|
||||
/// screen at 1x and at 4x is fitted at the same tolerance on screen, so it keeps as many points
|
||||
/// whatever the zoom (another tool chosen mid-stroke: `fine_fidelity_previews_at_half_a_pixel`).
|
||||
#[test]
|
||||
fn fidelity_is_in_screen_pixels_at_any_zoom() {
|
||||
let (d, _) = doc_with_rect();
|
||||
let s = Selection::default();
|
||||
let p = paint();
|
||||
let mut cx = cx(&d, &s, &p);
|
||||
let screen = [(0.0, 0.0), (40.0, 20.0), (80.0, 0.0), (120.0, 20.0)];
|
||||
for id in ["pencil", "paintbrush"] {
|
||||
let on_screen = |cx: &ToolContext, zoom: f64| {
|
||||
let pts: Vec<_> = screen.iter().map(|&(x, y)| (x / zoom, y / zoom)).collect();
|
||||
let mut t = GestureTool::new(id);
|
||||
t.set_option("fidelity", &json!(1.5));
|
||||
let acts = drag(&mut t, cx, &pts, Mods::default());
|
||||
let Some(Action::Preview(_, v)) = acts.last().and_then(|a| a.first()) else { panic!("{acts:?}") };
|
||||
v["fidelity"].as_f64().unwrap() * zoom
|
||||
};
|
||||
cx.zoom = 1.0;
|
||||
let at_1 = on_screen(&cx, 1.0);
|
||||
cx.zoom = 4.0;
|
||||
let at_4 = on_screen(&cx, 4.0);
|
||||
assert_eq!(at_1, 1.5, "{id}");
|
||||
assert_eq!(at_4, at_1, "{id}: fidelity in screen px at 1x vs 4x");
|
||||
}
|
||||
}
|
||||
|
||||
/// The Blob Brush's Fidelity stays in points (#5729 is the Pencil's and the Paintbrush's): its
|
||||
/// stroke, and another tool chosen while it is drawn, send it as set at any zoom.
|
||||
#[test]
|
||||
fn blob_brush_fidelity_stays_in_points_at_any_zoom() {
|
||||
let (d, _) = doc_with_rect();
|
||||
let s = Selection::default();
|
||||
let p = paint();
|
||||
let mut cx = cx(&d, &s, &p);
|
||||
let stroke = [(0.0, 0.0), (40.0, 20.0), (80.0, 0.0), (120.0, 20.0)];
|
||||
for zoom in [1.0, 4.0] {
|
||||
cx.zoom = zoom;
|
||||
let mut t = GestureTool::new("blobBrush");
|
||||
t.set_option("fidelity", &json!(1.5));
|
||||
let acts = drag(&mut t, &cx, &stroke, Mods::default());
|
||||
let sent: Vec<_> = acts
|
||||
.concat()
|
||||
.into_iter()
|
||||
.filter_map(|a| if let Action::Exec(_, v) | Action::Preview(_, v) = a { v["fidelity"].as_f64() } else { None })
|
||||
.collect();
|
||||
assert_eq!(sent, [1.5], "at {zoom}x");
|
||||
t.pointer(&cx, &PointerEvent::new(PointerKind::Down, 0.0, 0.0));
|
||||
for &(x, y) in &stroke[1..] {
|
||||
t.pointer(&cx, &PointerEvent::new(PointerKind::Drag, x, y));
|
||||
}
|
||||
let acts = t.deactivate(&cx);
|
||||
assert!(!acts.iter().any(|a| matches!(a, Action::Preview(id, _) if id == "path.freehand")), "at {zoom}x: {acts:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Keep selected is on by default, and every Pencil and Paintbrush preview sends it.
|
||||
#[test]
|
||||
fn pencil_and_paintbrush_send_keep_selected() {
|
||||
|
||||
@@ -3,7 +3,8 @@
|
||||
//! preferences. OK sets them with `tool.setOption {tool, values}`. The Pencil also has `liveFit`
|
||||
//! (Live curve fitting, on: its preview fits the stroke while it is drawn).
|
||||
//!
|
||||
//! Fields: `tool`; `fidelity` (points; Pencil, Paintbrush, Smooth and Blob Brush), `fill` (Pencil and
|
||||
//! Fields: `tool`; `fidelity` (screen pixels for the Pencil and Paintbrush, points for Smooth and
|
||||
//! Blob Brush), `fill` (Pencil and
|
||||
//! Paintbrush), `keepSelected` (Pencil, Paintbrush and Blob Brush), `mergeOnlySelection` (Blob
|
||||
//! Brush), `closeWithin` and `editWithin` (screen pixels, 0 turns it off; Pencil and Paintbrush),
|
||||
//! `angle` (degrees) and `roundness` (percent; Blob Brush and Eraser), `livePreview` (Pencil: the stroke shows
|
||||
@@ -166,16 +167,19 @@ fn blob_size(ui: &mut egui::Ui, d: &mut Dialog) {
|
||||
fn fidelity(ui: &mut egui::Ui, d: &mut Dialog) {
|
||||
let t = Tokens::get(ui.ctx());
|
||||
let v = d.f64("fidelity", 1.5);
|
||||
let rail = (vectorcraft_tools::draw2::min_fidelity(&d.str("tool")), FIDELITY_MAX);
|
||||
let tool = d.str("tool");
|
||||
let rail = (vectorcraft_tools::draw2::min_fidelity(&tool), FIDELITY_MAX);
|
||||
// The Pencil's and the Paintbrush's Fidelity is on screen, the same at any zoom (#5729).
|
||||
let unit = if matches!(tool.as_str(), "pencil" | "paintbrush") { "px" } else { "pt" };
|
||||
let rail_w = label_w(ui);
|
||||
let mut new = None;
|
||||
ui.label(egui::RichText::new(tl!("Fidelity:")).size(12.5).color(t.text));
|
||||
ui.horizontal(|ui| {
|
||||
let shown = widgets::SliderValue::field(v, rail.0..=rail.1, "pt", 2).with_step(fidelity_step);
|
||||
let shown = widgets::SliderValue::field(v, rail.0..=rail.1, unit, 2).with_step(fidelity_step);
|
||||
let (x, _, keyed) =
|
||||
widgets::color_slider_stepped(ui, "freehand-fidelity", tl!("Fidelity:"), Some(shown), fidelity_at(v, rail), rail_w, &|_| t.input_border);
|
||||
new = x.map(|x| fidelity_from(x, rail)).or(keyed);
|
||||
if let Some(x) = widgets::range_field_stepped(ui, "freehand-fidelity-field", v, rail.0..=100.0, "pt", 2, FIELD_W, fidelity_step) {
|
||||
if let Some(x) = widgets::range_field_stepped(ui, "freehand-fidelity-field", v, rail.0..=100.0, unit, 2, FIELD_W, fidelity_step) {
|
||||
new = Some(x);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1984,7 +1984,7 @@ fn dialog_sliders_report_the_numbers_their_fields_show() {
|
||||
app.run("tool.options", json!({"tool": "pencil"})).unwrap();
|
||||
set(&mut app, "fidelity", json!(2.5));
|
||||
let shown = tree(|ui| show(&mut app, ui.ctx()));
|
||||
assert_eq!(slider_value(&shown, "Fidelity:"), v(2.5, (0.01, 20.0), "2.5pt"));
|
||||
assert_eq!(slider_value(&shown, "Fidelity:"), v(2.5, (0.01, 20.0), "2.5px"), "the Pencil's Fidelity is on screen (#5729)");
|
||||
// The Smooth tool's rail starts at its own lowest Fidelity.
|
||||
app.ui.dialog = None;
|
||||
app.run("tool.options", json!({"tool": "smooth"})).unwrap();
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -861,7 +861,7 @@ events take `pressure` (0..1, default 1): it is the Liquify intensity while Use
|
||||
|
||||
Freehand Tool Options: double-clicking the Pencil, Paintbrush, Smooth, Blob Brush or Eraser tool
|
||||
(`tool.options {tool: "pencil"}`) opens a `freehandOptions` dialog. Its fields are `tool` and the options the tool
|
||||
keeps: `fidelity` (pt; Pencil, Paintbrush, Smooth, Blob Brush), `fill` (Pencil, Paintbrush), `keepSelected` (Pencil,
|
||||
keeps: `fidelity` (screen pixels for the Pencil and Paintbrush, the same at any zoom; pt for Smooth and Blob Brush), `fill` (Pencil, Paintbrush), `keepSelected` (Pencil,
|
||||
Paintbrush, Blob Brush; on by default for the Pencil and Paintbrush and off for the Blob Brush, and while it is off
|
||||
the path a stroke draws, continues or merges into is not selected afterwards), `mergeOnlySelection` (Blob Brush;
|
||||
off by default, and while it is on a stroke merges only with selected blobs), `closeWithin` and `editWithin`
|
||||
|
||||
Reference in New Issue
Block a user