diff --git a/Cargo.lock b/Cargo.lock index 71d25d8d1c..dd0b1435e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5023,6 +5023,7 @@ dependencies = [ name = "photocraft-algo" version = "0.17.0" dependencies = [ + "criterion", "photocraft-color", "photocraft-geom", "photocraft-raster", diff --git a/apps/photocraft-cli/src/batch.rs b/apps/photocraft-cli/src/batch.rs index bdaf249bc9..02bb1bb702 100644 --- a/apps/photocraft-cli/src/batch.rs +++ b/apps/photocraft-cli/src/batch.rs @@ -83,7 +83,10 @@ pub(crate) fn batch(a: &BatchArgs, out: &mut dyn Write, err: &mut dyn Write) -> h.session.actions.list = set.clone(); h.open(input)?; for (id, p) in &actions { - if photocraft_engine::actions_cmds::shell_view_command(id) || photocraft_engine::actions_cmds::shell_save_command(id) { + if photocraft_engine::actions_cmds::shell_view_command(id) + || photocraft_engine::actions_cmds::shell_save_command(id) + || photocraft_engine::actions_cmds::noop_step(&h.session, id, p) + { continue; } let r = h.command_run(id, p.clone()).map_err(|source| CliError::Command { id: id.clone(), source })?; diff --git a/apps/photocraft-cli/src/document.rs b/apps/photocraft-cli/src/document.rs index 9d730b4635..95d4c28288 100644 --- a/apps/photocraft-cli/src/document.rs +++ b/apps/photocraft-cli/src/document.rs @@ -93,6 +93,11 @@ pub(crate) fn run_cmds(a: &RunArgs, out: &mut dyn Write, err: &mut dyn Write) -> } let cmds = command_list(a)?; for (id, params) in cmds { + // A step that is a silent no-op in action playback (Deselect with nothing selected, an Arrange that cannot move the layer) is one in a script too (#8802). + if photocraft_engine::actions_cmds::noop_step(&h.session, &id, ¶ms) { + print_json(out, &json!({"command": id, "result": null}), JsonStyle::Compact)?; + continue; + } let r = h.command_run(&id, params).map_err(|source| CliError::Command { id: id.clone(), source })?; if let Some(message) = photocraft_engine::actions_cmds::nested_failure(&id, &r) { return Err(CliError::NestedFailure { id, message }); diff --git a/apps/photocraft-cli/tests/it/cli.rs b/apps/photocraft-cli/tests/it/cli.rs index 2c06a488fd..e592d552e2 100644 --- a/apps/photocraft-cli/tests/it/cli.rs +++ b/apps/photocraft-cli/tests/it/cli.rs @@ -1421,3 +1421,78 @@ fn control_token_env_vars_are_the_flags_fallbacks() { let err = serve_tcp_stderr(bin().args(["serve", "--port"]).arg(free_port().to_string()).env("PHOTOCRAFT_CONTROL_TOKEN", "")); assert!(err.contains("photocraft-cli serving on") && err.contains("photocraft-cli control token: "), "{err}"); } + +/// #8802: Deselect with nothing selected is skipped in a `run`, as action playback does (#6744), +/// so a script that deselects after Reveal Selection still finishes and saves. +#[test] +fn run_skips_deselect_with_nothing_selected() { + let folder = tmp("deselect-run"); + let native = folder.join("out.pcraft").to_string_lossy().to_string(); + let Invoke { code, stdout, stderr } = invoke(&[ + "run", + "--new", + r#"{"width":16,"height":16}"#, + "--cmd", + "select.rect", + "--params", + r#"{"x":0,"y":0,"width":8,"height":16}"#, + "--cmd", + "layer.new.layer", + "--cmd", + "layer.layerMask.revealSelection", + "--cmd", + "select.deselect", + "--cmd", + "layer.new.layer", + "--params", + r#"{"name":"after"}"#, + "--out", + &native, + ]); + assert_eq!(code, 0, "{stderr}"); + let results: Vec = stdout.lines().map(|l| serde_json::from_str(l).unwrap()).collect(); + assert_eq!(results.len(), 5, "every command reports a result: {stdout}"); + assert_eq!(results[3], json!({"command": "select.deselect", "result": null})); + let Invoke { code, stdout, stderr } = invoke(&["info", &native, "--compact"]); + assert_eq!(code, 0, "{stderr}"); + let info: Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!(info["layers"][0]["name"], "after", "the steps after Deselect ran: {info}"); + + // Deselect with a selection still deselects: Inverse afterwards finds nothing to invert. + let Invoke { code, stderr, .. } = invoke(&[ + "run", + "--new", + r#"{"width":16,"height":16}"#, + "--cmd", + "select.all", + "--cmd", + "select.deselect", + "--cmd", + "select.inverse", + "--out", + &native, + ]); + assert_eq!(code, 1, "{stderr}"); + assert!(stderr.contains("select.inverse") && stderr.contains("no selection"), "{stderr}"); + let _ = std::fs::remove_dir_all(&folder); +} + +/// #8802: a batched action that starts with Deselect plays on files with nothing selected, as the +/// same action does in the Actions panel (#6744). +#[test] +fn batch_skips_deselect_with_nothing_selected() { + let dir = tmp("deselect-batch"); + let path = |name: &str| dir.join(name).to_string_lossy().to_string(); + std::fs::create_dir_all(dir.join("in")).unwrap(); + let Invoke { code, stderr, .. } = invoke(&["run", "--new", r#"{"width":8,"height":6}"#, "--out", &path("in/a.png")]); + assert_eq!(code, 0, "{stderr}"); + std::fs::write(dir.join("steps.json"), json!([["select.deselect", {}], ["image.imageRotation.90cw", {}]]).to_string()).unwrap(); + let Invoke { code, stdout, stderr } = invoke(&["batch", "--actions", &path("steps.json"), "--in", &path("in"), "--out", &path("out")]); + assert_eq!(code, 0, "{stdout}{stderr}"); + assert!(stdout.contains("1 succeeded, 0 failed"), "{stdout}"); + let Invoke { code, stdout, stderr } = invoke(&["info", &path("out/a.png"), "--compact"]); + assert_eq!(code, 0, "{stderr}"); + let info: Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!((info["width"].as_u64(), info["height"].as_u64()), (Some(6), Some(8)), "the step after Deselect ran: {info}"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/apps/photocraft/src/services.rs b/apps/photocraft/src/services.rs index 585d475071..09d40d2cde 100644 --- a/apps/photocraft/src/services.rs +++ b/apps/photocraft/src/services.rs @@ -635,12 +635,6 @@ fn shell_word(s: &str) -> String { if s.bytes().all(|b| b.is_ascii_alphanumeric() || b"/._-+:,@".contains(&b)) { s.to_string() } else { format!("'{}'", s.replace('\'', r"'\''")) } } -/// The release-check transport, or `None` inside Flatpak: `flatpak update` owns updates there. -/// The sandbox has network (for the `--control` server), which does not change this. -fn update_transport(flatpak_id: Option<&std::ffi::OsStr>) -> Option { - flatpak_id.is_none().then(crate::updates::service) -} - pub fn native(automation: Option) -> Services { let clip: Rc>> = Rc::default(); let clip_unreadable: Rc> = Rc::default(); @@ -705,7 +699,7 @@ pub fn native(automation: Option) -> inbox: None, open_url: Some(Box::new(|url: &str| open::that(url).map_err(|e| e.to_string()))), beep: Some(platform_beep), - check_updates: update_transport(std::env::var_os("FLATPAK_ID").as_deref()), + check_updates: Some(crate::updates::service()), clipboard_set_image: Some({ let clip = clip.clone(); guarded_clipboard_set(move |w: u32, h: u32, px: &[u8]| { diff --git a/apps/photocraft/src/services/tests.rs b/apps/photocraft/src/services/tests.rs index b406e7797f..8f12fee908 100644 --- a/apps/photocraft/src/services/tests.rs +++ b/apps/photocraft/src/services/tests.rs @@ -182,9 +182,8 @@ fn open_filters_keep_literal_extensions_on_windows_and_macos() { } #[test] -fn flatpak_builds_have_no_update_transport() { - assert!(update_transport(Some(std::ffi::OsStr::new("ai.storyteller.photocraft"))).is_none()); - assert!(update_transport(None).is_some()); +fn native_services_provide_update_transport() { + assert!(native(None).check_updates.is_some()); } #[test] diff --git a/crates/algo/Cargo.toml b/crates/algo/Cargo.toml index af50c1d20e..767c8044d9 100644 --- a/crates/algo/Cargo.toml +++ b/crates/algo/Cargo.toml @@ -23,3 +23,8 @@ rayon = { workspace = true } [dev-dependencies] photocraft-testkit = { workspace = true } serde_json = { workspace = true } +criterion = { workspace = true } + +[[bench]] +name = "blur" +harness = false diff --git a/crates/algo/benches/blur.rs b/crates/algo/benches/blur.rs new file mode 100644 index 0000000000..3165b40cd0 --- /dev/null +++ b/crates/algo/benches/blur.rs @@ -0,0 +1,33 @@ +//! `cargo bench -p photocraft-algo` +use criterion::{Criterion, criterion_group, criterion_main}; +use photocraft_algo::{FilterParams, apply}; +use photocraft_color::PixelFormat; +use photocraft_geom::Rect; +use photocraft_raster::Surface; + +fn textured(n: u32) -> Surface { + let r = Rect::new(0, 0, n as i32, n as i32); + let mut s = Surface::new(PixelFormat::RGBA8); + let bytes: Vec = (0..n).flat_map(|y| (0..n).flat_map(move |x| [(x ^ y) as u8, (x / 3) as u8, (y * 7) as u8, 255])).collect(); + s.write_interleaved(r, &bytes); + s +} + +/// Gaussian Blur's direct-kernel path (radius up to 4; larger radii use box passes), including +/// the variance-matched kernels below radius 1 (#8544). +fn gaussian_direct(c: &mut Criterion) { + let n = 2048; + let s = textured(n); + let area = Rect::new(0, 0, n as i32, n as i32); + for radius in [0.3f32, 0.8, 3.0] { + let p = FilterParams::GaussianBlur { radius }; + c.bench_function(&format!("gaussian_blur_{radius}px_2048"), |b| b.iter(|| apply(&s, &p, area, area, None))); + } +} + +criterion_group! { + name = benches; + config = Criterion::default().sample_size(10); + targets = gaussian_direct +} +criterion_main!(benches); diff --git a/crates/algo/src/blur.rs b/crates/algo/src/blur.rs index 8260aadcfa..819bf6fa78 100644 --- a/crates/algo/src/blur.rs +++ b/crates/algo/src/blur.rs @@ -31,6 +31,43 @@ pub(crate) fn gaussian_kernel(sigma: f32) -> Vec { k.into_iter().map(|v| v / s).collect() } +/// Gaussian Blur's kernel for `radius` (its standard deviation). Sampling the curve at whole +/// pixels leaves a small radius almost no blur: at 0.3 the neighbour weight is about 0.004, a +/// tenth of the spread asked for (#8544). Below 1 the sampling width is instead chosen so the +/// kernel has exactly the variance `radius²`; from 1 up the sampled kernel is within about 0.4% +/// of it (its 3σ truncation), so larger radii are unchanged and the blur grows smoothly from 0.05. +pub(crate) fn gaussian_blur_kernel(radius: f32) -> Vec { + if !(0.05..1.0).contains(&radius) { + return gaussian_kernel(radius); + } + let variance = |k: &[f32]| { + let r = (k.len() / 2) as f32; + k.iter().enumerate().map(|(i, w)| w * (i as f32 - r).powi(2)).sum::() + }; + // Sampled at `width` with one fixed half-width for every radius below 1: its variance then + // rises steadily with `width` (the search can't settle on a step where taps are added), and + // the blur grows without the dips a width chosen per radius made at 1/3 and 2/3. Three is the + // half-width `gaussian_kernel` uses at radius 1, so the hand-over is smooth too. + let half = 3; + let sampled = |width: f32| { + let k: Vec = (-half..=half).map(|i| (-(i * i) as f32 / (2.0 * width * width)).exp()).collect(); + let s: f32 = k.iter().sum(); + k.into_iter().map(|v| v / s).collect::>() + }; + let target = radius * radius; + // The sampled kernel's variance only falls short of sigma², so the width lies above `radius`. + let (mut lo, mut hi) = (radius, radius + 1.0); + for _ in 0..40 { + let mid = 0.5 * (lo + hi); + if variance(&sampled(mid)) < target { + lo = mid; + } else { + hi = mid; + } + } + sampled(0.5 * (lo + hi)) +} + /// Separable convolution of `src` (premultiplied internally) for `out`. /// `src` must cover `out` grown by the kernel radii. pub(crate) fn conv_sep(src: &Image, out: Rect, kx: &[f32], ky: &[f32], alpha: bool) -> Vec { diff --git a/crates/algo/src/blur/tests.rs b/crates/algo/src/blur/tests.rs index 32256bc954..028ac65dcc 100644 --- a/crates/algo/src/blur/tests.rs +++ b/crates/algo/src/blur/tests.rs @@ -2,6 +2,76 @@ use super::*; use photocraft_color::{ColorMode, PixelFormat, SampleType}; use photocraft_raster::Surface; +/// #8544, rendered: Gaussian Blur at 0.1–0.3 px through the filter itself (the direct-kernel +/// path) visibly softens a hard edge, more with each step, as the kernel predicts. +#[test] +fn small_gaussian_radii_soften_a_rendered_edge() { + let canvas = Rect::new(0, 0, 20, 4); + let mut s = Surface::new(PixelFormat::RGBA8); + s.fill_rect(canvas, &[0.0, 0.0, 0.0, 1.0]); + s.fill_rect(Rect::new(10, 0, 20, 4), &[1.0, 1.0, 1.0, 1.0]); + let mut last = 0.0f32; + for radius in [0.1f32, 0.2, 0.3] { + let p = FilterParams::GaussianBlur { radius }; + assert!(box_widths(&p).is_none(), "the direct-kernel path"); + let out = crate::apply(&s, &p, canvas, canvas, None); + // The last black pixel takes the white side's share of the kernel. + let got = out.pixel(9, 1)[0]; + let k = gaussian_blur_kernel(radius); + let want: f32 = k[k.len() / 2 + 1..].iter().sum(); + assert!((got - want).abs() <= 1.0 / 255.0, "radius {radius}: {got} vs {want}"); + assert!(got >= 1.0 / 255.0 && got > last, "radius {radius}: {got} after {last}"); + last = got; + } +} + +/// #8544: Gaussian Blur spreads a hard edge for every radius from 0.1, growing smoothly +/// with the radius (no dead zone up to 0.3, then a jump); from 1 up the kernel is unchanged. +#[test] +fn small_gaussian_radii_blur_smoothly() { + let spread = |k: &[f32]| { + let r = (k.len() / 2) as f32; + k.iter().enumerate().map(|(i, w)| w * (i as f32 - r).powi(2)).sum::() + }; + let mut last = 0.0f32; + for step in 1..=20 { + let radius = step as f32 * 0.05; + let k = gaussian_blur_kernel(radius); + assert!((k.iter().sum::() - 1.0).abs() < 1e-5, "normalised at {radius}"); + if radius < 1.0 { + assert!((spread(&k) - radius * radius).abs() < 1e-4, "variance at {radius}: {}", spread(&k)); + } + // A hard 0 → 1 edge: the first pixel past it takes the kernel's tail beyond the centre. + let c = k.len() / 2; + let edge: f32 = k[c + 1..].iter().sum(); + assert!(edge > last, "radius {radius}: edge {edge} after {last}"); + if radius > 0.1 { + assert!(edge - last < 0.05, "radius {radius}: jump {} from {last}", edge - last); + } + last = edge; + } + assert!(gaussian_blur_kernel(0.1).len() >= 3, "0.1 blurs"); + // No step where the variance-matched range hands over to plain sampling. + let edge = |r: f32| { + let k = gaussian_blur_kernel(r); + k[k.len() / 2 + 1..].iter().sum::() + }; + assert!((edge(0.999) - edge(1.0)).abs() < 0.005, "{} vs {}", edge(0.999), edge(1.0)); + // No dip where a per-radius width used to add taps (1/3 and 2/3). + for pair in [(0.33f32, 0.34f32), (0.66, 0.67)] { + assert!(edge(pair.1) > edge(pair.0), "edge falls from {} to {}", pair.0, pair.1); + } + // Exact variance right up to the hand-over, past where `gaussian_kernel` changes width. + for radius in [0.98f32, 0.995, 0.998, 0.9995] { + let k = gaussian_blur_kernel(radius); + assert!((spread(&k) - radius * radius).abs() < 2e-5, "variance at {radius}: {}", spread(&k)); + } + for radius in [1.0, 1.5, 3.0, 4.0] { + assert_eq!(gaussian_blur_kernel(radius), gaussian_kernel(radius), "{radius}"); + } + assert_eq!(gaussian_blur_kernel(0.0), vec![1.0]); +} + #[test] fn radial_blur_samples_large_arcs_beyond_64_intervals() { let bounds = Rect::new(0, 0, 1000, 1000); diff --git a/crates/algo/src/lib.rs b/crates/algo/src/lib.rs index fe9235c729..03ffe38144 100644 --- a/crates/algo/src/lib.rs +++ b/crates/algo/src/lib.rs @@ -566,6 +566,9 @@ impl FilterParams { pub fn halo(&self) -> Halo { let g = |sigma: f32| Halo::Radius(((sigma.max(0.0) * 3.0).ceil() as i32).saturating_add(1)); match self { + // Small Gaussian radii use the calibrated seven-tap kernel as well: its reach is + // three pixels even when ceil(3 * radius) + 1 would be smaller. + FilterParams::GaussianBlur { radius } | FilterParams::HighPass { radius } if (0.05..1.0).contains(radius) => Halo::Radius(3), FilterParams::GaussianBlur { radius } | FilterParams::HighPass { radius } => g(*radius), FilterParams::UnsharpMask { radius, .. } => Halo::Radius(sharpen::exact::halo(*radius)), FilterParams::SmartSharpen { radius, .. } => g(*radius), diff --git a/crates/algo/src/separable.rs b/crates/algo/src/separable.rs index 7ab4705671..244a27a55a 100644 --- a/crates/algo/src/separable.rs +++ b/crates/algo/src/separable.rs @@ -25,7 +25,7 @@ impl Separable { match *params { FilterParams::GaussianBlur { radius } => Some(match blur::box_widths(params) { Some(boxes) => Separable::Boxes(boxes), - None => Separable::Kernel(blur::gaussian_kernel(radius)), + None => Separable::Kernel(blur::gaussian_blur_kernel(radius)), }), _ => None, } @@ -75,7 +75,7 @@ mod tests { use photocraft_color::{ColorMode, PixelFormat, SampleType}; use photocraft_raster::{Interrupt, Surface}; - const RADII: [f32; 9] = [0.01, 0.3, 0.8, 2.0, 4.0, 4.5, 12.0, 40.0, 250.0]; + const RADII: [f32; 11] = [0.01, 0.05, 0.1, 0.3, 0.8, 2.0, 4.0, 4.5, 12.0, 40.0, 250.0]; /// The CPU blur's own choice: the exact kernel up to radius 4, boxes beyond; and its halo /// (how far the CPU grows an output area) covers the reach. @@ -87,7 +87,7 @@ mod tests { match &blur { Separable::Kernel(k) => { assert!(radius <= 4.0, "radius {radius}"); - assert_eq!(k, &blur::gaussian_kernel(radius)); + assert_eq!(k, &blur::gaussian_blur_kernel(radius)); assert_eq!(k.len() % 2, 1); assert!((k.iter().sum::() - 1.0).abs() < 1e-5); } @@ -135,8 +135,17 @@ mod tests { // Nothing past the reach; the exact kernel reaches all of it. (A single pixel's box // tails are smaller than the running sums' residue, so they aren't checked for that.) assert!(dot.inflate(reach).contains_rect(&touched), "radius {radius}: {touched:?} past the reach {reach}"); - if matches!(Separable::of(¶ms), Some(Separable::Kernel(_))) { - assert_eq!(touched, dot.inflate(reach), "radius {radius}"); + if let Some(Separable::Kernel(k)) = Separable::of(¶ms) { + // Calibrated subpixel kernels have tails below the visibility threshold. + let center = k.len() / 2; + let visible = k + .iter() + .enumerate() + .filter(|(_, w)| **w * k[center] > 1e-6) + .map(|(i, _)| i.abs_diff(center)) + .max() + .unwrap_or(0); + assert_eq!(touched, dot.inflate(visible as i32), "radius {radius}"); } } } diff --git a/crates/compose/src/pattern.rs b/crates/compose/src/pattern.rs index 258ef5664a..b351bdef06 100644 --- a/crates/compose/src/pattern.rs +++ b/crates/compose/src/pattern.rs @@ -8,16 +8,27 @@ //! minified tile is box-filtered instead of aliased; at 100 % and above it is a single tap. use std::collections::HashMap; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex, OnceLock, Weak}; use photocraft_doc::{ColorMode, Pattern, PixelFormat, Rect, SampleType}; -pub(crate) const PREPARED_PATTERN_BYTES: usize = 64 << 20; +/// What one rendering call may keep of its patterns' working copies: every pattern it paints with +/// stays converted for the call (four of the largest 8-bit ones on desktop), so none is converted +/// once per output tile. A ceiling, not an allocation: only the patterns a call paints with are +/// converted. Past it a pattern is converted under one lock and shared while in use (see +/// [`PreparedPatterns::get`]). The web build keeps a quarter of that. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) const PREPARED_PATTERN_BYTES: usize = 1 << 30; +/// See the desktop value above. +#[cfg(target_arch = "wasm32")] +pub(crate) const PREPARED_PATTERN_BYTES: usize = 256 << 20; const MAX_PREPARED_PATTERNS: usize = 64; -/// Absolute cap on a single pattern's RGBA conversion on the uncached path. A pattern this -/// large cannot be cached, so re-expanding it for every output tile would be a DoS; above the -/// cap it is skipped instead. Independent of the cache budget so small patterns still convert. -const MAX_UNCACHED_PATTERN_BYTES: usize = 256 << 20; +/// Absolute cap on a single pattern's working copy on the uncached path: the document's own cap +/// on what paints (`Pattern::is_paintable`). A pattern above it is skipped, not converted. +const MAX_UNCACHED_PATTERN_BYTES: usize = photocraft_doc::pattern::MAX_WORKING_BYTES as usize; +// A call keeps at least two of the largest, or a second large fill is converted for every tile. +const _: () = assert!(2 * photocraft_doc::pattern::MAX_WORKING_BYTES <= PREPARED_PATTERN_BYTES as u64); type PreparedSlot = Arc>>>; struct PreparedState { @@ -25,6 +36,15 @@ struct PreparedState { reserved_bytes: usize, } +/// Patterns past the call's budget: converted one at a time under this lock, and shared by the +/// workers using one at the same moment (a `Weak` per pattern), so the extra copies in memory are +/// the patterns in use, not one per worker. +#[derive(Default)] +struct OverBudget { + live: Mutex>>, + warned: AtomicBool, +} + /// Demand-prepared pixels owned by one rendering call, never by the document. pub(crate) struct PreparedPatterns<'a> { patterns: &'a [Pattern], @@ -32,11 +52,18 @@ pub(crate) struct PreparedPatterns<'a> { /// The colour mode the patterns are painted into (see [`pattern_rgba`]). mode: ColorMode, state: Mutex, + over_budget: OverBudget, } impl<'a> PreparedPatterns<'a> { pub(crate) fn new(patterns: &'a [Pattern], budget: usize) -> Self { - Self { patterns, budget, mode: ColorMode::Rgb, state: Mutex::new(PreparedState { slots: HashMap::new(), reserved_bytes: 0 }) } + Self { + patterns, + budget, + mode: ColorMode::Rgb, + state: Mutex::new(PreparedState { slots: HashMap::new(), reserved_bytes: 0 }), + over_budget: OverBudget::default(), + } } /// Patterns painted into a document of colour mode `mode`. @@ -54,10 +81,7 @@ impl<'a> PreparedPatterns<'a> { if !p.is_paintable() { return None; } - let bytes = usize::try_from(p.width) - .ok() - .and_then(|w| usize::try_from(p.height).ok().and_then(|h| w.checked_mul(h))) - .and_then(|pixels| pixels.checked_mul(std::mem::size_of::<[f32; 4]>())); + let bytes = usize::try_from(p.working_bytes()).ok(); // The borrowed immutable slice pins these addresses for the complete call. let key = std::ptr::from_ref(p) as usize; let slot = { @@ -84,14 +108,40 @@ impl<'a> PreparedPatterns<'a> { // Run on the requesting worker to preserve its active CMYK profile. slot.get_or_init(|| Tile::new_in(p, self.mode).map(Arc::new)).clone() } - // Uncached path: a pattern whose full conversion would exceed an absolute cap is - // not materialized at all. Otherwise every output tile would re-expand the entire - // sparse pattern into RGBA floats (and could do so concurrently), exhausting CPU and - // memory even though cache admission already refused to retain it. The cap is - // independent of the (possibly zero) cache budget, so small patterns still convert - // on demand when nothing is cached. + // Uncached path: a pattern whose working copy would exceed an absolute cap is not + // materialized at all. One past the call's budget is converted under one lock, and + // shared while some worker still holds it, so workers painting tiles at the same + // time do not each convert their own copy (CPU and memory), and it says so once. + // The cap is independent of the (possibly zero) cache budget, so small patterns + // still convert on demand when nothing is cached. None if bytes.is_none_or(|b| b > MAX_UNCACHED_PATTERN_BYTES) => None, - None => Tile::new_in(p, self.mode).map(Arc::new), + None => { + // The lock is held through the conversion, so conversions of different + // over-budget patterns run one at a time. That is the point (one extra copy + // being built at once, not one per worker) and fine for a fallback: patterns + // that fit the budget never come here. + let mut live = self.over_budget.live.lock().unwrap_or_else(|e| e.into_inner()); + if let Some(shared) = live.get(&key).and_then(Weak::upgrade) { + return Some(shared); + } + if !self.over_budget.warned.swap(true, Ordering::Relaxed) { + // Written, not printed: a warning must never panic, even with no stderr. + #[cfg(not(target_arch = "wasm32"))] + { + use std::io::Write as _; + let _ = writeln!( + std::io::stderr(), + "photocraft: pattern \"{}\" is past this render's pattern memory ({} MiB) and is converted again as tiles need it", + p.display_name(), + self.budget >> 20 + ); + } + } + let tile = Tile::new_in(p, self.mode).map(Arc::new)?; + live.retain(|_, w| w.strong_count() > 0); + live.insert(key, Arc::downgrade(&tile)); + Some(tile) + } } } } @@ -103,29 +153,80 @@ impl<'a> PreparedPatterns<'a> { /// pattern in colour in its picker, filled in grey). Empty for a pattern that isn't /// [paintable](Pattern::is_paintable). pub fn pattern_rgba(p: &Pattern, mode: ColorMode) -> Vec<[f32; 4]> { - let Some(n) = p.is_paintable().then(|| usize::try_from(p.pixel_count()).ok()).flatten() else { - return Vec::new(); - }; - let mut px = vec![[0.0f32; 4]; n]; - p.surface.read_rgba_into(p.rect(), &mut px); - let paints_in = matches!(mode, ColorMode::Grayscale | ColorMode::Bitmap | ColorMode::Duotone | ColorMode::Cmyk); - if paints_in && p.surface.format().mode != mode { - let fmt = PixelFormat::new(mode, SampleType::F32, true); - let mut v = [0.0f32; 5]; - for q in &mut px { - let n = photocraft_raster::from_rgba_into(&fmt, *q, &mut v); - *q = photocraft_raster::to_rgba(&fmt, v.get(..n).unwrap_or_default()); - } + let mut px = Vec::new(); + if p.is_paintable() && pattern_rgba_rows(p, mode, 0, p.height, &mut px).is_none() { + px = Vec::new(); } px } -/// A pattern converted once to straight-alpha RGBA for sampling. +/// Rows `top..bottom` of [`pattern_rgba`] (of a paintable pattern), replacing `out`: a large +/// pattern is read a band at a time instead of as one copy at 16 bytes per pixel. `None` when the +/// memory for the rows cannot be had (on the web a failed allocation aborts the tab, so it is +/// asked for, not assumed). +pub fn pattern_rgba_rows(p: &Pattern, mode: ColorMode, top: u32, bottom: u32, out: &mut Vec<[f32; 4]>) -> Option<()> { + let bottom = bottom.min(p.height); + let top = top.min(bottom); + out.clear(); + let pixels = (p.width as usize).checked_mul((bottom - top) as usize)?; + out.try_reserve_exact(pixels).ok()?; + out.resize(pixels, [0.0f32; 4]); + p.surface.read_rgba_into(Rect::new(0, top as i32, p.width as i32, bottom as i32), out); + let paints_in = matches!(mode, ColorMode::Grayscale | ColorMode::Bitmap | ColorMode::Duotone | ColorMode::Cmyk); + if paints_in && p.surface.format().mode != mode { + let fmt = PixelFormat::new(mode, SampleType::F32, true); + let mut v = [0.0f32; 5]; + for q in out.iter_mut() { + let n = photocraft_raster::from_rgba_into(&fmt, *q, &mut v); + *q = photocraft_raster::to_rgba(&fmt, v.get(..n).unwrap_or_default()); + } + } + Some(()) +} + +/// The pixels of a paintable pattern as they paint in a document of colour mode `mode`, rounded +/// to the pattern's own sample grid by `quantize`, read a band of rows at a time: no full-size +/// `f32` copy. A pattern with its own colours comes through exactly (its samples are on that +/// grid); one converted to a grey or CMYK document is rounded to the grid it was drawn on. `None` +/// when the memory cannot be had. +fn quantized_rgba(p: &Pattern, mode: ColorMode, quantize: impl Fn(f32) -> T) -> Option> { + let mut out = Vec::new(); + out.try_reserve_exact((p.width as usize).checked_mul(p.height as usize)?).ok()?; + let mut band = Vec::new(); + for top in (0..p.height).step_by(photocraft_doc::pattern::BAND_ROWS as usize) { + pattern_rgba_rows(p, mode, top, top + photocraft_doc::pattern::BAND_ROWS, &mut band)?; + out.extend(band.iter().map(|c| [quantize(c[0]), quantize(c[1]), quantize(c[2]), quantize(c[3])])); + } + Some(out) +} + +/// `u8 / 255` as `f32`, for every `u8`: the value an 8-bit sample reads as. +const UNIT8: [f32; 256] = { + let mut table = [0.0f32; 256]; + let mut i = 0; + while i < 256 { + table[i] = i as f32 / 255.0; + i += 1; + } + table +}; + +/// A pattern's pixels as the sampler holds them. +enum Texels { + /// Premultiplied `f32` RGBA, 16 bytes per pixel: any pattern that fits. + Premultiplied(Vec<[f32; 4]>), + /// Straight RGBA8, 4 bytes per pixel, premultiplied as each tap reads it (see [`UNIT8`]): a + /// large 8-bit pattern, with the values the expanded copy would hold. + Rgba8(Vec<[u8; 4]>), + /// Straight RGBA16, 8 bytes per pixel, premultiplied as each tap reads it: a large 16-bit one. + Rgba16(Vec<[u16; 4]>), +} + +/// A pattern converted once for sampling. pub struct Tile { w: usize, h: usize, - /// Premultiplied RGBA. - px: Vec<[f32; 4]>, + texels: Texels, } impl Tile { @@ -139,21 +240,51 @@ impl Tile { if !p.is_paintable() { return None; } + Tile::build(p, mode, p.pixel_count().saturating_mul(16) > photocraft_doc::pattern::MAX_WORKING_BYTES) + } + + /// [`Tile::new_in`], held at the pattern's own depth when `compact` and it has a smaller form. + fn build(p: &Pattern, mode: ColorMode, compact: bool) -> Option { let (w, h) = (p.width as usize, p.height as usize); - let mut px = pattern_rgba(p, mode); - for q in &mut px { - for c in 0..3 { - q[c] *= q[3]; + let texels = match p.surface.format().sample { + // A large pattern is held at its own depth (`Pattern::working_bytes`). + SampleType::U8 if compact => Texels::Rgba8(quantized_rgba(p, mode, |v| (v.clamp(0.0, 1.0) * 255.0 + 0.5) as u8)?), + SampleType::U16 if compact => Texels::Rgba16(quantized_rgba(p, mode, |v| (v.clamp(0.0, 1.0) * 65535.0 + 0.5) as u16)?), + _ => { + let mut px = pattern_rgba(p, mode); + if px.len() as u64 != p.pixel_count() { + return None; + } + for q in &mut px { + for c in 0..3 { + q[c] *= q[3]; + } + } + Texels::Premultiplied(px) } - } - Some(Tile { w, h, px }) + }; + Some(Tile { w, h, texels }) } #[inline] fn at(&self, x: i64, y: i64) -> [f32; 4] { let xi = x.rem_euclid(self.w as i64) as usize; let yi = y.rem_euclid(self.h as i64) as usize; - self.px[yi * self.w + xi] + let i = yi * self.w + xi; + match &self.texels { + Texels::Premultiplied(px) => px.get(i).copied().unwrap_or_default(), + Texels::Rgba8(px) => { + let [r, g, b, a] = px.get(i).copied().unwrap_or_default(); + let a = UNIT8[usize::from(a)]; + [UNIT8[usize::from(r)] * a, UNIT8[usize::from(g)] * a, UNIT8[usize::from(b)] * a, a] + } + Texels::Rgba16(px) => { + let [r, g, b, a] = px.get(i).copied().unwrap_or_default(); + let unit = |v: u16| f32::from(v) / 65535.0; + let a = unit(a); + [unit(r) * a, unit(g) * a, unit(b) * a, a] + } + } } /// Bilinear, wrapping sample at continuous tile coordinates (pixel centres at `i + 0.5`). @@ -329,6 +460,114 @@ mod tests { assert!(PreparedPatterns::new(&list, 0).get("big", "").is_none()); } + /// Review of #8902: what `Pattern::is_paintable` admits, the compositor expands. A pattern that + /// fits is expanded as before; a large one is held at its own depth, so Define Pattern's + /// 8000 x 8000 fits at 8 bits. (Only the size is declared: the patterns themselves are empty.) + #[test] + fn the_most_paintable_pixels_still_expand() { + let sparse = |name: &str, (width, height): (u32, u32), format: PixelFormat| Pattern { + id: name.into(), + name: name.into(), + width, + height, + surface: Surface::new(format), + }; + let float = PixelFormat::new(ColorMode::Rgb, SampleType::F32, true); + let list = [ + sparse("max8", (8000, 8000), PixelFormat::RGBA8), + sparse("max16", (5792, 5792), PixelFormat::RGBA16), + sparse("max32", (4096, 4096), float), + sparse("over8", (9000, 8000), PixelFormat::RGBA8), + sparse("over16", (8000, 8000), PixelFormat::RGBA16), + sparse("over32", (4097, 4096), float), + ]; + assert_eq!(list.each_ref().map(|p| p.is_paintable()), [true, true, true, false, false, false]); + let prepared = PreparedPatterns::new(&list, PREPARED_PATTERN_BYTES); + let held = |name: &str| prepared.get(name, "").map(|t| (matches!(t.texels, Texels::Rgba8(_)), matches!(t.texels, Texels::Rgba16(_)))); + assert_eq!(held("max8"), Some((true, false)), "an 8-bit 8000 x 8000 pattern expands to 4 bytes a pixel"); + assert_eq!(held("max16"), Some((false, true)), "a 16-bit one to 8"); + for name in ["over8", "over16", "over32"] { + assert!(prepared.get(name, "").is_none(), "{name} is skipped, not allocated"); + } + assert!(PreparedPatterns::new(&list, 0).get("max8", "").is_some(), "uncached too, at the cap"); + } + + /// A render that paints with two large patterns (a pattern fill and a pattern overlay, say) + /// converts each once, not once per output tile: both stay in the call's budget. A third that + /// does not fit is converted when asked for (correct, just not kept). + #[test] + fn two_large_patterns_are_each_converted_once_per_call() { + let sparse = |name: &str| Pattern { id: name.into(), name: name.into(), width: 5000, height: 5000, surface: Surface::new(PixelFormat::RGBA8) }; + let list = [sparse("a"), sparse("b"), sparse("c")]; + assert!(list.iter().all(|p| p.working_bytes() == 100_000_000)); + let prepared = PreparedPatterns::new(&list, 250_000_000); + let same = |name: &str| { + let (first, second) = (prepared.get(name, "").unwrap(), prepared.get(name, "").unwrap()); + Arc::ptr_eq(&first, &second) + }; + assert!(same("a"), "the first is kept"); + assert!(same("b"), "and the second"); + // A third past the budget is converted under one lock and shared while a worker holds it, + // so concurrent tiles use one copy; it is converted again once nobody holds it. + assert!(same("c"), "shared while in use"); + let held = prepared.get("c", "").unwrap(); + let other = prepared.get("c", "").unwrap(); + assert!(Arc::ptr_eq(&held, &other), "two workers at once get one copy"); + drop((held, other)); + assert!(prepared.get("c", "").is_some(), "and it converts again after"); + // The real budget holds two of the largest paintable ones. + assert!(2 * photocraft_doc::pattern::MAX_WORKING_BYTES <= PREPARED_PATTERN_BYTES as u64); + } + + /// A large pattern's compact copy samples exactly as the expanded one: every 8-bit and 16-bit + /// value survives the round trip through `f32`, and a tap reads the same premultiplied numbers. + #[test] + fn the_compact_copies_read_the_same_as_the_expanded_one() { + for v in 0..=255u8 { + assert_eq!(((f32::from(v) / 255.0) * 255.0 + 0.5) as u8, v); + assert_eq!(UNIT8[usize::from(v)], f32::from(v) / 255.0); + } + for v in 0..=u16::MAX { + assert_eq!(((f32::from(v) / 65535.0) * 65535.0 + 0.5) as u16, v); + } + for (format, grid) in [(PixelFormat::RGBA8, 255.0f32), (PixelFormat::RGBA16, 65535.0)] { + let mut s = Surface::new(format); + for y in 0..16i32 { + for x in 0..16i32 { + let level = |v: i32| (v as f32 / 255.0 * grid).round() / grid; + s.fill_rect(Rect::new(x, y, x + 1, y + 1), &[level(x * 17), level(y * 17), level((x + y) * 8), level(255 - x * 13 - y * 3)]); + } + } + let p = Pattern::new("ramp", s, 16, 16); + let (expanded, compact) = (Tile::build(&p, ColorMode::Rgb, false).unwrap(), Tile::build(&p, ColorMode::Rgb, true).unwrap()); + assert!(matches!(expanded.texels, Texels::Premultiplied(_))); + assert!(matches!((&compact.texels, grid as u32), (Texels::Rgba8(_), 255) | (Texels::Rgba16(_), 65535))); + for y in 0..16 { + for x in 0..16 { + assert_eq!(compact.at(x, y), expanded.at(x, y), "{grid} ({x}, {y})"); + } + } + // And rendered, scaled, rotated and shifted across the wrap. + let place = Placement::new(Rect::new(3, 5, 40, 40), true, (2.0, 1.0), 0.6, 20.0); + assert_eq!(render(&compact, &place, Rect::new(-9, -7, 30, 25)), render(&expanded, &place, Rect::new(-9, -7, 30, 25))); + } + } + + /// Memory for the rows is asked for, not assumed: a pattern whose rows cannot be had (here a + /// declared size past what any allocation could be) gives `None`, never an aborted allocation. + #[test] + fn rows_that_cannot_be_allocated_give_none() { + let huge = Pattern { id: "huge".into(), name: "huge".into(), width: u32::MAX, height: u32::MAX, surface: Surface::new(PixelFormat::RGBA8) }; + let mut out = Vec::new(); + assert!(pattern_rgba_rows(&huge, ColorMode::Rgb, 0, u32::MAX, &mut out).is_none()); + assert!(quantized_rgba(&huge, ColorMode::Rgb, |v| v as u8).is_none()); + assert!(pattern_rgba(&huge, ColorMode::Rgb).is_empty()); + assert!(Tile::new(&huge).is_none()); + let small = Pattern { id: "small".into(), name: "small".into(), width: 3, height: 2, surface: Surface::new(PixelFormat::RGBA8) }; + assert_eq!(pattern_rgba_rows(&small, ColorMode::Rgb, 0, 2, &mut out), Some(())); + assert_eq!(out.len(), 6); + } + #[test] fn link_phase_scale_and_angle_move_the_origin() { let t = Tile::new(&checker()).unwrap(); diff --git a/crates/doc/src/pattern.rs b/crates/doc/src/pattern.rs index 9a43c0a467..7d46274638 100644 --- a/crates/doc/src/pattern.rs +++ b/crates/doc/src/pattern.rs @@ -2,16 +2,30 @@ //! and brush textures. Pure data; rendering lives in `photocraft-compose`, the PSD mapping //! (`Patt`/`Pat2`/`Pat3` global blocks) in `photocraft-io`. +use photocraft_color::SampleType; use photocraft_geom::Rect; use photocraft_raster::Surface; -/// Largest pattern edge a file may declare (the PSD reader's limit; Define Pattern stops at 4000). +/// Largest pattern edge a file may declare (the PSD reader's limit; Define Pattern stops at 8000). pub const MAX_EDGE: u32 = 30_000; -/// Most pixels a pattern may have to be painted. Renderers expand a pattern into a 16-byte-per-pixel -/// working copy, so this keeps that copy at 1 GiB; larger patterns paint nothing. +/// Most pixels a pattern may have to be painted (Define Pattern's 8000 × 8000 fits). Larger +/// patterns paint nothing. pub const MAX_PAINT_PIXELS: u64 = 1 << 26; +/// Most bytes of working copy the CPU compositor holds for one pattern (`photocraft-compose` +/// checks the two agree at build time). See [`Pattern::working_bytes`]. A browser tab has far less +/// memory than a desktop process, and a failed allocation there aborts the tab, so the web build +/// holds half as much: an 8-bit pattern paints up to 33,554,432 px (about 5792²) there. +#[cfg(not(target_arch = "wasm32"))] +pub const MAX_WORKING_BYTES: u64 = 256 << 20; +/// See the desktop value above. +#[cfg(target_arch = "wasm32")] +pub const MAX_WORKING_BYTES: u64 = 128 << 20; + +/// Rows of a pattern read at a time where a whole-tile `f32` copy would cost 16 bytes per pixel. +pub const BAND_ROWS: u32 = 256; + /// One pattern tile. Pixels cover `0..width × 0..height` of `surface`, in any pixel format /// (depth and colour model are the pattern's own, as in Photoshop). #[derive(Clone, Debug, PartialEq)] @@ -36,13 +50,18 @@ impl Pattern { /// defining a pattern is deterministic and replayable). pub fn new(name: impl Into, surface: Surface, width: u32, height: u32) -> Pattern { let name = name.into(); - let r = Rect::new(0, 0, width as i32, height as i32); - let px = surface.read_region(r); let mut h1 = fnv(0xcbf2_9ce4_8422_2325, name.as_bytes()); h1 = fnv(h1, &width.to_le_bytes()); h1 = fnv(h1, &height.to_le_bytes()); - for v in &px { - h1 = fnv(h1, &v.to_bits().to_le_bytes()); + // The samples row by row, a band at a time: the same sequence as one read of the whole + // tile, without a full-size `f32` copy of a large pattern. + let mut band = Vec::new(); + for top in (0..height).step_by(BAND_ROWS as usize) { + let bottom = (top + BAND_ROWS).min(height); + surface.read_region_into(Rect::new(0, top as i32, width as i32, bottom as i32), &mut band); + for v in &band { + h1 = fnv(h1, &v.to_bits().to_le_bytes()); + } } let h2 = fnv(h1 ^ 0x9e37_79b9_7f4a_7c15, format!("{:?}", surface.format()).as_bytes()); let id = format!("{:08x}-{:04x}-{:04x}-{:04x}-{:012x}", (h1 >> 32) as u32, (h1 >> 16) as u16, h1 as u16, (h2 >> 48) as u16, h2 & 0xffff_ffff_ffff); @@ -70,9 +89,61 @@ impl Pattern { u64::from(self.width) * u64::from(self.height) } - /// Whether the tile has pixels and is small enough to paint (see [`MAX_PAINT_PIXELS`]). + /// Bytes of working copy the compositor holds to paint this pattern. One that fits is expanded + /// to premultiplied `f32` RGBA (16 per pixel). A pattern too large for that is held at its own + /// depth instead, straight RGBA at 4 bytes per pixel for 8 bits and 8 for 16 bits: exactly its + /// values, in a quarter or half the memory. A float one has no smaller exact form. + pub fn working_bytes(&self) -> u64 { + working_bytes_for(self.pixel_count(), self.surface.format().sample) + } + + /// Why this pattern paints nothing, for the person or agent who chose it; `None` when it + /// paints. A pattern too large for the compositor's working copy renders as transparent, so + /// every command that picks a pattern, and the import that brought it in, says so. + pub fn paint_problem(&self) -> Option { + if self.is_paintable() { + return None; + } + if self.is_empty() { + return Some("it is empty".into()); + } + let sample = self.surface.format().sample; + Some(format!( + "it is {} × {} px at {} bits per channel, and a pattern of that depth paints up to {} px in all", + self.width, + self.height, + sample.bytes() * 8, + max_paint_pixels(sample) + )) + } + + /// Whether the tile has pixels and is small enough to paint (see [`MAX_PAINT_PIXELS`] and + /// [`MAX_WORKING_BYTES`]). pub fn is_paintable(&self) -> bool { - !self.is_empty() && self.pixel_count() <= MAX_PAINT_PIXELS + !self.is_empty() && self.pixel_count() <= MAX_PAINT_PIXELS && self.working_bytes() <= MAX_WORKING_BYTES + } +} + +/// [`Pattern::working_bytes`] of a pattern of `pixels` pixels with `sample` samples. +pub fn working_bytes_for(pixels: u64, sample: SampleType) -> u64 { + let expanded = pixels.saturating_mul(16); + if expanded <= MAX_WORKING_BYTES { + return expanded; + } + match sample { + SampleType::U8 => pixels.saturating_mul(4), + SampleType::U16 => pixels.saturating_mul(8), + SampleType::F32 => expanded, + } +} + +/// Most pixels a pattern with `sample` samples may have and still paint: [`MAX_PAINT_PIXELS`] for +/// 8 bits, fewer where the working copy is larger per pixel. +pub fn max_paint_pixels(sample: SampleType) -> u64 { + match sample { + SampleType::U8 => MAX_PAINT_PIXELS, + SampleType::U16 => MAX_PAINT_PIXELS.min(MAX_WORKING_BYTES / 8), + SampleType::F32 => MAX_PAINT_PIXELS.min(MAX_WORKING_BYTES / 16), } } @@ -118,4 +189,61 @@ mod tests { assert!(find(&list, "nope", "water").is_some()); assert!(find(&list, "nope", "").is_none()); } + + /// The id is a hash of every sample in row-major order; reading a tall pattern a band of rows + /// at a time must give the id one whole read gave, or saved documents would lose their + /// pattern links. + #[test] + fn the_id_of_a_tall_pattern_matches_a_single_read_of_it() { + let (w, h) = (7u32, BAND_ROWS * 2 + 37); + let mut s = Surface::new(PixelFormat::RGBA8); + for y in 0..h as i32 { + for x in 0..w as i32 { + s.fill_rect(Rect::new(x, y, x + 1, y + 1), &[(x % 5) as f32 / 4.0, (y % 7) as f32 / 6.0, ((x + y) % 3) as f32 / 2.0, 1.0]); + } + } + let name = "tall"; + let mut h1 = fnv(0xcbf2_9ce4_8422_2325, name.as_bytes()); + h1 = fnv(h1, &w.to_le_bytes()); + h1 = fnv(h1, &h.to_le_bytes()); + for v in s.read_region(Rect::new(0, 0, w as i32, h as i32)) { + h1 = fnv(h1, &v.to_bits().to_le_bytes()); + } + let h2 = fnv(h1 ^ 0x9e37_79b9_7f4a_7c15, format!("{:?}", s.format()).as_bytes()); + let expected = + format!("{:08x}-{:04x}-{:04x}-{:04x}-{:012x}", (h1 >> 32) as u32, (h1 >> 16) as u16, h1 as u16, (h2 >> 48) as u16, h2 & 0xffff_ffff_ffff); + assert_eq!(Pattern::new(name, s, w, h).id, expected); + } + + #[test] + fn working_bytes_keep_a_pattern_exact_in_less_memory() { + let sparse = + |format: PixelFormat, width: u32, height: u32| Pattern { id: String::new(), name: String::new(), width, height, surface: Surface::new(format) }; + let float = PixelFormat::new(photocraft_color::ColorMode::Rgb, SampleType::F32, true); + // What fits is expanded as before. + assert_eq!(sparse(PixelFormat::RGBA8, 10, 10).working_bytes(), 1600); + assert_eq!(sparse(float, 4096, 4096).working_bytes(), MAX_WORKING_BYTES); + // Past that, a pattern is held at its own depth. + assert_eq!(sparse(PixelFormat::RGBA8, 8000, 8000).working_bytes(), 256_000_000); + assert_eq!(sparse(PixelFormat::RGBA16, 5000, 5000).working_bytes(), 200_000_000); + assert!(sparse(PixelFormat::RGBA8, 8000, 8000).is_paintable()); + assert!(sparse(PixelFormat::RGBA16, 5792, 5792).is_paintable()); + assert!(!sparse(PixelFormat::RGBA16, 8000, 8000).is_paintable(), "512 MB at 16 bits"); + assert!(!sparse(float, 4097, 4096).is_paintable(), "float has no smaller exact form"); + assert!(!sparse(PixelFormat::RGBA8, 9000, 8000).is_paintable()); + assert!(!sparse(PixelFormat::RGBA8, 0, 8).is_paintable()); + } + + #[test] + fn a_pattern_that_paints_nothing_says_why() { + let sparse = + |format: PixelFormat, width: u32, height: u32| Pattern { id: String::new(), name: String::new(), width, height, surface: Surface::new(format) }; + let float = PixelFormat::new(photocraft_color::ColorMode::Rgb, SampleType::F32, true); + assert_eq!(sparse(PixelFormat::RGBA8, 8000, 8000).paint_problem(), None); + assert_eq!(sparse(PixelFormat::RGBA8, 0, 4).paint_problem().as_deref(), Some("it is empty")); + let reason = sparse(float, 5000, 5000).paint_problem().expect("float 5000 x 5000 paints nothing"); + assert!(reason.contains("5000 × 5000") && reason.contains("32 bits") && reason.contains("16777216"), "{reason}"); + let reason = sparse(PixelFormat::RGBA16, 8000, 8000).paint_problem().expect("16-bit 8000 x 8000 paints nothing"); + assert!(reason.contains("16 bits") && reason.contains("33554432"), "{reason}"); + } } diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 1598870479..aeb41bdad3 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -168,7 +168,7 @@ pub(crate) use layer_params::{NEW_LAYER_PATH, explicit_layer, layer_listing, lay use ops::{arrange, merge_group, set_mask, set_mask_and_deselect}; pub(crate) use ops::{layer_copy, translate_layer}; use params::{bad, set_clipped}; -pub(crate) use params::{check_f32_values, color_param, int, int_i32, parse_hex, u32_id, u32_id_param}; +pub(crate) use params::{check_f32_values, cmyk_param, color_param, int, int_i32, parse_hex, u32_id, u32_id_param}; pub fn command_specs() -> &'static [CommandSpec] { ®istry().specs @@ -778,13 +778,22 @@ fn build() -> Vec { "Solid Color…", ["Layer", "New Fill Layer"], None, - r##"{"color":"#rrggbb"=foreground,"path":"work"|saved path name|{…path}? (the active path: becomes the vector mask, making a shape; else the selection is the layer mask)}"##, + r##"{"color":"#rrggbb"=foreground,"cmyk":[c,m,y,k]? (inks 0..1 the colour was picked as: a CMYK document's fill holds exactly these; default the foreground's CMYK pick),"path":"work"|saved path name|{…path}? (the active path: becomes the vector mask, making a shape; else the selection is the layer mask)}"##, has_doc, |s, p| { let c = color_param("layer.newFillLayer.solidColor", p, "color", s.tools.foreground)?; + // The inks the colour was picked as, so CMYK 50/50/50/100 stays 50/50/50/100 instead + // of what its sRGB stand-in converts back to through the document profile (#8568). + let inks = match cmyk_param("layer.newFillLayer.solidColor", p, "cmyk")? { + Some(k) => Some(k), + None => s.tools.picked_cmyk([c[0], c[1], c[2]]), + }; let default_mask = layer_panel_option(s, "useDefaultMasksOnFillLayers", true); let id = s.edit("New Color Fill Layer", |doc, active| { - let fill = Fill::Solid(Color::rgba(c[0], c[1], c[2], c[3])).in_mode(doc.mode); + let fill = match inks { + Some(k) if doc.mode == ColorMode::Cmyk => Fill::Solid(Color { mode: ColorMode::Cmyk, c: k, alpha: c[3] }), + _ => Fill::Solid(Color::rgba(c[0], c[1], c[2], c[3])).in_mode(doc.mode), + }; let mut l = Layer::new(doc.next_layer_name("Color Fill"), LayerContent::Fill(fill)); mask_new_layer(doc, &mut l, p, "layer.newFillLayer.solidColor", default_mask)?; let id = crate::layer_place::insert_new(doc, *active, l, crate::layer_place::Place::Auto); @@ -923,17 +932,7 @@ fn build() -> Vec { r##"{"foreground":"#rrggbb"|[r,g,b,a?]?,"background":same?,"foregroundCmyk":[c,m,y,k]? (inks 0..1 the foreground was picked as: a CMYK document is filled with exactly these),"backgroundCmyk":same?}"##, always, |s, p| { - let cmyk = |key: &str| -> Result> { - let Some(v) = p.get(key).filter(|v| !v.is_null()) else { return Ok(None) }; - let inks: Option> = v - .as_array() - .filter(|a| a.len() == 4) - .and_then(|a| a.iter().map(|x| x.as_f64().filter(|x| x.is_finite()).map(|x| x.clamp(0.0, 1.0) as f32)).collect()); - match inks.as_deref() { - Some(&[c, m, y, k]) => Ok(Some([c, m, y, k])), - _ => Err(bad("tools.setColors", format!("`{key}` must be [c, m, y, k] numbers 0..1"))), - } - }; + let cmyk = |key: &str| cmyk_param("tools.setColors", p, key); let (fg_cmyk, bg_cmyk) = (cmyk("foregroundCmyk")?, cmyk("backgroundCmyk")?); let foreground = color_param("tools.setColors", p, "foreground", s.tools.foreground)?; let background = color_param("tools.setColors", p, "background", s.tools.background)?; diff --git a/crates/engine/src/cmd/actions_cmds.rs b/crates/engine/src/cmd/actions_cmds.rs index 88abe730ed..8e8d532d5b 100644 --- a/crates/engine/src/cmd/actions_cmds.rs +++ b/crates/engine/src/cmd/actions_cmds.rs @@ -410,8 +410,8 @@ fn stop(s: &mut Session, _p: &Value) -> Result { /// A step Photoshop plays as a silent no-op although its menu item is greyed: Deselect with an /// open document and nothing selected (actions often start with it to clear any selection, #6744), /// and an Arrange that can't move the layer, such as Bring to Front on a new adjustment layer that -/// is already on top (#8453). -fn noop_step(s: &Session, id: &str, params: &Value) -> bool { +/// is already on top (#8453). The CLI's `run` and `batch` scripts skip these too (#8802). +pub fn noop_step(s: &Session, id: &str, params: &Value) -> bool { let Some(d) = s.active() else { return false }; let dir = match id { "select.deselect" => return d.doc.selection.is_none(), diff --git a/crates/engine/src/cmd/brush_cmds.rs b/crates/engine/src/cmd/brush_cmds.rs index d2a885ccb4..497aec67f2 100644 --- a/crates/engine/src/cmd/brush_cmds.rs +++ b/crates/engine/src/cmd/brush_cmds.rs @@ -291,12 +291,16 @@ fn stroke_with(s: &mut Session, cmd: &str, p: &Value, label: &str, brush: BrushS return Ok(l.bounds); } let sel = doc.selection.clone(); + let table = crate::mode_cmds::paint_color_table(doc, p); let (surf, lock) = crate::channel_cmds::target_surface(doc, id, p)?; let mut brush = brush; erase_locked(&mut brush, lock, bg); if auto_erase { + // A stroke in the table's colour starts on what the foreground painted as. + let fg = table.as_ref().map_or(fg, |t| crate::mode_cmds::table_color(t, fg)); apply_auto_erase(&mut brush, surf, pts.first(), fg, bg); } + let unpainted = table.as_ref().map(|_| surf.clone()); let damage = if let Some(axis) = &symmetry { let reflected = axis.reflected_passes(&pts); let copies: Vec<_> = reflected @@ -321,6 +325,9 @@ fn stroke_with(s: &mut Session, cmd: &str, p: &Value, label: &str, brush: BrushS } else { render_stroke(surf, &brush, &pts, sel.as_ref(), lock, zoom) }; + if let (Some(table), Some(pre)) = (&table, &unpainted) { + crate::mode_cmds::keep_in_color_table(table, pre, surf, damage); + } Ok(damage) }) })?; @@ -605,9 +612,11 @@ impl LiveStroke { let base = (st.doc.id, st.revision); let mut doc = (*st.doc).clone(); let sel = doc.selection.clone(); + let table = crate::mode_cmds::paint_color_table(&doc, p); let (surf, lock) = crate::channel_cmds::target_surface(&mut doc, layer, p)?; erase_locked(&mut brush, lock, s.tools.background); if pencil && flag(p, "autoErase", false) { + let fg = table.as_ref().map_or(fg, |t| crate::mode_cmds::table_color(t, fg)); apply_auto_erase(&mut brush, surf, pts.first(), fg, s.tools.background); } let renderer = StrokeRenderer::new(&brush, Some(surf.format()), zoom); @@ -681,7 +690,7 @@ impl LiveStroke { check_coords(pts, &self.cmd)?; self.points.extend_from_slice(pts); let cmyk = self.cmyk.clone(); - photocraft_color::convert::with_cmyk_space(cmyk.as_ref(), || { + let dmg = photocraft_color::convert::with_cmyk_space(cmyk.as_ref(), || -> Result { self.renderer.push(pts); if let Some(symmetry) = &self.symmetry { for ((mirror, distinct), reflected) in self.mirrors.iter_mut().zip(&mut self.mirror_distinct).zip(symmetry.reflected_passes(pts)) { @@ -727,7 +736,13 @@ impl LiveStroke { dmg = dmg.union(&self.tail); } Ok(dmg) - }) + })?; + // The preview shows the pixels the commit keeps (see `stroke_with`). + if let Some(table) = crate::mode_cmds::paint_color_table(&self.doc, &self.params) { + let (surf, _) = crate::channel_cmds::target_surface(std::sync::Arc::make_mut(&mut self.doc), self.layer, &self.params)?; + crate::mode_cmds::keep_in_color_table(&table, &self.pre, surf, dmg); + } + Ok(dmg) } } diff --git a/crates/engine/src/cmd/layer_multi_cmds/align.rs b/crates/engine/src/cmd/layer_multi_cmds/align.rs index e7f77fc8fb..e33b11b24a 100644 --- a/crates/engine/src/cmd/layer_multi_cmds/align.rs +++ b/crates/engine/src/cmd/layer_multi_cmds/align.rs @@ -1,4 +1,4 @@ -use super::damage::layer_bounds; +use super::damage::align_bounds; use super::moves::{move_layers, with_links}; use super::selection::{selected, top_level}; use super::{Document, EngineError, LayerId, Rect, Result, Session, Value, bad, json}; @@ -44,7 +44,7 @@ impl Edge { /// Selected top-level layers that have content bounds, bottom-to-top. fn bounded_selection(doc: &Document, sel: &[LayerId]) -> Vec<(LayerId, Rect)> { - top_level(doc, sel).into_iter().filter_map(|id| Some((id, layer_bounds(doc.layer(id)?)?))).collect() + top_level(doc, sel).into_iter().filter_map(|id| Some((id, align_bounds(doc.layer(id)?, doc.bounds(), doc.pixel_format().sample)?))).collect() } pub(super) fn align(s: &mut Session, p: &Value, kind: &str) -> Result { diff --git a/crates/engine/src/cmd/layer_multi_cmds/damage.rs b/crates/engine/src/cmd/layer_multi_cmds/damage.rs index bad2821d5e..8440fab75b 100644 --- a/crates/engine/src/cmd/layer_multi_cmds/damage.rs +++ b/crates/engine/src/cmd/layer_multi_cmds/damage.rs @@ -1,3 +1,5 @@ +use photocraft_color::{PixelFormat, SampleType}; + use super::{Document, Layer, LayerContent, Rect, Session, VideoData}; /// Calls `f` with the rect of every tile that differs between two surfaces of the same format and @@ -580,8 +582,9 @@ pub(crate) fn note_insert(s: &mut Session, before: &Document) { st.last_damage = step_damage(before, &st.doc); } -/// Content bounds used by Align/Distribute: the layer's pixels (type and shape layers use their -/// rendered pixels), or the union of a group's children. Fill and adjustment layers have none. +/// Content bounds of a layer's pixels as stored (type and shape layers: their cached render), or +/// the union of a group's children. Fill and adjustment layers have none. Cheap: Layers list, +/// artboards, copy and slices call it for every layer. pub fn layer_bounds(l: &Layer) -> Option { let r = match &l.content { LayerContent::Group(g) => g.children.iter().filter_map(layer_bounds).fold(Rect::EMPTY, |a, b| a.union(&b)), @@ -589,3 +592,68 @@ pub fn layer_bounds(l: &Layer) -> Option { }; (!r.is_empty()).then_some(r) } + +/// The bounds Align and Distribute move by, in a document of bounds `canvas`: [`layer_bounds`], +/// except that a shape layer is measured over everything it draws, beyond the canvas too (its +/// cache is clipped to the canvas, #8660), by rendering it again. Only those commands pay for that +/// render, once per click; `canvas` is the frame a shape's gradients are laid out in, so an opacity +/// stop measures as it renders, and `sample` is the document's sample type (see +/// [`shape_pixel_bounds`]). +pub(crate) fn align_bounds(l: &Layer, canvas: Rect, sample: SampleType) -> Option { + let r = match &l.content { + LayerContent::Group(g) => g.children.iter().filter_map(|c| align_bounds(c, canvas, sample)).fold(Rect::EMPTY, |a, b| a.union(&b)), + LayerContent::Shape(sh) => shape_pixel_bounds(sh, canvas, sample).unwrap_or_else(|| { + let cached = l.surface().map(|s| s.content_bounds()).unwrap_or(Rect::EMPTY); + // A fill that shows everywhere covers the canvas, rendered yet or not. + if cached.is_empty() && unbounded_fill(sh) { canvas } else { cached } + }), + _ => return layer_bounds(l), + }; + (!r.is_empty()).then_some(r) +} + +/// Whether the shape's fill shows everywhere (an inverted path, or below 100 % density), so it has +/// no extent of its own. +fn unbounded_fill(sh: &photocraft_doc::ShapeLayer) -> bool { + sh.fill.is_some() && (sh.path.inverted || (sh.density.is_finite() && sh.density < 1.0)) +} + +/// Largest area [`shape_pixel_bounds`] renders to find a shape's extent; beyond it the path and +/// stroke bounds stand in. +const SHAPE_BOUNDS_MAX_PX: u64 = 64 << 20; + +/// Pixels [`shape_pixel_bounds`] renders at a time. +const SHAPE_BOUNDS_BAND_PX: u64 = 1 << 20; + +/// The pixels a shape layer draws, wherever they fall. Its cache is rendered clipped to the +/// canvas, so a shape partly off the canvas measured from it looked smaller and Align moved it +/// by the wrong amount, a little more on each click (#8660). The shape is rendered again over +/// everything it can draw (its path, stroke and feather), unclipped, and measured from that. A fill +/// that shows everywhere (an inverted path, or below 100 % density) has no extent of its own and +/// keeps the canvas-clipped measure (`None`). +pub(super) fn shape_pixel_bounds(sh: &photocraft_doc::ShapeLayer, canvas: Rect, sample: SampleType) -> Option { + if unbounded_fill(sh) { + return None; + } + let reach = photocraft_compose::shape_bounds(sh)?.inflate(1); + if u64::from(reach.width()) * u64::from(reach.height()) > SHAPE_BOUNDS_MAX_PX { + return photocraft_compose::hard_shape_bounds(sh); + } + // Gradients lay out in the document's frame (as the cached render does); the feather and the + // clip cover the shape's whole reach, beyond the canvas too. + let shape = photocraft_vector::CompiledShape::with_frames(sh, photocraft_vector::DEFAULT_TOLERANCE, reach, canvas); + // Alpha at the document's own precision, as the document stores it: an 8-bit render drops the + // faint stroke of a 16-bit or float document (the opaque body then aligns by a different + // extent than the document draws), and a float one would find alpha an 8-bit document never + // stores. A band of rows at a time keeps the working copy a few MB however large the shape. + let format = PixelFormat::new(photocraft_color::ColorMode::Grayscale, sample, true); + let band_rows = i32::try_from((SHAPE_BOUNDS_BAND_PX / u64::from(reach.width().max(1))).max(1)).unwrap_or(i32::MAX); + let mut bounds = Rect::EMPTY; + let mut top = reach.y0; + while top < reach.y1 { + let bottom = top.saturating_add(band_rows).min(reach.y1); + bounds = bounds.union(&shape.render(format, Rect::new(reach.x0, top, reach.x1, bottom)).content_bounds()); + top = bottom; + } + (!bounds.is_empty()).then_some(bounds) +} diff --git a/crates/engine/src/cmd/layer_multi_cmds/layers_panel_parity_tests.rs b/crates/engine/src/cmd/layer_multi_cmds/layers_panel_parity_tests.rs index 3982eb9cff..f58ea7d6ae 100644 --- a/crates/engine/src/cmd/layer_multi_cmds/layers_panel_parity_tests.rs +++ b/crates/engine/src/cmd/layer_multi_cmds/layers_panel_parity_tests.rs @@ -242,16 +242,18 @@ fn fill_layers_without_default_masks_keep_selection_and_explicit_path_masks() { assert!(mask.is_none() && !vector, "{cmd}: no selection, no path: no mask at all"); } - s.execute("select.rect", json!({"x": 4, "y": 4, "width": 8, "height": 6})).unwrap(); for (cmd, params) in KINDS { + s.execute("select.rect", json!({"x": 4, "y": 4, "width": 8, "height": 6})).unwrap(); let (mask, vector) = create(&mut s, cmd, params, None); let mask = mask.unwrap_or_else(|| panic!("{cmd}: the selection still becomes the layer mask")); assert!(!vector, "{cmd}"); assert!(mask.surface.pixel(6, 6)[0] > 0.99, "{cmd}: inside the selection reveals"); assert!(mask.surface.pixel(20, 12)[0] < 0.01, "{cmd}: outside the selection hides"); + assert!(s.active().unwrap().doc.selection.is_none(), "{cmd}: the selection went into the mask (#8801)"); } // An explicit path wins over the selection, as with the option on. + s.execute("select.rect", json!({"x": 4, "y": 4, "width": 8, "height": 6})).unwrap(); for selected in [true, false] { if !selected { s.execute("select.deselect", json!({})).unwrap(); diff --git a/crates/engine/src/cmd/layer_multi_cmds/tests.rs b/crates/engine/src/cmd/layer_multi_cmds/tests.rs index a983837cc6..a3c055cf7b 100644 --- a/crates/engine/src/cmd/layer_multi_cmds/tests.rs +++ b/crates/engine/src/cmd/layer_multi_cmds/tests.rs @@ -1,3 +1,6 @@ +use photocraft_color::SampleType; + +use super::damage::{align_bounds, shape_pixel_bounds}; use super::*; #[test] @@ -636,6 +639,86 @@ fn align_each_edge() { } } +/// #8660: a shape partly off the canvas aligns by everything it draws, not the part on the +/// canvas, so one click aligns the layers and a second moves nothing. +#[test] +fn align_uses_a_shape_layers_full_extent_off_the_canvas() { + let mut s = session(8); + let drawn = |s: &Session, id: LayerId| align_bounds(doc(s).layer(id).unwrap(), doc(s).bounds(), SampleType::U8).unwrap(); + let shape = LayerId(s.execute("shape.create", json!({"kind": "rect", "rect": [-20, 10, 40, 20], "fill": "#ff0000"})).unwrap()["layer"].as_u64().unwrap()); + let measured = align_bounds(doc(&s).layer(shape).unwrap(), doc(&s).bounds(), SampleType::U8); + assert_eq!(measured, Some(Rect::new(-20, 10, 20, 30)), "Align: the shape's own extent, not the canvas-clipped render"); + // Everything else keeps the cheap measure of the cached render (no re-render per layer). + assert_eq!(bounds(&s, shape), Rect::new(0, 10, 20, 30), "layer_bounds: the cache, clipped to the canvas"); + let pixels = rect_layer(&mut s, Rect::new(30, 40, 60, 50)); + select_all(&mut s, &[shape, pixels]); + s.execute("layer.align.leftEdges", json!({})).unwrap(); + assert_eq!(bounds(&s, pixels), Rect::new(-20, 40, 10, 50)); + assert_eq!(drawn(&s, shape), Rect::new(-20, 10, 20, 30)); + s.execute("layer.align.leftEdges", json!({})).unwrap(); + assert_eq!(bounds(&s, pixels), Rect::new(-20, 40, 10, 50), "a second click moves nothing"); + // Centred: the union is -20..20 wide on the shape's side and 30 px for the pixel layer. + s.execute("layer.align.horizontalCenters", json!({})).unwrap(); + assert_eq!(drawn(&s, shape), Rect::new(-20, 10, 20, 30)); + assert_eq!(bounds(&s, pixels), Rect::new(-15, 40, 15, 50)); + // A feathered shape is measured by its soft edge too, which reaches past the path. + let soft = LayerId(s.execute("shape.create", json!({"kind": "rect", "rect": [-20, 50, 40, 20], "fill": "#00ff00"})).unwrap()["layer"].as_u64().unwrap()); + s.edit("feather", |doc, _| { + if let Some(LayerContent::Shape(sh)) = doc.layer_mut(soft).map(|l| &mut l.content) { + sh.feather = 4.0; + } + Ok(()) + }) + .unwrap(); + // Measured as Align does, in the document's frame: the feather off the canvas still counts. + let b = drawn(&s, soft); + assert!(b.x0 < -20 && b.x1 > 20 && b.y0 < 50 && b.y1 > 70, "{b:?}"); + // A fill that shows everywhere keeps the canvas-clipped measure. + s.edit("invert", |doc, _| { + if let Some(LayerContent::Shape(sh)) = doc.layer_mut(soft).map(|l| &mut l.content) { + sh.feather = 0.0; + sh.path.inverted = true; + } + Ok(()) + }) + .unwrap(); + let LayerContent::Shape(sh) = &doc(&s).layer(soft).unwrap().content else { panic!("shape") }; + assert_eq!(shape_pixel_bounds(sh, doc(&s).bounds(), SampleType::U8), None); + // Without a cached render yet, such a fill still measures as the canvas, not as empty. + let mut uncached = doc(&s).layer(soft).unwrap().clone(); + if let LayerContent::Shape(sh) = &mut uncached.content { + sh.cache = None; + } + assert_eq!(align_bounds(&uncached, doc(&s).bounds(), SampleType::U8), Some(doc(&s).bounds())); +} + +/// Review of #8779: shape extents are measured at the document's own alpha precision. A faint +/// stroke (0.001 opacity) is stored by a 16-bit or float document, so it counts in the extent +/// and Align moves the opaque body as the document draws it; an 8-bit document stores it as +/// nothing, so it does not widen the measure. +#[test] +fn align_bounds_follow_the_documents_alpha_precision() { + for (depth, sample, expected) in [ + (8, SampleType::U8, Rect::new(60, 60, 120, 120)), + (16, SampleType::U16, Rect::new(50, 50, 130, 130)), + (32, SampleType::F32, Rect::new(50, 50, 130, 130)), + ] { + let mut s = session(depth); + let id = LayerId(s.execute("shape.create", json!({"kind": "rect", "rect": [60, 60, 60, 60], "fill": "#000000"})).unwrap()["layer"].as_u64().unwrap()); + s.edit("faint stroke", |doc, _| { + if let Some(LayerContent::Shape(sh)) = doc.layer_mut(id).map(|l| &mut l.content) { + sh.stroke = Some(photocraft_doc::ShapeStroke { width: 20.0, opacity: 0.001, ..Default::default() }); + } + Ok(()) + }) + .unwrap(); + assert_eq!(doc(&s).pixel_format().sample, sample); + let LayerContent::Shape(sh) = &doc(&s).layer(id).unwrap().content else { panic!("shape") }; + assert_eq!(shape_pixel_bounds(sh, doc(&s).bounds(), sample), Some(expected), "depth {depth}"); + assert_eq!(align_bounds(doc(&s).layer(id).unwrap(), doc(&s).bounds(), sample), Some(expected), "depth {depth}"); + } +} + #[test] fn align_single_layer_to_selection() { for depth in [8, 16, 32] { diff --git a/crates/engine/src/cmd/layer_params.rs b/crates/engine/src/cmd/layer_params.rs index dddac7ab62..f298893b33 100644 --- a/crates/engine/src/cmd/layer_params.rs +++ b/crates/engine/src/cmd/layer_params.rs @@ -63,7 +63,7 @@ pub fn blend_from_str(s: &str) -> Option { /// The active selection as the mask of a layer being created, or None without a selection. /// Photoshop masks a new adjustment or fill layer with the selection, so the adjustment acts on -/// the selected area only (#1250); the selection itself stays, as Reveal Selection leaves it. +/// the selected area only (#1250). [`mask_new_layer`] then drops the selection (#8801). pub(crate) fn selection_mask(doc: &Document) -> Result> { doc.selection.as_ref().map(|sel| mask_at_depth(LayerMask { surface: sel.clone(), ..LayerMask::reveal_all() }, doc.depth)).transpose() } @@ -88,9 +88,10 @@ pub(crate) fn layer_panel_option(s: &Session, key: &str, fallback: bool) -> bool /// Mask a fill or adjustment layer being created, as Photoshop does: the path selected in the /// Paths panel (`"path"`: `"work"`, a saved path's name or a path object) becomes its vector /// mask, which makes a Solid Color fill a shape (#1419); without one, the selection becomes its -/// layer mask ([`selection_mask`]); without either, a white mask reveals the whole layer (#1768). -/// A path that isn't there is an error. -pub(crate) fn mask_new_layer(doc: &Document, l: &mut Layer, p: &Value, cmd: &str, default_mask: bool) -> Result<()> { +/// layer mask ([`selection_mask`]) and is dropped, as Reveal Selection drops it (#5251, #8801); +/// without either, a white mask reveals the whole layer (#1768). A path that isn't there is an +/// error. +pub(crate) fn mask_new_layer(doc: &mut Document, l: &mut Layer, p: &Value, cmd: &str, default_mask: bool) -> Result<()> { let path = match p.get("path") { None | Some(Value::Null) => None, Some(Value::String(n)) if crate::vector_cmds::is_work(Some(n)) => Some(doc.work_path.clone().ok_or_else(|| bad(cmd, "no work path"))?), @@ -101,9 +102,13 @@ pub(crate) fn mask_new_layer(doc: &Document, l: &mut Layer, p: &Value, cmd: &str }; match path { Some(path) => l.vector_mask = Some(photocraft_doc::VectorMask::new(path)), - None => { - l.mask = selection_mask(doc)?.or_else(|| default_mask.then(LayerMask::reveal_all)).map(|mask| mask_at_depth(mask, doc.depth)).transpose()?; - } + None => match selection_mask(doc)? { + Some(mask) => { + l.mask = Some(mask); + doc.selection = None; + } + None => l.mask = default_mask.then(LayerMask::reveal_all).map(|mask| mask_at_depth(mask, doc.depth)).transpose()?, + }, } Ok(()) } diff --git a/crates/engine/src/cmd/mode_cmds.rs b/crates/engine/src/cmd/mode_cmds.rs index 413b768846..2419203caa 100644 --- a/crates/engine/src/cmd/mode_cmds.rs +++ b/crates/engine/src/cmd/mode_cmds.rs @@ -12,7 +12,7 @@ use photocraft_algo::transform::{Homography, Interp}; use photocraft_color::{ColorMode, PixelFormat, SampleType}; use photocraft_doc::adjust::CurvePoint; use photocraft_doc::{ColorTable, Document, Duotone, DuotoneInk, Layer, LayerContent, LayerId, Size}; -use photocraft_geom::Affine; +use photocraft_geom::{Affine, Rect}; use photocraft_raster::Surface; use serde_json::{Value, json}; @@ -411,6 +411,62 @@ fn color_table(s: &mut Session, p: &Value) -> Result { Ok(out) } +/// The colour table painting on `p`'s target must stay inside: an Indexed Color document's, +/// when the target is layer pixels (masks and channels are gray, not indexed). +pub(crate) fn paint_color_table(doc: &Document, p: &Value) -> Option { + let pixels = crate::channel_cmds::target_of(p) == crate::channel_cmds::Target::Pixels; + doc.color_table.clone().filter(|_| pixels && doc.mode == ColorMode::Indexed) +} + +/// The colour `c` paints as in a document with `table` (see [`keep_in_color_table`]). +pub(crate) fn table_color(table: &ColorTable, c: [f32; 4]) -> [f32; 4] { + let mut px = [c]; + if !table.colors.is_empty() { + quantize::quantize(&mut px, 1, &table.colors, Dither::None, 0.0, table.transparent.map(usize::from)); + } + let [c] = px; + c +} + +/// Map the pixels of `r` that the stroke changed (they differ from `pre`) to their nearest +/// colour-table entry, as the Indexed Color conversion does with no dither, so painting never +/// leaves the table (#8669). One tile at a time: a long stroke's bounds can span the canvas. +pub(crate) fn keep_in_color_table(table: &ColorTable, pre: &Surface, surf: &mut Surface, r: Rect) { + if table.colors.is_empty() { + return; + } + let fmt = surf.format(); + let n = fmt.channels(); + let mut enc = [0.0f32; 8]; + for tc in r.tiles() { + let t = tc.rect().intersect(&r); + if t.is_empty() || !surf.has_tiles_in(t) { + continue; + } + let (before, mut data) = (pre.read_region(t), surf.read_region(t)); + let (mut changed, mut px) = (Vec::new(), Vec::new()); + for (i, (q, b)) in data.chunks_exact(n).zip(before.chunks_exact(n)).enumerate() { + if q != b { + changed.push(i); + px.push(photocraft_raster::to_rgba(&fmt, q)); + } + } + if px.is_empty() { + continue; + } + let w = px.len(); + quantize::quantize(&mut px, w, &table.colors, Dither::None, 0.0, table.transparent.map(usize::from)); + let mut mapped = changed.into_iter().zip(px).peekable(); + for (i, q) in data.chunks_exact_mut(n).enumerate() { + if let Some((_, c)) = mapped.next_if(|(j, _)| *j == i) { + photocraft_raster::from_rgba_into(&fmt, c, &mut enc); + q.iter_mut().zip(enc).for_each(|(d, v)| *d = v); + } + } + surf.write_region(t, &data); + } +} + // ---------- Bitmap ---------- /// Bitmap's Output resolution range (ppi): what a new document may have. diff --git a/crates/engine/src/cmd/mode_cmds/tests.rs b/crates/engine/src/cmd/mode_cmds/tests.rs index 21836ff5ae..d4e3889f3a 100644 --- a/crates/engine/src/cmd/mode_cmds/tests.rs +++ b/crates/engine/src/cmd/mode_cmds/tests.rs @@ -749,3 +749,95 @@ fn color_table_rejects_an_invalid_transparent_index_without_editing() { s.execute("image.mode.colorTable", json!({"transparent": 0})).unwrap(); assert_eq!(doc(&s).color_table.as_ref().unwrap().transparent, Some(0)); } + +/// A white 16×16 document in Indexed Color with the two-entry black/white table (#8669's repro). +fn black_white_indexed(transparency: bool) -> Session { + let mut s = Session::new(); + let background = if transparency { "transparent" } else { "white" }; + s.execute("file.new", json!({"width": 16, "height": 16, "background": background})).unwrap(); + paint(&mut s, |x, _| { + if x < 2 { + [0.0; 4] + } else if x < 4 { + [0.0, 0.0, 0.0, 1.0] + } else { + [1.0; 4] + } + }); + s.execute("image.mode.indexedColor", json!({"palette": "exact", "forced": "none", "transparency": transparency, "dither": "none"})).unwrap(); + s +} + +#[test] +fn brush_stroke_stays_in_indexed_color_table() { + let mut s = black_white_indexed(false); + assert_eq!(doc(&s).color_table.as_ref().unwrap().colors, vec![[0, 0, 0], [255, 255, 255]]); + s.execute("paint.stroke", json!({"points": [[8, 8, 1]], "size": 8, "color": "#ff0000", "opacity": 1, "flow": 1})).unwrap(); + // Red's nearest entry is black. + assert_eq!(doc(&s).layers[0].surface().unwrap().rgba(8, 8), [0.0, 0.0, 0.0, 1.0]); + // Soft edges and partial opacity blend, and the blends map back onto the table too. + s.execute("paint.stroke", json!({"points": [[4, 12], [14, 12]], "size": 6, "hardness": 0.0, "color": "#808080", "opacity": 0.5})).unwrap(); + let table = doc(&s).color_table.clone().unwrap(); + assert!(colours(doc(&s)).iter().all(|c| table.colors.contains(c)), "off-table pixels: {:?}", colours(doc(&s))); + assert_eq!(doc(&s).mode, ColorMode::Indexed); +} + +#[test] +fn brush_stroke_keeps_indexed_transparency_binary() { + let mut s = black_white_indexed(true); + let table = doc(&s).color_table.clone().unwrap(); + assert!(table.transparent.is_some()); + s.execute("paint.stroke", json!({"points": [[0, 8], [15, 8]], "size": 8, "hardness": 0.0, "color": "#ff0000", "opacity": 0.7})).unwrap(); + let d = doc(&s); + let surf = d.layers[0].surface().unwrap(); + for (x, y) in (0..16).flat_map(|y| (0..16).map(move |x| (x, y))) { + let a = surf.rgba(x, y)[3]; + assert!(a == 0.0 || a == 1.0, "alpha {a} at ({x},{y})"); + } + assert!(colours(d).iter().all(|c| table.colors.contains(c)), "off-table pixels: {:?}", colours(d)); +} + +#[test] +fn live_brush_preview_matches_indexed_commit() { + let mut s = black_white_indexed(false); + let params = json!({"points": [[4, 8], [12, 8]], "size": 7, "hardness": 0.0, "color": "#ff0000", "opacity": 0.6, "smoothing": 0.0, "seed": 1}); + let preview = crate::brush_cmds::LiveStroke::begin(&s, ¶ms).unwrap(); + let table = doc(&s).color_table.clone().unwrap(); + assert!(colours(&preview.doc).iter().all(|c| table.colors.contains(c)), "preview off the table: {:?}", colours(&preview.doc)); + s.execute("paint.stroke", params).unwrap(); + let (live, committed) = (preview.doc.layers[0].surface().unwrap(), doc(&s).layers[0].surface().unwrap()); + for (x, y) in (0..16).flat_map(|y| (0..16).map(move |x| (x, y))) { + assert_eq!(live.rgba(x, y), committed.rgba(x, y), "preview/commit at ({x},{y})"); + } +} + +#[test] +fn indexed_brush_stroke_leaves_pixels_it_did_not_paint() { + let mut s = black_white_indexed(false); + // An off-table pixel inside the stroke's bounds but away from its dabs (Paint Bucket can + // still leave one) is not the stroke's to remap. + s.edit("setup", |doc, active| { + let surf = doc.layer_mut(active.unwrap()).unwrap().surface_mut().unwrap(); + let px = photocraft_raster::from_rgba(&surf.format(), [1.0, 0.0, 0.0, 1.0]); + surf.write_region(Rect::new(13, 2, 14, 3), &px); + Ok(()) + }) + .unwrap(); + s.execute("paint.stroke", json!({"points": [[2, 2], [14, 14]], "size": 2, "hardness": 1.0, "color": "#ff0000", "smoothing": 0.0})).unwrap(); + let surf = doc(&s).layers[0].surface().unwrap(); + assert_eq!(surf.rgba(13, 2), [1.0, 0.0, 0.0, 1.0]); + assert_eq!(surf.rgba(8, 8), [0.0, 0.0, 0.0, 1.0]); +} + +#[test] +fn pencil_auto_erase_knows_the_indexed_foreground() { + let mut s = black_white_indexed(false); + let pencil = json!({"points": [[8, 8]], "size": 3, "color": "#ff0000", "autoErase": true}); + s.execute("paint.pencil", pencil.clone()).unwrap(); + assert_eq!(doc(&s).layers[0].surface().unwrap().rgba(8, 8), [0.0, 0.0, 0.0, 1.0]); + // Red painted as black, so a stroke starting there auto-erases to the (white) background. + let preview = crate::brush_cmds::LiveStroke::begin_with(&s, "paint.pencil", &pencil).unwrap(); + assert_eq!(preview.doc.layers[0].surface().unwrap().rgba(8, 8), [1.0, 1.0, 1.0, 1.0]); + s.execute("paint.pencil", pencil).unwrap(); + assert_eq!(doc(&s).layers[0].surface().unwrap().rgba(8, 8), [1.0, 1.0, 1.0, 1.0]); +} diff --git a/crates/engine/src/cmd/params.rs b/crates/engine/src/cmd/params.rs index 7d78bacccc..f5d644dae7 100644 --- a/crates/engine/src/cmd/params.rs +++ b/crates/engine/src/cmd/params.rs @@ -69,6 +69,19 @@ pub(crate) fn check_f32_values(cmd: &str, value: &Value) -> Result<()> { check(cmd, value, 0) } +/// `p[key]` as CMYK inks 0..1 (`[c, m, y, k]`), the values a colour was picked as, or `None` +/// when absent or null. A CMYK document stores exactly these instead of what the colour's sRGB +/// stand-in converts back to (#4196, #8568). +pub(crate) fn cmyk_param(cmd: &str, p: &Value, key: &str) -> Result> { + let Some(v) = p.get(key).filter(|v| !v.is_null()) else { return Ok(None) }; + let inks: Option> = + v.as_array().filter(|a| a.len() == 4).and_then(|a| a.iter().map(|x| x.as_f64().filter(|x| x.is_finite()).map(|x| x.clamp(0.0, 1.0) as f32)).collect()); + match inks.as_deref() { + Some(&[c, m, y, k]) => Ok(Some([c, m, y, k])), + _ => Err(bad(cmd, format!("`{key}` must be [c, m, y, k] numbers 0..1"))), + } +} + pub(crate) fn color_param(cmd: &str, p: &Value, key: &str, default: [f32; 4]) -> Result<[f32; 4]> { Ok(match p.get(key) { Some(Value::Array(a)) if a.len() >= 3 => { diff --git a/crates/engine/src/cmd/pattern_cmds.rs b/crates/engine/src/cmd/pattern_cmds.rs index 265e29b48a..51cb0281c0 100644 --- a/crates/engine/src/cmd/pattern_cmds.rs +++ b/crates/engine/src/cmd/pattern_cmds.rs @@ -183,7 +183,7 @@ pub fn resolve(s: &Session, key: &str) -> Option { pub(crate) fn resolve_param(s: &Session, cmd: &str, p: &Value) -> Result { let key = p.get("pattern").and_then(Value::as_str).unwrap_or(""); if let Some(pat) = resolve(s, key) { - return Ok(pat); + return paintable(cmd, pat); } match library_pattern(s, key).map(|lp| full(s, lp)) { Some(Err(e)) => Err(bad(cmd, e)), @@ -191,6 +191,15 @@ pub(crate) fn resolve_param(s: &Session, cmd: &str, p: &Value) -> Result Result { + match pat.paint_problem() { + Some(why) => Err(bad(cmd, format!("pattern \"{}\" would paint nothing: {why}", pat.display_name()))), + None => Ok(pat), + } +} + /// Copies `pat` into the document's patterns unless it is already there (by id). pub fn ensure_in_doc(doc: &mut Document, pat: &Pattern) { if !doc.patterns.iter().any(|p| p.id == pat.id) { @@ -207,6 +216,7 @@ pub fn resolve_effect(s: &Session, fx: photocraft_doc::Effect) -> Result<(photoc Effect::PatternOverlay { common, name, id, scale, angle, link, phase } => { let key = if id.is_empty() { name } else { id }; let pat = resolve(s, &key).ok_or_else(|| bad("layer.layerStyle.patternOverlay", format!("no pattern \"{key}\" (see pattern.list)")))?; + let pat = paintable("layer.layerStyle.patternOverlay", pat)?; Ok((Effect::PatternOverlay { common, name: pat.name.clone(), id: pat.id.clone(), scale, angle, link, phase }, Some(pat))) } other => Ok((other, None)), @@ -219,6 +229,7 @@ fn info(p: &Pattern, size: (u32, u32), in_doc: bool, in_library: bool) -> Value "id": p.id, "name": p.display_name(), "storedName": p.name, "width": size.0, "height": size.1, "mode": format!("{:?}", f.mode), "depth": format!("{:?}", f.sample), "alpha": f.alpha, "inDocument": in_doc, "inLibrary": in_library, + "paintable": p.is_paintable(), "paintProblem": p.paint_problem(), }) } @@ -230,19 +241,45 @@ fn list(s: &mut Session, _: &Value) -> Result { Ok(Value::Array(out)) } -/// The visible composite over `r` as a surface in the document's pixel format (with alpha). -fn composite_surface(doc: &Document, r: Rect) -> Surface { - let buf = photocraft_compose::render(doc, r); +/// Pixels composited at a time by [`composite_surface`]: a band's render and conversion buffers +/// stay a few tens of MB however large the pattern. +const BAND_PIXELS: u32 = 1 << 20; + +/// The visible composite over `r` as a surface in the document's pixel format (with alpha), and +/// whether every pixel of it is opaque. +fn composite_surface(doc: &Document, r: Rect) -> (Surface, bool) { + composite_surface_in_bands(doc, r, (BAND_PIXELS / r.width().max(1)).max(1)) +} + +/// [`composite_surface`], `band_rows` rows at a time. Rendering and converting one band at a +/// time, and noting opacity as it goes, keeps an 8000 × 8000 pattern (64 MP, about 1 GB per full +/// `f32` copy) from holding several full-size copies at once. Opacity is judged on the stored +/// samples, as the surface quantizes them: an alpha of 0.999 in an 8-bit document is stored as 255. +fn composite_surface_in_bands(doc: &Document, r: Rect, band_rows: u32) -> (Surface, bool) { let df = doc.pixel_format(); let fmt = PixelFormat::new(df.mode, df.sample, true); let n = fmt.channels(); - let mut px = vec![0.0f32; buf.px.len() * n]; - for (o, p) in px.chunks_exact_mut(n).zip(&buf.px) { - photocraft_raster::from_rgba_into(&fmt, *p, o); - } + let (width, height) = (r.width(), r.height()); let mut s = Surface::new(fmt); - s.write_region(Rect::new(0, 0, r.width() as i32, r.height() as i32), &px); - s + let mut opaque = true; + let mut px = Vec::new(); + let mut top = 0; + while top < height { + let rows = band_rows.min(height - top); + let buf = photocraft_compose::render(doc, Rect::new(r.x0, r.y0 + top as i32, r.x1, r.y0 + (top + rows) as i32)); + px.clear(); + px.resize(buf.px.len() * n, 0.0f32); + for (o, p) in px.chunks_exact_mut(n).zip(&buf.px) { + photocraft_raster::from_rgba_into(&fmt, *p, o); + } + let band = Rect::new(0, top as i32, width as i32, (top + rows) as i32); + s.write_region(band, &px); + if opaque { + opaque = s.read_region(band).chunks_exact(n).all(|q| q[n - 1] >= 1.0); + } + top += rows; + } + (s, opaque) } /// The name Define Pattern gives a pattern without one: the active document's name without its @@ -259,6 +296,11 @@ pub fn default_name(s: &Session) -> String { (2..).map(|i| format!("{base} {i}")).find(|n| !taken(n)).unwrap_or(base) } +/// Longest edge Define Pattern accepts. Its area is limited too, to what the compositor holds +/// (`Pattern::working_bytes`): 8000 x 8000 at 8 bits, and fewer pixels at 16 bits and float, +/// where the working copy is larger per pixel. What is accepted always paints. +const MAX_DEFINE_EDGE: u32 = 8000; + fn define(s: &mut Session, p: &Value) -> Result { let cmd = "edit.definePattern"; let st = s.active().ok_or(EngineError::NoDocument)?; @@ -274,14 +316,20 @@ fn define(s: &mut Session, p: &Value) -> Result { if r.is_empty() { return Err(bad(cmd, "the area is empty")); } - if r.width() > 4000 || r.height() > 4000 { - return Err(bad(cmd, "patterns are limited to 4000 × 4000 px")); + if r.width() > MAX_DEFINE_EDGE || r.height() > MAX_DEFINE_EDGE { + return Err(bad(cmd, "patterns are limited to 8000 × 8000 px")); } - let mut surf = composite_surface(doc, r); + // A pattern the compositor could not hold would define but paint nothing: 8 bits go to the + // edge limit, 16-bit documents to 33,554,432 px in all, float to 16,777,216 (4096 × 4096). + let sample = doc.pixel_format().sample; + let max_px = photocraft_doc::pattern::max_paint_pixels(sample); + if u64::from(r.width()) * u64::from(r.height()) > max_px { + return Err(bad(cmd, format!("patterns in a {}-bit document are limited to {max_px} px in all", sample.bytes() * 8))); + } + let (mut surf, opaque) = composite_surface(doc, r); // Opaque patterns drop their alpha channel (as Photoshop does). - let fmt = surf.format(); - let px = surf.read_region(Rect::new(0, 0, r.width() as i32, r.height() as i32)); - if px.chunks_exact(fmt.channels()).all(|q| q[fmt.channels() - 1] >= 1.0) { + if opaque { + let fmt = surf.format(); surf = surf.convert(PixelFormat::new(fmt.mode, fmt.sample, false)); } let name = match p.get("name").and_then(Value::as_str) { @@ -411,15 +459,15 @@ fn brush_texture(s: &mut Session, p: &Value) -> Result { return Err(bad("brush.texturePattern", "the pattern is empty or too large to paint")); }; // Brush textures are grey: luminance of the composite over white. - let mut px = vec![[0.0f32; 4]; n]; - pat.surface.read_rgba_into(pat.rect(), &mut px); - let lum: Vec = px - .iter() - .map(|q| { + // Read a band of rows at a time: a whole-tile RGBA copy would cost 16 bytes a pixel. + let mut lum: Vec = Vec::new(); + lum.try_reserve_exact(n).map_err(|_| bad("brush.texturePattern", "not enough memory for the pattern"))?; + for_each_band(&pat, pat.width, pat.height, |band| { + lum.extend(band.iter().map(|q| { let l = 0.299 * q[0] + 0.587 * q[1] + 0.114 * q[2]; l * q[3] + (1.0 - q[3]) - }) - .collect(); + })); + }); let tile = photocraft_paint::GrayTile::from_f32(pat.width, pat.height, &lum); let t = &mut s.tools.brush.texture; t.pattern = photocraft_paint::Pattern::Tile(tile); @@ -435,13 +483,25 @@ pub fn average_color(p: &Pattern) -> Color { // A pattern too large to paint is averaged over its paintable top-left corner. let side = photocraft_doc::pattern::MAX_PAINT_PIXELS.isqrt() as u32; let (w, h) = (p.width.min(side), p.height.min(side)); - let mut px = vec![[0.0f32; 4]; w as usize * h as usize]; - p.surface.read_rgba_into(Rect::new(0, 0, w as i32, h as i32), &mut px); - let n = px.len().max(1) as f32; - let s = px.iter().fold([0.0f32; 3], |a, q| [a[0] + q[0], a[1] + q[1], a[2] + q[2]]); + let mut s = [0.0f32; 3]; + for_each_band(p, w, h, |band| s = band.iter().fold(s, |a, q| [a[0] + q[0], a[1] + q[1], a[2] + q[2]])); + let n = (u64::from(w) * u64::from(h)).max(1) as f32; Color::rgb(s[0] / n, s[1] / n, s[2] / n) } +/// Calls `f` with the straight RGBA pixels of the top-left `w × h` corner of `p`, a band of rows +/// at a time in row-major order. +fn for_each_band(p: &Pattern, w: u32, h: u32, mut f: impl FnMut(&[[f32; 4]])) { + let mut band = Vec::new(); + for top in (0..h).step_by(photocraft_doc::pattern::BAND_ROWS as usize) { + let bottom = (top + photocraft_doc::pattern::BAND_ROWS).min(h); + band.clear(); + band.resize(w as usize * (bottom - top) as usize, [0.0f32; 4]); + p.surface.read_rgba_into(Rect::new(0, top as i32, w as i32, bottom as i32), &mut band); + f(&band); + } +} + pub fn specs() -> Vec { vec![ CommandSpec { @@ -593,6 +653,128 @@ mod tests { } } + /// Review of the 8000 px cap: compositing a band of rows at a time gives the pixels and the + /// opacity of one pass, at every depth, whatever the band height. + #[test] + fn banded_compositing_matches_a_single_pass() { + for depth in [8u64, 16, 32] { + for transparent in [false, true] { + let mut s = Session::new(); + let background = if transparent { "transparent" } else { "white" }; + s.execute("file.new", json!({"width": 40, "height": 30, "depth": depth, "background": background})).unwrap(); + s.execute("select.rect", json!({"x": 3, "y": 4, "width": 20, "height": 13})).unwrap(); + s.execute("edit.fill", json!({"color": "#336699"})).unwrap(); + let doc = &s.active().unwrap().doc; + let area = Rect::new(2, 1, 38, 29); + let all = Rect::new(0, 0, 36, 28); + let (whole, whole_opaque) = composite_surface_in_bands(doc, area, 28); + for band_rows in [1, 5, 27] { + let (banded, opaque) = composite_surface_in_bands(doc, area, band_rows); + assert_eq!(opaque, whole_opaque, "depth {depth}, transparent {transparent}, bands of {band_rows}"); + assert_eq!(banded.read_region(all), whole.read_region(all), "depth {depth}, transparent {transparent}, bands of {band_rows}"); + } + assert_eq!(whole_opaque, !transparent); + } + } + } + + /// Review of the banded composite: opacity is the stored samples', so an alpha an 8-bit + /// document stores as 255 is opaque and a float document's 0.999 is not. + #[test] + fn alpha_that_stores_as_opaque_counts_as_opaque() { + for (depth, opaque) in [(8u64, true), (32, false)] { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8, "depth": depth, "background": "transparent"})).unwrap(); + s.execute("edit.fill", json!({"color": "#336699"})).unwrap(); + s.execute("layer.setProps", json!({"opacity": 0.999})).unwrap(); + let (_, all_opaque) = composite_surface_in_bands(&s.active().unwrap().doc, Rect::new(0, 0, 8, 8), 3); + assert_eq!(all_opaque, opaque, "depth {depth}"); + } + } + + /// Review of #8902: what Define Pattern accepts, the compositor paints. Areas over + /// 4096 x 4096, past the compositor's whole-copy cap, define and fill at 8 and 16 bits (held + /// at the pattern's own depth); a depth that has no smaller exact form is refused up front + /// with its limit, instead of defining a pattern that paints nothing. + #[test] + fn defined_patterns_over_4096_squared_paint_or_are_refused() { + for depth in [8, 16] { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 4100, "height": 4100, "depth": depth})).unwrap(); + s.execute("edit.fill", json!({"color": "#336699"})).unwrap(); + s.execute("edit.definePattern", json!({"name": "Big"})).unwrap(); + assert!(s.patterns.items.iter().any(|p| p.name == "Big" && p.is_paintable()), "depth {depth}"); + s.execute("file.new", json!({"width": 40, "height": 30, "depth": depth})).unwrap(); + s.execute("layer.newFillLayer.pattern", json!({"pattern": "Big"})).unwrap(); + let [r, g, b, a] = pixel(&s, 5, 5); + assert!(a == 1.0 && (r - 0.2).abs() < 0.01 && (g - 0.4).abs() < 0.01 && (b - 0.6).abs() < 0.01, "depth {depth}: {:?}", [r, g, b, a]); + } + let mut s = Session::new(); + s.execute("file.new", json!({"width": 4100, "height": 4100, "depth": 32})).unwrap(); + let e = s.execute("edit.definePattern", json!({})).unwrap_err().to_string(); + assert!(e.contains("32-bit") && e.contains("16777216"), "{e}"); + assert!(s.patterns.items.iter().all(|p| p.name != "Pattern"), "nothing was defined"); + let mut s = Session::new(); + s.execute("file.new", json!({"width": 6000, "height": 5600, "depth": 16})).unwrap(); + let e = s.execute("edit.definePattern", json!({})).unwrap_err().to_string(); + assert!(e.contains("16-bit") && e.contains("33554432"), "{e}"); + } + + /// A pattern of the largest area Define Pattern accepts at 8 bits is paintable: held at 4 + /// bytes a pixel, 256 MB, as the compositor's cap allows, and it renders. + #[test] + fn an_8000_by_8000_pattern_is_defined_and_paints() { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8000, "height": 8000, "depth": 8})).unwrap(); + s.execute("edit.fill", json!({"color": "#336699"})).unwrap(); + s.execute("edit.definePattern", json!({"name": "Largest"})).unwrap(); + let p = s.patterns.items.iter().find(|p| p.name == "Largest").unwrap(); + assert_eq!((p.width, p.height), (8000, 8000)); + assert!(p.is_paintable() && p.working_bytes() == 256_000_000); + s.execute("file.new", json!({"width": 40, "height": 30, "depth": 8})).unwrap(); + s.execute("layer.newFillLayer.pattern", json!({"pattern": "Largest"})).unwrap(); + let [r, g, b, a] = pixel(&s, 5, 5); + assert!(a == 1.0 && (r - 0.2).abs() < 0.01 && (g - 0.4).abs() < 0.01 && (b - 0.6).abs() < 0.01, "{:?}", [r, g, b, a]); + } + + /// Review of #8902: a pattern too large to paint is named by every command that would use it + /// and by pattern.list, instead of rendering as transparency with no message. + #[test] + fn a_pattern_that_would_paint_nothing_is_reported() { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 40, "height": 30, "depth": 8})).unwrap(); + // A 32-bit 5000 x 5000 pattern (declared only: 400 MB as a working copy, past the cap). + let surface = photocraft_raster::Surface::new(photocraft_doc::PixelFormat::RGBA32F); + s.patterns.items.push(Pattern { id: "huge".into(), name: "Huge".into(), width: 5000, height: 5000, surface }); + let list = s.execute("pattern.list", json!({})).unwrap(); + let huge = list.as_array().unwrap().iter().find(|p| p["name"] == "Huge").unwrap().clone(); + assert_eq!(huge["paintable"], json!(false)); + assert!(huge["paintProblem"].as_str().unwrap().contains("32 bits"), "{huge}"); + for (id, params) in [ + ("layer.newFillLayer.pattern", json!({"pattern": "Huge"})), + ("layer.layerStyle.patternOverlay", json!({"pattern": "Huge"})), + ("edit.fill", json!({"contents": "pattern", "pattern": "Huge"})), + ("brush.texturePattern", json!({"pattern": "Huge"})), + ] { + let e = s.execute(id, params).expect_err(id).to_string(); + assert!(e.contains("Huge") && e.contains("would paint nothing"), "{id}: {e}"); + } + let e = s.execute("layer.newFillLayer.pattern", json!({"pattern": "Huge"})).unwrap_err().to_string(); + assert!(e.contains("would paint nothing") && e.contains("5000 × 5000"), "{e}"); + assert!(list.as_array().unwrap().iter().any(|p| p["name"] != "Huge" && p["paintable"] == json!(true)), "built-in patterns paint"); + } + + #[test] + fn define_pattern_accepts_edges_over_4000_up_to_8000() { + for (w, ok) in [(4100, true), (8000, true), (8001, false)] { + let mut s = Session::new(); + s.execute("file.new", json!({"width": w, "height": 2, "depth": 8})).unwrap(); + s.execute("edit.fill", json!({"color": "#336699"})).unwrap(); + let r = s.execute("edit.definePattern", json!({})); + assert_eq!(r.is_ok(), ok, "width {w}: {r:?}"); + } + } + #[test] fn pattern_fill_layer_renders_at_every_depth_and_undoes() { for depth in [8u64, 16, 32] { @@ -684,6 +866,6 @@ mod tests { let huge = Pattern { id: "huge".into(), name: "Huge".into(), width: 65_536, height: 65_536, surface: Surface::new(PixelFormat::RGBA8) }; s.patterns.items.push(huge); let e = s.execute("brush.texturePattern", json!({"pattern": "huge"})).unwrap_err(); - assert!(e.to_string().contains("too large"), "{e}"); + assert!(e.to_string().contains("would paint nothing") && e.to_string().contains("65536 × 65536"), "{e}"); } } diff --git a/crates/engine/src/cmd/solid_fill_cmds.rs b/crates/engine/src/cmd/solid_fill_cmds.rs index efa43f6116..85d941760d 100644 --- a/crates/engine/src/cmd/solid_fill_cmds.rs +++ b/crates/engine/src/cmd/solid_fill_cmds.rs @@ -4,7 +4,7 @@ use photocraft_color::{Color, ColorMode}; use photocraft_doc::{Fill, LayerContent}; use serde_json::{Value, json}; -use crate::commands::{CommandSpec, check_pixels_unlocked, layer_param}; +use crate::commands::{CommandSpec, check_pixels_unlocked, cmyk_param, layer_param}; use crate::{EngineError, Result, Session}; pub const SET: &str = "layer.fill.solidColor.set"; @@ -57,7 +57,13 @@ fn set(s: &mut Session, p: &Value) -> Result { let id = layer_param(s, p)?; let layer = st.doc.layer(id).ok_or(EngineError::NoLayer(id))?; let LayerContent::Fill(Fill::Solid(original)) = layer.content else { return Err(bad("the target must be a Solid Color Fill layer")) }; - let color = color_param(p.get("color").ok_or_else(|| bad("color is required"))?, original, st.doc.mode)?; + let mut color = color_param(p.get("color").ok_or_else(|| bad("color is required"))?, original, st.doc.mode)?; + // The inks the colour was picked as: a CMYK document keeps them exactly (#8568). + if let Some(k) = cmyk_param(SET, p, "cmyk")? + && st.doc.mode == ColorMode::Cmyk + { + color = Color { mode: ColorMode::Cmyk, c: k, alpha: color.alpha }; + } if color != original { check_pixels_unlocked(&st.doc, id)?; if st.doc.effective_locks(id).transparency && color.alpha != original.alpha { @@ -78,7 +84,7 @@ pub fn specs() -> Vec { label: "Solid Color…", menu: &[], shortcut: None, - params: r##"{"layer":id? (default active),"document":id? (reject a stale dialog),"color":"#rrggbb"|"#rrggbbaa"|[r,g,b,a?]|{mode,c:[f32;4],alpha:f32}}; RGB values without alpha preserve it; stored Color preserves its model and precision"##, + params: r##"{"layer":id? (default active),"document":id? (reject a stale dialog),"color":"#rrggbb"|"#rrggbbaa"|[r,g,b,a?]|{mode,c:[f32;4],alpha:f32},"cmyk":[c,m,y,k]? (inks 0..1 the colour was picked as: a CMYK document keeps exactly these)}; RGB values without alpha preserve it; stored Color preserves its model and precision"##, enabled: |s| s.active().map(|_| ()).ok_or_else(|| "no document open".into()), run: set, journal: true, @@ -89,6 +95,47 @@ pub fn specs() -> Vec { mod tests { use super::*; + /// #8568: CMYK 50/50/50/100 entered for a Solid Color fill in a CMYK document stays exactly + /// those inks, on creation and on edit, instead of what its sRGB stand-in converts back to + /// through the document profile. RGB documents ignore the inks. + #[test] + fn cmyk_inks_survive_solid_color_fills_in_a_cmyk_document() { + let inks = [0.5, 0.5, 0.5, 1.0]; + let hex = |k: [f32; 4]| { + let [r, g, b] = photocraft_color::convert::cmyk_to_rgb(k).map(|c| (c.clamp(0.0, 1.0) * 255.0).round() as u8); + format!("#{r:02x}{g:02x}{b:02x}") + }; + let stored = |s: &Session, id: u64| match s.active().unwrap().doc.layer(photocraft_doc::LayerId(id)).unwrap().content { + LayerContent::Fill(Fill::Solid(c)) => c, + _ => panic!("not a solid fill"), + }; + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + s.execute("image.mode.cmyk", json!({})).unwrap(); + let id = s.execute("layer.newFillLayer.solidColor", json!({"color": hex(inks), "cmyk": inks})).unwrap()["layer"].as_u64().unwrap(); + assert_eq!(stored(&s, id), Color { mode: ColorMode::Cmyk, c: inks, alpha: 1.0 }); + // The sRGB stand-in alone does drift: that is what the inks prevent. + let drifted = s.execute("layer.newFillLayer.solidColor", json!({"color": hex(inks)})).unwrap()["layer"].as_u64().unwrap(); + assert_ne!(stored(&s, drifted).c, inks); + // Editing keeps the new inks too. + let edit = [0.2, 0.3, 0.4, 0.5]; + s.execute(SET, json!({"layer": id, "color": hex(edit), "cmyk": edit})).unwrap(); + assert_eq!(stored(&s, id), Color { mode: ColorMode::Cmyk, c: edit, alpha: 1.0 }); + // A foreground picked as CMYK seeds a new fill with its inks, as fills do (#4196). + s.execute("tools.setColors", json!({"foreground": hex(edit), "foregroundCmyk": edit})).unwrap(); + let seeded = s.execute("layer.newFillLayer.solidColor", json!({})).unwrap()["layer"].as_u64().unwrap(); + assert_eq!(stored(&s, seeded).c, edit); + for bad in [json!([0.1, 0.2, 0.3]), json!("50,50,50,100"), json!([0.1, 0.2, 0.3, f64::NAN])] { + assert!(s.execute("layer.newFillLayer.solidColor", json!({"cmyk": bad})).is_err(), "{bad}"); + assert!(s.execute(SET, json!({"layer": id, "color": "#000000", "cmyk": bad})).is_err(), "{bad}"); + } + // An RGB document takes the colour, not the inks. + let mut rgb = Session::new(); + rgb.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + let id = rgb.execute("layer.newFillLayer.solidColor", json!({"color": "#336699", "cmyk": inks})).unwrap()["layer"].as_u64().unwrap(); + assert_eq!(stored(&rgb, id).mode, ColorMode::Rgb); + } + #[test] fn edit_preserves_layer_and_masks_at_all_depths_and_undo_redo() { for depth in [8, 16, 32] { diff --git a/crates/engine/src/cmd/transform_cmds/tests.rs b/crates/engine/src/cmd/transform_cmds/tests.rs index f8ff204257..e9ed16f245 100644 --- a/crates/engine/src/cmd/transform_cmds/tests.rs +++ b/crates/engine/src/cmd/transform_cmds/tests.rs @@ -345,7 +345,6 @@ fn cached_gradient_fill() -> Session { let mut s = session(); s.execute("select.rect", json!({"x": 10, "y": 10, "width": 30, "height": 30})).unwrap(); s.execute("layer.newFillLayer.gradient", json!({"from": "#ff0000", "to": "#0000ff", "angle": 0})).unwrap(); - s.execute("select.deselect", json!({})).unwrap(); s.edit("cache", |doc, active| { let (fmt, canvas) = (doc.pixel_format(), doc.bounds()); let l = doc.layer_mut(active.unwrap()).unwrap(); diff --git a/crates/engine/src/kys.rs b/crates/engine/src/kys.rs index 643ed46057..1406e0a384 100644 --- a/crates/engine/src/kys.rs +++ b/crates/engine/src/kys.rs @@ -49,6 +49,18 @@ pub struct KysSet { pub windows: bool, } +/// The set name in the root `filename` attribute (`$$$/.../{name}={name}`). The name may itself +/// contain `/` or `=`, so split on the key prefix and the doubled name, not on the last separator. +fn filename_to_name(f: &str) -> &str { + let Some(rest) = f.strip_prefix("$$$/") else { return f.rsplit(['=', '/']).next().unwrap_or(f).trim() }; + let rest = rest.strip_prefix("FileName/Presets/KeyboardCustomization/").unwrap_or(rest); + let mid = rest.len() / 2; + if rest.len() % 2 == 1 && rest.is_char_boundary(mid) && rest.as_bytes()[mid] == b'=' && rest[..mid] == rest[mid + 1..] { + return rest[..mid].trim(); + } + rest.split_once('=').map_or(rest, |(_, n)| n).trim() +} + /// Parse a `.kys` file (or Photoshop's live `Keyboard Shortcuts.psp`, the same XML). Rows /// without a name or a shortcut are skipped. pub fn parse(text: &str) -> Result { @@ -61,7 +73,7 @@ pub fn parse(text: &str) -> Result { if root.tag_name().name() != "photoshop-keyboard-shortcuts" { return Err("not a keyboard shortcut set (.kys): no root".into()); } - let name = root.attribute("filename").map(|f| f.rsplit(['=', '/']).next().unwrap_or(f).trim()).filter(|n| !n.is_empty()).map(str::to_string); + let name = root.attribute("filename").map(filename_to_name).filter(|n| !n.is_empty()).map(str::to_string); let mut set = KysSet { name, ..Default::default() }; for node in root.children().filter(|n| n.is_element()) { match node.tag_name().name() { @@ -119,6 +131,7 @@ pub fn to_ours(set: &KysSet, sc: &str) -> Option { } let (mods, key) = match sc.strip_suffix("++") { Some(m) => (m, "+"), + None if sc == "+" => ("", "+"), None => sc.rsplit_once('+').unwrap_or(("", sc)), }; let mods: Vec<&str> = mods @@ -262,6 +275,7 @@ pub fn photoshop_key(sc: &str) -> Option { // The key is the last part; `Cmd++` ends in an empty part, so take it from the string. let (mods, key) = match n.strip_suffix("++") { Some(m) => (m, "+"), + None if n == "+" => ("", "+"), None => n.rsplit_once('+').unwrap_or(("", n.as_str())), }; let has = |m: &str| mods.split('+').any(|x| x == m); @@ -309,6 +323,13 @@ pub fn write(name: &str, entries: &[Entry]) -> String { mod tests { use super::*; + #[test] + fn set_name_with_separators_round_trips() { + for n in ["Retouch 1/2", "a=b", "x/y=z"] { + assert_eq!(parse(&write(n, &[])).unwrap().name.as_deref(), Some(n)); + } + } + const SET: &str = "\u{feff} \tCmd+L @@ -323,6 +344,28 @@ mod tests { Candidate { id: id.into(), label: label.into(), default: default.map(str::to_string) } } + #[test] + fn a_bare_plus_key_is_kept_when_saving() { + assert_eq!(photoshop_key("+").as_deref(), Some("+")); + assert_eq!(photoshop_key("Cmd++").as_deref(), Some("Cmd++")); + let e = Entry { id: "tools.zoom".into(), label: "Zoom Tool".into(), tool: true, keys: vec!["+".into()] }; + assert!(write("Mine", &[e]).contains(">+")); + } + + #[test] + fn a_bare_plus_command_survives_a_windows_set_with_a_ctrl_row() { + let xml = "\ + Ctrl+C\ + +\ + "; + let set = parse(xml).unwrap(); + assert!(set.windows); + assert_eq!(to_ours(&set, "+").as_deref(), Some("+")); + let p = plan([c("edit.copy", "Copy", Some("Cmd+C")), c("view.zoomIn", "Zoom In", None)], &set); + assert_eq!(p.set.get("view.zoomIn").map(String::as_str), Some("+")); + assert!(p.unreadable.is_empty()); + } + #[test] fn rows_match_by_label_and_the_default_holder_wins_a_shared_label() { let set = parse(SET).unwrap(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f10b76fe46..ac64b54025 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -265,8 +265,9 @@ fn new_adjustment_and_fill_layers_are_masked_by_the_selection() { assert_eq!(mask.value(10, 10), 1.0, "{id} at {depth}-bit: the mask hides the selected area"); assert_eq!(mask.value(2, 2), 0.0, "{id} at {depth}-bit: the mask reveals outside the selection"); assert!(mask.linked && mask.enabled, "{id}"); - // The selection stays, as after Layer › Layer Mask › Reveal Selection. - assert!(d.doc.selection.is_some(), "{id} dropped the selection"); + // The selection went into the mask, so Photoshop drops it, as Layer › Layer Mask › + // Reveal Selection does (#5251, #8801). + assert!(d.doc.selection.is_none(), "{id} at {depth}-bit kept the selection"); // The white canvas is untouched outside the selection. assert_eq!(px(&mut s, 2, 2), vec![1.0, 1.0, 1.0, 1.0], "{id} at {depth}-bit changed pixels outside the selection"); if blackens { @@ -274,6 +275,29 @@ fn new_adjustment_and_fill_layers_are_masked_by_the_selection() { } s.execute("edit.undo", json!({})).unwrap(); assert!(s.active().unwrap().doc.layer(layer).is_none(), "{id}: undo keeps the layer"); + let sel = s.active().unwrap().doc.selection.as_ref().unwrap_or_else(|| panic!("{id}: undo does not bring the selection back")); + assert_eq!(sel.content_bounds(), photocraft_geom::Rect::new(8, 8, 24, 24), "{id}: undo restores the same selection"); + } + } +} + +#[test] +fn an_edit_after_a_new_fill_layer_on_a_selection_reaches_the_whole_layer() { + // #8801: the selection that masked a new fill or adjustment layer is gone, so the next + // pixel edit on another layer is not limited to it. + for (id, params) in [("layer.newFillLayer.solidColor", json!({"color": "#000000"})), ("layer.newAdjustmentLayer.invert", json!({}))] { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 40, "height": 20})).unwrap(); + s.execute("edit.fill", json!({"contents": "color", "color": "#3366cc"})).unwrap(); + s.execute("select.rect", json!({"x": 0, "y": 0, "width": 20, "height": 20})).unwrap(); + s.execute(id, params).unwrap(); + s.execute("layer.setProps", json!({"visible": false})).unwrap(); + s.execute("layer.selectBottom", json!({})).unwrap(); + s.execute("layer.duplicate", json!({})).unwrap(); + s.execute("image.adjustments.desaturate", json!({})).unwrap(); + for x in [5, 35] { + let p = px(&mut s, x, 10); + assert!((p[0] - p[1]).abs() < 1e-3 && (p[1] - p[2]).abs() < 1e-3, "{id}: ({x}, 10) is not grey: {p:?}"); } } } @@ -351,6 +375,7 @@ fn new_fill_and_adjustment_layers_take_the_active_path_as_their_vector_mask() { let want = if path_name == "work" { d.doc.work_path.clone().unwrap() } else { d.doc.paths[0].path.clone() }; assert_eq!(vm.path, want, "{id}: the vector mask is the path"); assert!(l.mask.is_none(), "{id}: the path wins over the selection"); + assert!(d.doc.selection.is_some(), "{id}: an unused selection stays"); for (x, y) in [(2, 2), (42, 32), (30, 30)] { assert_eq!(px(&mut s, x, y), vec![1.0, 1.0, 1.0, 1.0], "{id} ({path_name}) at {depth}-bit changed pixels outside the path at ({x}, {y})"); } diff --git a/crates/engine/tests/it/integration_batch.rs b/crates/engine/tests/it/integration_batch.rs new file mode 100644 index 0000000000..14977beece --- /dev/null +++ b/crates/engine/tests/it/integration_batch.rs @@ -0,0 +1,34 @@ +//! Cross-PR acceptance checks for the merged parity batch: each test pins one fix through the +//! public command surface, so a bad merge between the PRs shows up here. +use photocraft_engine::Session; +use serde_json::json; + +fn session(width: u32, height: u32) -> Session { + let mut session = Session::new(); + session.execute("file.new", json!({"width": width, "height": height})).unwrap(); + session +} + +#[test] +fn a_new_fill_layer_consumes_the_selection_it_took_as_mask() { + let mut s = session(32, 24); + s.execute("select.rect", json!({"x": 2, "y": 2, "width": 10, "height": 8})).unwrap(); + s.execute("layer.newFillLayer.solidColor", json!({"color": "#4080c0"})).unwrap(); + assert!(s.active().unwrap().doc.selection.is_none()); +} + +#[test] +fn define_pattern_accepts_a_canvas_wider_than_the_old_4000_px_cap() { + let mut s = session(4500, 8); + s.execute("edit.definePattern", json!({"name": "Wide"})).unwrap(); + let mut s = session(8001, 8); + assert!(s.execute("edit.definePattern", json!({"name": "TooWide"})).is_err()); +} + +#[test] +fn gaussian_blur_accepts_radii_below_one_without_failing() { + for radius in [0.1, 0.3, 0.7, 1.0] { + let mut s = session(24, 16); + s.execute("filter.blur.gaussianBlur", json!({"radius": radius})).unwrap(); + } +} diff --git a/crates/engine/tests/it/layer_mask_depth.rs b/crates/engine/tests/it/layer_mask_depth.rs index 7093164a5f..cbe4138532 100644 --- a/crates/engine/tests/it/layer_mask_depth.rs +++ b/crates/engine/tests/it/layer_mask_depth.rs @@ -45,12 +45,10 @@ fn newly_created_layer_masks_follow_document_depth() { assert_eq!(m.value(20, 20), white); if selected { assert_eq!(m.value(0, 0), 0.0); - if command == "layer.layerMask.revealSelection" { - // The selection became the mask: Photoshop drops it, as Hide Selection does. - assert!(d.doc.selection.is_none(), "{command}: the selection is dropped"); - } else { - assert_eq!(d.doc.selection, before.selection, "the selection is unchanged"); - } + // The selection became the mask: Photoshop drops it, as Hide Selection does + // (#5251; new fill and adjustment layers too, #8801). + assert!(d.doc.selection.is_none(), "{command}: the selection is dropped"); + assert!(before.selection.is_some()); } else { assert_eq!(m.surface.tile_count(), 0, "uniform masks stay sparse"); assert_eq!(m.value(-300_000, 300_000), white); @@ -72,7 +70,6 @@ fn soft_mask_edits_match_after_native_and_psd_reopen() { let mut s = session(depth, "rgb"); s.execute("select.rect", json!({"x": 8, "y": 6, "width": 40, "height": 32, "antiAlias": false})).unwrap(); s.execute("layer.newFillLayer.solidColor", json!({"color": "#000000"})).unwrap(); - s.execute("select.deselect", json!({})).unwrap(); let before = s.active().unwrap().doc.clone(); let native = photocraft_format::save_to_bytes(&before, &Default::default()).unwrap(); let psd = photocraft_io::export(&before, "psd", &Default::default()).unwrap(); @@ -138,7 +135,8 @@ fn mask_creation_preserves_selection_coverage_and_existing_masks() { before.selection.as_ref().unwrap().read_region(before.bounds()), "{depth}-bit coverage is unchanged" ); - let kept = if command == "layer.layerMask.revealSelection" { None } else { before.selection.clone() }; + // The selection went into the mask, except for the Gradient tool's fill layer. + let kept = if command == "gradient.fill.create" { before.selection.clone() } else { None }; assert_eq!(after.selection, kept, "{command}"); assert_eq!(after.layers[0].mask, before.layers[0].mask, "creating a mask does not requantize an existing mask"); } diff --git a/crates/engine/tests/it/main.rs b/crates/engine/tests/it/main.rs index 25455e539c..f6a8360a4f 100644 --- a/crates/engine/tests/it/main.rs +++ b/crates/engine/tests/it/main.rs @@ -37,6 +37,7 @@ mod guide_layout_limit; mod guide_layout_preview; mod hdr_gamma_direction; mod huge_doc_flatten; +mod integration_batch; mod jpeg_exif_resolution; mod keyboard_shortcut_bulk; mod kra; diff --git a/crates/engine/tests/it/merge_transparent_bounds.rs b/crates/engine/tests/it/merge_transparent_bounds.rs index c796007233..03e9473fb2 100644 --- a/crates/engine/tests/it/merge_transparent_bounds.rs +++ b/crates/engine/tests/it/merge_transparent_bounds.rs @@ -101,7 +101,6 @@ fn merging_a_masked_fill_layer_keeps_content_bounds_in_every_mode() { let (mut s, a, _) = scene(mode, depth); s.execute("select.rect", json!({"x": B[0], "y": B[1], "width": B[2], "height": B[3]})).unwrap(); let fill = id_of(&s.execute("layer.newFillLayer.solidColor", json!({"color": "#3060c0"})).unwrap()); - s.execute("select.deselect", json!({})).unwrap(); (s, a, fill) }; for &(mode, depth) in CASES { diff --git a/crates/gpu/src/consts.rs b/crates/gpu/src/consts.rs index 40081a55c8..536aea8a49 100644 --- a/crates/gpu/src/consts.rs +++ b/crates/gpu/src/consts.rs @@ -33,6 +33,9 @@ pub const RESIDENT_BUDGET: u64 = 4 << 30; /// GPU memory pattern textures may hold before the least recently painted are dropped (those the /// current frame paints with always stay). pub const PATTERN_BUDGET: u64 = 256 << 20; +/// The most a single pattern's RGBA32F texture may take. A pattern over it paints on the CPU, which +/// holds a large 8-bit pattern at 4 bytes a pixel instead of 16 (`Pattern::working_bytes`). +pub(crate) const MAX_PATTERN_TEXTURE_BYTES: u64 = 256 << 20; /// A pattern texture no frame has painted with for this many frames is dropped. pub const PATTERN_IDLE_FRAMES: u64 = 240; diff --git a/crates/gpu/src/setup.rs b/crates/gpu/src/setup.rs index c5b5c90384..1eef2147b5 100644 --- a/crates/gpu/src/setup.rs +++ b/crates/gpu/src/setup.rs @@ -1,5 +1,5 @@ use super::batch::tile_grid; -use super::consts::{ACC_FORMAT, CHUNK, CHUNK_UNIFORM, FX_BUDGET, MAP16, MAP32, OP_UNIFORM, PAGE, PATTERN_BUDGET, RESIDENT_BUDGET}; +use super::consts::{ACC_FORMAT, CHUNK, CHUNK_UNIFORM, FX_BUDGET, MAP16, MAP32, MAX_PATTERN_TEXTURE_BYTES, OP_UNIFORM, PAGE, PATTERN_BUDGET, RESIDENT_BUDGET}; use super::kit::{Kit, SHADER, tex_entry, uniform_entry}; use super::paging::{cell_rect, field_reaches, first_error, grid_rects}; use super::textures::{FxEntry, Resident, Tex, TexKind}; @@ -372,19 +372,32 @@ impl Compositor { if s.lut.as_ref().is_some_and(|l| l.len() > 4096) { return Err(Unsupported(format!("effect blur on `{}` too wide for the GPU path", f.layer.name))); } + if let Some(pat) = &s.pattern { + self.check_pattern(pat)?; + } } } } for pass in &p.passes { - if let Some(pat) = pass.pattern - && (pat.width > self.device_max || pat.height > self.device_max) - { - return Err(Unsupported(format!("pattern `{}` larger than the GPU texture limit", pat.name))); + if let Some(pat) = pass.pattern { + self.check_pattern(pat)?; } } Ok(()) } + /// A pattern's texture must fit the device and the per-pattern cap; a larger one renders on + /// the CPU compositor. + fn check_pattern(&self, pat: &photocraft_doc::Pattern) -> Result<(), Unsupported> { + if pat.width > self.device_max || pat.height > self.device_max { + return Err(Unsupported(format!("pattern `{}` larger than the GPU texture limit", pat.name))); + } + if u64::from(pat.width) * u64::from(pat.height) * 16 > MAX_PATTERN_TEXTURE_BYTES { + return Err(Unsupported(format!("pattern `{}` too large for a GPU texture; the CPU path paints it", pat.name))); + } + Ok(()) + } + /// Drop GPU textures of a closed document. pub fn forget_doc(&mut self, doc: DocId) { self.residents.retain(|_, r| r.doc != doc); diff --git a/crates/gpu/tests/it/parity.rs b/crates/gpu/tests/it/parity.rs index 6900f56710..a2ae8586f4 100644 --- a/crates/gpu/tests/it/parity.rs +++ b/crates/gpu/tests/it/parity.rs @@ -2686,6 +2686,27 @@ fn closed_documents_release_their_pattern_textures() { check(&mut g, &a, "the pattern is uploaded again after being forgotten"); } +/// A pattern whose RGBA32F texture would pass the per-pattern cap (Define Pattern accepts 8-bit +/// ones up to 8000 x 8000) is declined by the GPU path and painted by the CPU compositor, which +/// holds a large 8-bit pattern at a quarter of the texture's size. +#[test] +fn a_pattern_too_large_for_a_gpu_texture_falls_back_to_the_cpu() { + let Some(mut g) = gpu() else { return }; + let mut d = Document::new("big pattern", Size::new(8, 6), ColorMode::Rgb, SampleType::U8); + // 100 MB as RGBA8, 400 MB as an RGBA32F texture. + let surface = photocraft_raster::Surface::with_default(PixelFormat::RGBA8, &[1.0, 0.0, 0.0, 1.0]); + let pat = Pattern { id: "big".into(), name: "big".into(), width: 5000, height: 5000, surface }; + assert!(pat.is_paintable()); + d.layers.push(Layer::new( + "pattern", + LayerContent::Fill(Fill::Pattern { name: pat.name.clone(), id: pat.id.clone(), scale: 1.0, angle: 0.0, link: true, phase: (0.0, 0.0) }), + )); + d.patterns.push(pat); + assert!(render_to_vec(&mut g.comp, &g.device, &g.queue, &d, d.bounds()).is_err(), "the GPU path declines it"); + assert_eq!(g.comp.pattern_cache_bytes(), 0, "nothing was uploaded"); + assert!(photocraft_compose::flatten(&d).px.iter().all(|p| *p == [1.0, 0.0, 0.0, 1.0]), "the CPU compositor paints it"); +} + /// #4971: pattern textures are bounded while their documents stay open, by a byte budget and /// by age. What the current frame paints with is never dropped. #[test] diff --git a/crates/io/src/effects_map.rs b/crates/io/src/effects_map.rs index 8d5d24daf9..0c7f60154f 100644 --- a/crates/io/src/effects_map.rs +++ b/crates/io/src/effects_map.rs @@ -69,9 +69,15 @@ const BLEND_LONG_NAMES: [(BlendMode, &str); 17] = [ (BlendMode::Luminosity, "luminosity"), ]; -/// A blend-mode enum from either its four-character code or its long string ID. +/// A blend-mode enum from its four-character code, its long string ID, or the layer record's key: +/// Photoshop 2026 writes `vLit`, `lddg`, `lbrn`… as an effect's `Md ` for some modes (#8567). pub(crate) fn blend_from_id(v: &[u8]) -> Option { - BLEND_NAMES.iter().chain(&BLEND_LONG_NAMES).find(|(_, n)| n.as_bytes() == v).map(|(m, _)| *m) + BLEND_NAMES + .iter() + .chain(&BLEND_LONG_NAMES) + .find(|(_, n)| n.as_bytes() == v) + .map(|(m, _)| *m) + .or_else(|| <[u8; 4]>::try_from(v).ok().and_then(BlendMode::from_psd_key)) } fn blend_of(d: &Descriptor, key: &str, default: BlendMode) -> BlendMode { diff --git a/crates/io/src/effects_map/tests.rs b/crates/io/src/effects_map/tests.rs index 8df09e6241..5a43cd194e 100644 --- a/crates/io/src/effects_map/tests.rs +++ b/crates/io/src/effects_map/tests.rs @@ -239,6 +239,35 @@ fn blend_names_are_unique() { } } +#[test] +fn layer_record_blend_keys_parse() { + for (key, mode) in [ + (b"vLit", BlendMode::VividLight), + (b"lLit", BlendMode::LinearLight), + (b"pLit", BlendMode::PinLight), + (b"hMix", BlendMode::HardMix), + (b"lbrn", BlendMode::LinearBurn), + (b"lddg", BlendMode::LinearDodge), + (b"dkCl", BlendMode::DarkerColor), + (b"lgCl", BlendMode::LighterColor), + (b"fsub", BlendMode::Subtract), + (b"fdiv", BlendMode::Divide), + ] { + assert_eq!(blend_from_id(key), Some(mode), "{}", String::from_utf8_lossy(key)); + } + assert_eq!(blend_from_id(b"zzzz"), None); + // The issue's Color Overlay: Vivid Light stored as `vLit`, not left as Normal. + let key = |v: &str| Value::Enumerated { type_id: Id::new("BlnM"), value: Id::new(v) }; + let d = Descriptor::new("null") + .with("masterFXSwitch", Value::Boolean(true)) + .with("SoFi", Value::Descriptor(Descriptor::new("SoFi").with("enab", Value::Boolean(true)).with("Md ", key("vLit")))); + let mut data = 0u32.to_be_bytes().to_vec(); + data.extend(VersionedDescriptor { version: 16, descriptor: d }.to_bytes()); + let (_, fx) = parse_lfx2(&data).unwrap(); + let modes: Vec = fx.iter().filter_map(|e| if let Effect::ColorOverlay { common, .. } = e { Some(common.blend) } else { None }).collect(); + assert_eq!(modes, vec![BlendMode::VividLight]); +} + #[test] fn long_string_blend_ids_parse() { // Some writers store effect modes as long string IDs (`BlnM` `multiply`, `colorDodge`) diff --git a/crates/io/src/pattern_map.rs b/crates/io/src/pattern_map.rs index eeda1943cc..4f0b5fe54d 100644 --- a/crates/io/src/pattern_map.rs +++ b/crates/io/src/pattern_map.rs @@ -110,6 +110,12 @@ pub fn to_psd(p: &Pattern) -> PsdPattern { } } +/// A warning for each of `patterns` too large to paint: such a pattern fills with transparency, +/// so the file says so on opening. +pub fn unpaintable_warnings(patterns: &[Pattern]) -> Vec { + patterns.iter().filter_map(|p| Some(format!("pattern \"{}\" would paint nothing: {}", p.display_name(), p.paint_problem()?))).collect() +} + /// Patterns decoded from the document's raw pattern blocks (malformed blocks are skipped). All /// blocks share one memory budget, so repeating a block can't multiply it. pub fn from_global_blocks(doc: &Document) -> Vec { @@ -278,3 +284,26 @@ mod tests { assert_eq!(export_global_blocks(&back), back.metadata.psd_global_blocks); } } + +#[cfg(test)] +mod unpaintable_tests { + use super::*; + use photocraft_doc::PixelFormat; + use photocraft_raster::Surface; + + #[test] + fn a_pattern_too_large_to_paint_is_named_on_opening() { + let sparse = |name: &str, format: PixelFormat, side: u32| Pattern { + id: name.into(), + name: name.into(), + width: side, + height: side, + surface: Surface::new(format), + }; + let patterns = + [sparse("Small", PixelFormat::RGBA8, 64), sparse("Huge float", PixelFormat::RGBA32F, 5000), sparse("Largest 8-bit", PixelFormat::RGBA8, 8000)]; + let warnings = unpaintable_warnings(&patterns); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + assert!(warnings[0].contains("Huge float") && warnings[0].contains("would paint nothing") && warnings[0].contains("32 bits"), "{warnings:?}"); + } +} diff --git a/crates/io/src/psd_import.rs b/crates/io/src/psd_import.rs index 1eb0da8664..19cc08b3f8 100644 --- a/crates/io/src/psd_import.rs +++ b/crates/io/src/psd_import.rs @@ -1122,6 +1122,7 @@ fn read_initial_metadata(file: &PsdFile, global: &[PsdGlobalBlock], doc: &mut Do )); } doc.patterns = crate::pattern_map::from_global_blocks(doc); + warnings.extend(crate::pattern_map::unpaintable_warnings(&doc.patterns)); doc.notes = crate::annotations_map::notes_from_blocks(doc); if crate::annotations_map::anno_unreadable(doc) { warnings.push("the notes (Anno block) could not be read; they are kept unchanged on PSD save unless a note is added".into()); diff --git a/crates/io/src/smart_map/halftone_tests.rs b/crates/io/src/smart_map/halftone_tests.rs index 931483c55b..d4f67dbfec 100644 --- a/crates/io/src/smart_map/halftone_tests.rs +++ b/crates/io/src/smart_map/halftone_tests.rs @@ -154,6 +154,19 @@ fn halftone_pattern_unknown_nested_class_stays_verbatim() { } } +/// Keeps a Gallery stack opaque when its own descriptor holds a setting beside the effect list, +/// which a save would otherwise drop. +#[test] +fn halftone_pattern_unknown_outer_setting_stays_verbatim() { + let valid = native_effect("ScrD", 2, 5); + let gallery = Descriptor::new(GALLERY_CLASS).with("GEfs", Value::List(vec![Value::Descriptor(valid)])).with("futureSetting", Value::Integer(7)); + let item = native_item(gallery); + let parsed = Descriptor::from_bytes(&item.to_bytes()).unwrap(); + let filter = filter_from_item(&parsed); + assert_eq!(filter.command, UNSUPPORTED_FILTER); + assert_eq!(item_for_filter(&filter).unwrap().to_bytes(), item.to_bytes()); +} + /// Rejects unsupported export values instead of silently changing their meaning. #[test] fn halftone_pattern_invalid_export_settings_are_reported() { diff --git a/crates/io/src/smart_map/import.rs b/crates/io/src/smart_map/import.rs index 55bd79ec06..f8f3f50e3f 100644 --- a/crates/io/src/smart_map/import.rs +++ b/crates/io/src/smart_map/import.rs @@ -710,7 +710,11 @@ fn gallery_params(item: &Descriptor, f: &Descriptor) -> Option { let mut params = Map::new(); if g.key == "halftonePattern" { // Do not turn an unfamiliar shape or malformed setting into the engine's defaults: - // keeping the whole item opaque preserves it through a subsequent save. + // keeping the whole item opaque preserves it through a subsequent save. That covers + // the stack's own descriptor too: a key beside `GEfs` would not be written back. + if f.get("GEfs").is_some() && !(f.class_id.is(GALLERY_CLASS) && only_keys(f, &["GEfs"])) { + return None; + } if !e.class_id.is(GALLERY_CLASS) || !only_keys(e, &["GEfk", "HlSz", "Cntr", "ScrT", "GELv"]) || !matches!(e.get("GEfk"), Some(Value::Enumerated { type_id, .. }) if type_id.is("GEft")) diff --git a/crates/io/tests/it/psd_kerning.rs b/crates/io/tests/it/psd_kerning.rs index 8b85c4f3ba..d873d4a3e2 100644 --- a/crates/io/tests/it/psd_kerning.rs +++ b/crates/io/tests/it/psd_kerning.rs @@ -199,14 +199,22 @@ type Reopened = (String, Option, Vec); /// `TextIndex`, bottom first), with a type layer "Hi" made the way the Type tool makes one /// (`build_tysh`, no index of its own) inserted at `at`, saved and reopened: each layer's /// index and font features (but `calt`, on by default and written on as Photoshop does), and -/// "Hi"'s style run in the saved `Txt2`. +/// "Hi"'s style run in the saved `Txt2`. The kept layers are styled as their objects are (font +/// and size too), so their objects are saved unchanged, which this checks. fn add_type_layer(objects: &[(&str, &str)], kept: &[(&str, i32)], at: usize) -> (Vec, E) { let mut doc = Document::new("k", photocraft_geom::Size::new(64, 32), photocraft_color::ColorMode::Rgb, photocraft_color::SampleType::U8); for (text, index) in kept { let mut t = layer(text, &[(text.len(), Kerning::Metrics, 0.0)]); + // `photoshop_txt2` writes MyriadPro-Regular at 12 pt (the document is at 72 dpi). + t.runs[0].style.size_pt = 12.0; + t.runs[0].style.postscript_name = Some("MyriadPro-Regular".into()); + t.sync_summary(); photocraft_text::TextEngine::new().render_layer(&mut t, doc.resolution_dpi, doc.pixel_format()); - let raw = photocraft_text::psd::build_tysh(&t, doc.resolution_dpi, None); - t.psd_raw = Some(std::sync::Arc::new(photocraft_text::psd::set_text_index(&raw, *index).unwrap())); + let raw = photocraft_text::psd::set_text_index(&photocraft_text::psd::build_tysh(&t, doc.resolution_dpi, None), *index).unwrap(); + // As opening the file does: a kept layer's style holds what its `Txt2` object says (#8415: + // a layer whose style disagrees with its object is saved as a style edit). + photocraft_text::psd::apply_txt2(&mut t, &raw, &photocraft_text::psd::parse_txt2(&photoshop_txt2(objects)).unwrap()); + t.psd_raw = Some(std::sync::Arc::new(raw)); doc.layers.push(Layer::new(*text, LayerContent::Text(t))); } doc.metadata.psd_global_blocks = vec![(*b"8BIM", *b"Txt2", std::sync::Arc::new(photoshop_txt2(objects)))]; @@ -228,6 +236,11 @@ fn add_type_layer(objects: &[(&str, &str)], kept: &[(&str, i32)], at: usize) -> (l.name.clone(), index, t.char_runs().iter().flat_map(|r| r.style.features.iter().filter(|f| f.tag != "calt").map(|f| f.tag.clone())).collect()) }) .collect(); + let source = photocraft_text::psd::parse_txt2(&photoshop_txt2(objects)).unwrap(); + let object = |txt2: &E, i: i32| txt2.path(&["1", "1"]).and_then(E::as_array).and_then(|a| a.get(usize::try_from(i).ok()?)).cloned(); + for (text, index) in kept { + assert_eq!(object(&txt2, *index), object(&source, *index), "{text}: the kept layer's object is saved unchanged"); + } let hi = usize::try_from(layers.iter().find(|l| l.0 == "Hi").and_then(|l| l.1).unwrap()).unwrap(); let run = txt2.path(&["1", "1"]).and_then(E::as_array).unwrap()[hi].path(&["0", "6", "0"]).and_then(E::as_array).unwrap()[0].clone(); (layers, run) diff --git a/crates/io/tests/it/text_corpus.rs b/crates/io/tests/it/text_corpus.rs index 32e914d051..b0deed7746 100644 --- a/crates/io/tests/it/text_corpus.rs +++ b/crates/io/tests/it/text_corpus.rs @@ -315,3 +315,65 @@ fn created_text_layer_roundtrips_through_psd() { assert_eq!(bt.transform, t.transform); assert!(bt.cache.as_ref().is_some_and(|c| c.content_bounds().width() > 10)); } + +/// Review of #8888: the older Photoshop's text objects (`/DocumentObjects /TextObjects`) in the +/// real psd-tools `text.psd`. Saving the type layer unedited leaves them (and the font list they +/// index) exactly as the file has them; editing only the size changes the size of the legacy style +/// run and nothing else in it, and the object's text, paragraph runs and view are untouched. +#[cfg(feature = "corpus")] +#[test] +fn corpus_text_psd_legacy_objects_are_edited_in_place() { + use photocraft_text::engine_data::Value as E; + + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/psd-tools/text.psd"); + assert!(path.is_file(), "{} is missing: run `cargo xtask corpus --all`", path.display()); + let bytes = std::fs::read(&path).unwrap(); + let doc = photocraft_io::import("text.psd", &bytes).unwrap().document; + let txt2 = doc.metadata.psd_global_blocks.iter().find(|b| &b.1 == b"Txt2").map(|b| b.2.clone()).expect("text.psd has a Txt2 block"); + let mut all = Vec::new(); + walk(&doc.layers, &mut all); + let layer = all.iter().find_map(|l| if let LayerContent::Text(t) = &l.content { Some(t.clone()) } else { None }).expect("a type layer"); + + let before = photocraft_text::psd::parse_txt2(&txt2).expect("the file's Txt2 parses"); + let legacy = |block: &E| block.path(&["DocumentObjects", "TextObjects"]).and_then(E::as_array).and_then(<[E]>::first).cloned().expect("the legacy object"); + let features = |entry: &E| { + entry + .path(&["Model", "StyleRun", "RunArray"]) + .and_then(E::as_array) + .and_then(<[E]>::first) + .and_then(|r| r.path(&["RunData", "StyleSheet", "Features"])) + .cloned() + }; + let items = |value: &E| if let E::Dict(items) = value { items.clone() } else { Vec::new() }; + + // Unedited: the legacy object and the font list are as the file has them. + let unedited = photocraft_text::psd::parse_txt2(&photocraft_text::psd::build_txt2_at(&[(0, &layer)], Some(&txt2), doc.resolution_dpi)).unwrap(); + assert!(legacy(&unedited) == legacy(&before), "an unedited layer leaves the legacy text object alone"); + assert!(unedited.path(&["DocumentResources", "FontSet"]) == before.path(&["DocumentResources", "FontSet"]), "and the font list it indexes"); + + // Edited: only the size. + let mut bigger = layer.clone(); + for run in &mut bigger.runs { + run.style.size_pt += 7.0; + } + let edited = photocraft_text::psd::parse_txt2(&photocraft_text::psd::build_txt2_at(&[(0, &bigger)], Some(&txt2), doc.resolution_dpi)).unwrap(); + let (was, now) = (features(&legacy(&before)).expect("features"), features(&legacy(&edited)).expect("features")); + let size = |f: &E| f.get("FontSize").and_then(E::as_f64).expect("FontSize"); + assert!((size(&now) - size(&was) - 7.0).abs() < 1e-6, "the legacy size follows the edit: {} -> {}", size(&was), size(&now)); + let rest = |f: &E| items(f).into_iter().filter(|(k, _)| k != "FontSize").collect::>(); + let changed: Vec = + rest(&was).iter().zip(rest(&now).iter()).filter(|(a, b)| a != b).map(|(a, b)| format!("{}: {:?} -> {:?}", a.0, a.1, b.1)).collect(); + assert!(rest(&was) == rest(&now), "every other style key of the run is as the file has it; changed: {changed:?}"); + let others = |entry: &E, key: &str| entry.path(&["Model", key]).cloned(); + for key in ["Text", "ParagraphRun", "KernRun"] { + assert!(others(&legacy(&edited), key) == others(&legacy(&before), key), "{key} is untouched"); + } + assert!(legacy(&edited).get("View") == legacy(&before).get("View"), "the view is untouched"); + let lengths = |entry: &E| { + entry + .path(&["Model", "StyleRun", "RunArray"]) + .and_then(E::as_array) + .map(|rs| rs.iter().map(|r| r.get("Length").and_then(E::as_i64)).collect::>()) + }; + assert!(lengths(&legacy(&edited)) == lengths(&legacy(&before)), "the runs still cover the text"); +} diff --git a/crates/ops/src/lib.rs b/crates/ops/src/lib.rs index 822af3804c..6dacd3ec97 100644 --- a/crates/ops/src/lib.rs +++ b/crates/ops/src/lib.rs @@ -585,10 +585,10 @@ impl History { /// beyond it unseen (#4530). The most recent state is still always kept. pub fn trim_with(&mut self, current: &Document, pinned: &[&Document]) -> usize { // While Clear History can be undone, the cleared states are what history holds: the - // oldest of them go first, as the list's would. - let (undo, redo) = match &mut self.cleared { - Some(c) => (&mut c.undo, &c.redo), - None => (&mut self.undo, &self.redo), + // oldest of them go first, as the list's would. The source set before it stays pinned. + let (undo, redo, brush_source) = match &mut self.cleared { + Some(c) => (&mut c.undo, &c.redo, c.brush_source), + None => (&mut self.undo, &self.redo, self.brush_source), }; if self.max_bytes == 0 || undo.len() <= 1 { return 0; @@ -622,7 +622,7 @@ impl History { } // The History Brush's source is kept whatever its age, so its own tiles come first; the // newest states then fill what remains of the budget (the most recent one always). - let source = self.brush_source.and_then(|id| undo.iter().position(|s| s.id == id)); + let source = brush_source.and_then(|id| undo.iter().position(|s| s.id == id)); if let Some(state) = source.and_then(|i| undo.get(i)) { total = total.saturating_add(tile_bytes(&state.doc, &mut seen)); } diff --git a/crates/ops/src/tests.rs b/crates/ops/src/tests.rs index b71b2b9ec4..7cb1fc328a 100644 --- a/crates/ops/src/tests.rs +++ b/crates/ops/src/tests.rs @@ -197,6 +197,31 @@ fn clear_states_resets_the_brush_source_until_undo() { assert_eq!((h.brush_source(), h.row_doc(1, &cur).unwrap().name.as_str()), (Some(1), "a")); } +/// The byte budget keeps the source set before Clear History among the cleared states, so +/// Undo Clear History brings it back as the History Brush's source. +#[test] +fn byte_budget_keeps_the_brush_source_of_the_cleared_states() { + let mut h = History::new(50); + let mut cur = Arc::new(base()); + for i in 0..5 { + edit(&mut h, &mut cur, &format!("paint {i}"), |d| { + let id = d.layers[0].id; + d.layer_mut(id).unwrap().surface_mut().unwrap().write_pixel(1, 1, &[i as f32 / 8.0, 0.0, 0.0, 1.0]); + }); + } + assert!(h.set_brush_source(1)); + let source = h.state(1).unwrap(); + assert!(h.clear_states()); + h.max_bytes = 1; + assert_eq!(h.trim(&cur), 3); + assert_counter(&h, &cur); + assert!(h.is_cleared() && h.past_len() == 0, "the list stays cleared"); + cur = h.undo(cur).unwrap().0; + assert_eq!(h.entries(), ["paint 0", "paint 3", "paint 4"], "the source is kept with the last step"); + assert_eq!(h.brush_source(), Some(0)); + assert!(Arc::ptr_eq(&h.row_doc(0, &cur).unwrap(), &source), "the History Brush paints from the same state"); +} + #[test] fn new_edit_clears_redo() { let mut h = History::default(); diff --git a/crates/text/src/engine_data.rs b/crates/text/src/engine_data.rs index 19f3af1886..4acd08ac50 100644 --- a/crates/text/src/engine_data.rs +++ b/crates/text/src/engine_data.rs @@ -11,6 +11,10 @@ pub enum Value { Dict(Vec<(String, Value)>), Array(Vec), String(String), + /// A string the file stored as plain bytes, without the `FE FF` mark (one char per byte). + /// Kept apart from [`Value::String`] so it is written back as the same bytes: Photoshop + /// rejects a `Txt2` whose raw strings come back as UTF-16 (#9091). + RawString(String), Int(i64), Real(f64), Bool(bool), @@ -67,7 +71,7 @@ impl Value { } pub fn as_str(&self) -> Option<&str> { match self { - Value::String(s) | Value::Name(s) => Some(s), + Value::String(s) | Value::RawString(s) | Value::Name(s) => Some(s), _ => None, } } @@ -173,7 +177,7 @@ impl Parser<'_> { } } } - Ok(Value::String(decode_string(&raw))) + Ok(decode_string(&raw)) } Some(b'/') => Ok(Value::Name(self.name())), Some(_) => { @@ -205,12 +209,12 @@ impl Parser<'_> { } } -fn decode_string(raw: &[u8]) -> String { +fn decode_string(raw: &[u8]) -> Value { if raw.len() >= 2 && raw[0] == 0xFE && raw[1] == 0xFF { let u: Vec = raw[2..].as_chunks::<2>().0.iter().map(|c| u16::from_be_bytes([c[0], c[1]])).collect(); - String::from_utf16_lossy(&u) + Value::String(String::from_utf16_lossy(&u)) } else { - raw.iter().map(|&c| c as char).collect() + Value::RawString(raw.iter().map(|&c| c as char).collect()) } } @@ -293,7 +297,18 @@ fn write_value(v: &Value, depth: usize, out: &mut Vec) { } out.extend_from_slice(b" ]"); } - Value::String(s) => { + Value::RawString(s) if s.chars().all(|c| u32::from(c) <= 0xFF) => { + out.push(b'('); + for c in s.chars() { + let c = u32::from(c) as u8; + if matches!(c, b'(' | b')' | b'\\') { + out.push(b'\\'); + } + out.push(c); + } + out.push(b')'); + } + Value::String(s) | Value::RawString(s) => { out.push(b'('); let mut raw = vec![0xFE, 0xFF]; for u in s.encode_utf16() { @@ -382,6 +397,17 @@ mod tests { assert_eq!(v.path(&["EngineDict", "Runs"]).unwrap().as_array().unwrap()[0].get("A").and_then(Value::as_i64), Some(1)); } + #[test] + fn raw_strings_are_written_back_as_the_same_bytes() { + let v = parse(b"<< /2 (0100\\)) /3 (\xFE\xFF\x00h) >>").unwrap(); + assert_eq!(v.get("2"), Some(&Value::RawString("0100)".into()))); + assert_eq!(v.get("2").and_then(Value::as_str), Some("0100)")); + assert_eq!(v.get("3"), Some(&Value::String("h".into()))); + let bytes = write(&v); + assert!(bytes.windows(8).any(|w| w == b"(0100\\))"), "{:?}", String::from_utf8_lossy(&bytes)); + assert_eq!(parse(&bytes).unwrap(), v); + } + #[test] fn write_parse_roundtrip() { let v = parse(SAMPLE).unwrap(); diff --git a/crates/text/src/psd/build.rs b/crates/text/src/psd/build.rs index 9e2604cfde..4be80f27ee 100644 --- a/crates/text/src/psd/build.rs +++ b/crates/text/src/psd/build.rs @@ -156,19 +156,9 @@ fn txt2_mode_runs(layer: &TextLayer) -> Vec<(i64, usize)> { out } -/// `Txt2` style runs for `layer`, one per character run: `base` (one of the file's style runs, -/// `<< /0 << /0 << /6 style >> >> /1 length >>`, every key Photoshop wrote) with its style set -/// from the run's own: font (`/0`, an index into `fonts`, the block's `/0 /1 /0` font list, which -/// a missing font is appended to), size (`/1`), faux bold and italic, leading, scales and tracking -/// (`/2`–`/8`, EngineData's order), fill colour (`/53`), the OpenType keys and the auto-kern mode -/// (`/11`). Photoshop reads the character style from here rather than from EngineData, and quit -/// on a run holding only the auto-kern mode. `None` when `base` has no style dictionary or is -/// larger than a template may be (it is copied once per run). -fn txt2_style_runs(layer: &TextLayer, base: &E, fonts: &mut Vec, dpi: f32) -> Option> { - if !matches!(base.path(&["0", "0", "6"]), Some(E::Dict(_))) || !fits(base, &mut { TEMPLATE_NODES }) { - return None; - } - let k = if dpi.is_finite() && dpi > 0.0 { dpi / 72.0 } else { 1.0 }; +/// The engine text of `layer` (`\r` breaks and the trailing `\r`) as `(character run, UTF-16 +/// length)` spans: one per run that has text. +fn char_spans(layer: &TextLayer) -> Vec<(usize, i64)> { let runs = layer.char_runs(); let ed_text = format!("{}\r", layer.text.replace('\n', "\r")); let mut spans: Vec<(usize, i64)> = Vec::new(); @@ -184,6 +174,24 @@ fn txt2_style_runs(layer: &TextLayer, base: &E, fonts: &mut Vec, dpi: f32) -> _ => spans.push((ri, n)), } } + spans +} + +/// `Txt2` style runs for `layer`, one per character run: `base` (one of the file's style runs, +/// `<< /0 << /0 << /6 style >> >> /1 length >>`, every key Photoshop wrote) with its style set +/// from the run's own: font (`/0`, an index into `fonts`, the block's `/0 /1 /0` font list, which +/// a missing font is appended to), size (`/1`), faux bold and italic, leading, scales and tracking +/// (`/2`–`/8`, EngineData's order), fill colour (`/53`), the OpenType keys and the auto-kern mode +/// (`/11`). Photoshop reads the character style from here rather than from EngineData, and quit +/// on a run holding only the auto-kern mode. `None` when `base` has no style dictionary or is +/// larger than a template may be (it is copied once per run). +fn txt2_style_runs(layer: &TextLayer, base: &E, fonts: &mut Vec, dpi: f32) -> Option> { + if !matches!(base.path(&["0", "0", "6"]), Some(E::Dict(_))) || !fits(base, &mut { TEMPLATE_NODES }) { + return None; + } + let k = if dpi.is_finite() && dpi > 0.0 { dpi / 72.0 } else { 1.0 }; + let runs = layer.char_runs(); + let spans = char_spans(layer); let default = CharStyle::default(); let mut out = Vec::with_capacity(spans.len()); let mut looked_up = std::collections::HashMap::new(); @@ -221,6 +229,278 @@ fn txt2_style_runs(layer: &TextLayer, base: &E, fonts: &mut Vec, dpi: f32) -> Some(out) } +/// Whether the style runs of a `Txt2` text model (`/6 /0`) agree with `layer`'s character runs +/// on every key [`txt2_style_runs`] writes, the auto-kern mode aside ([`with_modes`] refits that +/// on its own): font (by name, through `fonts`, the block's font list), size, faux bold and +/// italic, leading, scales, tracking, fill colour and the OpenType keys. A key the file's run +/// lacks, or holds as another type, is no evidence of an edit and is skipped, so an unedited +/// save still gets the file's block back byte for byte (#2374); a key that disagrees means the +/// layer's style was edited since the file was written (#8415). +fn txt2_style_matches(layer: &TextLayer, model: &E, fonts: &[E], dpi: f32) -> bool { + let Some(file_runs) = model.path(&["6", "0"]).and_then(E::as_array) else { return true }; + let k = if dpi.is_finite() && dpi > 0.0 { dpi / 72.0 } else { 1.0 }; + style_runs_match(layer, file_runs, "1", &["0", "0", "6"], |s, style| txt2_style_agrees(s, style, fonts, k)) +} + +/// [`txt2_style_matches`] for the older Photoshop's object at `/DocumentObjects /TextObjects` +/// (`entry`): its `Model /StyleRun /RunArray` runs hold the style under `RunData /StyleSheet +/// /Features`, named keys, with the run length under `Length`. `fonts` is the block's +/// `DocumentResources /FontSet /Resources`. +fn td_style_matches(layer: &TextLayer, entry: &E, fonts: &[E], dpi: f32) -> bool { + let Some(file_runs) = entry.path(&["Model", "StyleRun", "RunArray"]).and_then(E::as_array) else { return true }; + let k = if dpi.is_finite() && dpi > 0.0 { dpi / 72.0 } else { 1.0 }; + style_runs_match(layer, file_runs, "Length", &["RunData", "StyleSheet", "Features"], |s, style| td_style_agrees(s, style, fonts, k)) +} + +/// Whether `file_runs` (a run array whose runs hold their length under `len_key` and their style +/// at `style_path`) agree, run for run over the layer's engine text, with the layer's character +/// runs by `agrees`. A run without a style dictionary says nothing. +fn style_runs_match(layer: &TextLayer, file_runs: &[E], len_key: &str, style_path: &[&str], agrees: impl Fn(&CharStyle, &E) -> bool) -> bool { + let run_len = |i: usize| file_runs.get(i).and_then(|r| r.get(len_key)).and_then(E::as_i64).unwrap_or(0); + let runs = layer.char_runs(); + let default = CharStyle::default(); + let ed_text = format!("{}\r", layer.text.replace('\n', "\r")); + let (mut ri, mut run_end) = (0usize, runs.first().map_or(0, |r| r.len)); + let (mut fi, mut left) = (0usize, run_len(0)); + // Each (layer run, file run) pair is compared once, however long it is. + let mut compared = None; + for (b, ch) in ed_text.char_indices() { + while b >= run_end && ri + 1 < runs.len() { + ri += 1; + run_end = run_end.saturating_add(runs.get(ri).map_or(0, |r| r.len)); + } + while left <= 0 && fi + 1 < file_runs.len() { + fi += 1; + left = run_len(fi); + } + if compared != Some((ri, fi)) { + let s = runs.get(ri).map_or(&default, |r| &r.style); + if let Some(style) = file_runs.get(fi).and_then(|r| r.path(style_path)) + && !agrees(s, style) + { + return false; + } + compared = Some((ri, fi)); + } + left = left.saturating_sub(ch.len_utf16() as i64); + } + true +} + +/// Whether one `Txt2` style dictionary agrees with `s` (see [`txt2_style_matches`]); `k` is the +/// document's points per PostScript point, as in [`txt2_style_runs`]. +fn txt2_style_agrees(s: &CharStyle, style: &E, fonts: &[E], k: f32) -> bool { + let close = |a: f64, b: f64| (a - b).abs() <= 1e-3 * a.abs().max(b.abs()).max(1.0); + let number = |key: &str, want: f32| style.get(key).and_then(E::as_f64).is_none_or(|v| close(v, f64::from(want))); + let flag = |key: &str, want: bool| style.get(key).and_then(E::as_bool).is_none_or(|v| v == want); + let font = style.get("0").and_then(E::as_i64).and_then(|i| usize::try_from(i).ok()).and_then(|i| fonts.get(i)).and_then(|f| f.path(&["0", "0", "0"])); + if font.and_then(E::as_str).is_some_and(|name| name != postscript_for(s)) { + return false; + } + let auto_leading = s.leading_pt.is_none(); + if !(number("1", s.size_pt * k) && flag("2", s.faux_bold) && flag("3", s.faux_italic) && flag("4", auto_leading)) { + return false; + } + // An automatic leading's value is Photoshop's own; only a set one is ours to compare. + if style.get("4").and_then(E::as_bool) == Some(false) && !auto_leading && !number("5", s.leading_pt.unwrap_or(0.0) * k) { + return false; + } + if !(number("6", s.horizontal_scale) && number("7", s.vertical_scale) && number("8", s.tracking.round())) { + return false; + } + if let Some(paint) = style.get("53").and_then(|p| p.get("0")) { + let (kind, values) = fill_color(&s.color); + if paint.get("0").and_then(E::as_i64).is_some_and(|v| v != kind) { + return false; + } + if let Some(have) = paint.get("1").and_then(E::as_array) { + let same = + have.len() == values.len() && have.iter().zip(&values).all(|(a, b)| matches!((a.as_f64(), b.as_f64()), (Some(a), Some(b)) if close(a, b))); + if !same { + return false; + } + } + } + let feature = |tag: &str| s.features.iter().find(|f| f.tag == tag).map(|f| f.value > 0); + if !TXT2_FEATURES.iter().all(|&(key, tag)| flag(key, feature(tag).unwrap_or(tag == "calt"))) { + return false; + } + // A mask beyond the 20 sets is ignored on import (`add_txt2_features`), so here too. + let sets = (0..20).filter(|n| feature(&format!("ss{:02}", n + 1)) == Some(true)).fold(0i64, |m, n| m | 1 << n); + style.get(TXT2_STYLISTIC_SETS).and_then(E::as_i64).filter(|m| (0..1 << 20).contains(m)).is_none_or(|m| m == sets) +} + +/// EngineData's `FontCaps` code of a style: 0 normal, 1 small caps, 2 all caps. +fn caps_code(s: &CharStyle) -> i64 { + match s.caps { + Caps::Normal => 0, + Caps::SmallCaps => 1, + Caps::AllCaps => 2, + } +} + +/// EngineData's `FontBaseline` code of a style: 0 normal, 1 superscript, 2 subscript. +fn baseline_code(s: &CharStyle) -> i64 { + match s.baseline { + FontBaseline::Normal => 0, + FontBaseline::Superscript => 1, + FontBaseline::Subscript => 2, + } +} + +/// The OpenType flags of a legacy style (`Features`), by key, and the feature each turns on: the +/// named keys of [`TXT2_FEATURES`]. +const TD_FEATURES: [(&str, &str); 7] = [ + ("ContextualLigatures", "calt"), + ("Fractions", "frac"), + ("Ordinals", "ordn"), + ("Swash", "swsh"), + ("Titling", "titl"), + ("StylisticAlternates", "salt"), + ("Ornaments", "ornm"), +]; + +/// The legacy key of the Stylistic Sets bit mask, as [`TXT2_STYLISTIC_SETS`] is for `Txt2`'s own. +const TD_STYLISTIC_SETS: &str = "StylisticSets"; + +/// The PostScript name of font `index` in a legacy font list (`DocumentResources /FontSet +/// /Resources`: `<< /Resource << /StreamTag /CoolTypeFont /Identifier << /Name (name) /Type n >> >> >>`). +fn td_font_name(fonts: &[E], index: i64) -> Option<&str> { + fonts.get(usize::try_from(index).ok()?)?.path(&["Resource", "Identifier", "Name"])?.as_str() +} + +/// Whether one legacy `Features` dictionary agrees with `s`: [`txt2_style_agrees`] for the named +/// keys. A key the file's run lacks, or holds as another type, is no evidence of an edit. +fn td_style_agrees(s: &CharStyle, style: &E, fonts: &[E], k: f32) -> bool { + let close = |a: f64, b: f64| (a - b).abs() <= 1e-3 * a.abs().max(b.abs()).max(1.0); + let number = |key: &str, want: f32| style.get(key).and_then(E::as_f64).is_none_or(|v| close(v, f64::from(want))); + let flag = |key: &str, want: bool| style.get(key).and_then(E::as_bool).is_none_or(|v| v == want); + if style.get("Font").and_then(E::as_i64).and_then(|i| td_font_name(fonts, i)).is_some_and(|name| name != postscript_for(s)) { + return false; + } + let auto_leading = s.leading_pt.is_none(); + if !(number("FontSize", s.size_pt * k) && flag("FauxBold", s.faux_bold) && flag("FauxItalic", s.faux_italic) && flag("AutoLeading", auto_leading)) { + return false; + } + if style.get("AutoLeading").and_then(E::as_bool) == Some(false) && !auto_leading && !number("Leading", s.leading_pt.unwrap_or(0.0) * k) { + return false; + } + if !(number("HorizontalScale", s.horizontal_scale) && number("VerticalScale", s.vertical_scale) && number("Tracking", s.tracking.round())) { + return false; + } + // Per run, like the rest: a baseline shift, the case and baseline position, the underline and + // strikethrough (a non-zero position is on, whichever position the file chose), the ligatures + // and the auto-kern mode. + let whole = |key: &str, want: i64| style.get(key).and_then(E::as_i64).is_none_or(|v| v == want); + let on = |key: &str, want: bool| style.get(key).and_then(E::as_i64).is_none_or(|v| (v != 0) == want); + if !(number("BaselineShift", s.baseline_shift_pt * k) + && whole("FontCaps", caps_code(s)) + && whole("FontBaseline", baseline_code(s)) + && on("UnderlinePosition", s.underline) + && on("StrikethroughPosition", s.strikethrough) + && flag("Ligatures", s.ligatures) + && flag("DiscretionaryLigatures", s.discretionary_ligatures) + && whole("AutoKern", txt2_mode(s))) + { + return false; + } + if let Some(paint) = style.get("FillColor").and_then(|p| p.get("Color")) { + let (kind, values) = fill_color(&s.color); + if paint.get("Type").and_then(E::as_i64).is_some_and(|v| v != kind) { + return false; + } + if let Some(have) = paint.get("Values").and_then(E::as_array) { + let same = + have.len() == values.len() && have.iter().zip(&values).all(|(a, b)| matches!((a.as_f64(), b.as_f64()), (Some(a), Some(b)) if close(a, b))); + if !same { + return false; + } + } + } + let feature = |tag: &str| s.features.iter().find(|f| f.tag == tag).map(|f| f.value > 0); + if !TD_FEATURES.iter().all(|&(key, tag)| flag(key, feature(tag).unwrap_or(tag == "calt"))) { + return false; + } + let sets = (0..20).filter(|n| feature(&format!("ss{:02}", n + 1)) == Some(true)).fold(0i64, |m, n| m | 1 << n); + style.get(TD_STYLISTIC_SETS).and_then(E::as_i64).filter(|m| (0..1 << 20).contains(m)).is_none_or(|m| m == sets) +} + +/// The index of the PostScript font `name` in a legacy font list, appending it (shaped like the +/// list's first entry, with its font type) when missing. `None` for an empty list. +fn td_font(fonts: &mut Vec, name: &str) -> Option { + if let Some(i) = fonts.iter().position(|f| f.path(&["Resource", "Identifier", "Name"]).and_then(E::as_str) == Some(name)) { + return i64::try_from(i).ok(); + } + let mut entry = fonts.first()?.clone(); + entry.get_mut("Resource")?.get_mut("Identifier")?.set("Name", E::String(name.to_string())); + fonts.push(entry); + i64::try_from(fonts.len() - 1).ok() +} + +/// `entry` (an object at `/DocumentObjects /TextObjects`) with its `Model /StyleRun` runs remade +/// from the layer's character runs: the entry's first style run with its `Features` set from each +/// run's own, every other key Photoshop wrote kept, as [`txt2_style_runs`] does for the numbered +/// objects. Photoshop re-renders a type layer from this copy when the file has one, so a +/// style-only edit left here showed the old style on reopen. `None` when the first run has no +/// `Features` dictionary, or is larger than a template may be: the entry is then left as the file +/// has it. +fn td_restyled(entry: &E, layer: &TextLayer, fonts: &mut Vec, dpi: f32) -> Option { + let base = entry.path(&["Model", "StyleRun", "RunArray"])?.as_array()?.first()?; + if !matches!(base.path(&["RunData", "StyleSheet", "Features"]), Some(E::Dict(_))) || !fits(base, &mut { TEMPLATE_NODES }) { + return None; + } + let k = if dpi.is_finite() && dpi > 0.0 { dpi / 72.0 } else { 1.0 }; + let runs = layer.char_runs(); + let default = CharStyle::default(); + let mut out = Vec::new(); + for (ri, n) in char_spans(layer) { + let s = runs.get(ri).map_or(&default, |r| &r.style); + let mut run = base.clone(); + let features = run.get_mut("RunData")?.get_mut("StyleSheet")?.get_mut("Features")?; + if let Some(font) = td_font(fonts, &postscript_for(s)) { + features.set("Font", E::Int(font)); + } + features.set("FontSize", real(s.size_pt * k)); + features.set("FauxBold", E::Bool(s.faux_bold)); + features.set("FauxItalic", E::Bool(s.faux_italic)); + features.set("AutoLeading", E::Bool(s.leading_pt.is_none())); + // An automatic leading's value is Photoshop's own (`.01` in the files seen): kept. + if let Some(leading) = s.leading_pt { + features.set("Leading", real(leading * k)); + } + features.set("HorizontalScale", real(s.horizontal_scale)); + features.set("VerticalScale", real(s.vertical_scale)); + features.set("Tracking", E::Int(s.tracking.round() as i64)); + features.set("AutoKern", E::Int(txt2_mode(s))); + features.set("BaselineShift", real(s.baseline_shift_pt * k)); + features.set("FontCaps", E::Int(caps_code(s))); + features.set("FontBaseline", E::Int(baseline_code(s))); + // On keeps the position the file chose (a non-zero value), else the first one. + for (key, on) in [("UnderlinePosition", s.underline), ("StrikethroughPosition", s.strikethrough)] { + let kept = features.get(key).and_then(E::as_i64).filter(|v| *v != 0).unwrap_or(1); + features.set(key, E::Int(if on { kept } else { 0 })); + } + features.set("Ligatures", E::Bool(s.ligatures)); + features.set("DiscretionaryLigatures", E::Bool(s.discretionary_ligatures)); + if let Some(paint @ E::Dict(_)) = features.get_mut("FillColor").and_then(|p| p.get_mut("Color")) { + let (kind, values) = fill_color(&s.color); + paint.set("Type", E::Int(kind)); + paint.set("Values", E::Array(values)); + } + let feature = |tag: &str| s.features.iter().find(|f| f.tag == tag).map(|f| f.value > 0); + for (key, tag) in TD_FEATURES { + features.set(key, E::Bool(feature(tag).unwrap_or(tag == "calt"))); + } + let sets = (0..20).filter(|n| feature(&format!("ss{:02}", n + 1)) == Some(true)).fold(0i64, |m, n| m | 1 << n); + features.set(TD_STYLISTIC_SETS, E::Int(sets)); + run.set("Length", E::Int(n)); + out.push(run); + } + let mut entry = entry.clone(); + entry.get_mut("Model")?.get_mut("StyleRun")?.set("RunArray", E::Array(out)); + Some(entry) +} + /// The index of the PostScript font `name` in a `Txt2` font list (`<< /0 << /99 /CoolTypeFont /// /0 << /0 name /2 type >> >> >>` entries), appending it (shaped like the list's first entry) /// when missing. `None` for an empty list: there is no entry to shape a new one on. @@ -291,6 +571,10 @@ pub fn build_txt2_at(objects: &[(i32, &TextLayer)], previous: Option<&[u8]>, dpi // in place, never grown: a layer the block has no entry for has no stale copy to fix. let mut td: Option> = prev.as_ref().and_then(|p| p.path(&["DocumentObjects", "TextObjects"])).and_then(E::as_array).map(<[E]>::to_vec); let mut td_changed = false; + // The legacy block's font list (`DocumentResources /FontSet /Resources`), which its style runs index. + let mut td_fonts: Vec = + prev.as_ref().and_then(|p| p.path(&["DocumentResources", "FontSet", "Resources"])).and_then(E::as_array).map(<[E]>::to_vec).unwrap_or_default(); + let td_fonts_before = td_fonts.len(); let mut fonts: Vec = prev.as_ref().and_then(|p| p.path(&["0", "1", "0"]).and_then(E::as_array)).map(<[E]>::to_vec).unwrap_or_default(); let fonts_before = fonts.len(); // Looked up once per save, not per layer: a crafted block with many objects must not cost @@ -333,12 +617,27 @@ pub fn build_txt2_at(objects: &[(i32, &TextLayer)], previous: Option<&[u8]>, dpi *slot = fixed; td_changed = true; } + // And its character style, when it no longer says what the layer says: a style-only + // edit (font, size, faux bold, …) left the old style here, and Photoshop reads it back. + if let Some(entry) = entries.get_mut(at) + && !td_style_matches(layer, entry, &td_fonts, dpi) + && let Some(restyled) = td_restyled(entry, layer, &mut td_fonts, dpi) + { + *entry = restyled; + td_changed = true; + } } // A stub an earlier save wrote beside Photoshop's objects counts as no object at all, so // it is replaced like a missing one (below) and re-saving such a file repairs it, whether // or not the layer's text changed. let slot = prev_objects.get(at).filter(|o| !is_stub(o) || all_stubs); - let kept = slot.filter(|o| o.get("0").and_then(|m| m.get("0")).and_then(E::as_str) == Some(text.as_str())); + // Kept only while its style runs still say what the layer says (#8415): Photoshop reads + // the character style from them, so a style-only edit (font, faux bold, size, …) must + // restyle the object the way an edited one is, or the file's old style shows on reopen. + let kept = slot.filter(|o| { + let model = o.get("0"); + model.and_then(|m| m.get("0")).and_then(E::as_str) == Some(text.as_str()) && model.is_some_and(|m| txt2_style_matches(layer, m, &fonts, dpi)) + }); // An edited object (#6968): the file's object at this slot with its text changed. It keeps // the file's structure — paragraph runs, the view naming its frame, the rest — with every // run array refitted to the new text. A bare `<< /0 << /0 text /6 runs >> >>` stub in @@ -445,6 +744,11 @@ pub fn build_txt2_at(objects: &[(i32, &TextLayer)], previous: Option<&[u8]>, dpi && let Some(objects @ E::Dict(_)) = outer.get_mut("DocumentObjects") { objects.set("TextObjects", E::Array(entries)); + if td_fonts.len() != td_fonts_before + && let Some(set @ E::Dict(_)) = outer.get_mut("DocumentResources").and_then(|r| r.get_mut("FontSet")) + { + set.set("Resources", E::Array(td_fonts)); + } } // Nothing changed: the file's own bytes (writing them back would re-encode strings and numbers). if let (Some(bytes), Some(p)) = (previous, &prev) diff --git a/crates/text/src/psd/edit.rs b/crates/text/src/psd/edit.rs index 1b1b90e0ff..ba08ab1e79 100644 --- a/crates/text/src/psd/edit.rs +++ b/crates/text/src/psd/edit.rs @@ -36,7 +36,7 @@ fn fits_budget(v: &E, nodes: &mut usize, bytes: &mut usize) -> bool { fits_budget(x, nodes, bytes) }), E::Array(items) => items.iter().all(|x| fits_budget(x, nodes, bytes)), - E::String(s) | E::Name(s) => { + E::String(s) | E::RawString(s) | E::Name(s) => { let Some(rem) = bytes.checked_sub(s.len()) else { return false }; *bytes = rem; true @@ -150,7 +150,7 @@ pub(super) fn td_edited_object(object: &E, text: &str) -> Option { let E::Dict(entries) = model else { return None }; // The text must be there to replace: refitted runs over a text the model doesn't hold would // be a half-edited object. - if !entries.iter().any(|(k, v)| k == "Text" && matches!(v, E::String(_))) { + if !entries.iter().any(|(k, v)| k == "Text" && matches!(v, E::String(_) | E::RawString(_))) { return None; } let total = utf16_len(text) as usize; diff --git a/crates/text/src/psd/import.rs b/crates/text/src/psd/import.rs index 0d0927f27f..aea712d62c 100644 --- a/crates/text/src/psd/import.rs +++ b/crates/text/src/psd/import.rs @@ -32,19 +32,32 @@ pub fn apply_txt2(layer: &mut TextLayer, tysh: &[u8], txt2: &E) { }) else { return; }; - let Some(model) = txt2.path(&["1", "1"]).and_then(E::as_array).and_then(|a| a.get(index)).and_then(|o| o.get("0")) else { - return; + // The numbered object, else the older Photoshop's at `/DocumentObjects /TextObjects` (named + // keys: run lengths under `Length`, the style under `RunData /StyleSheet /Features`, the + // auto-kern mode under `AutoKern`). + let numbered = txt2 + .path(&["1", "1"]) + .and_then(E::as_array) + .and_then(|a| a.get(index)) + .and_then(|o| o.get("0")) + .and_then(|model| Some((model.get("0").and_then(E::as_str)?, model.path(&["6", "0"]).and_then(E::as_array)?))); + let legacy = || { + let model = txt2.path(&["DocumentObjects", "TextObjects"]).and_then(E::as_array).and_then(|a| a.get(index))?.get("Model")?; + Some((model.get("Text").and_then(E::as_str)?, model.path(&["StyleRun", "RunArray"]).and_then(E::as_array)?)) }; - let Some(text) = model.get("0").and_then(E::as_str) else { - return; + let (text, style_runs, is_numbered) = match numbered { + Some((text, runs)) => (text, runs, true), + None => { + let Some((text, runs)) = legacy() else { return }; + (text, runs, false) + } }; if text.strip_suffix('\r').unwrap_or(text).replace('\r', "\n") != layer.text { return; } - let Some(style_runs) = model.path(&["6", "0"]).and_then(E::as_array) else { - return; - }; - let styles: Vec<(f64, Option<&E>)> = style_runs.iter().map(|r| (r.get("1").and_then(E::as_f64).unwrap_or(0.0), r.path(&["0", "0", "6"]))).collect(); + let (len_key, style_path, kern_key): (&str, &[&str], &str) = + if is_numbered { ("1", &["0", "0", "6"], "11") } else { ("Length", &["RunData", "StyleSheet", "Features"], "AutoKern") }; + let styles: Vec<(f64, Option<&E>)> = style_runs.iter().map(|r| (r.get(len_key).and_then(E::as_f64).unwrap_or(0.0), r.path(style_path))).collect(); let lens = utf16_to_byte_lengths(text, &styles.iter().map(|m| m.0).collect::>()); // Txt2 style per byte segment, then split the model runs at segment boundaries. let mut segs: Vec<(usize, Option<&E>)> = lens.into_iter().zip(styles.iter().map(|m| m.1)).collect(); @@ -65,13 +78,13 @@ pub fn apply_txt2(layer: &mut TextLayer, tysh: &[u8], txt2: &E) { let txt2_style = segs.get(si).and_then(|s| s.1).filter(|_| at < seg_end); // Txt2 has the exact mode (EngineData's reading also marks neighbours of manual // kerns manual, like Photoshop's legacy reader). - match txt2_style.and_then(|st| st.get("11")).and_then(E::as_i64) { + match txt2_style.and_then(|st| st.get(kern_key)).and_then(E::as_i64) { Some(0) => style.kerning = Kerning::Off, Some(1) => style.kerning = Kerning::Metrics, Some(2) => style.kerning = Kerning::Optical, _ => {} } - if let Some(st) = txt2_style { + if let Some(st) = txt2_style.filter(|_| is_numbered) { add_txt2_features(&mut style, st); } let len = piece_end - at; diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 9dd58a55d4..b63a45b4ae 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -1436,6 +1436,56 @@ fn edited_text_keeps_the_files_txt2_object() { } } +/// #8415: a style-only edit (`type.setStyle` with no text change) reaches the file's `Txt2`. +/// Photoshop reads the character style from the object's style runs, and an object whose text +/// still matched kept the file's runs, so Photoshop reopened the layer in its old font with faux +/// bold still on. A layer whose style agrees with the runs still gets the block back byte for +/// byte (#2374); one whose font or faux bold changed is restyled like an edited object, and the +/// glyph pen positions laid out in the old style are dropped. +#[test] +fn style_only_edit_restyles_the_kept_txt2_object() { + use crate::engine_data::Value as E; + // Strings in the block are UTF-16 with a BOM; every character here is below U+0100. + let utf16 = |s: &str| format!("(\u{fe}\u{ff}{})", s.chars().map(|c| format!("\0{c}")).collect::()); + let font = |name: &str| format!("<< /0 << /99 /CoolTypeFont /0 << /0 {} /2 1 >> >> >> ", utf16(name)); + // One object as Photoshop writes it: paragraph runs, style runs, a view and pen positions. + let prev = format!( + "\n\n/98 << /0 14 >> /0 << /1 << /0 [ {}{}] >> >> /1 << /1 [ << /0 << /0 {} /5 << /0 [ << /0 << /0 << /0 (\u{fe}\u{ff}) /5 << /7 1.2 >> >> >> /1 3 >> ] >> /6 << /0 [ << /0 << /0 << /0 (\u{fe}\u{ff}) /6 << /0 0 /1 12.0 /2 true /3 false /11 1 >> >> >> /1 3 >> ] >> >> /1 << /0 [ << /0 0 >> ] >> /21 << /1 [ 1.25 2.5 ] >> >> ] >>", + font("Arial-Black"), + font("ArialMT"), + utf16("AV\r"), + ); + let prev: Vec = prev.chars().map(|c| c as u32 as u8).collect(); + let style = CharStyle { postscript_name: Some("Arial-Black".into()), size_pt: 12.0, faux_bold: true, ..Default::default() }; + let t = styled("AV", style.clone()); + assert_eq!(crate::psd::build_txt2(&[(0, &t)], Some(&prev)), prev, "an unedited style keeps the file's block"); + + let font_name = |out: &E, run: &E| { + let i = run.path(&["0", "0", "6", "0"]).and_then(E::as_i64).and_then(|i| usize::try_from(i).ok()).expect("a font index"); + out.path(&["0", "1", "0"]).and_then(E::as_array).and_then(|f| f.get(i)).and_then(|f| f.path(&["0", "0", "0"])).and_then(E::as_str).map(str::to_owned) + }; + for (what, edited, want_font) in [ + ("faux bold off", CharStyle { faux_bold: false, ..style.clone() }, "Arial-Black"), + ( + "font and faux bold", + CharStyle { postscript_name: Some("AppleSDGothicNeo-Heavy".into()), faux_bold: false, ..style.clone() }, + "AppleSDGothicNeo-Heavy", + ), + ] { + let t = styled("AV", edited); + let out = crate::psd::parse_txt2(&crate::psd::build_txt2(&[(0, &t)], Some(&prev))).unwrap(); + let object = &out.path(&["1", "1"]).and_then(E::as_array).unwrap()[0]; + assert_eq!(object.path(&["0", "0"]).and_then(E::as_str), Some("AV\r"), "{what}: the text is unchanged"); + let runs = object.path(&["0", "6", "0"]).and_then(E::as_array).unwrap(); + assert!(!runs.is_empty(), "{what}: the object has style runs"); + for run in runs { + assert_eq!(run.path(&["0", "0", "6", "2"]).and_then(E::as_bool), Some(false), "{what}: faux bold is off in Txt2"); + assert_eq!(font_name(&out, run).as_deref(), Some(want_font), "{what}: Txt2 names the layer's font"); + } + assert!(object.get("21").is_none(), "{what}: pen positions of the old style are dropped"); + } +} + /// A type layer the file's `Txt2` has no object for (one added before the save, #7698) gets an /// object shaped like the file's own, with a view naming its own frame and a frame in `/0 /8 /0`, /// instead of a bare stub: Photoshop and Photopea refused the file when the frames ran out. @@ -3158,3 +3208,285 @@ fn per_colour_passes_match_full_size_passes() { } } } + +/// A legacy (Photoshop-era) `Txt2` block shaped like a real one (psd-tools' `text.psd`): the +/// fonts in `DocumentResources /FontSet /Resources`, and each object's `Model /StyleRun` runs +/// holding their style under `RunData /StyleSheet /Features` (named keys) beside `Length`. +fn styled_era_txt2(text: &str, font: &str, size: f64, auto_kern: i64) -> Vec { + use crate::engine_data::Value as E; + let font_entry = |name: &str| { + E::Dict(vec![( + "Resource".into(), + E::Dict(vec![ + ("StreamTag".into(), E::Name("CoolTypeFont".into())), + ("Identifier".into(), E::Dict(vec![("Name".into(), E::String(name.into())), ("Type".into(), E::Int(1))])), + ]), + )]) + }; + let features = E::Dict(vec![ + ("Font".into(), E::Int(0)), + ("FontSize".into(), E::Real(size)), + ("FauxBold".into(), E::Bool(false)), + ("FauxItalic".into(), E::Bool(false)), + ("AutoLeading".into(), E::Bool(true)), + ("Leading".into(), E::Real(0.01)), + ("HorizontalScale".into(), E::Real(1.0)), + ("VerticalScale".into(), E::Real(1.0)), + ("Tracking".into(), E::Int(0)), + ("AutoKern".into(), E::Int(auto_kern)), + ("BaselineShift".into(), E::Real(0.0)), + ("FontCaps".into(), E::Int(0)), + ("FontBaseline".into(), E::Int(0)), + ("UnderlinePosition".into(), E::Int(0)), + ("StrikethroughPosition".into(), E::Int(0)), + ("Ligatures".into(), E::Bool(true)), + ("DiscretionaryLigatures".into(), E::Bool(false)), + ("ContextualLigatures".into(), E::Bool(true)), + ("StylisticSets".into(), E::Int(0)), + ( + "FillColor".into(), + E::Dict(vec![ + ("StreamTag".into(), E::Name("SimplePaint".into())), + ( + "Color".into(), + E::Dict(vec![("Type".into(), E::Int(1)), ("Values".into(), E::Array(vec![E::Real(1.0), E::Real(0.0), E::Real(0.0), E::Real(0.0)]))]), + ), + ]), + ), + ]); + let total = format!("{text}\r").encode_utf16().count() as i64; + let run = |len: i64, data: E| E::Dict(vec![("RunData".into(), data), ("Length".into(), E::Int(len))]); + let placeholder = || E::Dict(vec![("0".into(), E::String(String::new()))]); + let style_data = E::Dict(vec![( + "StyleSheet".into(), + E::Dict(vec![("Name".into(), E::String(String::new())), ("Parent".into(), E::Int(0)), ("Features".into(), features)]), + )]); + let entry = E::Dict(vec![ + ( + "Model".into(), + E::Dict(vec![ + ("Text".into(), E::String(format!("{text}\r"))), + ("ParagraphRun".into(), E::Dict(vec![("RunArray".into(), E::Array(vec![run(total, placeholder())]))])), + ("StyleRun".into(), E::Dict(vec![("RunArray".into(), E::Array(vec![run(total, style_data)]))])), + ("KernRun".into(), E::Dict(vec![("RunArray".into(), E::Array(vec![run(total, placeholder())]))])), + ]), + ), + ("View".into(), E::Dict(vec![("0".into(), E::Array(vec![E::Dict(vec![("0".into(), E::Int(0))])]))])), + ]); + crate::engine_data::write_bare(&[ + ("DocumentResources".into(), E::Dict(vec![("FontSet".into(), E::Dict(vec![("Resources".into(), E::Array(vec![font_entry(font)]))]))])), + ("DocumentObjects".into(), E::Dict(vec![("TextObjects".into(), E::Array(vec![entry]))])), + ]) +} + +/// The length and `Features` of each style run of the first legacy object. +fn era_runs(block: &crate::engine_data::Value) -> Vec<(i64, crate::engine_data::Value)> { + use crate::engine_data::Value as E; + block + .path(&["DocumentObjects", "TextObjects"]) + .and_then(E::as_array) + .and_then(<[E]>::first) + .and_then(|e| e.path(&["Model", "StyleRun", "RunArray"])) + .and_then(E::as_array) + .map(|rs| { + rs.iter() + .map(|r| (r.get("Length").and_then(E::as_i64).unwrap_or(-1), r.path(&["RunData", "StyleSheet", "Features"]).cloned().unwrap_or(E::Int(0)))) + .collect() + }) + .unwrap_or_default() +} + +/// Review of #8888 (CodeRabbit, psd.rs): a style-only edit (no text change) restyles the older +/// Photoshop's copy too, which Photoshop re-renders from. Its font list gains the new font, the +/// run keeps every key Photoshop wrote, and the lengths still cover the text. +#[test] +fn a_style_only_edit_restyles_the_photoshop_era_objects() { + use crate::engine_data::Value as E; + let prev = styled_era_txt2("Line 1", "ArialMT", 13.0, 1); + let style = CharStyle { + postscript_name: Some("Courier".into()), + size_pt: 20.0, + faux_bold: true, + color: Color::rgb(0.0, 0.5, 1.0), + tracking: 25.0, + ..Default::default() + }; + let out = crate::psd::parse_txt2(&crate::psd::build_txt2(&[(0, &styled("Line 1", style))], Some(&prev))).unwrap(); + let runs = era_runs(&out); + assert_eq!(runs.len(), 1); + let (length, f) = &runs[0]; + assert_eq!(*length, 7, "the run covers the text and its trailing return"); + assert_eq!(f.get("FontSize").and_then(E::as_f64), Some(20.0)); + assert_eq!(f.get("FauxBold").and_then(E::as_bool), Some(true)); + assert_eq!(f.get("Tracking").and_then(E::as_i64), Some(25)); + assert_eq!(f.get("Leading").and_then(E::as_f64), Some(0.01), "an automatic leading keeps Photoshop's own value"); + assert_eq!(f.path(&["FillColor", "Color", "Type"]).and_then(E::as_i64), Some(1)); + assert_eq!(f.path(&["FillColor", "Color", "Values"]).and_then(E::as_array).map(<[E]>::len), Some(4)); + assert_eq!(f.path(&["FillColor", "StreamTag"]).and_then(E::as_str), Some("SimplePaint"), "a key the file held is kept"); + // The font went into the block's list and the run names it. + let fonts = out.path(&["DocumentResources", "FontSet", "Resources"]).and_then(E::as_array).expect("the font list"); + let index = f.get("Font").and_then(E::as_i64).and_then(|i| usize::try_from(i).ok()).expect("the font index"); + let name = |i: usize| fonts.get(i).and_then(|r| r.path(&["Resource", "Identifier", "Name"])).and_then(E::as_str); + assert_eq!(name(index), Some("Courier")); + assert_eq!(name(0), Some("ArialMT"), "the file's font stays first"); + // The text did not change. + let text = + out.path(&["DocumentObjects", "TextObjects"]).and_then(E::as_array).and_then(<[E]>::first).and_then(|e| e.path(&["Model", "Text"])).and_then(E::as_str); + assert_eq!(text, Some("Line 1\r")); +} + +/// A layer whose style already agrees with the file's run leaves the legacy objects as they are. +#[test] +fn an_unedited_style_leaves_the_photoshop_era_objects_alone() { + let prev = styled_era_txt2("Line 1", "ArialMT", 13.0, 1); + let same = CharStyle { postscript_name: Some("ArialMT".into()), size_pt: 13.0, ..Default::default() }; + let out = crate::psd::build_txt2(&[(0, &styled("Line 1", same))], Some(&prev)); + assert_eq!(era_runs(&crate::psd::parse_txt2(&prev).unwrap()), era_runs(&crate::psd::parse_txt2(&out).unwrap())); +} + +/// Both object lists, as CodeRabbit asked: the numbered object and the older copy are restyled by +/// the same style-only edit. +#[test] +fn a_style_only_edit_restyles_both_object_lists() { + use crate::engine_data::Value as E; + let mut prev = photoshop_txt2(&[(4, 1)]); + prev.extend_from_slice(&styled_era_txt2("AVA", "ArialMT", 13.0, 1)); + let style = CharStyle { postscript_name: Some("ArialMT".into()), size_pt: 31.0, faux_italic: true, ..Default::default() }; + let out = crate::psd::parse_txt2(&crate::psd::build_txt2(&[(0, &styled("AVA", style))], Some(&prev))).unwrap(); + let legacy = era_runs(&out); + let first = legacy.first().map(|(l, f)| (*l, f.get("FontSize").and_then(E::as_f64), f.get("FauxItalic").and_then(E::as_bool))); + assert_eq!(first, Some((4, Some(31.0), Some(true)))); + let numbered = out + .path(&["1", "1"]) + .and_then(E::as_array) + .and_then(<[E]>::first) + .and_then(|o| o.path(&["0", "6", "0"])) + .and_then(E::as_array) + .and_then(<[E]>::first) + .and_then(|r| r.path(&["0", "0", "6"])); + assert_eq!(numbered.and_then(|s| s.get("1")).and_then(E::as_f64), Some(31.0), "the numbered object is restyled too"); +} + +/// Two character runs give two style runs in the legacy copy, with the lengths of the runs. +#[test] +fn a_two_run_layer_gives_the_photoshop_era_copy_two_style_runs() { + use crate::engine_data::Value as E; + let prev = styled_era_txt2("Hello", "ArialMT", 13.0, 1); + let small = CharStyle { postscript_name: Some("ArialMT".into()), size_pt: 10.0, ..Default::default() }; + let big = CharStyle { postscript_name: Some("ArialMT".into()), size_pt: 40.0, ..Default::default() }; + let t = runs_of("Hello", &[(2, small), (3, big)]); + let runs = era_runs(&crate::psd::parse_txt2(&crate::psd::build_txt2(&[(0, &t)], Some(&prev))).unwrap()); + let seen: Vec<(i64, Option)> = runs.iter().map(|(l, f)| (*l, f.get("FontSize").and_then(E::as_f64))).collect(); + assert_eq!(seen, vec![(2, Some(10.0)), (4, Some(40.0))], "the last run takes the trailing return"); +} + +/// Review of #8888: every key the legacy restyle writes is written per run, so a multi-run edit +/// does not copy the first run's baseline shift, case, underline or strikethrough to the others. +#[test] +fn each_photoshop_era_style_run_gets_its_own_values() { + use crate::engine_data::Value as E; + use photocraft_doc::text::{Caps, FontBaseline}; + let prev = styled_era_txt2("Hello", "ArialMT", 13.0, 1); + let base = CharStyle { postscript_name: Some("ArialMT".into()), size_pt: 13.0, ..Default::default() }; + let decorated = CharStyle { baseline_shift_pt: 3.0, caps: Caps::SmallCaps, baseline: FontBaseline::Superscript, underline: true, ..base.clone() }; + let struck = CharStyle { strikethrough: true, caps: Caps::AllCaps, ligatures: false, discretionary_ligatures: true, ..base.clone() }; + let t = runs_of("Hello", &[(2, decorated), (2, base.clone()), (1, struck)]); + let runs = era_runs(&crate::psd::parse_txt2(&crate::psd::build_txt2(&[(0, &t)], Some(&prev))).unwrap()); + let key = |i: usize, k: &str| runs.get(i).and_then(|(_, f)| f.get(k)).cloned(); + assert_eq!(runs.iter().map(|r| r.0).collect::>(), vec![2, 2, 2]); + assert_eq!(key(0, "BaselineShift").and_then(|v| v.as_f64()), Some(3.0)); + assert_eq!(key(1, "BaselineShift").and_then(|v| v.as_f64()), Some(0.0), "the second run is not the first"); + assert_eq!([0, 1, 2].map(|i| key(i, "FontCaps").and_then(|v| v.as_i64())), [Some(1), Some(0), Some(2)]); + assert_eq!([0, 1, 2].map(|i| key(i, "FontBaseline").and_then(|v| v.as_i64())), [Some(1), Some(0), Some(0)]); + assert_eq!([0, 1, 2].map(|i| key(i, "UnderlinePosition").and_then(|v| v.as_i64()).map(|v| v != 0)), [Some(true), Some(false), Some(false)]); + assert_eq!([0, 1, 2].map(|i| key(i, "StrikethroughPosition").and_then(|v| v.as_i64()).map(|v| v != 0)), [Some(false), Some(false), Some(true)]); + assert_eq!([0, 1, 2].map(|i| key(i, "Ligatures").and_then(|v| v.as_bool())), [Some(true), Some(true), Some(false)]); + assert_eq!([0, 1, 2].map(|i| key(i, "DiscretionaryLigatures").and_then(|v| v.as_bool())), [Some(false), Some(false), Some(true)]); + let _ = E::Int(0); +} + +/// Each of those keys alone is enough to restyle the legacy copy (an edit of only that key), and +/// a layer that agrees on all of them leaves the block's bytes alone. +#[test] +fn every_legacy_style_key_is_compared() { + use photocraft_doc::text::{Caps, FontBaseline, Kerning}; + let prev = styled_era_txt2("Hello", "ArialMT", 13.0, 1); + let base = CharStyle { postscript_name: Some("ArialMT".into()), size_pt: 13.0, ..Default::default() }; + let unedited = crate::psd::build_txt2(&[(0, &styled("Hello", base.clone()))], Some(&prev)); + assert!( + era_runs(&crate::psd::parse_txt2(&unedited).unwrap()) == era_runs(&crate::psd::parse_txt2(&prev).unwrap()), + "an agreeing layer leaves the legacy copy as it is" + ); + let edits = [ + CharStyle { baseline_shift_pt: 2.0, ..base.clone() }, + CharStyle { caps: Caps::AllCaps, ..base.clone() }, + CharStyle { baseline: FontBaseline::Subscript, ..base.clone() }, + CharStyle { underline: true, ..base.clone() }, + CharStyle { strikethrough: true, ..base.clone() }, + CharStyle { ligatures: false, ..base.clone() }, + CharStyle { discretionary_ligatures: true, ..base.clone() }, + CharStyle { kerning: Kerning::Optical, ..base.clone() }, + ]; + for (i, edit) in edits.into_iter().enumerate() { + let out = crate::psd::build_txt2(&[(0, &styled("Hello", edit))], Some(&prev)); + assert!(era_runs(&crate::psd::parse_txt2(&out).unwrap()) != era_runs(&crate::psd::parse_txt2(&prev).unwrap()), "edit {i} restyles the legacy copy"); + } +} + +/// Review of #8888: the legacy `AutoKern` mode is read on import, as `/11` is for the numbered +/// objects, so the layer a legacy file opens as agrees with its legacy copy. +#[test] +fn legacy_auto_kern_is_read_on_import() { + use photocraft_doc::text::Kerning; + let t = styled("AVA", CharStyle::default()); + let tysh = with_text_index(&crate::psd::build_tysh(&t, 72.0, None), 0); + for (mode, want) in [(0, Kerning::Off), (1, Kerning::Metrics), (2, Kerning::Optical)] { + let txt2 = crate::psd::parse_txt2(&styled_era_txt2("AVA", "ArialMT", 13.0, mode)).unwrap(); + let mut back = crate::psd::text_layer_from_tysh(&tysh, 72.0).unwrap(); + crate::psd::apply_txt2(&mut back, &tysh, &txt2); + assert!( + back.char_runs().iter().all(|r| r.style.kerning == want), + "AutoKern {mode}: {:?}", + back.char_runs().iter().map(|r| r.style.kerning).collect::>() + ); + } + // Other text, or another object, changes nothing. + let txt2 = crate::psd::parse_txt2(&styled_era_txt2("AVX", "ArialMT", 13.0, 2)).unwrap(); + let mut back = crate::psd::text_layer_from_tysh(&tysh, 72.0).unwrap(); + crate::psd::apply_txt2(&mut back, &tysh, &txt2); + assert!(back.char_runs().iter().all(|r| r.style.kerning == Kerning::Metrics)); +} + +/// Review of the batch (FredApps, with #9104): a style-only save rewrites `Txt2`, and the raw +/// (non-BOM) strings Photoshop wrote in it, `/2 (0100)` and `/3 (01110)` under `/98`, must come +/// back as the same bytes: re-encoded as `FE FF` UTF-16 they make Photoshop refuse the file. +#[test] +fn a_style_only_save_keeps_the_raw_strings_of_the_file() { + let mut prev = photoshop_txt2(&[(4, 1)]); + let version = b"/98 << /0 14 >>"; + let at = prev.windows(version.len()).position(|w| w == version).expect("the version key"); + prev.splice(at..at + version.len(), b"/98 << /0 14 /2 (0100) /3 (01110) >>".iter().copied()); + // A size the file's runs do not have: the block is rewritten. + let style = CharStyle { size_pt: 31.0, faux_bold: false, ..Default::default() }; + let out = crate::psd::build_txt2(&[(0, &styled("AVA", style))], Some(&prev)); + assert_ne!(out, prev, "the style edit rewrites the block"); + let has = |needle: &[u8]| out.windows(needle.len()).any(|w| w == needle); + assert!(has(b"(0100)") && has(b"(01110)"), "the raw strings come back as the same bytes: {:?}", String::from_utf8_lossy(&out)); + assert!(!has(b"(\xfe\xff\x000\x001\x000\x000)"), "and not as UTF-16"); +} + +/// The same for the older Photoshop's objects: restyling the legacy copy leaves a raw string +/// elsewhere in the block as the file wrote it. +#[test] +fn restyling_the_photoshop_era_objects_keeps_the_raw_strings_of_the_file() { + use crate::engine_data::Value as E; + let era = crate::psd::parse_txt2(&styled_era_txt2("Line 1", "ArialMT", 13.0, 1)).unwrap(); + let E::Dict(mut items) = era else { panic!("a dictionary") }; + items.push(("98".into(), E::Dict(vec![("0".into(), E::Int(14)), ("2".into(), E::RawString("0100".into()))]))); + let prev = crate::engine_data::write_bare(&items); + let style = CharStyle { postscript_name: Some("ArialMT".into()), size_pt: 24.0, ..Default::default() }; + let out = crate::psd::build_txt2(&[(0, &styled("Line 1", style))], Some(&prev)); + assert!(out.windows(6).any(|w| w == b"(0100)"), "{:?}", String::from_utf8_lossy(&out)); + let runs = era_runs(&crate::psd::parse_txt2(&out).unwrap()); + assert_eq!(runs.first().and_then(|(_, f)| f.get("FontSize")).and_then(E::as_f64), Some(24.0), "and the legacy copy was restyled"); +} diff --git a/crates/ui-egui/examples/snapshot.rs b/crates/ui-egui/examples/snapshot.rs index 7fa07a10bc..8429b1c466 100644 --- a/crates/ui-egui/examples/snapshot.rs +++ b/crates/ui-egui/examples/snapshot.rs @@ -22,6 +22,8 @@ //! `--right-click-at X,Y` opens a screen-space context menu after the script, including panel //! and document-tab menus that are outside the document-coordinate control pointer. //! `--click-at X,Y` opens a screen-space menu (for example the top Select menu) after the script. +//! `--hover-at X,Y` leaves the pointer there without a click, for what a tool shows under it (the +//! brush outline, the Clone Source overlay). use photocraft_ui_egui::control::{ControlRequest, Outcome, handle}; use photocraft_ui_egui::{PhotocraftApp, Services}; @@ -170,6 +172,10 @@ fn main() { harness.event(egui::Event::PointerButton { pos, button, pressed: false, modifiers: egui::Modifiers::NONE }); harness.run_steps(4); } + if let Some((x, y)) = arg(&args, "--hover-at").and_then(|s| s.split_once(',').and_then(|(x, y)| Some((x.parse::().ok()?, y.parse::().ok()?)))) { + harness.event(egui::Event::PointerMoved(egui::pos2(x, y))); + harness.run_steps(4); + } let t_settle = std::time::Instant::now(); while t_settle.elapsed() < std::time::Duration::from_millis(settle_ms) { harness.step(); diff --git a/crates/ui-egui/src/canvas/strokes.rs b/crates/ui-egui/src/canvas/strokes.rs index 92b8ee2802..32e9b19fcc 100644 --- a/crates/ui-egui/src/canvas/strokes.rs +++ b/crates/ui-egui/src/canvas/strokes.rs @@ -6,14 +6,73 @@ use super::{Color32, DRect, PhotocraftApp, Pos2, Stroke, Tool, json, pos2}; pub(crate) struct ClonePreviewCache { pub(super) key: String, - texture: egui::TextureHandle, + /// The brush a clipped overlay took its coverage from: a hardness or tip change redraws it. + pub(super) brush: Option, + /// The document area the texture covers. + pub(super) rect: DRect, + pub(super) texture: egui::TextureHandle, } -/// A transient source image beneath the brush outline; never part of the document render. +/// The most document pixels one dab's coverage is rendered over for the overlay: a 1000 px brush, +/// the largest the overlay draws, at hardness 0 reaches about 1.73 × its radius (`tip_reach`). +const CLONE_PREVIEW_MAX_PIXELS: u64 = 1800 * 1800; + +/// The largest side `retouch_cmds::clone_preview_periodic` samples, for the overlay's area and for +/// the source area it reads (`Mapping::source_rect`: the overlay's, mapped, with a 2 px apron). +const CLONE_PREVIEW_MAX_SIDE: u32 = 1024; + +/// The coverage of one dab of `brush` at `at`, as the brush engine paints it with opacity and flow +/// at full: its tip, angle, roundness and hardness (a soft tip reaches past its radius). The area +/// it covers and its `width × height` coverage values in 0..1. A very large soft tip, or a source +/// the Clone Source panel scales down (which reads a larger area than it covers), is cropped +/// around `at` to the sampling budget, keeping the middle of the dab. +fn clone_dab_coverage( + brush: &photocraft_paint::BrushSettings, + at: [f64; 2], + map: &photocraft_engine::presets::clone_source::Mapping, +) -> Option<(DRect, Vec)> { + let brush = photocraft_paint::BrushSettings { opacity: 1.0, flow: 1.0, erase: false, ..brush.clone() }; + let stroke = photocraft_paint::Stroke { brush, points: vec![photocraft_paint::StrokePoint::new(at[0], at[1], 1.0)] }; + let (rect, cov) = photocraft_paint::retouch::stroke_coverage_within(&stroke, CLONE_PREVIEW_MAX_PIXELS)?; + let fits = |r: DRect| r.width() <= CLONE_PREVIEW_MAX_SIDE && r.height() <= CLONE_PREVIEW_MAX_SIDE; + let (cx, cy) = (at[0].floor() as i32, at[1].floor() as i32); + let around = |half: i32| rect.intersect(&DRect::new(cx.saturating_sub(half), cy.saturating_sub(half), cx.saturating_add(half), cy.saturating_add(half))); + // The source area scales with the overlay's: shrink by what overflows, a few times for the + // apron's rounding. + let mut half = i32::try_from((CLONE_PREVIEW_MAX_SIDE - 4) / 2).unwrap_or(510); + let mut window = around(half); + for _ in 0..8 { + let src = map.source_rect(window); + if fits(window) && fits(src) { + break; + } + let over = f64::from(src.width().max(src.height())) / f64::from(CLONE_PREVIEW_MAX_SIDE - 4); + half = ((f64::from(half) / over.max(1.0)).floor() as i32).saturating_sub(2).max(1); + window = around(half); + } + if window.is_empty() || !fits(window) || !fits(map.source_rect(window)) { + return None; + } + if window == rect { + return Some((rect, cov)); + } + let stride = rect.width() as usize; + let mut cropped = Vec::with_capacity(window.width() as usize * window.height() as usize); + for y in window.y0..window.y1 { + let row = (y - rect.y0) as usize * stride; + let from = row + (window.x0 - rect.x0) as usize; + cropped.extend_from_slice(cov.get(from..from + window.width() as usize)?); + } + Some((window, cropped)) +} + +/// A transient source image beneath the brush outline; never part of the document render. The +/// Clone Source panel drives the Clone Stamp and the Healing Brush alike, so both show it. pub(super) fn draw_clone_preview(app: &mut PhotocraftApp, painter: &egui::Painter, xf: &ViewXform, at: [f64; 2], output: Option) { use photocraft_engine::presets::clone_source::{Mapping, overlay_blend, transform_matrix}; let overlay = app.session.presets.clone.overlay.clone(); - if app.ui.tool != Tool::CloneStamp || !overlay.show || (overlay.auto_hide && app.drag.is_some()) { + let tool = app.ui.tool; + if !matches!(tool, Tool::CloneStamp | Tool::Healing) || !overlay.show || (overlay.auto_hide && app.drag.is_some()) { return; } let slot = app.session.presets.clone.active().clone(); @@ -22,30 +81,43 @@ pub(super) fn draw_clone_preview(app: &mut PhotocraftApp, painter: &egui::Painte if !radius.is_finite() || radius <= 0.0 || radius > 500.0 || !at.iter().all(|v| v.is_finite() && v.abs() < 1_000_000.0) { return; } + // View › Pattern Preview repeats the source for the Clone Stamp only (`paint.cloneStamp`'s + // `wrap`); the Healing Brush samples it as it is. + let wrap = tool == Tool::CloneStamp && app.ui.view.pattern_preview; let stroke_start = app.drag.as_ref().map(|d| d.start); let anchor = if app.ui.tool_options.clone_aligned { slot.anchor.or(stroke_start) } else { stroke_start }.unwrap_or(at); let map = Mapping { source: (source[0], source[1]), anchor: (anchor[0], anchor[1]), m: transform_matrix(slot.scale, slot.rotation, slot.flip_h, slot.flip_v) }; - let rect = DRect::new((at[0] - radius).floor() as i32, (at[1] - radius).floor() as i32, (at[0] + radius).ceil() as i32, (at[1] + radius).ceil() as i32); let Some(st) = app.session.active() else { return }; let (display, display_key) = canvas_display(app, &st.doc, output); - // A shaped tip (sampled bitmap, or a rotated / flattened round one) clips the overlay to its - // outline instead of a circle, as Photoshop does with Clipped on. - let tip_rings = if overlay.clipped { crate::brush_cursor::rings(painter.ctx(), &app.session.tools.brush, true) } else { None }; - let tip_key = tip_rings.as_ref().map(|r| { - use std::hash::{Hash, Hasher}; - let mut h = std::collections::hash_map::DefaultHasher::new(); - r.iter().flatten().for_each(|p| (p[0].to_bits(), p[1].to_bits()).hash(&mut h)); - (h.finish(), radius.to_bits(), at.map(f64::to_bits)) - }); let key = format!( - "{:?}:{}:{:?}:{rect:?}:{map:?}:{}:{}:{}:{}:{}:{tip_key:?}", - st.doc.id, st.revision, st.active_layer, app.ui.tool_options.clone_sample, display_key, overlay.invert, overlay.blend, app.ui.view.pattern_preview + "{:?}:{}:{:?}:{:?}:{map:?}:{}:{}:{}:{}:{wrap}:{}:{}", + st.doc.id, + st.revision, + st.active_layer, + at.map(f64::to_bits), + app.ui.tool_options.clone_sample, + display_key, + overlay.invert, + overlay.blend, + overlay.clipped, + radius.to_bits() ); - if app.clone_preview.as_ref().is_none_or(|c| c.key != key) { - let Ok(mut buf) = - photocraft_engine::retouch_cmds::clone_preview_periodic(&app.session, rect, &map, &app.ui.tool_options.clone_sample, app.ui.view.pattern_preview) - else { + let brush = overlay.clipped.then_some(&app.session.tools.brush); + if app.clone_preview.as_ref().is_none_or(|c| c.key != key || c.brush.as_ref() != brush) { + // Clipped (Photoshop's default): the dab the tool paints here, so a soft brush previews + // soft and a shaped tip keeps its outline. Unclipped: the brush's whole square. + let (rect, coverage) = match brush { + Some(b) => { + let Some((rect, coverage)) = clone_dab_coverage(b, at, &map) else { return }; + (rect, Some(coverage)) + } + None => ( + DRect::new((at[0] - radius).floor() as i32, (at[1] - radius).floor() as i32, (at[0] + radius).ceil() as i32, (at[1] + radius).ceil() as i32), + None, + ), + }; + let Ok(mut buf) = photocraft_engine::retouch_cmds::clone_preview_periodic(&app.session, rect, &map, &app.ui.tool_options.clone_sample, wrap) else { return; }; if overlay.invert { @@ -59,55 +131,43 @@ pub(super) fn draw_clone_preview(app: &mut PhotocraftApp, painter: &egui::Painte // composite, whatever Sample reads), so Difference turns black where they line up. if overlay.blend != "normal" { let here = Mapping { source: (0.0, 0.0), anchor: (0.0, 0.0), m: [1.0, 0.0, 0.0, 1.0] }; - let Ok(under) = photocraft_engine::retouch_cmds::clone_preview_periodic(&app.session, rect, &here, "all", app.ui.view.pattern_preview) else { + let Ok(under) = photocraft_engine::retouch_cmds::clone_preview_periodic(&app.session, rect, &here, "all", wrap) else { return; }; for (px, d) in buf.px.iter_mut().zip(&under.px) { *px = overlay_blend(&overlay.blend, *px, *d); } } - if let Some(rings) = &tip_rings { - let mask = crate::brush_cursor::tip_mask(rings, at, radius, [rect.x0, rect.y0, rect.x1, rect.y1]); - for (px, inside) in buf.px.iter_mut().zip(mask) { - if !inside { - *px = [0.0; 4]; - } + if let Some(coverage) = &coverage { + for (px, c) in buf.px.iter_mut().zip(coverage) { + px[3] *= c.clamp(0.0, 1.0); } } let image = display_image(display.as_deref(), &buf); + let brush = brush.cloned(); match app.clone_preview.as_mut() { Some(cache) => { cache.texture.set(image, canvas_texture_options(1)); cache.key = key; + cache.brush = brush; + cache.rect = rect; + } + None => { + let texture = painter.ctx().load_texture("clone-preview", image, canvas_texture_options(1)); + app.clone_preview = Some(ClonePreviewCache { key, brush, rect, texture }); } - None => app.clone_preview = Some(ClonePreviewCache { key, texture: painter.ctx().load_texture("clone-preview", image, canvas_texture_options(1)) }), } } let Some(cache) = &app.clone_preview else { return }; + let rect = cache.rect; let tint = Color32::from_white_alpha((overlay.opacity.clamp(0.0, 100.0) * 2.55).round() as u8); - // A textured triangle fan provides a circular clip, including mirrored canvas views. + // A textured quad over the covered area: its texels carry the clip, in mirrored and rotated + // canvas views too. let mut mesh = egui::Mesh::with_texture(cache.texture.id()); - let vertex = |x: f64, y: f64| egui::epaint::Vertex { - pos: xf.to_screen(x as f32, y as f32), - uv: pos2((x - f64::from(rect.x0)) as f32 / rect.width() as f32, (y - f64::from(rect.y0)) as f32 / rect.height() as f32), - color: tint, - }; - if !overlay.clipped || tip_rings.is_some() { - for (x, y) in [(rect.x0, rect.y0), (rect.x1, rect.y0), (rect.x1, rect.y1), (rect.x0, rect.y1)] { - mesh.vertices.push(vertex(f64::from(x), f64::from(y))); - } - mesh.indices.extend_from_slice(&[0, 1, 2, 0, 2, 3]); - painter.add(egui::Shape::mesh(mesh)); - return; - } - mesh.vertices.push(vertex(at[0], at[1])); - for i in 0..=64 { - let angle = f64::from(i) * std::f64::consts::TAU / 64.0; - mesh.vertices.push(vertex(at[0] + radius * angle.cos(), at[1] + radius * angle.sin())); - } - for i in 1..=64 { - mesh.indices.extend_from_slice(&[0, i, i + 1]); + for (x, y, u, v) in [(rect.x0, rect.y0, 0.0, 0.0), (rect.x1, rect.y0, 1.0, 0.0), (rect.x1, rect.y1, 1.0, 1.0), (rect.x0, rect.y1, 0.0, 1.0)] { + mesh.vertices.push(egui::epaint::Vertex { pos: xf.to_screen(x as f32, y as f32), uv: pos2(u, v), color: tint }); } + mesh.indices.extend_from_slice(&[0, 1, 2, 0, 2, 3]); painter.add(egui::Shape::mesh(mesh)); } diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index e6e1810f76..6bce89cc0d 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -1175,70 +1175,151 @@ fn dragging_a_tab_off_the_strip_floats_it_in_a_window() { } } +/// An app with a 64 × 64 white document, a clone source and the Clone Stamp at `size` px. +fn clone_overlay_app(size: f32) -> PhotocraftApp { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); + app.run("file.new", json!({"width": 64, "height": 64})).unwrap(); + app.run("cloneSource.set", json!({"source": [16, 16]})).unwrap(); + app.ui.tool = Tool::CloneStamp; + app.session.tools.brush.size = size; + app +} + +/// One frame of the overlay at (32, 32): its shapes and texture uploads. +fn clone_overlay_frame(ctx: &egui::Context, app: &mut PhotocraftApp, xf: &ViewXform) -> egui::FullOutput { + ctx.run_ui(Default::default(), |ui| { + let painter = ui.ctx().layer_painter(egui::LayerId::new(egui::Order::Foreground, egui::Id::unique("clone-test"))); + draw_clone_preview(app, &painter, xf, [32.0, 32.0], None); + }) +} + +/// The overlay texture a frame uploaded and the alpha at a document pixel, read from the +/// document area the overlay covers (transparent outside it). +fn clone_overlay_alpha(app: &PhotocraftApp, output: &mut egui::FullOutput) -> impl Fn(i32, i32) -> u8 + use<> { + let cache = app.clone_preview.as_ref().expect("an overlay was drawn"); + let image = output + .textures_delta + .set + .get(&cache.texture.id()) + .and_then(|deltas| deltas.last()) + .map(|d| match &d.image { + egui::ImageData::Color(c) => c.clone(), + }) + .expect("the overlay uploads a texture"); + output.textures_delta.clear(); + let rect = cache.rect; + assert_eq!(image.size, [rect.width() as usize, rect.height() as usize], "the texture covers the overlay's area"); + move |x, y| { + if x < rect.x0 || y < rect.y0 || x >= rect.x1 || y >= rect.y1 { + return 0; + } + image.pixels[(y - rect.y0) as usize * image.width() + (x - rect.x0) as usize].a() + } +} + +fn clone_overlay_view(side: f32, zoom: f32) -> ViewXform { + ViewXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(side, side)), zoom, center: [32.0, 32.0], flip: false, rotation: 0.0, aspect: 1.0 } +} + #[test] fn clone_overlay_is_circular_and_respects_visibility() { let ctx = egui::Context::default(); - let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); - app.run("file.new", json!({"width": 64, "height": 64})).unwrap(); - app.run("cloneSource.set", json!({"source": [16,16]})).unwrap(); - app.ui.tool = Tool::CloneStamp; - app.session.tools.brush.size = 20.0; - let xf = ViewXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(100.0, 100.0)), zoom: 1.0, center: [32.0, 32.0], flip: false, rotation: 0.0, aspect: 1.0 }; + let mut app = clone_overlay_app(20.0); + let xf = clone_overlay_view(100.0, 1.0); let revision = app.session.active().unwrap().revision; - let mut output = ctx.run_ui(Default::default(), |ui| { - let ctx = ui.ctx(); - let painter = ctx.layer_painter(egui::LayerId::new(egui::Order::Foreground, egui::Id::unique("clone-test"))); - draw_clone_preview(&mut app, &painter, &xf, [32.0, 32.0], None); - }); - output.textures_delta.clear(); - let mesh = output - .shapes - .iter() - .find_map(|s| match &s.shape { - egui::Shape::Mesh(m) => Some(m), - _ => None, - }) - .unwrap(); - assert_eq!(mesh.indices.len(), 64 * 3); - let center = xf.to_screen(32.0, 32.0); - assert!(mesh.vertices.iter().all(|v| v.pos.distance(center) <= 10.01)); + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); + assert!(output.shapes.iter().any(|s| matches!(s.shape, egui::Shape::Mesh(_))), "the overlay is drawn"); + let alpha = clone_overlay_alpha(&app, &mut output); + assert_eq!(alpha(32, 32), 255, "the centre shows the source"); + assert_eq!(alpha(32 + 8, 32), 255, "inside the hard 10 px radius"); + assert_eq!(alpha(32 + 8, 32 + 8), 0, "the brush square's corner, outside the circle, is clipped"); + assert_eq!(alpha(32 + 12, 32), 0, "past the hard tip's rim"); assert_eq!(app.session.active().unwrap().revision, revision); assert!(app.session.presets.clone.active().anchor.is_none()); let ordinary_key = app.clone_preview.as_ref().unwrap().key.clone(); app.ui.view.pattern_preview = true; - let mut periodic_output = ctx.run_ui(Default::default(), |ui| { - let painter = ui.ctx().layer_painter(egui::LayerId::new(egui::Order::Foreground, egui::Id::unique("clone-test"))); - draw_clone_preview(&mut app, &painter, &xf, [32.0, 32.0], None); - }); + let mut periodic_output = clone_overlay_frame(&ctx, &mut app, &xf); assert_ne!(app.clone_preview.as_ref().unwrap().key, ordinary_key, "toggling Pattern Preview invalidates source pixels"); assert!(!periodic_output.textures_delta.set.is_empty(), "the overlay texture is refreshed without a document edit"); periodic_output.textures_delta.clear(); assert_eq!(app.session.active().unwrap().revision, revision); app.session.presets.clone.overlay.show = false; - let mut output = ctx.run_ui(Default::default(), |ui| { - let ctx = ui.ctx(); - let painter = ctx.layer_painter(egui::LayerId::new(egui::Order::Foreground, egui::Id::unique("clone-test"))); - draw_clone_preview(&mut app, &painter, &xf, [32.0, 32.0], None); - }); + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); output.textures_delta.clear(); assert!(output.shapes.iter().all(|s| !matches!(s.shape, egui::Shape::Mesh(_)))); } +/// A soft brush previews soft: the overlay fades towards the rim with the brush's hardness, as +/// the stroke it previews does, and reaches past the radius as a soft tip paints. A brush +/// change redraws an overlay that hasn't moved. +#[test] +fn clone_overlay_follows_the_brush_hardness() { + let ctx = egui::Context::default(); + let mut app = clone_overlay_app(20.0); + let xf = clone_overlay_view(100.0, 1.0); + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); + let hard = clone_overlay_alpha(&app, &mut output); + app.session.tools.brush.hardness = 0.0; + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); + let soft = clone_overlay_alpha(&app, &mut output); + assert!(hard(32, 32) == 255 && soft(32, 32) > 240, "both show the source at the centre: {} {}", hard(32, 32), soft(32, 32)); + assert_eq!(hard(32 + 8, 32), 255); + assert!(soft(32 + 8, 32) < 200, "a soft tip fades before its rim: {}", soft(32 + 8, 32)); + assert!(soft(32 + 4, 32) > soft(32 + 8, 32), "and fades outwards"); + assert!(hard(32 + 12, 32) == 0 && soft(32 + 13, 32) > 0, "a soft tip reaches past the radius, a hard one doesn't"); + // The largest brush the overlay draws still shows at its softest. + let mut big = clone_overlay_app(1000.0); + big.session.tools.brush.hardness = 0.0; + let mut output = clone_overlay_frame(&ctx, &mut big, &xf); + assert_eq!(clone_overlay_alpha(&big, &mut output)(32, 32), 255, "a 1000 px soft brush previews too"); +} + +/// A scaled Clone Source reads a larger source area than the overlay covers: a large brush's +/// overlay shrinks around the pointer until that area fits the sampling budget, rather than +/// vanishing. +#[test] +fn clone_overlay_of_a_scaled_source_stays_within_the_sampling_budget() { + let ctx = egui::Context::default(); + let xf = clone_overlay_view(100.0, 1.0); + for (size, scale) in [(600.0, 200), (1000.0, 300), (1000.0, 50)] { + let mut app = clone_overlay_app(size); + app.run("cloneSource.set", json!({"width": scale, "height": scale})).unwrap(); + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); + assert!(output.shapes.iter().any(|s| matches!(s.shape, egui::Shape::Mesh(_))), "{size} px at {scale} %: the overlay shows"); + assert_eq!(clone_overlay_alpha(&app, &mut output)(32, 32), 255, "{size} px at {scale} %: the source under the pointer"); + } +} + +/// The Healing Brush shares the Clone Stamp's source and overlay (Photoshop's Clone Source +/// panel drives both); other tools show none. +#[test] +fn clone_overlay_shows_for_the_healing_brush_too() { + let ctx = egui::Context::default(); + let xf = clone_overlay_view(100.0, 1.0); + for (tool, shown) in [(Tool::Healing, true), (Tool::CloneStamp, true), (Tool::Brush, false), (Tool::SpotHealing, false)] { + let mut app = clone_overlay_app(20.0); + app.ui.tool = tool; + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); + assert_eq!(output.shapes.iter().any(|s| matches!(s.shape, egui::Shape::Mesh(_))), shown, "{tool:?}"); + if shown { + assert_eq!(clone_overlay_alpha(&app, &mut output)(32, 32), 255, "{tool:?}: the source shows under the brush"); + } + output.textures_delta.clear(); + } +} + #[test] fn clone_overlay_keeps_magnified_pixels_crisp_on_upload_and_refresh() { let ctx = egui::Context::default(); - let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); - app.run("file.new", json!({"width": 64, "height": 64})).unwrap(); - app.run("cloneSource.set", json!({"source": [16, 16]})).unwrap(); - app.ui.tool = Tool::CloneStamp; - app.session.tools.brush.size = 20.0; - let xf = ViewXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(200.0, 200.0)), zoom: 4.0, center: [32.0, 32.0], flip: false, rotation: 0.0, aspect: 1.0 }; + let mut app = clone_overlay_app(20.0); + let xf = clone_overlay_view(200.0, 4.0); for invert in [false, true] { app.session.presets.clone.overlay.invert = invert; let mut output = ctx.run_ui(Default::default(), |ui| { draw_clone_preview(&mut app, ui.painter(), &xf, [32.0, 32.0], None); }); - let upload = output.textures_delta.set.values().flatten().find(|d| d.image.size() == [20, 20]).expect("clone overlay upload"); + let id = app.clone_preview.as_ref().unwrap().texture.id(); + let upload = output.textures_delta.set.get(&id).and_then(|d| d.last()).expect("clone overlay upload"); let options = upload.options; output.textures_delta.clear(); assert_eq!(options.magnification, egui::TextureFilter::Nearest, "invert={invert}"); @@ -1249,27 +1330,20 @@ fn clone_overlay_keeps_magnified_pixels_crisp_on_upload_and_refresh() { #[test] fn clone_overlay_blend_mode_mixes_with_the_image_under_the_brush() { let ctx = egui::Context::default(); - let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); - app.run("file.new", json!({"width": 64, "height": 64})).unwrap(); - app.run("cloneSource.set", json!({"source": [16,16]})).unwrap(); - app.ui.tool = Tool::CloneStamp; - app.session.tools.brush.size = 20.0; - let xf = ViewXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(100.0, 100.0)), zoom: 1.0, center: [32.0, 32.0], flip: false, rotation: 0.0, aspect: 1.0 }; - // The centre texel of the overlay texture uploaded for `mode`. + let mut app = clone_overlay_app(20.0); + let xf = clone_overlay_view(100.0, 1.0); + // The overlay texel at the brush centre for `mode`. let mut centre = |mode: &str| { app.run("cloneSource.overlay", json!({"blend": mode})).unwrap(); - let mut output = ctx.run_ui(Default::default(), |ui| { - let painter = ui.ctx().layer_painter(egui::LayerId::new(egui::Order::Foreground, egui::Id::unique("clone-test"))); - draw_clone_preview(&mut app, &painter, &xf, [32.0, 32.0], None); - }); - // The brush's 20 × 20 px square (the font atlas may upload too). - let image = output.textures_delta.set.values().flatten().find_map(|d| match &d.image { - egui::ImageData::Color(c) if c.size == [20, 20] => Some(c.clone()), - _ => None, + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); + let cache = app.clone_preview.as_ref().unwrap(); + let image = output.textures_delta.set.get(&cache.texture.id()).and_then(|d| d.last()).map(|d| match &d.image { + egui::ImageData::Color(c) => c.clone(), }); output.textures_delta.clear(); let image = image.expect("the overlay uploads a texture"); - image.pixels[image.width() * (image.height() / 2) + image.width() / 2] + let rect = cache.rect; + image.pixels[(32 - rect.y0) as usize * image.width() + (32 - rect.x0) as usize] }; // A white source over the white image: Normal shows white, Difference black. assert_eq!(centre("normal"), Color32::WHITE); @@ -1281,32 +1355,14 @@ fn clone_overlay_blend_mode_mixes_with_the_image_under_the_brush() { #[test] fn clone_overlay_is_clipped_to_a_shaped_tip() { let ctx = egui::Context::default(); - let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); - app.run("file.new", json!({"width": 64, "height": 64})).unwrap(); - app.run("cloneSource.set", json!({"source": [16,16]})).unwrap(); - app.ui.tool = Tool::CloneStamp; - app.session.tools.brush.size = 20.0; + let mut app = clone_overlay_app(20.0); app.session.tools.brush.roundness = 0.25; - let xf = ViewXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(100.0, 100.0)), zoom: 1.0, center: [32.0, 32.0], flip: false, rotation: 0.0, aspect: 1.0 }; - let mut output = ctx.run_ui(Default::default(), |ui| { - let painter = ui.ctx().layer_painter(egui::LayerId::new(egui::Order::Foreground, egui::Id::unique("clone-test"))); - draw_clone_preview(&mut app, &painter, &xf, [32.0, 32.0], None); - }); - let image = output - .textures_delta - .set - .values() - .flatten() - .find_map(|d| match &d.image { - egui::ImageData::Color(c) if c.size == [20, 20] => Some(c.clone()), - _ => None, - }) - .expect("the overlay uploads a texture"); - output.textures_delta.clear(); - let alpha = |x: usize, y: usize| image.pixels[y * image.width() + x].a(); - assert_eq!(alpha(10, 10), 255, "the tip's centre shows the source"); - assert_eq!(alpha(10, 2), 0, "inside the circle but outside the flat tip: clipped"); - assert_eq!(alpha(1, 10), 255, "the tip's long axis stays"); + let xf = clone_overlay_view(100.0, 1.0); + let mut output = clone_overlay_frame(&ctx, &mut app, &xf); + let alpha = clone_overlay_alpha(&app, &mut output); + assert_eq!(alpha(32, 32), 255, "the tip's centre shows the source"); + assert_eq!(alpha(32, 24), 0, "inside the circle but outside the flat tip: clipped"); + assert!(alpha(25, 32) > 240, "the tip's long axis stays: {}", alpha(25, 32)); } #[test] diff --git a/crates/ui-egui/src/dialogs/color_picker_ui.rs b/crates/ui-egui/src/dialogs/color_picker_ui.rs index 0d48956651..c448d51414 100644 --- a/crates/ui-egui/src/dialogs/color_picker_ui.rs +++ b/crates/ui-egui/src/dialogs/color_picker_ui.rs @@ -326,6 +326,13 @@ where (hex(to_rgb(v)) == hex(rgb)).then_some(v) } +/// The CMYK values typed into the picker, while they still match its colour: the inks a command +/// should keep in a CMYK document (#4196, #8568). +pub(crate) fn picked_cmyk(f: &Map) -> Option<[f32; 4]> { + let rgb = f.get("color").and_then(Value::as_str).and_then(parse_hex)?; + kept::<4>(f, "__cmyk", rgb, photocraft_color::convert::cmyk_to_rgb) +} + /// Current colour, keeping the stored HSB, Lab or CMYK values while they still match `color`. pub fn current(f: &Map) -> Components { let rgb = f.get("color").and_then(Value::as_str).and_then(parse_hex).unwrap_or([0.0; 3]); diff --git a/crates/ui-egui/src/dialogs/export_dialog.rs b/crates/ui-egui/src/dialogs/export_dialog.rs index e473504e4b..68f46af65f 100644 --- a/crates/ui-egui/src/dialogs/export_dialog.rs +++ b/crates/ui-egui/src/dialogs/export_dialog.rs @@ -91,7 +91,14 @@ fn remember(app: &mut PhotocraftApp, f: &Map) { } pub fn open(app: &mut PhotocraftApp) -> Result { - let st = app.session.active().ok_or("no document")?; + let size = app.session.active().ok_or("no document")?.doc.size; + open_sized(app, size.width, size.height) +} + +/// The dialog with its size fields fitted to a `src_w` × `src_h` source: the document, or the +/// layer Layer › Export As exports. Fitting the document first would replace a remembered pixel +/// size the document can't take with the percent, even when the layer can take it. +fn open_sized(app: &mut PhotocraftApp, src_w: u32, src_h: u32) -> Result { let mut f = Map::new(); f.insert("__export".into(), json!(true)); f.insert("__label".into(), json!("Export As")); @@ -107,42 +114,54 @@ pub fn open(app: &mut PhotocraftApp) -> Result { f.insert("lockAspect".into(), json!(true)); f.insert("profile".into(), json!("document")); f.insert("embedProfile".into(), json!(true)); - f.insert("width".into(), json!(st.doc.size.width)); - f.insert("height".into(), json!(st.doc.size.height)); - f.insert("__w".into(), json!(st.doc.size.width)); - f.insert("__h".into(), json!(st.doc.size.height)); + f.insert("width".into(), json!(src_w)); + f.insert("height".into(), json!(src_h)); + f.insert("__w".into(), json!(src_w)); + f.insert("__h".into(), json!(src_h)); seed_remembered(app, &mut f); - let explicit = s(&f, "sizing") == "pixels" && f.contains_key("width") && f.contains_key("height"); - if !explicit && let Ok(size) = resize_from_scale(f64::from(st.doc.size.width), f64::from(st.doc.size.height), n(&f, "scale", 100.0)) { + fit_size(&mut f, f64::from(src_w), f64::from(src_h)); + Ok(app.ui.open_dialog(DialogKind::Command, f)) +} + +/// The size fields for exporting a `src_w` × `src_h` source. A remembered percent scales it. +/// Remembered exact pixels with Constrain proportions keep their width and take the height from +/// this source, as ticking the checkbox does: the last export's proportions would stretch an +/// image of another shape (#8738). Unlinked, the exact pixels are the size the user asked for. +/// A linked width this source can't reach within the Scale field's 1..=1000 % falls back to the +/// remembered percent. +fn fit_size(f: &mut Map, src_w: f64, src_h: f64) { + if s(f, "sizing") == "pixels" && f.contains_key("width") && f.contains_key("height") { + if !f.get("lockAspect").and_then(Value::as_bool).unwrap_or(true) { + return; + } + if let Ok(size) = resize_from_side(src_w, src_h, n(f, "width", src_w.max(1.0)), n(f, "height", src_h.max(1.0)), true, true) + && (1.0..=1000.0).contains(&size.scale) + { + write_size(f, size, "pixels"); + return; + } + } + if let Ok(size) = resize_from_scale(src_w, src_h, n(f, "scale", 100.0)) { f.insert("width".into(), json!(size.width)); f.insert("height".into(), json!(size.height)); - if (n(&f, "scale", 100.0) - size.scale).abs() > 1e-9 { + if (n(f, "scale", 100.0) - size.scale).abs() > 1e-9 { f.insert("scale".into(), json!(size.scale)); } f.insert("sizing".into(), json!("scale")); } - Ok(app.ui.open_dialog(DialogKind::Command, f)) } /// Layer › Export As…: the same dialog for just the active layer (trimmed to its pixels). pub fn open_layer(app: &mut PhotocraftApp, layer: photocraft_doc::LayerId) -> Result { let st = app.session.active().ok_or("no document")?; let ldoc = photocraft_engine::layer_menu_cmds::layer_document(&st.doc, layer).map_err(|e| e.to_string())?; - let id = open(app)?; + // The size fields follow the layer's trimmed size, so the dialog shows and edits the size + // the layer exports at. + let id = open_sized(app, ldoc.size.width, ldoc.size.height)?; if let Some(d) = app.ui.dialogs.iter_mut().find(|d| d.id == id) { let f = &mut d.fields; f.insert("__layer".into(), json!(layer.0)); f.insert("__label".into(), json!(crate::i18n::fmt(crate::i18n::t("Export As: {name}"), &[("name", &ldoc.name)]))); - f.insert("__w".into(), json!(ldoc.size.width)); - f.insert("__h".into(), json!(ldoc.size.height)); - // `open` seeded width and height from the document; in scale mode they follow the layer's - // trimmed size, so the dialog shows and edits the size the layer exports at. Exact pixels - // (sizing "pixels") are a size the user asked for, so those stay. - if s(f, "sizing") != "pixels" - && let Ok(size) = resize_from_scale(f64::from(ldoc.size.width), f64::from(ldoc.size.height), n(f, "scale", 100.0)) - { - write_size(f, size, "scale"); - } } Ok(id) } @@ -356,17 +375,18 @@ pub(crate) fn resize_from_scale(src_w: f64, src_h: f64, scale: f64) -> Result Result { let src_w = src_w.max(1.0); let src_h = src_h.max(1.0); let (width, height) = if edited_width { let width = finite_px(width)?; - let height = if lock { finite_px(src_h * width / src_w)? } else { finite_px(height)? }; + let height = if lock { finite_px((src_h * width / src_w).max(1.0))? } else { finite_px(height)? }; (width, height) } else { let height = finite_px(height)?; - let width = if lock { finite_px(src_w * height / src_h)? } else { finite_px(width)? }; + let width = if lock { finite_px((src_w * height / src_h).max(1.0))? } else { finite_px(width)? }; (width, height) }; let scale = if edited_width { width / src_w * 100.0 } else { height / src_h * 100.0 }; diff --git a/crates/ui-egui/src/dialogs/export_dialog/tests.rs b/crates/ui-egui/src/dialogs/export_dialog/tests.rs index 86aa4e88c2..c5a7bdfa2f 100644 --- a/crates/ui-egui/src/dialogs/export_dialog/tests.rs +++ b/crates/ui-egui/src/dialogs/export_dialog/tests.rs @@ -99,8 +99,9 @@ fn a_replayed_export_needs_a_path_and_a_known_format() { } /// Layer › Export As opens with the layer's trimmed size, not the document's: `open` seeds the -/// size fields from the document, and the layer replaces them in scale mode. A remembered -/// percent scales the layer; remembered exact pixels stay the size the user asked for. +/// size fields to the document, and the layer refits them. A remembered percent scales the +/// layer; remembered exact pixels stay the size the user asked for, in the layer's proportions +/// when they are linked. #[test] fn layer_export_as_opens_with_the_layer_size() { let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); @@ -119,10 +120,105 @@ fn layer_export_as_opens_with_the_layer_size() { let f = &app.ui.dialog_mut(id).unwrap().fields; assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(5.0), Some(3.0))); - app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), json!({"sizing": "pixels", "width": 120, "height": 80}))); + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), json!({"sizing": "pixels", "width": 120, "height": 80, "lockAspect": false}))); let id = open_layer(&mut app, layer).unwrap(); let f = &app.ui.dialog_mut(id).unwrap().fields; assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(120.0), Some(80.0))); + + // With Constrain proportions the remembered width stays and the height follows the layer. + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), json!({"sizing": "pixels", "width": 50, "height": 80, "lockAspect": true}))); + let id = open_layer(&mut app, layer).unwrap(); + let f = &app.ui.dialog_mut(id).unwrap().fields; + assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(50.0), Some(30.0))); + assert_eq!(f["sizing"], "pixels"); +} + +/// #8738: a pixel size remembered from a landscape export, with Constrain proportions on, +/// keeps its width and takes the height from the portrait image being exported, as ticking +/// the checkbox does; unlinked, both sides are the exact size asked for. A recorded action +/// step is not the remembered size: it replays with exactly the pixels it was recorded with. +#[test] +fn a_remembered_pixel_size_keeps_this_images_proportions() { + let sizes = std::rc::Rc::new(std::cell::RefCell::new(Vec::new())); + let log = sizes.clone(); + let services = crate::Services { + export: Some(Box::new(move |doc, _, _| { + log.borrow_mut().push((doc.size.width, doc.size.height)); + Ok((Vec::new(), Vec::new())) + })), + write: Some(Box::new(|_, _| Ok(()))), + ..Default::default() + }; + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), services); + app.run("file.new", json!({"width": 30, "height": 40})).unwrap(); + let landscape = json!({"sizing": "pixels", "width": 20, "height": 15, "scale": 50, "lockAspect": true}); + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), landscape.clone())); + let id = open(&mut app).unwrap(); + let f = &app.ui.dialog_mut(id).unwrap().fields; + assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(20.0), Some(27.0))); + assert_eq!(f["sizing"], "pixels"); + assert!((f["scale"].as_f64().unwrap() - 200.0 / 3.0).abs() < 1e-9, "{}", f["scale"]); + + let mut unlinked = landscape.clone(); + unlinked["lockAspect"] = json!(false); + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), unlinked)); + let id = open(&mut app).unwrap(); + let f = &app.ui.dialog_mut(id).unwrap().fields; + assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(20.0), Some(15.0))); + + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), landscape.clone())); + let mut step = landscape; + step["path"] = json!("/x.png"); + step["format"] = json!("png"); + replay(&mut app, &step).unwrap(); + assert_eq!(*sizes.borrow(), [(20, 15)]); +} + +/// A remembered linked width this image can't take within the Scale field's 1..=1000 % +/// (2000 px from a 4000 px photo, on a 100 px image) falls back to the remembered percent. +#[test] +fn a_remembered_pixel_size_out_of_scale_falls_back_to_the_percent() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 100, "height": 100})).unwrap(); + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), json!({"sizing": "pixels", "width": 2000, "height": 1500, "scale": 50, "lockAspect": true}))); + let id = open(&mut app).unwrap(); + let f = &app.ui.dialog_mut(id).unwrap().fields; + assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(50.0), Some(50.0))); + assert_eq!((f["sizing"].as_str(), f["scale"].as_f64()), (Some("scale"), Some(50.0))); +} + +/// A remembered linked width whose height rounds below one pixel keeps its width: the height +/// is one pixel, as the Scale field gives at the same 10 %, and not the remembered percent. +#[test] +fn a_remembered_pixel_width_keeps_a_one_pixel_height() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 1000, "height": 1})).unwrap(); + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), json!({"sizing": "pixels", "width": 100, "height": 75, "scale": 100, "lockAspect": true}))); + let id = open(&mut app).unwrap(); + let f = &app.ui.dialog_mut(id).unwrap().fields; + assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(100.0), Some(1.0))); + assert_eq!(f["sizing"], "pixels"); + // The same for an edited height on a 1 × 1000 image. + let size = resize_from_side(1.0, 1000.0, 1.0, 100.0, true, false).unwrap(); + assert_eq!((size.width, size.height), (1.0, 100.0)); +} + +/// Layer › Export As fits the remembered size to the layer, not to the document: a remembered +/// linked width the tall document can't take (800 px wide is 320 000 px high) still fits the +/// 100 × 10 layer at 800 × 80. +#[test] +fn a_layer_export_fits_the_remembered_size_to_the_layer() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 100, "height": 40000})).unwrap(); + app.run("layer.new.layer", json!({})).unwrap(); + app.run("select.rect", json!({"x": 0, "y": 0, "width": 100, "height": 10})).unwrap(); + app.run("edit.fill", json!({"color": "#ff0000"})).unwrap(); + let layer = app.session.active().unwrap().active_layer.unwrap(); + app.session.prefs.edit(|p| p.dialogs.insert(COMMAND.into(), json!({"sizing": "pixels", "width": 800, "height": 600, "scale": 50, "lockAspect": true}))); + let id = open_layer(&mut app, layer).unwrap(); + let f = &app.ui.dialog_mut(id).unwrap().fields; + assert_eq!((f["width"].as_f64(), f["height"].as_f64()), (Some(800.0), Some(80.0))); + assert_eq!(f["sizing"], "pixels"); } /// Only the dialog's own settings are remembered: not the destination, the document size or diff --git a/crates/ui-egui/src/dialogs/liquify_ui.rs b/crates/ui-egui/src/dialogs/liquify_ui.rs index 5ab1fb7043..332dcacb9a 100644 --- a/crates/ui-egui/src/dialogs/liquify_ui.rs +++ b/crates/ui-egui/src/dialogs/liquify_ui.rs @@ -1156,7 +1156,7 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { let center = d.brush_resize.map_or(hp, |(anchor, _, _)| anchor); let r = d.opts.size * 0.5 * d.zoom; if d.brush_resize.is_some() { - crate::brush_resize::tip_preview( + crate::brush_resize::liquify_tip_preview( &clip, center, r, diff --git a/crates/ui-egui/src/dialogs/solid_fill_ui.rs b/crates/ui-egui/src/dialogs/solid_fill_ui.rs index 5d9112abab..37844b86c2 100644 --- a/crates/ui-egui/src/dialogs/solid_fill_ui.rs +++ b/crates/ui-egui/src/dialogs/solid_fill_ui.rs @@ -27,7 +27,14 @@ pub(crate) fn open(app: &mut PhotocraftApp) -> Option { let layer = st.active_layer?; let LayerContent::Fill(Fill::Solid(color)) = st.doc.layer(layer)?.content else { return None }; let params = json!({"layer": layer.0, "document": st.doc.id.0}); - Some(color_picker_ui::open_for_command(app, "Color Picker (Solid Color)", color.to_rgb(), cmds::SET, params)) + let id = color_picker_ui::open_for_command(app, "Color Picker (Solid Color)", color.to_rgb(), cmds::SET, params); + // A CMYK fill reopens showing its own inks, not ones recomputed from its sRGB stand-in (#8568). + if color.mode == photocraft_color::ColorMode::Cmyk + && let Some(d) = app.ui.dialog_mut(id) + { + d.fields.insert("__cmyk".into(), json!(color.c)); + } + Some(id) } /// Creating a Solid Color fill is transactional: the document is unchanged until OK. @@ -57,7 +64,8 @@ pub(crate) fn confirm_new(app: &mut PhotocraftApp, f: &Map) -> Re return Err("the target document is no longer active".into()); } let color = f.get("color").and_then(Value::as_str).ok_or("missing Solid Color colour")?; - app.run("layer.newFillLayer.solidColor", json!({"color": color})) + // CMYK typed into the picker stays those inks in a CMYK document (#8568). + app.run("layer.newFillLayer.solidColor", json!({"color": color, "cmyk": color_picker_ui::picked_cmyk(f)})) } pub(crate) fn owns(f: &Map) -> bool { @@ -72,14 +80,39 @@ pub(crate) fn thumbnail_color(app: &PhotocraftApp, document: DocId, layer: Layer .filter(|d| owns(&d.fields)) .find_map(|d| { let params = d.fields.get("__params")?; - if params.get("document")?.as_u64()? != document.0 || params.get("layer")?.as_u64()? != layer.0 || d.fields.get("color") == d.fields.get("__orig") { + if params.get("document")?.as_u64()? != document.0 || params.get("layer")?.as_u64()? != layer.0 { return None; } - cmds::color_param(d.fields.get("color")?, original, app.session.documents().iter().find(|st| st.doc.id == document)?.doc.mode).ok() + let mode = app.session.documents().iter().find(|st| st.doc.id == document)?.doc.mode; + picked_color(&d.fields, original, mode) }) .unwrap_or(original) } +/// The colour the picker would commit to a fill of `original` in a `mode` document, built as the +/// engine builds it: the picked hex, and in a CMYK document the inks it was picked as (#8568). +/// `None` while the picker still shows the fill as it is ([`is_edit`]). +fn picked_color(f: &Map, original: Color, mode: photocraft_color::ColorMode) -> Option { + if !is_edit(f, original, mode) { + return None; + } + let color = cmds::color_param(f.get("color")?, original, mode).ok()?; + Some(color_picker_ui::picked_cmyk(f).filter(|_| mode == photocraft_color::ColorMode::Cmyk).map_or(color, |c| Color { + mode: photocraft_color::ColorMode::Cmyk, + c, + alpha: color.alpha, + })) +} + +/// Whether the picker differs from the fill it edits, inks included: different inks can share a +/// display colour (CMYK 0/0/0/100 and 50/50/50/100 both read black), so the hex alone doesn't +/// tell, and an RGB or Lab fill given inks in a CMYK document is an edit too. +fn is_edit(f: &Map, original: Color, mode: photocraft_color::ColorMode) -> bool { + let cmyk = photocraft_color::ColorMode::Cmyk; + let inks_differ = mode == cmyk && color_picker_ui::picked_cmyk(f).is_some_and(|k| original.mode != cmyk || original.c != k); + f.get("color") != f.get("__orig") || inks_differ +} + /// No-op OK preserves imported high-precision / non-RGB colour values rather than quantising /// them to the picker's display hex. A stale document must never receive the edit. pub(crate) fn confirm(app: &mut PhotocraftApp, f: &Map) -> Result { @@ -89,15 +122,18 @@ pub(crate) fn confirm(app: &mut PhotocraftApp, f: &Map) -> Result return Err("the target document is no longer active".into()); } let layer = LayerId(params.get("layer").and_then(Value::as_u64).ok_or("missing Solid Color layer")?); - if !st.doc.layer(layer).is_some_and(|l| matches!(l.content, LayerContent::Fill(Fill::Solid(_)))) { + let Some(LayerContent::Fill(Fill::Solid(original))) = st.doc.layer(layer).map(|l| &l.content) else { return Err("the target Solid Color Fill layer no longer exists".into()); - } - if f.get("color") == f.get("__orig") { + }; + if !is_edit(f, *original, st.doc.mode) { return Ok(json!({"layer": layer.0, "unchanged": true})); } let color = f.get("color").cloned().ok_or("missing color")?; let object = params.as_object_mut().ok_or("invalid Solid Color target")?; object.insert("color".into(), color); + if let Some(inks) = color_picker_ui::picked_cmyk(f) { + object.insert("cmyk".into(), json!(inks)); + } app.run(cmds::SET, params) } @@ -109,12 +145,12 @@ pub(crate) fn display_doc(app: &mut PhotocraftApp, idx: usize) -> Option<(Arc, inks: [f32; 4]| { + let [r, g, b] = photocraft_color::convert::cmyk_to_rgb(inks).map(|c| (c.clamp(0.0, 1.0) * 255.0).round() as u8); + f.insert("color".into(), json!(format!("#{r:02x}{g:02x}{b:02x}"))); + f.insert("__cmyk".into(), json!(inks)); + }; + let fill = |app: &PhotocraftApp| { + let st = app.session.active().unwrap(); + match st.doc.layer(st.active_layer.unwrap()).unwrap().content { + LayerContent::Fill(Fill::Solid(c)) => c, + _ => panic!("not a solid fill"), + } + }; + let inks = [0.5, 0.5, 0.5, 1.0]; + let id = open_new(&mut app).unwrap(); + let mut f = app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields.clone(); + typed(&mut f, inks); + confirm_new(&mut app, &f).unwrap(); + assert_eq!(fill(&app), Color { mode: ColorMode::Cmyk, c: inks, alpha: 1.0 }); + + let edit = [0.25, 0.75, 0.0, 0.1]; + let id = open(&mut app).unwrap(); + let mut f = app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields.clone(); + typed(&mut f, edit); + confirm(&mut app, &f).unwrap(); + assert_eq!(fill(&app).c, edit); + // Reopening the picker shows the stored inks. + let id = open(&mut app).unwrap(); + let shown = color_picker_ui::current(&app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields); + assert_eq!(shown.cmyk, edit); + + // Other inks with the same display colour are still an edit. + let near = [0.5, 0.5, 0.499, 1.0]; + let id = open(&mut app).unwrap(); + let mut f = app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields.clone(); + typed(&mut f, near); + confirm(&mut app, &f).unwrap(); + let rich = [0.5, 0.5, 0.5, 1.0]; + let id = open(&mut app).unwrap(); + let mut f = app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields.clone(); + typed(&mut f, rich); + assert_eq!(f.get("color"), f.get("__orig"), "both inks show as the same colour"); + confirm(&mut app, &f).unwrap(); + assert_eq!(fill(&app).c, rich); +} + +fn with_inks(f: &mut serde_json::Map, inks: [f32; 4]) { + let [r, g, b] = photocraft_color::convert::cmyk_to_rgb(inks).map(|c| (c.clamp(0.0, 1.0) * 255.0).round() as u8); + f.insert("color".into(), json!(format!("#{r:02x}{g:02x}{b:02x}"))); + f.insert("__cmyk".into(), json!(inks)); +} + +/// Inks that share the picker's hex with the fill's own are still a preview and a thumbnail, as +/// they are an edit on OK (#8568). +#[test] +fn same_hex_inks_preview_and_thumbnail_as_ok_commits_them() { + let mut s = photocraft_engine::Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + s.execute("image.mode.cmyk", json!({})).unwrap(); + let mut app = PhotocraftApp::new(s, crate::Services::default()); + let near = [0.5, 0.5, 0.499, 1.0]; + let id = open_new(&mut app).unwrap(); + let mut f = app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields.clone(); + with_inks(&mut f, near); + confirm_new(&mut app, &f).unwrap(); + + let rich = [0.5, 0.5, 0.5, 1.0]; + let id = open(&mut app).unwrap(); + with_inks(&mut app.ui.dialog_mut(id).unwrap().fields, rich); + let fields = &app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields; + assert_eq!(fields.get("color"), fields.get("__orig"), "both inks show as the same colour"); + + let st = app.session.active().unwrap(); + let (document, layer) = (st.doc.id, st.active_layer.unwrap()); + let LayerContent::Fill(Fill::Solid(original)) = st.doc.layer(layer).unwrap().content else { panic!("not a solid fill") }; + assert_eq!(thumbnail_color(&app, document, layer, original).c, rich); + let (shown, _) = display_doc(&mut app, 0).expect("a preview of the new inks"); + let LayerContent::Fill(Fill::Solid(previewed)) = shown.layer(layer).unwrap().content else { panic!("not a solid fill") }; + assert_eq!(previewed.c, rich); +} + +/// An RGB fill inside a CMYK document that the picker gives inks at the unchanged hex is an edit; +/// in an RGB document, which ignores the inks, it is not. +#[test] +fn inks_given_to_a_non_cmyk_fill_in_a_cmyk_document_are_an_edit() { + let original = Color::rgba(0.2, 0.3, 0.4, 1.0); + let mut f = serde_json::Map::new(); + with_inks(&mut f, [0.3, 0.2, 0.1, 0.4]); + f.insert("__orig".into(), f["color"].clone()); + assert!(is_edit(&f, original, ColorMode::Cmyk)); + assert!(!is_edit(&f, original, ColorMode::Rgb)); + assert!(picked_color(&f, original, ColorMode::Cmyk).is_some_and(|c| c.mode == ColorMode::Cmyk && c.c == [0.3, 0.2, 0.1, 0.4])); +} diff --git a/crates/ui-egui/src/file_dialog/tests.rs b/crates/ui-egui/src/file_dialog/tests.rs index a70d6acd8e..52149ab8e0 100644 --- a/crates/ui-egui/src/file_dialog/tests.rs +++ b/crates/ui-egui/src/file_dialog/tests.rs @@ -1472,3 +1472,15 @@ fn save_a_copy_appends_copy_unless_the_preference_says_not_to() { ); } } + +#[test] +fn import_variable_data_sets_menu_asks_for_a_csv_and_imports_it() { + let (mut app, open, _) = app(); + let ctx = egui::Context::default(); + assert_eq!(crate::menus::invoke(&mut app, &ctx, "file.import.variableDataSets", json!({})).unwrap(), json!({"fileDialog": "open"})); + app.poll_file_dialog(&ctx, None); + assert!(matches!(open.borrow().as_slice(), [(FileDialogRequest::Open { multiple: false, extensions: Some(e), .. }, _)] if e == &["csv", "txt"])); + answer(&open, Some(FileDialogAnswer::Contents("sets.csv".into(), b"headline\nHello\nWorld\n".to_vec()))); + app.poll_file_dialog(&ctx, None); + assert_eq!(app.session.active().map(|d| d.doc.variables.data_sets.len()), Some(2)); +} diff --git a/crates/ui-egui/src/input/shortcut_dispatch/tests.rs b/crates/ui-egui/src/input/shortcut_dispatch/tests.rs index df8eb96608..3536d58afe 100644 --- a/crates/ui-egui/src/input/shortcut_dispatch/tests.rs +++ b/crates/ui-egui/src/input/shortcut_dispatch/tests.rs @@ -638,10 +638,12 @@ fn delete_with_several_layers_selected_deletes_them_even_with_the_mask_targeted( let first = photocraft_doc::LayerId(s.execute("layer.newAdjustmentLayer.curves", json!({})).unwrap()["layer"].as_u64().unwrap()); let second = photocraft_doc::LayerId(s.execute("layer.newAdjustmentLayer.invert", json!({})).unwrap()["layer"].as_u64().unwrap()); s.execute("layer.select", json!({"layer": first.0, "mode": "add"})).unwrap(); + // The new layers took the harness's selection as their masks (#8801): select again. + s.execute("select.all", json!({})).unwrap(); let st = s.active().unwrap(); assert_eq!(st.selected_layers().len(), 2); assert!(st.active_layer.and_then(|l| st.doc.layer(l)).is_some_and(|l| l.mask.is_some()), "the active adjustment layer has a mask"); - assert!(st.doc.selection.is_some(), "the harness keeps a selection"); + assert!(st.doc.selection.is_some(), "a selection is active"); h.state_mut().ui.mask_target = true; h.run_steps(2); let n = names(&h).len(); diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index feb4459db1..68af926b5e 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -214,6 +214,18 @@ fn invoke_file_commands(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, "file.import.videoFramesToLayers" if params.get("path").and_then(Value::as_str).is_none() => { app.ask_folder(|app, path| app.run("file.import.videoFramesToLayers", json!({"path": path})).map_err(|e| e.to_string())) } + // File > Import > Variable Data Sets… asks for the CSV (#8816); with a `path` the engine reads it itself. + "file.import.variableDataSets" if params.get("path").and_then(Value::as_str).is_none() => { + let doc = app.active_doc_id()?; + let params = params.clone(); + app.pick_file_bytes_filtered(&["csv", "txt"], move |app, _name, bytes| { + app.refocus(doc)?; + let text = data_sets_text(bytes)?; + let r = photocraft_engine::import_data_sets_text(&mut app.session, &text, ¶ms).map_err(|e| e.to_string()); + app.sync_views(); + r + }) + } "file.save" => { // Writes back only to a layered file that holds the whole document; anything else // goes through Save As, so the original is never overwritten with less (#3806). @@ -448,6 +460,13 @@ pub(crate) fn invoke_unguarded(app: &mut PhotocraftApp, ctx: &egui::Context, id: } } +/// The text of a picked Variable Data Sets file. Invalid UTF-8 is refused, as the engine's +/// path-based import refuses it, so a failed read never replaces the document's data sets with +/// U+FFFD-substituted values. +pub(super) fn data_sets_text(bytes: Vec) -> Result { + String::from_utf8(bytes).map_err(|e| format!("the data set file is not valid UTF-8: {e}")) +} + fn open_path(app: &mut PhotocraftApp, path: &str) -> Result { app.open_path(path).map(|w| json!({"warnings": w})) } diff --git a/crates/ui-egui/src/menus/tests.rs b/crates/ui-egui/src/menus/tests.rs index 2527b0fc3f..c3ec327b15 100644 --- a/crates/ui-egui/src/menus/tests.rs +++ b/crates/ui-egui/src/menus/tests.rs @@ -1,5 +1,12 @@ +use super::invoke::data_sets_text; use super::*; +#[test] +fn data_set_files_must_be_valid_utf8() { + assert_eq!(data_sets_text(b"DataSet,headline\nS,Jos\xc3\xa9\n".to_vec()).unwrap(), "DataSet,headline\nS,José\n"); + assert!(data_sets_text(b"DataSet,headline\nS\xff,Jos\xe9\n".to_vec()).is_err()); +} + #[test] fn showing_grid_restores_hidden_extras() { let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index 5834447464..be80f832bb 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -1953,6 +1953,45 @@ mod sampler_readout_tests { } } +#[cfg(test)] +mod brush_chip_tests { + use super::brush_controls::brush_preset_chip; + + /// The options-bar chip's tip texture for `brush`, row-major alpha (32 × 32). + fn chip_tip(brush: &photocraft_engine::BrushSettings) -> Vec { + let ctx = egui::Context::default(); + let mut state = crate::state::UiState::default(); + let mut out = ctx.run_ui(Default::default(), |ui| brush_preset_chip(ui, brush, &mut state)); + // `Painter::image` is a textured mesh. + let id = out.shapes.iter().find_map(|c| match &c.shape { + egui::Shape::Mesh(m) if (m.calc_bounds().width() - 14.0).abs() < 0.01 => Some(m.texture_id), + _ => None, + }); + let image = id.and_then(|id| out.textures_delta.set.get(&id)).and_then(|d| d.last()).map(|d| match &d.image { + egui::ImageData::Color(c) => c.clone(), + }); + out.textures_delta.clear(); + let image = image.expect("the chip draws its tip as an uploaded texture"); + assert_eq!(image.size, [32, 32]); + image.pixels.iter().map(|p| p.a()).collect() + } + + /// The chip shows the brush's own tip: a soft brush fades from its centre as its dabs do, a + /// hard one is solid to its rim, and a flattened tip is flat, not a round blob. + #[test] + fn the_brush_chip_draws_the_brushs_own_tip() { + let at = |a: &[u8], x: usize, y: usize| a[y * 32 + x]; + let hard = chip_tip(&photocraft_engine::BrushSettings { hardness: 1.0, ..Default::default() }); + let soft = chip_tip(&photocraft_engine::BrushSettings { hardness: 0.0, ..Default::default() }); + assert!(at(&hard, 16, 16) == 255 && at(&hard, 16 + 11, 16) == 255, "hard: solid to near the rim"); + assert!(at(&soft, 16, 16) > 230, "soft: full at the centre"); + assert!(at(&soft, 16 + 11, 16) < 80, "soft: faint near the rim: {}", at(&soft, 16 + 11, 16)); + assert!(at(&soft, 16 + 6, 16) > at(&soft, 16 + 11, 16), "and fading outwards"); + let flat = chip_tip(&photocraft_engine::BrushSettings { hardness: 1.0, roundness: 0.3, ..Default::default() }); + assert!(at(&flat, 16 + 11, 16) > 200 && at(&flat, 16, 16 + 11) == 0, "a flattened tip is flat"); + } +} + #[cfg(test)] mod panel_state_migration_tests { use super::{migrate_panel_state, panel_state_id}; diff --git a/crates/ui-egui/src/panels/brush_controls.rs b/crates/ui-egui/src/panels/brush_controls.rs index 634c4e2be9..d618999d9f 100644 --- a/crates/ui-egui/src/panels/brush_controls.rs +++ b/crates/ui-egui/src/panels/brush_controls.rs @@ -1,18 +1,5 @@ use super::{Align2, BlendMode, Color32, PhotocraftApp, Rect, Sense, Stroke, StrokeKind, Tokens, c32, icons, json, pos2, vec2, widgets}; -/// Photoshop's brush preset picker chip: a soft/hard round tip preview with the size underneath. -/// Draw a round brush tip preview (hard core fading to a soft edge). -fn brush_tip(p: &egui::Painter, c: egui::Pos2, rad: f32, hardness: f32, color: Color32) { - let inner = rad * hardness.clamp(0.05, 1.0); - let steps = 8; - for k in (0..=steps).rev() { - let f = k as f32 / steps as f32; - let rr = inner + (rad - inner) * f; - let a = if hardness >= 0.99 { 1.0 } else { (1.0 - f).powf(1.5) }; - p.circle_filled(c, rr, color.gamma_multiply(a)); - } -} - /// A painting tool's Mode dropdown over `modes` (`None` is Clear); the wheel over it steps the /// mode. Returns the mode picked this frame. pub(super) fn tool_mode_dropdown(ui: &mut egui::Ui, id: &str, modes: &[Option], current: Option) -> Option> { @@ -122,7 +109,11 @@ pub(super) fn brush_preset_chip(ui: &mut egui::Ui, b: &photocraft_engine::BrushS let c = pos2(r.left() + 14.0, r.top() + 11.0); // Photoshop's white tip vanishes on light chrome; light themes use their icon colour. let tip = if t.dark() { Color32::WHITE } else { t.icon }; - brush_tip(ui.painter(), c, 7.0, b.hardness, tip); + // The brush's own tip, as the Brush Preset picker's thumbnails draw it: the engine's falloff + // for its hardness, its angle and roundness, a sampled tip's shape. + let tex = crate::brush_preview::tip_texture(ui.ctx(), "options-bar-chip", &b.tip, (b.hardness, b.angle, b.roundness), 32, tip); + let uv = Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)); + ui.painter().image(tex.id(), Rect::from_center_size(c, vec2(14.0, 14.0)), uv, Color32::WHITE); ui.painter().text(pos2(c.x, r.bottom() - 5.0), Align2::CENTER_CENTER, format!("{}", b.size.round() as i64), egui::FontId::proportional(9.5), t.text_dim); icons::paint(ui, Rect::from_center_size(pos2(r.right() - 9.0, c.y), vec2(10.0, 10.0)), "chevron-down", 9.0, t.text_faint); resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, tl!("Brush Preset picker"))); diff --git a/crates/ui-egui/src/tools/brush_cursor.rs b/crates/ui-egui/src/tools/brush_cursor.rs index 6bc71a78f7..1710679e95 100644 --- a/crates/ui-egui/src/tools/brush_cursor.rs +++ b/crates/ui-egui/src/tools/brush_cursor.rs @@ -134,43 +134,6 @@ pub(crate) fn rings(ctx: &egui::Context, brush: &BrushSettings, full: bool) -> O traced } -/// Which texels of the document-pixel `rect` (`x0, y0, x1, y1`, row-major) lie inside the tip -/// `rings` (radius units, even-odd so holes stay clear) when the tip is centred on `at` with -/// `radius` document pixels. The Clone Source overlay uses it to clip to a shaped tip. -pub(crate) fn tip_mask(rings: &[Vec<[f32; 2]>], at: [f64; 2], radius: f64, rect: [i32; 4]) -> Vec { - let [x0, y0, x1, y1] = rect; - let (w, h) = (usize::try_from(x1 - x0).unwrap_or(0), usize::try_from(y1 - y0).unwrap_or(0)); - let mut mask = vec![false; w.saturating_mul(h)]; - let mut hits: Vec = Vec::new(); - for row in 0..h { - let y = f64::from(y0) + row as f64 + 0.5; - hits.clear(); - for ring in rings { - for (i, a) in ring.iter().enumerate() { - let Some(b) = ring.get((i + 1) % ring.len()) else { continue }; - let (ax, ay) = (at[0] + f64::from(a[0]) * radius, at[1] + f64::from(a[1]) * radius); - let (bx, by) = (at[0] + f64::from(b[0]) * radius, at[1] + f64::from(b[1]) * radius); - if (ay <= y) != (by <= y) { - hits.push(ax + (y - ay) / (by - ay) * (bx - ax)); - } - } - } - hits.sort_by(f64::total_cmp); - for pair in hits.chunks(2) { - let [a, b] = pair else { continue }; - let (from, to) = (a - f64::from(x0) - 0.5, b - f64::from(x0) - 0.5); - let start = from.ceil().max(0.0) as usize; - let end = ((to.floor() + 1.0).max(0.0) as usize).min(w); - for x in start..end { - if let Some(m) = mask.get_mut(row * w + x) { - *m = true; - } - } - } - } - mask -} - #[cfg(test)] mod tests { use super::*; @@ -295,19 +258,4 @@ mod tests { assert!(ex.x.is_finite() && ex.y.is_finite() && ey.x.is_finite() && ey.y.is_finite(), "size {size}"); } } - - /// The overlay clip follows the tip: a square ring covers its own texels, a hole stays clear, - /// and a rotated flat tip leaves the corners of the brush square outside. - #[test] - fn tip_mask_follows_the_ring_shape() { - let square = vec![[-0.5, -0.5], [0.5, -0.5], [0.5, 0.5], [-0.5, 0.5]]; - let hole = vec![[-0.1, -0.1], [0.1, -0.1], [0.1, 0.1], [-0.1, 0.1]]; - let mask = tip_mask(&[square.clone(), hole], [10.0, 10.0], 10.0, [0, 0, 20, 20]); - let at = |x: usize, y: usize| mask[y * 20 + x]; - assert!(at(6, 10) && at(14, 10) && at(10, 6) && at(10, 14), "inside the square"); - assert!(!at(3, 10) && !at(10, 16) && !at(0, 0), "outside the square"); - assert!(!at(10, 10) && at(8, 10), "the hole is clear"); - assert_eq!(mask.iter().filter(|m| **m).count(), 10 * 10 - 2 * 2, "10x10 square minus the 2x2 hole"); - assert!(tip_mask(&[square], [10.0, 10.0], 10.0, [5, 5, 5, 9]).is_empty(), "an empty rect is harmless"); - } } diff --git a/crates/ui-egui/src/tools/brush_resize.rs b/crates/ui-egui/src/tools/brush_resize.rs index dc6ca08942..e22faf2347 100644 --- a/crates/ui-egui/src/tools/brush_resize.rs +++ b/crates/ui-egui/src/tools/brush_resize.rs @@ -138,14 +138,14 @@ fn update(app: &mut PhotocraftApp, r: Resize, x: f64, y: f64) { } /// Draw the resize feedback: the brush circle at the anchor, filled with the brush preview -/// colour whose solid core shows the hardness (Photoshop's), and the readout beside the pointer. +/// colour as a dab of the brush's hardness covers (Photoshop's), and the readout beside the pointer. /// Returns true while a resize is in progress (the canvas then hides its cursor). pub fn draw(app: &PhotocraftApp, painter: &egui::Painter, xf: &ViewXform) -> bool { let Some(r) = app.brush_resize else { return false }; let b = &app.session.tools.brush; let c = xf.to_screen(r.anchor[0] as f32, r.anchor[1] as f32); let radius = (b.size / 2.0 * xf.zoom).max(1.0); - tip_preview(painter, c, radius, b.hardness, preview_tint(app)); + tip_preview(painter, c, b.size / 2.0, xf.zoom, b.hardness, preview_tint(app)); painter.circle_stroke(c, radius + 0.5, Stroke::new(1.0, Color32::from_black_alpha(140))); painter.circle_stroke(c, radius, Stroke::new(1.0, Color32::from_white_alpha(220))); // Photoshop hides the pointer while the circle stays put. @@ -169,9 +169,10 @@ fn preview_tint(app: &PhotocraftApp) -> Color32 { }) } +/// Liquify density preview, kept separate from the paint engine's hardness falloff. /// A round tip of `radius` screen points: solid out to `hardness` of the radius, fading to clear /// at the edge. One mesh (two rings and a centre), so its cost doesn't grow with the brush. -pub(crate) fn tip_preview(painter: &egui::Painter, c: Pos2, radius: f32, hardness: f32, color: Color32) { +pub(crate) fn liquify_tip_preview(painter: &egui::Painter, c: Pos2, radius: f32, hardness: f32, color: Color32) { let clip = painter.clip_rect(); if !clip.expand(radius).contains(c) { return; @@ -199,5 +200,47 @@ pub(crate) fn tip_preview(painter: &egui::Painter, c: Pos2, radius: f32, hardnes painter.add(mesh); } +/// The round tip as the brush engine paints it: `radius` document pixels at `zoom` screen points +/// per document pixel, solid out to its core and then the engine's falloff (`tip_falloff`) out to +/// its reach, past the radius for a soft tip, so the disc shows what a dab of this hardness covers. +/// One mesh of rings, so its cost doesn't grow with the brush. +fn tip_preview(painter: &egui::Painter, c: Pos2, radius: f32, zoom: f32, hardness: f32, color: Color32) { + /// Rings across the falloff: the shoulder is smooth, so linear steps between them are close. + const RINGS: usize = 16; + if !(radius.is_finite() && zoom.is_finite() && radius > 0.0 && zoom > 0.0) { + return; + } + let (core, _) = photocraft_paint::tip_shape(radius, hardness); + let reach = photocraft_paint::tip_reach(radius, hardness).max(core); + let outer = reach * zoom; + let clip = painter.clip_rect(); + if !clip.expand(outer).contains(c) { + return; + } + // Enough segments for a smooth edge at any size, capped so huge brushes stay cheap. + let n = ((outer * 0.5) as usize).clamp(24, 160); + let mut mesh = egui::Mesh::default(); + mesh.colored_vertex(c, color); + for k in 0..=RINGS { + let d = core + (reach - core) * k as f32 / RINGS as f32; + // The last ring is the reach, where the falloff has run out. + let tint = if k == RINGS { Color32::TRANSPARENT } else { color.gamma_multiply(photocraft_paint::tip_falloff(d, radius, hardness)) }; + for j in 0..n { + let a = j as f32 / n as f32 * std::f32::consts::TAU; + mesh.colored_vertex(c + vec2(a.cos(), a.sin()) * d * zoom, tint); + } + } + let n = n as u32; + let at = |ring: u32, j: u32| 1 + ring * n + j % n; + for j in 0..n { + mesh.add_triangle(0, at(0, j), at(0, j + 1)); + for ring in 0..RINGS as u32 { + mesh.add_triangle(at(ring, j), at(ring + 1, j), at(ring + 1, j + 1)); + mesh.add_triangle(at(ring, j), at(ring + 1, j + 1), at(ring, j + 1)); + } + } + painter.add(mesh); +} + #[cfg(test)] mod tests; diff --git a/crates/ui-egui/src/tools/brush_resize/tests.rs b/crates/ui-egui/src/tools/brush_resize/tests.rs index 6967c7bb9a..3cba8bef99 100644 --- a/crates/ui-egui/src/tools/brush_resize/tests.rs +++ b/crates/ui-egui/src/tools/brush_resize/tests.rs @@ -344,6 +344,45 @@ fn a_rotated_view_keeps_horizontal_for_size_and_vertical_for_hardness() { assert_eq!(drag_in_view(90.0, true, 0.0, 30.0), (100.0, 0.2), "rotated and flipped: screen right grows"); } +/// The tip's mesh, painted at the origin: (distance from the centre, alpha) per vertex. +fn tip_mesh(radius: f32, zoom: f32, hardness: f32) -> Vec<(f32, u8)> { + let ctx = egui::Context::default(); + let color = Color32::from_rgba_unmultiplied(255, 0, 0, 200); + let mut out = ctx.run_ui(Default::default(), |ui| { + let painter = ui.painter().with_clip_rect(egui::Rect::from_center_size(Pos2::ZERO, vec2(4000.0, 4000.0))); + tip_preview(&painter, Pos2::ZERO, radius, zoom, hardness, color); + }); + out.textures_delta.clear(); + let mesh = out + .shapes + .iter() + .find_map(|c| match &c.shape { + egui::Shape::Mesh(m) => Some(m.clone()), + _ => None, + }) + .expect("the tip is drawn"); + mesh.vertices.iter().map(|v| (v.pos.to_vec2().length(), v.color.a())).collect() +} + +/// The disc follows the engine's round tip: a hard one is solid to its rim, a soft one fades +/// as the engine's falloff does and reaches past the radius as its dabs do. +#[test] +fn the_tip_preview_follows_the_brush_falloff() { + for zoom in [1.0, 2.5] { + let hard = tip_mesh(50.0, zoom, 1.0); + assert!(hard.iter().filter(|(d, _)| *d < 48.0 * zoom).all(|(_, a)| *a == 200), "zoom {zoom}: a hard tip is solid inside"); + assert!(hard.iter().all(|(d, a)| *d <= 52.0 * zoom || *a == 0), "zoom {zoom}: and ends at its rim"); + let soft = tip_mesh(50.0, zoom, 0.0); + for (d, a) in &soft { + let want = 200.0 * photocraft_paint::tip_falloff(d / zoom, 50.0, 0.0); + assert!((f32::from(*a) - want).abs() <= 3.0 || *a == 0, "zoom {zoom}: at {d}: {a}, the engine {want}"); + } + let reach = photocraft_paint::tip_reach(50.0, 0.0); + assert!(soft.iter().any(|(d, a)| *d > 60.0 * zoom && *a > 0), "zoom {zoom}: a soft tip shows past its radius"); + assert!(soft.iter().all(|(d, _)| *d <= reach * zoom + 0.5), "zoom {zoom}: and no further than it reaches"); + } +} + /// Preferences ▸ Cursors ▸ Brush Preview colour tints the HUD's tip preview (red by default). #[test] fn the_tip_preview_uses_the_brush_preview_colour() { @@ -383,3 +422,26 @@ fn stylus_barrel_button_resizes_with_alt_and_never_samples_color() { h.run_steps(2); assert!(h.state().brush_resize.is_none()); } + +/// Liquify density keeps its radius-bounded preview; the paint engine's soft-brush reach is +/// deliberately used only by the paint HUD. +#[test] +fn liquify_density_preview_stays_inside_its_radius() { + let ctx = egui::Context::default(); + let radius = 50.0; + let mut output = ctx.run_ui(Default::default(), |ui| { + let painter = ui.painter().with_clip_rect(egui::Rect::from_center_size(Pos2::ZERO, vec2(400.0, 400.0))); + liquify_tip_preview(&painter, Pos2::ZERO, radius, 0.0, Color32::RED); + }); + output.textures_delta.clear(); + let mesh = output + .shapes + .iter() + .find_map(|shape| match &shape.shape { + egui::Shape::Mesh(mesh) => Some(mesh), + _ => None, + }) + .expect("the Liquify preview is drawn"); + assert!(mesh.vertices.iter().all(|v| v.pos.to_vec2().length() <= radius + 0.01)); + assert!(mesh.vertices.iter().any(|v| v.pos.to_vec2().length() >= radius - 0.01)); +} diff --git a/crates/ui-egui/src/tools/retouch_ui/tests.rs b/crates/ui-egui/src/tools/retouch_ui/tests.rs index d157336960..fa291e7417 100644 --- a/crates/ui-egui/src/tools/retouch_ui/tests.rs +++ b/crates/ui-egui/src/tools/retouch_ui/tests.rs @@ -52,6 +52,28 @@ fn clone_stamp_in_pattern_preview_wraps_a_dab_across_the_right_edge() { } } +/// A Clone Stamp click paints with the brush's hardness (the Brush popup's), not a hard tip: +/// near the rim a soft brush paints only part of the source, a hard one all of it. +#[test] +fn clone_stamp_paints_with_the_brush_hardness() { + let rim = |hardness: f32| { + let mut app = app(); + app.run("tools.setBrush", json!({"brush": {"size": 24, "hardness": hardness}})).unwrap(); + app.run("paint.pencil", json!({"points": [[15, 30]], "size": 40, "color": "#ff0000"})).unwrap(); + app.ui.tool = Tool::CloneStamp; + app.ui.clone_source = Some([15.0, 30.0]); + let m = egui::Modifiers::NONE; + tool_event(&mut app, ToolEvent::Down { x: 70.0, y: 30.0, pressure: 1.0 }, m); + tool_event(&mut app, ToolEvent::Up { x: 70.0, y: 30.0 }, m); + assert!(!app.ui.status_error, "{}", app.ui.status); + // 9 px from the centre of a 12 px radius: inside a hard tip, on a soft tip's falloff. + 1.0 - active(&app).surface().unwrap().rgba(79, 30)[1] + }; + let (hard, soft) = (rim(1.0), rim(0.0)); + assert!(hard > 0.99, "a hard tip clones fully near its rim: {hard}"); + assert!(soft < 0.8, "a soft tip clones only part of the source there: {soft}"); +} + #[test] fn alt_flips_dodge_burn_and_blur_sharpen() { for (tool, flipped) in [(Tool::Dodge, Tool::Burn), (Tool::Burn, Tool::Dodge), (Tool::Blur, Tool::Sharpen), (Tool::Sharpen, Tool::Blur)] { diff --git a/crates/ui-egui/src/tools/vector_ui/tests.rs b/crates/ui-egui/src/tools/vector_ui/tests.rs index 973682ab86..1fa3fa1c5e 100644 --- a/crates/ui-egui/src/tools/vector_ui/tests.rs +++ b/crates/ui-egui/src/tools/vector_ui/tests.rs @@ -1108,11 +1108,14 @@ fn new_fill_layer_takes_the_selected_path_as_its_vector_mask() { // Deselected (or a stale name): the selection is the layer mask, no vector mask. for sel in [None, Some("Gone".to_string())] { app.ui.selected_path = sel; + // The selection goes into the new layer's mask, so each fill layer needs its own (#8801). + app.run("select.rect", json!({"x": 150, "y": 150, "width": 20, "height": 20})).unwrap(); let r = app.run("layer.newFillLayer.solidColor", json!({})); let l = layer(&app, r); - assert!(l.vector_mask.is_none() && l.mask.is_some()); + assert!(l.vector_mask.is_none() && l.mask.as_ref().is_some_and(|m| m.value(160, 160) == 1.0 && m.value(5, 5) == 0.0)); } // Make Work Path selects the new work path. + app.run("select.rect", json!({"x": 150, "y": 150, "width": 20, "height": 20})).unwrap(); app.run("select.toWorkPath", json!({"tolerance": 2.0})).unwrap(); assert_eq!(app.ui.selected_path.as_deref(), Some("work")); } diff --git a/crates/vector/src/lib.rs b/crates/vector/src/lib.rs index d6c21e3ac3..7ccaa1b103 100644 --- a/crates/vector/src/lib.rs +++ b/crates/vector/src/lib.rs @@ -567,14 +567,21 @@ pub struct CompiledShape { impl CompiledShape { /// Compiles the shape at flattening tolerance `tol`. `canvas` is the frame gradients - /// without "Align with layer" are laid out in. + /// without "Align with layer" are laid out in, and the area a feather is computed over. pub fn new(shape: &ShapeLayer, tol: f64, canvas: Rect) -> Self { + Self::with_frames(shape, tol, canvas, canvas) + } + + /// [`CompiledShape::new`] with the feather computed over `coverage` while gradients still lay + /// out in `gradient_frame`: measuring a shape beyond the canvas it is drawn in. + pub fn with_frames(shape: &ShapeLayer, tol: f64, coverage: Rect, gradient_frame: Rect) -> Self { let pb = shape.path.control_bounds().unwrap_or((0.0, 0.0, 0.0, 0.0)); - let cb = (f64::from(canvas.x0), f64::from(canvas.y0), f64::from(canvas.x1), f64::from(canvas.y1)); + let g = gradient_frame; + let cb = (f64::from(g.x0), f64::from(g.y0), f64::from(g.x1), f64::from(g.y1)); let area = fill_rasterizer(&shape.path, tol); let mut bounds = None; let fill = shape.fill.as_ref().map(|f| { - let coverage = FillCoverage::new(shape, &area, canvas); + let coverage = FillCoverage::new(shape, &area, coverage); bounds = coverage.bounds(); (coverage, Paint::from_fill(f, pb, cb)) }); diff --git a/docs/releasing.md b/docs/releasing.md index d5687dbc72..b4fb5f3103 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -20,11 +20,10 @@ in the manual check window. Concurrent requests share one check. HTTPS validates certificates and host names against the operating system's trust store. Tags must be semantic versions (`v0.8.0` or `0.8.0`); only published stable versions newer -than the installed workspace version trigger a notice. The notice opens the official release -page to choose an installer; installation remains a user action. Drafts and prereleases do -not trigger it. Web/headless frontends and the Flatpak (no network in its sandbox; `flatpak update` -owns updates there) have no update transport. No release-workflow changes -are required: publishing a newer stable GitHub Release makes it discoverable. +than the installed workspace version trigger a notice. Drafts and prereleases do not trigger it. +Flatpak builds also check for releases; repository-managed installations are updated through +their package manager. Web/headless frontends have no update transport. No release-workflow +changes are required: publishing a newer stable GitHub Release makes it discoverable. ## Cutting a release @@ -280,10 +279,9 @@ Why these formats: Sandbox permissions (justified in the manifest): Wayland with X11 fallback, IPC (X11 shared memory), `dri` for the GPU, and read/write access to Pictures and Documents. Every - other file goes through the file-chooser and document portals. There's no network, so the - `--control` server is only reachable from inside the sandbox until the user runs - `flatpak override --user --share=network ai.storyteller.photocraft`. Host fonts are read from - `/run/host/fonts` and `/run/host/user-fonts`. Locally (on Linux, with `flatpak` and + other file goes through the file-chooser and document portals. Network access enables update + checks and makes the optional loopback `--control` server reachable from the host. Host fonts + are read from `/run/host/fonts` and `/run/host/user-fonts`. Locally (on Linux, with `flatpak` and `flatpak-builder`): `packaging/linux/package.sh --formats tar && packaging/linux/flatpak-bundle.sh`. - **Flathub**: `packaging/linux/flatpak/ai.storyteller.photocraft.yml` builds from source and is ready for a Flathub submission (it keeps the same runtime and `finish-args` as the bundle diff --git a/packaging/linux/flatpak/ai.storyteller.photocraft.bundle.yml b/packaging/linux/flatpak/ai.storyteller.photocraft.bundle.yml index c716cf310f..091e7159eb 100644 --- a/packaging/linux/flatpak/ai.storyteller.photocraft.bundle.yml +++ b/packaging/linux/flatpak/ai.storyteller.photocraft.bundle.yml @@ -22,9 +22,8 @@ finish-args: - --socket=fallback-x11 # X11 shared memory (MIT-SHM). Without it the X11 fallback is slow or fails. - --share=ipc - # Network for the optional --control server: it listens on 127.0.0.1 in the sandbox, which with - # network shared is also reachable from the host's network namespace. The in-app update check - # is disabled under Flatpak (flatpak update owns updates), so this does not enable it. + # Network for update checks and the optional --control server. The server listens on 127.0.0.1, + # which with network shared is also reachable from the host's network namespace. - --share=network # GPU access for the wgpu compositor (Vulkan, or GL as a fallback). - --device=dri diff --git a/packaging/linux/flatpak/ai.storyteller.photocraft.yml b/packaging/linux/flatpak/ai.storyteller.photocraft.yml index 7f333cf914..32e81545fb 100644 --- a/packaging/linux/flatpak/ai.storyteller.photocraft.yml +++ b/packaging/linux/flatpak/ai.storyteller.photocraft.yml @@ -23,9 +23,8 @@ finish-args: - --socket=fallback-x11 # X11 shared memory (MIT-SHM). Without it the X11 fallback is slow or fails. - --share=ipc - # Network for the optional --control server: it listens on 127.0.0.1 in the sandbox, which with - # network shared is also reachable from the host's network namespace. The in-app update check - # is disabled under Flatpak (flatpak update owns updates), so this does not enable it. + # Network for update checks and the optional --control server. The server listens on 127.0.0.1, + # which with network shared is also reachable from the host's network namespace. - --share=network # GPU access for the wgpu compositor (Vulkan, or GL as a fallback). - --device=dri