diff --git a/crates/algo/src/blur.rs b/crates/algo/src/blur.rs index 9d2fe45001..2ca3bcc88b 100644 --- a/crates/algo/src/blur.rs +++ b/crates/algo/src/blur.rs @@ -348,19 +348,83 @@ fn average_samples(src: &Image, out: Rect, ctx: &Ctx, mut offsets: impl FnMut(f3 res } -pub(crate) fn motion(src: &Image, out: Rect, ctx: &Ctx, angle: f32, distance: f32) -> Vec { +/// Sample offsets of a motion blur of `distance` at `angle`, relative to the pixel centre. +fn motion_offsets(angle: f32, distance: f32) -> Vec<(f32, f32)> { let d = distance.abs(); - if d < 0.5 { - return src.crop(out); - } let (s, c) = angle.to_radians().sin_cos(); let steps = d.ceil() as i32; - average_samples(src, out, ctx, |x, y, pts| { - for i in 0..=steps { + (0..=steps) + .map(|i| { let t = i as f32 / steps as f32 - 0.5; - pts.push((x + c * d * t, y - s * d * t)); + (c * d * t, -s * d * t) + }) + .collect() +} + +/// Motion blur: the mean of bilinear samples along a line centred on the pixel. The offsets are the +/// same for every pixel, so their bilinear taps are merged once into a fixed kernel, which is +/// applied tap by tap to whole rows of a premultiplied window (the same premultiplied mean the +/// per-sample loop takes, summed in another order). +pub(crate) fn motion(src: &Image, out: Rect, ctx: &Ctx, angle: f32, distance: f32) -> Vec { + if distance.abs() < 0.5 { + return src.crop(out); + } + let offs = motion_offsets(angle, distance); + let k = 1.0 / offs.len() as f32; + // Merged taps (dx, dy, weight), in the order first seen. + let mut taps: Vec<(i32, i32, f32)> = Vec::new(); + let mut slot = std::collections::HashMap::new(); + for &(ox, oy) in &offs { + let (x0, y0) = (ox.floor(), oy.floor()); + let (ax, ay) = (ox - x0, oy - y0); + let (x0, y0) = (x0 as i32, y0 as i32); + for (dx, dy, w) in [(0, 0, (1.0 - ax) * (1.0 - ay)), (1, 0, ax * (1.0 - ay)), (0, 1, (1.0 - ax) * ay), (1, 1, ax * ay)] { + if w <= 0.0 { + continue; + } + let i = *slot.entry((x0 + dx, y0 + dy)).or_insert_with(|| { + taps.push((x0 + dx, y0 + dy, 0.0)); + taps.len() - 1 + }); + taps[i].2 += w * k; } - }) + } + let reach = taps.iter().map(|t| t.0.abs().max(t.1.abs())).max().unwrap_or(0); + let n = src.ch; + let win = out.inflate(reach); + let ww = win.width() as usize; + // Premultiplied window; outside the source reads as transparent, as the sampler does. + let mut pw = vec![0.0f32; ww * win.height() as usize * n]; + for y in win.y0..win.y1 { + for x in win.x0..win.x1 { + let i = ((y - win.y0) as usize * ww + (x - win.x0) as usize) * n; + let a = if ctx.alpha { src.get(x, y, n - 1) } else { 1.0 }; + for c in 0..n { + let v = src.get(x, y, c); + pw[i + c] = if ctx.alpha && c < n - 1 { v * a } else { v }; + } + } + } + let (ow, oh) = (out.width() as usize, out.height() as usize); + let mut res = vec![0.0f32; ow * oh * n]; + let r = reach as usize; + for (oy, row) in res.chunks_exact_mut(ow * n).enumerate() { + for &(dx, dy, w) in &taps { + let start = ((oy + r).wrapping_add_signed(dy as isize) * ww + r.wrapping_add_signed(dx as isize)) * n; + for (o, v) in row.iter_mut().zip(&pw[start..start + ow * n]) { + *o += w * v; + } + } + if ctx.alpha { + for px in row.chunks_exact_mut(n) { + let a = px[n - 1]; + for v in px.iter_mut().take(n - 1) { + *v = if a > 1e-7 { *v / a } else { 0.0 }; + } + } + } + } + res } pub(crate) fn radial(src: &Image, out: Rect, ctx: &Ctx, amount: f32, method: RadialMethod, center: (f32, f32)) -> Vec { @@ -702,4 +766,34 @@ mod tests { img.data[i] = 0.5 / 255.0; assert!(surface_8bit(&img, out, &ctx, 4, 0.1, 26).is_none()); } + + #[test] + fn motion_kernel_matches_the_per_pixel_samples() { + // Noisy RGBA with some transparent pixels; the output reaches the source edge. + let src_rect = Rect::new(-30, -30, 50, 40); + let mut img = Image::new(src_rect, 4); + let mut s = 0x1b87_3593u32; + for px in img.data.as_chunks_mut::<4>().0 { + for v in px.iter_mut() { + s ^= s << 13; + s ^= s >> 17; + s ^= s << 5; + *v = (s % 1000) as f32 / 999.0; + } + if s.is_multiple_of(5) { + px[3] = 0.0; + } + } + let out = Rect::new(-12, -6, 30, 22); + for alpha in [true, false] { + let ctx = Ctx { bounds: src_rect, mode: crate::ColorMode::Rgb, alpha }; + for (angle, distance) in [(0.0, 1.0), (0.0, 9.0), (30.0, 7.0), (90.0, 12.0), (-45.0, 20.0), (137.0, 33.5), (200.0, 40.0)] { + let fast = motion(&img, out, &ctx, angle, distance); + let offs = motion_offsets(angle, distance); + let slow = average_samples(&img, out, &ctx, |x, y, pts| pts.extend(offs.iter().map(|&(dx, dy)| (x + dx, y + dy)))); + let err = slow.iter().zip(&fast).map(|(a, b)| (a - b).abs()).fold(0.0f32, f32::max); + assert!(err < 1e-5, "alpha {alpha} angle {angle} distance {distance}: max error {err}"); + } + } + } } diff --git a/crates/engine/src/variables_cmds.rs b/crates/engine/src/variables_cmds.rs index 65e3ffee00..c6e036b0f4 100644 --- a/crates/engine/src/variables_cmds.rs +++ b/crates/engine/src/variables_cmds.rs @@ -164,13 +164,16 @@ fn apply_to_doc(doc: &mut Document, vars: &Variables, set: &DataSet) -> Result<( } } (VarKind::TextReplacement, VarValue::Text(text)) => { + let snapshot = doc.clone(); if let Some(l) = doc.layer_mut(def.layer) && let LayerContent::Text(t) = &mut l.content { t.text = text.clone(); - t.cache = None; // force re-render t.runs.clear(); // re-flow as one run from the summary style t.paragraphs.clear(); + // The compositor draws a type layer from its cache: clearing it left the + // layer blank until some other edit re-rendered it (#990). + crate::type_cmds::refresh(&snapshot, t); } } (VarKind::PixelReplacement { method, align, clip }, VarValue::Pixels(path)) => { diff --git a/crates/engine/src/variables_cmds/tests.rs b/crates/engine/src/variables_cmds/tests.rs index 55ff20540d..69301add02 100644 --- a/crates/engine/src/variables_cmds/tests.rs +++ b/crates/engine/src/variables_cmds/tests.rs @@ -190,3 +190,25 @@ fn pixel_replacement_changes_the_layer() { assert!(b > r, "replacement is blue-ish: {rgba:?}"); } } + +#[test] +fn applied_text_is_rendered_in_the_document_and_in_exports() { + // Applying a text value cleared the type layer's render cache and nothing rebuilt it, so the + // text vanished from the canvas and from every exported file (#990). + let mut s = Session::new(); + s.execute("file.new", json!({"width": 200, "height": 60, "background": "white"})).unwrap(); + let id = s.execute("type.create", json!({"x": 4, "y": 40, "text": "HELLO", "size": 32, "color": "#000000"})).unwrap()["layer"].clone(); + let dark = |d: &Document| photocraft_compose::render(d, d.bounds()).px.iter().filter(|p| p[0] < 0.5).count(); + assert!(dark(doc(&s)) > 0); + s.execute("image.variables.define", json!({"defs": [{"name": "headline", "layer": id, "type": "textReplacement"}]})).unwrap(); + s.execute("image.variables.dataSets", json!({"dataSets": [{"name": "row1", "values": [{"variable": "headline", "kind": "text", "value": "BYE"}]}]})) + .unwrap(); + let dir = tmp("text-export"); + let out = s.execute("file.export.dataSetsAsFiles", json!({"dir": dir, "format": "png"})).unwrap(); + let path = out["files"][0].as_str().unwrap(); + let exported = photocraft_io::import(path, &std::fs::read(path).unwrap()).unwrap().document; + assert!(dark(&exported) > 0, "the exported row keeps its text"); + s.execute("image.applyDataSet", json!({"name": "row1"})).unwrap(); + assert!(dark(doc(&s)) > 0, "the applied text is drawn"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/io/src/psd_export.rs b/crates/io/src/psd_export.rs index d9f9cc09e8..f8f0337339 100644 --- a/crates/io/src/psd_export.rs +++ b/crates/io/src/psd_export.rs @@ -244,7 +244,11 @@ impl Ex { fn encode(&self, id: i16, plane: &[u8], w: usize, h: usize) -> ChannelData { let depth = psd_depth(self.fmt.sample); - ChannelData::encode(id, self.compression(), plane, w, h, depth, self.version) + // Photoshop writes empty layer/mask channels as just the raw compression code. + // A ZIP stream for a zero-sized channel makes readers try to decompress an + // invalid image extent (notably psd-tools for 32-bit adjustment layers). + let compression = if w == 0 || h == 0 { Compression::Raw } else { self.compression() }; + ChannelData::encode(id, compression, plane, w, h, depth, self.version) .or_else(|_| ChannelData::encode(id, Compression::Raw, plane, w, h, depth, self.version)) .unwrap_or(ChannelData { id, compression: Some(Compression::Raw), data: plane.to_vec() }) } diff --git a/crates/io/tests/psd_empty_channels.rs b/crates/io/tests/psd_empty_channels.rs new file mode 100644 index 0000000000..285f03cfcd --- /dev/null +++ b/crates/io/tests/psd_empty_channels.rs @@ -0,0 +1,44 @@ +mod common; +use common::Features; +use photocraft_color::{ColorMode, SampleType}; +use photocraft_io::{ExportOptions, export}; +use photocraft_psd::{Compression, PsdFile}; + +#[test] +fn empty_layer_and_mask_channels_use_raw_without_a_payload() { + for mode in [ColorMode::Rgb, ColorMode::Grayscale] { + for depth in [SampleType::U8, SampleType::U16, SampleType::F32] { + for ext in ["out.psd", "out.psb"] { + let mut doc = common::gen_doc(mode, depth, Features::ALL); + let mut masked = common::raster("Uniform mask", doc.pixel_format(), photocraft_geom::Rect::new(0, 0, 3, 3), 1, true); + masked.mask = Some(photocraft_doc::LayerMask { + surface: photocraft_raster::Surface::new(photocraft_color::PixelFormat::new(ColorMode::Grayscale, depth, false)), + enabled: true, + linked: true, + density: 1.0, + feather: 0.0, + }); + doc.layers.push(masked); + let bytes = export(&doc, ext, &ExportOptions::default()).unwrap().bytes; + let file = PsdFile::from_bytes(&bytes).unwrap(); + let mut empty = 0; + let mut empty_masks = 0; + for layer in file.layers() { + for channel in &layer.channels { + let rect = layer.channel_rect(channel.id); + if rect.width() == 0 || rect.height() == 0 { + empty += 1; + if channel.id == -2 { + empty_masks += 1; + } + assert_eq!(channel.compression, Some(Compression::Raw), "{mode:?} {depth:?} {ext}: {} channel {}", layer.name(), channel.id); + assert!(channel.data.is_empty()); + } + } + } + assert!(empty > 0); + assert!(empty_masks > 0); + } + } + } +} diff --git a/crates/psd/src/layer.rs b/crates/psd/src/layer.rs index a0c6bd5ffb..2ecf0df6f3 100644 --- a/crates/psd/src/layer.rs +++ b/crates/psd/src/layer.rs @@ -214,11 +214,31 @@ impl LayerMask { /// Whether `d` (a whole mask record with mask parameters) stores the real-mask fields before /// the parameters, as Photoshop does. The spec's order wins when it reads as parameters - /// followed by padding alone (no real mask), the one layout the two orders can share. + /// followed by padding alone (no real mask), the one layout the two orders can share. When + /// both orders fit the record's length, Photoshop's is taken unless its reading is + /// implausible (a real-mask background that is neither 0 nor 255, or parameter flag bits + /// the spec does not define) while the spec's reading with a real mask is sound. pub(crate) fn real_before_parameters(d: &[u8]) -> bool { + // Parameters at `pf_at`, then 0..=3 bytes of padding, and nothing else. let fits = |pf_at: usize| d.get(pf_at).is_some_and(|&pf| (pf_at + 1 + mask_parameter_bytes(pf)..=pf_at + 4 + mask_parameter_bytes(pf)).contains(&d.len())); - !fits(18) && fits(36) + if fits(18) { + return false; + } + let photoshop = fits(36); + if !photoshop { + return false; + } + let known_flags = |pf: u8| pf & 0xF0 == 0; + let real_background_ok = |at: usize| d.get(at + 1).is_some_and(|&bg| bg == 0 || bg == 255); + // Photoshop's reading: real-mask fields at 18, parameters at 36. + let photoshop_plausible = real_background_ok(18) && d.get(36).is_some_and(|&pf| known_flags(pf)); + // The spec's reading: parameters at 18, then the 18 real-mask bytes, then padding. + let spec_plausible = d.get(18).is_some_and(|&pf| { + let real_at = 19 + mask_parameter_bytes(pf); + known_flags(pf) && (real_at + 18..=real_at + 21).contains(&d.len()) && real_background_ok(real_at) + }); + photoshop_plausible || !spec_plausible } fn parse(d: &[u8]) -> Result { @@ -845,6 +865,45 @@ mod tests { assert_eq!(out, d); } + /// A density-only record in the spec's order (#949): the parameter flags byte at 18 says one + /// value byte, so the spec's reading excludes the 38-byte length only if the real mask is + /// ignored, while byte 36 (inside the real rectangle's `right`) is 0 and makes Photoshop's + /// reading fit too. Photoshop's reading puts the density (64) where a 0-or-255 background + /// belongs, so the spec's order wins and the density survives. + #[test] + fn density_only_spec_layout_is_not_read_as_photoshop_order() { + for (density, right) in [(64u8, 1u32), (200, 1), (64, 300), (1, 7)] { + let mut d = Vec::new(); + Rect::from_xywh(0, 0, 1, 1).write(&mut d); + d.extend([255, LayerMask::FLAG_PARAMETERS]); + d.extend([0b0001, density]); + d.extend([0, 0]); + Rect::from_xywh(0, 0, right, 1).write(&mut d); + let m = LayerMask::parse(&d).unwrap(); + assert!(!m.real_first, "density {density}, right {right}: {m:?}"); + assert_eq!(m.parameters.unwrap().user_density, Some(density)); + assert_eq!(m.real, Some(RealMask { flags: 0, background: 0, rect: Rect::from_xywh(0, 0, right, 1) })); + assert!(m.trailing.is_empty()); + let mut out = Vec::new(); + m.write(&mut out); + assert_eq!(out, d); + } + // Photoshop's order with the same lengths still reads as Photoshop's order. + let mut d = Vec::new(); + Rect::from_xywh(0, 0, 1, 1).write(&mut d); + d.extend([255, LayerMask::FLAG_PARAMETERS]); + d.extend([0, 255]); + Rect::from_xywh(0, 0, 1, 1).write(&mut d); + d.extend([0b0001, 64]); + let m = LayerMask::parse(&d).unwrap(); + assert!(m.real_first, "{m:?}"); + assert_eq!(m.parameters.unwrap().user_density, Some(64)); + assert_eq!(m.real.map(|r| r.background), Some(255)); + let mut out = Vec::new(); + m.write(&mut out); + assert_eq!(out, d); + } + /// The spec's order (parameters, then real fields) still parses and stays byte-stable. #[test] fn real_mask_after_parameters_spec_layout() { diff --git a/crates/psd/tests/roundtrip.rs b/crates/psd/tests/roundtrip.rs index 8666ba3cc4..1f23ee6284 100644 --- a/crates/psd/tests/roundtrip.rs +++ b/crates/psd/tests/roundtrip.rs @@ -285,3 +285,30 @@ fn save_and_open() { assert_eq!(g, f); let _ = std::fs::remove_dir_all(&dir); } + +/// A density-only mask record written in the spec's order (parameters, then the real-mask +/// fields) used to be read back in Photoshop's order, which moved the density into the real +/// mask's background and dropped it from the parameters (#949). The model must survive the +/// round trip, not only the bytes. +#[test] +fn spec_order_density_only_mask_round_trips_as_a_model() { + let mut f = testgen::small(Version::Psd, Compression::Raw); + let layer = &mut f.layers_mut()[0]; + layer.mask = MaskData::Mask(LayerMask { + rect: Rect::from_xywh(0, 0, 1, 1), + default_color: 255, + flags: LayerMask::FLAG_PARAMETERS, + parameters: Some(MaskParameters { flags: 1, user_density: Some(64), ..Default::default() }), + real: Some(RealMask { flags: 0, background: 0, rect: Rect::from_xywh(0, 0, 1, 1) }), + trailing: vec![], + real_first: false, + }); + // The generated 2 x 2 mask channel is replaced by one that matches the 1 x 1 rectangle. + let mask_channel = layer.channels.iter_mut().find(|c| c.id == -2).expect("the generated layer has a mask channel"); + *mask_channel = ChannelData::encode(-2, Compression::Raw, &[64], 1, 1, 8, Version::Psd).unwrap(); + let b = f.to_bytes().unwrap(); + let g = PsdFile::from_bytes(&b).unwrap(); + assert_eq!(g, f, "model round trip"); + assert_eq!(g.layers()[0].decode_channel(-2, 8, Version::Psd).unwrap(), vec![64]); + assert_eq!(g.to_bytes().unwrap(), b, "byte stability"); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 7e93f02b26..259db97037 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1817,12 +1817,14 @@ pub fn canvas_view(app: &mut PhotocraftApp, ui: &mut egui::Ui, idx: usize, rect: { crate::canvas_tool_menu::open_transform(app, [p.x, p.y]); } + let lasso_retracted = tool == Tool::Lasso && response.secondary_clicked() && crate::lasso_ui::undo_last_vertex(app); if tool == Tool::Lasso { crate::lasso_ui::canvas_input(app, &ctx, &xf, &response); (buttons.started, buttons.dragged, buttons.stopped, buttons.clicked) = (false, false, false, false); } // Right-click with the Move tool, or ⌘/Ctrl+right-click: the layers under the pointer. if response.secondary_clicked() + && !lasso_retracted && !transforming && crate::layer_pick_ui::is_gesture(tool, mods) && let Some(p) = response.interact_pointer_pos() @@ -1832,6 +1834,7 @@ pub fn canvas_view(app: &mut PhotocraftApp, ui: &mut egui::Ui, idx: usize, rect: crate::layer_pick_ui::open(app, [p.x, p.y], d[0], d[1]); } if response.secondary_clicked() + && !lasso_retracted && !transforming && !crate::layer_pick_ui::is_gesture(tool, mods) && let Some(p) = response.interact_pointer_pos() diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index cf99a88276..96e8693bf6 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -397,8 +397,10 @@ fn dispatch(app: &mut PhotocraftApp, ctx: &egui::Context, req: &ControlRequest) None => err("no such dialog"), }, "ui.window.open" => { - if let Some(d) = u("document") { - app.session.set_active(d as usize); + if let Some(d) = u("document") + && !app.session.set_active(d as usize) + { + return err(format!("no document {d}")); } wrap(crate::menus::invoke(app, ctx, "window.newWindowForDocument", json!({}))) } @@ -805,6 +807,22 @@ mod tests { } } + #[test] + fn ui_window_open_rejects_an_unknown_document_index() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + let ctx = egui::Context::default(); + app.run("file.new", json!({"width": 8, "height": 8})).unwrap(); + // An out-of-range index must error instead of silently switching the active document + // and opening the window for whatever is active now. + let before = app.session.active_index(); + let r = call(&mut app, &ctx, "ui.window.open", json!({"document": 99})); + assert_eq!(r["ok"], false, "{r}"); + assert!(r["error"].as_str().unwrap().contains("no document 99"), "{r}"); + assert_eq!(app.session.active_index(), before, "the active document is untouched"); + // A valid index still opens the window. + assert_eq!(call(&mut app, &ctx, "ui.window.open", json!({"document": 0}))["ok"], true); + } + #[test] fn ui_set_brush_size_dispatches_a_journaled_brush_command() { let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 5e2dfa2898..88727983cf 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -324,7 +324,11 @@ pub fn confirm(app: &mut PhotocraftApp, id: u64) -> Result { app.filter_preview = None; let cmd = d.fields.get("__command").and_then(|v| v.as_str().map(str::to_string)).ok_or("dialog has no command")?; let (cmd, params) = crate::smart_ui::confirm_command(&d.fields, cmd, crate::filter_dialog::params_of(&d.fields)); - app.run(&cmd, params) + let result = app.run(&cmd, params.clone()); + if result.is_ok() && cmd == "view.newGuideLayout" { + app.ui.view.guide_layout = params; + } + result } DialogKind::LayerStyle => crate::layer_style::confirm(app, &d.fields), DialogKind::About | DialogKind::Error => Ok(Value::Null), diff --git a/crates/ui-egui/src/lasso_ui.rs b/crates/ui-egui/src/lasso_ui.rs index 8897148184..2328f748bd 100644 --- a/crates/ui-egui/src/lasso_ui.rs +++ b/crates/ui-egui/src/lasso_ui.rs @@ -20,6 +20,29 @@ pub fn active(app: &PhotocraftApp) -> bool { app.drag.as_ref().is_some_and(|d| d.tool == Tool::Lasso && d.lasso.is_some()) } +/// Between polygonal clicks, Backspace/right-click retracts the last fixed vertex. +/// Do not affect a freehand stroke whose primary button is still held. +pub fn waiting_for_vertex(app: &PhotocraftApp) -> bool { + app.ui.tool == Tool::Lasso + && app + .drag + .as_ref() + .is_some_and(|d| d.tool == Tool::Lasso && d.lasso.as_ref().is_some_and(|l| !l.down && l.document == app.session.active().map(|st| st.doc.id))) +} + +pub fn undo_last_vertex(app: &mut PhotocraftApp) -> bool { + cancel_stale(app); + if !waiting_for_vertex(app) { + return false; + } + if app.drag.as_ref().is_some_and(|d| d.points.len() <= 1) { + app.drag = None; + } else if let Some(d) = app.drag.as_mut() { + d.points.pop(); + } + true +} + /// A lasso drag that moves the selection (started inside it, `canvas::selection_drag_kind`): /// the canvas's own selection-move path handles it, this module only feeds it the events. fn moving_selection(app: &PhotocraftApp) -> bool { diff --git a/crates/ui-egui/src/lasso_ui/tests.rs b/crates/ui-egui/src/lasso_ui/tests.rs index 7639170ed8..97480f6b3f 100644 --- a/crates/ui-egui/src/lasso_ui/tests.rs +++ b/crates/ui-egui/src/lasso_ui/tests.rs @@ -199,3 +199,54 @@ fn a_drag_inside_the_selection_moves_it() { assert!(h.state().drag.is_none()); assert_eq!(bounds(h.state()), Some(photocraft_geom::Rect::new(110, 105, 160, 145))); } + +#[test] +fn retracting_polygonal_vertices_keeps_the_document_unchanged() { + let mut app = app(); + begin(&mut app, Modifiers::NONE); + event(&mut app, "up", 120.0, 50.0, Modifiers::ALT); + assert!(waiting_for_vertex(&app)); + let layer = app.session.active().unwrap().active_layer.unwrap(); + assert_eq!(app.drag.as_ref().unwrap().points.len(), 3); + assert!(undo_last_vertex(&mut app)); + assert_eq!(app.drag.as_ref().unwrap().points.len(), 2); + assert!(undo_last_vertex(&mut app)); + assert_eq!(app.drag.as_ref().unwrap().points.len(), 1); + assert!(undo_last_vertex(&mut app)); + assert!(!active(&app)); + assert!(!undo_last_vertex(&mut app)); + let st = app.session.active().unwrap(); + assert!(st.doc.layer(layer).is_some()); + assert!(st.doc.selection.is_none()); +} + +#[test] +fn real_canvas_backspace_and_right_click_retract_polygonal_points() { + let mut h = harness(); + mouse(&mut h, "down", 50.0, 50.0, Modifiers::NONE); + mouse(&mut h, "move", 120.0, 50.0, Modifiers::NONE); + mouse(&mut h, "up", 120.0, 50.0, Modifiers::ALT); + mouse(&mut h, "down", 180.0, 100.0, Modifiers::ALT); + mouse(&mut h, "up", 180.0, 100.0, Modifiers::ALT); + assert_eq!(h.state().drag.as_ref().unwrap().points.len(), 3); + let layer = h.state().session.active().unwrap().active_layer.unwrap(); + + h.event(Event::Key { key: egui::Key::Backspace, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::ALT }); + h.run_steps(2); + assert_eq!(h.state().drag.as_ref().unwrap().points.len(), 2); + assert!(h.state().session.active().unwrap().doc.layer(layer).is_some()); + + let state = h.state(); + let view = &state.ui.views[0]; + let xf = ViewXform { rect: crate::rulers::content_rect(state, state.last_canvas_rect), zoom: view.zoom, center: view.center, flip: false }; + let pos = xf.to_screen(155.0, 100.0); + h.event(Event::PointerMoved(pos)); + h.event(Event::PointerButton { pos, button: PointerButton::Secondary, pressed: true, modifiers: Modifiers::ALT }); + h.run_steps(1); + h.event(Event::PointerButton { pos, button: PointerButton::Secondary, pressed: false, modifiers: Modifiers::ALT }); + h.run_steps(2); + assert_eq!(h.state().drag.as_ref().unwrap().points.len(), 1); + assert!(h.state().ui.canvas_tool_menu.is_none()); + assert!(h.state().session.active().unwrap().doc.layer(layer).is_some()); + assert!(h.state().session.active().unwrap().doc.selection.is_none()); +} diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 078c9f3b7f..b413c44ea8 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -296,6 +296,15 @@ pub fn handle(app: &mut PhotocraftApp, ctx: &egui::Context) { if crate::magnetic_lasso_ui::keys(app, ctx) { return; } + // While placing polygonal lasso vertices, Backspace/Delete retracts the last point; + // it must win over the regular Edit › Clear shortcut, which edits document pixels. + if crate::lasso_ui::waiting_for_vertex(app) { + let mods = ctx.input(|i| i.modifiers); + if !mods.command && !mods.ctrl && !mods.shift && ctx.input_mut(|i| i.consume_key(mods, Key::Backspace) || i.consume_key(mods, Key::Delete)) { + crate::lasso_ui::undo_last_vertex(app); + return; + } + } // Inline type editing eats text and navigation keys; ⌘-shortcuts still reach the menus. let editing = crate::type_tool::handle_keys(app, ctx); // Registry, UI and menu-catalogue shortcuts (see [`crate::shortcut_dispatch::bindings`]). diff --git a/crates/ui-egui/src/view_cmds.rs b/crates/ui-egui/src/view_cmds.rs index 1184ea0910..1072b3eccf 100644 --- a/crates/ui-egui/src/view_cmds.rs +++ b/crates/ui-egui/src/view_cmds.rs @@ -94,6 +94,12 @@ pub struct ViewOptions { /// and the Middle Eastern & South Asian composer. pub language_features: String, pub middle_eastern_composer: bool, + /// Parameters from the last successfully applied New Guide Layout dialog. + pub guide_layout: Value, +} + +fn default_guide_layout() -> Value { + json!({"columns": 8, "gutter": 20, "rows": 0, "rowGutter": 0, "margin": 0, "centerColumns": false, "clearExisting": false}) } impl Default for ViewOptions { @@ -112,6 +118,7 @@ impl Default for ViewOptions { font_preview_size: "medium".into(), language_features: "defaultFeatures".into(), middle_eastern_composer: false, + guide_layout: default_guide_layout(), } } } @@ -846,7 +853,8 @@ fn front(app: &mut PhotocraftApp, id: &str, params: &Value) -> Option { - dialog(app, json!({"columns": 8, "gutter": 20, "rows": 0, "rowGutter": 0, "margin": 0, "centerColumns": false, "clearExisting": false}), json!({})) + let fields = app.ui.view.guide_layout.clone(); + dialog(app, fields, json!({})) } "type.warpText" => { let styles: Vec<&str> = std::iter::once("none").chain(photocraft_text::warp::STYLES.iter().map(|(_, s)| *s)).collect(); diff --git a/crates/ui-egui/src/view_cmds/tests.rs b/crates/ui-egui/src/view_cmds/tests.rs index eaabdddd66..f5519b806c 100644 --- a/crates/ui-egui/src/view_cmds/tests.rs +++ b/crates/ui-egui/src/view_cmds/tests.rs @@ -172,3 +172,43 @@ fn engine_commands_open_their_dialogs() { crate::menus::invoke_unguarded(&mut app, &ctx, "file.closeAll", json!({})).unwrap(); assert!(app.ui.views.is_empty() && app.session.documents().is_empty()); } + +#[test] +fn new_guide_layout_remembers_successful_values_not_cancelled_edits() { + let (mut app, ctx) = app_with(1); + let first = menu(&mut app, &ctx, "view.newGuideLayout", json!({})).unwrap()["dialog"].as_u64().unwrap(); + assert_eq!(app.ui.dialog_mut(first).unwrap().fields["columns"], 8); + assert_eq!(app.ui.dialog_mut(first).unwrap().fields["gutter"], 20); + for (key, value) in [("columns", json!(2)), ("gutter", json!(0)), ("rows", json!(2)), ("rowGutter", json!(0))] { + app.ui.dialog_mut(first).unwrap().fields.insert(key.into(), value); + } + crate::dialogs::confirm(&mut app, first).unwrap(); + + let second = menu(&mut app, &ctx, "view.newGuideLayout", json!({})).unwrap()["dialog"].as_u64().unwrap(); + let fields = &app.ui.dialog_mut(second).unwrap().fields; + assert_eq!(fields["columns"], 2); + assert_eq!(fields["gutter"], 0); + assert_eq!(fields["rows"], 2); + assert_eq!(fields["rowGutter"], 0); + assert_eq!(fields["clearExisting"], false); + // A cancelled edit must not replace the last successfully used values. + app.ui.dialog_mut(second).unwrap().fields.insert("columns".into(), json!(9)); + app.ui.close_dialog(second); + let third = menu(&mut app, &ctx, "view.newGuideLayout", json!({})).unwrap()["dialog"].as_u64().unwrap(); + assert_eq!(app.ui.dialog_mut(third).unwrap().fields["columns"], 2); + + // The remembered state is part of the serializable UI state. + let json_state = serde_json::to_value(&app.ui).unwrap(); + assert_eq!(json_state["view"]["guide_layout"]["columns"], 2); + let restored: crate::state::UiState = serde_json::from_value(json_state).unwrap(); + assert_eq!(restored.view.guide_layout["rows"], 2); +} + +#[test] +fn new_guide_layout_does_not_remember_a_failed_command() { + let (mut app, _) = app_with(0); + let id = front(&mut app, "view.newGuideLayout", &json!({})).unwrap().unwrap()["dialog"].as_u64().unwrap(); + app.ui.dialog_mut(id).unwrap().fields.insert("columns".into(), json!(3)); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert_eq!(app.ui.view.guide_layout["columns"], 8); +}