From bf80e2d2fa6a565f9d5d4bc53271fb9597adaa39 Mon Sep 17 00:00:00 2001 From: David Mann Date: Fri, 30 Oct 2026 14:58:18 +0100 Subject: [PATCH 01/12] Select and Mask: Shift Edge is a curve on coverage, so hard edges stay put MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5176 made ±100% move the edge by max(radius, feather, 2σ smooth), at least 1 px. Measured in the reference application (a 100 px square, Radius/Feather/Smooth each varied, Shift Edge -100/-50/+100): - A hard selection does not move at all, at any Shift Edge. - On a feathered edge the result is a fixed per-pixel curve, the same at 2, 5 and 10 px feather: the pixel's odds a/(1-a) are multiplied by 49^(shift/100). At -100% a 50% pixel becomes 2%, at -50% 1/8, at +100% 98%. The fit holds within ±0.05 in log-odds on every sample. So Shift Edge is now that curve, applied last as before. It needs no neighbours, so the morphology callback and the halo/spread it added are gone, with the ignored benchmark that timed it (matting::morph itself stays for Select › Modify and the mask view). Tests: a hard edge is unchanged at ±50/±100%; seven measured before→after coverage pairs; a feathered edge is pulled in; the engine refine tests now check the hard edge stays and a feathered one moves. Co-Authored-By: Claude Opus 5.5 --- crates/algo/src/matting.rs | 130 +++++++----------- crates/algo/src/matting/morphology/tests.rs | 59 -------- .../engine/src/cmd/smartselect_cmds/tests.rs | 17 ++- docs/octagonal-mask-performance.md | 8 +- 4 files changed, 72 insertions(+), 142 deletions(-) diff --git a/crates/algo/src/matting.rs b/crates/algo/src/matting.rs index 0901dab0c0..6cc03e2094 100644 --- a/crates/algo/src/matting.rs +++ b/crates/algo/src/matting.rs @@ -35,10 +35,11 @@ pub struct RefineParams { pub feather: f32, /// 0–100. pub contrast: f32, - /// −100–100. 100% moves the edge by its soft width: `max(radius, feather, 2 × smooth σ)`, - /// at least 1 px, so a hard edge moves a pixel (Photoshop's Shift Edge moves soft borders; - /// a band of several pixels around hard line art fills it in solid). Values between whole - /// pixels blend the two neighbouring shifts, so the slider does not jump. + /// −100–100. A curve on the refined coverage, applied last: each pixel's odds (`a / (1 − a)`) + /// are multiplied by `49^(shift / 100)`, so at −100% a half-covered pixel drops to 2% and at + /// +100% it rises to 98%. Fully selected and unselected pixels never change, so Shift Edge + /// moves only soft borders and a hard edge stays put (measured against the reference + /// application: the fit holds within ±0.05 in log-odds at feathers of 2, 5 and 10 px). pub shift_edge: f32, } @@ -62,20 +63,10 @@ impl RefineParams { fn feather_sigma(&self) -> f32 { self.feather.max(0.0) / 2.0 } - /// Signed shift in pixels, fractional. 100% is the edge's soft width (at least 1 px), - /// clamped to ±64. - fn shift_amount(&self) -> f32 { - let soft = self.radius.max(self.feather).max(2.0 * self.smooth_sigma()).max(1.0); - let px = self.shift_edge.clamp(-100.0, 100.0) / 100.0 * soft; - if px.is_finite() { px.clamp(-64.0, 64.0) } else { 0.0 } - } - /// How many pixels the mask can move: the farther of the two integer radii a fractional - /// shift blends. Whole-pixel amounts stay exact (`ceil(4) == 4`). - fn shift_px(&self) -> i32 { - let px = self.shift_amount(); - let reach = px.abs().ceil(); - let signed = if px.is_sign_negative() { -reach } else { reach }; - if signed.is_finite() { signed as i32 } else { 0 } + /// The factor Shift Edge multiplies each pixel's odds by: `49^(shift / 100)`, 1 at 0%. + fn shift_odds(&self) -> f32 { + let s = self.shift_edge.clamp(-100.0, 100.0) / 100.0; + if s.is_finite() { 49f32.powf(s) } else { 1.0 } } fn uses_image(&self) -> bool { self.radius > 0.0 @@ -88,14 +79,13 @@ impl RefineParams { } else { 0 }; - edge + (3.0 * self.smooth_sigma()).ceil() as i32 + (3.0 * self.feather_sigma()).ceil() as i32 + self.shift_px().abs() + 2 + edge + (3.0 * self.smooth_sigma()).ceil() as i32 + (3.0 * self.feather_sigma()).ceil() as i32 + 2 } /// How far the refined mask can extend beyond the original. pub fn spread(&self) -> i32 { (if self.uses_image() { self.radius.ceil() as i32 } else { 0 }) + (3.0 * self.smooth_sigma()).ceil() as i32 + (3.0 * self.feather_sigma()).ceil() as i32 - + self.shift_px().max(0) + 1 } } @@ -311,7 +301,7 @@ pub fn edge_width(img: &RgbImage, r: usize) -> Vec { /// Refines a soft mask buffer `m` (`w × h`) with `img` as guide (required when `radius > 0`). pub fn refine_buffer(img: Option<&RgbImage>, m: &[f32], w: usize, h: usize, p: &RefineParams) -> Vec { - refine_buffer_with(img, m, w, h, p, None, morph) + refine_buffer_with(img, m, w, h, p, None) } /// The Refine Edge Brush over one buffer: signed coverage (`w × h`, see [`EdgeBrush`]) and the @@ -322,10 +312,7 @@ struct BrushBuf<'a> { window: usize, } -fn refine_buffer_with(img: Option<&RgbImage>, m: &[f32], w: usize, h: usize, p: &RefineParams, brush: Option, morphology: F) -> Vec -where - F: Fn(&[f32], usize, usize, usize, bool) -> Vec + Copy, -{ +fn refine_buffer_with(img: Option<&RgbImage>, m: &[f32], w: usize, h: usize, p: &RefineParams, brush: Option) -> Vec { let mut a = m.to_vec(); if let (true, Some(img)) = (p.uses_image(), img) { let bin: Vec = m.iter().map(|v| *v >= 0.5).collect(); @@ -385,36 +372,23 @@ where *v = ((*v - 0.5) * gain + 0.5).clamp(0.0, 1.0); } } - let amount = p.shift_amount(); - if amount.abs() >= 1.0e-4 { - a = shift_mask(&a, w, h, amount, morphology); + let k = p.shift_odds(); + if (k - 1.0).abs() >= 1.0e-6 { + for v in &mut a { + *v = shift_coverage(*v, k); + } } a } -/// Grow or shrink `src` by a fractional pixel distance. Whole pixels use one morphology; -/// the remainder blends that result with the next pixel, so 0.3 px is not the same step as 1 px. -fn shift_mask(src: &[f32], w: usize, h: usize, amount: f32, morphology: F) -> Vec -where - F: Fn(&[f32], usize, usize, usize, bool) -> Vec + Copy, -{ - let mag = amount.abs(); - if !mag.is_finite() || mag < 1.0e-4 { - return src.to_vec(); +/// Shift Edge on one coverage value: its odds times `k`. 0 and 1 stay exact. +fn shift_coverage(v: f32, k: f32) -> f32 { + let v = v.clamp(0.0, 1.0); + if v <= 0.0 || v >= 1.0 { + return v; } - let grow = amount > 0.0; - let whole = mag.floor() as usize; - let frac = mag - whole as f32; - if frac < 1.0e-4 { - return morphology(src, w, h, whole, grow); - } - let lo = if whole == 0 { src.to_vec() } else { morphology(src, w, h, whole, grow) }; - let hi = morphology(src, w, h, whole + 1, grow); - let n = w.saturating_mul(h); - if lo.len() < n || hi.len() < n || lo.len() != hi.len() { - return hi; - } - lo.into_iter().zip(hi).map(|(a, b)| a + (b - a) * frac).collect() + let shifted = k * v / (k * v + (1.0 - v)); + if shifted.is_finite() { shifted.clamp(0.0, 1.0) } else { v } } /// Most pixels one refine should cover. Its result is a dense [`Region`] over [`refine_area`] @@ -437,7 +411,7 @@ pub fn refine_area(content: Rect, canvas: Rect, p: &RefineParams, brush: Option< /// its non-zero pixels; the result is clipped to `canvas`. `sampler` supplies the guide image /// (only read when `radius > 0`). Returns `None` when nothing remains selected. pub fn refine_mask(sampler: &dyn Sampler, mask: &(dyn Fn(Rect) -> Vec + Sync), content: Rect, canvas: Rect, p: &RefineParams) -> Option { - refine_mask_with(sampler, mask, content, canvas, p, None, morph) + refine_mask_with(sampler, mask, content, canvas, p, None) } /// [`refine_mask`] with the Refine Edge Brush: where `brush` adds, coverage comes from the image @@ -451,21 +425,17 @@ pub fn refine_mask_brushed( p: &RefineParams, brush: Option<&EdgeBrush>, ) -> Option { - refine_mask_with(sampler, mask, content, canvas, p, brush, morph) + refine_mask_with(sampler, mask, content, canvas, p, brush) } -fn refine_mask_with( +fn refine_mask_with( sampler: &dyn Sampler, mask: &(dyn Fn(Rect) -> Vec + Sync), content: Rect, canvas: Rect, p: &RefineParams, brush: Option<&EdgeBrush>, - morphology: F, -) -> Option -where - F: Fn(&[f32], usize, usize, usize, bool) -> Vec + Copy + Sync + Send, -{ +) -> Option { let bbox = refine_area(content, canvas, p, brush); if bbox.is_empty() { return None; @@ -505,7 +475,7 @@ where (Some(v), Some(b)) => Some(BrushBuf { v, window: b.window() }), _ => None, }; - let out = refine_buffer_with(img.as_ref(), &m, rw, rh, p, bb, morphology); + let out = refine_buffer_with(img.as_ref(), &m, rw, rh, p, bb); (*t, core(&out)) }; #[cfg(not(target_arch = "wasm32"))] @@ -769,24 +739,30 @@ mod tests { assert!(f[5 * w + 29] > 0.5 && f[5 * w + 29] < 0.8 && f[5 * w + 31] > 0.1); let c = refine_buffer(None, &f, w, h, &RefineParams { contrast: 100.0, ..Default::default() }); assert!(c[5 * w + 29] == 1.0 && c[5 * w + 31] == 0.0); - // A hard edge (no radius, feather or smooth) moves 1 px at 100%, so 50% is half a - // pixel. The pixel just outside fades in instead of snapping on. - let grown = refine_buffer(None, &m, w, h, &RefineParams { shift_edge: 100.0, ..Default::default() }); - assert_eq!(m[5 * w + 30], 0.0); - assert_eq!(grown[5 * w + 30], 1.0); - assert_eq!(grown[5 * w + 31], 0.0); - let shrunk = refine_buffer(None, &m, w, h, &RefineParams { shift_edge: -100.0, ..Default::default() }); - assert_eq!(shrunk[5 * w + 28], 1.0); - assert_eq!(shrunk[5 * w + 29], 0.0); - let half = refine_buffer(None, &m, w, h, &RefineParams { shift_edge: 50.0, ..Default::default() }); - assert!((half[5 * w + 30] - 0.5).abs() < 0.02, "half pixel {}", half[5 * w + 30]); - assert_eq!(half[5 * w + 31], 0.0, "the blend must not jump a second pixel"); - let pulled = refine_buffer(None, &m, w, h, &RefineParams { shift_edge: -50.0, ..Default::default() }); - assert!((pulled[5 * w + 29] - 0.5).abs() < 0.02, "half-pixel contract {}", pulled[5 * w + 29]); - // A softer edge moves farther: 100% is the feather, here 4 px. - let wide = refine_buffer(None, &m, w, h, &RefineParams { feather: 4.0, contrast: 100.0, shift_edge: 100.0, ..Default::default() }); - assert_eq!(wide[5 * w + 33], 1.0); - assert_eq!(wide[5 * w + 34], 0.0); + // Shift Edge leaves a hard edge where it is, at any amount: only soft coverage moves. + for shift_edge in [-100.0, -50.0, 50.0, 100.0] { + let s = refine_buffer(None, &m, w, h, &RefineParams { shift_edge, ..Default::default() }); + assert_eq!(s, m, "Shift Edge {shift_edge}% moved a hard edge"); + } + // Coverage before → after, read off the reference application's feathered edges. + for (shift_edge, before, after) in [ + (-100.0, 0.898, 0.153), + (-100.0, 0.612, 0.031), + (-100.0, 0.965, 0.357), + (-50.0, 0.537, 0.141), + (-50.0, 0.898, 0.557), + (100.0, 0.102, 0.847), + (100.0, 0.388, 0.969), + ] { + let flat = vec![before; w * h]; + let s = refine_buffer(None, &flat, w, h, &RefineParams { shift_edge, ..Default::default() }); + assert!((s[5 * w + 30] - after).abs() < 0.01, "{shift_edge}%: {before} became {}, expected {after}", s[5 * w + 30]); + } + // So a feathered edge moves: at −100% its half-covered point lands where it was 98%. + let soft = refine_buffer(None, &m, w, h, &RefineParams { feather: 6.0, ..Default::default() }); + let pulled = refine_buffer(None, &m, w, h, &RefineParams { feather: 6.0, shift_edge: -100.0, ..Default::default() }); + let half = |v: &[f32]| (0..w).find(|x| v[5 * w + x] < 0.5).unwrap_or(w); + assert!(half(&pulled) < half(&soft), "−100% did not pull the soft edge in"); let sm = refine_buffer(None, &m, w, h, &RefineParams { smooth: 50.0, ..Default::default() }); assert!(sm[5 * w + 10] == 1.0 && sm[5 * w + 50] == 0.0); } diff --git a/crates/algo/src/matting/morphology/tests.rs b/crates/algo/src/matting/morphology/tests.rs index 4100f70bed..0866419cfd 100644 --- a/crates/algo/src/matting/morphology/tests.rs +++ b/crates/algo/src/matting/morphology/tests.rs @@ -230,65 +230,6 @@ fn octagonal_morphology_release_comparison() { } } -#[test] -#[ignore = "release comparison of the full tiled Select and Mask CPU path"] -#[allow(clippy::assertions_on_constants)] -fn tiled_refinement_release_comparison() { - use crate::{ - matting::{RefineParams, refine_mask_with, region_reader}, - segment::{ImageSampler, RgbImage, trim_region}, - selection::Region, - }; - use photocraft_geom::Rect; - use std::{hint::black_box, time::Instant}; - assert!(!cfg!(debug_assertions), "run with --release"); - for (w, h) in [(6000usize, 4000usize), (7200, 5000)] { - let image = RgbImage::from_fn(w, h, |x, y| { - let t = x as f32 / w as f32; - let u = y as f32 / h as f32; - [0.2 + 0.5 * t, 0.15 + 0.55 * u, 0.3 + 0.15 * ((x / 17 + y / 23) % 3) as f32] - }); - let sampler = ImageSampler { img: &image, origin: (0, 0) }; - let canvas = Rect::new(0, 0, w as i32, h as i32); - let mask: Vec<_> = (0..w * h) - .map(|i| { - let dx = (i % w) as f64 - w as f64 * 0.51; - let dy = (i / w) as f64 - h as f64 * 0.48; - let d = (dx * dx + dy * dy).sqrt(); - ((0.5 + (w.min(h) as f64 * 0.27 - d) / 7.0).clamp(0.0, 1.0) * 255.0).round() as u8 - }) - .collect(); - let region = trim_region(Region { bbox: canvas, mask }).unwrap(); - let reader = region_reader(®ion); - let p = RefineParams { radius: 64.0, shift_edge: 100.0, ..Default::default() }; - let old = refine_mask_with(&sampler, &reader, region.bbox, canvas, &p, None, repeated).unwrap(); - let new = refine_mask_with(&sampler, &reader, region.bbox, canvas, &p, None, morph).unwrap(); - assert_eq!(new.bbox, old.bbox); - assert_eq!(new.mask, old.mask); - let (mut old_ms, mut new_ms) = (Vec::new(), Vec::new()); - for run in 0..3 { - let mut measure = |fast: bool| { - let start = Instant::now(); - let out = if fast { - refine_mask_with(&sampler, black_box(&reader), region.bbox, canvas, &p, None, morph) - } else { - refine_mask_with(&sampler, black_box(&reader), region.bbox, canvas, &p, None, repeated) - } - .unwrap(); - let elapsed = start.elapsed().as_secs_f64() * 1000.0; - assert_eq!(out.bbox, old.bbox); - assert_eq!(out.mask, old.mask); - if fast { new_ms.push(elapsed) } else { old_ms.push(elapsed) } - }; - measure(run % 2 == 0); - measure(run % 2 != 0); - } - old_ms.sort_by(f64::total_cmp); - new_ms.sort_by(f64::total_cmp); - println!("REFINE {w}x{h} radius=64 shift=100 old_ms={old_ms:?} new_ms={new_ms:?} speedup={:.2}", old_ms[1] / new_ms[1]); - } -} - #[test] #[ignore = "release comparison on a dense fractional 24 MP matte"] #[allow(clippy::assertions_on_constants)] diff --git a/crates/engine/src/cmd/smartselect_cmds/tests.rs b/crates/engine/src/cmd/smartselect_cmds/tests.rs index eca3d3aa7f..5fecac7969 100644 --- a/crates/engine/src/cmd/smartselect_cmds/tests.rs +++ b/crates/engine/src/cmd/smartselect_cmds/tests.rs @@ -451,9 +451,15 @@ fn refine_edge_outputs_to_a_new_document() { #[test] fn refine_edge_shift_and_contrast() { let mut s = blurred_edge_selected(); - // A hard selection moves 1 px at 100%: no band of several pixels. - s.execute("select.refineEdge", json!({"shiftEdge": 100})).unwrap(); - assert!(cov(&s, 40, 20) > 0.99 && cov(&s, 41, 20) < 0.01); + // Shift Edge leaves a hard selection where it is (selected x < 40), at either end. + for shift in [100, -100] { + s.execute("select.refineEdge", json!({"shiftEdge": shift})).unwrap(); + assert!(cov(&s, 39, 20) > 0.99 && cov(&s, 40, 20) < 0.01, "Shift Edge {shift}% moved a hard edge"); + s.execute("edit.undo", json!({})).unwrap(); + } + // A feathered one moves: -100% pulls its half-covered point inside the old edge. + s.execute("select.refineEdge", json!({"feather": 4, "shiftEdge": -100})).unwrap(); + assert!(cov(&s, 37, 20) < 0.5 && cov(&s, 30, 20) > 0.99); s.execute("edit.undo", json!({})).unwrap(); s.execute("select.refineEdge", json!({"feather": 8, "contrast": 100})).unwrap(); assert!(cov(&s, 38, 20) == 1.0 && cov(&s, 42, 20) == 0.0); @@ -661,8 +667,9 @@ fn refine_edge_edits_match_the_tools_and_reach_the_outputs() { // Refinement runs on the edited selection: Shift Edge contracts the brushed bar too. let mut s = two_regions(8); s.execute("select.rect", json!({"x": 0, "y": 0, "width": 20, "height": 20})).unwrap(); - s.execute("select.refineEdge", json!({"shiftEdge": -100, "edits": [{"tool": "brush", "points": [[30, 80], [100, 80]], "size": 20}]})).unwrap(); - assert!(cov(&s, 60, 80) >= 0.5 && cov(&s, 60, 89) < 0.5 && cov(&s, 60, 70) < 0.5, "the hard 20 px bar shrank by one pixel at -100% Shift Edge"); + s.execute("select.refineEdge", json!({"feather": 4, "shiftEdge": -100, "edits": [{"tool": "brush", "points": [[30, 80], [100, 80]], "size": 20}]})) + .unwrap(); + assert!(cov(&s, 60, 80) >= 0.5 && cov(&s, 60, 87) < 0.5 && cov(&s, 60, 72) < 0.5, "the feathered 20 px bar did not shrink at -100% Shift Edge"); // Layer mask and new-layer outputs take the edited selection. let mut s = blurred_edge_selected(); let r = s.execute("select.refineEdge", json!({"output": "layerMask", "edits": [{"tool": "invert"}]})).unwrap(); diff --git a/docs/octagonal-mask-performance.md b/docs/octagonal-mask-performance.md index 77d9c68111..b009d90df2 100644 --- a/docs/octagonal-mask-performance.md +++ b/docs/octagonal-mask-performance.md @@ -1,6 +1,12 @@ # Exact octagonal mask shifts -Select and Mask applies Shift Edge as flat grayscale dilation or erosion. Previously, +> **2026-10-30:** Shift Edge no longer uses this morphology. Measured against the reference +> application, it is a curve on each pixel's coverage (odds times `49^(shift / 100)`), so it +> leaves hard edges in place; see `RefineParams::shift_edge`. `matting::morph` remains for the +> Select and Mask view and Select › Modify, and the kernel numbers below still hold. The full +> refinement comparison further down timed the old Shift Edge and is kept as history. + +Select and Mask applied Shift Edge as flat grayscale dilation or erosion. Previously, `matting::morph` ran one full 3x3 pass for each radius step, alternating a square and a cross. A 64 px shift needed 64 image passes. The filter supports fractional coverage. From 833e69794d77ab87ef9e889777549ddc6a9d2fa0 Mon Sep 17 00:00:00 2001 From: David Mann Date: Fri, 30 Oct 2026 15:07:22 +0100 Subject: [PATCH 02/12] HUD Color Picker: square on the left, current colour under the pointer #6236 drew the hue strip left of the saturation/value square and opened the HUD just below the press point. In the reference application the square is on the left with the hue strip to its right, and the HUD opens so the colour you start from sits under the pointer: pressing without moving changes nothing, and a small move nudges the colour from there. The HUD now uses that layout and placement (still pulled in to stay on screen near an edge). The rects come from one `layout` function, shared by drawing, hit-testing and the tests. Tests: the square is left of the strip and the starting colour's spot is under the pointer; a mid colour opens with the pointer inside the square and picks nothing until it moves; near the bottom-right corner the HUD stays on screen and the press alone picks nothing. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/color_hud.rs | 92 ++++++++++++++++++-------- docs/color-picker.md | 2 +- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/crates/ui-egui/src/panels/color_hud.rs b/crates/ui-egui/src/panels/color_hud.rs index 5f15c9f954..47dca2b65e 100644 --- a/crates/ui-egui/src/panels/color_hud.rs +++ b/crates/ui-egui/src/panels/color_hud.rs @@ -1,5 +1,6 @@ //! Photoshop's HUD Color Picker (#6098): with a painting tool, Shift+Alt+right-click on the -//! canvas opens a floating hue strip and saturation/value square anchored under the pointer. +//! canvas opens a floating saturation/value square with a hue strip to its right, placed so the +//! current colour sits under the pointer (as the reference application does). //! While the right button stays down, moving the pointer over the HUD picks the foreground colour //! live (`tools.setColors` calls sharing one `coalesce` key, so the gesture is one history step, //! Rule 1); releasing the button keeps the colour and closes the HUD, Escape restores the colour @@ -26,15 +27,28 @@ use crate::state::UiState; const HUE_W: f32 = 16.0; /// Side of the saturation/value square, and the strip's height. const SQUARE: f32 = 128.0; -/// Gap between the strip and the square. +/// Gap between the square and the strip. const GAP: f32 = 8.0; /// The HUD's content size (the frame's chrome sits around it). -pub const SIZE: [f32; 2] = [HUE_W + GAP + SQUARE, SQUARE]; +pub const SIZE: [f32; 2] = [SQUARE + GAP + HUE_W, SQUARE]; + +/// The square (left) and the hue strip (right) inside the HUD's content rect. +fn layout(content: Rect) -> (Rect, Rect) { + let square = Rect::from_min_size(content.min, vec2(SQUARE, SQUARE)); + let strip = Rect::from_min_size(content.min + vec2(SQUARE + GAP, 0.0), vec2(HUE_W, SQUARE)); + (strip, square) +} + +/// Where the current colour (saturation, value) sits inside the content rect, from its top left. +fn colour_offset(hsv: [f32; 3]) -> Vec2 { + let f = |v: f32| if v.is_finite() { v.clamp(0.0, 1.0) } else { 0.0 }; + vec2(f(hsv[1]) * SQUARE, (1.0 - f(hsv[2])) * SQUARE) +} /// The HUD Color Picker while it is open. #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub struct Hud { - /// Where the gesture began, in screen points: the HUD anchors just below it. + /// Where the gesture began, in screen points: the HUD opens with the starting colour here. pub anchor: [f32; 2], /// The colour being picked: hue 0..360, saturation and value 0..1. pub hsv: [f32; 3], @@ -175,18 +189,21 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { close(&mut app.ui); return; } - // Keep the HUD on screen: anchored just under the press point, pulled in at the edges. + // The starting colour's spot in the square sits under the press point, as in the reference + // application; near a screen edge the HUD is pulled in so it stays whole. let screen = ctx.content_rect(); let style = ctx.global_style(); - let size = Vec2::from(SIZE) + style.spacing.menu_margin.sum() + egui::Vec2::splat(2.0 * style.visuals.window_stroke().width); - let at = egui::pos2(hud.anchor[0], hud.anchor[1] + 4.0); + let m = style.spacing.menu_margin; + let stroke = style.visuals.window_stroke().width; + let size = Vec2::from(SIZE) + m.sum() + egui::Vec2::splat(2.0 * stroke); + let start = rgb_to_hsb([hud.start[0], hud.start[1], hud.start[2]]); + let at = egui::pos2(hud.anchor[0], hud.anchor[1]) - colour_offset(start) - vec2(f32::from(m.left) + stroke, f32::from(m.top) + stroke); let pos = egui::pos2(at.x.min(screen.right() - size.x).max(screen.left()), at.y.min(screen.bottom() - size.y).max(screen.top())); let area = egui::Area::new(hud_id()).order(egui::Order::Foreground).fixed_pos(pos).show(ctx, |ui| { egui::Frame::popup(ui.style()) .show(ui, |ui| { let (rect, _) = ui.allocate_exact_size(vec2(SIZE[0], SIZE[1]), egui::Sense::hover()); - let strip = Rect::from_min_size(rect.min, vec2(HUE_W, SQUARE)); - let square = Rect::from_min_max(rect.min + vec2(HUE_W + GAP, 0.0), rect.max); + let (strip, square) = layout(rect); let p = ui.painter(); paint_hue_strip(p, strip); paint_sv_square(p, square, hud.hsv[0]); @@ -203,9 +220,8 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { .inner }); let (strip, square) = area.inner; - // Near a screen edge the clamped HUD can appear under the stationary pointer: picking starts - // only once the pointer has moved from where the gesture began, so the opening press alone - // never changes the colour. + // The HUD opens under the stationary pointer: picking starts only once the pointer has moved + // from where the gesture began, so the opening press alone never changes the colour. if hud.just_opened && hover.is_some_and(|p| p != egui::pos2(hud.anchor[0], hud.anchor[1])) { hud.just_opened = false; } @@ -285,8 +301,7 @@ mod tests { let w = style.visuals.window_stroke().width; let inner = Rect::from_min_max(area.min + vec2(f32::from(m.left) + w, f32::from(m.top) + w), area.max - vec2(f32::from(m.right) + w, f32::from(m.bottom) + w)); - let strip = Rect::from_min_size(inner.min, vec2(HUE_W, SQUARE)); - (strip, Rect::from_min_max(inner.min + vec2(HUE_W + GAP, 0.0), inner.max)) + layout(inner) } fn foreground(h: &Harness<'static, PhotocraftApp>) -> [f32; 4] { @@ -304,10 +319,13 @@ mod tests { assert!(hud.hold); assert_eq!(hud.start, [0.0, 0.0, 1.0, 1.0], "seeded from the foreground"); assert_eq!(h.state().ui.brush_picker, None, "the Brush Preset picker stays closed"); - // The HUD shows under the pointer, on screen. + // The square is left of the strip, and the pointer sits on the current colour: blue is + // full saturation and value, the square's top-right corner. let (strip, square) = rects(&h); assert!(strip.height() > 100.0 && square.width() > 100.0); - assert!(square.right() < h.ctx.content_rect().right()); + assert!(square.right() <= strip.left(), "square {square:?} left of strip {strip:?}"); + assert!(strip.right() < h.ctx.content_rect().right()); + assert!((square.right_top() - c).length() < 1.0, "the current colour {:?} is under the pointer {c:?}", square.right_top()); // Nothing painted. assert!(!h.state().session.journal.iter().any(|(id, _)| id == "paint.stroke")); } @@ -330,26 +348,43 @@ mod tests { assert!(h.state().ui.canvas_tool_menu.is_some(), "the selection menu opens, once"); } - /// A press near the bottom-right edge clamps the HUD under the still pointer: the opening - /// press alone must not pick — the colour changes only once the pointer moves (#6098). + /// The HUD opens with the current colour under the still pointer: the opening press alone + /// must not pick, the colour changes only once the pointer moves (#6098). Near the + /// bottom-right edge the HUD is pulled in to stay on screen, and still picks nothing. #[test] - fn a_hud_clamped_under_the_still_pointer_picks_nothing_until_it_moves() { + fn the_opening_press_picks_nothing_until_the_pointer_moves() { let mut h = harness_size(1200, 800); - let near = egui::pos2(h.state().last_canvas_rect.max.x - 10.0, h.state().last_canvas_rect.max.y - 10.0); - h.event(egui::Event::PointerMoved(near)); + h.state_mut().run("tools.setColors", json!({"foreground": [0.4, 0.3, 0.6, 1.0]})).unwrap(); + let start = foreground(&h); + let c = h.state().last_canvas_rect.center(); + h.event(egui::Event::PointerMoved(c)); h.run_steps(1); - button(&mut h, near, true, SHIFT_ALT); - assert!(h.state().ui.color_hud.is_some_and(|hud| hud.just_opened), "the HUD opened under the pointer"); + button(&mut h, c, true, SHIFT_ALT); + assert!(h.state().ui.color_hud.is_some_and(|hud| hud.just_opened), "the HUD opened"); let (_strip, square) = rects(&h); - assert!(square.contains(near), "the clamped HUD sits under the pointer: {near:?} in {square:?}"); + let hsv = rgb_to_hsb([start[0], start[1], start[2]]); + let spot = square.min + colour_offset([hsv[0], hsv[1], hsv[2]]); + assert!(square.contains(c) && (spot - c).length() < 1.0, "the current colour {spot:?} is under the pointer {c:?}"); h.run_steps(4); - assert_eq!(foreground(&h), [0.0, 0.0, 1.0, 1.0], "the stationary opening pointer picks nothing"); + assert_eq!(foreground(&h), start, "the stationary opening pointer picks nothing"); // Moving inside the HUD now picks, as during any other gesture. let to = square.min + vec2(1.0, square.height() - 1.0); h.event(egui::Event::PointerMoved(to)); h.run_steps(2); - assert_ne!(foreground(&h), [0.0, 0.0, 1.0, 1.0], "the move picks"); + assert_ne!(foreground(&h), start, "the move picks"); button(&mut h, to, false, SHIFT_ALT); + // At the bottom-right corner the HUD stays whole on screen, and the press alone picks nothing. + let near = egui::pos2(h.state().last_canvas_rect.max.x - 10.0, h.state().last_canvas_rect.max.y - 10.0); + let start = foreground(&h); + h.event(egui::Event::PointerMoved(near)); + h.run_steps(1); + button(&mut h, near, true, SHIFT_ALT); + let (strip, square) = rects(&h); + let screen = h.ctx.content_rect(); + assert!(screen.contains_rect(square.union(strip)), "on screen: {square:?} {strip:?} in {screen:?}"); + h.run_steps(4); + assert_eq!(foreground(&h), start, "the press near the edge picks nothing"); + button(&mut h, near, false, SHIFT_ALT); } #[test] @@ -490,13 +525,12 @@ mod tests { #[test] fn pick_at_maps_the_rects_and_ignores_the_gaps() { - let strip = Rect::from_min_size(Pos2::new(0.0, 0.0), vec2(HUE_W, SQUARE)); - let square = Rect::from_min_max(vec2(HUE_W + GAP, 0.0).to_pos2(), Pos2::new(HUE_W + GAP + SQUARE, SQUARE)); + let (strip, square) = layout(Rect::from_min_size(Pos2::ZERO, Vec2::from(SIZE))); assert_eq!(pick_at([120.0, 0.5, 0.5], strip, square, square.center()), Some([120.0, 0.5, 0.5])); let p = pick_at([120.0, 0.5, 0.5], strip, square, square.left_top() + vec2(1.0, 1.0)).expect("in the square"); assert!(p[0] == 120.0 && p[1] < 0.02 && p[2] > 0.98, "top-left is saturated and bright: {p:?}"); assert_eq!(pick_at([0.0, 0.5, 0.5], strip, square, strip.center()), Some([180.0, 0.5, 0.5])); - assert_eq!(pick_at([0.0, 0.5, 0.5], strip, square, Pos2::new(HUE_W + GAP / 2.0, 10.0)), None, "the gap picks nothing"); + assert_eq!(pick_at([0.0, 0.5, 0.5], strip, square, Pos2::new(SQUARE + GAP / 2.0, 10.0)), None, "the gap picks nothing"); assert_eq!(pick_at([0.0, 0.5, 0.5], strip, square, Pos2::new(1e9, -1e9)), None); } } diff --git a/docs/color-picker.md b/docs/color-picker.md index b53ce499a9..4c2e1e144f 100644 --- a/docs/color-picker.md +++ b/docs/color-picker.md @@ -4,7 +4,7 @@ Click the color swatch in the Type options bar or Character panel to open the fu ## HUD Color Picker -With a painting tool, **Shift+Alt+right-click** on the canvas opens Photoshop's HUD Color Picker under the pointer: a hue strip and a saturation/value square. While the right button stays down, moving the pointer over the HUD sets the foreground color live; releasing the button keeps it. **Esc** restores the color the gesture started from. The whole gesture is one coalesced `tools.setColors` history step. The agents' equivalent is `ui.pointer` with `"button": "right"`, `"shift": true, "alt": true` (the down opens the HUD, the up closes it). +With a painting tool, **Shift+Alt+right-click** on the canvas opens the HUD Color Picker: a saturation/value square with a hue strip to its right, placed so the current color sits under the pointer. While the right button stays down, moving the pointer over the HUD sets the foreground color live; releasing the button keeps it. **Esc** restores the color the gesture started from. The whole gesture is one coalesced `tools.setColors` history step. The agents' equivalent is `ui.pointer` with `"button": "right"`, `"shift": true, "alt": true` (the down opens the HUD, the up closes it). The existing canvas eyedropper continues to sample the document while the Color Picker is open, including canvas zoom and pan. This works without operating-system screen-capture permission. From ac3dbfd2bc80031ebcc5e42bcbe0987fd61bcac3 Mon Sep 17 00:00:00 2001 From: David Mann Date: Fri, 30 Oct 2026 15:08:33 +0100 Subject: [PATCH 03/12] HUD Color Picker: Esc closes the HUD and keeps the picked colour #6236 made Escape put back the colour the gesture started from. In the reference application Escape while the right button is held closes the HUD and keeps the colour picked so far; moving the pointer afterwards changes nothing (measured: picked 0,40,96 from 40,90,160; after Esc and a further move it stayed 0,40,96, also after the release). Escape now ends the gesture like the button release does. The pick is still one coalesced history step, so Undo takes it back. Test: Escape closes the HUD with the picked colour kept, a later move picks nothing, and releasing the button afterwards opens nothing. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/color_hud.rs | 35 +++++++++++++------------- docs/color-picker.md | 2 +- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/crates/ui-egui/src/panels/color_hud.rs b/crates/ui-egui/src/panels/color_hud.rs index 47dca2b65e..8ca7522147 100644 --- a/crates/ui-egui/src/panels/color_hud.rs +++ b/crates/ui-egui/src/panels/color_hud.rs @@ -3,9 +3,9 @@ //! current colour sits under the pointer (as the reference application does). //! While the right button stays down, moving the pointer over the HUD picks the foreground colour //! live (`tools.setColors` calls sharing one `coalesce` key, so the gesture is one history step, -//! Rule 1); releasing the button keeps the colour and closes the HUD, Escape restores the colour -//! the gesture started from. A left press closes it too, which is how the control channel ends a -//! gesture it opened with `ui.pointer`. +//! Rule 1); releasing the button or pressing Escape closes the HUD and keeps the colour picked so +//! far, as in the reference application (Undo takes it back). A left press closes it too, which is +//! how the control channel ends a gesture it opened with `ui.pointer`. //! //! The HUD is drawn as plain meshes and hit-tested from the raw pointer position rather than with //! interactive widgets: the selection follows the pointer while the *right* button is held, which @@ -52,7 +52,7 @@ pub struct Hud { pub anchor: [f32; 2], /// The colour being picked: hue 0..360, saturation and value 0..1. pub hsv: [f32; 3], - /// The foreground colour when the gesture began; Escape restores it. + /// The foreground colour when the gesture began: where the HUD opens, and the alpha it keeps. pub start: [f32; 4], /// The `coalesce` key of this gesture's `tools.setColors` calls. pub key: u64, @@ -176,16 +176,7 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { i.pointer.hover_pos(), ) }); - if esc { - let start = hud.start; - let changed = app.session.tools.foreground != start; - close(&mut app.ui); - if changed { - set_foreground(app, start, hud.key); - } - return; - } - if (hud.hold && released) || pressed_left { + if esc || (hud.hold && released) || pressed_left { close(&mut app.ui); return; } @@ -427,8 +418,10 @@ mod tests { assert_eq!(foreground(&h), kept); } + /// The reference application's Escape ends the pick where it is: the HUD closes, the colour + /// picked so far stays, and moving afterwards changes nothing. #[test] - fn escape_restores_the_starting_colour() { + fn escape_closes_the_hud_and_keeps_the_picked_colour() { let mut h = harness(); let c = h.state().last_canvas_rect.center(); h.event(egui::Event::PointerMoved(c)); @@ -437,11 +430,19 @@ mod tests { let (_strip, square) = rects(&h); h.event(egui::Event::PointerMoved(square.center())); h.run_steps(2); - assert_ne!(foreground(&h), [0.0, 0.0, 1.0, 1.0], "the pick is live"); + let picked = foreground(&h); + assert_ne!(picked, [0.0, 0.0, 1.0, 1.0], "the pick is live"); h.key_press(egui::Key::Escape); h.run_steps(2); assert_eq!(h.state().ui.color_hud, None); - assert_eq!(foreground(&h), [0.0, 0.0, 1.0, 1.0], "Escape restores the colour"); + assert_eq!(foreground(&h), picked, "Escape keeps the picked colour"); + h.event(egui::Event::PointerMoved(square.min + vec2(2.0, 2.0))); + h.run_steps(2); + assert_eq!(foreground(&h), picked, "moving after Escape picks nothing"); + button(&mut h, square.min + vec2(2.0, 2.0), false, SHIFT_ALT); + assert_eq!(h.state().ui.color_hud, None); + assert_eq!(h.state().ui.brush_picker, None, "releasing the button afterwards opens nothing"); + assert_eq!(foreground(&h), picked); } #[test] diff --git a/docs/color-picker.md b/docs/color-picker.md index 4c2e1e144f..d79a6f0273 100644 --- a/docs/color-picker.md +++ b/docs/color-picker.md @@ -4,7 +4,7 @@ Click the color swatch in the Type options bar or Character panel to open the fu ## HUD Color Picker -With a painting tool, **Shift+Alt+right-click** on the canvas opens the HUD Color Picker: a saturation/value square with a hue strip to its right, placed so the current color sits under the pointer. While the right button stays down, moving the pointer over the HUD sets the foreground color live; releasing the button keeps it. **Esc** restores the color the gesture started from. The whole gesture is one coalesced `tools.setColors` history step. The agents' equivalent is `ui.pointer` with `"button": "right"`, `"shift": true, "alt": true` (the down opens the HUD, the up closes it). +With a painting tool, **Shift+Alt+right-click** on the canvas opens the HUD Color Picker: a saturation/value square with a hue strip to its right, placed so the current color sits under the pointer. While the right button stays down, moving the pointer over the HUD sets the foreground color live; releasing the button keeps it. **Esc** closes the HUD and keeps the color picked so far (Undo takes it back). The whole gesture is one coalesced `tools.setColors` history step. The agents' equivalent is `ui.pointer` with `"button": "right"`, `"shift": true, "alt": true` (the down opens the HUD, the up closes it). The existing canvas eyedropper continues to sample the document while the Color Picker is open, including canvas zoom and pan. This works without operating-system screen-capture permission. From f1518435918a44f2fa19ca4dfc6332ad27371421 Mon Sep 17 00:00:00 2001 From: David Mann Date: Fri, 30 Oct 2026 15:36:17 +0100 Subject: [PATCH 04/12] Type: a copied type layer gets the destination's point size, not a transform #9594 fixed type copied between documents of different resolutions re-laying out at the wrong size on its next edit by running the Image Size resolution step on the copies: the type kept its point size (24 pt) and got a compensating transform of src/dst ppi, with box and path text scaled up to match. The reference application, measured by script, keeps the pixels and rewrites the point size instead: 24 pt copied from a 72 ppi document into a 300 ppi one becomes 5.76 pt (24 x 72/300), and 24 pt from 300 ppi into 72 ppi becomes 100 pt. There is no transform, the Character panel shows the rewritten size, and later edits keep it. adapt_copies, which both Copy Layers to Document (Duplicate Layer to another document, dragging layers between documents) and Paste of a copied layer go through, now calls canvas_geom::retarget_type_resolution: every size in points is multiplied by src/dst ppi (font size, leading, baseline shift, first-line/start/end indents, space before/after) and the PSD TySh is rebuilt. Box, path and transform are in pixels and stay as they were; tracking, kerning and auto leading are relative to the size and stay too. A resolution that isn't a positive number counts as 72 ppi (what the text engine lays it out at), and the scaled sizes stay finite and above zero. Image Size keeps its own behaviour. Tests: copy and Paste in Place, 72->300 (5.76 pt) and 300->72 (100 pt): leading, baseline shift and indent scale, tracking stays, no transform, the same ink bounds as the source, and after an edit the size is kept and bounds and coverage match the source's render. Paragraph text keeps its box and wrapping. Bad and absurd resolutions don't panic or produce non-finite sizes. The #9594 test is replaced by these. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/canvas_geom.rs | 40 +++++ crates/engine/src/cmd/layer_copy_cmds.rs | 8 +- .../engine/src/cmd/layer_copy_cmds/tests.rs | 148 +++++++++++++++--- 3 files changed, 172 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/canvas_geom.rs b/crates/engine/src/canvas_geom.rs index 0a4ed6faa4..6276e432a3 100644 --- a/crates/engine/src/canvas_geom.rs +++ b/crates/engine/src/canvas_geom.rs @@ -107,6 +107,46 @@ pub(crate) fn set_resolution(doc: &mut Document, dpi: f32) { }); } +/// Moves the type of `doc` (scratch copies on their way to another document) to `dpi` keeping its +/// pixels, as the reference application does when a type layer is copied between documents of +/// different resolutions: every size in points is rewritten by old / new (24 pt at 72 ppi is +/// 5.76 pt at 300 ppi), so `pt x dpi / 72` pixels stay the same and the layer gets no +/// compensating transform. Box, path and transform are in pixels and stay; tracking and kerning +/// (1/1000 em) and auto leading (a multiple of the size) follow the size. +pub(crate) fn retarget_type_resolution(doc: &mut Document, dpi: f32) { + // The text engine lays out a resolution that isn't a positive number at 72 ppi. + let effective = |d: f32| if d.is_finite() && d > 0.0 { f64::from(d) } else { 72.0 }; + let old = std::mem::replace(&mut doc.resolution_dpi, dpi); + let k = effective(old) / effective(dpi); + if !(k.is_finite() && k > 0.0) || k == 1.0 { + return; + } + crate::image_cmds::for_each_layer(&mut doc.layers, &mut |l| { + let LayerContent::Text(t) = &mut l.content else { return }; + t.size_pt = point_size(t.size_pt, k); + for r in &mut t.runs { + let st = &mut r.style; + st.size_pt = point_size(st.size_pt, k); + st.leading_pt = st.leading_pt.map(|v| point_size(v, k)); + st.baseline_shift_pt = grown(st.baseline_shift_pt, k); + } + for p in &mut t.paragraphs { + let st = &mut p.style; + for v in [&mut st.first_line_indent_pt, &mut st.start_indent_pt, &mut st.end_indent_pt, &mut st.space_before_pt, &mut st.space_after_pt] { + *v = grown(*v, k); + } + } + retag_text(dpi, t); + }); +} + +/// A positive size in points times `k`, kept finite and above zero (a tiny ratio must not make +/// the text vanish). +fn point_size(v: f32, k: f64) -> f32 { + let out = grown(v, k); + if v > 0.0 && out <= 0.0 { f32::MIN_POSITIVE } else { out } +} + /// `v x k` kept finite (a huge box from a file must not become infinite). fn grown(v: f32, k: f64) -> f32 { (f64::from(v) * k).clamp(-f64::from(f32::MAX), f64::from(f32::MAX)) as f32 diff --git a/crates/engine/src/cmd/layer_copy_cmds.rs b/crates/engine/src/cmd/layer_copy_cmds.rs index d497133380..2a4ce26493 100644 --- a/crates/engine/src/cmd/layer_copy_cmds.rs +++ b/crates/engine/src/cmd/layer_copy_cmds.rs @@ -118,10 +118,10 @@ pub(crate) fn adapt_copies(s: &Session, copies: &mut Document, dest: &Document) *surf = surf.convert(f); }); } - // Type keeps its size in pixels at the destination's resolution, as an Image Size resolution - // change keeps it (Photoshop adapts the point size: 24 pt at 72 ppi is 5.76 pt at 300 ppi). - // Left at the source's, a copy re-laid out at the destination's on its next edit (#9560). - crate::canvas_geom::set_resolution(copies, dest.resolution_dpi); + // Type keeps its pixels and takes the destination's point size, as in the reference + // application (24 pt at 72 ppi becomes 5.76 pt at 300 ppi, with no transform). Left at the + // source's, a copy re-laid out at the destination's resolution on its next edit (#9560). + crate::canvas_geom::retarget_type_resolution(copies, dest.resolution_dpi); let new_canvas = copies.bounds() != dest.bounds(); prepare_shapes(&mut copies.layers, dest, new_canvas); Ok(()) diff --git a/crates/engine/src/cmd/layer_copy_cmds/tests.rs b/crates/engine/src/cmd/layer_copy_cmds/tests.rs index cdd7d55b7f..f3176af0af 100644 --- a/crates/engine/src/cmd/layer_copy_cmds/tests.rs +++ b/crates/engine/src/cmd/layer_copy_cmds/tests.rs @@ -370,25 +370,133 @@ fn a_copied_group_cannot_nest_the_destination_past_the_group_depth_limit() { assert_eq!(*doc(&s, 1), before, "nothing was copied"); } -/// #9560: a type layer copied between documents of different resolutions keeps its size in -/// pixels, also after its next text edit (it used to re-lay out at the destination's resolution: -/// a 24 pt line from 72 ppi grew about 4.2 times in a 300 ppi document). Photoshop adapts the -/// point size instead (24 pt at 72 ppi is 5.76 pt at 300 ppi); the pixels are the same. -#[test] -fn copied_type_keeps_its_pixel_size_across_resolutions_after_an_edit() { - for (from, to) in [(72, 300), (300, 72)] { - let mut s = Session::new(); - s.execute("file.new", json!({"width": 400, "height": 200, "resolution": from})).unwrap(); - let t = LayerId(s.execute("type.create", json!({"text": "H", "size": 24, "x": 128, "y": 100})).unwrap()["layer"].as_u64().unwrap()); - let original = bounds(&s, 0, t); - s.execute("file.new", json!({"width": 400, "height": 200, "resolution": to})).unwrap(); - s.set_active(0); - s.execute("layer.select", json!({"layer": t.0})).unwrap(); - let id = copied(&s.execute("layer.copyToDocument", json!({"document": 1})).unwrap()); - assert_eq!(bounds(&s, 1, id).size(), original.size(), "{from}→{to}: the copy"); - s.execute("type.edit", json!({"layer": id.0, "text": "M"})).unwrap(); - s.execute("type.edit", json!({"layer": id.0, "text": "H"})).unwrap(); - let after = bounds(&s, 1, id); - assert_eq!(after.size(), original.size(), "{from}→{to}: after an edit, {after:?} vs {original:?}"); +/// The type layer `id` of document `i`. +fn text(s: &Session, i: usize, id: LayerId) -> &photocraft_doc::TextLayer { + match &doc(s, i).layer(id).unwrap().content { + LayerContent::Text(t) => t, + other => panic!("not type: {other:?}"), } } + +/// Bounds and summed alpha of a layer's rendered pixels. +fn ink(s: &Session, i: usize, id: LayerId) -> (Rect, f64) { + let surf = doc(s, i).layer(id).unwrap().surface().unwrap(); + let b = surf.content_bounds(); + let a = surf.read_region(b).chunks(surf.channels()).map(|px| f64::from(*px.last().unwrap())).sum(); + (b, a) +} + +/// Document 0 at `from` ppi with a 24 pt "H" (leading 30 pt, baseline shift 2 pt, first-line +/// indent 6 pt) and document 1 at `to` ppi; the type layer is selected in document 0. +fn type_docs(from: u32, to: u32) -> (Session, LayerId) { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 400, "height": 200, "resolution": from})).unwrap(); + let t = LayerId( + s.execute("type.create", json!({"text": "H", "size": 24, "leading": 30, "baselineShift": 2, "x": 128, "y": 100})).unwrap()["layer"].as_u64().unwrap(), + ); + s.execute("type.setStyle", json!({"layer": t.0, "firstLineIndent": 6})).unwrap(); + s.execute("file.new", json!({"width": 400, "height": 200, "resolution": to})).unwrap(); + s.set_active(0); + s.execute("layer.select", json!({"layer": t.0})).unwrap(); + (s, t) +} + +fn close(a: f32, b: f32) -> bool { + (a - b).abs() <= 1e-4 * b.abs().max(1.0) +} + +/// Checks a copy of `src` (document 0) as `dst` (document 1) at `pt` points (the reference +/// application's rewritten size for 24 pt), then that an edit keeps that size and the pixels. +fn check_rewritten(s: &mut Session, src: LayerId, dst: LayerId, pt: f32, label: &str) { + let k = pt / 24.0; + let (from, to) = (text(s, 0, src), text(s, 1, dst)); + assert_eq!(to.transform, from.transform, "{label}: no compensating transform"); + assert!(close(to.size_pt, pt), "{label}: {} pt", to.size_pt); + let run = &to.runs[0].style; + assert!(close(run.size_pt, pt), "{label}: run {} pt", run.size_pt); + assert!(close(run.leading_pt.unwrap(), 30.0 * k), "{label}: leading {:?}", run.leading_pt); + assert!(close(run.baseline_shift_pt, 2.0 * k), "{label}: baseline shift {}", run.baseline_shift_pt); + assert_eq!(run.tracking, from.runs[0].style.tracking, "{label}: tracking is in 1/1000 em"); + assert!(close(to.paragraphs[0].style.first_line_indent_pt, 6.0 * k), "{label}: indent {}", to.paragraphs[0].style.first_line_indent_pt); + let (b0, a0) = ink(s, 0, src); + assert_eq!(ink(s, 1, dst).0, b0, "{label}: the copy's pixels"); + // Re-laid out at the destination's resolution, the copy renders as the source does. + s.execute("type.edit", json!({"layer": dst.0, "text": "M"})).unwrap(); + s.execute("type.edit", json!({"layer": dst.0, "text": "H"})).unwrap(); + assert!(close(text(s, 1, dst).size_pt, pt), "{label}: an edit keeps {} pt", text(s, 1, dst).size_pt); + let (b1, a1) = ink(s, 1, dst); + assert_eq!(b1, b0, "{label}: after an edit, {b1:?} vs {b0:?}"); + assert!((a1 - a0).abs() <= a0 * 0.005, "{label}: coverage {a1} vs {a0}"); +} + +/// #9560: a type layer copied between documents of different resolutions keeps its pixels and +/// takes the destination's point size, as in the reference application (measured: 24 pt from +/// 72 ppi is 5.76 pt at 300 ppi, from 300 ppi 100 pt at 72 ppi, with no transform; later edits +/// keep that size). It used to keep 24 pt and re-lay out about 4.2 times larger or smaller on +/// its next edit, and then kept 24 pt behind a compensating transform. +#[test] +fn copied_type_takes_the_destinations_point_size_and_keeps_its_pixels() { + for (from, to, pt) in [(72, 300, 5.76), (300, 72, 100.0)] { + let (mut s, t) = type_docs(from, to); + let id = copied(&s.execute("layer.copyToDocument", json!({"document": 1})).unwrap()); + check_rewritten(&mut s, t, id, pt, &format!("copy {from}→{to}")); + } +} + +/// The same through Copy and Paste in Place of the whole type layer. +#[test] +fn pasted_type_takes_the_destinations_point_size_and_keeps_its_pixels() { + for (from, to, pt) in [(72, 300, 5.76), (300, 72, 100.0)] { + let (mut s, t) = type_docs(from, to); + s.execute("edit.copy", json!({})).unwrap(); + s.set_active(1); + let id = LayerId(s.execute("edit.pasteSpecial.pasteInPlace", json!({})).unwrap()["layer"].as_u64().unwrap()); + check_rewritten(&mut s, t, id, pt, &format!("paste {from}→{to}")); + } +} + +/// Paragraph text keeps its box (in pixels) and its wrapping: only the point sizes change. +#[test] +fn copied_paragraph_type_keeps_its_box() { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 400, "height": 200, "resolution": 72})).unwrap(); + let t = + LayerId(s.execute("type.create", json!({"text": "Hello wrapping world", "size": 24, "box": [20, 20, 120, 150]})).unwrap()["layer"].as_u64().unwrap()); + s.execute("file.new", json!({"width": 400, "height": 200, "resolution": 300})).unwrap(); + s.set_active(0); + s.execute("layer.select", json!({"layer": t.0})).unwrap(); + let id = copied(&s.execute("layer.copyToDocument", json!({"document": 1})).unwrap()); + assert_eq!(text(&s, 1, id).shape, text(&s, 0, t).shape); + assert!(close(text(&s, 1, id).size_pt, 5.76)); + let (b0, _) = ink(&s, 0, t); + s.execute("type.edit", json!({"layer": id.0, "text": "Hello wrapping world"})).unwrap(); + assert_eq!(ink(&s, 1, id).0, b0, "the same lines after a re-layout"); +} + +/// Equal resolutions change nothing; one that isn't a positive number lays out at 72 ppi, so it +/// is treated as 72 rather than producing an infinite or zero size. +#[test] +fn retargeting_type_survives_bad_resolutions() { + let mut d = Document::new("", photocraft_geom::Size::new(10, 10), ColorMode::Rgb, SampleType::U8); + d.layers.push(photocraft_doc::Layer::new("t", LayerContent::Text(photocraft_doc::TextLayer { text: "H".into(), size_pt: 24.0, ..Default::default() }))); + let size = |d: &Document| match &d.layers[0].content { + LayerContent::Text(t) => t.size_pt, + _ => panic!("not type"), + }; + d.resolution_dpi = 72.0; + crate::canvas_geom::retarget_type_resolution(&mut d, 72.0); + assert_eq!(size(&d), 24.0); + for bad in [0.0, -5.0, f32::NAN, f32::INFINITY] { + crate::canvas_geom::retarget_type_resolution(&mut d, bad); + assert_eq!(size(&d), 24.0, "{bad}"); + d.resolution_dpi = bad; + crate::canvas_geom::retarget_type_resolution(&mut d, 72.0); + assert_eq!(size(&d), 24.0, "from {bad}"); + } + // Absurd ratios stay finite and positive. + d.resolution_dpi = f32::MAX; + crate::canvas_geom::retarget_type_resolution(&mut d, f32::MIN_POSITIVE); + assert!(size(&d).is_finite() && size(&d) > 0.0, "{}", size(&d)); + crate::canvas_geom::retarget_type_resolution(&mut d, f32::MAX); + assert!(size(&d).is_finite() && size(&d) > 0.0, "{}", size(&d)); +} From 28185d6dc3a987d63db7f2c511b86a9abd57a6a8 Mon Sep 17 00:00:00 2001 From: David Mann Date: Fri, 30 Oct 2026 15:56:21 +0100 Subject: [PATCH 05/12] =?UTF-8?q?Shift+right-click=20menu:=20Edit=20Brush?= =?UTF-8?q?=E2=80=A6=20and=20every=20painting=20tool's=20own=20Mode=20list?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4985 added a canvas Shift+right-click popup that worked only for the Brush and listed only its painting modes. In the reference application Shift+right-click opens a menu for every painting tool: Edit Brush… (the Brush Preset picker a plain right-click opens), a separator, then the tool's own Mode list from its options bar: - Brush, Pencil: the painting modes with Behind and Clear - Clone Stamp, Pattern Stamp, History Brush: the blend modes - Mixer Brush, Color Replacement, Background Eraser: Edit Brush… only - Eraser: Brush / Pencil / Block (no Edit Brush…) - Blur, Sharpen, Smudge: Edit Brush… (the reference lists modes too) - Dodge, Burn: Shadows / Midtones / Highlights; Sponge: Desaturate / Saturate - Healing Brush: its blend modes; Gradient: the blend modes; Paint Bucket: the blend modes with Clear (no Edit Brush…) - Spot Healing, Magic Eraser and non-painting tools: no menu, the gesture keeps its old route brush_mode_menu.rs is now a per-tool item list (Item enum + items(tool)) instead of a Brush-only list. Each row sets exactly what the tool's options-bar control sets: a brush-based tool's mode through tools.setBrush (Brush/Pencil also Behind and their own Clear flag), the Eraser mode, tone range, sponge mode, gradient and bucket modes as tool options. Blend modes are grouped with separators; the current choice is checked; Behind and Clear are greyed on the Background layer. The menu remembers the tool that opened it and closes on a tool change. Only settings the options bar already has are offered. Not offered yet (no setting here): Behind for the stamps, History Brush, Gradient and Paint Bucket; the Blur/Sharpen/Smudge modes; the Spot Healing modes; the Healing Brush's Sampled/Pattern source and Replace; the History Brush source. Control: ui.inspect's brushModeMenu reports tool, mode, choices (the active tool's labels) and items (label/enabled/checked, separators); ui.context.choose {id: