Merge pull request #69 from storytold/fix/issue-47-masking-fit

Issue #47: Masking: keep the panel's contents within its width when a mask is selected
This commit is contained in:
Brandon Thomas
2026-10-04 13:59:55 -04:00
committed by GitHub
3 changed files with 82 additions and 6 deletions
+26 -5
View File
@@ -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);
+55
View File
@@ -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}));
+1 -1
View File
@@ -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 |