From 4218d07f402b69bf77c53153aec3411bd8f77f6e Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 13:26:26 -0400 Subject: [PATCH] Never crash: the engine is panic-free (Rule 9) (#54) Every unwrap/expect/unreachable in non-test engine code now returns an EngineError (or a predicate's Err string) or uses a real fallback: - enabled predicates read the active layer through active_layer_of() instead of unwrap() after has_layer(); - new raster layers get their pixels through pixels_mut()? instead of surface_mut().expect("raster"); - layer paths use split_last().ok_or(..)? (Merge Down, Arrange, Ungroup, Release Clipping), Auto-Align/Auto-Blend/Photomerge carry placements instead of re-unwrapping them, and "next free name" searches are bounded; - web export builds its options map directly, quantize_rows has a wasm tail, parse_script rejects non-array/object JSON, the shape-layer edit returns an error instead of unreachable!, and the gray composite profile falls back to sRGB. The engine denies unwrap/expect/panic/unimplemented/todo/unreachable. Co-authored-by: Claude Opus 5.5 --- crates/engine/src/adjust_cmds.rs | 2 +- crates/engine/src/align_cmds.rs | 18 ++++++------- crates/engine/src/artboard_cmds.rs | 3 ++- crates/engine/src/automate_cmds.rs | 2 +- crates/engine/src/channel_cmds.rs | 36 ++++++++++++-------------- crates/engine/src/color_cmds.rs | 3 ++- crates/engine/src/commands.rs | 24 ++++++++--------- crates/engine/src/comps_cmds.rs | 3 ++- crates/engine/src/edit_cmds.rs | 6 ++--- crates/engine/src/extra_cmds.rs | 2 +- crates/engine/src/frame_cmds.rs | 6 ++--- crates/engine/src/layer_menu_cmds.rs | 2 +- crates/engine/src/layer_multi_cmds.rs | 6 ++--- crates/engine/src/lib.rs | 14 ++++++++++ crates/engine/src/mode_cmds.rs | 8 +++--- crates/engine/src/multichannel_cmds.rs | 13 +++++----- crates/engine/src/photo_cmds.rs | 27 +++++++------------ crates/engine/src/presets/shapes.rs | 12 ++++++--- crates/engine/src/variables_cmds.rs | 16 +++++++----- crates/engine/src/vector_cmds.rs | 4 ++- crates/engine/src/web_cmds.rs | 19 +++++++++++--- 21 files changed, 128 insertions(+), 98 deletions(-) diff --git a/crates/engine/src/adjust_cmds.rs b/crates/engine/src/adjust_cmds.rs index 03d37316..08780c06 100644 --- a/crates/engine/src/adjust_cmds.rs +++ b/crates/engine/src/adjust_cmds.rs @@ -307,7 +307,7 @@ fn hdr_toning(s: &mut Session, p: &Value) -> Result { let data: Vec = px.iter().flat_map(|q| photocraft_raster::from_rgba(&fmt, *q)).collect(); let mut bg = Layer::raster("Background", fmt); bg.locks.transparency = true; - bg.surface_mut().expect("raster").write_region(doc.bounds(), &data); + crate::pixels_mut(&mut bg)?.write_region(doc.bounds(), &data); *active = Some(bg.id); doc.layers = vec![bg]; doc.selection = None; diff --git a/crates/engine/src/align_cmds.rs b/crates/engine/src/align_cmds.rs index fc3d0c43..76a363cd 100644 --- a/crates/engine/src/align_cmds.rs +++ b/crates/engine/src/align_cmds.rs @@ -80,18 +80,18 @@ fn auto_align(s: &mut Session, p: &Value) -> Result { let warped: Vec<(LayerId, Surface, Option)> = moves .iter() .map(|&i| { - let pl = al.placements[i].as_ref().expect("placed"); - let l = doc.layer(ids[i]).expect("exists"); - let surf = l.surface().expect("raster"); + let pl = al.placements[i].as_ref().ok_or_else(|| EngineError::Other("Auto-Align lost a layer's placement".into()))?; + let l = doc.layer(ids[i]).ok_or(EngineError::NoLayer(ids[i]))?; + let surf = l.surface().ok_or(EngineError::NoLayer(ids[i]))?; let b = surf.content_bounds(); let px = if b.is_empty() { surf.clone() } else { warp_placed(surf, b, (0.0, 0.0), pl, (0.0, 0.0), interp) }; let mask = l.mask.as_ref().map(|m| { let mb = m.surface.content_bounds(); if mb.is_empty() { m.surface.clone() } else { warp_placed(&m.surface, mb, (0.0, 0.0), pl, (0.0, 0.0), Interp::Bilinear) } }); - (ids[i], px, mask) + Ok((ids[i], px, mask)) }) - .collect(); + .collect::>()?; s.edit("Auto-Align Layers", |doc, _| { for (id, px, mask) in &warped { let l = doc.layer_mut(*id).ok_or(EngineError::NoLayer(*id))?; @@ -113,9 +113,9 @@ fn auto_align(s: &mut Session, p: &Value) -> Result { }; let aligned: Vec = moves .iter() - .map(|&i| { - let pl = al.placements[i].as_ref().expect("placed"); - json!({"layer": ids[i].0, "model": model, "matches": inliers[i], "matrix": plane_matrix(pl, (0.0, 0.0)).map(|h| h.0.to_vec())}) + .filter_map(|&i| { + let pl = al.placements[i].as_ref()?; + Some(json!({"layer": ids[i].0, "model": model, "matches": inliers[i], "matrix": plane_matrix(pl, (0.0, 0.0)).map(|h| h.0.to_vec())})) }) .collect(); Ok(json!({"reference": reference.0, "layout": al.layout.name(), "focal": al.focal, "rms": al.rms, "aligned": aligned, "failed": failed})) @@ -191,7 +191,7 @@ fn auto_blend(s: &mut Session, p: &Value) -> Result { let blended = seamless.then(|| b.composite.read_region(area)); (weights, blended) }; - let top = *ids.last().expect("two or more"); + let top = *ids.last().ok_or_else(|| EngineError::Other("Auto-Blend needs two or more layers".into()))?; let label = "Auto-Blend Layers"; let new_layer = s.edit(label, |doc, active| { for (id, wt) in ids.iter().zip(&weights) { diff --git a/crates/engine/src/artboard_cmds.rs b/crates/engine/src/artboard_cmds.rs index d78ff685..1ebc4395 100644 --- a/crates/engine/src/artboard_cmds.rs +++ b/crates/engine/src/artboard_cmds.rs @@ -137,7 +137,8 @@ fn fit_canvas(doc: &mut Document) { fn next_artboard_name(doc: &Document) -> String { let names: Vec<&str> = doc.artboards().into_iter().map(|b| b.1).collect(); - (1..).map(|n| format!("Artboard {n}")).find(|n| !names.contains(&n.as_str())).expect("infinite") + // `names.len() + 1` candidates always include a free one. + (1..=names.len() + 1).map(|n| format!("Artboard {n}")).find(|n| !names.contains(&n.as_str())).unwrap_or_else(|| "Artboard".into()) } fn set_artboard(l: &mut Layer, a: Artboard) { diff --git a/crates/engine/src/automate_cmds.rs b/crates/engine/src/automate_cmds.rs index bf370bc3..4d86a007 100644 --- a/crates/engine/src/automate_cmds.rs +++ b/crates/engine/src/automate_cmds.rs @@ -55,7 +55,7 @@ pub fn parse_script(text: &str) -> Result> { Value::Object(o) => { o.get("steps").or_else(|| o.get("action").and_then(|a| a.get("steps").or(Some(a)))).ok_or_else(|| bad(cmd, "script object needs \"steps\""))? } - _ => unreachable!(), + _ => return Err(bad(cmd, "script JSON must be an array of steps or an object")), }; return parse_steps(steps, cmd); } diff --git a/crates/engine/src/channel_cmds.rs b/crates/engine/src/channel_cmds.rs index 30491ca8..965aca8d 100644 --- a/crates/engine/src/channel_cmds.rs +++ b/crates/engine/src/channel_cmds.rs @@ -165,7 +165,8 @@ pub fn channel_format(doc: &Document) -> PixelFormat { /// Next free "Alpha N" name. fn next_alpha_name(doc: &Document, base: &str) -> String { - (1..).map(|n| format!("{base} {n}")).find(|n| !doc.channels.iter().any(|c| &c.name == n)).expect("infinite") + // `channels.len() + 1` candidates always include a free one. + (1..=doc.channels.len() + 1).map(|n| format!("{base} {n}")).find(|n| !doc.channels.iter().any(|c| &c.name == n)).unwrap_or_else(|| base.to_string()) } // --------------------------------------------------------------------------------------------- @@ -339,12 +340,14 @@ fn ref_planes(doc: &Document, layer: Option, active: Option, r /// One grayscale plane (composites reduce to luminosity, as in Calculations). fn gray_plane(doc: &Document, layer: Option, active: Option, r: ChanRef) -> Result> { - let planes = ref_planes(doc, layer, active, r)?; - if planes.len() == 1 { - return Ok(planes.into_iter().next().expect("one plane")); + let mut planes = ref_planes(doc, layer, active, r)?; + if planes.len() == 1 + && let Some(only) = planes.pop() + { + return Ok(only); } let fmt = doc.pixel_format(); - let n = planes[0].len(); + let n = planes.first().map_or(0, Vec::len); let mut px = vec![0.0f32; fmt.channels()]; Ok((0..n) .map(|i| { @@ -596,17 +599,12 @@ fn has_selection(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; if s.active().is_some_and(|d| d.doc.selection.is_some()) { Ok(()) } else { Err("no selection".into()) } } -fn has_layer(s: &Session) -> std::result::Result<(), String> { - let d = s.active().ok_or("no document open")?; - d.active_layer.filter(|id| d.doc.layer(*id).is_some()).map(|_| ()).ok_or_else(|| "no active layer".into()) -} fn has_spot(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; if s.active().is_some_and(|d| d.doc.channels.iter().any(|c| c.spot.is_some())) { Ok(()) } else { Err("the document has no spot channels".into()) } } fn can_split(s: &Session) -> std::result::Result<(), String> { - has_doc(s)?; - let d = s.active().expect("checked"); + let d = s.active().ok_or("no document open")?; if color_count(&d.doc) + d.doc.channels.len() < 2 { Err("only one channel to split".into()) } else { Ok(()) } } fn can_merge(s: &Session) -> std::result::Result<(), String> { @@ -691,8 +689,9 @@ fn load_selection(s: &mut Session, p: &Value) -> Result { return Err(bad(cmd, "missing `channel`")); } // Composite and colour channels load their luminosity / values, as ⌘-clicking them does. - let plane = source(s, p, cmd, true)?.remove(0); - let label = match p.get("channel").and_then(|v| parse_ref(v, &s.active().expect("source checked").doc)) { + let plane = source(s, p, cmd, true)?.into_iter().next().ok_or_else(|| bad(cmd, "the channel has no pixels"))?; + let doc = &s.active().ok_or(EngineError::NoDocument)?.doc; + let label = match p.get("channel").and_then(|v| parse_ref(v, doc)) { Some(ChanRef::Transparency) => "Load Transparency", Some(ChanRef::LayerMask) => "Load Layer Mask", _ => "Load Selection", @@ -910,7 +909,7 @@ fn delete_channel(s: &mut Session, p: &Value) -> Result { let doc = &s.active().ok_or(EngineError::NoDocument)?.doc; let i = match p.get("channel") { Some(_) => alpha_index(doc, p, "channel", cmd)?, - None => match s.active().expect("checked").channel_view.target { + None => match s.active().ok_or(EngineError::NoDocument)?.channel_view.target { ChannelTarget::Alpha(i) => i, _ => return Err(bad(cmd, "missing `channel`")), }, @@ -1050,7 +1049,7 @@ fn set_visible(s: &mut Session, p: &Value) -> Result { let r = p.get("channel").and_then(|v| parse_ref(v, doc)).ok_or_else(|| bad(cmd, "missing or unknown `channel`"))?; let colors = color_count(doc); let n = doc.channels.len(); - let st = s.active_mut().expect("checked"); + let st = s.active_mut().ok_or(EngineError::NoDocument)?; let v = &mut st.channel_view; let visible = |cur: bool| p.get("visible").and_then(Value::as_bool).unwrap_or(!cur); match r { @@ -1151,7 +1150,7 @@ fn merge_spot(s: &mut Session, p: &Value) -> Result { let doc = &s.active().ok_or(EngineError::NoDocument)?.doc; let i = match p.get("channel") { Some(_) => alpha_index(doc, p, "channel", cmd)?, - None => match s.active().expect("checked").channel_view.target { + None => match s.active().ok_or(EngineError::NoDocument)?.channel_view.target { ChannelTarget::Alpha(i) => i, _ => doc.channels.iter().position(|c| c.spot.is_some()).ok_or_else(|| bad(cmd, "no spot channel"))?, }, @@ -1563,9 +1562,8 @@ fn has_apply_target(s: &Session) -> std::result::Result<(), String> { if edits_channel(s) { return Ok(()); } - has_layer(s)?; - let d = s.active().expect("checked"); - let l = d.doc.layer(d.active_layer.expect("checked")).expect("checked"); + let d = s.active().ok_or("no document open")?; + let l = d.active_layer.and_then(|id| d.doc.layer(id)).ok_or("no active layer")?; if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { diff --git a/crates/engine/src/color_cmds.rs b/crates/engine/src/color_cmds.rs index 63605a07..2c473871 100644 --- a/crates/engine/src/color_cmds.rs +++ b/crates/engine/src/color_cmds.rs @@ -408,7 +408,8 @@ pub fn composite_profile(doc: &Document) -> Arc { ColorSpace::Rgb => document_profile(doc), ColorSpace::Gray => { let p = document_profile(doc); - p.gray_as_rgb().map(Arc::new).unwrap_or_else(|| Arc::new(Builtin::SGray.profile().gray_as_rgb().expect("sGray is a TRC profile"))) + // sGray is a TRC profile, so its RGB view always exists; sRGB is the last resort. + p.gray_as_rgb().or_else(|| Builtin::SGray.profile().gray_as_rgb()).map(Arc::new).unwrap_or_else(|| Arc::new(Builtin::Srgb.profile().clone())) } _ => Arc::new(Builtin::Srgb.profile().clone()), } diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 0bb47224..4e26560e 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -39,9 +39,7 @@ fn has_layer(s: &Session) -> std::result::Result<(), String> { d.active_layer.filter(|id| d.doc.layer(*id).is_some()).map(|_| ()).ok_or_else(|| "no active layer".into()) } fn has_pixel_layer(s: &Session) -> std::result::Result<(), String> { - has_layer(s)?; - let d = s.active().unwrap(); - let l = d.doc.layer(d.active_layer.unwrap()).unwrap(); + let l = crate::active_layer_of(s)?; if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is a {} layer, not a pixel layer", l.content.kind_name())) } } /// A pixel layer, or a targeted alpha channel / Quick Mask (adjustments and fills apply to it). @@ -56,9 +54,7 @@ pub(crate) fn has_paintable(s: &Session) -> std::result::Result<(), String> { if crate::channel_cmds::edits_channel(s) { return Ok(()); } - has_layer(s)?; - let d = s.active().unwrap(); - let l = d.doc.layer(d.active_layer.unwrap()).unwrap(); + let l = crate::active_layer_of(s)?; if matches!(l.content, LayerContent::Raster(_)) || l.mask.is_some() { Ok(()) } else { @@ -623,17 +619,17 @@ fn build() -> Vec { let id = layer_param(s, p)?; s.edit("Merge Down", |doc, active| { let path = doc.path_of(id).ok_or(EngineError::NoLayer(id))?; - let idx = *path.last().unwrap(); + let Some((&idx, parent)) = path.split_last() else { return Err(EngineError::NoLayer(id)) }; if idx == 0 { return Err(EngineError::Other("no layer below to merge into".into())); } - let mut below = path.clone(); - *below.last_mut().unwrap() = idx - 1; + let mut below = parent.to_vec(); + below.push(idx - 1); let lower = doc.layer_at(&below).ok_or(EngineError::NoLayer(id))?.clone(); let upper = doc.layer(id).ok_or(EngineError::NoLayer(id))?.clone(); let merged = pixels::merge_down(doc.bounds(), &lower, &upper, doc.pixel_format()); doc.remove(id); - *doc.layer_at_mut(&below).unwrap() = merged; + *doc.layer_at_mut(&below).ok_or(EngineError::NoLayer(lower.id))? = merged; *active = Some(lower.id); Ok(()) })?; @@ -646,7 +642,7 @@ fn build() -> Vec { let data: Vec = buf.px.iter().flat_map(|p| photocraft_raster::from_rgba(&fmt, *p)).collect(); let mut bg = Layer::raster("Background", fmt); bg.locks.transparency = true; - bg.surface_mut().unwrap().write_region(doc.bounds(), &data); + crate::pixels_mut(&mut bg)?.write_region(doc.bounds(), &data); *active = Some(bg.id); doc.layers = vec![bg]; Ok(()) @@ -725,11 +721,13 @@ fn build() -> Vec { g.push(layer); } other => { - let (&last, parent) = path.split_last().expect("non-empty path"); + let (&last, parent) = path.split_last().ok_or(EngineError::NoLayer(target))?; let sib = if parent.is_empty() { &mut doc.layers } else { - doc.layer_at_mut(parent).and_then(|t| t.children_mut()).expect("parent is a group") + doc.layer_at_mut(parent) + .and_then(|t| t.children_mut()) + .ok_or_else(|| EngineError::Other("target's parent is not a group".into()))? }; let at = if other == "below" { last } else { last + 1 }; sib.insert(at.min(sib.len()), layer); diff --git a/crates/engine/src/comps_cmds.rs b/crates/engine/src/comps_cmds.rs index d2e2cf8b..386288c2 100644 --- a/crates/engine/src/comps_cmds.rs +++ b/crates/engine/src/comps_cmds.rs @@ -102,7 +102,8 @@ fn new_comp(s: &mut Session, p: &Value) -> Result { let d = s.active().ok_or(EngineError::NoDocument)?; let name = p.get("name").and_then(Value::as_str).map(str::to_string).unwrap_or_else(|| { let names: Vec<&str> = d.doc.layer_comps.iter().map(|c| c.name.as_str()).collect(); - (1..).map(|n| format!("Layer Comp {n}")).find(|n| !names.contains(&n.as_str())).expect("infinite") + // `names.len() + 1` candidates always include a free one. + (1..=names.len() + 1).map(|n| format!("Layer Comp {n}")).find(|n| !names.contains(&n.as_str())).unwrap_or_else(|| "Layer Comp".into()) }); let comment = p.get("comment").and_then(Value::as_str).unwrap_or_default().to_string(); let (v, pos, a) = (flag(p, "visibility", true), flag(p, "position", true), flag(p, "appearance", true)); diff --git a/crates/engine/src/edit_cmds.rs b/crates/engine/src/edit_cmds.rs index 05eb9634..904f4e45 100644 --- a/crates/engine/src/edit_cmds.rs +++ b/crates/engine/src/edit_cmds.rs @@ -124,7 +124,7 @@ fn paste(s: &mut Session, p: &Value, in_place: bool) -> Result { let surf = if moved.format() == target { moved } else { moved.convert(target) }; let id = s.edit("Paste", |doc, active| { let mut l = Layer::raster(doc.next_layer_name("Layer"), target); - *l.surface_mut().expect("raster") = surf; + *crate::pixels_mut(&mut l)? = surf; let id = doc.insert_above(*active, l); *active = Some(id); doc.selection = None; @@ -161,7 +161,7 @@ fn layer_via(s: &mut Session, cut: bool) -> Result { clear_selected(doc, id)?; } let mut l = Layer::raster(doc.next_layer_name("Layer"), clip.surface.format()); - *l.surface_mut().expect("raster") = clip.surface; + *crate::pixels_mut(&mut l)? = clip.surface; let nid = doc.insert_above(Some(id), l); *active = Some(nid); Ok(nid) @@ -188,7 +188,7 @@ fn merge_visible(s: &mut Session) -> Result { let data: Vec = buf.px.iter().flat_map(|p| photocraft_raster::from_rgba(&fmt, *p)).collect(); let mut merged = Layer::raster(base.name.clone(), fmt); merged.locks = base.locks; - let surf = merged.surface_mut().expect("raster"); + let surf = crate::pixels_mut(&mut merged)?; surf.write_region(doc.bounds(), &data); surf.prune(); let mid = merged.id; diff --git a/crates/engine/src/extra_cmds.rs b/crates/engine/src/extra_cmds.rs index 1a7fefc9..1ee63dd6 100644 --- a/crates/engine/src/extra_cmds.rs +++ b/crates/engine/src/extra_cmds.rs @@ -417,7 +417,7 @@ fn ungroup(s: &mut Session, p: &Value) -> Result { return Err(EngineError::Other("the active layer is not a group".into())); }; let children = group.children.clone(); - let (&idx, parent) = path.split_last().unwrap(); + let (&idx, parent) = path.split_last().ok_or(EngineError::NoLayer(id))?; let sib = if parent.is_empty() { &mut doc.layers } else { doc.layer_at_mut(parent).and_then(Layer::children_mut).ok_or(EngineError::NoLayer(id))? }; sib.remove(idx); let top = children.last().map(|l| l.id); diff --git a/crates/engine/src/frame_cmds.rs b/crates/engine/src/frame_cmds.rs index 2b6606e8..f8eff8b3 100644 --- a/crates/engine/src/frame_cmds.rs +++ b/crates/engine/src/frame_cmds.rs @@ -40,9 +40,9 @@ fn frame_from_layers(s: &mut Session, p: &Value) -> Result { } let grouped = s.execute("layer.new.groupFromLayers", Value::Object(gp))?; let gid = LayerId(grouped.get("layer").and_then(Value::as_u64).ok_or_else(|| EngineError::Other("grouping did not return a layer".into()))?); - let rect = crate::layer_multi_cmds::layer_bounds(s.active().ok_or(EngineError::NoDocument)?.doc.layer(gid).ok_or(EngineError::NoLayer(gid))?) - .filter(|r| !r.is_empty()) - .unwrap_or_else(|| s.active().unwrap().doc.bounds()); + let d = s.active().ok_or(EngineError::NoDocument)?; + let rect = + crate::layer_multi_cmds::layer_bounds(d.doc.layer(gid).ok_or(EngineError::NoLayer(gid))?).filter(|r| !r.is_empty()).unwrap_or_else(|| d.doc.bounds()); s.edit("Frame from Layers", |doc, _| { let g = doc.layer_mut(gid).ok_or(EngineError::NoLayer(gid))?; diff --git a/crates/engine/src/layer_menu_cmds.rs b/crates/engine/src/layer_menu_cmds.rs index 5e604b7a..20869913 100644 --- a/crates/engine/src/layer_menu_cmds.rs +++ b/crates/engine/src/layer_menu_cmds.rs @@ -606,7 +606,7 @@ fn create_layer(s: &mut Session, p: &Value) -> Result { let n = below.len() + above.len(); // Insert in order: below layers under the source, clipped layers above it. let path = doc.path_of(id).ok_or(EngineError::NoLayer(id))?; - let (&at, parent) = path.split_last().expect("non-empty path"); + let (&at, parent) = path.split_last().ok_or_else(|| other("bad layer path"))?; let sib = if parent.is_empty() { &mut doc.layers } else { doc.layer_at_mut(parent).and_then(|t| t.children_mut()).ok_or_else(|| other("bad layer path"))? }; let src = &mut sib[at]; diff --git a/crates/engine/src/layer_multi_cmds.rs b/crates/engine/src/layer_multi_cmds.rs index a572c997..e47308a6 100644 --- a/crates/engine/src/layer_multi_cmds.rs +++ b/crates/engine/src/layer_multi_cmds.rs @@ -521,7 +521,7 @@ pub fn group_layers(s: &mut Session, p: &Value) -> Result { for id in &ids { children.push(doc.remove(*id).ok_or(EngineError::NoLayer(*id))?); } - *doc.layer_mut(gid).and_then(Layer::children_mut).expect("new group") = children; + *doc.layer_mut(gid).and_then(Layer::children_mut).ok_or(EngineError::NoLayer(gid))? = children; *active = Some(gid); Ok(gid) })?; @@ -536,7 +536,7 @@ fn merge_layers(s: &mut Session) -> Result { } let mid = s.edit("Merge Layers", |doc, active| { let ids = top_level(doc, &sel); - let top_id = *ids.last().expect("two or more"); + let top_id = *ids.last().ok_or_else(|| EngineError::Other("Merge Layers needs two or more layers".into()))?; let top = doc.layer(top_id).ok_or(EngineError::NoLayer(top_id))?.clone(); let mut solo = doc.clone(); solo.layers = ids.iter().filter_map(|id| doc.layer(*id)).filter(|l| l.visible).cloned().collect(); @@ -549,7 +549,7 @@ fn merge_layers(s: &mut Session) -> Result { let data: Vec = buf.px.iter().flat_map(|p| photocraft_raster::from_rgba(&fmt, *p)).collect(); let mut merged = Layer::raster(top.name.clone(), fmt); merged.locks = top.locks; - let surf = merged.surface_mut().expect("raster"); + let surf = crate::pixels_mut(&mut merged)?; surf.write_region(doc.bounds(), &data); surf.prune(); let mid = merged.id; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c276427f..a5fd9fd1 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -5,6 +5,7 @@ //! [`Session::execute`] entry point: the egui UI, the CLI, the remote-control channel and the MCP //! server. This is what makes the UI swappable and the app fully scriptable. #![forbid(unsafe_code)] +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] pub mod adjust_cmds; pub mod align_cmds; @@ -97,6 +98,19 @@ pub enum EngineError { pub type Result = std::result::Result; +/// The pixels of a raster layer (typically one a command just created), as an error instead +/// of a panic if the layer has none. +pub(crate) fn pixels_mut(l: &mut photocraft_doc::Layer) -> Result<&mut photocraft_raster::Surface> { + let id = l.id; + l.surface_mut().ok_or_else(|| EngineError::Other(format!("layer {id:?} has no pixels"))) +} + +/// The active document's active layer, for `enabled` predicates. +pub(crate) fn active_layer_of(s: &Session) -> std::result::Result<&photocraft_doc::Layer, String> { + let d = s.active().ok_or("no document open")?; + d.active_layer.and_then(|id| d.doc.layer(id)).ok_or_else(|| "no active layer".into()) +} + /// Per-document editing state. #[derive(Clone, Debug)] pub struct DocState { diff --git a/crates/engine/src/mode_cmds.rs b/crates/engine/src/mode_cmds.rs index 9c7ac2fe..b23a46f1 100644 --- a/crates/engine/src/mode_cmds.rs +++ b/crates/engine/src/mode_cmds.rs @@ -83,9 +83,8 @@ fn rotate_arbitrary(s: &mut Session, p: &Value) -> Result { let fmt = doc.pixel_format(); for l in doc.layers.iter_mut() { let background = l.name == "Background" && l.locks.position && matches!(l.content, LayerContent::Raster(_)); - if background { + if background && let Some(surf) = l.surface_mut() { // The Background stays a Background: rotated pixels over the background colour. - let surf = l.surface_mut().expect("raster"); let src = surf.content_bounds(); let rotated = photocraft_algo::transform::warp_surface(surf, src, &h, interp); let mut base = Surface::new(fmt); @@ -146,7 +145,10 @@ fn single_layer(doc: &mut Document, active: &mut Option, mode: ColorMod l.locks.transparency = true; l.locks.position = true; } - l.surface_mut().expect("raster").write_region(doc.bounds(), &data); + // A new raster layer always has pixels. + if let Some(surf) = l.surface_mut() { + surf.write_region(doc.bounds(), &data); + } *active = Some(l.id); doc.layers = vec![l]; for ch in doc.channels.iter_mut() { diff --git a/crates/engine/src/multichannel_cmds.rs b/crates/engine/src/multichannel_cmds.rs index 006978f3..a6e74101 100644 --- a/crates/engine/src/multichannel_cmds.rs +++ b/crates/engine/src/multichannel_cmds.rs @@ -108,13 +108,12 @@ fn inks_of(doc: &Document) -> Vec<(String, Color, Vec)> { .map(|((n, c), p)| (n.to_string(), c, p)) .collect(), ColorMode::Lab => planes.iter().enumerate().map(|(k, p)| (format!("Alpha {}", k + 1), Color::BLACK, inv(p))).collect(), - ColorMode::Duotone if doc.duotone.as_ref().is_some_and(|d| !d.inks.is_empty()) => { - let d = doc.duotone.as_ref().expect("checked"); - d.inks - .iter() - .map(|ink| (ink.name.clone(), Color::rgb(ink.color[0], ink.color[1], ink.color[2]), planes[0].iter().map(|g| ink.density(1.0 - g)).collect())) - .collect() - } + ColorMode::Duotone if doc.duotone.as_ref().is_some_and(|d| !d.inks.is_empty()) => doc + .duotone + .iter() + .flat_map(|d| d.inks.iter()) + .map(|ink| (ink.name.clone(), Color::rgb(ink.color[0], ink.color[1], ink.color[2]), planes[0].iter().map(|g| ink.density(1.0 - g)).collect())) + .collect(), _ => vec![("Black".to_string(), Color::BLACK, inv(&planes[0]))], } } diff --git a/crates/engine/src/photo_cmds.rs b/crates/engine/src/photo_cmds.rs index 6c77afdd..4893ea1f 100644 --- a/crates/engine/src/photo_cmds.rs +++ b/crates/engine/src/photo_cmds.rs @@ -227,8 +227,7 @@ pub(crate) fn seam_blend(warped: &[Surface], canvas: Rect, order: &[usize], seam } for y in 0..roi.h { for x in 0..roi.w { - let r2 = if vig { - let (pl, sizes) = geo.expect("vignette needs geometry"); + let r2 = if vig && let Some((pl, sizes)) = geo { pl[i].inverse((roi.x0 + x as i32) as f64 + 0.5, (roi.y0 + y as i32) as f64 + 0.5).map_or(0.0, |(sx, sy)| { let (w, h) = (sizes[i].0 as f64, sizes[i].1 as f64); let (dx, dy) = (sx - w / 2.0, sy - h / 2.0); @@ -410,11 +409,13 @@ fn photomerge(s: &mut Session, p: &Value) -> Result { .ok_or_else(|| EngineError::Other("Photomerge couldn't find enough matching detail between the images".into()))?; let t_reg = t0.ms() / 1000.0; let placed: Vec = (0..srcs.len()).filter(|&i| al.placements[i].is_some()).collect(); + // Parallel to `placed`. + let placements: Vec<&Placement> = placed.iter().filter_map(|&i| al.placements[i].as_ref()).collect(); let failed: Vec = (0..srcs.len()).filter(|&i| al.placements[i].is_none()).map(|i| srcs[i].name.clone()).collect(); // Canvas: union of the placed images. let (mut x0, mut y0, mut x1, mut y1) = (f64::MAX, f64::MAX, f64::MIN, f64::MIN); - for &i in &placed { - let b = al.placements[i].as_ref().expect("placed").bounds(srcs[i].w as f64, srcs[i].h as f64); + for (&i, pl) in placed.iter().zip(&placements) { + let b = pl.bounds(srcs[i].w as f64, srcs[i].h as f64); x0 = x0.min(b[0]); y0 = y0.min(b[1]); x1 = x1.max(b[2]); @@ -427,18 +428,10 @@ fn photomerge(s: &mut Session, p: &Value) -> Result { let canvas = Rect::new(0, 0, cw as i32, ch as i32); let offset = (-x0.floor(), -y0.floor()); let warped: Vec = { - let jobs: Vec = placed.clone(); - jobs.iter() - .map(|&i| { - warp_placed( - &srcs[i].surf, - Rect::new(0, 0, srcs[i].w as i32, srcs[i].h as i32), - (0.0, 0.0), - al.placements[i].as_ref().expect("placed"), - offset, - Interp::Bicubic, - ) - }) + placed + .iter() + .zip(&placements) + .map(|(&i, pl)| warp_placed(&srcs[i].surf, Rect::new(0, 0, srcs[i].w as i32, srcs[i].h as i32), (0.0, 0.0), pl, offset, Interp::Bicubic)) .collect() }; let t_warp = t0.ms() / 1000.0; @@ -447,7 +440,7 @@ fn photomerge(s: &mut Session, p: &Value) -> Result { let mut info = json!({}); let ref_pos = placed.iter().position(|&i| i == al.reference).unwrap_or(0); if blend { - let pls: Vec = placed.iter().map(|&i| al.placements[i].clone().expect("placed")).collect(); + let pls: Vec = placements.iter().map(|p| (*p).clone()).collect(); let sizes: Vec<(usize, usize)> = placed.iter().map(|&i| (srcs[i].w, srcs[i].h)).collect(); let order = seam_order(&warped, ref_pos); let geo = vignette.then_some((pls.as_slice(), sizes.as_slice(), offset)); diff --git a/crates/engine/src/presets/shapes.rs b/crates/engine/src/presets/shapes.rs index a60f502e..e8a966e4 100644 --- a/crates/engine/src/presets/shapes.rs +++ b/crates/engine/src/presets/shapes.rs @@ -68,8 +68,10 @@ pub fn parse(src: &str) -> std::result::Result { s.closed = closed; if closed && s.knots.len() > 1 { let (f, l) = (s.knots[0].anchor, s.knots[s.knots.len() - 1].anchor); - if (f.x - l.x).abs() < 1e-9 && (f.y - l.y).abs() < 1e-9 { - let last = s.knots.pop().expect("len > 1"); + if (f.x - l.x).abs() < 1e-9 + && (f.y - l.y).abs() < 1e-9 + && let Some(last) = s.knots.pop() + { s.knots[0].in_ctrl = last.in_ctrl; } } @@ -99,7 +101,7 @@ pub fn parse(src: &str) -> std::result::Result { "C" | "Q" => { let s = cur.as_mut().ok_or("curve before M")?; while is_num(i) { - let last = s.knots.last().expect("M pushed a knot").anchor; + let last = s.knots.last().ok_or("curve before M")?.anchor; let (c1, c2, p) = if t == "C" { let c1 = Point::new(num(&mut i)?, num(&mut i)?); let c2 = Point::new(num(&mut i)?, num(&mut i)?); @@ -113,7 +115,9 @@ pub fn parse(src: &str) -> std::result::Result { p, ) }; - s.knots.last_mut().expect("non-empty").out_ctrl = c1; + if let Some(k) = s.knots.last_mut() { + k.out_ctrl = c1; + } s.knots.push(Knot { anchor: p, in_ctrl: c2, out_ctrl: p, smooth: false }); } } diff --git a/crates/engine/src/variables_cmds.rs b/crates/engine/src/variables_cmds.rs index c219e8da..71d5ae60 100644 --- a/crates/engine/src/variables_cmds.rs +++ b/crates/engine/src/variables_cmds.rs @@ -70,6 +70,14 @@ fn kind_json(k: &VarKind) -> Value { } } +/// `a` with `b`'s fields added (both are JSON objects). +fn merged(mut a: Value, b: Value) -> Value { + if let (Some(ao), Value::Object(bo)) = (a.as_object_mut(), b) { + ao.extend(bo); + } + a +} + fn value_json(v: &VarValue) -> Value { match v { VarValue::Visibility(b) => json!({"kind": "visibility", "value": b}), @@ -355,16 +363,12 @@ fn list(s: &mut Session) -> Result { let v = &s.active().ok_or(EngineError::NoDocument)?.doc.variables; Ok(json!({ "defs": v.defs.iter().map(|d| { - let mut o = json!({"name": d.name, "layer": d.layer.0}); - o.as_object_mut().unwrap().extend(kind_json(&d.kind).as_object().unwrap().clone()); - o + merged(json!({"name": d.name, "layer": d.layer.0}), kind_json(&d.kind)) }).collect::>(), "dataSets": v.data_sets.iter().map(|ds| json!({ "name": ds.name, "values": ds.values.iter().map(|dv| { - let mut o = json!({"variable": dv.variable}); - o.as_object_mut().unwrap().extend(value_json(&dv.value).as_object().unwrap().clone()); - o + merged(json!({"variable": dv.variable}), value_json(&dv.value)) }).collect::>(), })).collect::>(), "active": v.active, diff --git a/crates/engine/src/vector_cmds.rs b/crates/engine/src/vector_cmds.rs index 104d0ac2..2660650c 100644 --- a/crates/engine/src/vector_cmds.rs +++ b/crates/engine/src/vector_cmds.rs @@ -439,7 +439,9 @@ fn with_shape(s: &mut Session, id: LayerId, label: &str, f: impl FnOnce(&mut if !matches!(l.content, LayerContent::Shape(_)) { return Err(EngineError::Other(format!("layer {} is a {} layer, not a shape layer", id.0, l.content.kind_name()))); } - let LayerContent::Shape(mut sh) = std::mem::replace(&mut l.content, LayerContent::Fill(Fill::Solid(Color::BLACK))) else { unreachable!() }; + let LayerContent::Shape(mut sh) = std::mem::replace(&mut l.content, LayerContent::Fill(Fill::Solid(Color::BLACK))) else { + return Err(EngineError::Other(format!("layer {} is not a shape layer", id.0))); + }; let r = f(&mut sh, l); refresh_shape(&snapshot, &mut sh); l.content = LayerContent::Shape(sh); diff --git a/crates/engine/src/web_cmds.rs b/crates/engine/src/web_cmds.rs index a9060c45..b52fd3a1 100644 --- a/crates/engine/src/web_cmds.rs +++ b/crates/engine/src/web_cmds.rs @@ -618,8 +618,9 @@ fn quantize_rows(px: &mut [[f32; 4]], w: usize, pal: &[[u8; 3]], dither: Dither, .for_each(|(i, (p, out))| out.copy_from_slice(&quantize_band(p, w, i * 64, pal, dither, amount, t))); idx } + // Single-threaded on wasm (the early return above always takes this path there). #[cfg(target_arch = "wasm32")] - unreachable!() + quantize_band(px, w, 0, pal, dither, amount, t) } /// A file-name-safe slice name. @@ -840,8 +841,19 @@ fn quick_export(s: &mut Session, p: &Value) -> Result { } }; let metadata = serde_json::to_value(prefs.metadata).ok().and_then(|v| v.as_str().map(str::to_string)).unwrap_or_default(); - let mut q = json!({"metadata": if metadata == "all" { "all" } else if metadata == "none" { "none" } else { "copyright" }, "convertToSrgb": prefs.convert_to_srgb, "transparency": true}); - let o = q.as_object_mut().unwrap_or_else(|| unreachable!()); + let mut o = serde_json::Map::new(); + o.insert( + "metadata".into(), + json!(if metadata == "all" { + "all" + } else if metadata == "none" { + "none" + } else { + "copyright" + }), + ); + o.insert("convertToSrgb".into(), json!(prefs.convert_to_srgb)); + o.insert("transparency".into(), json!(true)); match fmt.as_str() { "jpg" => { o.insert("format".into(), json!("jpeg")); @@ -861,6 +873,7 @@ fn quick_export(s: &mut Session, p: &Value) -> Result { o.insert("format".into(), json!("png24")); } } + let q = Value::Object(o); let st = WebSettings::from_params(&q, cmd)?; let (wdoc, _, _) = web_document(&doc, &json!({}), &st)?; let buf = photocraft_compose::flatten(&wdoc);