Keep the test scene's own arithmetic from overflowing a u64

The first compile this branch ever had. `cargo fmt` reflowed four files
and clippy passed at -D warnings untouched, but one test panicked:
`the_signature_does_not_change_with_scale`, on "attempt to multiply with
overflow".

It is the fixture, not the feature. `scene()`'s little LCG multiplied the
block's y by the golden-ratio constant with a plain `*` while the term
beside it already used `wrapping_mul`, so any scene taller than about 104
pixels overflowed in debug. Only the scale test builds one that large,
which is why 345 of 346 passed around it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-29 23:20:26 +02:00
co-authored by Claude Opus 5
parent e5db849f04
commit 55cb9b5b30
4 changed files with 23 additions and 20 deletions
+18 -9
View File
@@ -609,7 +609,9 @@ pub fn members(conn: &Connection, burst: ImageId) -> Result<Vec<ImageId>, Catalo
WHERE bm.burst_id = ?1 WHERE bm.burst_id = ?1
ORDER BY i.captured_at ASC, i.id ASC", ORDER BY i.captured_at ASC, i.id ASC",
)?; )?;
let rows = stmt.query_map([burst.0 as i64], |r| Ok(ImageId(r.get::<_, i64>(0)? as u64)))?; let rows = stmt.query_map([burst.0 as i64], |r| {
Ok(ImageId(r.get::<_, i64>(0)? as u64))
})?;
Ok(rows.collect::<Result<Vec<_>, _>>()?) Ok(rows.collect::<Result<Vec<_>, _>>()?)
} }
@@ -754,7 +756,7 @@ mod tests {
let mut s = seed let mut s = seed
.wrapping_add(bx as u64) .wrapping_add(bx as u64)
.wrapping_mul(6_364_136_223_846_793_005) .wrapping_mul(6_364_136_223_846_793_005)
.wrapping_add(by as u64 * 1_442_695_040_888_963_407); .wrapping_add((by as u64).wrapping_mul(1_442_695_040_888_963_407));
s ^= s >> 33; s ^= s >> 33;
let v = (s % 256) as i32 + brighter; let v = (s % 256) as i32 + brighter;
out[y * w + x] = v.clamp(0, 255) as u8; out[y * w + x] = v.clamp(0, 255) as u8;
@@ -836,7 +838,10 @@ mod tests {
// The top bit is the one at risk: SQLite integers are signed. // The top bit is the one at risk: SQLite integers are signed.
let s = Signature(u64::MAX); let s = Signature(u64::MAX);
assert_eq!(Signature::from_stored(s.to_stored()), s); assert_eq!(Signature::from_stored(s.to_stored()), s);
assert_eq!(Signature::from_stored(Signature(0).to_stored()), Signature(0)); assert_eq!(
Signature::from_stored(Signature(0).to_stored()),
Signature(0)
);
} }
// ── the grouping ────────────────────────────────────────────────────── // ── the grouping ──────────────────────────────────────────────────────
@@ -844,7 +849,11 @@ mod tests {
#[test] #[test]
fn frames_a_second_apart_and_alike_are_one_burst() { fn frames_a_second_apart_and_alike_are_one_burst() {
let g = group( let g = group(
&[frame(1, 1000, 0xFF00), frame(2, 1001, 0xFF00), frame(3, 1001, 0xFF01)], &[
frame(1, 1000, 0xFF00),
frame(2, 1001, 0xFF00),
frame(3, 1001, 0xFF01),
],
Rules::default(), Rules::default(),
); );
assert_eq!(g, vec![vec![ImageId(1), ImageId(2), ImageId(3)]]); assert_eq!(g, vec![vec![ImageId(1), ImageId(2), ImageId(3)]]);
@@ -866,10 +875,7 @@ mod tests {
); );
assert_eq!( assert_eq!(
g, g,
vec![ vec![vec![ImageId(1), ImageId(2)], vec![ImageId(3), ImageId(4)],],
vec![ImageId(1), ImageId(2)],
vec![ImageId(3), ImageId(4)],
],
"the three-second pause did not end the first burst" "the three-second pause did not end the first burst"
); );
} }
@@ -1109,7 +1115,10 @@ mod tests {
choose_representative(cat.connection(), ImageId(2)).unwrap(); choose_representative(cat.connection(), ImageId(2)).unwrap();
let m = memberships(cat.connection(), &[ImageId(2)]).unwrap(); let m = memberships(cat.connection(), &[ImageId(2)]).unwrap();
assert!(m[&ImageId(2)].representative, "the pick did not take effect"); assert!(
m[&ImageId(2)].representative,
"the pick did not take effect"
);
regroup(cat.connection(), Rules::default()).unwrap(); regroup(cat.connection(), Rules::default()).unwrap();
let m = memberships(cat.connection(), &[ImageId(1), ImageId(2)]).unwrap(); let m = memberships(cat.connection(), &[ImageId(1), ImageId(2)]).unwrap();
+1 -5
View File
@@ -179,11 +179,7 @@ thread_local! {
/// Deliberately silent otherwise. The sweeps around it report progress because /// Deliberately silent otherwise. The sweeps around it report progress because
/// they run for tens of minutes; this is seconds, and a status line for it would /// they run for tens of minutes; this is seconds, and a status line for it would
/// be a line the user must read in order to learn nothing. /// be a line the user must read in order to learn nothing.
pub fn start_pass( pub fn start_pass(catalog_path: PathBuf, thumbs_dir: PathBuf, grouped: impl Fn(usize) + 'static) {
catalog_path: PathBuf,
thumbs_dir: PathBuf,
grouped: impl Fn(usize) + 'static,
) {
// A second pass would read the same rows and write the same answer over the // A second pass would read the same rows and write the same answer over the
// first one's transactions. // first one's transactions.
if RUNNING.get() { if RUNNING.get() {
+1 -5
View File
@@ -5066,11 +5066,7 @@ mod tests {
"UPDATE images SET captured_at = ?2, camera = 'Canon EOS R5', "UPDATE images SET captured_at = ?2, camera = 'Canon EOS R5',
perceptual_hash = ?3 perceptual_hash = ?3
WHERE id = ?1", WHERE id = ?1",
rusqlite::params![ rusqlite::params![id.0 as i64, 1_000 + n as i64, Signature(hash).to_stored()],
id.0 as i64,
1_000 + n as i64,
Signature(hash).to_stored()
],
) )
.unwrap(); .unwrap();
} }
+3 -1
View File
@@ -3790,7 +3790,9 @@ fn start_thumbnail_sweep(window: &AppWindow, ctl: &Rc<LibraryController>) {
library::catalog_path(&conn.account), library::catalog_path(&conn.account),
library::thumbs_dir(&conn.account), library::thumbs_dir(&conn.account),
move |bursts| { move |bursts| {
let Some(w) = weak_after.upgrade() else { return }; let Some(w) = weak_after.upgrade() else {
return;
};
if bursts > 0 && w.get_show_library() { if bursts > 0 && w.get_show_library() {
schedule_reload(&w, &ctl_after); schedule_reload(&w, &ctl_after);
} }