From a28aa2182870b4282586b18c030e2d8181040462 Mon Sep 17 00:00:00 2001 From: NaufalDzz Date: Wed, 4 Nov 2026 09:51:52 +0700 Subject: [PATCH 1/3] fix(transform): refuse the Background among several layers and apply the box on a selection change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Free Transform of several selected layers left the Background out without a word, where Photoshop refuses with its locked-layer alert. And with the box open, taking a layer out of the selection (or adding one) without changing the active layer left the box open, still moving the layers it started with. - targets_of keeps the Background; edit.transform's selection default refuses with `layer "Background" is locked`, and Ctrl+T / Edit › Transform show the same alert as a position-locked layer. An explicit `layers` list and the Background alone keep today's behaviour. - Move › Show Transform Controls shows no box for that selection either. - Any change to the selected layers while a layer box is open applies the box to the layers it moves, and doesn't open it again. Fixes #11101 --- crates/engine/src/cmd/transform_cmds.rs | 26 ++- .../src/cmd/transform_cmds/multi_tests.rs | 10 +- .../ui-egui/src/canvas/transform_controls.rs | 11 +- .../src/canvas/transform_controls_tests.rs | 5 +- crates/ui-egui/src/lib.rs | 3 + .../ui-egui/src/tools/transform_tool/begin.rs | 3 +- .../src/tools/transform_tool/commit.rs | 23 ++- .../src/transform_selection_rules_tests.rs | 164 ++++++++++++++++++ crates/ui-egui/src/transform_several_tests.rs | 24 +-- 9 files changed, 237 insertions(+), 32 deletions(-) create mode 100644 crates/ui-egui/src/transform_selection_rules_tests.rs diff --git a/crates/engine/src/cmd/transform_cmds.rs b/crates/engine/src/cmd/transform_cmds.rs index 55f3d9a39e..9b64677802 100644 --- a/crates/engine/src/cmd/transform_cmds.rs +++ b/crates/engine/src/cmd/transform_cmds.rs @@ -266,9 +266,9 @@ pub fn locked_for_transform(doc: &Document, id: LayerId) -> bool { } /// The layers Free Transform moves with the Layers panel's selection, bottom-to-top: with several -/// layers selected, every one that no selected group already carries, as in Photoshop (#6501), but -/// the Background, which Show Transform Controls doesn't frame either. With one layer selected (or -/// only the Background besides it), that layer; the active layer when nothing else is. +/// layers selected, every one that no selected group already carries, as in Photoshop (#6501). The +/// Background stays among them, so such a box refuses as locked (#11101, [`background_among`]). +/// With one layer selected, that layer; the active layer when nothing else is. pub fn selection_targets(st: &crate::DocState) -> Vec { targets_of(&st.doc, &st.selected_layers(), st.active_layer) } @@ -279,10 +279,19 @@ pub(crate) fn targets_of(doc: &Document, selected: &[LayerId], active: Option = crate::layer_multi_cmds::top_level(doc, selected).into_iter().filter(|id| !doc.is_background(*id)).collect(); + let ids = crate::layer_multi_cmds::top_level(doc, selected); if ids.is_empty() { active.into_iter().collect() } else { ids } } +/// The Background when it is among several layers one box would move: Photoshop refuses that Free +/// Transform with its locked-layer alert rather than leaving the Background out (#11101). +pub fn background_among(doc: &Document, ids: &[LayerId]) -> Option { + if ids.len() < 2 { + return None; + } + ids.iter().copied().find(|id| doc.is_background(*id)) +} + /// The `layers` param: the layers one box transforms together (at most 10000, each in the /// document), without those a listed group carries. `None` when it isn't given. fn layers_param(doc: &Document, p: &Value) -> Result>> { @@ -821,7 +830,14 @@ fn transform(s: &mut Session, p: &Value) -> Result { (Some(ids), _) => ids, _ if lone.is_some() => Vec::new(), (None, Some(id)) => vec![id], - (None, None) => selection_targets(st), + (None, None) => { + let ids = selection_targets(st); + if let Some(bg) = background_among(&st.doc, &ids) { + let name = st.doc.layer(bg).map_or("Background", |l| l.name.as_str()); + return Err(EngineError::Other(format!("layer \"{name}\" is locked"))); + } + ids + } }; if let Some(missing) = ids.iter().find(|id| st.doc.layer(**id).is_none()) { return Err(EngineError::NoLayer(*missing)); diff --git a/crates/engine/src/cmd/transform_cmds/multi_tests.rs b/crates/engine/src/cmd/transform_cmds/multi_tests.rs index e26572d6e7..9c0cd7044b 100644 --- a/crates/engine/src/cmd/transform_cmds/multi_tests.rs +++ b/crates/engine/src/cmd/transform_cmds/multi_tests.rs @@ -91,13 +91,17 @@ fn the_layers_param_names_the_layers_and_a_listed_group_carries_its_children() { } #[test] -fn the_background_stays_out_of_a_selection_of_several() { +fn the_background_among_a_selection_of_several_refuses_as_locked() { + // Photoshop shows its locked-layer alert rather than leaving the Background out (#11101). let (mut s, paint, text) = two_layers(); let bg = s.active().unwrap().doc.layers[0].id; s.execute("layer.select", json!({"layer": bg.0, "mode": "add"})).unwrap(); - let r = s.execute("edit.transform", json!({"matrix": [1, 0, 0, 1, 3, 0]})).unwrap(); - assert_eq!(r["layers"], json!([paint.0, text.0])); let st = s.active().unwrap(); + let (before, revision) = (st.doc.clone(), st.revision); + let err = s.execute("edit.transform", json!({"matrix": [1, 0, 0, 1, 3, 0]})).unwrap_err().to_string(); + assert!(err.contains("\"Background\" is locked"), "{err}"); + let st = s.active().unwrap(); + assert!(st.doc == before && st.revision == revision, "nothing moved, not even {paint:?} or {text:?}"); assert!(st.doc.is_background(bg), "still the Background"); assert_eq!(st.doc.layer(bg).unwrap().surface().unwrap().content_bounds(), Rect::new(0, 0, 200, 200)); } diff --git a/crates/ui-egui/src/canvas/transform_controls.rs b/crates/ui-egui/src/canvas/transform_controls.rs index 7e00fd8cb3..a458b75ba8 100644 --- a/crates/ui-egui/src/canvas/transform_controls.rs +++ b/crates/ui-egui/src/canvas/transform_controls.rs @@ -22,7 +22,8 @@ pub(crate) fn layer_edges_rect(app: &PhotocraftApp, idx: usize) -> Option