Merge pull request #7569 from aiongg/integration-4

Integration batch: 12 PRs on main
This commit is contained in:
aiongg
2026-10-18 23:29:05 -04:00
committed by GitHub
24 changed files with 543 additions and 34 deletions
+50 -10
View File
@@ -14,8 +14,9 @@
//! later (lower-right) cell wins. Work is split per destination tile (parallel on native).
use photocraft_color::PixelFormat;
use photocraft_geom::Rect;
use photocraft_raster::Surface;
use photocraft_geom::{Rect, TileCoord};
use photocraft_raster::{Surface, Tile};
use std::sync::Arc;
use crate::transform::{Interp, bicubic};
@@ -207,7 +208,7 @@ pub fn warp_triangles(src: &Surface, src_rect: Rect, verts: &[([f64; 2], [f64; 2
}
let n = fmt.channels();
let a = n - 1;
let work = |(t, tris): (&Rect, &Vec<[usize; 3]>)| -> Option<(Rect, Vec<f32>)> {
let work = |(t, tris): (&Rect, &Vec<[usize; 3]>)| -> Option<(TileCoord, Arc<Tile>)> {
if tris.is_empty() {
return None;
}
@@ -296,6 +297,11 @@ pub fn warp_triangles(src: &Surface, src_rect: Rect, verts: &[([f64; 2], [f64; 2
f64::from(px[((y - foot.y0) as usize * fw + (x - foot.x0) as usize) * n + c])
}
};
// Offset of pixel (x, y) when the `taps` x `taps` block from it lies inside `foot`, so
// the samplers can index `px` directly instead of bounds-checking every tap.
let base = |x: i32, y: i32, taps: i32| -> Option<usize> {
(x >= foot.x0 && y >= foot.y0 && x + taps <= foot.x1 && y + taps <= foot.y1).then(|| ((y - foot.y0) as usize * fw + (x - foot.x0) as usize) * n)
};
let mut outp = vec![0.0f32; w * h * n];
let mut any = false;
let mut acc = [0.0f64; 8];
@@ -313,6 +319,17 @@ pub fn warp_triangles(src: &Surface, src_rect: Rect, verts: &[([f64; 2], [f64; 2
Interp::Bilinear => {
let (ix, iy) = (u.floor() as i32, v.floor() as i32);
let (fx, fy) = (u - f64::from(ix), v - f64::from(iy));
if let Some(o) = base(ix, iy, 2) {
for (dy, wy) in [(0, 1.0 - fy), (1, fy)] {
for (dx, wx) in [(0, 1.0 - fx), (1, fx)] {
let p = &px[o + (dy * fw + dx) * n..][..n];
for (s, v) in acc.iter_mut().zip(p) {
*s += f64::from(*v) * wx * wy;
}
}
}
return;
}
for (dy, wy) in [(0, 1.0 - fy), (1, fy)] {
for (dx, wx) in [(0, 1.0 - fx), (1, fx)] {
for (c, s) in acc.iter_mut().enumerate().take(n) {
@@ -327,6 +344,21 @@ pub fn warp_triangles(src: &Surface, src_rect: Rect, verts: &[([f64; 2], [f64; 2
let (u, v) = (snap(u), snap(v));
let (ix, iy) = (u.floor() as i32, v.floor() as i32);
let (wx, wy) = (bicubic(u - f64::from(ix)), bicubic(v - f64::from(iy)));
if let Some(o) = base(ix - 1, iy - 1, 4) {
for (j, wyj) in wy.iter().enumerate() {
for (i, wxi) in wx.iter().enumerate() {
let k = wxi * wyj;
if k == 0.0 {
continue;
}
let p = &px[o + (j * fw + i) * n..][..n];
for (s, v) in acc.iter_mut().zip(p) {
*s += f64::from(*v) * k;
}
}
}
return;
}
for (j, wyj) in wy.iter().enumerate() {
for (i, wxi) in wx.iter().enumerate() {
let k = wxi * wyj;
@@ -378,19 +410,27 @@ pub fn warp_triangles(src: &Surface, src_rect: Rect, verts: &[([f64; 2], [f64; 2
}
outp[b + a] = al as f32;
}
any.then_some((*t, outp))
if !any {
return None;
}
// Encoded (and dropped if fully transparent) here, in parallel, rather than one tile at
// a time once all are done. `t` lies in one tile.
let mut one = Surface::new(fmt);
one.write_region(*t, &outp);
one.prune();
if one.tile_count() == 0 {
return None;
}
one.take_tiles(*t).pop()
};
#[cfg(not(target_arch = "wasm32"))]
let done: Vec<(Rect, Vec<f32>)> = {
let done: Vec<(TileCoord, Arc<Tile>)> = {
use rayon::prelude::*;
tiles.par_iter().zip(bins.par_iter()).filter_map(work).collect()
};
#[cfg(target_arch = "wasm32")]
let done: Vec<(Rect, Vec<f32>)> = tiles.iter().zip(bins.iter()).filter_map(work).collect();
for (r, v) in done {
out.write_region(r, &v);
}
out.prune();
let done: Vec<(TileCoord, Arc<Tile>)> = tiles.iter().zip(bins.iter()).filter_map(work).collect();
out.put_tiles(done);
out
}
+36 -10
View File
@@ -6,7 +6,9 @@
//! the same reader ([`super::tiff_ifd::File`]): reading a TIFF builds a block from the file's own
//! directories, writing one copies the block's directories into the file. Values are copied as
//! bytes (re-ordered when the byte orders differ), so tags PhotoCraft doesn't know survive.
//! Classic TIFF only on the reading side: an EXIF block has 4-byte offsets.
//! Classic TIFF only on the reading side: an EXIF block has 4-byte offsets. What is copied is
//! bounded by the stream's own size and a fixed cap, because entries of a hostile file can
//! overlap (see [`read`]).
use tiff::encoder::{DirectoryEncoder, TiffEncoder, TiffKind};
use tiff::tags::{Tag, Type};
@@ -27,6 +29,11 @@ const TAG_INTEROP_IFD: u16 = 40965;
const IFD0_TAGS: [u16; 7] = [270, 271, 272, 305, 306, 315, 33432];
/// At most this many entries are copied from one directory.
const MAX_FIELDS: usize = 1024;
/// At most this many value bytes are copied from one stream, over all its directories. Real EXIF
/// is far smaller (the largest Nikon maker note seen is under 1 MB); the cap leaves room for
/// that and for a few such notes. A stream never yields more than its own length either (see
/// [`read`]).
const MAX_COPIED_BYTES: usize = 4 * 1024 * 1024;
/// One entry with its value bytes in the byte order of the stream it came from.
#[derive(Clone, Debug, PartialEq)]
@@ -53,13 +60,22 @@ impl ExifDirs {
}
}
fn fields(f: &File<'_>, ifd: &Ifd, keep: impl Fn(u16) -> bool) -> Vec<Field> {
ifd.entries
.iter()
.filter(|e| keep(e.tag) && !matches!(e.tag, TAG_EXIF_IFD | TAG_GPS_IFD | TAG_INTEROP_IFD))
.filter_map(|e| Some(Field { tag: e.tag, ty: e.ty, count: u32::try_from(e.count).ok()?, value: f.data(e)?.to_vec() }))
.take(MAX_FIELDS)
.collect()
/// The entries of `ifd` that `keep` accepts, with their values copied. An entry's value only has
/// to lie inside the file, so entries may overlap: `budget` is the number of value bytes that may
/// still be copied (shared by all the directories of one stream), and an entry whose value does
/// not fit in what is left is skipped, so later, smaller entries can still be kept.
fn fields(f: &File<'_>, ifd: &Ifd, budget: &mut usize, keep: impl Fn(u16) -> bool) -> Vec<Field> {
let mut out = Vec::new();
for e in ifd.entries.iter().filter(|e| keep(e.tag) && !matches!(e.tag, TAG_EXIF_IFD | TAG_GPS_IFD | TAG_INTEROP_IFD)) {
if out.len() >= MAX_FIELDS {
break;
}
let (Ok(count), Some(value)) = (u32::try_from(e.count), f.data(e)) else { continue };
let Some(left) = budget.checked_sub(value.len()) else { continue };
*budget = left;
out.push(Field { tag: e.tag, ty: e.ty, count, value: value.to_vec() });
}
out
}
fn sub(f: &File<'_>, ifd: &Ifd, tag: u16) -> Option<Ifd> {
@@ -69,14 +85,24 @@ fn sub(f: &File<'_>, ifd: &Ifd, tag: u16) -> Option<Ifd> {
/// The EXIF parts of the classic TIFF stream `b` (a TIFF file, or an EXIF block without its
/// `Exif\0\0` prefix): IFD0's descriptive tags and the Exif and GPS directories.
///
/// The values copied add up to at most [`MAX_COPIED_BYTES`] and to at most `b.len()`: the values
/// of a well-formed stream are distinct bytes of it, while a hostile one can have many entries
/// point at the same bytes and so ask for far more than its size. Entries that do not fit in what
/// is left are left out (the small descriptive directories come first, so a huge maker note
/// cannot crowd out the GPS position).
pub(crate) fn read(b: &[u8]) -> Option<ExifDirs> {
let f = File::parse(b)?;
if f.big {
return None;
}
let ifd0 = f.ifd(f.first_ifd)?;
let dir = |tag: u16| sub(&f, &ifd0, tag).map(|d| fields(&f, &d, |_| true)).unwrap_or_default();
Some(ExifDirs { le: f.le, ifd0: fields(&f, &ifd0, |t| IFD0_TAGS.contains(&t)), exif: dir(TAG_EXIF_IFD), gps: dir(TAG_GPS_IFD) })
let mut budget = MAX_COPIED_BYTES.min(b.len());
let ifd0_fields = fields(&f, &ifd0, &mut budget, |t| IFD0_TAGS.contains(&t));
let mut dir = |tag: u16| sub(&f, &ifd0, tag).map(|d| fields(&f, &d, &mut budget, |_| true)).unwrap_or_default();
let gps = dir(TAG_GPS_IFD);
let exif = dir(TAG_EXIF_IFD);
Some(ExifDirs { le: f.le, ifd0: ifd0_fields, exif, gps })
}
/// Bytes per element whose order a byte-order change reverses: a RATIONAL is two LONGs.
+94
View File
@@ -166,3 +166,97 @@ fn no_exif_is_written_without_embed_metadata() {
let tiff = encode(&img, Format::Tiff, &EncodeOptions { embed_metadata: false, ..Default::default() }).unwrap();
assert_eq!(decode(&tiff).unwrap().meta.exif, None);
}
// ---------------------------------------------------------------------------
// Bounded copying: an entry's value only has to lie inside the file, so a hostile file can have
// many entries point at the same bytes.
// ---------------------------------------------------------------------------
/// A valid 1x1 gray TIFF whose Exif directory has `entries` UNDEFINED entries of `count` bytes
/// that all point at the same `count` bytes: the file is about `count` bytes long and asks to
/// copy `entries * count`.
fn overlapping_tiff(entries: usize, count: u32) -> Vec<u8> {
let entry = |tag: u16, ty: u16, count: u32, value: u32| {
[tag.to_le_bytes().to_vec(), ty.to_le_bytes().to_vec(), count.to_le_bytes().to_vec(), value.to_le_bytes().to_vec()].concat()
};
let at_exif = 8 + 2 + 12 * 10 + 4;
let data_at = at_exif + 2 + 12 * entries + 4;
let mut out = b"II".to_vec();
out.extend(42u16.to_le_bytes());
out.extend(8u32.to_le_bytes());
out.extend(10u16.to_le_bytes());
for (tag, ty, value) in
[(256, 3, 1), (257, 3, 1), (258, 3, 8), (259, 3, 1), (262, 3, 1), (273, 4, 0), (277, 3, 1), (278, 3, 1), (279, 4, 1), (34665, 4, at_exif as u32)]
{
out.extend(entry(tag, ty, 1, value));
}
out.extend(0u32.to_le_bytes());
out.extend((entries as u16).to_le_bytes());
for i in 0..entries {
out.extend(entry(i as u16 + 1, 7, count, data_at as u32));
}
out.extend(0u32.to_le_bytes());
out.extend(vec![0x5A; count as usize]);
out
}
#[test]
fn overlapping_field_values_are_bounded() {
// About 62 KB on disk; copied entry by entry it would be 1,000 x 50,000 bytes.
let tiff = overlapping_tiff(1000, 50_000);
assert!(tiff.len() < 64 * 1024);
let img = decode(&tiff).expect("the image itself is fine");
assert_eq!(img.dimensions(), (1, 1));
let exif = img.meta.exif.expect("the first entries are kept");
assert!(exif.len() <= 2 * tiff.len(), "{} bytes of EXIF from a {} byte file", exif.len(), tiff.len());
}
#[test]
fn overlapping_field_values_in_an_exif_block_are_bounded_on_export() {
// The same block as an image's EXIF: the writer reads it with the same code.
let block = overlapping_tiff(1000, 50_000);
let mut img = Image::from_raw(2, 2, ChannelLayout::Gray, SampleType::U8, vec![0; 4]).unwrap();
img.meta.exif = Some(block.clone());
let tiff = encode(&img, Format::Tiff, &EncodeOptions::default()).unwrap();
assert!(tiff.len() <= 2 * block.len(), "a {} byte block wrote a {} byte file", block.len(), tiff.len());
}
/// An EXIF block with a maker note of `len` bytes, an exposure time and a GPS latitude reference.
fn block_with_maker_note(le: bool, len: usize) -> Vec<u8> {
let b = Block { le };
b.build(
vec![(271, 2, 4, b"Cam\0".to_vec())],
vec![(33434, 5, 1, b.rational(1, 125)), (37500, 7, len as u32, (0..len).map(|i| (i * 7) as u8).collect())],
vec![(1, 2, 2, b"N\0".to_vec())],
)
}
#[test]
fn large_maker_notes_still_survive() {
// Nikon maker notes run from 100 KB to nearly 1 MB.
for len in [200 * 1024, 1000 * 1024] {
let block = block_with_maker_note(true, len);
let mut img = Image::from_raw(4, 3, ChannelLayout::Rgb, SampleType::U8, vec![90; 36]).unwrap();
img.meta.exif = Some(block.clone());
let tiff = encode(&img, Format::Tiff, &EncodeOptions::default()).unwrap();
let back = decode(&tiff).unwrap().meta.exif.expect("EXIF read back");
let (ble, _, bexif, bgps) = dirs(&back);
let (ole, _, oexif, ogps) = dirs(&block);
assert_eq!(normalized(&bexif, ble), normalized(&oexif, ole), "{len} byte maker note");
assert_eq!(normalized(&bgps, ble), normalized(&ogps, ole));
}
}
#[test]
fn a_field_beyond_the_cap_is_dropped_and_the_rest_kept() {
let block = block_with_maker_note(false, 6 * 1024 * 1024);
let mut img = Image::from_raw(2, 2, ChannelLayout::Gray, SampleType::U8, vec![0; 4]).unwrap();
img.meta.exif = Some(block);
let tiff = encode(&img, Format::Tiff, &EncodeOptions::default()).unwrap();
assert!(tiff.len() < 1024 * 1024, "{} bytes", tiff.len());
let back = decode(&tiff).unwrap().meta.exif.expect("EXIF read back");
let (_, ifd0, exif, gps) = dirs(&back);
assert_eq!(exif.iter().map(|e| e.0).collect::<Vec<_>>(), [33434], "the exposure time stays, the 6 MiB maker note goes");
assert_eq!(gps.iter().map(|e| e.0).collect::<Vec<_>>(), [1]);
assert_eq!(ifd0.iter().map(|e| e.0).collect::<Vec<_>>(), [271]);
}
+2
View File
@@ -270,6 +270,8 @@ pub(crate) fn adjust_as_smart_filter(s: &mut Session, kind: &str, label: &str, p
if !smart {
return Ok(false);
}
// A pixel or full lock refuses the smart filter, as for `filter.*` on a smart object.
crate::commands::check_pixels_unlocked(&s.active().ok_or(EngineError::NoDocument)?.doc, id)?;
// The recorded params are the command's own, minus the engine's private `__kind` key.
let mut params = p.clone();
if let Value::Object(m) = &mut params {
+20
View File
@@ -479,6 +479,26 @@ fn adjustment_smart_filters_take_a_selection_as_their_mask() {
assert_eq!(at(&now, 40, 16), at(&original, 40, 16), "outside the mask the composite is unchanged");
}
/// #5655: as for `filter.*` smart filters, a pixel or full lock refuses an adjustment smart
/// filter, leaving the layer and the history as they were.
#[test]
fn locked_smart_objects_refuse_adjustment_smart_filters() {
let adjustments =
[("image.adjustments.invert", json!({})), ("image.adjustments.shadowsHighlights", json!({"shadowAmount": 100, "blackClip": 0, "whiteClip": 0}))];
for lock in [json!({"all": true}), json!({"pixels": true})] {
for (cmd, p) in &adjustments {
let mut s = smart_session();
s.execute("layer.lockLayers", lock.clone()).unwrap();
let (steps, flat) = (s.active().unwrap().history.past_len(), flatten(&s));
let r = s.execute(cmd, p.clone());
assert!(r.is_err(), "{cmd} added a smart filter to a smart object locked with {lock}: {r:?}");
assert!(smart_filters_of(&s).is_empty(), "{cmd} {lock}: nothing recorded");
assert_eq!(s.active().unwrap().history.past_len(), steps, "{cmd} {lock}: no history step");
assert_eq!(flatten(&s), flat, "{cmd} {lock}: pixels untouched");
}
}
}
/// A wrong-typed value errors before anything is recorded (never a panic), and the adjustments
/// that can't re-render from a source (Replace Color, Match Color) stay off a smart object.
#[test]
+4 -1
View File
@@ -9,7 +9,7 @@
use photocraft_color::{Color, ColorMode, PixelFormat, SampleType};
use photocraft_doc::{Artboard, ArtboardBackground, Document, Layer, LayerContent, LayerId, Size};
use photocraft_geom::Rect;
use photocraft_geom::{Affine, Rect};
use photocraft_raster::Surface;
use serde_json::{Value, json};
@@ -506,6 +506,9 @@ pub fn artboard_document(doc: &Document, id: LayerId) -> Result<Option<Document>
one.layer_comps.clear();
one.last_applied_comp = None;
one.last_document_state = None;
// Paths, notes, slices and Count/Ruler marks move with the board. This also shifts
// `one.guides`, but they are rebuilt below from the source's, so nothing moves twice.
crate::canvas_geom::transform_doc_marks(&mut one, &Affine::translate(f64::from(dx), f64::from(dy)));
let shift = |v: &[f64], o: i32, len: u32| v.iter().map(|g| g - f64::from(o)).filter(|g| *g > 0.0 && *g < f64::from(len)).collect::<Vec<_>>();
one.guides.vertical = shift(&doc.guides.vertical, r.x0, r.width());
one.guides.horizontal = shift(&doc.guides.horizontal, r.y0, r.height());
+27
View File
@@ -331,6 +331,33 @@ fn export_rejects_a_board_too_far_from_the_origin() {
assert_eq!(one.artboards()[0].2.rect, Rect::new(0, 0, 50, 50));
}
/// Artboards to Files moves paths, notes, slices and Count/Ruler marks with the board, like its
/// layers and guides, so they land on the same pixels in the exported file.
#[test]
fn artboard_export_moves_paths_notes_slices_and_marks_with_the_board() {
use photocraft_doc::{CountGroup, NamedPath, Note, Path, Slice, Subpath};
let mut s = session(8);
s.execute("layer.new.artboard", json!({"x": 200, "y": 100, "width": 50, "height": 50})).unwrap();
s.edit("marks", |d, _| {
d.paths.push(NamedPath { name: "P".into(), path: Path::new(vec![Subpath::polygon(&[(210.0, 110.0), (240.0, 110.0), (240.0, 140.0)])]), psd_raw: None });
d.work_path = Some(Path::new(vec![Subpath::polygon(&[(205.0, 105.0), (215.0, 105.0), (215.0, 115.0)])]));
d.notes.push(Note { position: [220.0, 120.0], popup: [220.0, 120.0, 260.0, 160.0], ..Default::default() });
d.slices.list.push(Slice { id: 1, rect: Rect::new(210, 110, 230, 130), ..Default::default() });
d.measurement.count_groups.push(CountGroup { points: vec![[225.0, 125.0]], ..Default::default() });
Ok(())
})
.unwrap();
let one = artboard_document(doc(&s), doc(&s).artboards()[0].0).unwrap().unwrap();
let k = &one.paths[0].path.subpaths[0].knots[0].anchor;
assert_eq!((k.x, k.y), (10.0, 10.0));
let w = &one.work_path.as_ref().unwrap().subpaths[0].knots[0].anchor;
assert_eq!((w.x, w.y), (5.0, 5.0));
assert_eq!(one.notes[0].position, [20.0, 20.0]);
assert_eq!(one.notes[0].popup, [20.0, 20.0, 60.0, 60.0]);
assert_eq!(one.slices.list[0].rect, Rect::new(10, 10, 30, 30));
assert_eq!(one.measurement.count_groups[0].points, vec![[25.0, 25.0]]);
}
/// #1531: a board 40×40 at (10, 10) holding a red square at (10..30)², the artboard active.
fn board_with_square(s: &mut Session) -> LayerId {
add_square(s, Rect::new(10, 10, 30, 30), [1.0, 0.0, 0.0, 1.0]);
+8 -2
View File
@@ -55,9 +55,15 @@ fn map_fill(a: &Affine, f: &mut Fill) {
if let Fill::Gradient { angle, offset, .. } = f {
*angle = map_angle(a, *angle);
// The centre offset (a fraction of the frame) turns with the canvas: exact for 90° turns
// and flips, where the frame's sides swap with the axes.
// and flips, where the frame's sides swap with the axes. Scale (Image Size) resizes the
// frame too, so only each axis's direction applies.
let unit = |x: f64, y: f64| {
let n = x.hypot(y);
if n < EPS { (x, y) } else { (x / n, y / n) }
};
let ((m0, m1), (m2, m3)) = (unit(a.m[0], a.m[1]), unit(a.m[2], a.m[3]));
let (ox, oy) = (f64::from(offset.0), f64::from(offset.1));
*offset = ((a.m[0] * ox + a.m[2] * oy) as f32, (a.m[1] * ox + a.m[3] * oy) as f32);
*offset = ((m0 * ox + m2 * oy) as f32, (m1 * ox + m3 * oy) as f32);
}
}
+33
View File
@@ -188,6 +188,39 @@ fn gradient_fill_angle_follows_the_canvas() {
assert_eq!(map_angle(&Turn::Ccw90.affine(10.0, 20.0), 0.0), 90.0);
}
/// The gradient fill's centre offset after `cmd`, as `gradient.fill.get` reports it (% of the frame).
fn fill_offset_after(cmd: &str, p: serde_json::Value) -> (f64, f64) {
let mut s = Session::new();
s.execute("file.new", json!({"width": 100, "height": 100})).unwrap();
s.execute("gradient.fill.create", json!({"from": [10, 50], "to": [90, 50], "style": "radial"})).unwrap();
s.execute("gradient.fill.set", json!({"angle": 0, "offset": [25, 0]})).unwrap();
s.execute(cmd, p).unwrap();
let g = s.execute("gradient.fill.get", json!({})).unwrap();
(g["offset"][0].as_f64().unwrap(), g["offset"][1].as_f64().unwrap())
}
#[test]
fn gradient_fill_offset_stays_a_fraction_of_the_frame() {
let near = |a: (f64, f64), b: (f64, f64)| (a.0 - b.0).abs() < 1e-3 && (a.1 - b.1).abs() < 1e-3;
// Image Size scales the frame, so the offset (a fraction of it) stays put.
for (w, h) in [(200, 200), (50, 50), (200, 50)] {
let o = fill_offset_after("image.imageSize", json!({"width": w, "height": h}));
assert!(near(o, (25.0, 0.0)), "{w}x{h}: {o:?}");
}
// Flips and 90° turns still mirror and turn it.
assert!(near(fill_offset_after("image.imageRotation.flipCanvasHorizontal", json!({})), (-25.0, 0.0)));
assert!(near(fill_offset_after("image.imageRotation.90cw", json!({})), (0.0, 25.0)));
assert!(near(fill_offset_after("image.imageRotation.180", json!({})), (-25.0, 0.0)));
// Non-uniform scale with a turn: only the axis swap applies.
let mut f = Fill::gradient(Vec::new(), 0.0, 1.0, photocraft_doc::GradientStyle::Linear, false);
if let Fill::Gradient { offset, .. } = &mut f {
*offset = (0.25, -0.1);
}
map_fill(&Affine { m: [0.0, 3.0, -0.5, 0.0, 7.0, 1.0] }, &mut f);
let Fill::Gradient { offset, .. } = f else { panic!("not a gradient") };
assert!((offset.0 - 0.1).abs() < 1e-6 && (offset.1 - 0.25).abs() < 1e-6, "{offset:?}");
}
fn type_and_shape_origin(d: &Document) -> ([f64; 6], [f64; 2]) {
let mut t = None;
let mut k = None;
+7
View File
@@ -721,6 +721,13 @@ fn content_aware_fill_as(s: &mut Session, p: &Value, cmd: &'static str, label: &
let cancel = || ctx.cancelled();
let plan = content_aware_plan_with(&doc, id, &p, cmd, &photocraft_raster::Interrupt::cancel_only(&cancel))?;
let CafPlan { layer, rect, fmt, img, layer_px, cover, hole, source, opts, output, .. } = plan;
// Patches come only from the sampling area outside the fill area. With none (a
// Rectangular `margin` of 0, a Custom area inside the selection or off the canvas)
// the fill would be black (#6532), so refuse instead. The workspace's preview builds
// the same plan and may show an empty area while the Sampling Brush adds to it.
if !source.iter().any(|s| *s) {
return Err(bad(cmd, "nothing to sample from: the sampling area has no pixels outside the fill area"));
}
let (w, h) = (rect.width() as usize, rect.height() as usize);
let n = fmt.channels();
// The fill changes only the fill area: keep the layer's pixels and the coverage over
+26 -4
View File
@@ -137,10 +137,15 @@ fn blob_session(depth: u32) -> Session {
fn content_aware_fill_huge_area_neither_panics_nor_wraps() {
let mut s = blob_session(8);
// `[1e30, 0, 1e30, 10]` overflowed the i32 additions in the area parser (a debug-build
// panic, a garbage sampling window in release); the additions saturate now, so this is
// simply a whole-canvas window.
let r = s.execute("edit.contentAwareFill", json!({"sampling": "custom", "area": [1e30, 0.0, 1e30, 10.0], "colorAdaptation": "none"})).unwrap();
assert!(r["filled"].as_u64().unwrap() > 100);
// panic, a garbage sampling window in release); the additions saturate now, so the area lies
// wholly right of the canvas: nothing to sample, which is an error, not a black fill (#6532).
let before = s.active().unwrap().doc.clone();
let past = s.active().unwrap().history.past_len();
let err = s.execute("edit.contentAwareFill", json!({"sampling": "custom", "area": [1e30, 0.0, 1e30, 10.0], "colorAdaptation": "none"})).unwrap_err();
assert!(err.to_string().contains("nothing to sample from"), "{err}");
let st = s.active().unwrap();
assert!(std::sync::Arc::ptr_eq(&before, &st.doc), "the document changed");
assert_eq!(st.history.past_len(), past, "a history step was recorded");
// A malformed area names the problem instead of silently falling back.
for area in [json!([0.0, 0.0, null, 10.0]), json!([0.0, 0.0, 10.0]), json!("nope")] {
let err = s.execute("edit.contentAwareFill", json!({"sampling": "custom", "area": area})).unwrap_err();
@@ -344,6 +349,23 @@ fn content_aware_fill_takes_the_workspace_edits() {
assert!(r["filled"].as_u64().unwrap() > 2800);
}
/// #6532: a sampling area with no pixels outside the fill area is refused instead of filling it
/// black, and leaves the document untouched; any margin past the selection fills.
#[test]
fn content_aware_fill_with_nothing_to_sample_is_refused() {
let mut s = blob_session(8);
let before = px(&s, 30, 22);
let revision = s.active().unwrap().revision;
for p in [json!({"sampling": "rectangular", "margin": 0}), json!({"sampling": "custom", "area": [28, 18, 4, 4]})] {
let e = s.execute("edit.contentAwareFill", p.clone()).unwrap_err().to_string();
assert!(e.contains("nothing to sample from"), "{p}: {e}");
assert_eq!(px(&s, 30, 22), before, "{p}: nothing was filled");
assert_eq!(s.active().unwrap().revision, revision, "{p}: no history step");
}
s.execute("edit.contentAwareFill", json!({"sampling": "rectangular", "margin": 1})).unwrap();
assert_ne!(px(&s, 30, 22), before, "the red blob is filled from around the selection");
}
#[test]
fn content_aware_fill_outputs_and_sampling() {
let mut s = blob_session(8);
+3
View File
@@ -362,6 +362,9 @@ pub(crate) fn edit_groups<T: Named>(groups: &mut Vec<Group<T>>, action: &str, p:
let g = req_str(p, "group", cmd)?;
let to = req_str(p, "name", cmd)?.to_string();
let i = groups.iter().position(|x| x.name == g).ok_or_else(|| bad(cmd, format!("no group \"{g}\"")))?;
if g != to && groups.iter().any(|x| x.name == to) {
return Err(bad(cmd, format!("a group named \"{to}\" already exists")));
}
groups[i].name = to.clone();
Ok(json!({"group": to}))
}
+17
View File
@@ -317,6 +317,23 @@ fn gradient_new_rename_move_delete_groups() {
assert!(s.execute("gradient.presets.edit", json!({"action": "explode"})).is_err());
}
#[test]
fn gradient_rename_group_refuses_a_name_another_group_has() {
let mut s = Session::new();
s.execute("gradient.presets.edit", json!({"action": "newGroup", "name": "Pastels"})).unwrap();
let names = |s: &mut Session| -> Vec<Value> {
s.execute("gradient.presets.list", json!({})).unwrap()["groups"].as_array().unwrap().iter().map(|g| g["name"].clone()).collect()
};
let before = names(&mut s);
assert!(before.contains(&json!("Blues")));
let err = s.execute("gradient.presets.edit", json!({"action": "renameGroup", "group": "Pastels", "name": "Blues"})).unwrap_err();
assert!(err.to_string().contains("already exists"), "{err}");
assert_eq!(names(&mut s), before, "a refused rename leaves the groups unchanged");
// Renaming a group to its own name is fine.
s.execute("gradient.presets.edit", json!({"action": "renameGroup", "group": "Pastels", "name": "Pastels"})).unwrap();
assert_eq!(names(&mut s), before);
}
#[test]
fn presets_persist_through_preferences_json() {
let mut s = Session::new();
+6
View File
@@ -1065,6 +1065,9 @@ fn delete_filter(s: &mut Session, p: &Value) -> Result<Value> {
edit_filters(s, p, "Delete Smart Filter", |sm| {
let i = filter_index("layer.smartFilter.delete", p, sm)?;
sm.smart_filters.remove(i);
if sm.smart_filters.is_empty() {
sm.filter_mask = None;
}
Ok(())
})
}
@@ -1134,6 +1137,9 @@ fn transfer_filters(s: &mut Session, p: &Value) -> Result<Value> {
match index {
Some(i) => {
src.smart_filters.remove(i);
if src.smart_filters.is_empty() {
src.filter_mask = None;
}
}
None => {
src.smart_filters.clear();
+31
View File
@@ -330,6 +330,37 @@ fn selection_becomes_the_filter_mask() {
assert!(!s.is_enabled("layer.smartFilter.deleteFilterMask"));
}
/// #5619: deleting (or dragging away) the last smart filter removes the filter mask with it, so
/// the next filter takes the new selection instead of the old one.
#[test]
fn removing_the_last_filter_removes_its_filter_mask() {
let mut s = session(8);
paint(&mut s);
let original = flat(&s);
let a = convert(&mut s);
s.execute("select.rect", json!({"x": 0, "y": 0, "width": 16, "height": 48})).unwrap();
s.execute("filter.blur.gaussianBlur", json!({"radius": 3})).unwrap();
assert!(active_smart(&s).filter_mask.is_some());
s.execute("layer.smartFilter.delete", json!({})).unwrap();
assert!(active_smart(&s).filter_mask.is_none(), "the last filter takes its mask along");
s.execute("select.rect", json!({"x": 20, "y": 0, "width": 20, "height": 48})).unwrap();
s.execute("filter.blur.gaussianBlur", json!({"radius": 3})).unwrap();
let px = flat(&s);
let at = |v: &Vec<[f32; 4]>, x: i32, y: i32| v[(y * W + x) as usize];
assert_eq!(at(&px, 6, 20), at(&original, 6, 20), "the old selection is not filtered");
assert_ne!(at(&px, 30, 20), at(&original, 30, 20), "the new selection is filtered");
// Dragging the only filter to another smart object leaves no mask behind either.
s.execute("select.deselect", json!({})).unwrap();
s.execute("layer.new.layer", json!({})).unwrap();
paint(&mut s);
let b = convert(&mut s);
s.execute("layer.smartFilter.transfer", json!({"from": a, "to": b, "filter": 0})).unwrap();
let doc = &s.active().unwrap().doc;
let LayerContent::Smart(sm) = &doc.layer(LayerId(a)).unwrap().content else { panic!("not smart") };
assert!(sm.smart_filters.is_empty() && sm.filter_mask.is_none());
}
/// Scales `id` to 10 % around the origin and back, returning the composite.
fn shrink_and_grow(s: &mut Session, id: u64) -> Vec<[f32; 4]> {
s.execute("edit.transform", json!({"layer": id, "matrix": [0.1, 0, 0, 0.1, 0, 0]})).unwrap();
+1 -1
View File
@@ -59,7 +59,7 @@ fn duration_param(cmd: &str, p: &Value) -> Result<Option<usize>> {
}
/// The optional `fps` param, rejected unless it's a finite number in `(0, MAX_FPS]` (#1009).
fn fps_param(cmd: &str, p: &Value) -> Result<Option<f32>> {
pub(crate) fn fps_param(cmd: &str, p: &Value) -> Result<Option<f32>> {
let Some(v) = p.get("fps").filter(|v| !v.is_null()) else { return Ok(None) };
v.as_f64()
.filter(|f| f.is_finite() && *f > 0.0 && *f <= f64::from(MAX_FPS))
+54
View File
@@ -378,6 +378,9 @@ fn apply_transform(
// With a selection only the selected pixels move, and so only the same region of a linked
// mask (#205). Groups, type, shapes and smart objects move whole.
let mask_sel = doc_sel.filter(|_| !moves_whole(l));
// The masks (and so a fill's frame) move, but a fill layer's cached render isn't warped:
// drop it, the fill re-renders from its model (as `transform_layer_geometry` does).
l.fill_cache = None;
match &mut l.content {
LayerContent::Group(g) => {
for c in g.children.iter_mut() {
@@ -992,6 +995,57 @@ mod tests {
assert!(!active_bounds(&s).is_empty());
}
/// A gradient fill layer whose mask (a selection when it was made) frames the gradient over
/// 10..40, with the cached render a PSD import gives it: exactly the pixels the fill renders.
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();
let LayerContent::Fill(f) = &l.content else { panic!("a fill layer") };
let buf = photocraft_compose::render_fill_content(l, f, canvas, &[]);
let mut surface = Surface::new(fmt);
let vals: Vec<f32> = buf.px.iter().flat_map(|p| photocraft_raster::from_rgba(&fmt, *p)).collect();
surface.write_region(canvas, &vals);
l.fill_cache = Some(photocraft_doc::FillCache { fill: f.clone(), surface });
Ok(())
})
.unwrap();
s
}
/// How far the composite is from the composite with the fill re-rendered from its model
/// (cache dropped): the largest channel difference.
fn off_from_fresh(s: &Session) -> f32 {
let st = s.active().unwrap();
let mut fresh: Document = (*st.doc).clone();
fresh.layer_mut(st.active_layer.unwrap()).unwrap().fill_cache = None;
let (a, b) = (photocraft_compose::render(&st.doc, st.doc.bounds()).px, photocraft_compose::render(&fresh, fresh.bounds()).px);
a.iter().zip(&b).flat_map(|(p, q)| p.iter().zip(q).map(|(x, y)| (x - y).abs())).fold(0.0, f32::max)
}
#[test]
fn transforming_a_fill_layer_drops_its_stale_cached_render() {
let mut s = cached_gradient_fill();
let off = off_from_fresh(&s);
assert!(off < 0.01, "the cache is the fill's own render: {off}");
// The mask (and so the gradient's frame) moves 50 px right and doubles in height.
s.execute("edit.transform", json!({"matrix": [1, 0, 0, 2, 50, 0]})).unwrap();
let off = off_from_fresh(&s);
assert!(off < 0.01, "the composite shows the fill laid out in its new frame, not the old render: {off}");
}
#[test]
fn warping_a_fill_layer_drops_its_stale_cached_render() {
let mut s = cached_gradient_fill();
s.execute("edit.transform.warp", json!({"style": "arc", "bend": 50})).unwrap();
let off = off_from_fresh(&s);
assert!(off < 0.01, "the composite shows the fill laid out in its new frame, not the old render: {off}");
}
#[test]
fn type_moves_with_the_move_command() {
let mut s = session();
+3 -3
View File
@@ -301,7 +301,7 @@ fn new_from_file(s: &mut Session, p: &Value) -> Result<Value> {
let fmt = s.active().ok_or(EngineError::NoDocument)?.doc.pixel_format();
let Footage { frames, fps: footage_fps } = load_frames(&path, fmt)?;
// An explicit `fps` wins, then the rate the footage implies, then 30.
let fps = p.get("fps").and_then(Value::as_f64).map(|f| f as f32).or(footage_fps).unwrap_or(30.0);
let fps = crate::timeline_cmds::fps_param("layer.videoLayers.newVideoLayerFromFile", p)?.or(footage_fps).unwrap_or(30.0);
let n = frames.len();
let id = s.edit("New Video Layer from File", |doc, active| {
store(doc);
@@ -341,10 +341,10 @@ fn replace_footage(s: &mut Session, p: &Value) -> Result<Value> {
}
fn interpret_footage(s: &mut Session, p: &Value) -> Result<Value> {
let fps = p.get("fps").and_then(Value::as_f64);
let fps = crate::timeline_cmds::fps_param("layer.videoLayers.interpretFootage", p)?;
edit_video(s, "Interpret Footage", move |v, _| {
if let Some(f) = fps {
v.fps = f as f32;
v.fps = f;
}
Ok(())
})
+26
View File
@@ -363,6 +363,32 @@ fn a_still_gif_is_one_frame_at_the_default_rate() {
assert_eq!(s.active().unwrap().doc.layer(id).unwrap().video.as_ref().unwrap().fps, 10.0);
}
/// Interpret Footage and New Video Layer from File reject an fps that isn't a finite number in
/// `(0, MAX_FPS]` and keep the layer as it was (#5657). 1e39 used to become an infinite rate the
/// next `.pcraft` save refused.
#[test]
fn an_invalid_frame_rate_is_an_error_and_changes_nothing() {
let mut s = session();
let id = LayerId(s.execute("layer.videoLayers.newBlankVideoLayer", json!({})).unwrap()["layer"].as_u64().unwrap());
let fps = |s: &Session| s.active().unwrap().doc.layer(id).unwrap().video.as_ref().unwrap().fps;
assert_eq!(fps(&s), 30.0);
for bad in [json!(0), json!(-1), json!(1e39), json!("x")] {
assert!(s.execute("layer.videoLayers.interpretFootage", json!({"fps": bad})).is_err(), "fps {bad}");
assert_eq!(fps(&s), 30.0, "fps {bad}");
}
s.execute("layer.videoLayers.interpretFootage", json!({"fps": 24})).unwrap();
assert_eq!(fps(&s), 24.0);
let dir = tmpdir("gifin-badfps");
let path = format!("{dir}/anim.gif");
write_gif(&path, 8, 8, &[([255, 0, 0], 10), ([0, 255, 0], 10)]);
let layers = s.active().unwrap().doc.layer_count();
for bad in [json!(0), json!(-1), json!(1e39), json!("x")] {
assert!(s.execute("layer.videoLayers.newVideoLayerFromFile", json!({"path": path, "fps": bad})).is_err(), "fps {bad}");
assert_eq!(s.active().unwrap().doc.layer_count(), layers, "fps {bad}");
}
}
#[test]
fn video_frames_to_layers_takes_every_gif_frame() {
let dir = tmpdir("gifin-ftl");
+3
View File
@@ -121,6 +121,9 @@ fn warp_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut Layer, w: &Warp,
// A selection limits the linked mask exactly as it limits the pixels (#4175, as Free Transform
// and Perspective Warp do, #3083); a layer that moves whole takes its whole mask along.
let mask_sel = doc_sel.filter(|_| !crate::transform_cmds::moves_whole(l));
// The masks (and so a fill's frame) move, but a fill layer's cached render isn't warped:
// drop it, the fill re-renders from its model.
l.fill_cache = None;
match &mut l.content {
LayerContent::Group(g) => {
for c in g.children.iter_mut() {
+32
View File
@@ -907,6 +907,38 @@ fn smoothing_without_catch_up_stops_while_the_pointer_pauses() {
assert_eq!(out.len(), n, "held still: the brush doesn't move");
}
#[test]
fn a_smoothed_brush_keeps_the_pressure_of_where_it_is_when_the_pen_lifts() {
// #5713: a fast stroke at full pressure, lifted in the last sample. The brush trails far
// behind the pointer; it used to take the lift's pressure there, and the end catch-up drew
// a hairline from the trailing brush to the pointer.
let pts: Vec<StrokePoint> =
(0..=20).map(|i| StrokePoint { time: f64::from(i) * 4.0, ..StrokePoint::new(f64::from(i) * 10.0, 0.0, if i < 20 { 1.0 } else { 0.05 }) }).collect();
let out = smooth_points(&Smoothing { amount: 0.8, ..Default::default() }, 1.0, &pts);
let (last, rest) = out.split_last().unwrap();
assert_eq!((last.x, last.pressure), (200.0, 0.05), "the catch-up still ends at the pen's last sample");
let lifted = rest.last().unwrap();
assert!(lifted.x < 190.0, "the brush lags the pointer: {}", lifted.x);
for p in rest {
assert!(p.pressure > 0.5, "the trailing brush at x {} has pressure {}", p.x, p.pressure);
}
}
#[test]
fn a_pulled_string_brush_takes_the_pose_along_the_string() {
let ps = Smoothing { amount: 0.2, pulled_string: true, catch_up_on_end: false, ..Default::default() };
let a = StrokePoint { rotation: 350.0, ..StrokePoint::new(0.0, 0.0, 1.0) };
let b = StrokePoint { rotation: 10.0, tilt_x: 40.0, ..StrokePoint::new(100.0, 0.0, 0.0) };
let out = smooth_points(&ps, 1.0, &[a, b]);
// The 20 px string leaves the brush 80 % of the way to the pointer.
let p = out.last().unwrap();
assert!((p.x - 80.0).abs() < 1e-4, "{}", p.x);
assert!((p.pressure - 0.2).abs() < 1e-4, "{}", p.pressure);
assert!((p.tilt_x - 32.0).abs() < 1e-3, "{}", p.tilt_x);
// Rotation the short way round: 350° + 80 % of 20°.
assert!((p.rotation - 6.0).abs() < 1e-3, "{}", p.rotation);
}
#[test]
fn pose_overrides_stylus_values() {
let b = BrushSettings {
+23 -2
View File
@@ -55,6 +55,27 @@ fn lerp_pt(a: &StrokePoint, b: &StrokePoint, f: f64) -> StrokePoint {
// Smoothing
// ---------------------------------------------------------------------------
/// The smoothed brush moved `f` of the way from `brush` towards the pointer `p`. Its pose
/// (pressure, tilt, rotation, wheel) moves with it, so the lagging brush paints with the pose the
/// pen had along the way, not the pointer's newest one. Taking the pointer's pressure made a fast
/// stroke's lift thin the brush while it still trailed far behind, and the end catch-up a hairline
/// (#5713). Time is the pointer's.
fn pull(brush: &StrokePoint, p: &StrokePoint, f: f64) -> StrokePoint {
let ff = f as f32;
// Barrel rotation takes the shorter way round.
let dr = (p.rotation - brush.rotation + 540.0).rem_euclid(360.0) - 180.0;
StrokePoint {
x: brush.x + (p.x - brush.x) * f,
y: brush.y + (p.y - brush.y) * f,
pressure: brush.pressure + (p.pressure - brush.pressure) * ff,
tilt_x: brush.tilt_x + (p.tilt_x - brush.tilt_x) * ff,
tilt_y: brush.tilt_y + (p.tilt_y - brush.tilt_y) * ff,
rotation: (brush.rotation + dr * ff).rem_euclid(360.0),
wheel: brush.wheel + (p.wheel - brush.wheel) * ff,
time: p.time,
}
}
/// Incremental stroke smoothing: exponential (with optional time-based catch-up) or pulled string.
#[derive(Clone, Debug)]
pub struct Smoother {
@@ -96,7 +117,7 @@ impl Smoother {
let d = (dx * dx + dy * dy).sqrt();
if d > len {
let k = (d - len) / d;
let np = StrokePoint { x: pos.x + dx * k, y: pos.y + dy * k, ..p };
let np = pull(&pos, &p, k);
self.pos = Some(np);
out.push(np);
}
@@ -122,7 +143,7 @@ impl Smoother {
};
let mut cur = pos;
for _ in 0..iters {
cur = StrokePoint { x: cur.x + (p.x - cur.x) * step, y: cur.y + (p.y - cur.y) * step, ..p };
cur = pull(&cur, &p, step);
out.push(cur);
}
self.pos = Some(cur);
+6 -1
View File
@@ -218,7 +218,12 @@ impl Keys {
self.alt_down = false;
tap |= std::mem::take(&mut self.alt_tap);
}
Event::Key { pressed: true, .. } | Event::Text(_) | Event::PointerButton { pressed: true, .. } => self.alt_tap = false,
// Alt+wheel (zoom, brush size…) or Alt going to the desktop is no tap either.
Event::Key { pressed: true, .. }
| Event::Text(_)
| Event::PointerButton { pressed: true, .. }
| Event::MouseWheel { .. }
| Event::WindowFocused(false) => self.alt_tap = false,
_ => {}
}
}
+31
View File
@@ -76,6 +76,37 @@ fn alt_focuses_the_bar_arrows_move_and_down_opens_the_lit_menu() {
assert!(!is_open(&h.ctx) && Keys::current(&h.ctx).focus.is_none(), "Esc leaves the menu and the bar");
}
/// Alt+wheel zooms; letting go of Alt after it is no tap, so Space (the hand tool) can't then
/// open File and run New….
#[test]
fn alt_with_a_wheel_scroll_or_a_lost_focus_is_no_tap() {
let mut h = bar();
h.key_down(egui::Key::AltLeft);
h.run_steps(2);
h.event_modifiers(
egui::Event::MouseWheel {
unit: egui::MouseWheelUnit::Line,
delta: egui::vec2(0.0, -1.0),
phase: egui::TouchPhase::Move,
modifiers: egui::Modifiers::ALT,
},
egui::Modifiers::ALT,
);
h.run_steps(2);
h.key_up(egui::Key::AltLeft);
h.run_steps(3);
assert_eq!(Keys::current(&h.ctx).focus, None, "Alt+scroll leaves the bar alone");
key(&mut h, egui::Key::Space);
assert!(!is_open(&h.ctx), "Space opens no menu");
h.key_down(egui::Key::AltLeft);
h.run_steps(2);
h.event(egui::Event::WindowFocused(false));
h.event(egui::Event::WindowFocused(true));
h.key_up(egui::Key::AltLeft);
h.run_steps(3);
assert_eq!(Keys::current(&h.ctx).focus, None, "Alt that went to the desktop leaves the bar alone");
}
#[test]
fn a_second_alt_or_esc_or_a_click_takes_the_keyboard_off_the_bar() {
let mut h = bar();