From 26af00d1084ec69e66221a6d9a4924efd88e1249 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 17 Oct 2026 05:54:04 -0700 Subject: [PATCH] Effects: vectorcraft-effects passes clippy on Rust 1.95, the minimum toolchain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Rust 1.95, the workspace's rust-version, cargo clippy --workspace --all-targets -- -D warnings stopped at vectorcraft-effects: clippy 1.95 reports nonminimal_bool on the parameter and pixel-size guards of Cutout, Poster Edges and Neon Glow, and on the size check in Rasterize::compose (Effect › Rasterize). Clippy 1.99 does not report them. Each check now has the form clippy suggests, one !(a && b && ...) in place of !a || !(b && c) (!a || !b || !(c && d) in compose). The operands and their order are unchanged, so the filters and compose return early in the same cases as before. New tests check that the three filters leave the art unchanged for a pixel size of 0, -1, NaN or infinity and for non-finite parameters, and that compose leaves the raster unchanged when a buffer's length does not match its size or the image's area has no positive width or height. --- crates/effects/src/pixel/artistic.rs | 6 ++-- crates/effects/src/pixel/tests.rs | 43 +++++++++++++++++++++++++++ crates/effects/src/raster.rs | 2 +- crates/effects/src/tests.rs | 44 ++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/crates/effects/src/pixel/artistic.rs b/crates/effects/src/pixel/artistic.rs index 0a7e3aacc..f18b1709a 100644 --- a/crates/effects/src/pixel/artistic.rs +++ b/crates/effects/src/pixel/artistic.rs @@ -165,7 +165,7 @@ fn planes(px: &[[u8; 4]]) -> ([Vec; 3], Vec) { /// colours keep their hue. Slivers (the blends along softened edges) take the nearest large /// piece's colour, so the pieces meet with crisp edges. Coverage is kept. pub(super) fn cutout(px: &mut [[u8; 4]], w: usize, h: usize, space: &PixelSpace, levels: f64, simplicity: f64, fidelity: f64) { - if ![levels, simplicity, fidelity].iter().all(|v| v.is_finite()) || !(space.px.is_finite() && space.px > 0.0) { + if !([levels, simplicity, fidelity].iter().all(|v| v.is_finite()) && space.px.is_finite() && space.px > 0.0) { return; } let levels = levels.round().clamp(2.0, 8.0) as usize; @@ -212,7 +212,7 @@ pub(super) fn cutout(px: &mut [[u8; 4]], w: usize, h: usize, space: &PixelSpace, /// channel, with black lines drawn on the edges of any of its colour channels and of its outline. `thickness` (0..10) widens the lines and /// `intensity` (0..10) darkens them and lets fainter edges take a line. Coverage is kept. pub(super) fn poster_edges(px: &mut [[u8; 4]], w: usize, h: usize, space: &PixelSpace, thickness: f64, intensity: f64, posterization: f64) { - if ![thickness, intensity, posterization].iter().all(|v| v.is_finite()) || !(space.px.is_finite() && space.px > 0.0) { + if !([thickness, intensity, posterization].iter().all(|v| v.is_finite()) && space.px.is_finite() && space.px > 0.0) { return; } let steps = (posterization.round().clamp(0.0, 6.0) + 1.0) as f32; @@ -248,7 +248,7 @@ pub(super) fn glow_spread(size: f64) -> f64 { /// above 0) or from its dark areas (below 0), spread as far as |`size`| (0..24) says and as /// strong as `brightness` (0..50). Coverage is kept. pub(super) fn neon_glow(px: &mut [[u8; 4]], w: usize, h: usize, space: &PixelSpace, size: f64, brightness: f64, colour: [f32; 3]) { - if ![size, brightness].iter().all(|v| v.is_finite()) || !(space.px.is_finite() && space.px > 0.0) { + if !([size, brightness].iter().all(|v| v.is_finite()) && space.px.is_finite() && space.px > 0.0) { return; } let strength = (brightness / 50.0).clamp(0.0, 1.0) as f32; diff --git a/crates/effects/src/pixel/tests.rs b/crates/effects/src/pixel/tests.rs index 1a599a534..41300884f 100644 --- a/crates/effects/src/pixel/tests.rs +++ b/crates/effects/src/pixel/tests.rs @@ -2133,6 +2133,49 @@ fn artistic_params_take_defaults_and_stay_in_range() { } } +#[test] +fn artistic_filters_leave_invalid_coordinate_spaces_alone() { + let (w, h) = (16, 12); + let src = ramp(w, h); + let run = |f: &PixelFx, s: &PixelSpace| { + let mut out = src.clone(); + f.apply(&mut out, w, h, s); + out + }; + let colour = [0.3, 0.65, 1.0]; + let filters = [ + PixelFx::Cutout { levels: 4.0, simplicity: 4.0, fidelity: 2.0 }, + PixelFx::NeonGlow { size: 5.0, brightness: 15.0, colour }, + PixelFx::PosterEdges { thickness: 2.0, intensity: 1.0, posterization: 2.0 }, + ]; + for f in &filters { + assert_ne!(run(f, &space(w, h)), src, "{f:?} changes the art in a valid space"); + } + let mut changed = Vec::new(); + for f in &filters { + for px in [0.0, -1.0, f64::NAN, f64::INFINITY] { + if run(f, &PixelSpace { px, ..space(w, h) }) != src { + changed.push(format!("{f:?} at px {px}")); + } + } + } + for f in [ + PixelFx::Cutout { levels: f64::NAN, simplicity: 4.0, fidelity: 2.0 }, + PixelFx::Cutout { levels: 4.0, simplicity: f64::INFINITY, fidelity: 2.0 }, + PixelFx::Cutout { levels: 4.0, simplicity: 4.0, fidelity: f64::NEG_INFINITY }, + PixelFx::NeonGlow { size: f64::NAN, brightness: 15.0, colour }, + PixelFx::NeonGlow { size: 5.0, brightness: f64::INFINITY, colour }, + PixelFx::PosterEdges { thickness: f64::NAN, intensity: 1.0, posterization: 2.0 }, + PixelFx::PosterEdges { thickness: 2.0, intensity: f64::INFINITY, posterization: 2.0 }, + PixelFx::PosterEdges { thickness: 2.0, intensity: 1.0, posterization: f64::NEG_INFINITY }, + ] { + if run(&f, &space(w, h)) != src { + changed.push(format!("{f:?}")); + } + } + assert!(changed.is_empty(), "directly constructed non-finite parameters and invalid spaces are harmless: {changed:?}"); +} + #[test] fn cutout_paints_a_few_flat_colours_and_keeps_coverage() { let (w, h) = (64, 24); diff --git a/crates/effects/src/raster.rs b/crates/effects/src/raster.rs index e9a51fc43..23d67dff6 100644 --- a/crates/effects/src/raster.rs +++ b/crates/effects/src/raster.rs @@ -197,7 +197,7 @@ impl Rasterize { let (px, rest) = data.as_chunks_mut::<4>(); let (src, src_rest) = image.as_chunks::<4>(); let fits = |n: usize, w: usize, h: usize, rest: usize| w > 0 && h > 0 && rest == 0 && w.checked_mul(h) == Some(n); - if !fits(px.len(), w, h, rest.len()) || !fits(src.len(), cols, rows, src_rest.len()) || !(rect.width() > 0.0 && rect.height() > 0.0) { + if !(fits(px.len(), w, h, rest.len()) && fits(src.len(), cols, rows, src_rest.len()) && rect.width() > 0.0 && rect.height() > 0.0) { return; } let (sx, sy) = (cols as f64 / rect.width(), rows as f64 / rect.height()); diff --git a/crates/effects/src/tests.rs b/crates/effects/src/tests.rs index f075e4095..34047cc05 100644 --- a/crates/effects/src/tests.rs +++ b/crates/effects/src/tests.rs @@ -834,6 +834,50 @@ fn drop_shadow_darkness_replaces_its_color_when_chosen() { assert_eq!(darkness(json!({})), None); } +#[test] +fn rasterize_compose_leaves_the_raster_alone_when_the_sizes_do_not_fit() { + let r = match raster_effects(&[fx(RASTERIZE, json!({}))]).as_slice() { + [RasterFx::Rasterize(r)] => *r, + other => panic!("{other:?}"), + }; + let space = PixelSpace { + to_doc: Affine::IDENTITY, + px: 1.0, + center: Point::new(1.0, 1.0), + channels: pixel::Channels::Rgb, + line: 1.0, + extent: std::f64::consts::SQRT_2, + }; + let compose = |len: usize, w: usize, h: usize, image: &[u8], cols: usize, rows: usize, rect: Rect| { + let mut data = vec![7; len]; + r.compose(&mut data, w, h, &space, image, cols, rows, rect); + data + }; + // A 2 × 2 red image over a 2 × 2 raster of one-point pixels: each pixel takes the image's color. + let red = [255, 0, 0, 255].repeat(4); + let mut long = red.clone(); + long.extend([255, 0]); + let (img, long, area) = (red.as_slice(), long.as_slice(), Rect::new(0.0, 0.0, 2.0, 2.0)); + assert_eq!(compose(16, 2, 2, img, 2, 2, area), red); + let cases = [ + ("w × h is not the raster's pixel count", 16, 1, 2, img, 2, 2, area), + ("the raster's length is not a multiple of 4", 18, 2, 2, img, 2, 2, area), + ("cols × rows is not the image's pixel count", 16, 2, 2, img, 1, 2, area), + ("the image's length is not a multiple of 4", 16, 2, 2, long, 2, 2, area), + ("the image has no width", 16, 2, 2, img, 2, 2, Rect::new(0.0, 0.0, 0.0, 2.0)), + ("the image has no height", 16, 2, 2, img, 2, 2, Rect::new(0.0, 0.0, 2.0, 0.0)), + ("the image has a negative width", 16, 2, 2, img, 2, 2, Rect::new(2.0, 0.0, 0.0, 2.0)), + ("the image has a NaN width", 16, 2, 2, img, 2, 2, Rect::new(0.0, 0.0, f64::NAN, 2.0)), + ("the image has a NaN height", 16, 2, 2, img, 2, 2, Rect::new(0.0, 0.0, 2.0, f64::NAN)), + ]; + let failures: Vec<_> = cases + .iter() + .filter(|&&(_, len, w, h, image, cols, rows, rect)| compose(len, w, h, image, cols, rows, rect) != vec![7; len]) + .map(|c| c.0) + .collect(); + assert!(failures.is_empty(), "{failures:?}"); +} + #[test] fn blend_mode_names() { use vectorcraft_doc::color::BlendMode;