shifathosn
/
lightcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Issue #47: Masking: keep the panel's contents within its width when a mask is selected 

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 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas 2026-10-04 11:41:35 -04:00
parent d6476e7497
commit 0ad313d445
3 changed files with 82 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
crates/ui-egui/src/panels/masking.rs
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
crates/ui-egui/src/tests_panels.rs
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}));

2
docs/parity.md
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 |