Apply the transform when another layer is selected (#1400) (#1496)

While Free Transform was open (Ctrl+T, or after dragging a Move-tool
transform control), selecting another layer in the Layers panel left the
box and the lifted preview on the old layer. end_if_left only ended the
session when its layer or document went away or the tool changed.

It now also commits the session when the active layer is no longer the
one being transformed, like Photoshop applying the transform on a layer
switch; the next transform (or the Move tool's controls) then starts on
the newly selected layer. Transform Selection is not tied to a layer and
stays open.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
David MannandClaude Opus 5.5 authored and GitHub committed 2026-10-09 09:01:36 +07:00
1 parent 6c4d4e9dc9
commit 086d78a023
1 file changed
+59 -5
+59 -5
View File
@@ -559,14 +559,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.
/// or selecting another layer (Layers panel, ⌘-click, the control channel) applies it, so the box
/// never stays on a layer that is no longer the active one (#1400). 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().is_none_or(|st| app.transform_preview.as_ref().is_some_and(|pv| pv.doc.id != st.doc.id) || st.doc.layer(LayerId(t.layer)).is_none())
{
let Some(st) = app.session.active() else { return cancel(app) };
if app.transform_preview.as_ref().is_some_and(|pv| pv.doc.id != st.doc.id) || st.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) {
} else if app.transform_preview.as_ref().is_some_and(|pv| pv.tool != app.ui.tool)
|| (!t.selection && st.active_layer.is_some_and(|active| active.0 != t.layer))
{
commit(app);
}
}
@@ -2075,6 +2079,56 @@ mod tests {
assert_eq!(st.doc.layer(st.active_layer.unwrap()).unwrap().surface().unwrap().content_bounds(), photocraft_geom::Rect::new(28, 8, 44, 24));
}
/// #1400: selecting another layer while transforming applies the box to the layer it was on,
/// and the next transform starts on the newly selected layer.
#[test]
fn selecting_another_layer_applies_the_transform_and_the_next_one_follows() {
let ctx = egui::Context::default();
let mut app = app_with_square(64, photocraft_geom::Rect::new(8, 8, 24, 24));
let first = app.session.active().unwrap().active_layer.unwrap();
app.run("layer.new.layer", json!({})).unwrap();
app.session
.edit("paint", |doc, a| {
doc.layer_mut(a.unwrap()).unwrap().surface_mut().unwrap().fill_rect(photocraft_geom::Rect::new(40, 40, 56, 60), &[0.0, 0.0, 1.0, 1.0]);
Ok(())
})
.unwrap();
let second = app.session.active().unwrap().active_layer.unwrap();
app.run("layer.select", json!({"layer": first.0})).unwrap();
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();
app.run("layer.select", json!({"layer": second.0})).unwrap();
end_if_left(&mut app);
assert!(app.ui.transform.is_none() && app.transform_preview.is_none(), "the box left the old layer");
let st = app.session.active().unwrap();
assert_eq!(st.active_layer, Some(second));
assert_eq!(st.history.past_len(), steps + 1, "applied as one step");
let bounds = |id: LayerId| st.doc.layer(id).unwrap().surface().unwrap().content_bounds();
assert_eq!(bounds(first), photocraft_geom::Rect::new(28, 8, 44, 24), "applied to the layer it was on");
assert_eq!(bounds(second), photocraft_geom::Rect::new(40, 40, 56, 60), "the new layer is untouched");
crate::menus::invoke(&mut app, &ctx, "edit.freeTransform", json!({})).unwrap();
let t = app.ui.transform.as_ref().unwrap();
assert_eq!((t.layer, t.rect), (second.0, [40.0, 40.0, 56.0, 60.0]), "the next box is on the selected layer");
}
/// Transform Selection moves the selection outline, not a layer: selecting a layer keeps it open.
#[test]
fn selecting_another_layer_keeps_a_transform_selection_open() {
let ctx = egui::Context::default();
let mut app = app_with_square(64, photocraft_geom::Rect::new(8, 8, 24, 24));
let first = app.session.active().unwrap().active_layer.unwrap();
app.run("layer.new.layer", json!({})).unwrap();
app.run("select.all", json!({})).unwrap();
begin_selection(&mut app, &ctx).unwrap();
app.run("layer.select", json!({"layer": first.0})).unwrap();
end_if_left(&mut app);
assert!(app.ui.transform.as_ref().is_some_and(|t| t.selection));
}
/// 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() {