Polygonal Lasso: drop the polygon in progress when another document becomes active (#5015)

Follow-up to #4021, which drops the polygon when another tool is selected. The polygon now also remembers the document it was started on (UiState::polygon_doc) and is dropped when another document becomes active, as the Lasso and the Magnetic Lasso already do; before, its outline showed on the other document. Adds tests that every selection tool and Move/Brush drop the polygon (#4021 tests Quick Selection only), that a document switch drops it, and that a held Space or Cmd temporary tool keeps it.

Co-authored-by: ZulejhaAvduli <269043497+avdyliisni9-png@users.noreply.github.com>
This commit is contained in:
avdyliisni9-png 2026-10-13 18:24:22 +02:00 committed by GitHub
parent 62bf7391ce
commit cd35577c45
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 119 additions and 0 deletions

View File

@ -5513,6 +5513,7 @@ fn polygon_click(app: &mut PhotocraftApp, x: f64, y: f64, mods: egui::Modifiers)
return;
}
if app.ui.polygon.is_empty() {
app.ui.polygon_doc = app.session.active().map(|st| st.doc.id);
// ⌥ with nothing selected draws freehand; there is nothing to subtract from.
let nothing_selected = app.session.active().is_none_or(|st| st.doc.selection.is_none());
let intent = if nothing_selected { egui::Modifiers { alt: false, ..mods } } else { mods };
@ -5536,6 +5537,22 @@ pub fn polygon_retract(app: &mut PhotocraftApp) -> bool {
true
}
/// The polygon in progress belongs to the document it was started on (`UiState::polygon_doc`):
/// once another document becomes active it is dropped, so its outline doesn't show on (or get
/// committed to) the other document, and the selection it would have replaced is shown again.
/// Switching tools is handled in `PhotocraftApp::sync_views`. Like the Lasso's
/// ([`crate::lasso_ui::cancel_stale`]) and the Magnetic Lasso's ([`crate::magnetic_lasso_ui::frame`]).
pub fn polygon_cancel_stale(app: &mut PhotocraftApp) {
let active = app.session.active().map(|st| st.doc.id);
if app.ui.polygon.is_empty() {
app.ui.polygon_doc = None;
} else if app.ui.polygon_doc.is_some_and(|d| Some(d) != active) {
app.ui.polygon.clear();
app.ui.polygon_mode.clear();
app.ui.polygon_doc = None;
}
}
/// A new-selection polygonal (or magnetic) lasso is being drawn, so the selection it will replace
/// is hidden.
pub fn polygon_replaces_selection(app: &PhotocraftApp) -> bool {

View File

@ -1116,6 +1116,8 @@ impl PhotocraftApp {
self.ui.polygon.clear();
self.ui.polygon_mode.clear();
}
// ... and to the document it was started on.
crate::canvas::polygon_cancel_stale(self);
crate::crop_ui::cancel_stale(self);
let ids: Vec<DocId> = self.session.documents().iter().map(|d| d.doc.id).collect();
// Where the document of view `i` is now. Views not tracked yet keep their index.

View File

@ -405,3 +405,98 @@ fn shift_polygonal_lasso_snap_has_all_eight_directions_and_handles_zero_length()
}
assert_eq!(crate::canvas::constrained_polygon_point(origin, origin), origin);
}
/// Two vertices placed with the Polygonal Lasso, well apart in time so they aren't a double-click.
fn two_vertices(h: &mut Harness<'static, PhotocraftApp>) {
click(h, 50.0, 50.0, Modifiers::NONE);
h.run_steps(30);
click(h, 150.0, 50.0, Modifiers::NONE);
h.run_steps(30);
assert_eq!(h.state().ui.polygon.len(), 2);
}
/// The polygon, its mode and its document are all gone, and the selection it would have replaced
/// is shown again.
fn assert_polygon_dropped(h: &Harness<'static, PhotocraftApp>, what: &str) {
let app = h.state();
assert!(app.ui.polygon.is_empty(), "{what}: the outline is gone: {:?}", app.ui.polygon);
assert!(app.ui.polygon_mode.is_empty(), "{what}: the mode is gone");
assert!(app.ui.polygon_doc.is_none(), "{what}: the document is forgotten");
assert!(!crate::canvas::polygon_replaces_selection(app), "{what}: the old outline is shown again");
}
/// Every other tool drops the polygon in progress, not just the Quick Selection tool, and coming
/// back to the Polygonal Lasso starts a new one.
#[test]
fn every_other_tool_drops_the_polygon_in_progress() {
let tools = [
Tool::QuickSelection,
Tool::ObjectSelection,
Tool::MagicWand,
Tool::RectMarquee,
Tool::EllipseMarquee,
Tool::Lasso,
Tool::MagneticLasso,
Tool::Move,
Tool::Brush,
];
for tool in tools {
let mut h = harness();
h.state_mut().run("select.rect", json!({"x": 200, "y": 150, "width": 100, "height": 100})).unwrap();
h.run_steps(1);
let before = steps(&h);
two_vertices(&mut h);
assert!(crate::canvas::polygon_replaces_selection(h.state()), "{tool:?}: the old outline is hidden while drawing");
h.state_mut().ui.tool = tool;
h.state_mut().sync_views();
assert_polygon_dropped(&h, &format!("{tool:?}"));
assert!(selection(&h).is_some_and(|r| near(r, 200, 150, 300, 250)), "{tool:?}: {:?}", selection(&h));
assert_eq!(steps(&h), before, "{tool:?}: nothing was recorded");
h.state_mut().ui.tool = Tool::PolygonLasso;
h.state_mut().sync_views();
assert!(h.state().ui.polygon.is_empty(), "{tool:?}: coming back starts a new polygon");
}
}
/// The polygon belongs to the document it was started on: another document becoming active drops
/// it, while the tool stays the Polygonal Lasso (its outline would otherwise show on the other
/// document).
#[test]
fn switching_documents_drops_the_polygon_in_progress() {
let mut h = harness();
two_vertices(&mut h);
let first = h.state().session.active().map(|st| st.doc.id);
assert!(first.is_some());
assert_eq!(h.state().ui.polygon_doc, first, "the polygon remembers its document");
h.state_mut().run("file.new", json!({"width": 200, "height": 100})).unwrap();
assert_ne!(h.state().session.active().map(|st| st.doc.id), first, "another document is active");
assert_polygon_dropped(&h, "new document");
assert_eq!(h.state().ui.tool, Tool::PolygonLasso, "the tool stays");
// Back on the first document, nothing is left of it.
h.state_mut().run("document.activate", json!({"document": 0})).unwrap();
h.state_mut().sync_views();
assert!(h.state().ui.polygon.is_empty());
}
/// A held temporary tool (Space is the Hand, ⌘ the Move tool) doesn't change the selected tool, so
/// the polygon being drawn stays.
#[test]
fn a_held_temporary_tool_keeps_the_polygon() {
let mut h = harness();
two_vertices(&mut h);
h.event(egui::Event::Key { key: egui::Key::Space, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE });
h.run_steps(3);
h.state_mut().sync_views();
assert_eq!(h.state().ui.tool, Tool::PolygonLasso);
assert_eq!(h.state().ui.polygon.len(), 2, "Space held");
h.event(egui::Event::Key { key: egui::Key::Space, physical_key: None, pressed: false, repeat: false, modifiers: Modifiers::NONE });
h.run_steps(3);
h.event(egui::Event::ModifiersChanged(Modifiers::COMMAND));
h.run_steps(3);
h.state_mut().sync_views();
assert_eq!(h.state().ui.tool, Tool::PolygonLasso);
assert_eq!(h.state().ui.polygon.len(), 2, "⌘ held");
h.event(egui::Event::ModifiersChanged(Modifiers::NONE));
h.run_steps(3);
assert_eq!(h.state().ui.polygon.len(), 2, "released");
}

View File

@ -1222,6 +1222,10 @@ pub struct UiState {
/// modifiers held at its first click.
#[serde(default)]
pub polygon_mode: String,
/// The document the polygon is drawn on (set at its first click): another document becoming
/// active drops it (`canvas::polygon_cancel_stale`).
#[serde(default)]
pub polygon_doc: Option<photocraft_doc::DocId>,
/// Magnetic Lasso border in progress (`magnetic_lasso_ui`).
#[serde(default)]
pub magnetic: crate::magnetic_lasso_ui::MagneticLasso,
@ -1329,6 +1333,7 @@ impl Default for UiState {
tool_options: ToolOptions::default(),
polygon: Vec::new(),
polygon_mode: String::new(),
polygon_doc: None,
magnetic: Default::default(),
crop_rect: None,
crop_angle: 0.0,