Merge pull request #12790 from tjbbjt/fix/sampler-tests-pixel-centre

Tests: expect Color Sampler points on pixel centres after #12174

crates/engine/src/cmd/artboard_cmds/tests.rs

@@ -723,6 +723,8 @@ fn moving_an_artboard_left_of_or_above_the_origin_grows_the_canvas_there() {
     }
 }
 
+/// Moving or making a board past the canvas origin re-bases the document; the Color Sampler
+/// points and the painting symmetry shift with it and stay on the same pixels.
 #[test]
 fn a_board_past_the_origin_moves_the_color_samplers_and_symmetry_with_the_document() {
     // #9302: the re-based document moved everything but the Color Sampler points and the
@@ -737,9 +739,10 @@ fn a_board_past_the_origin_moves_the_color_samplers_and_symmetry_with_the_docume
         s.execute("layer.select", json!({"layer": gid.0})).unwrap();
         s.execute(cmd, p).unwrap();
         // The board lands at (0, 0) after the document shifted right 50 and down 20; the sampler
-        // is pinned to the canvas, not the board, so it shifts with the document only.
+        // is pinned to the canvas, not the board, so it shifts with the document only. Sampler
+        // points sit on pixel centres (#11725): one placed at 15, 15 is stored as 15.5, 15.5.
         assert_eq!(board_rect(&s, gid.0), Rect::new(0, 0, 40, 40), "{cmd}");
-        assert_eq!(samplers(&s), vec![[65.0, 35.0]], "{cmd}");
+        assert_eq!(samplers(&s), vec![[65.5, 35.5]], "{cmd}");
         assert_eq!(center(&s), Some([120.0, 40.0]), "{cmd}: the symmetry stays on its pixels");
     }
     // A new board left of the canvas shifts the photo right by its overhang.
@@ -747,7 +750,7 @@ fn a_board_past_the_origin_moves_the_color_samplers_and_symmetry_with_the_docume
     add_square(&mut s, Rect::new(10, 10, 30, 30), [1.0, 0.0, 0.0, 1.0]);
     s.execute("view.colorSamplers.add", json!({"x": 15, "y": 15})).unwrap();
     s.execute("layer.new.artboard", json!({"rect": [-200, 0, 100, 80]})).unwrap();
-    assert_eq!(samplers(&s), vec![[215.0, 15.0]]);
+    assert_eq!(samplers(&s), vec![[215.5, 15.5]]);
     assert!(red_at(&s, 215, 15));
     // A board made from layers (or a group) left of the canvas shifts by their overhang.
     for cmd in ["layer.new.artboardFromLayers", "layer.new.artboardFromGroup"] {
@@ -759,7 +762,7 @@ fn a_board_past_the_origin_moves_the_color_samplers_and_symmetry_with_the_docume
             s.execute("layer.groupLayers", json!({})).unwrap();
         }
         s.execute(cmd, json!({})).unwrap();
-        assert_eq!(samplers(&s), vec![[45.0, 15.0]], "{cmd}");
+        assert_eq!(samplers(&s), vec![[45.5, 15.5]], "{cmd}");
         assert!(red_at(&s, 45, 15), "{cmd}");
     }
     // The "+" button left of a board at the origin shifts by the board and the gap.
@@ -770,13 +773,15 @@ fn a_board_past_the_origin_moves_the_color_samplers_and_symmetry_with_the_docume
     s.execute("layer.artboard.addBeside", json!({"layer": gid.0, "side": "left"})).unwrap();
     let by = 40 + GAP;
     assert_eq!(board_rect(&s, gid.0), Rect::new(by, 0, by + 40, 40));
-    assert_eq!(samplers(&s), vec![[f64::from(by) + 5.0, 5.0]]);
+    assert_eq!(samplers(&s), vec![[f64::from(by) + 5.5, 5.5]]);
     assert!(red_at(&s, by + 5, 5));
     // A board right of the canvas shifts nothing.
     s.execute("layer.artboard.addBeside", json!({"layer": gid.0, "side": "right"})).unwrap();
-    assert_eq!(samplers(&s), vec![[f64::from(by) + 5.0, 5.0]]);
+    assert_eq!(samplers(&s), vec![[f64::from(by) + 5.5, 5.5]]);
 }
 
+/// Duplicating a board whose document has boards past the origin (one or several selected
+/// layers) re-bases it; the Color Sampler points shift with the document.
 #[test]
 fn duplicating_a_board_beside_one_past_the_origin_moves_the_color_samplers_with_the_document() {
     // #9302 review: Layer › Duplicate puts the copy right of its board, but `fit_canvas` still
@@ -799,7 +804,7 @@ fn duplicating_a_board_beside_one_past_the_origin_moves_the_color_samplers_with_
             s.execute("layer.duplicate", json!({"layer": gid.0})).unwrap();
         }
         assert_eq!(board_rect(&s, gid.0), Rect::new(0, 10, 40, 50), "multi {multi}: the document shifted right 40");
-        assert_eq!(s.active().unwrap().color_samplers, vec![[105.0, 15.0]], "multi {multi}");
+        assert_eq!(s.active().unwrap().color_samplers, vec![[105.5, 15.5]], "multi {multi}");
         assert!(red_at(&s, 105, 15), "multi {multi}: the sampler is still on the square");
     }
 }

crates/ui-egui/src/tools/analysis_ui/sampler_menu_tests.rs

@@ -98,6 +98,8 @@ fn right_click_off_a_sampler_greys_every_row() {
     assert_eq!(samplers(&app).len(), 2, "nothing deleted, no sampler placed");
 }
 
+/// The right-click menu's Delete removes the sampler under the pointer, closes the menu, and
+/// the samplers after it move up a number.
 #[test]
 fn delete_removes_the_sampler_under_the_pointer_and_renumbers_the_rest() {
     let (mut app, ctx) = app();
@@ -106,12 +108,14 @@ fn delete_removes_the_sampler_under_the_pointer_and_renumbers_the_rest() {
     assert_eq!(app.ui.canvas_tool_menu.as_ref().and_then(|m| m.sampler), Some(1));
     let r = control(&mut app, &ctx, "ui.context.choose", json!({"id": SAMPLER_DELETE}));
     assert_eq!(r["ok"], json!(true), "{r}");
-    assert_eq!(samplers(&app), [[20.0, 20.0], [180.0, 90.0]], "#3 became #2");
+    assert_eq!(samplers(&app), [[20.5, 20.5], [180.5, 90.5]], "#3 became #2");
     assert!(app.ui.canvas_tool_menu.is_none(), "choosing closes the menu");
     let listed = app.run("view.colorSamplers.list", json!({})).unwrap();
     assert_eq!(listed["samplers"][1]["number"], json!(2));
 }
 
+/// Delete never removes a different sampler when the list changes while the menu is open: one
+/// deleted or moved meanwhile, or another document with a sampler at the same index and spot.
 #[test]
 fn delete_never_removes_another_sampler_after_the_list_changes_under_the_menu() {
     let (mut app, ctx) = app();
@@ -122,7 +126,7 @@ fn delete_never_removes_another_sampler_after_the_list_changes_under_the_menu()
     app.run("view.colorSamplers.delete", json!({"index": 0})).unwrap();
     let r = control(&mut app, &ctx, "ui.context.choose", json!({"id": SAMPLER_DELETE}));
     assert_eq!(r["ok"], json!(false), "Delete greys out: {r}");
-    assert_eq!(samplers(&app), [[120.0, 60.0], [180.0, 90.0]], "no other sampler is deleted");
+    assert_eq!(samplers(&app), [[120.5, 60.5], [180.5, 90.5]], "no other sampler is deleted");
     // The menu itself closes on the next frame.
     right_click(&mut app, &ctx, 120.0, 60.0);
     app.run("view.colorSamplers.move", json!({"index": 0, "x": 30, "y": 30})).unwrap();
@@ -150,6 +154,8 @@ fn readout_rows_do_nothing_until_samplers_have_readout_modes() {
     assert_eq!(samplers(&app).len(), 2);
 }
 
+/// A sampler gone while the menu is open, a non-finite point, or no document at all: Delete
+/// greys out and nothing panics.
 #[test]
 fn a_stale_sampler_or_odd_point_fails_gracefully() {
     let (mut app, ctx) = app();
@@ -160,7 +166,7 @@ fn a_stale_sampler_or_odd_point_fails_gracefully() {
     let menu = app.ui.canvas_tool_menu.clone().unwrap();
     assert!(!crate::canvas_tool_menu::entry_enabled(&app, &menu, SAMPLER_DELETE));
     crate::canvas_tool_menu::choose(&mut app, &ctx, SAMPLER_DELETE);
-    assert_eq!(samplers(&app), [[5.0, 5.0]]);
+    assert_eq!(samplers(&app), [[5.5, 5.5]]);
     for p in [[f64::NAN, 5.0], [f64::INFINITY, 5.0], [5.0, f64::NEG_INFINITY]] {
         assert_eq!(super::sampler_at(&app, p), None);
     }