From e8b906f5d997b61d22d945ff4095b195a9acb005 Mon Sep 17 00:00:00 2001 From: x-labb <154824493+x-labb@users.noreply.github.com> Date: Sun, 25 Oct 2026 23:35:59 +0100 Subject: [PATCH] fix(ui): Free Transform keeps plain layers over the moving one under a blend or adjustment layer (#9387) The preview draws the layers over the moving layer on top of it again, but the search for them stopped at the first layer that isn't plain (an adjustment, a blend mode), so every layer under that one, including plain Normal layers over the moving layer, stayed under the box and the moving layer was drawn over them. A PSD usually has such a layer somewhere on top. Pass over layers that can't be drawn alone instead of stopping, so the plain layers around them still cover the moving layer. An adjustment layer over covering layers stays in the document under the box and is composited with them too, so they keep its adjustment. Layers with a blend mode still stay under the box, and the moving layer is not adjusted or blended by what is over it in the preview. --- .../src/tools/transform_tool/preview.rs | 49 +++++++--- .../ui-egui/src/tools/transform_tool/tests.rs | 91 +++++++++++++++++-- 2 files changed, 120 insertions(+), 20 deletions(-) diff --git a/crates/ui-egui/src/tools/transform_tool/preview.rs b/crates/ui-egui/src/tools/transform_tool/preview.rs index d39d6575f3..f7ec84fffe 100644 --- a/crates/ui-egui/src/tools/transform_tool/preview.rs +++ b/crates/ui-egui/src/tools/transform_tool/preview.rs @@ -78,13 +78,16 @@ pub(super) struct Above { pub(super) max_side: usize, /// The covering layers (whole layers or groups, at any depth). pub(super) ids: Vec, + /// Adjustment layers over the covering ones: they stay in the document under the box as well, + /// and are composited with the covering layers so those keep the adjustment. + pub(super) applied: Vec, /// Their composite and the canvas area it covers; `None` when nothing visible covers the layer. pub(super) shown: Option<(crate::transform_tex::PreviewTextures, photocraft_geom::Rect)>, } impl Above { pub(super) fn new(ctx: &egui::Context, name: String, doc: &Document, id: LayerId, max_side: usize) -> Self { - let mut a = Self { ctx: ctx.clone(), name, max_side, ids: Vec::new(), shown: None }; + let mut a = Self { ctx: ctx.clone(), name, max_side, ids: Vec::new(), applied: Vec::new(), shown: None }; a.rebuild(doc, id); a } @@ -92,11 +95,16 @@ impl Above { /// Composite what covers layer `id` of `doc` (the live document) again. pub(super) fn rebuild(&mut self, doc: &Document, id: LayerId) { self.ids.clear(); - covering(&doc.layers, id, &mut self.ids); + self.applied.clear(); + covering(&doc.layers, id, &mut self.ids, &mut self.applied); let mut up = doc.clone(); - show_only(&mut up.layers, &self.ids); + let mut covers = doc.clone(); + show_only(&mut covers.layers, &self.ids); + let keep: Vec = self.ids.iter().chain(&self.applied).copied().collect(); + show_only(&mut up.layers, &keep); let canvas = up.bounds(); - let rect = up + // An adjustment touches only pixels the covering layers have, so they alone bound the composite. + let rect = covers .layers .iter() .filter(|l| l.visible) @@ -174,11 +182,15 @@ fn plain(l: &photocraft_doc::Layer) -> bool { } /// The layers over `id` in `layers` (bottom to top, groups holding `id` searched too) that cover -/// it in the preview: from the top down, while they are [`plain`]. The first layer that isn't -/// stops the search, so it and everything under it stay in the document under the box as before -/// (an adjustment there keeps applying to the layers under it). A clipping group goes as a whole, -/// and the layers clipped to `id` (or to the group holding it) stay with it. -pub(super) fn covering(layers: &[photocraft_doc::Layer], id: LayerId, out: &mut Vec) { +/// it in the preview, into `out`: every [`plain`] one, from the top down. A layer that isn't plain +/// (an adjustment, a blend mode) is passed over and stays in the document under the box: it can't +/// be drawn on its own, so the plain layers under it no longer wait for it to cover the moving +/// layer (#9387). An adjustment layer over covering layers goes to `applied`: it stays under the +/// box and is composited with them as well, so they keep its adjustment. A clipping group goes as +/// a whole, and the layers clipped to `id` (or to the group holding it) stay with it. +pub(super) fn covering(layers: &[photocraft_doc::Layer], id: LayerId, out: &mut Vec, applied: &mut Vec) { + // Adjustments met since the last covering layer: they count once one lies under them. + let mut pending: Vec = Vec::new(); let mut end = layers.len(); while end > 0 { // The clipping group ending at `end`: its base and the layers clipped to it. @@ -194,14 +206,25 @@ pub(super) fn covering(layers: &[photocraft_doc::Layer], id: LayerId, out: &mut && let LayerContent::Group(group) = &g.content && matches!(g.blend, photocraft_color::BlendMode::Normal | photocraft_color::BlendMode::PassThrough) { - covering(&group.children, id, out); + let before = out.len(); + covering(&group.children, id, out, applied); + if out.len() > before { + applied.append(&mut pending); + } } return; } - if !unit.iter().all(plain) { - return; + if unit.iter().all(plain) { + out.extend(unit.iter().map(|l| l.id)); + if unit.iter().any(|l| l.visible) { + applied.append(&mut pending); + } + } else if let [l] = unit + && l.visible + && matches!(l.content, LayerContent::Adjustment(_)) + { + pending.push(l.id); } - out.extend(unit.iter().map(|l| l.id)); end = start; } } diff --git a/crates/ui-egui/src/tools/transform_tool/tests.rs b/crates/ui-egui/src/tools/transform_tool/tests.rs index 57d323b84a..3621aad6ae 100644 --- a/crates/ui-egui/src/tools/transform_tool/tests.rs +++ b/crates/ui-egui/src/tools/transform_tool/tests.rs @@ -1921,6 +1921,53 @@ fn free_transform_keeps_the_layer_under_the_layers_over_it() { cancel(&mut app); } +/// #9387: a layer under others keeps its place in the stack when something over it blends or +/// adjusts (a PSD usually has one): the plain layers over the moving one still cover it. +#[test] +fn plain_layers_still_cover_the_moving_one_under_a_blend_or_adjustment_layer() { + use photocraft_color::BlendMode; + for adjustment in [false, true] { + let mut app = app_with_square(64, photocraft_geom::Rect::new(8, 8, 24, 24)); + let red = app.session.active().unwrap().active_layer.unwrap(); + app.session.execute("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(16, 16, 40, 40), &[0.0, 0.0, 1.0, 1.0]); + Ok(()) + }) + .unwrap(); + let blue = app.session.active().unwrap().active_layer.unwrap(); + let bg = app.session.active().unwrap().doc.layers[0].id; + let top = if adjustment { + app.session.execute("layer.newAdjustmentLayer.invert", json!({})).unwrap(); + app.session.active().unwrap().active_layer.unwrap() + } else { + app.session.execute("layer.new.layer", json!({})).unwrap(); + let top = app.session.active().unwrap().active_layer.unwrap(); + app.session + .edit("blend", |doc, _| { + doc.layer_mut(top).unwrap().blend = BlendMode::Multiply; + Ok(()) + }) + .unwrap(); + top + }; + app.session.execute("layer.select", json!({"layer": red.0})).unwrap(); + begin(&mut app, &gpu_ctx()).unwrap(); + let pv = app.transform_preview.as_ref().unwrap(); + assert!(!pv.doc.layer(blue).unwrap().visible, "adjustment={adjustment}: the plain layer is drawn over the box, not under it"); + assert!(pv.doc.layer(top).unwrap().visible, "adjustment={adjustment}: the layer that blends stays in the document under the box"); + assert!(pv.doc.layer(bg).unwrap().visible); + let (texture, rect) = pv.above.as_ref().and_then(|a| a.shown.as_ref()).expect("the plain layer covers the moving one"); + assert_eq!(*rect, photocraft_geom::Rect::new(16, 16, 40, 40), "adjustment={adjustment}"); + let (image, _) = texture.level_image(0).unwrap(); + let centre = image.pixels[12 * image.size[0] + 12]; + // The adjustment is composited with the layer it covers: inverted blue is yellow. + assert_eq!(centre, if adjustment { Color32::from_rgb(255, 255, 0) } else { Color32::BLUE }, "adjustment={adjustment}"); + cancel(&mut app); + } +} + #[test] fn plain_layers_from_the_top_cover_the_moving_one() { use photocraft_color::BlendMode; @@ -1928,7 +1975,7 @@ fn plain_layers_from_the_top_cover_the_moving_one() { let px = |name: &str| Layer::raster(name, photocraft_color::PixelFormat::RGBA8); let covers = |layers: &[Layer], id: LayerId| { let mut out = Vec::new(); - covering(layers, id, &mut out); + covering(layers, id, &mut out, &mut Vec::new()); out }; // In a group: the group's layers over it, then the ones over the group; never the layers @@ -1941,7 +1988,7 @@ fn plain_layers_from_the_top_cover_the_moving_one() { assert_eq!(covers(&layers, c), [tp, inner], "a clipped layer: covered by what is over its clipping group"); // Under the box, the covering layers leave the document; over it, they alone show. - let above = Above { ctx: egui::Context::default(), name: String::new(), max_side: 64, ids: vec![tp, inner], shown: None }; + let above = Above { ctx: egui::Context::default(), name: String::new(), max_side: 64, ids: vec![tp, inner], applied: Vec::new(), shown: None }; let mut under = layers.clone(); above.hide_in(&mut under); let vis = |ls: &[Layer]| [ls[0].visible, ls[1].visible, ls[2].visible]; @@ -1955,20 +2002,50 @@ fn plain_layers_from_the_top_cover_the_moving_one() { }; assert_eq!(inside, [false, false, true]); - // A layer that blends with what is under it stops the search: it and everything under it - // stay in the document under the box. + // A layer that blends with what is under it can't be drawn on its own: it stays in the + // document under the box, and the plain layers around it still cover (#9387). layers[2].blend = BlendMode::Multiply; - assert!(covers(&layers, t).is_empty()); + assert_eq!(covers(&layers, t), [inner]); layers[2].blend = BlendMode::Normal; let mut multiply = px("multiply"); multiply.blend = BlendMode::Multiply; + let m = multiply.id; layers.insert(2, multiply); - assert_eq!(covers(&layers, t), [tp]); + assert_eq!(covers(&layers, t), [tp, inner]); + assert!(!covers(&layers, t).contains(&m)); // A group that isn't Normal or Pass Through keeps what is inside it. layers[1].blend = BlendMode::Screen; layers.remove(2); assert_eq!(covers(&layers, t), [tp]); - assert_eq!(covers(&layers, LayerId::fresh()), [tp], "not in the stack: down to the first layer that isn't plain"); + assert_eq!(covers(&layers, LayerId::fresh()), [tp, layers[0].id], "not in the stack: every plain layer, the Screen group passed over"); +} + +/// #9387: an adjustment layer over covering layers is composited with them as well as staying +/// under the box; one with nothing covering under it has nothing to adjust. +#[test] +fn adjustments_over_covering_layers_are_applied_to_them() { + use photocraft_doc::{Adjustment, Layer}; + let px = |name: &str| Layer::raster(name, photocraft_color::PixelFormat::RGBA8); + let adjustment = |name: &str| Layer::new(name, LayerContent::Adjustment(Adjustment::Invert)); + let find = |layers: &[Layer], id: LayerId| { + let (mut out, mut applied) = (Vec::new(), Vec::new()); + covering(layers, id, &mut out, &mut applied); + (out, applied) + }; + let (target, under, over, adj_top, adj_between, adj_under) = + (px("target"), px("under"), px("over"), adjustment("top"), adjustment("between"), adjustment("under")); + let (t, u, o, top, between, below) = (target.id, under.id, over.id, adj_top.id, adj_between.id, adj_under.id); + // Bottom to top: target, an adjustment with nothing to adjust over it, two plain layers + // around another adjustment, and an adjustment on top. + let layers = vec![target, adj_under, under, adj_between, over, adj_top]; + let (covers, applied) = find(&layers, t); + assert_eq!(covers, [o, u]); + assert_eq!(applied, [top, between], "adjustments over a covering layer, top down"); + assert!(!applied.contains(&below), "nothing covers under it"); + + // Nothing covers: no adjustment applies. + let alone = vec![px("a"), adjustment("b")]; + assert_eq!(find(&alone, alone[0].id), (Vec::new(), Vec::new())); } /// A context that reports a real GPU's texture limit (the default reports 2048).