pshock13/effectcraft
1
0 Fork 0
mirror of https://github.com/storytold/effectcraft.git synced 2026-10-14 19:27:05 -04:00
Code Issues Packages Projects Releases Wiki Activity

Never crash: effects crate without expect()

Browse Source 
Effects without expect(): nested parameter groups are looked up with ? (falling back to the
effect's top level), the OCIO effects use named constants for the built-in ACES spaces instead
of expect()ing a table lookup, curve fitting and Path Text no longer expect on just-pushed
vectors. Test-only unreachable!() becomes panic!(). Denies the panic lints in the crate.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Brandon ThomasandClaude Opus 5.5 committed 2026-10-04 14:14:16 -04:00
1 parent 10644a83aa
commit 5dd7d9bb57
5 files changed
+41 -33
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

clippy.toml
+4
View File
@@ -0,0 +1,4 @@
allow-unwrap-in-tests = true
allow-expect-in-tests = true
allow-panic-in-tests = true
allow-indexing-slicing-in-tests = true
crates/effects/src/lib.rs
+9 -7
View File
@@ -6,6 +6,8 @@
//! [`instantiate`]), so parameters animate, take expressions and are addressable like any other
//! property.
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
pub mod audio_fx;
mod blur2;
mod blur3;
@@ -551,8 +553,8 @@ pub fn instantiate(spec: &EffectSpec, ids: &mut Ids, instance_name: &str, layer_
if matches!(ps.ui, ParamUi::Checkbox | ParamUi::Popup { .. } | ParamUi::Layer) {
pr.hold_only = true;
}
match path {
Some(path) => nested_group(&mut g, ids, path).children.push(pr.into()),
match path.and_then(|path| nested_group(&mut g, ids, path)) {
Some(sub) => sub.children.push(pr.into()),
None => g.children.push(pr.into()),
}
// Layer parameters carry a source choice (Source / Masks / Effects & Masks) unless the
@@ -563,8 +565,8 @@ pub fn instantiate(spec: &EffectSpec, ids: &mut Ids, instance_name: &str, layer_
if matches!(ps.ui, ParamUi::Layer) && !spec.params.iter().any(|q| q.id == sid) {
let mut src = Property::new(ids.alloc(), &layer_source_id(leaf), &format!("{} Source", ps.name), Value::Enum(2)).with_ui(ParamUi::Hidden);
src.hold_only = true;
match path {
Some(path) => nested_group(&mut g, ids, path).children.push(src.into()),
match path.and_then(|path| nested_group(&mut g, ids, path)) {
Some(sub) => sub.children.push(src.into()),
None => g.children.push(src.into()),
}
}
@@ -765,7 +767,7 @@ fn group_name(m: &str) -> &str {
}
/// The nested group at `path` (`borders/autoScale`) under `g`, created on first use.
fn nested_group<'a>(g: &'a mut PropGroup, ids: &mut Ids, path: &str) -> &'a mut PropGroup {
fn nested_group<'a>(g: &'a mut PropGroup, ids: &mut Ids, path: &str) -> Option<&'a mut PropGroup> {
let (first, rest) = match path.split_once('/') {
Some((a, b)) => (a, Some(b)),
None => (path, None),
@@ -773,10 +775,10 @@ fn nested_group<'a>(g: &'a mut PropGroup, ids: &mut Ids, path: &str) -> &'a mut
if g.sub(first).is_none() {
g.children.push(ids.group(first, group_name(first)).into());
}
let sub = g.sub_mut(first).expect("just added");
let sub = g.sub_mut(first)?;
match rest {
Some(r) => nested_group(sub, ids, r),
None => sub,
None => Some(sub),
}
}
crates/effects/src/migrate.rs
+8 -8
View File
@@ -192,7 +192,7 @@ fn prop_at<'a>(g: &'a mut PropGroup, id: &str) -> Option<&'a mut Property> {
}
}
fn group_at<'a>(g: &'a mut PropGroup, ids: &mut Ids, path: &str) -> &'a mut PropGroup {
fn group_at<'a>(g: &'a mut PropGroup, ids: &mut Ids, path: &str) -> Option<&'a mut PropGroup> {
let (first, rest) = match path.split_once('/') {
Some((a, b)) => (a, Some(b)),
None => (path, None),
@@ -200,10 +200,10 @@ fn group_at<'a>(g: &'a mut PropGroup, ids: &mut Ids, path: &str) -> &'a mut Prop
if g.sub(first).is_none() {
g.children.push(ids.group(first, group_name(first)).into());
}
let sub = g.sub_mut(first).expect("just added");
let sub = g.sub_mut(first)?;
match rest {
Some(r) => group_at(sub, ids, r),
None => sub,
None => Some(sub),
}
}
@@ -288,8 +288,8 @@ pub fn upgrade_instance(spec: &EffectSpec, g: &mut PropGroup, ids: &mut Ids, lay
pr
}
};
match path {
Some(path) => group_at(g, ids, path).children.push(pr.into()),
match path.and_then(|path| group_at(g, ids, path)) {
Some(sub) => sub.children.push(pr.into()),
None => g.children.push(pr.into()),
}
}
@@ -310,8 +310,8 @@ pub fn upgrade_instance(spec: &EffectSpec, g: &mut PropGroup, ids: &mut Ids, lay
src.hold_only = true;
src
});
match path {
Some(path) => group_at(g, ids, path).children.push(src.into()),
match path.and_then(|path| group_at(g, ids, path)) {
Some(sub) => sub.children.push(src.into()),
None => g.children.push(src.into()),
}
}
@@ -363,7 +363,7 @@ mod tests {
let removed = spec.params.last().unwrap().id;
g.children.retain(|c| c.match_id() != removed);
let popup = spec.params.iter().find(|p| matches!(p.ui, ParamUi::Popup { .. })).expect("Gaussian Blur has a popup");
let ParamUi::Popup { options } = &popup.ui else { unreachable!() };
let ParamUi::Popup { options } = &popup.ui else { panic!("not a popup") };
{
let pr = g.get_mut(popup.id).unwrap();
pr.name = "Old Name".into();
crates/effects/src/ocio.rs
+16 -11
View File
@@ -128,10 +128,14 @@ const fn sp(name: &'static str, prims: [[f64; 2]; 3], white: [f64; 2], tf: Tf) -
}
/// The built-in configuration's colour spaces, in popup order.
const ACES2065_1: Space = sp("ACES2065-1", AP0, ACES_WHITE, Tf::Linear);
const ACES_CG: Space = sp("ACEScg", AP1, ACES_WHITE, Tf::Linear);
const ACES_CCT: Space = sp("ACEScct", AP1, ACES_WHITE, Tf::AcesCct);
pub const SPACES: &[Space] = &[
sp("ACES2065-1", AP0, ACES_WHITE, Tf::Linear),
sp("ACEScg", AP1, ACES_WHITE, Tf::Linear),
sp("ACEScct", AP1, ACES_WHITE, Tf::AcesCct),
ACES2065_1,
ACES_CG,
ACES_CCT,
sp("Linear Rec.709 (sRGB)", REC709, D65, Tf::Linear),
sp("sRGB", REC709, D65, Tf::Srgb),
sp("Gamma 2.4 Rec.709", REC709, D65, Tf::Gamma(2.4)),
@@ -147,6 +151,7 @@ pub fn space_names() -> Vec<&'static str> {
SPACES.iter().map(|s| s.name).collect()
}
#[cfg(test)]
pub fn space_by_name(n: &str) -> Option<&'static Space> {
SPACES.iter().find(|s| s.name.eq_ignore_ascii_case(n))
}
@@ -778,7 +783,7 @@ fn ocio_look(ctx: &EffectCtx, mut b: Buf) -> Buf {
}
let (src, dst) = (space_at(ctx.params.e("source")), space_at(ctx.params.e("destination")));
let inv = ctx.params.e("direction") == 1;
let cct = space_by_name("ACEScct").expect("built-in");
let cct = &ACES_CCT;
let (a, z) = if inv { (dst, src) } else { (src, dst) };
let to_p = Xform::new(a, cct);
let from_p = Xform::new(cct, z);
@@ -805,8 +810,8 @@ fn profile(ctx: &EffectCtx, i: u32) -> (Space, bool) {
ColorSpace::Rec709 => sp("Rec.709", REC709, D65, Tf::Gamma(2.4)),
ColorSpace::Rec2020 => sp("Rec.2020", REC2020, D65, Tf::Gamma(2.4)),
ColorSpace::DisplayP3 => sp("Display P3", P3, D65, Tf::Srgb),
ColorSpace::AcesCg => *space_by_name("ACEScg").expect("built-in"),
ColorSpace::Aces2065 => *space_by_name("ACES2065-1").expect("built-in"),
ColorSpace::AcesCg => ACES_CG,
ColorSpace::Aces2065 => ACES2065_1,
// HDR encodings are output spaces, never a working space.
ColorSpace::Rec2100Pq | ColorSpace::Rec2100Hlg => sp("Rec.2020", REC2020, D65, Tf::Gamma(2.4)),
};
@@ -816,8 +821,8 @@ fn profile(ctx: &EffectCtx, i: u32) -> (Space, bool) {
2 => (ws(ColorSpace::Rec709), false),
3 => (ws(ColorSpace::Rec2020), false),
4 => (ws(ColorSpace::DisplayP3), false),
5 => (*space_by_name("ACEScg").expect("built-in"), true),
_ => (*space_by_name("ACES2065-1").expect("built-in"), true),
5 => (ACES_CG, true),
_ => (ACES2065_1, true),
}
}
@@ -1089,7 +1094,7 @@ pub fn color_program(id: &str, ctx: &EffectCtx) -> Option<(Vec<ColorOp>, Straigh
if !list.is_empty() {
let (src, dst) = (space_at(pr.e("source")), space_at(pr.e("destination")));
let inv = pr.e("direction") == 1;
let cct = space_by_name("ACEScct").expect("built-in");
let cct = &ACES_CCT;
let (a, z) = if inv { (dst, src) } else { (src, dst) };
if inv {
list.reverse();
@@ -1157,7 +1162,7 @@ fn curve3(xs: [f64; 3], ys: [f64; 3], v: f64) -> f64 {
let mut pairs: Vec<(f64, f64)> = xs.iter().cloned().zip(ys.iter().cloned()).collect();
pairs.sort_by(|a, b| a.0.total_cmp(&b.0));
for (x, y) in pairs {
if x > pts.last().expect("non-empty").0 + 1e-4 && x < 1.0 - 1e-4 {
if pts.last().is_some_and(|l| x > l.0 + 1e-4) && x < 1.0 - 1e-4 {
pts.push((x, y));
}
}
@@ -1196,7 +1201,7 @@ pub fn color_stabilizer_maps(ctx: &EffectCtx, b: &Buf) -> Option<[Vec<(f64, f64)
let mut pairs: Vec<(f64, f64)> = (0..3).map(|i| (cur[i][k], refv[i][k])).collect();
pairs.sort_by(|a, b| a.0.total_cmp(&b.0));
for (x, y) in pairs {
if x > v.last().expect("non-empty").0 + 1e-4 && x < 1.0 - 1e-4 {
if v.last().is_some_and(|l| x > l.0 + 1e-4) && x < 1.0 - 1e-4 {
v.push((x, y));
}
}
crates/effects/src/textfx.rs
+4 -7
View File
@@ -686,13 +686,10 @@ fn path_text_passes(ctx: &EffectCtx, b: &Buf) -> Vec<TextPass> {
let js = if j_scale != 0.0 { (1.0 + rnd(i, 4) * j_scale).max(0.05) } else { 1.0 };
let jb = if j_base != 0.0 { rnd(i, 1) * j_base } else { 0.0 };
let left = ink_left(c, true);
let set = match polys_by_alpha.iter_mut().find(|(a, _)| (*a - alpha).abs() < 1e-4) {
Some((_, v)) => v,
None => {
polys_by_alpha.push((alpha, Vec::new()));
&mut polys_by_alpha.last_mut().expect("pushed").1
}
};
if !polys_by_alpha.iter().any(|(a, _)| (*a - alpha).abs() < 1e-4) {
polys_by_alpha.push((alpha, Vec::new()));
}
let Some((_, set)) = polys_by_alpha.iter_mut().find(|(a, _)| (*a - alpha).abs() < 1e-4) else { continue };
for st in parse_glyph(c) {
set.push(
st.iter()