From b24c0360f1839885812f87feabaa102afcce9c3c Mon Sep 17 00:00:00 2001 From: Michael Higgins <55243596+Higgs32584@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:37:56 -0400 Subject: [PATCH 1/5] Dependencies: require ruzstd 0.9.1 for RUSTSEC-2026-0344 (#4952) (#5225) ruzstd before 0.9.1 lets a crafted zstd input under 128 KB allocate several gigabytes at once. PhotoCraft decodes zstd when it opens .pcraft and Affinity files, so such a file could end the process with an out-of-memory abort. Raise the minimum in both crates that use it and update the lockfile from 0.9.0 to 0.9.1. Co-authored-by: Claude Opus 5.5 --- Cargo.lock | 4 ++-- crates/affinity/Cargo.toml | 2 +- crates/format/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b3a268add9..138ca3d814 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6113,9 +6113,9 @@ dependencies = [ [[package]] name = "ruzstd" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a252f5e20f038fe7b4ea53e073e65398d652c864cc162fc77c56c2f13717b888" +checksum = "69e4ce4a039f42bf25920b95667e4d789e98749ab11bbb8d2c2936f788421bb5" dependencies = [ "twox-hash", ] diff --git a/crates/affinity/Cargo.toml b/crates/affinity/Cargo.toml index e686e737ef..49d859c2f8 100644 --- a/crates/affinity/Cargo.toml +++ b/crates/affinity/Cargo.toml @@ -16,7 +16,7 @@ synth = [] [dependencies] crc32fast = "1.5" flate2 = { workspace = true } -ruzstd = "0.9" +ruzstd = "0.9.1" [dev-dependencies] proptest = { workspace = true } diff --git a/crates/format/Cargo.toml b/crates/format/Cargo.toml index 45d7a4ccf3..113d9ba026 100644 --- a/crates/format/Cargo.toml +++ b/crates/format/Cargo.toml @@ -22,7 +22,7 @@ flate2 = { workspace = true } # Same IEEE CRC-32 the ZIP container needs; already in Cargo.lock through flate2. crc32fast = "1.5" blake3 = { version = "1.8", default-features = false, features = ["pure"] } -ruzstd = "0.9" +ruzstd = "0.9.1" # Unix directory cleanup uses no-follow descriptor-relative operations; the same # dependency provides the plain-fsync fallback on Apple file systems. From 64520f645e073b23960b774a677d54f504094843 Mon Sep 17 00:00:00 2001 From: Michael Higgins <55243596+Higgs32584@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:39:19 -0400 Subject: [PATCH 2/5] Control channel: connections must authenticate within 10 s, however active (#4965) (#5229) The only limit on an unauthenticated control connection was the 30 s idle timeout, which starts again with every read. A local client that kept sending bytes or blank lines without ever completing `auth` kept its connection, and one of the 16 connection permits, for as long as it liked; sixteen of them locked every other client out. AuthDeadlineStream wraps the read half of a connection: until the connection authenticates, reads fail once AUTH_DEADLINE (10 s) has passed since it connected, and a blocking read waits no longer than what is left. Both transports use it (the desktop app's control server and the headless `photocraft-cli serve`). After `auth` succeeds only the idle timeout applies, as before. Co-authored-by: Claude Opus 5.5 --- apps/photocraft/src/control_server.rs | 45 ++++++++++- crates/automation/src/rpc.rs | 69 ++++++++++++++-- crates/automation/src/security.rs | 110 +++++++++++++++++++++++++- 3 files changed, 213 insertions(+), 11 deletions(-) diff --git a/apps/photocraft/src/control_server.rs b/apps/photocraft/src/control_server.rs index 258d89e1ec..30ae610b70 100644 --- a/apps/photocraft/src/control_server.rs +++ b/apps/photocraft/src/control_server.rs @@ -5,11 +5,12 @@ use std::io::{BufReader, Write}; use std::net::{TcpListener, TcpStream}; use std::sync::Arc; use std::sync::mpsc::{Receiver, Sender, channel}; -use std::time::Instant; +use std::time::{Duration, Instant}; use photocraft_automation::budgets::write_reply; use photocraft_automation::security::{ - ConnectionLimiter, LineRead, MAX_CONNECTIONS, MAX_REQUEST_BYTES, authentication_reply, configure_stream, read_bounded_line, repeat_auth_reply, + AUTH_DEADLINE, AuthDeadlineStream, ConnectionLimiter, LineRead, MAX_CONNECTIONS, MAX_REQUEST_BYTES, authentication_reply, configure_stream, + read_bounded_line, repeat_auth_reply, }; use photocraft_ui_egui::ControlRequest; use photocraft_ui_egui::control::REPLY_DEADLINE; @@ -47,11 +48,17 @@ pub fn start(port: u16, token: String, ctx: egui::Context) -> Receiver, ctx: egui::Context) { + serve_with_auth_deadline(stream, token, tx, ctx, AUTH_DEADLINE); +} + +/// [`serve`] for a connection that must complete `auth` within `auth_deadline` of connecting, +/// however much it sends meanwhile (#4965); after that only the idle timeout applies. +fn serve_with_auth_deadline(stream: TcpStream, token: &str, tx: Sender, ctx: egui::Context, auth_deadline: Duration) { if configure_stream(&stream).is_err() { return; } let Ok(read) = stream.try_clone() else { return }; - let mut reader = BufReader::new(read); + let mut reader = BufReader::new(AuthDeadlineStream::new(read, auth_deadline)); let mut out = stream; let mut line = String::new(); let mut authenticated = false; @@ -74,6 +81,9 @@ fn serve(stream: TcpStream, token: &str, tx: Sender, ctx: egui:: if !authenticated { let (reply, ok) = authentication_reply(&line, token); authenticated = ok; + if ok && reader.get_mut().authenticated().is_err() { + break; + } if write_reply(&mut out, &reply).is_err() || out.flush().is_err() || !authenticated { break; } @@ -120,7 +130,34 @@ fn serve(stream: TcpStream, token: &str, tx: Sender, ctx: egui:: mod tests { use super::*; use std::io::BufRead; - use std::time::Duration; + + /// #4965: a connection that keeps sending without authenticating is closed at the deadline + /// (releasing its connection permit), and nothing it sent is dispatched. + #[test] + fn an_unauthenticated_connection_is_closed_at_the_deadline_whatever_it_sends() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let (tx, rx) = channel::(); + let server = std::thread::spawn(move || { + let (stream, _) = listener.accept().unwrap(); + let started = Instant::now(); + serve_with_auth_deadline(stream, "unused", tx, egui::Context::default(), Duration::from_millis(300)); + started.elapsed() + }); + // Blank lines and partial frames, one byte every 20 ms: each read succeeds long before + // the idle timeout. + let mut stream = TcpStream::connect(addr).unwrap(); + for i in 0..150 { + let byte: &[u8] = if i % 5 == 0 { b"\n" } else { b" " }; + if stream.write_all(byte).is_err() { + break; + } + std::thread::sleep(Duration::from_millis(20)); + } + let served = server.join().unwrap(); + assert!(served < Duration::from_secs(3), "the trickle did not extend the deadline: {served:?}"); + assert!(rx.try_recv().is_err(), "nothing was dispatched"); + } #[test] fn oversized_reply_preserves_framing_id_and_the_next_control_request() { diff --git a/crates/automation/src/rpc.rs b/crates/automation/src/rpc.rs index dc01f0f2b4..15a0e24e30 100644 --- a/crates/automation/src/rpc.rs +++ b/crates/automation/src/rpc.rs @@ -29,8 +29,8 @@ use serde_json::{Value, json}; use crate::budgets::{BatchReplyBudget, write_reply}; use crate::security::{ - ConnectionLimiter, LineRead, MAX_BATCH_STEPS, MAX_CONNECTIONS, MAX_REQUEST_BYTES, authentication_reply, configure_stream, discard_rest_of_line, - read_bounded_line, repeat_auth_reply, + AUTH_DEADLINE, AuthDeadlineStream, ConnectionLimiter, LineRead, MAX_BATCH_STEPS, MAX_CONNECTIONS, MAX_REQUEST_BYTES, authentication_reply, + configure_stream, discard_rest_of_line, read_bounded_line, repeat_auth_reply, }; use crate::{AutomationError, Headless}; @@ -262,10 +262,12 @@ pub fn serve_lines(h: &Mutex, mut r: impl BufRead, mut w: impl Write) } } -fn serve_tcp_connection(h: &Mutex, stream: std::net::TcpStream, token: &str) -> std::io::Result<()> { +/// Serve one connection. It must complete `auth` within `auth_deadline` of connecting, however +/// much it sends meanwhile (#4965); after that only the idle timeout applies. +fn serve_tcp_connection(h: &Mutex, stream: std::net::TcpStream, token: &str, auth_deadline: std::time::Duration) -> std::io::Result<()> { configure_stream(&stream)?; let read = stream.try_clone()?; - let mut reader = std::io::BufReader::new(read); + let mut reader = std::io::BufReader::new(AuthDeadlineStream::new(read, auth_deadline)); let mut out = stream; let mut line = String::new(); let mut authenticated = false; @@ -290,6 +292,9 @@ fn serve_tcp_connection(h: &Mutex, stream: std::net::TcpStream, token: } else { let (reply, ok) = authentication_reply(&line, token); authenticated = ok; + if ok { + reader.get_mut().authenticated()?; + } reply }; write_reply(&mut out, &reply)?; @@ -322,7 +327,7 @@ pub fn serve_tcp(addr: &str, h: Arc>, token: String, ready: impl let token = Arc::clone(&token); std::thread::spawn(move || { let _permit = permit; - let _ = serve_tcp_connection(&h, stream, &token); + let _ = serve_tcp_connection(&h, stream, &token, AUTH_DEADLINE); }); } Ok(()) @@ -814,6 +819,60 @@ mod tests { assert!(v["result"].as_array().unwrap().iter().any(|m| m == "batch")); } + /// #4965: a connection that keeps sending without authenticating is closed at the deadline, + /// and one that authenticated in time keeps being served after it. + #[test] + fn tcp_connections_must_authenticate_before_the_deadline_whatever_they_send() { + use std::io::{BufReader, Write as _}; + use std::time::{Duration, Instant}; + const TOKEN: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let server = std::thread::spawn(move || { + let h = Mutex::new(Headless::new()); + let mut results = Vec::new(); + for _ in 0..2 { + let (stream, _) = listener.accept().unwrap(); + let started = Instant::now(); + let r = serve_tcp_connection(&h, stream, TOKEN, Duration::from_millis(300)); + results.push((r.map_err(|e| e.kind()), started.elapsed())); + } + results + }); + + // Never a newline, a byte every 20 ms: each read succeeds long before the idle timeout. + let mut slow = std::net::TcpStream::connect(addr).unwrap(); + for _ in 0..150 { + if slow.write_all(b" ").is_err() { + break; + } + std::thread::sleep(Duration::from_millis(20)); + } + drop(slow); + + // Authenticates at once, then asks again 300 ms after the deadline. + let mut s = std::net::TcpStream::connect(addr).unwrap(); + s.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + writeln!(s, r#"{{"id":"auth","method":"auth","params":{{"token":"{TOKEN}"}}}}"#).unwrap(); + let mut reader = BufReader::new(s.try_clone().unwrap()); + let mut line = String::new(); + reader.read_line(&mut line).unwrap(); + assert_eq!(serde_json::from_str::(&line).unwrap()["result"]["authenticated"], true); + std::thread::sleep(Duration::from_millis(600)); + writeln!(s, r#"{{"id":"a","method":"methods"}}"#).unwrap(); + line.clear(); + reader.read_line(&mut line).unwrap(); + assert_eq!(serde_json::from_str::(&line).unwrap()["id"], "a"); + drop(reader); + drop(s); + + let results = server.join().unwrap(); + let (slow_result, slow_time) = &results[0]; + assert!(matches!(slow_result, Err(std::io::ErrorKind::TimedOut | std::io::ErrorKind::WouldBlock)), "{slow_result:?}"); + assert!(*slow_time < Duration::from_secs(3), "the trickle did not extend the deadline: {slow_time:?}"); + assert_eq!(results[1].0, Ok(()), "the authenticated connection ended with its client"); + } + #[test] fn tcp_repeat_auth_reports_already_authenticated_and_keeps_serving() { use std::io::{BufReader, Write as _}; diff --git a/crates/automation/src/security.rs b/crates/automation/src/security.rs index 4659a3ff17..7928420b12 100644 --- a/crates/automation/src/security.rs +++ b/crates/automation/src/security.rs @@ -1,11 +1,11 @@ //! Shared security primitives for the loopback JSON control transports. use std::fs::OpenOptions; -use std::io::{BufRead, Write}; +use std::io::{BufRead, Read, Write}; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::time::Duration; +use std::time::{Duration, Instant}; use serde_json::{Value, json}; @@ -21,6 +21,10 @@ pub const MAX_CONNECTIONS: usize = 16; pub const MAX_BATCH_STEPS: usize = 256; /// Idle/read and write timeout for loopback TCP connections. pub const IO_TIMEOUT: Duration = Duration::from_secs(30); +/// How long a new TCP connection has to authenticate, however active it is. [`IO_TIMEOUT`] starts +/// again with every read, so a client that kept sending without ever completing `auth` held one +/// of the [`MAX_CONNECTIONS`] permits for as long as it liked (#4965). +pub const AUTH_DEADLINE: Duration = Duration::from_secs(10); const TOKEN_BYTES: usize = 32; const TOKEN_HEX_LEN: usize = TOKEN_BYTES * 2; @@ -242,6 +246,44 @@ pub fn configure_stream(stream: &std::net::TcpStream) -> std::io::Result<()> { stream.set_write_timeout(Some(IO_TIMEOUT)) } +/// The read half of a control connection. Until [`Self::authenticated`] is called, reads fail +/// with `TimedOut` once the authentication deadline has passed, whatever arrived in the meantime; +/// the caller then drops the connection and its [`ConnectionPermit`]. +pub struct AuthDeadlineStream { + stream: std::net::TcpStream, + /// `None` once authenticated. + deadline: Option, +} + +impl AuthDeadlineStream { + /// `stream` must authenticate within `limit` from now (normally [`AUTH_DEADLINE`]). + pub fn new(stream: std::net::TcpStream, limit: Duration) -> Self { + // A limit too large to represent expires at once rather than never. + let now = Instant::now(); + Self { stream, deadline: Some(now.checked_add(limit).unwrap_or(now)) } + } + + /// The connection authenticated: only the idle timeout applies from here on. + pub fn authenticated(&mut self) -> std::io::Result<()> { + self.deadline = None; + self.stream.set_read_timeout(Some(IO_TIMEOUT)) + } +} + +impl Read for AuthDeadlineStream { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if let Some(deadline) = self.deadline { + let left = deadline.saturating_duration_since(Instant::now()); + if left.is_zero() { + return Err(std::io::Error::new(std::io::ErrorKind::TimedOut, "not authenticated in time")); + } + // Wait no longer than what is left, so a silent client is dropped at the deadline too. + self.stream.set_read_timeout(Some(left.min(IO_TIMEOUT)))?; + } + self.stream.read(buf) + } +} + /// Environment-aware token inputs shared by the desktop app and CLI. pub fn token_inputs(supplied: Option, token_file: Option) -> (Option, Option) { let supplied = supplied.or_else(|| std::env::var("PHOTOCRAFT_CONTROL_TOKEN").ok()); @@ -327,6 +369,70 @@ mod tests { std::fs::remove_file(path).unwrap(); } + /// A connected loopback pair: the accepted (server) end and the client end. + fn tcp_pair() -> (std::net::TcpStream, std::net::TcpStream) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let client = std::net::TcpStream::connect(listener.local_addr().unwrap()).unwrap(); + let (server, _) = listener.accept().unwrap(); + configure_stream(&server).unwrap(); + (server, client) + } + + /// #4965: activity must not extend the time a connection may stay unauthenticated. + #[test] + fn an_active_connection_is_dropped_at_the_authentication_deadline() { + let (server, mut client) = tcp_pair(); + // A byte every 20 ms and never a newline: each read succeeds long before the idle timeout. + let trickle = std::thread::spawn(move || { + for _ in 0..150 { + if client.write_all(b"x").is_err() { + break; + } + std::thread::sleep(Duration::from_millis(20)); + } + }); + let mut reader = std::io::BufReader::new(AuthDeadlineStream::new(server, Duration::from_millis(300))); + let mut line = String::new(); + let started = Instant::now(); + let err = read_bounded_line(&mut reader, &mut line).unwrap_err(); + assert!(matches!(err.kind(), std::io::ErrorKind::TimedOut | std::io::ErrorKind::WouldBlock), "{err}"); + assert!(started.elapsed() >= Duration::from_millis(250), "not before the deadline: {:?}", started.elapsed()); + assert!(started.elapsed() < Duration::from_secs(3), "the trickle did not extend it: {:?}", started.elapsed()); + // It stays expired. + assert!(read_bounded_line(&mut reader, &mut line).is_err()); + drop(reader); + trickle.join().unwrap(); + } + + #[test] + fn a_silent_connection_is_dropped_at_the_authentication_deadline() { + let (server, _client) = tcp_pair(); + let mut reader = std::io::BufReader::new(AuthDeadlineStream::new(server, Duration::from_millis(200))); + let mut line = String::new(); + let started = Instant::now(); + assert!(read_bounded_line(&mut reader, &mut line).is_err()); + assert!(started.elapsed() < Duration::from_secs(3), "{:?}", started.elapsed()); + } + + #[test] + fn an_authenticated_connection_outlives_the_authentication_deadline() { + let (server, mut client) = tcp_pair(); + let sender = std::thread::spawn(move || { + client.write_all(b"one\n").unwrap(); + std::thread::sleep(Duration::from_millis(500)); + client.write_all(b"two\n").unwrap(); + }); + let mut reader = std::io::BufReader::new(AuthDeadlineStream::new(server, Duration::from_millis(200))); + let mut line = String::new(); + assert_eq!(read_bounded_line(&mut reader, &mut line).unwrap(), LineRead::Line); + assert_eq!(line, "one\n"); + reader.get_mut().authenticated().unwrap(); + // Sent 300 ms after the deadline: only the idle timeout applies now. + assert_eq!(read_bounded_line(&mut reader, &mut line).unwrap(), LineRead::Line); + assert_eq!(line, "two\n"); + sender.join().unwrap(); + } + #[test] fn connection_limiter_releases_capacity() { let limiter = ConnectionLimiter::new(1); From e4b2868a02c99afb30ed5c61022e03b13574e873 Mon Sep 17 00:00:00 2001 From: jonathanwxh-cell Date: Fri, 16 Oct 2026 03:40:50 +0800 Subject: [PATCH 3/5] Check deep EXR tile count tables against max_alloc (#5632) * Check deep EXR tile count tables against max_alloc A deep tile's sample-count table was decompressed at the header's full tile size with no budget check, so a 20 KB file could build a 4 MiB table and a scratch copy under a 1 MiB max_alloc. Try that size only when max_alloc has room for both; otherwise read the clipped size. Fixes #4958. Co-Authored-By: Claude Opus 5.5 * Keep deep EXR decompression within the size it was admitted at The count-table check admits the table and its scratch copy, 2x the table, but Vec doubling gave a 4 MiB ZIPS table 8 MiB of capacity (a 3 MiB RLE table 4 MiB), so an admitted table and its copy could take 3x. Grow the decompressed buffer by doubling capped at the expected size, inflating straight into it, and allocate it and the scratch copy fallibly. Tests: decompressed buffers never reserve past the expected size; a near-maximal tile with a clipped table decodes under every budget; per-tile admission; half and uint channels after full-size tables; the 8 MiB boundary for uncompressed tables too. Refs #4958. Co-Authored-By: Claude Opus 5.5 * Tests: show deep tile tables are admitted one tile at a time The 17x17 image in 16x16 tiles had an interior tile, which never reaches the max_alloc check, so only three edge tables (6 KiB) were checked against the 8000-byte budget and a check that summed admissions across tiles still passed. Use a 48x1 image instead: its three tiles are all edge tiles, and under 4096 bytes a summed check fails at the third tile. Refs #4958. Co-Authored-By: Claude Opus 5.5 * Complete deep EXR count-table allocation admission Account for scratch space on clipped and interior compressed tables while preserving raw-table budgets. Borrow packed input before admission and make initial RLE reservations and raw copies fallible. Add single-channel admission boundaries, raw fallback controls, and hostile-size borrowed-read regressions. * ci: run deep EXR regressions independently * Drop the separate deep EXR CI job It was added so these tests could run while main's workspace suite was failing. Main is green again, and the regular test jobs run them. This reverts commit 5aa4869b45ffd8e89cdd3f02c9aa71d64e217929. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: jonathnwxh <258340642+jonathanwxh-cell@users.noreply.github.com> --- crates/codecs/src/codecs/deep_exr.rs | 195 ++++++++++++++++++++++++--- crates/codecs/tests/deep_exr.rs | 152 ++++++++++++++++++++- 2 files changed, 325 insertions(+), 22 deletions(-) diff --git a/crates/codecs/src/codecs/deep_exr.rs b/crates/codecs/src/codecs/deep_exr.rs index 0726228be5..29f3762ebc 100644 --- a/crates/codecs/src/codecs/deep_exr.rs +++ b/crates/codecs/src/codecs/deep_exr.rs @@ -23,7 +23,7 @@ use half::f16; use crate::Format; use crate::error::CodecError; use crate::image::{ChannelLayout, DecodeWarning, DeepChannel, DeepImage, Image, SampleType}; -use crate::options::Limits; +use crate::options::{Limits, alloc_zeroed}; const F: Format = Format::OpenExr; @@ -39,9 +39,15 @@ fn unsup(reason: impl Into) -> CodecError { // Every length and offset comes from the file, so every read is bounds-checked and a short // read is a malformed file, never a panic. -fn read_exact(cur: &mut Cursor<&[u8]>, n: usize, what: &str) -> Result, CodecError> { - let mut v = vec![0u8; n]; - cur.read_exact(&mut v).map_err(|_| err(format!("truncated file while reading {what}")))?; +fn read_exact<'a>(cur: &mut Cursor<&'a [u8]>, n: usize, what: &str) -> Result<&'a [u8], CodecError> { + // Borrow packed payloads from the input. Copying them here allocates before count-table + // admission and needlessly duplicates the table even when the second pass just skips it. + let truncated = || err(format!("truncated file while reading {what}")); + let start = usize::try_from(cur.position()).map_err(|_| truncated())?; + let end = start.checked_add(n).ok_or_else(truncated)?; + let bytes = *cur.get_ref(); + let v = bytes.get(start..end).ok_or_else(truncated)?; + cur.set_position(end as u64); Ok(v) } @@ -81,10 +87,11 @@ fn differences_to_samples(buf: &mut [u8]) { } /// Undo the ZIP half-split: the stream is `[first half][second half]` with the second half -/// holding every second byte of the original; weave them back together. -fn interleave_byte_blocks(buf: &mut [u8]) { +/// holding every second byte of the original; weave them back together. The scratch copy is +/// allocated fallibly, so a buffer that passed `max_alloc` but not the allocator is an error. +fn interleave_byte_blocks(buf: &mut [u8]) -> Result<(), CodecError> { let half = buf.len().div_ceil(2); - let mut out = vec![0u8; buf.len()]; + let mut out = alloc_zeroed::(buf.len())?; let mut k = 0usize; for i in 0..buf.len() / 2 { let (a, b) = (buf.get(i).copied(), buf.get(half + i).copied()); @@ -105,12 +112,29 @@ fn interleave_byte_blocks(buf: &mut [u8]) { } } buf.copy_from_slice(&out); + Ok(()) +} + +/// Make room in `out` for `extra` more bytes, `limit` in all. Grows by doubling like `Vec`, +/// but never reserves past `limit`: callers admit exactly that size against `max_alloc`, and +/// plain doubling reserved up to twice as much. Fallible, so a failed allocation is an error. +fn reserve_up_to(out: &mut Vec, extra: usize, limit: usize, what: &str) -> Result<(), CodecError> { + let need = out.len().saturating_add(extra); + if need > limit { + return Err(err(format!("{what}: data expands past {limit} bytes"))); + } + if need > out.capacity() { + let target = out.capacity().saturating_mul(2).max(need).min(limit); + out.try_reserve_exact(target.saturating_sub(out.len())).map_err(|_| CodecError::LimitExceeded(format!("cannot allocate {target} bytes for {what}")))?; + } + Ok(()) } /// OpenEXR RLE tokens: a negative byte `n` means `-n` literal bytes follow, a non-negative /// byte `n` means the next byte repeats `n + 1` times (as `ImfRle.cpp` and the `exr` crate). fn rle_decode(src: &[u8], expected: usize, what: &str) -> Result, CodecError> { - let mut out = Vec::with_capacity(expected.min(1 << 20)); + let mut out = Vec::new(); + reserve_up_to(&mut out, expected.min(1 << 20), expected, what)?; let mut i = 0; while i < src.len() { let count = src[i] as i8; @@ -119,6 +143,7 @@ fn rle_decode(src: &[u8], expected: usize, what: &str) -> Result, CodecE if out.len().saturating_add(n) > expected { return Err(err(format!("{what}: RLE data expands past {expected} bytes"))); } + reserve_up_to(&mut out, n, expected, what)?; if count < 0 { let end = i.saturating_add(n); let run = src.get(i..end.min(src.len())).ok_or_else(|| err(format!("{what}: RLE run points past the compressed data")))?; @@ -143,7 +168,10 @@ fn rle_decode(src: &[u8], expected: usize, what: &str) -> Result, CodecE /// has the uncompressed length is stored raw whatever the declared compression. fn decompress(compression: Compression, packed: &[u8], expected: usize, what: &str) -> Result, CodecError> { if packed.len() == expected { - return Ok(packed.to_vec()); + let mut out = Vec::new(); + reserve_up_to(&mut out, expected, expected, what)?; + out.extend_from_slice(packed); + return Ok(out); } match compression { Compression::Uncompressed => Err(err(format!("{what}: expected {expected} uncompressed bytes, found {}", packed.len()))), @@ -151,20 +179,38 @@ fn decompress(compression: Compression, packed: &[u8], expected: usize, what: &s // Like ZIPS, RLE runs over the predicted, byte-split stream. let mut out = rle_decode(packed, expected, what)?; differences_to_samples(&mut out); - interleave_byte_blocks(&mut out); + interleave_byte_blocks(&mut out)?; Ok(out) } Compression::ZIP1 => { - let mut out = Vec::with_capacity(expected.min(1 << 20)); - flate2::read::ZlibDecoder::new(packed) - .take(expected as u64 + 1) - .read_to_end(&mut out) - .map_err(|_| err(format!("{what}: zlib data is malformed")))?; + // Inflate straight into `out`, grown with `reserve_up_to` rather than by + // `read_to_end`, whose doubling reserves past `expected`. Once `expected` bytes are + // in, a one-byte read checks that the stream ends there. + let malformed = || err(format!("{what}: zlib data is malformed")); + let mut zlib = flate2::read::ZlibDecoder::new(packed); + let mut out = Vec::new(); + let mut filled = 0; + while filled < expected { + if filled == out.len() { + let grow = out.len().max(1 << 13).min(expected - filled); + reserve_up_to(&mut out, grow, expected, what)?; + out.resize(filled + grow, 0); + } + let n = zlib.read(out.get_mut(filled..).unwrap_or_default()).map_err(|_| malformed())?; + if n == 0 { + break; + } + filled += n; + } + out.truncate(filled); + if filled == expected && zlib.read(&mut [0u8; 1]).map_err(|_| malformed())? != 0 { + return Err(err(format!("{what}: zlib data expands past {expected} bytes"))); + } if out.len() != expected { return Err(err(format!("{what}: zlib data expands to {} bytes, expected {expected}", out.len()))); } differences_to_samples(&mut out); - interleave_byte_blocks(&mut out); + interleave_byte_blocks(&mut out)?; Ok(out) } other => Err(unsup(format!("deep data with {other:?} compression (NONE, RLE and ZIPS are supported)"))), @@ -340,13 +386,41 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res if packed_table > remaining || packed_data > remaining.saturating_sub(packed_table) { return Err(err("deep chunk sizes point past the end of the file")); } - let table_bytes = read_exact(&mut cur, packed_table as usize, "the sample-count table")?; + let what = "the sample-count table"; + let table_bytes = read_exact(&mut cur, packed_table as usize, what)?; let expected_table = block_pixels.len().checked_mul(4).ok_or_else(|| err("count table overflows"))?; + // A compressed clipped table also needs its scratch copy. The data-window check only + // budgets four bytes per channel per pixel, insufficient for a one-channel image. + // An equal-sized payload takes decompress's raw path, which allocates no scratch. + let buffers = if table_bytes.len() == expected_table { 1 } else { 2 }; + let clipped_need = (expected_table as u64).saturating_mul(buffers); + if clipped_need > limits.max_alloc { + return Err(CodecError::LimitExceeded(format!( + "{what} needs {clipped_need} bytes including decompression scratch, exceeding max_alloc {}", + limits.max_alloc + ))); + } // An edge tile's table may still be stored at the full tile size (its pixels first, in // rows of the clipped width, then unused entries), as the OpenEXR library writes it. - let full_table = tile_w.saturating_mul(tile_h).saturating_mul(4).max(expected_table); - let raw = decompress(compression, &table_bytes, full_table, "the sample-count table") - .or_else(|_| decompress(compression, &table_bytes, expected_table, "the sample-count table"))?; + // That size comes from the header's tile size alone, which nothing ties to the data + // window, so it is tried only when `max_alloc` has room for the table and for the copy + // that undoes its byte split. Otherwise only the clipped size is read. + let full_table = tile_w.checked_mul(tile_h).and_then(|n| n.checked_mul(4)); + let raw = match full_table { + // Scan lines and interior tiles: the full size is the clipped size. + Some(full) if full <= expected_table => decompress(compression, table_bytes, expected_table, what)?, + Some(full) if (full as u64).saturating_mul(2) <= limits.max_alloc => { + decompress(compression, table_bytes, full, what).or_else(|_| decompress(compression, table_bytes, expected_table, what))? + } + _ => decompress(compression, table_bytes, expected_table, what).map_err(|_| { + // Four bytes per pixel of the tile, twice: the table and its scratch copy. + let need = (tile_w as u64).saturating_mul(tile_h as u64).saturating_mul(8); + CodecError::LimitExceeded(format!( + "{what} of a {tile_w}x{tile_h} deep tile does not decode at the clipped size, and at the full tile size it needs {need} bytes with its scratch copy, exceeding max_alloc {}", + limits.max_alloc + )) + })?, + }; // The table is cumulative within each row of the block (a tile restarts at every row); // `prefix` is cumulative over the whole block, and each pixel's own count is the difference. @@ -423,7 +497,7 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res // The count table sits between the sizes and the sample data; skip its bytes. read_exact(&mut cur, packed_table as usize, "the sample-count table")?; let data_bytes = read_exact(&mut cur, chunk.packed_data as usize, "the deep sample data")?; - let raw = decompress(compression, &data_bytes, chunk.unpacked_data as usize, "the deep sample data")?; + let raw = decompress(compression, data_bytes, chunk.unpacked_data as usize, "the deep sample data")?; // Row by row (a scan line block is one row), and within a row channel by channel, as // flat tiles are laid out: each pixel's samples are contiguous within its channel. let mut in_off = 0usize; @@ -611,3 +685,82 @@ pub(crate) fn flatten(deep: &DeepImage) -> Result { img.warnings.push(DecodeWarning::DeepFlattened { format: F, max_samples_per_pixel: max_samples }); Ok(img) } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write as _; + + #[test] + fn packed_reads_borrow_input_and_reject_bad_sizes_before_allocation() { + let bytes = [1u8, 2, 3, 4]; + let mut cur = Cursor::new(bytes.as_slice()); + let prefix = read_exact(&mut cur, 3, "x").expect("three input bytes"); + assert_eq!(prefix, &[1, 2, 3]); + assert_eq!(prefix.as_ptr(), bytes.as_ptr()); + assert_eq!(cur.position(), 3); + assert!(matches!(read_exact(&mut cur, usize::MAX, "x"), Err(CodecError::Malformed { .. }))); + assert_eq!(cur.position(), 3); + assert_eq!(read_exact(&mut cur, 1, "x").expect("last byte"), &[4]); + assert!(matches!(read_exact(&mut cur, 1, "x"), Err(CodecError::Malformed { .. }))); + cur.set_position(u64::MAX); + assert!(matches!(read_exact(&mut cur, 1, "x"), Err(CodecError::Malformed { .. }))); + } + + /// What the writer stores before compressing: the byte split, then the +128 predictor. + fn predicted(raw: &[u8]) -> Vec { + let half = raw.len().div_ceil(2); + let mut out = vec![0u8; raw.len()]; + for (i, &v) in raw.iter().enumerate() { + out[if i % 2 == 0 { i / 2 } else { half + i / 2 }] = v; + } + let mut prev = 0i16; + for (i, b) in out.iter_mut().enumerate() { + let v = i16::from(*b); + if i > 0 { + *b = (v - prev + 128) as u8; + } + prev = v; + } + out + } + + /// #4958: callers admit `expected` bytes for a decompressed buffer (twice that for a count + /// table, with the copy that undoes its byte split), so decompression must not reserve + /// more. `Vec`'s doubling gave a 4 MiB ZIPS table 8 MiB of capacity and a 3 MiB RLE table + /// 4 MiB, so the "admitted" table and its copy took up to three times the table size. + #[test] + fn decompressed_buffers_reserve_no_more_than_expected() { + for n in [1usize << 20, 2 << 20, (3 << 20) + 1, 4 << 20, 5 << 20] { + let raw = vec![0u8; n]; + let p = predicted(&raw); + let mut z = flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::fast()); + z.write_all(&p).expect("write to a Vec"); + let zips = z.finish().expect("finish a Vec"); + // RLE: the first byte as a literal, then runs of up to 128 copies of the next. + let mut rle = vec![0xFF, p[0]]; + for run in p[1..].chunks(128) { + rle.extend_from_slice(&[(run.len() - 1) as u8, run[0]]); + } + for (compression, packed) in [(Compression::ZIP1, &zips), (Compression::RLE, &rle)] { + let out = decompress(compression, packed, n, "the table").expect("decompresses"); + assert_eq!(out, raw, "{compression:?}, {n} bytes"); + assert!(out.capacity() <= n, "{compression:?}: a {n} byte buffer reserved {} bytes", out.capacity()); + } + // One byte short, the same data expands past the expected size and is refused. + for (compression, packed) in [(Compression::ZIP1, &zips), (Compression::RLE, &rle)] { + assert!(decompress(compression, packed, n - 1, "the table").is_err(), "{compression:?}, {n} bytes"); + } + } + } + + /// A size past the limit is malformed data, and a reservation the allocator refuses is an + /// error rather than an abort. + #[test] + fn reservations_past_the_limit_or_the_allocator_are_errors() { + let mut v = Vec::new(); + assert!(matches!(reserve_up_to(&mut v, 2, 1, "x"), Err(CodecError::Malformed { .. }))); + assert!(matches!(reserve_up_to(&mut v, isize::MAX as usize + 1, usize::MAX, "x"), Err(CodecError::LimitExceeded(_)))); + assert_eq!(v.capacity(), 0); + } +} diff --git a/crates/codecs/tests/deep_exr.rs b/crates/codecs/tests/deep_exr.rs index 5a6d6ae75e..f80a42f17b 100644 --- a/crates/codecs/tests/deep_exr.rs +++ b/crates/codecs/tests/deep_exr.rs @@ -118,6 +118,23 @@ fn gen_deep( tile: Option<(usize, usize)>, counts: &[u32], sample: &dyn Fn(usize, usize) -> Vec, +) -> Vec { + gen_deep_tables(w, h, channels, compression, tile, counts, sample, false) +} + +/// [`gen_deep`]; with `full_tile_tables` every tile's count table is stored at the full tile +/// size, as the OpenEXR library writes edge tiles: the block's entries first (in rows of its +/// clipped width), then zeros. +#[allow(clippy::too_many_arguments)] +fn gen_deep_tables( + w: usize, + h: usize, + channels: &[Chan], + compression: u8, + tile: Option<(usize, usize)>, + counts: &[u32], + sample: &dyn Fn(usize, usize) -> Vec, + full_tile_tables: bool, ) -> Vec { let mut out = Vec::new(); out.extend_from_slice(&0x0131_2f76u32.to_le_bytes()); @@ -216,6 +233,10 @@ fn gen_deep( table.extend_from_slice(&acc.to_le_bytes()); } } + assert_eq!(table.len(), table_len(b)); + if let (true, Some((tw, th))) = (full_tile_tables, tile) { + table.resize(tw * th * 4, 0); + } let mut data = Vec::new(); for row in b.pixels.chunks(b.row_w) { for (ci, ch) in channels.iter().enumerate() { @@ -237,7 +258,6 @@ fn gen_deep( c.extend_from_slice(&(data.len() as u64).to_le_bytes()); c.extend_from_slice(&ct); c.extend_from_slice(&cd); - assert_eq!(table.len(), table_len(b)); assert_eq!(data.len(), data_len(b)); chunks.push((bi, c)); } @@ -608,6 +628,136 @@ fn hostile_sizes_and_offsets_error() { assert!(matches!(decode_deep_exr(&far, &Limits::none()), Err(CodecError::Malformed { .. }))); } +/// #4958: a deep tile's count table may be stored at the full tile size, and that size comes +/// from the header alone: nothing ties it to the data window. One 1024×1024 tile over a 4×4 +/// image, its table stored at that size, makes a compressed file of tens of KB whose table +/// inflates to 4 MiB, plus as much again for the copy that undoes the byte split. Under a +/// budget smaller than both the table must be refused before it is allocated; with room for +/// both it decodes. Tables stored at the clipped size, and the full-size edge tables of +/// ordinary tiles, still decode under a small budget. +#[test] +fn deep_tile_count_table_is_admitted_against_max_alloc() { + let budget = |max_alloc: u64| Limits { max_alloc, ..Limits::none() }; + let tight = budget(1 << 20); + let table: u64 = 1024 * 1024 * 4; // the full tile's count table, in bytes + let scan = decode_deep_exr(&gen_deep(4, 4, &RGBAZ, 0, None, COUNTS_ONE, &sample_rgba), &Limits::none()).expect("scan lines"); + for compression in [0u8, 1, 2] { + let big = gen_deep_tables(4, 4, &RGBAZ, compression, Some((1024, 1024)), COUNTS_ONE, &sample_rgba, true); + assert_eq!(decode_deep_exr(&big, &Limits::none()).expect("full-size table, no limits"), scan, "compression {compression}"); + let got = decode_deep_exr(&big, &tight); + assert!( + matches!(got, Err(CodecError::LimitExceeded(_))), + "compression {compression}: a 4 MiB count table under a 1 MiB budget gave {:?}", + got.map(|d| d.total_samples()) + ); + let flat = photocraft_codecs::decode_with(&big, &photocraft_codecs::DecodeOptions { limits: tight, ..Default::default() }); + assert!(matches!(flat, Err(CodecError::LimitExceeded(_))), "compression {compression}: the flat decode gave {:?}", flat.map(|i| i.width())); + // The same tile with its table stored at the clipped size fits the small budget. + let clipped = gen_deep(4, 4, &RGBAZ, compression, Some((1024, 1024)), COUNTS_ONE, &sample_rgba); + assert_eq!(decode_deep_exr(&clipped, &tight).expect("clipped table, 1 MiB budget"), scan, "compression {compression}"); + if compression != 0 { + assert!(big.len() < 1 << 20, "compression {compression}: the file is {} bytes", big.len()); + } + // The table and its scratch copy are admitted together: 8 MiB. + assert_eq!(decode_deep_exr(&big, &budget(2 * table)).expect("room for the table and its copy"), scan, "compression {compression}"); + let got = decode_deep_exr(&big, &budget(2 * table - 1)); + assert!(matches!(got, Err(CodecError::LimitExceeded(_))), "compression {compression}: one byte short gave {:?}", got.map(|d| d.total_samples())); + } + // Ordinary tiles: full-size edge tables decode under the small budget, like clipped ones. + let counts: Vec = (0..35).map(|p| (p * 3) % 4).collect(); + let sample = |p: usize, ci: usize| -> Vec { (0..counts[p]).map(|k| [0.5, 0.25, 0.125 * k as f32, 1.0, 10.0 - k as f32][ci]).collect() }; + for compression in [0u8, 1, 2] { + let full = gen_deep_tables(7, 5, &RGBAZ, compression, Some((3, 2)), &counts, &sample, true); + let clipped = gen_deep(7, 5, &RGBAZ, compression, Some((3, 2)), &counts, &sample); + assert_eq!( + decode_deep_exr(&full, &tight).expect("full-size edge tables"), + decode_deep_exr(&clipped, &tight).expect("clipped edge tables"), + "compression {compression}" + ); + } +} + +/// #4958, the edges of that admission. A tile of the largest size exr accepts (2^30 - 2 a side, +/// a 2^62 byte count table) decodes from a clipped table under every budget, including no limits, +/// where its full size passes the check and is only tried as the data arrives. Each tile is +/// admitted on its own, because its table is freed before the next chunk: three 16×16 tiles over +/// a 48×1 image are all edge tiles, so every full-size table reaches the check (an interior tile's +/// full size equals its clipped size, so it uses the clipped check). They need 2 KiB each for +/// the table and its copy, 6 KiB in all, and decode under a budget of 4096 bytes. Half and uint +/// channels decode after full-size tables like float ones. +#[test] +fn deep_tile_table_admission_edges() { + let budget = |max_alloc: u64| Limits { max_alloc, ..Limits::none() }; + let scan = decode_deep_exr(&gen_deep(4, 4, &RGBAZ, 0, None, COUNTS_ONE, &sample_rgba), &Limits::none()).expect("scan lines"); + let side = (1 << 30) - 2; + for compression in [0u8, 1, 2] { + let bytes = gen_deep(4, 4, &RGBAZ, compression, Some((side, side)), COUNTS_ONE, &sample_rgba); + for limits in [Limits::none(), Limits::default(), budget(1 << 20)] { + assert_eq!(decode_deep_exr(&bytes, &limits).expect("the largest tile, clipped table"), scan, "compression {compression}, {limits:?}"); + } + } + let counts: Vec = (0..48).map(|p| (p % 3) as u32).collect(); + let half: [Chan; 5] = RGBAZ.map(|c| Chan { ty: 1, ..c }); + let [a, b, g, r, z] = RGBAZ; + let with_id = [a, b, g, r, z, Chan { name: "id", ty: 0 }]; + for channels in [&RGBAZ[..], &half[..], &with_id[..]] { + let sample = |p: usize, ci: usize| -> Vec { (0..counts[p]).map(|k| [0.5, 0.25, 0.125, 1.0, 2.0 + k as f32, 7.0][ci]).collect() }; + for compression in [0u8, 1, 2] { + let full = gen_deep_tables(48, 1, channels, compression, Some((16, 16)), &counts, &sample, true); + let clipped = gen_deep(48, 1, channels, compression, Some((16, 16)), &counts, &sample); + let what = format!("{} channels, compression {compression}", channels.len()); + let want = decode_deep_exr(&clipped, &Limits::none()).expect("clipped tables"); + assert_eq!(decode_deep_exr(&full, &budget(4096)).expect("full-size tables, admitted one at a time"), want, "{what}"); + } + } +} + +/// A one-channel data-window check admits only four bytes per pixel, but a compressed count +/// table and its scratch copy need eight. Cover scanlines, interior tiles and the clipped fallback; +/// the existing multichannel cases have enough pixel allowance to hide this gap. +#[test] +fn clipped_count_tables_include_scratch_in_the_budget() { + let channels = [Chan { name: "Y", ty: 2 }]; + let counts = vec![0u32; 64]; + let sample = |_: usize, _: usize| Vec::new(); + let budget = |max_alloc: u64| Limits { max_alloc, ..Limits::none() }; + for tile in [None, Some((64, 1)), Some((128, 2))] { + for compression in [0u8, 1, 2] { + let bytes = gen_deep(64, 1, &channels, compression, tile, &counts, &sample); + let what = format!("tile {tile:?}, compression {compression}"); + let want = decode_deep_exr(&bytes, &Limits::none()).expect("unlimited decode"); + let required = if compression == 0 { 256 } else { 512 }; + assert_eq!(decode_deep_exr(&bytes, &budget(required)).expect("table and any scratch fit exactly"), want, "{what}"); + assert!(matches!(decode_deep_exr(&bytes, &budget(required - 1)), Err(CodecError::LimitExceeded(_))), "{what}"); + let flat = photocraft_codecs::decode_with(&bytes, &photocraft_codecs::DecodeOptions { limits: budget(required - 1), ..Default::default() }); + assert!(matches!(flat, Err(CodecError::LimitExceeded(_))), "{what}"); + if compression == 0 { + assert_eq!(decode_deep_exr(&bytes, &budget(511)).expect("raw table needs no scratch"), want, "{what}"); + } + } + } +} + +/// RLE and ZIPS store incompressible tables raw too. Their actual payload, not the header's +/// compression tag, decides whether a scratch buffer is needed. +#[test] +fn raw_count_tables_need_no_scratch_even_with_compression() { + let channels = [Chan { name: "Y", ty: 2 }]; + let limits = Limits { max_alloc: 4, ..Limits::none() }; + for tile in [None, Some((1, 1)), Some((2, 2))] { + for compression in [0u8, 1, 2] { + let bytes = gen_deep(1, 1, &channels, compression, tile, &[0], &|_, _| Vec::new()); + let header_end = header_end_of(&bytes); + let chunk = u64::from_le_bytes(bytes[header_end..header_end + 8].try_into().expect("chunk offset")) as usize; + let table_size = chunk + if tile.is_some() { 16 } else { 4 }; + let packed = u64::from_le_bytes(bytes[table_size..table_size + 8].try_into().expect("table size")); + assert_eq!(packed, 4, "the four-byte count table is stored raw"); + let want = decode_deep_exr(&bytes, &Limits::none()).expect("unlimited decode"); + assert_eq!(decode_deep_exr(&bytes, &limits).expect("raw table fits"), want, "tile {tile:?}, compression {compression}"); + } + } +} + /// Walk the attributes like the format prescribes: the offset table starts after the /// header's terminating zero byte. fn header_end_of(bytes: &[u8]) -> usize { From 77a258b9a0c0a165a7ccca414883e18415ef7323 Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Thu, 15 Oct 2026 13:10:22 -0700 Subject: [PATCH 4/5] Clip groups: Release Clipping Mask takes the layers above; merges keep clipping; Delete Hidden settles (#5868) * Clip groups: Release Clipping Mask takes the layers above; merges keep clipping; Delete Hidden settles From the layers audit (L2, silently wrong results): - Release Clipping Mask on a clipped layer also releases the clipped layers above it, as Adobe documents ("removes the selected layer and any layers above it from the clipping mask"). Before, they stayed clipped to the released layer. - Merge Down of two clipped layers, and Merge Layers of clipped layers of one chain, give a layer still clipped to their base (it was always unclipped). Merge Down's result also keeps the lower layer's colour label and link; Merge Layers' keeps the top layer's label. - Delete Hidden Layers releases the layers clipped to a deleted hidden base (clip_group rule 3, as single Delete already does), instead of leaving them clipped to the layer below. * Merge Layers keeps the result clipped only when the clipped layers share one clip chain * fmt --- crates/engine/src/clip_group.rs | 13 ++++ crates/engine/src/commands.rs | 19 +++++- crates/engine/src/extra_cmds.rs | 5 ++ crates/engine/src/layer_multi_cmds.rs | 6 ++ crates/engine/src/pixels.rs | 5 ++ crates/engine/tests/clipping_semantics.rs | 75 +++++++++++++++++++++++ 6 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 crates/engine/tests/clipping_semantics.rs diff --git a/crates/engine/src/clip_group.rs b/crates/engine/src/clip_group.rs index 4e356efce9..31f62696a3 100644 --- a/crates/engine/src/clip_group.rs +++ b/crates/engine/src/clip_group.rs @@ -42,6 +42,11 @@ fn group_of(doc: &Document, id: LayerId) -> Vec { if group.len() > 1 { group } else { Vec::new() } } +/// The base `id` clips to (itself when it is a base), when it is in a clip group. +pub(crate) fn base_of(doc: &Document, id: LayerId) -> Option { + group_of(doc, id).first().copied() +} + /// The layers clipped to `id`: the clipped run directly above it. Empty when `id` is itself /// clipped (it is not a base) or nothing is clipped to it. fn clipped_to(doc: &Document, id: LayerId) -> Vec { @@ -51,6 +56,14 @@ fn clipped_to(doc: &Document, id: LayerId) -> Vec { } } +/// The clipped layers directly above `id` among its siblings (Release Clipping Mask releases them +/// with it). +pub(crate) fn clipped_above(doc: &Document, id: LayerId) -> Vec { + let Some(path) = doc.path_of(id) else { return Vec::new() }; + let Some((list, idx)) = siblings(doc, &path) else { return Vec::new() }; + list.iter().skip(idx.saturating_add(1)).take_while(|l| l.clipped).map(|l| l.id).collect() +} + /// The clip groups of layers about to move or be deleted, read before they leave their slots. #[derive(Default)] pub(crate) struct Before { diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index a05c5b060c..edb60b06dc 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -253,7 +253,10 @@ pub(crate) fn parse_hex(s: &str) -> Option<[f32; 4]> { } /// Layer › Create / Release Clipping Mask. With a `layer` param, just that layer. Otherwise, as in /// Photoshop, the selection (#1248): Create clips every selected layer except the lowest, which -/// becomes the base; Release releases every selected layer. One history step either way. +/// becomes the base; Release releases every selected layer. Releasing a clipped layer also +/// releases the clipped layers above it in its group, as Adobe documents ("removes the selected +/// layer and any layers above it from the clipping mask"), so none is left clipped to a layer +/// that is no longer clipped. One history step either way. fn set_clipped(s: &mut Session, p: &Value, clip: bool) -> Result { let label = if clip { "Create Clipping Mask" } else { "Release Clipping Mask" }; let ids: Vec = if p.get("layer").is_some() { @@ -267,6 +270,20 @@ fn set_clipped(s: &mut Session, p: &Value, clip: bool) -> Result { (false, all) => all.to_vec(), } }; + let ids = if clip { + ids + } else { + let doc = &s.active().ok_or(EngineError::NoDocument)?.doc; + let mut all = ids.clone(); + for id in ids.iter().filter(|id| doc.layer(**id).is_some_and(|l| l.clipped)) { + for above in crate::clip_group::clipped_above(doc, *id) { + if !all.contains(&above) { + all.push(above); + } + } + } + all + }; s.edit(label, |doc, _| { for id in &ids { doc.layer_mut(*id).ok_or(EngineError::NoLayer(*id))?.clipped = clip; diff --git a/crates/engine/src/extra_cmds.rs b/crates/engine/src/extra_cmds.rs index 3a27c99be3..65d181b955 100644 --- a/crates/engine/src/extra_cmds.rs +++ b/crates/engine/src/extra_cmds.rs @@ -522,11 +522,16 @@ fn delete_where(s: &mut Session, label: &str, pred: fn(&Layer) -> bool) -> Resul n } let backup = doc.layers.clone(); + // The clip groups of the doomed layers, read before they go (clip_group rule 3: layers + // clipped to a deleted base are released). + let doomed: Vec = doc.walk().into_iter().filter(|(_, _, l)| pred(l)).map(|(_, _, l)| l.id).collect(); + let before = crate::clip_group::Before::read(doc, &doomed); let n = rec(&mut doc.layers, pred); if doc.layers.is_empty() { doc.layers = backup; return Err(EngineError::Other("a document must keep at least one layer".into())); } + crate::clip_group::settle(doc, &[], &before, false); if active.is_none_or(|id| doc.layer(id).is_none()) { *active = doc.top_layer(); } diff --git a/crates/engine/src/layer_multi_cmds.rs b/crates/engine/src/layer_multi_cmds.rs index 9a79a72050..bfbd036ec3 100644 --- a/crates/engine/src/layer_multi_cmds.rs +++ b/crates/engine/src/layer_multi_cmds.rs @@ -1312,6 +1312,12 @@ fn merge_layers(s: &mut Session) -> Result { let fmt = PixelFormat::new(fmt.mode, fmt.sample, true); let mut merged = Layer::raster(top.name.clone(), fmt); merged.locks = top.locks; + merged.label = top.label; + // Clipped layers of one chain merge into a layer still clipped to their base. Clipped + // layers of different chains don't: the result, at the top one's slot, would clip the + // lower chain's pixels to the upper chain's base. + let base = crate::clip_group::base_of(doc, top_id); + merged.clipped = base.is_some() && solo.layers.iter().all(|l| l.clipped && crate::clip_group::base_of(doc, l.id) == base); *crate::pixels_mut(&mut merged)? = crate::pixels::composite_layers_uncropped(&solo, fmt); let mid = merged.id; // Hidden selected layers are discarded, as in Photoshop. diff --git a/crates/engine/src/pixels.rs b/crates/engine/src/pixels.rs index e0cea4f39e..b046de03bd 100644 --- a/crates/engine/src/pixels.rs +++ b/crates/engine/src/pixels.rs @@ -415,5 +415,10 @@ pub fn merge_down(doc: &Document, lower: &Layer, upper: &Layer) -> Layer { // Merge Down replaces the lower layer, so keep its editing restrictions. // In particular, a merged Background must remain opaque and position-locked. merged.locks = lower.locks; + // And its place: two layers clipped in one chain merge into a layer still clipped to the base, + // with the lower layer's colour label and link. + merged.clipped = lower.clipped && upper.clipped; + merged.label = lower.label; + merged.link_group = lower.link_group; merged } diff --git a/crates/engine/tests/clipping_semantics.rs b/crates/engine/tests/clipping_semantics.rs new file mode 100644 index 0000000000..1a4aa562a7 --- /dev/null +++ b/crates/engine/tests/clipping_semantics.rs @@ -0,0 +1,75 @@ +//! Clip groups through Release Clipping Mask, Merge Down, Merge Layers and Delete Hidden Layers, +//! as in Photoshop (layers audit L2). The stack: Background, B, C1*, C2* (C1 and C2 clipped to B). + +use photocraft_engine::Session; +use serde_json::json; + +fn stack() -> (Session, [u64; 3]) { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 32, "height": 32, "background": "white"})).unwrap(); + let mut ids = [0u64; 3]; + for (i, color) in ["#ff0000", "#00ff00", "#0000ff"].iter().enumerate() { + ids[i] = s.execute("layer.new.layer", json!({})).unwrap()["layer"].as_u64().unwrap(); + s.execute("select.rect", json!({"x": 4 * i, "y": 4, "width": 12, "height": 12})).unwrap(); + s.execute("edit.fill", json!({"color": color})).unwrap(); + s.execute("select.deselect", json!({})).unwrap(); + } + for c in &ids[1..] { + s.execute("layer.setProps", json!({"layer": c, "clipped": true})).unwrap(); + } + (s, ids) +} + +fn clipped(s: &Session, id: u64) -> bool { + s.active().unwrap().doc.layer(photocraft_doc::LayerId(id)).unwrap().clipped +} + +#[test] +fn releasing_a_middle_layer_releases_the_ones_above_it() { + let (mut s, [_, c1, c2]) = stack(); + let r = s.execute("layer.releaseClippingMask", json!({"layer": c1})).unwrap(); + assert!(!clipped(&s, c1) && !clipped(&s, c2), "C2 isn't left clipped to the released C1: {r}"); + // Releasing the top one leaves the one below it alone. + let (mut s, [_, c1, c2]) = stack(); + s.execute("layer.releaseClippingMask", json!({"layer": c2})).unwrap(); + assert!(clipped(&s, c1) && !clipped(&s, c2)); +} + +#[test] +fn merging_clipped_layers_keeps_the_result_clipped() { + let (mut s, [_, c1, c2]) = stack(); + s.execute("layer.select", json!({"layer": c2})).unwrap(); + let r = s.execute("layer.mergeDown", json!({})).unwrap(); + let merged = r["layer"].as_u64().unwrap_or(c1); + assert!(clipped(&s, merged), "C2 merged down into C1 stays clipped to B"); + let (mut s, [_, c1, c2]) = stack(); + s.execute("layer.select", json!({"layer": c1})).unwrap(); + s.execute("layer.select", json!({"layer": c2, "mode": "add"})).unwrap(); + let r = s.execute("layer.mergeLayers", json!({})).unwrap(); + assert!(clipped(&s, r["layer"].as_u64().unwrap()), "C1 + C2 merged stay clipped"); + // Clipped layers of two chains (Background, B, C1*, E, D*: C1 clipped to B, D to E) merge + // into an ordinary layer: kept clipped at D's slot, C1's pixels would clip to E. + let (mut s, [_, c1, e]) = stack(); + s.execute("layer.setProps", json!({"layer": e, "clipped": false})).unwrap(); + let d = s.execute("layer.new.layer", json!({})).unwrap()["layer"].as_u64().unwrap(); + s.execute("edit.fill", json!({"color": "#808080"})).unwrap(); + s.execute("layer.setProps", json!({"layer": d, "clipped": true})).unwrap(); + s.execute("layer.select", json!({"layer": c1})).unwrap(); + s.execute("layer.select", json!({"layer": d, "mode": "add"})).unwrap(); + let r = s.execute("layer.mergeLayers", json!({})).unwrap(); + assert!(!clipped(&s, r["layer"].as_u64().unwrap()), "C1 + D come from two clip chains"); + // Merging the base with its clipped layers gives an ordinary layer. + let (mut s, [b, _, c2]) = stack(); + s.execute("layer.select", json!({"layer": b})).unwrap(); + s.execute("layer.select", json!({"layer": c2, "mode": "add"})).unwrap(); + let r = s.execute("layer.mergeLayers", json!({})).unwrap(); + assert!(!clipped(&s, r["layer"].as_u64().unwrap())); +} + +#[test] +fn deleting_a_hidden_base_releases_its_clipped_layers() { + let (mut s, [b, c1, c2]) = stack(); + s.execute("layer.setProps", json!({"layer": b, "visible": false})).unwrap(); + s.execute("layer.delete.hiddenLayers", json!({})).unwrap(); + assert!(!clipped(&s, c1) && !clipped(&s, c2), "they don't clip to the layer below the deleted base"); +} From c11ef015b7640b4dad43bf775373fd345902e382 Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Thu, 15 Oct 2026 13:10:28 -0700 Subject: [PATCH 5/5] Paste Layer Style carries Blend If and channels; align to an active selection with several layers (#5881) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Paste Layer Style carries Blend If and channels; align to an active selection with several layers From the layers audit (L2): - Copy / Paste Layer Style copied the effects, blend mode, Fill and advanced blending but dropped Blend If and the channel restrictions; Photoshop's layer style carries all of its Blending Options. Across colour modes only Blend If's Gray range carries over and the channels reset (RGB channel ranges mean nothing in CMYK). The copied style is now a named struct (CopiedStyle) instead of a tuple. - With an active selection, aligning (Layer › Align, "auto") aligned two or more layers to their own bounds; Photoshop aligns them to the selection (Layer › Align Layers to Selection), as it already did for one layer. * Paste Layer Style keeps Blend If normalised across modes; Align docs say it follows an active selection for any count; undo, group and 16-bit tests --- crates/engine/src/extra_cmds.rs | 36 ++++++++-- crates/engine/src/layer_multi_cmds.rs | 10 +-- crates/engine/src/lib.rs | 17 ++++- crates/engine/tests/style_and_align.rs | 99 ++++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 crates/engine/tests/style_and_align.rs diff --git a/crates/engine/src/extra_cmds.rs b/crates/engine/src/extra_cmds.rs index 65d181b955..91c9a9bc1a 100644 --- a/crates/engine/src/extra_cmds.rs +++ b/crates/engine/src/extra_cmds.rs @@ -883,8 +883,17 @@ pub fn specs() -> Vec { ), spec!("layer.layerStyle.copyLayerStyle", "Copy Layer Style", &["Layer", "Layer Style"], None, r##"{"layer":id?}"##, has_layer, |s, p| { let id = layer_param(s, p)?; - let l = s.active().and_then(|d| d.doc.layer(id)).ok_or(EngineError::NoLayer(id))?; - let style = (l.effects.clone(), l.blend, l.fill_opacity, l.advanced); + let d = s.active().ok_or(EngineError::NoDocument)?; + let l = d.doc.layer(id).ok_or(EngineError::NoLayer(id))?; + let style = crate::CopiedStyle { + effects: l.effects.clone(), + blend: l.blend, + fill: l.fill_opacity, + advanced: l.advanced, + blend_if: l.blend_if.clone(), + excluded_channels: l.excluded_channels, + mode: d.doc.mode, + }; s.style_clipboard = Some(style); Ok(Value::Null) }), @@ -900,17 +909,32 @@ pub fn specs() -> Vec { }, |s, p| { let ids = style_targets(s, p, |_| true)?; - let (mut fx, blend, fill, advanced) = s.style_clipboard.clone().ok_or(EngineError::Other("no layer style has been copied".into()))?; + let style = s.style_clipboard.clone().ok_or(EngineError::Other("no layer style has been copied".into()))?; + let mut fx = style.effects; s.edit("Paste Layer Style", |doc, _| { // A style copied from a document in another mode takes this one's colours. let mode = doc.mode; fx.items = std::mem::take(&mut fx.items).into_iter().map(|e| e.in_mode(mode)).collect(); + // Blend If's Gray range carries over to any mode; its channel ranges and the + // channel restrictions only to the mode they were set in. + let same_mode = style.mode == mode; + let blend_if = if same_mode { + style.blend_if.clone() + } else { + // Through `set`, which keeps the normalised form (no trailing full + // entries), so a full Gray range pastes as the default. + let mut gray = photocraft_doc::BlendIf::default(); + gray.set(0, style.blend_if.get(0)); + gray + }; for id in ids { let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?; l.effects = fx.clone(); - l.blend = blend; - l.fill_opacity = fill; - l.advanced = advanced; + l.blend = style.blend; + l.fill_opacity = style.fill; + l.advanced = style.advanced; + l.blend_if = blend_if.clone(); + l.excluded_channels = if same_mode { style.excluded_channels } else { 0 }; } Ok(()) })?; diff --git a/crates/engine/src/layer_multi_cmds.rs b/crates/engine/src/layer_multi_cmds.rs index bfbd036ec3..87bb507e4d 100644 --- a/crates/engine/src/layer_multi_cmds.rs +++ b/crates/engine/src/layer_multi_cmds.rs @@ -263,7 +263,8 @@ fn can_align(s: &Session) -> std::result::Result<(), String> { let d = doc_state(s)?; match d.selected_layers().len() { 0 => Err("no layer selected".into()), - // Photoshop aligns a single layer to the canvas (or to an active selection); 2+ align to each other. + // Photoshop aligns to an active selection however many layers are selected; without one, + // a single layer aligns to the canvas and 2+ align to each other. _ => Ok(()), } } @@ -961,8 +962,9 @@ fn align(s: &mut Session, p: &Value, kind: &str) -> Result { "canvas" => doc.bounds(), "selection" => doc.selection.as_ref().map(|m| m.content_bounds()).ok_or_else(|| EngineError::Other("there is no selection to align to".into()))?, "auto" | "layers" => match (&doc.selection, sel.len()) { - // Photoshop: one layer plus an active selection aligns to the selection bounds. - (Some(m), 1) if to == "auto" => m.content_bounds(), + // Photoshop: with an active selection, the layers align to its bounds (Layer › Align + // Layers to Selection), however many are selected. + (Some(m), _) if to == "auto" && !m.content_bounds().is_empty() => m.content_bounds(), // One layer with no selection aligns to the canvas (otherwise it would align to itself). (None, 1) if to == "auto" => doc.bounds(), _ => items.iter().fold(Rect::EMPTY, |a, (_, b)| a.union(b)), @@ -1437,7 +1439,7 @@ pub fn specs() -> Vec { CommandSpec { id: $id, label: $label, menu: $menu, shortcut: $sc, params: $params, enabled: $enabled, journal: true, run: $run } }; } - const ALIGN: &str = r##"{"to":"auto|layers|selection|canvas"="auto"} (auto: the selection bounds with one layer and an active selection, else the selected layers' bounds)"##; + const ALIGN: &str = r##"{"to":"auto|layers|selection|canvas"="auto"} (auto: the active selection's bounds when there is one, however many layers are selected; else one layer aligns to the canvas and several to their combined bounds)"##; vec![ spec!("select.allLayers", "All Layers", &["Select"], Some("Cmd+Alt+A"), "{}", has_doc, |s, _| select_all_layers(s)), spec!("select.deselectLayers", "Deselect Layers", &["Select"], None, "{}", has_doc, |s, _| { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index d2aecf37db..2e0684de12 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -406,6 +406,21 @@ impl Default for ToolState { } } +/// Layer › Layer Style › Copy Layer Style: the effects and the Blending Options, as Photoshop +/// copies them (Paste Layer Style puts them all on each target). +#[derive(Clone, Debug)] +pub struct CopiedStyle { + pub effects: photocraft_doc::Effects, + pub blend: photocraft_color::BlendMode, + pub fill: f32, + pub advanced: photocraft_doc::AdvancedBlending, + pub blend_if: photocraft_doc::BlendIf, + pub excluded_channels: u32, + /// The colour mode it was copied in: Blend If's channel ranges and the channel restrictions + /// only carry over to a document in the same mode. + pub mode: photocraft_color::ColorMode, +} + #[derive(Default)] pub struct Session { docs: Vec, @@ -419,7 +434,7 @@ pub struct Session { pub clipboard: Option, /// Layer › Layer Style › Copy Layer Style: effects, blend mode, fill opacity and the Advanced /// Blending switches. - pub style_clipboard: Option<(photocraft_doc::Effects, photocraft_color::BlendMode, f32, photocraft_doc::AdvancedBlending)>, + pub style_clipboard: Option, /// Shape path context menu: copied vector fill and stroke styles. pub path_fill_clipboard: Option, pub path_stroke_clipboard: Option, diff --git a/crates/engine/tests/style_and_align.rs b/crates/engine/tests/style_and_align.rs new file mode 100644 index 0000000000..ad17330507 --- /dev/null +++ b/crates/engine/tests/style_and_align.rs @@ -0,0 +1,99 @@ +//! Copy / Paste Layer Style carries the whole style, Blending Options included (Blend If and the +//! channel restrictions); align with an active selection aligns to it however many layers are +//! selected (layers audit L2). + +use photocraft_engine::Session; +use serde_json::json; + +fn layer(s: &Session, id: u64) -> photocraft_doc::Layer { + s.active().unwrap().doc.layer(photocraft_doc::LayerId(id)).unwrap().clone() +} + +fn new_doc(s: &mut Session, mode: &str) -> (u64, u64) { + s.execute("file.new", json!({"width": 64, "height": 64, "mode": mode, "background": "white"})).unwrap(); + let a = s.execute("layer.new.layer", json!({})).unwrap()["layer"].as_u64().unwrap(); + let b = s.execute("layer.new.layer", json!({})).unwrap()["layer"].as_u64().unwrap(); + (a, b) +} + +#[test] +fn paste_layer_style_carries_blend_if_and_channels() { + let mut s = Session::new(); + let (a, b) = new_doc(&mut s, "rgb"); + s.execute( + "layer.layerStyle.blendingOptions", + json!({"layer": a, "blendIf": [{"channel": "gray", "underlying": [0, 128]}, {"channel": "red", "thisLayer": [10, 200]}]}), + ) + .unwrap(); + s.execute("layer.setProps", json!({"layer": a, "channels": [true, false, true]})).unwrap(); + s.execute("layer.layerStyle.copyLayerStyle", json!({"layer": a})).unwrap(); + s.execute("layer.layerStyle.pasteLayerStyle", json!({"layer": b})).unwrap(); + let (la, lb) = (layer(&s, a), layer(&s, b)); + assert_eq!(lb.blend_if, la.blend_if, "Blend If, Gray and channel ranges"); + assert_eq!(lb.excluded_channels, la.excluded_channels, "the unchecked G channel"); + // Into a CMYK document: only the Gray range carries over; channels reset to all. + let (c, _) = new_doc(&mut s, "cmyk"); + s.execute("layer.layerStyle.pasteLayerStyle", json!({"layer": c})).unwrap(); + let lc = layer(&s, c); + assert_eq!(lc.blend_if.get(0), la.blend_if.get(0)); + assert_eq!(lc.blend_if.ranges.len(), 1, "no RGB channel range on CMYK"); + assert_eq!(lc.excluded_channels, 0); +} + +#[test] +fn paste_layer_style_into_another_mode_keeps_blend_if_normalised() { + // A full Gray range and only a Red range: in CMYK nothing is left, so it is the default + // (which compares equal, so the paste isn't seen as a Blend If change). + let mut s = Session::new(); + let (a, _) = new_doc(&mut s, "rgb"); + s.execute("layer.layerStyle.blendingOptions", json!({"layer": a, "blendIf": [{"channel": "red", "thisLayer": [10, 200]}]})).unwrap(); + s.execute("layer.layerStyle.copyLayerStyle", json!({"layer": a})).unwrap(); + let (c, _) = new_doc(&mut s, "cmyk"); + s.execute("layer.layerStyle.pasteLayerStyle", json!({"layer": c})).unwrap(); + assert_eq!(layer(&s, c).blend_if, photocraft_doc::BlendIf::default()); +} + +#[test] +fn paste_layer_style_on_several_layers_is_one_undo_step() { + // 16-bit, onto a pixel layer and a group: one History step, and one Undo restores both. + let mut s = Session::new(); + s.execute("file.new", json!({"width": 64, "height": 64, "depth": 16, "background": "white"})).unwrap(); + let a = s.execute("layer.new.layer", json!({})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.layerStyle.blendingOptions", json!({"layer": a, "blendIf": [{"channel": "gray", "underlying": [0, 128]}]})).unwrap(); + s.execute("layer.setProps", json!({"layer": a, "fill": 0.5, "blend": "multiply"})).unwrap(); + s.execute("layer.layerStyle.copyLayerStyle", json!({"layer": a})).unwrap(); + let b = s.execute("layer.new.layer", json!({})).unwrap()["layer"].as_u64().unwrap(); + let g = s.execute("layer.new.group", json!({})).unwrap()["layer"].as_u64().unwrap(); + let before = (layer(&s, b), layer(&s, g)); + s.execute("layer.select", json!({"layer": b})).unwrap(); + s.execute("layer.select", json!({"layer": g, "mode": "add"})).unwrap(); + s.execute("layer.layerStyle.pasteLayerStyle", json!({})).unwrap(); + let la = layer(&s, a); + for id in [b, g] { + let l = layer(&s, id); + assert_eq!((l.blend_if.clone(), l.fill_opacity, l.blend), (la.blend_if.clone(), la.fill_opacity, la.blend), "layer {id}"); + } + s.execute("edit.undo", json!({})).unwrap(); + let after = (layer(&s, b), layer(&s, g)); + assert_eq!((after.0.blend_if, after.0.blend, after.1.blend_if, after.1.blend), (before.0.blend_if, before.0.blend, before.1.blend_if, before.1.blend)); + assert_eq!((after.0.fill_opacity, after.1.fill_opacity), (before.0.fill_opacity, before.1.fill_opacity)); +} + +#[test] +fn several_layers_align_to_an_active_selection() { + let mut s = Session::new(); + let (a, b) = new_doc(&mut s, "rgb"); + for (id, x) in [(a, 4), (b, 30)] { + s.execute("layer.select", json!({"layer": id})).unwrap(); + s.execute("select.rect", json!({"x": x, "y": 10, "width": 8, "height": 8})).unwrap(); + s.execute("edit.fill", json!({"color": "#ff0000"})).unwrap(); + } + s.execute("select.rect", json!({"x": 40, "y": 40, "width": 20, "height": 20})).unwrap(); + s.execute("layer.select", json!({"layer": a})).unwrap(); + s.execute("layer.select", json!({"layer": b, "mode": "add"})).unwrap(); + s.execute("layer.align.leftEdges", json!({})).unwrap(); + for id in [a, b] { + let x0 = layer(&s, id).surface().unwrap().content_bounds().x0; + assert_eq!(x0, 40, "layer {id} aligned to the selection's left edge"); + } +}