Merge PR #8053: Pencil: align 1 px click snapping with the line walker (#4895)

crates/paint/src/brush_tests.rs

@@ -155,6 +155,8 @@ fn pencil_dabs_sit_on_the_pixel_grid() {
     // Odd sizes centre on the pixel holding the point, even ones on the nearest corner.
     assert_eq!(grid_center(10.0, 5.0, 1.0), (10.5, 5.5));
     assert_eq!(grid_center(10.9, 5.2, 3.0), (10.5, 5.5));
+    assert_eq!(grid_center(10.0 - 5e-8, 5.0 - 5e-8, 1.0), (10.5, 5.5));
+    assert_eq!(grid_center(10.0 - 2e-7, 5.0 - 2e-7, 1.0), (9.5, 4.5));
     assert_eq!(grid_center(10.4, 5.6, 2.0), (10.0, 6.0));
     assert_eq!(grid_square(10.2, 5.7, 1.0), [10.0, 5.0, 11.0, 6.0]);
     assert_eq!(grid_square(10.2, 5.7, 4.0), [8.0, 4.0, 12.0, 8.0]);

crates/paint/src/dynamics.rs

@@ -29,8 +29,8 @@ const MAX_AIRBRUSH_DABS_PER_SEGMENT: usize = 512;
 
 /// The dab centre of the pixel-grid cell holding `v` (see [`PathWalker::pixel_grid`]): the
 /// pixel's centre for `offset` 0.5, the nearest pixel corner for 0. A value within rounding
-/// noise of a cell boundary counts as on it, so a line lands on the same pixels both ways.
-fn grid_cell(v: f64, offset: f64) -> f64 {
+/// noise of a cell boundary counts as on it, so snapping and line stepping agree.
+pub(crate) fn grid_cell(v: f64, offset: f64) -> f64 {
     let w = v - offset + 0.5;
     let w = if (w - w.round()).abs() < 1e-7 { w.round() } else { w };
     w.floor() + offset

crates/paint/src/render.rs

@@ -15,7 +15,7 @@ use photocraft_geom::Rect;
 use photocraft_raster::{Surface, from_rgba_into, to_rgba};
 
 use crate::brush::{BrushSettings, MaskMode, Pattern, TipShape};
-use crate::dynamics::DabGenerator;
+use crate::dynamics::{DabGenerator, grid_cell};
 use crate::retouch::{Footprint, alpha_index, over_native};
 use crate::rng::hash2;
 use crate::tile::{Mips, PatternImage};
@@ -52,7 +52,7 @@ const NOISE_SALT: u64 = 0x006E_6F69_7365;
 /// the Pencil cursor shows ([`grid_square`]).
 pub fn grid_center(x: f64, y: f64, diameter: f32) -> (f32, f32) {
     let n = if diameter.is_finite() { diameter.round().max(1.0) as i64 } else { 1 };
-    let snap = |v: f64| if n % 2 == 1 { v.floor() + 0.5 } else { v.round() };
+    let snap = |v: f64| if n % 2 == 1 { grid_cell(v, 0.5) } else { v.round() };
     (snap(x) as f32, snap(y) as f32)
 }
 

crates/ui-egui/src/pencil_tests.rs

@@ -101,6 +101,50 @@ fn shift_click_draws_a_pencil_line() {
     assert_eq!(app.session.active().unwrap().history.past_len(), 2);
 }
 
+#[test]
+fn shift_clicked_one_pixel_line_uses_the_clicked_pixels_not_subpixel_positions() {
+    let fractions = [("TL", (0.05, 0.05)), ("TR", (0.95, 0.05)), ("BL", (0.05, 0.95)), ("BR", (0.95, 0.95)), ("C", (0.5, 0.5))];
+    let starts = [&fractions[0], &fractions[1], &fractions[2], &fractions[3]];
+
+    for (start_name, start_fraction) in starts {
+        for (end_name, end_fraction) in fractions {
+            let paint = |raw: bool| {
+                let mut app = app();
+                app.run("tools.setBrush", json!({"brush": {"size": 1.0, "hardness": 1.0}})).unwrap();
+                let at = |origin: f64, fraction: f64| origin + if raw { fraction } else { 0.5 };
+                let start = (at(10.0, start_fraction.0), at(10.0, start_fraction.1));
+                let end = (at(60.0, end_fraction.0), at(45.0, end_fraction.1));
+                for (point, mods) in [(start, Modifiers::NONE), (end, Modifiers::SHIFT)] {
+                    tool_event(&mut app, ToolEvent::Down { x: point.0, y: point.1, pressure: 1.0 }, mods);
+                    tool_event(&mut app, ToolEvent::Up { x: point.0, y: point.1 }, mods);
+                }
+                (0..80).flat_map(|y| (0..120).map(move |x| (x, y))).filter(|&(x, y)| rgba(&app, x, y)[3] > 0.0).collect::<Vec<_>>()
+            };
+
+            assert_eq!(paint(true), paint(false), "{start_name}->{end_name}");
+        }
+    }
+}
+
+#[test]
+fn shift_clicked_pencil_does_not_split_a_click_near_a_pixel_boundary() {
+    let mut app = app();
+    app.run("tools.setBrush", json!({"brush": {"size": 1.0, "hardness": 1.0}})).unwrap();
+    let click = |app: &mut PhotocraftApp, x: f64, y: f64, modifiers| {
+        tool_event(app, ToolEvent::Down { x, y, pressure: 1.0 }, modifiers);
+        tool_event(app, ToolEvent::Up { x, y }, modifiers);
+    };
+
+    click(&mut app, 30.5, 30.5, Modifiers::NONE);
+    // The pointer is just below the grid corner; numerical rounding must not make the Pencil
+    // snap to one pixel and the line walker rasterize the neighbouring one.
+    let just_below_corner = 30.0 - 5e-8;
+    click(&mut app, just_below_corner, just_below_corner, Modifiers::SHIFT);
+
+    assert_eq!(rgba(&app, 29, 29)[3], 0.0, "the click should remain in the pixel chosen by the line walker");
+    assert_eq!(rgba(&app, 30, 30)[3], 1.0);
+}
+
 #[test]
 fn ctrl_alt_drag_resizes_the_pencil_without_painting() {
     let mut app = app();