From 273ff014e3d21882b3959959eb7da00a42290e56 Mon Sep 17 00:00:00 2001 From: Valentin Klein Date: Sun, 11 Oct 2026 12:12:57 +0200 Subject: [PATCH] Layers: double-click a shape layer's thumbnail to pick its fill colour (#2812) (#2820) Like Photoshop, double-clicking a solid-colour shape layer's thumbnail (or Layer > Layer Content Options...) opens the Color Picker on the shape's fill. The canvas previews the pick, Cancel changes nothing and OK commits one undoable shape.edit with the new fill (keeping the fill's alpha). Gradient, pattern or no-fill shapes keep opening their Properties controls. Co-authored-by: Claude Opus 5.5 --- crates/ui-egui/src/canvas.rs | 3 + crates/ui-egui/src/color_picker_ui.rs | 3 + crates/ui-egui/src/lib.rs | 4 + crates/ui-egui/src/menus.rs | 5 +- crates/ui-egui/src/panels.rs | 7 +- crates/ui-egui/src/shape_fill_ui.rs | 124 ++++++++++++++++++++++ crates/ui-egui/src/shape_fill_ui/tests.rs | 72 +++++++++++++ 7 files changed, 213 insertions(+), 5 deletions(-) create mode 100644 crates/ui-egui/src/shape_fill_ui.rs create mode 100644 crates/ui-egui/src/shape_fill_ui/tests.rs diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 2fcf73a330..2df3e15a7e 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -955,6 +955,9 @@ pub(crate) fn display_doc(app: &mut PhotocraftApp, idx: usize) -> (std::sync::Ar if let Some(shown) = crate::solid_fill_ui::display_doc(app, idx) { return shown; } + if let Some(shown) = crate::shape_fill_ui::display_doc(app, idx) { + return shown; + } if let Some(shown) = crate::shape_stroke_ui::display_doc(app, idx) { return shown; } diff --git a/crates/ui-egui/src/color_picker_ui.rs b/crates/ui-egui/src/color_picker_ui.rs index c781b6b750..41f1af04ea 100644 --- a/crates/ui-egui/src/color_picker_ui.rs +++ b/crates/ui-egui/src/color_picker_ui.rs @@ -694,6 +694,9 @@ pub fn confirm(app: &mut PhotocraftApp, f: &Map) -> Result, String>)>, pub(crate) solid_fill_preview: Option, + /// Shape fill Color Picker preview (shape_fill_ui). + pub(crate) shape_fill_preview: Option, /// Liquify dialog, Puppet Warp and Perspective Warp sessions (distort_ui). pub(crate) distort: distort_ui::Distort, /// Gradient tool live-mode drags and previews (gradient_ui). @@ -719,6 +722,7 @@ impl PhotocraftApp { style_preview: None, text_style_preview: None, solid_fill_preview: None, + shape_fill_preview: None, distort: Default::default(), gradient: Default::default(), camera_raw: None, diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index d4e4a5cb1f..427d57d52d 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -411,9 +411,10 @@ pub(crate) fn invoke_unguarded(app: &mut PhotocraftApp, ctx: &egui::Context, id: } Ok(Value::Null) } - // Solid Color uses the Color Picker; other adjustment / fill controls use Properties. + // Solid Color fills and solid-colour shapes use the Color Picker; other adjustment / + // fill / shape controls use Properties. "layer.layerContentOptions" => { - if let Some(dialog) = crate::solid_fill_ui::open(app) { + if let Some(dialog) = crate::solid_fill_ui::open(app).or_else(|| crate::shape_fill_ui::open(app)) { return Ok(json!({"dialog": dialog})); } let r = app.run(id, params)?; diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index e0b963e7e9..4748d2bc04 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -2599,8 +2599,9 @@ fn layer_row( } // Double-click: the name renames in place (over the row's full height, not just the glyphs: // #651); the Background, which can't be renamed while it's locked, becomes a normal layer; an - // adjustment or fill thumbnail opens its settings and a Smart Object thumbnail its contents, - // and a type thumbnail edits its text; anywhere else on the row opens Layer Style (#350, #537). + // adjustment, fill or shape thumbnail opens its settings (a solid colour: the Color Picker, + // #2812) and a Smart Object thumbnail its contents, and a type thumbnail edits its text; + // anywhere else on the row opens Layer Style (#350, #537). // The first click already made this the active layer. if resp.double_clicked() { let pos = resp.interact_pointer_pos(); @@ -2626,7 +2627,7 @@ fn layer_row( } } else if pos.and_then(|p| masks.hit(p)).is_none() { let id = match &l.content { - LayerContent::Adjustment(_) | LayerContent::Fill(_) if on(thumb) => "layer.layerContentOptions", + LayerContent::Adjustment(_) | LayerContent::Fill(_) | LayerContent::Shape(_) if on(thumb) => "layer.layerContentOptions", LayerContent::Smart(_) if on(thumb) => "layer.smartObjects.editContents", LayerContent::Text(_) if on(thumb) => "type.editText", _ => "layer.layerStyle.blendingOptions", diff --git a/crates/ui-egui/src/shape_fill_ui.rs b/crates/ui-egui/src/shape_fill_ui.rs new file mode 100644 index 0000000000..37731ce243 --- /dev/null +++ b/crates/ui-egui/src/shape_fill_ui.rs @@ -0,0 +1,124 @@ +//! A solid-colour shape layer's thumbnail (or Layer › Layer Content Options…) opens the shared +//! Color Picker on the shape's fill, like Photoshop (#2812). The canvas previews a shallow +//! document snapshot; Cancel changes nothing and OK commits exactly one `shape.edit`. + +use std::hash::{Hash, Hasher}; +use std::sync::Arc; + +use photocraft_color::Color; +use photocraft_doc::{DocId, Document, Fill, LayerContent, LayerId}; +use serde_json::{Map, Value, json}; + +use crate::{PhotocraftApp, color_picker_ui}; + +const COMMAND: &str = "shape.edit"; +const MARKER: &str = "__shapeFill"; + +pub(crate) struct Preview { + document: DocId, + revision: u64, + dialog: u64, + layer: LayerId, + color: Color, + shown: Arc, + key: u64, +} + +/// The solid fill colour of shape layer `layer`, if it has one (not a gradient, pattern or none). +fn solid_fill(doc: &Document, layer: LayerId) -> Option { + match &doc.layer(layer)?.content { + LayerContent::Shape(sh) => match sh.fill { + Some(Fill::Solid(c)) => Some(c), + _ => None, + }, + _ => None, + } +} + +/// Opens the picker for the active layer when it is a shape filled with a solid colour. +pub(crate) fn open(app: &mut PhotocraftApp) -> Option { + let st = app.session.active()?; + let layer = st.active_layer?; + let color = solid_fill(&st.doc, layer)?; + let params = json!({"layer": layer.0, "document": st.doc.id.0}); + let id = color_picker_ui::open_for_command(app, "Color Picker (Solid Color)", color.to_rgb(), COMMAND, params); + if let Some(d) = app.ui.dialog_mut(id) { + d.fields.insert(MARKER.into(), json!(true)); + } + Some(id) +} + +pub(crate) fn owns(f: &Map) -> bool { + f.get(MARKER).and_then(Value::as_bool) == Some(true) +} + +/// The picker's colour as `shape.edit` stores it: RGB with the original fill's alpha. +fn picked(f: &Map, original: Color) -> Option { + let [r, g, b] = color_picker_ui::parse_hex(f.get("color")?.as_str()?)?; + Some(Color::rgba(r, g, b, original.alpha)) +} + +/// The target layer and its current fill, unless the dialog is stale (another document is +/// active, or the layer is gone or no longer a solid-colour shape). +fn target(app: &PhotocraftApp, f: &Map, idx: usize) -> Result<(LayerId, Color), String> { + let params = f.get("__params").ok_or("missing shape fill target")?; + let st = app.session.documents().get(idx).ok_or("no document open")?; + if params.get("document").and_then(Value::as_u64) != Some(st.doc.id.0) { + return Err("the target document is no longer active".into()); + } + let layer = LayerId(params.get("layer").and_then(Value::as_u64).ok_or("missing shape layer")?); + let original = solid_fill(&st.doc, layer).ok_or("the target shape layer no longer has a solid colour fill")?; + Ok((layer, original)) +} + +/// OK: one `shape.edit` with the new fill; an unchanged colour keeps the stored one as it is. +pub(crate) fn confirm(app: &mut PhotocraftApp, f: &Map) -> Result { + let idx = app.session.active_index().ok_or("no document open")?; + let (layer, original) = target(app, f, idx)?; + if f.get("color") == f.get("__orig") { + return Ok(json!({"layer": layer.0, "unchanged": true})); + } + let color = picked(f, original).ok_or("not a colour")?; + app.run(COMMAND, json!({"layer": layer.0, "fill": [color.c[0], color.c[1], color.c[2], color.alpha]})) +} + +/// The canvas preview while the picker is open: the document with the shape re-rendered in the +/// picked colour, cached per dialog, revision and colour. +pub(crate) fn display_doc(app: &mut PhotocraftApp, idx: usize) -> Option<(Arc, u64)> { + let Some(d) = app.ui.dialogs.iter().rev().find(|d| owns(&d.fields)) else { + app.shape_fill_preview = None; + return None; + }; + if d.fields.get("color") == d.fields.get("__orig") { + return None; + } + let (layer, original) = target(app, &d.fields, idx).ok()?; + let color = picked(&d.fields, original)?; + let st = app.session.documents().get(idx)?; + if let Some(p) = &app.shape_fill_preview + && p.document == st.doc.id + && p.revision == st.revision + && p.dialog == d.id + && p.layer == layer + && p.color == color + { + return Some((p.shown.clone(), p.key)); + } + let mut doc = (*st.doc).clone(); + if let LayerContent::Shape(sh) = &mut doc.layer_mut(layer)?.content { + sh.fill = Some(Fill::Solid(color)); + photocraft_engine::vector_cmds::refresh_shape(&st.doc, sh); + } + let mut hash = std::collections::hash_map::DefaultHasher::new(); + (st.doc.id.0, st.revision, d.id, layer.0).hash(&mut hash); + for component in color.c.into_iter().chain([color.alpha]) { + component.to_bits().hash(&mut hash); + } + let key = (1 << 46) | (hash.finish() & ((1 << 40) - 1)); + let shown = Arc::new(photocraft_engine::mode_cmds::display_document(&doc).unwrap_or(doc)); + app.shape_fill_preview = Some(Preview { document: st.doc.id, revision: st.revision, dialog: d.id, layer, color, shown: shown.clone(), key }); + Some((shown, key)) +} + +#[cfg(test)] +mod tests; diff --git a/crates/ui-egui/src/shape_fill_ui/tests.rs b/crates/ui-egui/src/shape_fill_ui/tests.rs new file mode 100644 index 0000000000..95fd0dd7c8 --- /dev/null +++ b/crates/ui-egui/src/shape_fill_ui/tests.rs @@ -0,0 +1,72 @@ +use egui::{Event, Modifiers, PointerButton, Pos2, pos2, vec2}; +use egui_kittest::Harness; +use photocraft_doc::{Fill, LayerContent, LayerId}; +use serde_json::json; + +use super::*; + +fn double_click(h: &mut Harness<'_, PhotocraftApp>, at: Pos2) { + h.run_steps(40); + for _ in 0..2 { + h.hover_at(at); + h.step(); + for pressed in [true, false] { + h.event(Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.step(); + } + h.run_steps(3); + } +} + +fn fill(app: &PhotocraftApp, id: LayerId) -> Option { + match &app.session.active().unwrap().doc.layer(id).unwrap().content { + LayerContent::Shape(sh) => sh.fill.clone(), + _ => None, + } +} + +/// #2812: double-clicking a shape layer's thumbnail opens the Color Picker on its fill; the +/// canvas previews the pick, Cancel changes nothing and OK is one undoable `shape.edit`. +#[test] +fn shape_thumbnail_double_click_picks_the_fill_colour() { + let mut s = photocraft_engine::Session::new(); + s.execute("file.new", json!({"width": 64, "height": 48})).unwrap(); + let id = LayerId(s.execute("shape.create", json!({"rect": [8, 8, 40, 30], "fill": "#336699"})).unwrap()["layer"].as_u64().unwrap()); + s.execute("layer.new.layer", json!({"name": "Other"})).unwrap(); + let mut h = Harness::builder().with_size(vec2(1440.0, 900.0)).with_step_dt(1.0 / 60.0).with_max_steps(64).build_eframe(move |cc| { + PhotocraftApp::setup_context(&cc.egui_ctx, Default::default()); + PhotocraftApp::new(s, crate::Services::default()) + }); + let ctx = h.ctx.clone(); + let (r, _) = crate::control::ControlRequest::new("ui.set", json!({"dock": {"collapsed": ["color", "properties", "history", "navigator"]}})); + crate::control::handle(h.state_mut(), &ctx, &r); + h.run_steps(8); + let row = crate::layer_row_ui::recorded(&h.ctx).into_iter().find(|r| r.layer == id.0).unwrap().row; + double_click(&mut h, pos2(row.left() + 46.0, row.center().y)); + + let app = h.state_mut(); + assert_eq!(app.session.active().unwrap().active_layer, Some(id)); + let before = app.session.active().unwrap().doc.clone(); + let steps = app.session.active().unwrap().history.entries().len(); + let dialog = app.ui.dialogs.last().filter(|d| owns(&d.fields)).expect("Color Picker opened on the shape fill"); + assert_eq!(dialog.fields["color"], "#336699"); + let dialog = dialog.id; + + // Picking previews on the canvas without touching the document; Cancel drops it. + app.ui.dialog_mut(dialog).unwrap().fields.insert("color".into(), json!("#ff8800")); + let (shown, _) = display_doc(app, 0).unwrap(); + let LayerContent::Shape(sh) = &shown.layer(id).unwrap().content else { panic!() }; + assert_eq!(sh.fill, Some(Fill::Solid(Color::rgb(1.0, 136.0 / 255.0, 0.0)))); + app.ui.close_dialog(dialog); + assert!(display_doc(app, 0).is_none()); + assert!(Arc::ptr_eq(&before, &app.session.active().unwrap().doc), "Cancel leaves the shape alone"); + + // OK commits the colour as one undo step. + let dialog = open(app).unwrap(); + app.ui.dialog_mut(dialog).unwrap().fields.insert("color".into(), json!("#ff8800")); + crate::dialogs::confirm(app, dialog).unwrap(); + assert_eq!(fill(app, id), Some(Fill::Solid(Color::rgb(1.0, 136.0 / 255.0, 0.0)))); + assert_eq!(app.session.active().unwrap().history.entries().len(), steps + 1); + app.run("edit.undo", json!({})).unwrap(); + assert_eq!(*app.session.active().unwrap().doc, *before); +}