mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-14 11:16:53 -04:00
Fix Transform Selection hang on far-off-canvas transforms (Rule 9)
warp_gray cleared the old + warped region as one dense buffer spanning their union, so moving a selection/mask/channel far away (e.g. dx=1e5) allocated a ~1e5x1e5 buffer and hung the app. Write the two regions separately instead — Surface is tile-sparse, so a distant region costs only its own tiles. Found by the panic_hunt fuzz (now also catches hangs via a per-command thread timeout). Regression test covers a far transform. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
a196adfe93
commit
b70fbd4ff6
3 files changed
+58
-27
No files matched your search
@@ -376,3 +376,23 @@ mod tests {
|
||||
assert!(!s.is_enabled("select.transformSelection"));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod hang_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn transform_selection_with_extreme_params_terminates() {
|
||||
// Regression (panic_hunt): a huge off-canvas transform used to allocate a buffer spanning
|
||||
// the old + new bounds and hang. It must return quickly without crashing.
|
||||
let mut s = Session::new();
|
||||
s.execute("file.new", json!({"width": 24, "height": 16})).unwrap();
|
||||
s.execute("select.rect", json!({"x": 1, "y": 1, "width": 6, "height": 5})).unwrap();
|
||||
// dx/dy move the selection far off-canvas; scaleX enlarges it — the old code allocated a
|
||||
// buffer spanning the original + moved bounds (~1e5 × 1e5) and hung.
|
||||
let r = s.execute("select.transformSelection", json!({"dx": 1e5, "dy": -1e5, "scaleX": 300.0}));
|
||||
assert!(r.is_ok(), "far transform returns without hanging: {r:?}");
|
||||
// The document is still usable.
|
||||
assert!(s.execute("select.all", json!({})).is_ok());
|
||||
}
|
||||
}
|
||||
@@ -47,10 +47,12 @@ pub(crate) fn warp_gray(s: &Surface, h: &Homography, interp: Interp) -> Surface
|
||||
let v = s.read_region(src);
|
||||
tmp.write_region(src, &v.iter().flat_map(|g| [*g, 1.0]).collect::<Vec<f32>>());
|
||||
let w = warp_surface(&tmp, src, h, interp);
|
||||
// Everything the old content covered becomes default, then the warped content composites over.
|
||||
let cover = src.union(&w.content_bounds());
|
||||
let old: Vec<f32> = vec![default; cover.width() as usize * cover.height() as usize];
|
||||
out.write_region(cover, &old);
|
||||
// Clear the old content region, then composite the warped content over it. These are written as
|
||||
// two sparse regions rather than one dense `src ∪ warped` block: a transform that moves the
|
||||
// content far away (e.g. a huge translation) would otherwise allocate a buffer spanning both and
|
||||
// hang/OOM. `Surface` is tile-sparse, so distant regions cost only their own tiles.
|
||||
let old: Vec<f32> = vec![default; src.width() as usize * src.height() as usize];
|
||||
out.write_region(src, &old);
|
||||
let b = w.content_bounds();
|
||||
if !b.is_empty() {
|
||||
let px = w.read_region(b);
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
//! Fuzz every command with adversarial params; a command must return `Err`, never panic.
|
||||
//! (Rule 9 in AGENTS.md.) Params here keep the canvas small on purpose — validating absurd
|
||||
//! dimensions is a separate concern; this hunts missing-param / empty-state / bad-index panics.
|
||||
//! Fuzz every command with adversarial params; a command must return `Err`, never panic or hang.
|
||||
//! (Rule 9 in AGENTS.md.) Each command runs on its own thread with a timeout, so one bad command
|
||||
//! can't wedge the run — panics are caught and hangs are reported. Opt-in (slow):
|
||||
//! `cargo test -p photocraft-engine --test panic_hunt -- --ignored`.
|
||||
use photocraft_engine::{command_specs, Session};
|
||||
use serde_json::{json, Value};
|
||||
use std::panic::{catch_unwind, AssertUnwindSafe};
|
||||
use std::sync::mpsc;
|
||||
use std::time::Duration;
|
||||
|
||||
fn fresh() -> Session {
|
||||
let mut s = Session::new();
|
||||
@@ -13,36 +16,42 @@ fn fresh() -> Session {
|
||||
s
|
||||
}
|
||||
|
||||
// Slow (fresh session per command × params, in debug). Opt-in: `cargo test -p photocraft-engine
|
||||
// --test panic_hunt -- --ignored`. Fast targeted regressions live beside the fixed commands.
|
||||
/// Run `id` with `p` on a fresh session, on a worker thread. Returns `Err` with a reason on panic
|
||||
/// or timeout, `Ok` otherwise (whether the command returned Ok or a graceful Err).
|
||||
fn run_guarded(id: &str, p: &Value) -> Result<(), String> {
|
||||
let (tx, rx) = mpsc::channel();
|
||||
let (id, p) = (id.to_string(), p.clone());
|
||||
std::thread::spawn(move || {
|
||||
let r = catch_unwind(AssertUnwindSafe(|| {
|
||||
let mut s = fresh();
|
||||
let _ = s.execute(&id, p);
|
||||
}));
|
||||
let _ = tx.send(r.is_ok());
|
||||
});
|
||||
match rx.recv_timeout(Duration::from_secs(4)) {
|
||||
Ok(true) => Ok(()),
|
||||
Ok(false) => Err("panicked".into()),
|
||||
Err(_) => Err("hung (>4s)".into()),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "slow full-registry fuzz; run with --ignored"]
|
||||
fn no_command_panics_on_adversarial_params() {
|
||||
// None of these resize the canvas, so the fuzz stays fast.
|
||||
fn no_command_panics_or_hangs_on_adversarial_params() {
|
||||
let params: Vec<Value> = vec![
|
||||
json!({}),
|
||||
json!({"x": -9, "y": -9, "radius": 0, "amount": 0, "angle": 0, "opacity": 0, "scale": 0, "levels": 0, "tolerance": 0, "gamma": 0, "layer": 999999, "channel": 999, "index": 999999}),
|
||||
json!({"x": 1e6, "y": -1e6, "radius": 500, "amount": 1000, "angle": 1e6, "opacity": -100, "scale": -5, "gamma": -1}),
|
||||
json!({"x": -9, "y": -9, "radius": 0, "amount": 0, "angle": 0, "opacity": 0, "scale": 0, "levels": 0, "tolerance": 0, "gamma": 0, "layer": 999999, "channel": 999, "index": 999999, "count": 0}),
|
||||
json!({"x": 1e5, "y": -1e5, "radius": 300, "amount": 300, "angle": 1e5, "opacity": -100, "scale": -5, "gamma": -1}),
|
||||
json!({"points": [], "from": [0, 0], "to": [0, 0], "stops": [], "matrix": [0, 0, 0, 0, 0, 0], "colors": [], "name": "", "mode": "", "style": ""}),
|
||||
];
|
||||
let prev = std::panic::take_hook();
|
||||
std::panic::set_hook(Box::new(|_| {}));
|
||||
let mut panicked: Vec<String> = Vec::new();
|
||||
let mut bad: Vec<String> = Vec::new();
|
||||
for spec in command_specs() {
|
||||
for p in ¶ms {
|
||||
let id = spec.id.to_string();
|
||||
let pc = p.clone();
|
||||
let crashed = catch_unwind(AssertUnwindSafe(|| {
|
||||
let mut s = fresh();
|
||||
let _ = s.execute(&id, pc);
|
||||
}))
|
||||
.is_err();
|
||||
if crashed {
|
||||
panicked.push(format!("{} <- {}", spec.id, p));
|
||||
if let Err(why) = run_guarded(spec.id, p) {
|
||||
bad.push(format!("{} [{why}] <- {p}", spec.id));
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
std::panic::set_hook(prev);
|
||||
assert!(panicked.is_empty(), "{} commands panicked:\n{}", panicked.len(), panicked.join("\n"));
|
||||
assert!(bad.is_empty(), "{} commands panicked or hung:\n{}", bad.len(), bad.join("\n"));
|
||||
}
|
||||
Reference in new issue
Block a user