collective

Browse / github.com/storytold/photocraft / patch

Fix adjustment layer name clicks targeting masks

[bug] Clicking over name of adjustment layer selects it's layer mask instead of the adjustment layer 1. Clicking over name of adjustment layer 2. Its layer mask gets selected instead of the adjustment layer This behavior is wrong. compared to what selecting a normal layer does. 1. Clicking over name of a layer 2. The layer gets selected even if it has layer mask Please fix this. it's tricking me almost every time.

Status
open
Score
0.00
Adopters
0 other operators
Author
vanvo (imported from pull request #9703, open)
Base
59cb4c49bd236df04d8e37d4764253f29ca6600a
Size
+45 −6 in 2 files
Review
pending
Shared
2026-10-28
Id
5bb234f631ca2f8409bba2fa

How it went

No other operator has reported using it yet.

Diff

raw · crates/ui-egui/src/mask_thumbs_ui/tests.rs crates/ui-egui/src/panels.rs

diff --git a/crates/ui-egui/src/mask_thumbs_ui/tests.rs b/crates/ui-egui/src/mask_thumbs_ui/tests.rs--- a/crates/ui-egui/src/mask_thumbs_ui/tests.rs+++ b/crates/ui-egui/src/mask_thumbs_ui/tests.rs@@ -804,7 +804,8 @@ fn a_new_adjustment_or_fill_layer_shows_its_settings_until_the_mask_thumbnail_is         // Clicking the row (not a thumbnail) keeps the settings.         let row = h.get_by_label(&layer(&h, id).name).rect().center();         click_with(&mut h, row, Modifiers::NONE);-        assert!(h.state().ui.mask_target);+        assert_eq!(h.state().ui.mask_target, command.starts_with("layer.newFillLayer."));+        assert!(!h.state().ui.vector_mask_target);         assert!(h.query_all_by_label("Density").next().is_none(), "{command}: a row click keeps the settings");         // The mask thumbnail switches Properties to the mask.         let r = mask_rect(&h, id, MaskKind::Pixel);@@ -836,3 +837,41 @@ fn a_new_adjustment_layer_brings_properties_to_the_front() {     assert!(!h.state().ui.dock.is_collapsed(crate::dock::Group::Properties), "the group is expanded");     assert!(h.query_all_by_label("Reset to defaults").next().is_some(), "the Curves settings show"); }++#[test]+fn adjustment_names_target_contents_instead_of_masks() {+    for command in ["layer.newAdjustmentLayer.hueSaturation", "layer.newAdjustmentLayer.curves"] {+        for ppp in [1.0, 2.0] {+            let (mut s, _, other) = session();+            s.execute(command, json!({})).unwrap();+            let id = s.active().unwrap().active_layer.unwrap().0;+            let path = json!({"subpaths": [{"knots": [[20, 10], [120, 10], [120, 90], [20, 90]]}]});+            s.execute("layer.vectorMask.add", json!({"layer": id, "path": path})).unwrap();+            let mut h = harness(s, 0, ppp, 360.0);+            for kind in [MaskKind::Pixel, MaskKind::Vector] {+                let at = mask_rect(&h, id, kind).center();+                click_with(&mut h, at, Modifiers::NONE);+                assert_eq!(h.state().ui.mask_target, kind == MaskKind::Pixel);+                assert_eq!(h.state().ui.vector_mask_target, kind == MaskKind::Vector);+                let name = crate::layer_row_ui::recorded(&h.ctx).into_iter().find(|r| r.layer == id).unwrap().name.unwrap().center();+                click_with(&mut h, name, Modifiers::NONE);+                assert!(!h.state().ui.mask_target && !h.state().ui.vector_mask_target, "{command} @{ppp}x after {kind:?}");+                let thumb = crate::layer_row_ui::recorded(&h.ctx).into_iter().find(|r| r.layer == id).unwrap().thumbnail.unwrap();+                assert!(near(super::brackets(&h.ctx).unwrap(), thumb.expand(3.0)), "the content thumbnail is selected");+                assert_eq!(h.state().session.active().unwrap().active_layer.unwrap().0, id);+            }+            let name = crate::layer_row_ui::recorded(&h.ctx).into_iter().find(|r| r.layer == other).unwrap().name.unwrap().center();+            click_with(&mut h, name, Modifiers::NONE);+            let name = crate::layer_row_ui::recorded(&h.ctx).into_iter().find(|r| r.layer == id).unwrap().name.unwrap().center();+            click_with(&mut h, name, Modifiers::NONE);+            assert_eq!(h.state().session.active().unwrap().active_layer.unwrap().0, id);+            assert!(!h.state().ui.mask_target && !h.state().ui.vector_mask_target, "{command} @{ppp}x from another layer");+            assert_eq!(crate::canvas::paint_target(h.state()), json!("mask"));+            let content = layer(&h, id).content;+            h.state_mut().run("edit.fill", json!({"contents": "black"})).unwrap();+            assert_eq!(layer(&h, id).content, content, "filling must preserve the adjustment or fill content");+            assert_eq!(layer(&h, id).mask.unwrap().surface.sample_channel(50, 50, 0), 0.0, "Fill edits the mask");+            assert!(!h.state().ui.mask_target && !h.state().ui.vector_mask_target, "painting leaves the content thumbnail selected");+        }+    }+}diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs--- a/crates/ui-egui/src/panels.rs+++ b/crates/ui-egui/src/panels.rs@@ -3490,8 +3490,7 @@ fn layer_row(         let mode = if m.command && on_thumb && doc.is_background(l.id) { "replace" } else { select_mode(m) };         actions.push(("layer.select".into(), json!({"layer": l.id.0, "mode": mode})));         crate::smart_ui::deselect(ctx, doc.id);-        // A thumbnail explicitly selects content or a mask. Selecting an adjustment/fill row-        // defaults to its mask, but clicking its content thumbnail must leave the mask target.+        // Adjustment names target contents; fill rows keep their existing mask default.         let on_mask = mask_rect.zip(pos).is_some_and(|(r, p)| r.expand(2.0).contains(p));         let on_vector = pos.and_then(|p| masks.hit(p)) == Some(crate::mask_thumbs_ui::MaskKind::Vector);         // Clicking the layer thumbnail leaves mask view (#196).@@ -3500,19 +3499,20 @@ fn layer_row(             let mask = if v.filter { "filter" } else { "layer" };             actions.push((photocraft_engine::mask_view_cmds::ID.into(), json!({"layer": l.id.0, "mode": "off", "mask": mask})));         }-        let content_less = matches!(l.content, LayerContent::Adjustment(_) | LayerContent::Fill(_));+        let adjustment = matches!(l.content, LayerContent::Adjustment(_));+        let fill_mask = matches!(l.content, LayerContent::Fill(_)) && l.mask.is_some();         if on_vector {             // The vector mask thumbnail targets the vector mask: the path tools edit it and the             // Paths panel selects the layer's path.             app.ui.selected_path = Some("layer".into());             actions.push(("ui.vectorMaskTarget".into(), json!(true)));-        } else if on_mask || (content_less && l.mask.is_some() && !on_thumb) {+        } else if on_mask || (fill_mask && !on_thumb) {             if on_mask {                 // The mask thumbnail also switches Properties to the mask (#3369).                 app.ui.properties_layer = None;             }             actions.push(("ui.maskTarget".into(), json!(true)));-        } else if on_thumb || !row.primary {+        } else if on_thumb || adjustment || !row.primary {             actions.push(("ui.maskTarget".into(), json!(false)));         }         // ⌥-click the layer thumbnail: fit the layer on screen, as Photoshop does.