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 |