fix(transform): picking another tool applies an open transform (#732)

Fixes #670.
This commit is contained in:
reese 2026-10-07 15:08:39 -05:00 committed by GitHub
parent 3a3984075a
commit 3bea3f31c5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 70 additions and 7 deletions

View File

@ -2245,6 +2245,7 @@ pub fn tool_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers)
// An Alt+right-drag armed for this press (`paint_mouse`): taken before anything else can
// consume the event, so it never outlives the press it was armed for (#297).
let armed = std::mem::take(&mut app.brush_resize_armed);
crate::transform_tool::end_if_left(app);
// View › Snap / Snap To and smart guides (snap_ui.rs).
let raw = ev;
let ev = crate::snap_ui::filter_event(app, ev, mods);

View File

@ -871,12 +871,7 @@ impl eframe::App for PhotocraftApp {
if self.ui.pen.is_some() && self.ui.tool != state::Tool::Pen {
vector_ui::pen_commit(self, false);
}
// A transform whose layer or document went away (undo, close) ends silently.
if let Some(t) = &self.ui.transform
&& self.session.active().and_then(|s| s.doc.layer(photocraft_doc::LayerId(t.layer))).is_none()
{
transform_tool::cancel(self);
}
transform_tool::end_if_left(self);
self.collect_screenshots(ctx);
self.issue_screenshots(ctx);
prefs_ui::tick(self, ctx);

View File

@ -18,7 +18,7 @@ use serde_json::json;
use crate::PhotocraftApp;
use crate::canvas::{ToolEvent, ViewXform};
use crate::state::TransformSession;
use crate::state::{Tool, TransformSession};
/// Which Split button is armed. The guide follows the pointer and the split is added on release.
#[derive(Clone, Copy, PartialEq, Eq)]
@ -64,6 +64,8 @@ pub struct TransformPreview {
/// Option-click places a split without a button armed.
split_quick: bool,
steps: Steps,
/// The tool the session began with: picking another one applies the transform.
tool: Tool,
}
/// Grab radius of the box's handles, in screen points. Generous, so a corner is easy to catch;
@ -176,6 +178,7 @@ pub fn begin(app: &mut PhotocraftApp, ctx: &egui::Context) -> Result<(), String>
split_placing: false,
split_quick: false,
steps: Steps::default(),
tool: app.ui.tool,
});
app.ui.transform = Some(TransformSession {
session,
@ -274,6 +277,7 @@ fn begin_lone(
split_placing: false,
split_quick: false,
steps: Steps::default(),
tool: app.ui.tool,
});
app.ui.transform = Some(TransformSession {
session,
@ -331,6 +335,7 @@ pub fn begin_selection(app: &mut PhotocraftApp, ctx: &egui::Context) -> Result<(
split_placing: false,
split_quick: false,
steps: Steps::default(),
tool: app.ui.tool,
});
app.ui.transform = Some(TransformSession {
session,
@ -507,6 +512,18 @@ pub fn commit(app: &mut PhotocraftApp) {
}
}
/// A transform whose layer or document went away (undo, close) ends silently; picking another tool
/// applies it. Checked every frame and before each pointer event, so a press with a tool chosen
/// just before it (`ui.pointer`'s `tool`) goes to that tool.
pub fn end_if_left(app: &mut PhotocraftApp) {
let Some(t) = &app.ui.transform else { return };
if app.session.active().and_then(|s| s.doc.layer(LayerId(t.layer))).is_none() {
cancel(app);
} else if app.transform_preview.as_ref().is_some_and(|pv| pv.tool != app.ui.tool) {
commit(app);
}
}
pub fn cancel(app: &mut PhotocraftApp) {
let copy = app.ui.transform.take().is_some_and(|t| t.copy);
app.transform_preview = None;
@ -1748,6 +1765,56 @@ mod tests {
assert_eq!(app.session.active().unwrap().doc.layers.len(), layers);
}
/// #670: picking another tool applies the open transform, as one history step.
#[test]
fn picking_another_tool_applies_the_transform() {
let mut app = app_with_square(64, photocraft_geom::Rect::new(8, 8, 24, 24));
app.ui.tool = Tool::Move;
let steps = app.session.active().unwrap().history.past_len();
let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).with_max_steps(64).build_eframe(move |cc| {
PhotocraftApp::setup_context(&cc.egui_ctx, Default::default());
app
});
h.run_steps(4);
let ctx = h.ctx.clone();
crate::menus::invoke(h.state_mut(), &ctx, "edit.freeTransform", json!({})).unwrap();
if let Some(t) = h.state_mut().ui.transform.as_mut() {
t.quad = t.quad.map(|[x, y]| [x + 20.0, y]);
}
h.run_steps(2);
assert!(h.state().ui.transform.is_some(), "open while the tool stays");
h.key_press(egui::Key::B);
h.run_steps(2);
let app = h.state();
assert_eq!(app.ui.tool, Tool::Brush);
assert!(app.ui.transform.is_none() && app.transform_preview.is_none());
let st = app.session.active().unwrap();
assert_eq!(st.history.past_len(), steps + 1);
assert_eq!(st.doc.layer(st.active_layer.unwrap()).unwrap().surface().unwrap().content_bounds(), photocraft_geom::Rect::new(28, 8, 44, 24));
}
/// A control-channel stroke that picks another tool applies the box first, then paints.
#[test]
fn a_pointer_request_with_another_tool_applies_the_transform_first() {
let ctx = egui::Context::default();
let mut app = app_with_square(64, photocraft_geom::Rect::new(8, 8, 24, 24));
app.ui.tool = Tool::Move;
crate::menus::invoke(&mut app, &ctx, "edit.freeTransform", json!({})).unwrap();
if let Some(t) = app.ui.transform.as_mut() {
t.quad = t.quad.map(|[x, y]| [x + 20.0, y]);
}
let steps = app.session.active().unwrap().history.past_len();
let ev = |kind: &str, x: f64| json!({"kind": kind, "x": x, "y": 50.0});
let events = json!([ev("down", 4.0), ev("move", 32.0), ev("move", 60.0), ev("up", 60.0)]);
let (req, _rx) = crate::control::ControlRequest::new("ui.pointer", json!({"tool": "brush", "events": events}));
let _ = crate::control::handle(&mut app, &ctx, &req);
assert!(app.ui.transform.is_none());
let st = app.session.active().unwrap();
assert_eq!(st.history.past_len(), steps + 2, "the transform, then the stroke");
let b = st.doc.layer(st.active_layer.unwrap()).unwrap().surface().unwrap().content_bounds();
assert!(b.x0 < 8 && b.x1 >= 44 && b.y0 == 8 && b.y1 > 50, "moved square and stroke: {b:?}");
}
fn app_with_square(size: u32, fill: photocraft_geom::Rect) -> PhotocraftApp {
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
app.session.execute("file.new", json!({"width": size, "height": size})).unwrap();