diff --git a/crates/ui-egui/src/tool_memory.rs b/crates/ui-egui/src/tool_memory.rs index 1464052aad..2b05c523d6 100644 --- a/crates/ui-egui/src/tool_memory.rs +++ b/crates/ui-egui/src/tool_memory.rs @@ -26,10 +26,6 @@ pub(crate) fn select(app: &mut PhotocraftApp, tool: Tool) { app.stylus.tool_before_eraser = None; app.ui.spring = None; app.ui.tool = tool; - // Picking the Crop tool, even the one already picked, brings back a frame a commit hid. - if tool == Tool::Crop { - crate::crop_ui::show_frame(app); - } } /// Temporary canvas overrides do not change `ui.tool`. Spring-loaded letters and the pen's diff --git a/crates/ui-egui/src/tools/crop_mode/tests.rs b/crates/ui-egui/src/tools/crop_mode/tests.rs index a676cd6e5f..4401e9b15d 100644 --- a/crates/ui-egui/src/tools/crop_mode/tests.rs +++ b/crates/ui-egui/src/tools/crop_mode/tests.rs @@ -579,9 +579,9 @@ fn p_toggles_classic_mode_while_a_crop_is_in_progress() { assert!(!classic(&app)); } -/// After a commit the tool shows no frame until the next press or re-picking the tool, which -/// brings its untouched default frame back: H and P are the Hand and Pen keys there, as when the -/// tool is picked, not Show Cropped Area and Classic Mode (#9731, #6536). +/// After a commit the tool shows no frame until the next press, which brings its untouched default +/// frame back: H and P are the Hand and Pen keys there, as when the tool is picked, not Show +/// Cropped Area and Classic Mode (#9731, #6536). #[test] fn h_and_p_pick_their_tools_over_the_default_frame_after_a_commit() { for (k, tool) in [(egui::Key::H, Tool::Hand), (egui::Key::P, Tool::Pen)] { @@ -591,9 +591,8 @@ fn h_and_p_pick_their_tools_over_the_default_frame_after_a_commit() { assert_eq!(app.session.active().map(|st| st.doc.size), Some(Size::new(60, 40)), "the crop committed"); crate::crop_ui::ensure_frame(&mut app); assert!(app.ui.crop_rect.is_none(), "the commit hides the frame"); - crate::tool_memory::select(&mut app, Tool::Crop); - crate::crop_ui::ensure_frame(&mut app); - assert!(app.ui.crop_rect.is_some() && app.crop.default_frame, "re-picking the tool brings the default frame back"); + crate::crop_ui::show_frame(&mut app); + assert!(app.ui.crop_rect.is_some() && app.crop.default_frame, "a press brings the default frame back"); key(&mut app, k, NONE); assert_eq!(app.ui.tool, tool, "{k:?}"); let s = &app.ui.tool_options.crop_shield; @@ -740,11 +739,8 @@ fn a_recrop_handle_snaps_exactly_to_the_hidden_pixels_edge() { let v = &mut app.ui.views[0]; (v.zoom, v.center, v.fit_pending, v.rotation) = (1.0, [50.0, 25.0], false, 0.0); app.ui.extras.snap = true; - // Re-picking the tool brings back the default frame the commit hid. - app.ui.tool = Tool::Move; - crate::crop_ui::ensure_frame(&mut app); - app.ui.tool = Tool::Crop; - crate::crop_ui::ensure_frame(&mut app); + // A press brings back the default frame the commit hid. + crate::crop_ui::show_frame(&mut app); assert_eq!(app.ui.crop_rect, Some([0.0, 0.0, 100.0, 50.0])); if ratio { app.ui.tool_options.crop_ratio = "original".into(); diff --git a/crates/ui-egui/src/tools/crop_ui.rs b/crates/ui-egui/src/tools/crop_ui.rs index 4bd756dcd8..74c6add2f7 100644 --- a/crates/ui-egui/src/tools/crop_ui.rs +++ b/crates/ui-egui/src/tools/crop_ui.rs @@ -381,7 +381,6 @@ pub fn ensure_frame(app: &mut PhotocraftApp) { fn frame(app: &mut PhotocraftApp) { if app.ui.tool != Tool::Crop { app.crop.editing = false; - app.crop.dismissed = None; crate::crop_straighten::reset(app); // Leaving the tool drops an untouched default frame and, as in Photoshop, commits a // drawn one (#3368). @@ -449,6 +448,9 @@ fn commit_on_leave(app: &mut PhotocraftApp) { cancel(app); } app.ui.tool = tool; + // Committed by leaving the tool: back on it the default frame shows at once, as in Photoshop + // (only a commit with the Crop tool still picked hides it). + app.crop.dismissed = None; } /// A crop gesture is in progress: Space repositions the frame rather than panning. @@ -471,8 +473,9 @@ pub fn cancel(app: &mut PhotocraftApp) { } /// After a committed crop, with the Crop tool picked: no new default frame for the cropped -/// document until a press on the canvas or re-picking the tool, so its corners don't make the -/// applied crop look still pending. +/// document until a press on the canvas (or a ratio), so its corners don't make the applied crop +/// look still pending. Picking the tool again, also after another tool, keeps it hidden, as in +/// Photoshop. pub fn hide_frame(app: &mut PhotocraftApp) { if app.ui.tool == Tool::Crop { app.crop.dismissed = dismiss_key(app); diff --git a/crates/ui-egui/src/tools/crop_ui/tests.rs b/crates/ui-egui/src/tools/crop_ui/tests.rs index 0eb5d13434..280b854c00 100644 --- a/crates/ui-egui/src/tools/crop_ui/tests.rs +++ b/crates/ui-egui/src/tools/crop_ui/tests.rs @@ -122,14 +122,16 @@ fn enter_on_the_selection_frame_crops_to_it_at_8_and_16_bit() { assert_eq!(doc.size, Size::new(30, 40), "{depth:?}"); assert_eq!(doc.depth, depth); assert!(doc.selection.is_none(), "cropping to the selection deselects, as in Photoshop"); - // No frame until a press or re-picking the tool; then the default one is the whole - // (cropped) canvas, and ↵ on it does nothing. + // No frame until a press, also after another tool and back; then the default one is the + // whole (cropped) canvas, and ↵ on it does nothing. ensure_frame(&mut app); assert_eq!(app.ui.crop_rect, None); app.ui.tool = Tool::Move; ensure_frame(&mut app); - app.ui.tool = Tool::Crop; + crate::tool_memory::select(&mut app, Tool::Crop); ensure_frame(&mut app); + assert_eq!(app.ui.crop_rect, None, "picking the tool again keeps it hidden"); + show_frame(&mut app); assert_eq!(app.ui.crop_rect, Some([0.0, 0.0, 30.0, 40.0])); let steps = app.session.active().unwrap().history.past_len(); crate::canvas::commit_crop(&mut app); @@ -138,10 +140,10 @@ fn enter_on_the_selection_frame_crops_to_it_at_8_and_16_bit() { } } -/// A commit hides the frame; picking the Crop tool again, a ratio, or a press brings back the -/// default frame, and the press's crop keeps its box Undo and Redo. +/// A commit hides the frame; picking the Crop tool again keeps it hidden, as in Photoshop; a ratio +/// or a press brings back the default frame, and the press's crop keeps its box Undo and Redo. #[test] -fn a_hidden_frame_comes_back_on_re_picking_a_ratio_or_a_press_with_redo_intact() { +fn a_hidden_frame_comes_back_on_a_ratio_or_a_press_with_redo_intact() { let committed = || { let mut app = app(SampleType::U8); app.ui.crop_rect = Some([20.0, 10.0, 120.0, 60.0]); @@ -151,9 +153,17 @@ fn a_hidden_frame_comes_back_on_re_picking_a_ratio_or_a_press_with_redo_intact() app }; let whole = Some([0.0, 0.0, 100.0, 50.0]); - // Picking the Crop tool while it is still picked. + // Picking the Crop tool while it is still picked, or after another tool: still hidden. let mut app = committed(); crate::tool_memory::select(&mut app, Tool::Crop); + ensure_frame(&mut app); + assert_eq!(app.ui.crop_rect, None, "re-picked"); + crate::tool_memory::select(&mut app, Tool::Move); + ensure_frame(&mut app); + crate::tool_memory::select(&mut app, Tool::Crop); + ensure_frame(&mut app); + assert_eq!(app.ui.crop_rect, None, "picked after another tool"); + show_frame(&mut app); assert_eq!(app.ui.crop_rect, whole); assert!(app.crop.default_frame); // A ratio fits the frame it brings back. @@ -1283,7 +1293,7 @@ fn info_size_is_shown_in_the_ruler_unit() { /// #3145 review: after the crop is cancelled or committed, or the tool is left, W and H are the /// selection's again. With the Crop tool still picked, the new default frame is the selection's /// bounds (or the canvas), as in Photoshop, so the panel shows the same size. After a commit that -/// frame waits for the next press or re-picking the tool. +/// frame waits for the next press. #[test] fn info_size_returns_to_the_selection_after_cancel_or_commit() { let mut app = app(SampleType::U8); @@ -1306,8 +1316,8 @@ fn info_size_returns_to_the_selection_after_cancel_or_commit() { assert_eq!(info_size(&app), None, "committed: no frame"); ensure_frame(&mut app); assert_eq!(info_size(&app), None, "committed: the frame stays hidden"); - crate::tool_memory::select(&mut app, Tool::Crop); - ensure_frame(&mut app); + // A press brings it back. + show_frame(&mut app); assert_eq!(info_size(&app), Some([40.0, 30.0]), "the new frame is the cropped canvas"); // Leaving the tool: the panel shows the selection, whatever the frame. app.ui.tool = Tool::Move;