From 195a519c0788fc2a24327f9834649ce06ed905e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Mon, 19 Oct 2026 07:57:06 +0800 Subject: [PATCH] Address review: regression tests and overlay outline parity - Add two regression tests in tests_masking.rs (next to mask_overlay_keys_and_pins) covering #2210: every visible mask is outlined under the all-masks overlay while only the selected mask registers maskHandle: widgets, and an unselected mask's outline is painted on screen. - Match the outline condition to the painted region: only outline masks with components, like view_overlay already does. - Add examples/mask_overlay_shot.rs (+ dev-dep png) to render a screenshot of the overlay for the PR. - Revert the earlier standalone test in headless.rs; make mask_overlay and CanvasMap::new pub(crate) so the masking tests can reach them. --- Cargo.lock | 1 + crates/ui-egui/Cargo.toml | 1 + crates/ui-egui/examples/mask_overlay_shot.rs | 49 +++++++++++++++++ crates/ui-egui/src/panels/detail.rs | 9 ++-- crates/ui-egui/src/tests_masking.rs | 57 ++++++++++++++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 crates/ui-egui/examples/mask_overlay_shot.rs diff --git a/Cargo.lock b/Cargo.lock index a33b59a81..8bb819d53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3525,6 +3525,7 @@ dependencies = [ "lightcraft-slideshow", "lightcraft-testkit", "log", + "png", "serde", "serde_json", "toml", diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 1ae33a03d..3b423f8e8 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -35,6 +35,7 @@ lightcraft-denoise = { workspace = true, features = ["runtime"] } lightcraft-codecs = { workspace = true } lightcraft-faces = { workspace = true } lightcraft-merge = { workspace = true } +png = { workspace = true } [build-dependencies] serde_json = { workspace = true } diff --git a/crates/ui-egui/examples/mask_overlay_shot.rs b/crates/ui-egui/examples/mask_overlay_shot.rs new file mode 100644 index 000000000..36875aee9 --- /dev/null +++ b/crates/ui-egui/examples/mask_overlay_shot.rs @@ -0,0 +1,49 @@ +//! Render a screenshot of the masking view with the all-masks overlay on: several visible masks, +//! each outlined, only the selected one draggable. Used to attach a visual to PR #2210. +//! +//! Run: `cargo run -p lightcraft-ui-egui --example mask_overlay_shot -- ` + +use std::time::Duration; + +use lightcraft_raster::Rgba8; +use serde_json::json; + +use lightcraft_ui_egui::headless::Headless; +use lightcraft_ui_egui::{LightcraftApp, Services}; + +fn main() { + let out = std::env::args().nth(1).unwrap_or_else(|| "mask-overlay.png".into()); + let t = Duration::from_secs(20); + + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + let _ = h.request("ui.set", json!({"view": "detail"}), t); + let _ = h.request("engine.execute", json!({"command": "panel.masking"}), t); + + // A few visible masks; mask.add selects the newest, so the last one is selected. + for params in [ + json!({"kind": "radial", "center": [0.3, 0.4], "rx": 0.12, "ry": 0.12}), + json!({"kind": "linear", "start": [0.65, 0.25], "end": [0.65, 0.6]}), + json!({"kind": "radial", "center": [0.7, 0.65], "rx": 0.1, "ry": 0.14, "angle": 30}), + ] { + let _ = h.request("engine.execute", json!({"command": "mask.add", "params": params}), t); + } + let _ = h.request("engine.execute", json!({"command": "view.maskOverlayAll"}), t); + let _ = h.request("engine.execute", json!({"command": "view.maskPins", "params": {"show": false}}), t); + h.settle(Duration::from_secs(120)); + + let px = h.view.paint(&Default::default()); + let rgba = Rgba8::from_fn(px.width(), px.height(), |x, y| { + let p = px.pixels[y * px.width() + x]; + [p.r(), p.g(), p.b(), p.a()] + }); + + let file = std::fs::File::create(&out).expect("create png"); + let mut enc = png::Encoder::new(file, rgba.width as u32, rgba.height as u32); + enc.set_color(png::ColorType::Rgba); + enc.set_depth(png::BitDepth::Eight); + let mut w = enc.write_header().expect("png header"); + w.write_image_data(&rgba.as_bytes()).expect("encode png"); + println!("wrote {out} ({}, {})", rgba.width, rgba.height); +} diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 3d0297831..1f24f7a43 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -81,7 +81,7 @@ pub struct CanvasMap { } impl CanvasMap { - fn new(frame: &Frame, rect: Rect) -> CanvasMap { + pub(crate) fn new(frame: &Frame, rect: Rect) -> CanvasMap { let k = 8.0f32; let (w, h) = ((rect.width() * k).round().max(1.0) as usize, (rect.height() * k).round().max(1.0) as usize); let to_norm = frame.out_to_norm(w, h); @@ -1996,7 +1996,7 @@ fn color_range_pick(app: &mut LightcraftApp, ui: &egui::Ui, resp: &egui::Respons /// The Masking tool on the photo: outlines and handles of the selected mask, a pin per mask /// component (click selects its mask, drag moves the component), and brush painting. The mask /// itself shows as a rendered overlay ([`view_overlay`]). -fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, d: &DevelopSettings) { +pub(crate) fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, d: &DevelopSettings) { let long = (map.rect.width().max(map.rect.height())) as f64; let active = app.session.active_mask; let clip = app.canvas_rect.unwrap_or(map.rect); @@ -2013,8 +2013,9 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons let sel = Some(m.id) == active; // Ctrl+Shift+O (`mask_overlay_all`) shows every visible mask's region; outline each one // too, not just the selected mask. Handles and the rotation grip stay on the selected - // mask so nothing else becomes draggable. - let show_outline = sel || (app.ui.mask_overlay_all && m.visible); + // mask so nothing else becomes draggable. The component check keeps this in step with the + // painted region (`view_overlay` at detail.rs:1354 only covers masks with components). + let show_outline = sel || (app.ui.mask_overlay_all && m.visible && !m.components.is_empty()); let stroke_a: u8 = if sel { 230 } else { 170 }; for (ci, c) in m.components.iter().enumerate() { match &c.shape { diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 4e397208e..c78183a6e 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -317,6 +317,63 @@ fn mask_overlay_keys_and_pins() { h.settle(SETTLE); } +/// #2210: Ctrl+Shift+O (the all-masks overlay) draws the white outline of every visible +/// Radial/Linear mask, not just the selected one — but only the selected mask registers +/// `maskHandle:` elements, so nothing else becomes draggable. +#[test] +fn all_masks_overlay_outlines_every_visible_mask_but_only_the_selected_one_is_draggable() { + let mut h = detail("panel.masking"); + // Two visible radial masks; mask 2 ends up selected (mask.add selects the new one). + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.3, 0.5], "rx": 0.12, "ry": 0.12})); + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.7, 0.5], "rx": 0.12, "ry": 0.12})); + exec(&mut h, "view.maskOverlayAll", json!({})); + assert!(h.app.ui.mask_overlay_all); + assert_eq!(h.app.session.active_mask, Some(2)); + // Pins add their own elements; keep them out of the handle count. + exec(&mut h, "view.maskPins", json!({"show": false})); + h.settle(SETTLE); + + let handles_of = |h: &Headless, id: usize| h.app.widgets.iter().filter(|(w, _)| w.starts_with(&format!("maskHandle:{id}:"))).count(); + // The selected mask keeps its 4 edge handles and its rotation grip. + assert_eq!(handles_of(&h, 2), 5, "the selected mask keeps its handles"); + // The other visible mask draws its outline but registers no handle: it is not draggable. + assert_eq!(handles_of(&h, 1), 0, "an unselected mask must not become draggable"); + // …while a mask with no components has nothing to outline and nothing to grab either. + exec(&mut h, "mask.add", json!({"kind": "luminanceRange"})); + exec(&mut h, "mask.addComponent", json!({"kind": "radial", "op": "add", "center": [0.5, 0.8], "rx": 0.08, "ry": 0.08})); + assert_eq!(h.app.session.active_mask, Some(3)); + assert_eq!(handles_of(&h, 1), 0); + h.settle(SETTLE); +} + +/// #2210: the outlines of *unselected* masks are painted, not registered as widgets, so the way to +/// observe them is the rendered pixels. With the all-masks overlay on, the unselected mask's +/// ellipse edge must carry white stroke pixels the photo alone does not have. +#[test] +fn all_masks_overlay_paints_the_outline_of_an_unselected_mask() { + let mut h = detail("panel.masking"); + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.3, 0.5], "rx": 0.12, "ry": 0.12})); + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.7, 0.5], "rx": 0.12, "ry": 0.12})); + assert_eq!(h.app.session.active_mask, Some(2)); + exec(&mut h, "view.maskPins", json!({"show": false})); + + // Sample the whole canvas: the outline is white (alpha 170) over the photo, so the overlay adds + // near-white pixels along the unselected mask's ellipse edge. + let white_pixels = |h: &mut Headless, on: bool| -> usize { + let overlay = h.app.ui.mask_overlay_all; + if overlay != on { + exec(h, "view.maskOverlayAll", json!({})); + } + assert_eq!(h.app.ui.mask_overlay_all, on); + h.settle(SETTLE); + let px = h.view.paint(&Default::default()); + px.pixels.iter().filter(|p| p.r() > 200 && p.g() > 200 && p.b() > 200).count() + }; + let off = white_pixels(&mut h, false); + let on = white_pixels(&mut h, true); + assert!(on > off, "the unselected mask's outline must add white pixels: off {off}, on {on}"); +} + /// #518: holding the brush still, however long, paints the same feathered spot as a quick press. #[test] fn holding_the_brush_still_does_not_harden_the_spot() {