Skip to content

Commit 9ab8513

Browse files
authored
vello_cpu: Fix clippy::cast_possible_wrap in Gaussian blur (#1364)
Fixed by adding a reasonable limitation on `MAX_KERNEL_SIZE`, and storing the kernel size as a `u8` instead of `usize`. The lint itself checks for possible wrapping when going from an unsigned to signed integer, and is not currently in our lint set (hence not triggered on CI). It triggers when manually requested. It probably should be included in the lint set, but that's for another time (linebender/linebender.github.io#135).
1 parent ac0b716 commit 9ab8513

2 files changed

Lines changed: 48 additions & 27 deletions

File tree

sparse_strips/vello_cpu/src/filter/drop_shadow.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ pub(crate) struct DropShadow {
3838
/// Only the first `kernel_size` elements are valid.
3939
kernel: [f32; MAX_KERNEL_SIZE],
4040
/// Actual length of the kernel (kernel is padded to `MAX_KERNEL_SIZE`).
41-
kernel_size: usize,
41+
kernel_size: u8,
4242
}
4343

4444
impl DropShadow {
@@ -76,7 +76,7 @@ impl FilterEffect for DropShadow {
7676
self.dy,
7777
self.std_deviation,
7878
self.n_decimations,
79-
&self.kernel[..self.kernel_size],
79+
&self.kernel[..usize::from(self.kernel_size)],
8080
self.color,
8181
self.edge_mode,
8282
layer_manager,

sparse_strips/vello_cpu/src/filter/gaussian_blur.rs

Lines changed: 46 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,23 @@ use vello_common::peniko::color::PremulRgba8;
2828
use vello_common::peniko::kurbo::common::FloatFuncs as _;
2929
use vello_common::pixmap::Pixmap;
3030

31-
/// Maximum size of the Gaussian kernel (must be odd).
31+
/// Maximum size of the Gaussian kernel (must be odd and equal to or smaller than [`u8::MAX`]).
32+
///
3233
/// The multi-scale decimation algorithm guarantees that kernel size never exceeds this value.
3334
/// Decimation stops when remaining variance ≤ 4.0 (σ ≤ 2.0), which produces kernels of size
3435
/// at most 13 (radius = ceil(3σ) = 6, size = 1 + 2×6 = 13).
3536
pub(crate) const MAX_KERNEL_SIZE: usize = 13;
3637

38+
#[cfg(test)]
39+
const _: () = const {
40+
if MAX_KERNEL_SIZE.is_multiple_of(2) {
41+
panic!("`MAX_KERNEL_SIZE` must be odd");
42+
}
43+
if MAX_KERNEL_SIZE > u8::MAX as usize {
44+
panic!("`MAX_KERNEL_SIZE` must be less than or equal to `u8::MAX`");
45+
}
46+
};
47+
3748
pub(crate) struct GaussianBlur {
3849
std_deviation: f32,
3950
/// Number of 2× decimation levels to use (0 means no decimation, direct convolution).
@@ -42,7 +53,7 @@ pub(crate) struct GaussianBlur {
4253
/// Only the first `kernel_size` elements are valid.
4354
kernel: [f32; MAX_KERNEL_SIZE],
4455
/// Actual length of the kernel (rest is padding up to `MAX_KERNEL_SIZE`).
45-
kernel_size: usize,
56+
kernel_size: u8,
4657
/// Edge mode for handling out-of-bounds sampling.
4758
edge_mode: EdgeMode,
4859
}
@@ -76,7 +87,7 @@ impl FilterEffect for GaussianBlur {
7687
pixmap,
7788
scratch,
7889
self.n_decimations,
79-
&self.kernel[..self.kernel_size],
90+
&self.kernel[..usize::from(self.kernel_size)],
8091
self.edge_mode,
8192
);
8293
}
@@ -94,7 +105,7 @@ impl FilterEffect for GaussianBlur {
94105
/// - `n_decimations`: Number of 2× downsampling steps to perform (per axis)
95106
/// - `kernel`: Pre-computed Gaussian kernel weights (fixed-size array)
96107
/// - `kernel_size`: Actual length of the kernel (rest is zero-padded)
97-
pub(crate) fn plan_decimated_blur(std_deviation: f32) -> (usize, [f32; MAX_KERNEL_SIZE], usize) {
108+
pub(crate) fn plan_decimated_blur(std_deviation: f32) -> (usize, [f32; MAX_KERNEL_SIZE], u8) {
98109
if std_deviation <= 0.0 {
99110
// Invalid standard deviation, return identity kernel (no blur)
100111
let mut kernel = [0.0; MAX_KERNEL_SIZE];
@@ -135,19 +146,19 @@ pub(crate) fn plan_decimated_blur(std_deviation: f32) -> (usize, [f32; MAX_KERNE
135146
/// Returns (`kernel_weights`, `kernel_size`) where `kernel_size = 2×radius + 1`.
136147
/// The kernel is stored in a fixed-size array to avoid heap allocation.
137148
/// Uses the standard Gaussian formula: G(x) = exp(-x² / (2σ²)), normalized to sum to 1.
138-
pub(crate) fn compute_gaussian_kernel(std_deviation: f32) -> ([f32; MAX_KERNEL_SIZE], usize) {
149+
pub(crate) fn compute_gaussian_kernel(std_deviation: f32) -> ([f32; MAX_KERNEL_SIZE], u8) {
139150
// Use radius = 3σ to capture 99.7% of the Gaussian distribution.
140151
// Beyond ±3σ, the Gaussian values are negligible (<0.3%).
141152
let radius = (3.0 * std_deviation).ceil() as usize;
142-
let kernel_size = (1 + radius * 2).min(MAX_KERNEL_SIZE);
153+
let kernel_size = (1 + radius * 2).min(MAX_KERNEL_SIZE) as u8;
143154

144155
let mut kernel = [0.0; MAX_KERNEL_SIZE];
145156
// Compute Gaussian weights using the formula: G(x) = exp(-x² / (2σ²))
146157
// This creates a symmetric bell curve centered at the middle of the kernel.
147158
let gaussian_denominator = 2.0 * std_deviation * std_deviation;
148159
let mut sum = 0.0;
149160
let kernel_center = (kernel_size / 2) as f32;
150-
for (i, weight) in kernel.iter_mut().enumerate().take(kernel_size) {
161+
for (i, weight) in kernel.iter_mut().enumerate().take(usize::from(kernel_size)) {
151162
// Compute distance from center (0 at center, increases outward)
152163
let x = (i as f32) - kernel_center;
153164
// Apply Gaussian formula: weight decreases exponentially with squared distance
@@ -158,7 +169,7 @@ pub(crate) fn compute_gaussian_kernel(std_deviation: f32) -> ([f32; MAX_KERNEL_S
158169
// Normalize weights to sum to 1.0, ensuring the blur doesn't change overall brightness.
159170
// Without normalization, blurring a uniform gray area could make it brighter/darker.
160171
let scale = 1.0 / sum;
161-
for weight in kernel.iter_mut().take(kernel_size) {
172+
for weight in kernel.iter_mut().take(usize::from(kernel_size)) {
162173
*weight *= scale;
163174
}
164175

@@ -180,7 +191,7 @@ pub(crate) fn apply_blur(
180191
kernel: &[f32],
181192
edge_mode: EdgeMode,
182193
) {
183-
let radius = kernel.len() / 2;
194+
let radius = (kernel.len() / 2) as u8;
184195
let width = pixmap.width();
185196
let height = pixmap.height();
186197

@@ -235,7 +246,7 @@ pub(crate) fn convolve(
235246
width: u16,
236247
height: u16,
237248
kernel: &[f32],
238-
radius: usize,
249+
radius: u8,
239250
edge_mode: EdgeMode,
240251
) {
241252
convolve_x(src, scratch, width, height, kernel, radius, edge_mode);
@@ -253,7 +264,7 @@ pub(crate) fn convolve_x(
253264
src_width: u16,
254265
src_height: u16,
255266
kernel: &[f32],
256-
radius: usize,
267+
radius: u8,
257268
edge_mode: EdgeMode,
258269
) {
259270
for y in 0..src_height {
@@ -262,7 +273,12 @@ pub(crate) fn convolve_x(
262273

263274
// Sum contributions from all kernel positions: output = Σ(weight[j] × pixel[x+j-radius])
264275
for (j, &k) in kernel.iter().enumerate() {
265-
let src_x = x as i32 + j as i32 - radius as i32;
276+
#[expect(
277+
clippy::cast_possible_wrap,
278+
reason = "This cast never wraps because `kernel.len()` is never greater than `u8::MAX` due to the restriction on `MAX_KERNEL_SIZE`"
279+
)]
280+
let j = j as i32;
281+
let src_x = x as i32 + j - radius as i32;
266282
let p = sample_x(src, src_x, y, src_width, edge_mode);
267283

268284
rgba[0] += p.r as f32 * k;
@@ -297,7 +313,7 @@ pub(crate) fn convolve_y(
297313
src_width: u16,
298314
src_height: u16,
299315
kernel: &[f32],
300-
radius: usize,
316+
radius: u8,
301317
edge_mode: EdgeMode,
302318
) {
303319
for y in 0..src_height {
@@ -306,7 +322,12 @@ pub(crate) fn convolve_y(
306322

307323
// Sum contributions from all kernel positions: output = Σ(weight[j] × pixel[y+j-radius])
308324
for (j, &k) in kernel.iter().enumerate() {
309-
let src_y = y as i32 + j as i32 - radius as i32;
325+
#[expect(
326+
clippy::cast_possible_wrap,
327+
reason = "This cast never wraps because `kernel.len()` is never greater than `u8::MAX` due to the restriction on `MAX_KERNEL_SIZE`"
328+
)]
329+
let j = j as i32;
330+
let src_y = y as i32 + j - radius as i32;
310331
let p = sample_y(src, x, src_y, src_height, edge_mode);
311332

312333
rgba[0] += p.r as f32 * k;
@@ -647,18 +668,18 @@ mod tests {
647668

648669
// Kernel should be symmetric
649670
for i in 0..size / 2 {
650-
assert!((kernel[i] - kernel[size - 1 - i]).abs() < 1e-6);
671+
assert!((kernel[usize::from(i)] - kernel[usize::from(size - 1 - i)]).abs() < 1e-6);
651672
}
652673

653674
// Kernel should sum to 1.0 (normalized)
654-
let sum: f32 = kernel.iter().take(size).sum();
675+
let sum: f32 = kernel.iter().take(usize::from(size)).sum();
655676
assert!((sum - 1.0).abs() < 1e-6);
656677

657678
// Center should be the largest weight
658679
let center_idx = size / 2;
659680
for i in 0..size {
660681
if i != center_idx {
661-
assert!(kernel[center_idx] >= kernel[i]);
682+
assert!(kernel[usize::from(center_idx)] >= kernel[usize::from(i)]);
662683
}
663684
}
664685
}
@@ -670,7 +691,7 @@ mod tests {
670691
// For σ=0.1, radius = ceil(0.3) = 1, size = 3
671692
assert_eq!(size, 3);
672693
// Should sum to 1.0
673-
let sum: f32 = kernel.iter().take(size).sum();
694+
let sum: f32 = kernel.iter().take(usize::from(size)).sum();
674695
assert!((sum - 1.0).abs() < 1e-6);
675696
// Center weight should be dominant for very small σ
676697
assert!(kernel[1] > 0.9); // Center is highly weighted
@@ -684,7 +705,7 @@ mod tests {
684705
assert_eq!(size, 5);
685706

686707
// Should still sum to 1.0
687-
let sum: f32 = kernel.iter().take(size).sum();
708+
let sum: f32 = kernel.iter().take(usize::from(size)).sum();
688709
assert!((sum - 1.0).abs() < 1e-6);
689710
}
690711

@@ -934,7 +955,7 @@ mod tests {
934955
&mut pixmap,
935956
&mut scratch,
936957
n_decimations,
937-
&kernel[..kernel_size],
958+
&kernel[..usize::from(kernel_size)],
938959
EdgeMode::None,
939960
);
940961
});
@@ -997,7 +1018,7 @@ mod tests {
9971018
&mut dst,
9981019
5,
9991020
3,
1000-
&kernel[..kernel_size],
1021+
&kernel[..usize::from(kernel_size)],
10011022
kernel_size / 2,
10021023
EdgeMode::Duplicate,
10031024
);
@@ -1038,7 +1059,7 @@ mod tests {
10381059
&mut dst,
10391060
3,
10401061
5,
1041-
&kernel[..kernel_size],
1062+
&kernel[..usize::from(kernel_size)],
10421063
kernel_size / 2,
10431064
EdgeMode::Duplicate,
10441065
);
@@ -1089,10 +1110,10 @@ mod tests {
10891110
let (kernel, kernel_size) = compute_gaussian_kernel(100.0);
10901111
// For σ=100, radius = ceil(300) = 300, size would be 601
10911112
// But it should be clamped to MAX_KERNEL_SIZE
1092-
assert_eq!(kernel_size, MAX_KERNEL_SIZE);
1113+
assert_eq!(usize::from(kernel_size), MAX_KERNEL_SIZE);
10931114

10941115
// The clamped kernel should still sum to 1.0 (normalized)
1095-
let sum: f32 = kernel.iter().take(kernel_size).sum();
1116+
let sum: f32 = kernel.iter().take(usize::from(kernel_size)).sum();
10961117
assert!((sum - 1.0).abs() < 1e-6);
10971118
}
10981119

@@ -1205,7 +1226,7 @@ mod tests {
12051226
fn test_kernel_normalization_precision() {
12061227
for sigma in [0.1, 0.5, 1.0, 2.0, 5.0, 10.0] {
12071228
let (kernel, kernel_size) = compute_gaussian_kernel(sigma);
1208-
let sum: f32 = kernel.iter().take(kernel_size).sum();
1229+
let sum: f32 = kernel.iter().take(usize::from(kernel_size)).sum();
12091230
assert!(
12101231
(sum - 1.0).abs() < 1e-6,
12111232
"Kernel for σ={} not normalized: sum={}",

0 commit comments

Comments
 (0)