From 0ad313d445fdeea88f72aa3a5cc9dadeef2f87a4 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:41:35 -0400 Subject: [PATCH] Issue #47: Masking: keep the panel's contents within its width when a mask is selected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a mask added the Add/Subtract/Intersect/Invert/Delete row in one non-wrapping horizontal layout (~273 pt in a ~220 pt column), and the Create New Mask grid needed 226 pt: the panel's contents grew wider than the panel (a 250 pt panel came out 275 pt wide) and were drawn shifted left, clipped by the photo area. The action row now wraps, the tile grid sizes its tiles and columns to the available width (four tiles of 48–52 pt, else three), and a component's name is truncated before its options button. A headless test checks that every widget stays inside the right panel at 250, 270, 330 and 520 pt, with and without a selected mask and with the brush settings open. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/masking.rs | 31 +++++++++++++--- crates/ui-egui/src/tests_panels.rs | 55 ++++++++++++++++++++++++++++ docs/parity.md | 2 +- 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 0f501f9f..ab494432 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -58,6 +58,16 @@ fn kind_label(s: &MaskShape) -> (&'static str, Icon) { } } +const TILE: f32 = 52.0; +const TILE_GAP: f32 = 6.0; + +/// Columns and tile width of the Create New Mask grid in `width`: four tiles of up to 52 pt (at +/// least 48, so the labels fit), else three (narrowed if even those don't fit). +fn tile_layout(width: f32) -> (usize, f32) { + let tile = |n: f32| ((width - (n - 1.0) * TILE_GAP) / n).floor().min(TILE); + if tile(4.0) >= 48.0 { (4, tile(4.0)) } else { (3, tile(3.0).max(24.0)) } +} + pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { let t = Tokens::get(ui.ctx()); let d = app.session.develop_of(id).unwrap_or_default(); @@ -75,9 +85,11 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { ("luminanceRange", "Luminance", Icon::Sliders), ("colorRange", "Color", Icon::Picker), ]; - egui::Grid::new("mask-tiles").spacing(vec2(6.0, 6.0)).show(ui, |ui| { + // four 52 pt tiles a row when they fit, else as many as fit (at least three, shrunk) + let (cols, tile) = tile_layout(ui.available_width()); + egui::Grid::new("mask-tiles").spacing(vec2(TILE_GAP, TILE_GAP)).show(ui, |ui| { for (i, (kind, label, icon)) in tiles.iter().enumerate() { - let (r, resp) = ui.allocate_exact_size(vec2(52.0, 52.0), Sense::click()); + let (r, resp) = ui.allocate_exact_size(vec2(tile, 52.0), Sense::click()); register(ui.ctx(), format!("maskNew:{kind}"), r); ui.painter().rect_filled(r, 4.0, if resp.hovered() { t.hover } else { t.inset }); paint(ui.painter(), Rect::from_center_size(r.center() - vec2(0.0, 7.0), vec2(20.0, 20.0)), *icon, t.text_label); @@ -101,7 +113,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } } } - if i % 4 == 3 { + if i % cols == cols - 1 { ui.end_row(); } } @@ -228,7 +240,14 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } return; } - let resp = ui.add(egui::Label::new(format!("{op}{label}{}", if c.invert { " (inverted)" } else { "" })).sense(Sense::click())); + // a long name is cut short (with …) before the options button, not past the panel + let text = format!("{op}{label}{}", if c.invert { " (inverted)" } else { "" }); + let room = (ui.available_width() - 30.0).max(0.0); + let resp = ui.scope(|ui| { + ui.set_max_width(room); + ui.add(egui::Label::new(text).truncate().sense(Sense::click())) + }); + let resp = resp.inner; register(ui.ctx(), format!("component:{i}"), resp.rect); if resp.double_clicked() { app.ui.renaming_component = Some((m.id, i, label.clone())); @@ -243,7 +262,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { range_controls(app, ui, i, &c.shape); } ui.add_space(6.0); - ui.horizontal(|ui| { + // wraps: the five actions are wider than a narrow panel + ui.horizontal_wrapped(|ui| { + ui.spacing_mut().item_spacing.y = 6.0; let add = text_button(ui, "maskAddComp", "Add", false); egui::Popup::menu(&add).show(|ui| component_menu(app, ui, "add")); let sub = text_button(ui, "maskSubComp", "Subtract", false); diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index 2f6873ed..7de05e0b 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -79,6 +79,61 @@ fn side_panels_resize_by_their_inner_edge_and_remember_it() { assert_eq!(u.sanitized().right_width, RIGHT_WIDTH.max); } +/// Every widget drawn over the right panel lies within it (issue #47: selecting a mask added a +/// row wider than the panel, which pushed the panel's contents left, clipping them). +fn assert_inside_right_panel(h: &Headless, what: &str) { + let panel = widget(h, "panel:right_panel"); + let mut n = 0; + for (id, r) in &h.app.widgets { + // widgets of the photo area (pins, filmstrip cells clipped at its edge) and of the tool + // strip are outside on purpose + if ["panel:", "film:", "maskPin"].iter().any(|p| id.starts_with(p)) + || r.bottom() <= panel.top() + || r.right() <= panel.left() + 1.5 + || r.left() >= panel.right() - 1.5 + { + continue; + } + n += 1; + assert!(r.left() >= panel.left() - 0.5 && r.right() <= panel.right() + 0.5, "{what}: {id} {r:?} sticks out of the panel {panel:?}"); + } + assert!(n > 10, "{what}: only {n} widgets in the panel"); +} + +#[test] +fn masking_contents_fit_the_right_panel_at_any_width() { + let mut h = demo([1400.0, 900.0], json!({"view": "detail", "right": "masking"})); + for (kind, op) in [("subject", None), ("linear", None), ("luminanceRange", Some("subtract")), ("radial", Some("intersect"))] { + let (command, params) = match op { + None => ("mask.add", json!({"kind": kind})), + Some(op) => ("mask.addComponent", json!({"kind": kind, "op": op})), + }; + let r = h.request("engine.execute", json!({"command": command, "params": params}), T); + assert_eq!(r["ok"], true, "{r}"); + } + let r = h.request( + "engine.execute", + json!({"command": "mask.component", "params": {"component": 0, "action": "rename", "name": "A rather long component name for this mask"}}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + for width in [RIGHT_WIDTH.min, RIGHT_WIDTH.default, 330.0, RIGHT_WIDTH.max] { + for (selected, tool) in [(false, ""), (true, ""), (true, "brush")] { + let mask = if selected { json!(2) } else { serde_json::Value::Null }; + h.request("engine.execute", json!({"command": "mask.select", "params": {"id": mask}}), T); + h.request("ui.set", json!({"rightWidth": width, "tool": tool}), T); + h.step(); + h.step(); + let what = format!("width {width}, mask selected {selected}, tool {tool:?}"); + assert_eq!(widget(&h, "panel:right_panel").width(), width, "{what}"); + if selected { + assert!(h.app.widgets.iter().any(|(id, _)| id == "button:maskInvert"), "{what}: no mask actions"); + } + assert_inside_right_panel(&h, &what); + } + } +} + #[test] fn a_narrow_window_shrinks_the_panels_without_forgetting_their_width() { let mut h = demo([1400.0, 900.0], json!({"view": "detail", "right": "edit", "leftPanel": true, "rightWidth": 480.0, "leftWidth": 400.0})); diff --git a/docs/parity.md b/docs/parity.md index 55cf2f73..3699bea8 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -309,7 +309,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-MASK-PANEL | Masks panel | P0 | ✅ | `cmd:panel.masking`, `cmd:mask.add`, `cmd:mask.select`, `cmd:mask.rename`, `cmd:mask.duplicate` (`invert`), `cmd:mask.move`, `cmd:mask.visible`, `cmd:mask.delete`, `crates/ui-egui/src/panels/masking.rs` | list with per-mask show/hide eye, double-click rename, right-click menu (duplicate, duplicate and invert, invert, hide, move up/down, rename, delete); no drag-to-reorder | +| LR-MASK-PANEL | Masks panel | P0 | ✅ | `cmd:panel.masking`, `cmd:mask.add`, `cmd:mask.select`, `cmd:mask.rename`, `cmd:mask.duplicate` (`invert`), `cmd:mask.move`, `cmd:mask.visible`, `cmd:mask.delete`, `crates/ui-egui/src/panels/masking.rs` | list with per-mask show/hide eye, double-click rename, right-click menu (duplicate, duplicate and invert, invert, hide, move up/down, rename, delete); fits any panel width (create tiles go to three columns, mask actions wrap, long component names are cut short); no drag-to-reorder | | LR-MASK-SUBJECT | Select subject | P2 | 🟡 | `cmd:mask.add` (`subject`), `crates/pipeline/src/masks.rs` | saliency heuristic, no segmentation model | | LR-MASK-SKY | Select sky | P2 | 🟡 | `cmd:mask.add` (`sky`) | heuristic | | LR-MASK-BACKGROUND | Select background | P2 | 🟡 | `cmd:mask.add` (`background`) | inverse of the subject heuristic |