From 7e92c28b23c759c3f76fed7e677845ba3fb7551f Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 31 Oct 2026 08:05:02 +0000 Subject: [PATCH 01/11] Reject shutter fractions with a non-positive numerator or denominator Found while testing crates/catalog/src/model.rs:160 (parse_shutter_seconds) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/catalog/src/model.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/catalog/src/model.rs b/crates/catalog/src/model.rs index eebc9b3e45..ed1c190c59 100644 --- a/crates/catalog/src/model.rs +++ b/crates/catalog/src/model.rs @@ -157,7 +157,10 @@ impl CopyrightStatus { pub fn parse_shutter_seconds(s: &str) -> Option { let s = s.trim().trim_end_matches(['s', '"']).trim(); match s.split_once('/') { - Some((n, d)) => Some(n.trim().parse::().ok()? / d.trim().parse::().ok()?), + Some((n, d)) => { + let (n, d) = (n.trim().parse::().ok()?, d.trim().parse::().ok()?); + if n > 0.0 && d > 0.0 { Some(n / d) } else { None } + } None => s.parse().ok(), } .filter(|v: &f64| v.is_finite() && *v > 0.0) @@ -716,3 +719,16 @@ mod edited_tests { } } } + +#[cfg(test)] +mod shutter_tests { + use super::parse_shutter_seconds; + + #[test] + fn a_fraction_needs_a_positive_numerator_and_denominator() { + for s in ["-1/-250", "1/-250", "-1/250", "0/250"] { + assert_eq!(parse_shutter_seconds(s), None, "{s}"); + } + assert_eq!(parse_shutter_seconds("1/250"), Some(0.004)); + } +} From 9d4f33a77cb912b606276050c3de2be142f7ce0e Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:25:18 +0000 Subject: [PATCH 02/11] Match library search date: token case-insensitively Found while testing crates/catalog/src/query.rs:178 (token_matches, the "date" branch at :215) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/catalog/src/query.rs | 2 +- crates/catalog/src/tests.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index 7b5ad5ecd0..efa62ea6d8 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -226,7 +226,7 @@ fn token_matches(p: &Photo, tok: &str) -> bool { "person" | "who" => has_person(p, val), "type" | "kind" => format!("{:?}", p.kind).eq_ignore_ascii_case(val), "edited" => (val == "true" || val == "yes") == p.is_edited(), - "date" => p.captured.as_deref().is_some_and(|c| c.starts_with(val)), + "date" => p.captured.as_deref().is_some_and(|c| c.get(..val.len()).is_some_and(|h| h.eq_ignore_ascii_case(val))), "copy" | "virtual" => (val == "true" || val == "yes") == p.copy_of.is_some(), "name" | "file" => p.file_name.to_lowercase().contains(val), // Not a known field: the colon is part of a plain word ("10:30", "Re:"), its value unquoted diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index 0b31ca9750..6813d0a917 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -12,6 +12,17 @@ fn photo(c: &mut Catalog, name: &str, date: &str) -> PhotoId { id } +#[test] +fn date_token_matches_time_part_case_insensitively() { + let mut c = Catalog::new(); + let a = photo(&mut c, "a.jpg", "2026-05-01T10:00:00"); + let q = |c: &Catalog, text: &str| c.query(&Filter { text: text.into(), ..Default::default() }, &Sort::default()); + assert_eq!(q(&c, "date:2026-05-01T10"), vec![a]); + assert_eq!(q(&c, "date:2026-05-01t10"), vec![a]); + assert_eq!(q(&c, "date:2026-05"), vec![a]); + assert!(q(&c, "date:2026-05-01T11").is_empty()); +} + #[test] fn apply_and_inverse_roundtrip() { let mut c = Catalog::new(); From 4cb62dcfaa2e23b6e82eb0a4f24938a6e417911b Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:00:19 +0000 Subject: [PATCH 03/11] Reject overflowing LIGHTCRAFT_GPU_LIMITS values instead of panicking Found while testing crates/gpu/src/ctx.rs:768 (parse_limits) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/gpu/src/ctx.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/gpu/src/ctx.rs b/crates/gpu/src/ctx.rs index d3e56f7491..1cd7a14c96 100644 --- a/crates/gpu/src/ctx.rs +++ b/crates/gpu/src/ctx.rs @@ -764,8 +764,8 @@ fn parse_limits(v: &str) -> Option<(u64, u64)> { } n => { let mb: u64 = n.trim_end_matches("mib").trim_end_matches("mb").trim().parse().ok()?; - let b = mb.max(1) << 20; - Some((b, 2 * b)) + let b = mb.max(1).checked_mul(1 << 20)?; + Some((b, b.checked_mul(2)?)) } } } @@ -906,6 +906,8 @@ mod tests { assert_eq!(super::parse_limits("512MiB"), Some((512 << 20, 1024 << 20))); assert_eq!(super::parse_limits(""), None); assert_eq!(super::parse_limits("lots"), None); + assert_eq!(super::parse_limits("9223372036854775807"), None); + assert_eq!(super::parse_limits("17592186044416"), None); } /// Issue #2936: the kernels are built on several threads; every entry point of every module has one. From fc99d212f6216d4126846c26753ecb9b65c5bef6 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Sun, 11 Oct 2026 12:42:29 -0500 Subject: [PATCH 04/11] test(gpu): #1050 opt-in software adapter for the equivalence tests `Gpu::new` refuses software adapters (llvmpipe / lavapipe, WARP): the CPU pipeline is faster. CI runners have nothing else, so the CPU <-> GPU equivalence tests have always skipped there. `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` admits them. The app never sets it and the default is unchanged. On lavapipe all 134 compared renders stay within max 1 LSB, mean <= 0.0003 LSB, the same as Metal on an M4 Pro. The two big-source window tests used a 24 MP (288 MB) source. That is over lavapipe's 128 MiB storage binding (the WebGPU default), so geometry ran on the CPU and nothing was uploaded: one test failed and the other passed without checking anything. They now use 3200 x 3200 (117 MiB): above RETAIN_SOURCE_BYTES, so the source is still shared, and uploaded on every adapter. Co-Authored-By: Claude Opus 5.5 --- crates/gpu/src/ctx.rs | 33 ++++++++++++++++++++++++++++-- crates/gpu/tests/it/equivalence.rs | 30 +++++++++++++++++---------- docs/gpu-pipeline.md | 13 ++++++++++++ 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/crates/gpu/src/ctx.rs b/crates/gpu/src/ctx.rs index d3e56f7491..73d7dc3663 100644 --- a/crates/gpu/src/ctx.rs +++ b/crates/gpu/src/ctx.rs @@ -386,7 +386,8 @@ fn module_source(m: &Module, consts: &str) -> String { } impl Gpu { - /// Create a device on the best available adapter (no software fallback), or why there is none. + /// Create a device on the best available adapter (no software adapter unless + /// `LIGHTCRAFT_GPU_ALLOW_SOFTWARE`), or why there is none. pub fn new(backends: wgpu::Backends) -> Result { let mut desc = wgpu::InstanceDescriptor::new_without_display_handle(); // only these drivers are loaded (`crate::backend`: never Vulkan on Windows unless asked) @@ -401,10 +402,13 @@ impl Gpu { ) .map_err(|e| format!("no GPU adapter among {backends:?} ({e})"))?; let info = adapter.get_info(); - if info.device_type == wgpu::DeviceType::Cpu { + if refuse_adapter(info.device_type, software_allowed()) { log::info!("gpu: only a software adapter ({}), using the CPU pipeline", info.name); return Err(format!("software adapter ({}) skipped: the CPU pipeline is faster", info.name)); } + if info.device_type == wgpu::DeviceType::Cpu { + log::info!("gpu: LIGHTCRAFT_GPU_ALLOW_SOFTWARE: using the software adapter {}", info.name); + } let mut limits = adapter.limits(); if limits.max_storage_buffers_per_shader_stage < 10 { log::info!("gpu: {} has too few storage buffers per stage", info.name); @@ -747,6 +751,20 @@ impl Gpu { /// normally reset a hung GPU much sooner). const READBACK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); +/// Whether to turn down an adapter of `device_type`: a software rasterizer (llvmpipe / lavapipe, +/// WARP) is slower than the CPU pipeline, so it is refused unless `allow_software`. +fn refuse_adapter(device_type: wgpu::DeviceType, allow_software: bool) -> bool { + device_type == wgpu::DeviceType::Cpu && !allow_software +} + +/// `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1`: accept a software adapter. For tests on machines without a +/// GPU (CI runners: Mesa lavapipe on Linux, WARP on Windows), so the CPU ↔ GPU equivalence tests +/// run there instead of skipping; the app never sets it (software rendering is slower than the CPU +/// pipeline). +fn software_allowed() -> bool { + std::env::var("LIGHTCRAFT_GPU_ALLOW_SOFTWARE").is_ok_and(|v| matches!(v.trim().to_ascii_lowercase().as_str(), "1" | "true" | "yes" | "on")) +} + /// `LIGHTCRAFT_GPU_LIMITS`: lower the device limits, e.g. to reproduce a smaller adapter. /// `webgpu` / `downlevel` = 128 MiB storage bindings and 256 MiB buffers (the WebGPU defaults), /// `` = n MiB storage bindings and 2n MiB buffers. Returns (binding bytes, buffer bytes). @@ -1151,6 +1169,17 @@ mod tests { println!("CHILD: survived"); } + #[test] + fn software_adapters_need_the_opt_in() { + use wgpu::DeviceType::{Cpu, DiscreteGpu, IntegratedGpu, Other, VirtualGpu}; + assert!(super::refuse_adapter(Cpu, false), "llvmpipe / WARP are refused by default"); + assert!(!super::refuse_adapter(Cpu, true), "LIGHTCRAFT_GPU_ALLOW_SOFTWARE accepts them"); + for t in [DiscreteGpu, IntegratedGpu, VirtualGpu, Other] { + assert!(!super::refuse_adapter(t, false), "{t:?}"); + assert!(!super::refuse_adapter(t, true), "{t:?}"); + } + } + #[test] fn dispatches_stay_within_the_workgroup_limit() { // 1-D kernels over a 100 MP image and 2-D ones over 16k × 16k stay under 65535 per axis diff --git a/crates/gpu/tests/it/equivalence.rs b/crates/gpu/tests/it/equivalence.rs index 7432ebb7c8..b043152fc1 100644 --- a/crates/gpu/tests/it/equivalence.rs +++ b/crates/gpu/tests/it/equivalence.rs @@ -2,7 +2,8 @@ //! `lightcraft_gpu::render`, compared in 8-bit sRGB. //! //! Bounds (documented in `docs/gpu-pipeline.md`): mean |Δ| < 0.5 LSB and max |Δ| ≤ 3 LSB per -//! channel for every case. Skips (passes with a note) when no GPU adapter exists, e.g. on CI. +//! channel for every case. Skips (passes with a note) when no GPU adapter exists; CI runs them on +//! software adapters with `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` (lavapipe, WARP) and on Metal. use std::sync::Arc; @@ -889,13 +890,20 @@ fn windows_match() { check("window: vignette corner", &src, &info, &s, &corner); } -/// Issue #323: a window of a 24 MP photo is cut from a source of 288 MB on the device. Keeping that -/// upload with the view's stages counted it against the stage budget on every tick of a drag and -/// got the stages cleared, and the upload and resampling redone: the window's stages are its own -/// pixels, not the photo's. +/// Issue #323: a window of a big photo is cut from the whole source on the device (288 MB for 24 MP). +/// Keeping that upload with the view's stages counted it against the stage budget on every tick of +/// a drag and got the stages cleared, and the upload and resampling redone: the window's stages are +/// its own pixels, not the photo's. // (these two share the process's one big source: a test lock keeps them from replacing each other's) static BIG_SOURCE_TESTS: std::sync::Mutex<()> = std::sync::Mutex::new(()); +/// The big photo of these tests: 10.24 MP, a 117 MiB source. That is over what a view keeps with its +/// stages (96 MiB, so it is shared) and within a 128 MiB storage binding (the WebGPU default, and +/// the limit of Mesa's lavapipe in CI), so the source goes to the device on every adapter. A source +/// over the device's limit is resampled on the CPU instead and never uploaded, which would leave +/// these tests nothing to check. +const BIG: usize = 3200; + #[test] fn a_window_does_not_keep_the_whole_source_on_the_device() { let _lock = BIG_SOURCE_TESTS.lock().unwrap_or_else(|e| e.into_inner()); @@ -903,11 +911,11 @@ fn a_window_does_not_keep_the_whole_source_on_the_device() { return; } use lightcraft_pipeline::PixelWindow; - let src = scene(1, 6000, 4000); + let src = scene(1, BIG, BIG); let info = SourceInfo { raw: true, ..Default::default() }; let stages = StageCache::default(); let s = DevelopSettings::default(); - let req = RenderRequest { window: Some(PixelWindow { x: 2000, y: 1500, w: 640, h: 480 }), ..RenderRequest::fit(6000, 4000) }; + let req = RenderRequest { window: Some(PixelWindow { x: 2000, y: 1500, w: 640, h: 480 }), ..RenderRequest::fit(BIG, BIG) }; let _ = lightcraft_gpu::render(&src, &info, &s, &req, Some(&stages)).expect("gpu render"); let held = lightcraft_gpu::stage_bytes(&stages); assert!(held < 100 << 20, "the view's GPU stages hold {} MiB for a 640×480 window", held >> 20); @@ -919,7 +927,7 @@ fn a_window_does_not_keep_the_whole_source_on_the_device() { assert!(lightcraft_gpu::stage_bytes(&stages) < 100 << 20); } -/// …but the 288 MB is uploaded once for every window of every view of the photo, not once per +/// …but the source is uploaded once for every window of every view of the photo, not once per /// window: panning at 1:1, and the Before and After windows, share one device copy of it, which /// goes when the app idles (`trim_pool(0)`). #[test] @@ -929,17 +937,17 @@ fn windows_of_a_big_photo_share_one_uploaded_source() { return; } use lightcraft_pipeline::PixelWindow; - let src = scene(1, 6000, 4000); + let src = scene(1, BIG, BIG); let info = SourceInfo { raw: true, ..Default::default() }; let s = DevelopSettings::default(); let (after, before) = (StageCache::default(), StageCache::default()); - let at = |x: usize| RenderRequest { window: Some(PixelWindow { x, y: 1500, w: 640, h: 480 }), ..RenderRequest::fit(6000, 4000) }; + let at = |x: usize| RenderRequest { window: Some(PixelWindow { x, y: 1500, w: 640, h: 480 }), ..RenderRequest::fit(BIG, BIG) }; let start = lightcraft_gpu::source_uploads(); for (cache, x) in [(&after, 1000), (&after, 1256), (&after, 1512), (&before, 1512), (&before, 1768)] { lightcraft_gpu::render(&src, &info, &s, &at(x), Some(cache)).expect("gpu render"); } assert_eq!(lightcraft_gpu::source_uploads() - start, 1, "one upload for five windows of two views"); - assert!(lightcraft_gpu::shared_source_bytes() >= 280_000_000, "{}", lightcraft_gpu::shared_source_bytes()); + assert!(lightcraft_gpu::shared_source_bytes() >= BIG * BIG * 12, "{}", lightcraft_gpu::shared_source_bytes()); assert!(lightcraft_gpu::stage_bytes(&after) < 100 << 20, "and it is not in the views' stages"); lightcraft_gpu::trim_pool(0); assert_eq!(lightcraft_gpu::shared_source_bytes(), 0, "an idle app gives it back"); diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md index 8dceb96ca9..de246f4325 100644 --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -49,6 +49,9 @@ Overrides (read once at launch): - `LIGHTCRAFT_GPU=0`: GPU rendering off for the process (the window is unaffected). - Both `LIGHTCRAFT_GPU=0` and `LIGHTCRAFT_GPU_BACKEND=off` also keep AI Denoise, the face models and the AI mask model (SAM 3 on Metal) on the CPU. +- `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1`: accept a software adapter (llvmpipe / lavapipe, WARP), which is + otherwise refused because the CPU pipeline is faster. For tests on machines without a GPU (CI); + the app never sets it. **DX12 shader compiler (issue #471).** Every instance (window and compute) compiles DX12 shaders with FXC (`d3dcompiler_47.dll`, part of Windows; `lightcraft_gpu::backend::backend_options`). wgpu's @@ -272,6 +275,16 @@ bit-identical, and one (the point colour range overlay) has max 2 LSB on 0.0002% mean 0.0003 LSB. That is the same picture as on Metal, inside the bounds (`cargo test -p lightcraft-gpu --release --test it equivalence:: -- --nocapture`). +**In CI** (`.github/workflows/ci.yml`, issue #1050) they run on every platform: on Metal on the macOS +runners, and on software adapters on Linux (Mesa lavapipe) and Windows (WARP, DX12 with FXC), which +`LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` admits (tests only: the app never sets it). The step fails if they +skip. Measured on lavapipe (CI and locally) and WARP: all 134 compared renders within **max 1 LSB, +mean ≤ 0.0003 LSB**, as on the M4 Pro; on the macOS runner's (paravirtual) Metal device one case reaches +2 LSB (point colour range overlay, 0.0002% of pixels over 1), the rest max 1. The suite takes about +50 s on lavapipe, 30 s on the macOS runner and 2–7.5 min on WARP. lavapipe's +storage bindings stop at 128 MiB (the WebGPU default), so the two big-source tests use a 117 MiB +source: above what a view keeps with its stages (96 MiB) and small enough to be uploaded there. + Remaining differences come from f32 vs f64 coordinate math, fast-math transcendental functions on Metal and summation order in the box filters — all far below one 8-bit step. From 21a7d64d467d0dfd39e0344b02f07458201f3508 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Fri, 30 Oct 2026 01:01:58 -0500 Subject: [PATCH 05/11] docs(gpu): #1050 run the GPU tests locally on a software adapter docs/gpu-pipeline.md says how to run lightcraft-gpu's tests on a machine without a GPU: LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1 admits Mesa's lavapipe on Linux and WARP on Windows (Metal on macOS needs nothing), and LIGHTCRAFT_SKIP_LOG shows whether anything still skipped for want of an adapter. The comments on the variable and in equivalence.rs no longer say CI runs them: the platform CI legs of #1148 are not landing. Co-Authored-By: Claude Opus 5.5 --- crates/gpu/src/ctx.rs | 5 ++-- crates/gpu/tests/it/equivalence.rs | 6 ++--- docs/gpu-pipeline.md | 37 +++++++++++++++++++++--------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/crates/gpu/src/ctx.rs b/crates/gpu/src/ctx.rs index 73d7dc3663..9897faf5fc 100644 --- a/crates/gpu/src/ctx.rs +++ b/crates/gpu/src/ctx.rs @@ -758,9 +758,8 @@ fn refuse_adapter(device_type: wgpu::DeviceType, allow_software: bool) -> bool { } /// `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1`: accept a software adapter. For tests on machines without a -/// GPU (CI runners: Mesa lavapipe on Linux, WARP on Windows), so the CPU ↔ GPU equivalence tests -/// run there instead of skipping; the app never sets it (software rendering is slower than the CPU -/// pipeline). +/// GPU (Mesa lavapipe on Linux, WARP on Windows), so the CPU ↔ GPU equivalence tests run there +/// instead of skipping; the app never sets it (software rendering is slower than the CPU pipeline). fn software_allowed() -> bool { std::env::var("LIGHTCRAFT_GPU_ALLOW_SOFTWARE").is_ok_and(|v| matches!(v.trim().to_ascii_lowercase().as_str(), "1" | "true" | "yes" | "on")) } diff --git a/crates/gpu/tests/it/equivalence.rs b/crates/gpu/tests/it/equivalence.rs index b043152fc1..6a1a8464c4 100644 --- a/crates/gpu/tests/it/equivalence.rs +++ b/crates/gpu/tests/it/equivalence.rs @@ -2,8 +2,8 @@ //! `lightcraft_gpu::render`, compared in 8-bit sRGB. //! //! Bounds (documented in `docs/gpu-pipeline.md`): mean |Δ| < 0.5 LSB and max |Δ| ≤ 3 LSB per -//! channel for every case. Skips (passes with a note) when no GPU adapter exists; CI runs them on -//! software adapters with `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` (lavapipe, WARP) and on Metal. +//! channel for every case. Skips (passes with a note) when no GPU adapter exists; +//! `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` runs them on a software adapter (lavapipe, WARP) instead. use std::sync::Arc; @@ -899,7 +899,7 @@ static BIG_SOURCE_TESTS: std::sync::Mutex<()> = std::sync::Mutex::new(()); /// The big photo of these tests: 10.24 MP, a 117 MiB source. That is over what a view keeps with its /// stages (96 MiB, so it is shared) and within a 128 MiB storage binding (the WebGPU default, and -/// the limit of Mesa's lavapipe in CI), so the source goes to the device on every adapter. A source +/// the limit of Mesa's lavapipe), so the source goes to the device on every adapter. A source /// over the device's limit is resampled on the CPU instead and never uploaded, which would leave /// these tests nothing to check. const BIG: usize = 3200; diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md index de246f4325..736c208714 100644 --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -50,8 +50,8 @@ Overrides (read once at launch): - Both `LIGHTCRAFT_GPU=0` and `LIGHTCRAFT_GPU_BACKEND=off` also keep AI Denoise, the face models and the AI mask model (SAM 3 on Metal) on the CPU. - `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1`: accept a software adapter (llvmpipe / lavapipe, WARP), which is - otherwise refused because the CPU pipeline is faster. For tests on machines without a GPU (CI); - the app never sets it. + otherwise refused because the CPU pipeline is faster. For running the GPU tests on a machine + without a GPU (below); the app never sets it. **DX12 shader compiler (issue #471).** Every instance (window and compute) compiles DX12 shaders with FXC (`d3dcompiler_47.dll`, part of Windows; `lightcraft_gpu::backend::backend_options`). wgpu's @@ -275,15 +275,30 @@ bit-identical, and one (the point colour range overlay) has max 2 LSB on 0.0002% mean 0.0003 LSB. That is the same picture as on Metal, inside the bounds (`cargo test -p lightcraft-gpu --release --test it equivalence:: -- --nocapture`). -**In CI** (`.github/workflows/ci.yml`, issue #1050) they run on every platform: on Metal on the macOS -runners, and on software adapters on Linux (Mesa lavapipe) and Windows (WARP, DX12 with FXC), which -`LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` admits (tests only: the app never sets it). The step fails if they -skip. Measured on lavapipe (CI and locally) and WARP: all 134 compared renders within **max 1 LSB, -mean ≤ 0.0003 LSB**, as on the M4 Pro; on the macOS runner's (paravirtual) Metal device one case reaches -2 LSB (point colour range overlay, 0.0002% of pixels over 1), the rest max 1. The suite takes about -50 s on lavapipe, 30 s on the macOS runner and 2–7.5 min on WARP. lavapipe's -storage bindings stop at 128 MiB (the WebGPU default), so the two big-source tests use a 117 MiB -source: above what a view keeps with its stages (96 MiB) and small enough to be uploaded there. +**Running them without a GPU (issue #1050).** `Gpu::new` turns down software adapters, so on a +machine without a hardware GPU (a VM, a container) every GPU test skips. +`LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` lets them through for one run (tests only: the app never sets it): + +```sh +rm -f /tmp/gpu-skips.jsonl # the log is appended to; an absolute path, as tests run in crates/gpu +LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1 LIGHTCRAFT_SKIP_LOG=/tmp/gpu-skips.jsonl cargo test -p lightcraft-gpu --tests +grep '"reason":"no GPU adapter' /tmp/gpu-skips.jsonl # no output: nothing skipped for want of an adapter +``` + +- **Linux:** Mesa's lavapipe (Vulkan; `mesa-vulkan-drivers` on Debian and Ubuntu). Its storage + bindings stop at 128 MiB (the WebGPU default), so the two big-source tests use a 117 MiB source: + above what a view keeps with its stages (96 MiB) and small enough to be uploaded there. +- **Windows:** WARP, the DX12 software rasterizer that ships with Windows (shaders through FXC, as + above). +- **macOS:** Metal is a hardware adapter, so the tests run without the variable. + +A machine with a hardware GPU picks it over the software adapter, variable or not. Each test that +skips prints `skipped (gpu): …` and appends a line to the `LIGHTCRAFT_SKIP_LOG` file; with a software +adapter the only one left should be the device-loss test (`no GPU device, or one that can't be lost on +purpose here`). Measured 2026-10-11 on lavapipe and WARP, and again 2026-10-30 on lavapipe (Mesa's +llvmpipe on LLVM 20, 143 compared renders), every compared render stayed within **max 1 LSB, mean ≤ +0.0003 LSB**, as on the M4 Pro. The equivalence binary takes about 20 s on lavapipe and several +minutes on WARP. Remaining differences come from f32 vs f64 coordinate math, fast-math transcendental functions on Metal and summation order in the box filters — all far below one 8-bit step. From 15bacc76602ef7e8ffbe3a6f7227e942ebfe43c6 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Fri, 30 Oct 2026 12:45:13 -0500 Subject: [PATCH 06/11] docs(gpu): #1050 the skip-log check catches every GPU skip and logs to a private directory The check grepped for "no GPU adapter" only, so the ctx unit tests' "no GPU device" skips (and nn's "no GPU runner here") went unseen. It now lists every "need":"gpu" line except the expected device-loss one. On the 2026-10-30 lavapipe logs it shows all 24 GPU skips without the opt-in (the old grep showed 21) and nothing with it. The log goes in a new mktemp -d directory instead of a fixed /tmp name another local user could plant a symlink at. Co-Authored-By: Claude Opus 5.5 --- docs/gpu-pipeline.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md index 736c208714..cd7fd03e33 100644 --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -280,9 +280,9 @@ machine without a hardware GPU (a VM, a container) every GPU test skips. `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1` lets them through for one run (tests only: the app never sets it): ```sh -rm -f /tmp/gpu-skips.jsonl # the log is appended to; an absolute path, as tests run in crates/gpu -LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1 LIGHTCRAFT_SKIP_LOG=/tmp/gpu-skips.jsonl cargo test -p lightcraft-gpu --tests -grep '"reason":"no GPU adapter' /tmp/gpu-skips.jsonl # no output: nothing skipped for want of an adapter +log="$(mktemp -d)/gpu-skips.jsonl" # a new private directory; an absolute path, as tests run in crates/gpu +LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1 LIGHTCRAFT_SKIP_LOG="$log" cargo test -p lightcraft-gpu --tests +grep '"need":"gpu"' "$log" | grep -v 'lost on purpose' # no output: no GPU test skipped but the device-loss one ``` - **Linux:** Mesa's lavapipe (Vulkan; `mesa-vulkan-drivers` on Debian and Ubuntu). Its storage From 50879d4d2682aa856c29ce5f770163867d19bea9 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 1 Nov 2026 03:50:13 +0000 Subject: [PATCH 07/11] Fix Histogram::percentile returning black when the target truncates to 0 Found while testing crates/raster/src/histogram.rs:171 (Histogram::percentile) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/raster/src/histogram.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/raster/src/histogram.rs b/crates/raster/src/histogram.rs index 75667bd1c7..b7a2b0d78e 100644 --- a/crates/raster/src/histogram.rs +++ b/crates/raster/src/histogram.rs @@ -168,7 +168,10 @@ impl Histogram { /// Luma value (0..1) below which `q` of the pixels fall. pub fn percentile(&self, q: f32) -> f32 { - let target = (q.clamp(0.0, 1.0) * self.total as f32) as u32; + if self.total == 0 { + return 0.0; + } + let target = ((q.clamp(0.0, 1.0) * self.total as f32) as u32).max(1); let mut acc = 0; for (i, &c) in self.luma.iter().enumerate() { acc += c; @@ -199,6 +202,23 @@ mod tests { assert_eq!(h.above_sdr(), 0.0); } + #[test] + fn percentile_zero_skips_empty_bins() { + let img = Rgba8::from_fn(4, 4, |_, _| [200, 200, 200, 255]); + let h = Histogram::of_srgb8(&img); + // every pixel is in one bin, so every percentile is that bin's luma, never black + assert!(h.percentile(1.0) > 0.0); + assert_eq!(h.percentile(0.0), h.percentile(1.0)); + assert_eq!(h.percentile(0.01), h.percentile(1.0)); + } + + #[test] + fn percentile_of_empty_histogram_is_zero() { + let h = Histogram::of_hdr(&[], 4.0); + assert_eq!(h.percentile(0.0), 0.0); + assert_eq!(h.percentile(0.5), 0.0); + } + #[test] fn hdr_bins_split_sdr_and_stops() { let px = [[0.0, 0.0, 0.0], [1.0, 1.0, 1.0], [4.0, 4.0, 4.0], [16.0, 16.0, 16.0], [f32::NAN, -1.0, f32::INFINITY]]; From fe3b1813b11c8be0d928407267de89eec4d2e671 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Sat, 31 Oct 2026 23:25:02 -0500 Subject: [PATCH 08/11] docs(gpu): #1050 the skip check also expects the device-memory test, which a software adapter can't run out of #3419 added a test that holds device memory until an allocation fails; on lavapipe the allocations never fail, so it skips with the opt-in too. Co-Authored-By: Claude Opus 5.5 --- docs/gpu-pipeline.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md index cd7fd03e33..a35218c46d 100644 --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -282,7 +282,7 @@ machine without a hardware GPU (a VM, a container) every GPU test skips. ```sh log="$(mktemp -d)/gpu-skips.jsonl" # a new private directory; an absolute path, as tests run in crates/gpu LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1 LIGHTCRAFT_SKIP_LOG="$log" cargo test -p lightcraft-gpu --tests -grep '"need":"gpu"' "$log" | grep -v 'lost on purpose' # no output: no GPU test skipped but the device-loss one +grep '"need":"gpu"' "$log" | grep -v -e 'lost on purpose' -e 'memory never ran out' # no output: only the two below skipped ``` - **Linux:** Mesa's lavapipe (Vulkan; `mesa-vulkan-drivers` on Debian and Ubuntu). Its storage @@ -294,8 +294,9 @@ grep '"need":"gpu"' "$log" | grep -v 'lost on purpose' # no output: no GPU tes A machine with a hardware GPU picks it over the software adapter, variable or not. Each test that skips prints `skipped (gpu): …` and appends a line to the `LIGHTCRAFT_SKIP_LOG` file; with a software -adapter the only one left should be the device-loss test (`no GPU device, or one that can't be lost on -purpose here`). Measured 2026-10-11 on lavapipe and WARP, and again 2026-10-30 on lavapipe (Mesa's +adapter the only ones left should be the device-loss test (`no GPU device, or one that can't be lost on +purpose here`) and the device-memory test (`no GPU device, or its memory never ran out`: on lavapipe its +allocations never fail, seen 2026-10-31). Measured 2026-10-11 on lavapipe and WARP, and again 2026-10-30 on lavapipe (Mesa's llvmpipe on LLVM 20, 143 compared renders), every compared render stayed within **max 1 LSB, mean ≤ 0.0003 LSB**, as on the M4 Pro. The equivalence binary takes about 20 s on lavapipe and several minutes on WARP. From 3e3955bb3031791900c8da41c8ead73d0e1a4175 Mon Sep 17 00:00:00 2001 From: Fridtjof Michels Date: Sun, 1 Nov 2026 21:12:47 +0100 Subject: [PATCH 09/11] =?UTF-8?q?LR-VIEW-SURVEY:=20=E2=8C=98/Ctrl-click=20?= =?UTF-8?q?on=20a=20survey=20tile=20takes=20the=20photo=20out=20of=20the?= =?UTF-8?q?=20survey?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As in Lightroom Classic, a ⌘/Ctrl-click on a tile in Survey deselects that photo (library.select, mode toggle), as the hover × does; a plain click still only makes it active, and the last photo stays so the survey never empties. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/compare.rs | 7 +++++- crates/ui-egui/src/tests/compare.rs | 34 ++++++++++++++++++++++++++++ docs/parity-checklist.md | 2 +- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/compare.rs b/crates/ui-egui/src/panels/compare.rs index dad49923fa..b7034f3a6d 100644 --- a/crates/ui-egui/src/panels/compare.rs +++ b/crates/ui-egui/src/panels/compare.rs @@ -569,7 +569,12 @@ pub fn show_survey(app: &mut LightcraftApp, ui: &mut egui::Ui) { if app.session.selection.active == Some(*id) { app.image_rect = Some(img); } - if resp.clicked() { + // ⌘/Ctrl-click removes the photo from the survey (deselects it), as the × does + if resp.clicked() && ui.input(|i| i.modifiers.command) { + if photos.len() > 1 { + let _ = app.run("library.select", json!({"ids": [id.0], "mode": "toggle"})); + } + } else if resp.clicked() { app.session.selection.active = Some(*id); } if resp.double_clicked() { diff --git a/crates/ui-egui/src/tests/compare.rs b/crates/ui-egui/src/tests/compare.rs index b0ccb0b149..e4d8389749 100644 --- a/crates/ui-egui/src/tests/compare.rs +++ b/crates/ui-egui/src/tests/compare.rs @@ -359,3 +359,37 @@ fn survey_rings_the_active_tile_in_the_theme_colour() { .count(); assert_eq!(white, 0, "no white 2 pt frame"); } + +/// Survey with three photos selected and the second active; returns the three ids. +fn survey_of_three(h: &mut Headless) -> [u64; 3] { + let vis: Vec = h.app.session.visible_cloned().iter().map(|p| p.0).collect(); + let ids = [vis[0], vis[1], vis[2]]; + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": ids, "active": ids[1]}}), T); + let r = h.request("engine.execute", json!({"command": "view.survey"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + ids +} + +/// Move the pointer to `pos` and press and release the primary button there, with `modifiers`. +fn click_with(h: &mut Headless, pos: egui::Pos2, modifiers: egui::Modifiers) { + h.app.synthetic.push(egui::Event::PointerMoved(pos)); + h.settle(SETTLE); + h.app.synthetic.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: true, modifiers }); + h.app.synthetic.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: false, modifiers }); + h.settle(SETTLE); +} + +/// ⌘/Ctrl-click on a survey tile takes it out of the survey, as in Lightroom Classic; a plain +/// click only makes it active. +#[test] +fn survey_command_click_takes_the_photo_out() { + let mut h = demo(); + let [a, b, c] = survey_of_three(&mut h); + let img = h.app.image_rect.expect("the active tile is laid out"); + click_with(&mut h, img.center(), egui::Modifiers::NONE); + assert_eq!(h.app.session.selection.ids.len(), 3, "a plain click keeps the photo"); + click_with(&mut h, img.center(), egui::Modifiers::COMMAND); + let sel: Vec = h.app.session.selection.ids.iter().map(|p| p.0).collect(); + assert_eq!(sel, vec![a, c], "the ⌘/Ctrl-clicked photo left the survey (was {b})"); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 5e851e721d..46aabad737 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -134,7 +134,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-VIEW-NAVIGATOR | Navigator mini map | P1 | ✅ | `cmd:view.navigator`, `crates/ui-egui/src/panels/detail.rs` | shown while zoomed (bottom right); click/drag pans | | LR-VIEW-BEFOREAFTER | Before / after | P0 | ✅ | `cmd:view.showOriginal`, `cmd:view.beforeAfter`, `cmd:view.beforeAfterSplit`, `cmd:view.beforeAfterTopBottom`, `cmd:view.beforeAfterSplitTopBottom`, `cmd:beforeAfter.setBefore`, `cmd:beforeAfter.copyAfterToBefore`, `cmd:beforeAfter.copyBeforeToAfter`, `cmd:beforeAfter.swap`, `cmd:beforeAfter.resetBefore` | all four layouts; before = the import state (defaults + import preset) or a chosen history step / version / the current settings; copy and swap (View → Before/After Settings, History row menu). The chosen before lasts for the session; in side by side and top / bottom, clicks with the white-balance selector, colour samplers and targeted adjustment work in the Before view too (`crates/ui-egui/src/tests/before_after.rs`). Zoomed in, the side by side and stacked views each stay in their pane, with their labels under it. Click zoom animates both panes together, including cached window tiles; input follows the displayed image and render requests use the final size (#1821) | | LR-VIEW-COMPARE | Compare two photos | P1 | ✅ | `cmd:view.compare`, `cmd:compare.swap`, `cmd:compare.makeSelect`, `cmd:compare.set`, `crates/ui-egui/src/panels/compare.rs`, `crates/ui-egui/src/tests/compare.rs` | select / candidate, synced zoom + pan, arrows move the candidate; a filmstrip click puts the photo into the active side (a photo on the other side swaps), so the active photo is always one of the two shown; a ⌘/Ctrl- or Shift-click on the filmstrip changes the selection as in the grid while the two photos shown stay selected and the active photo is unchanged (#187); the active pane is framed in the active photo's two-tone selection ring (theme tokens, 3 pt), as in the grids; 1:1 to 8:1 sharp on both sides (zoom windows, see LR-VIEW-ZOOM); no zoom-link toggle; each pane says “Rendering…” (widget `compare:rendering:`) or “This photo can't be opened” (`compare:renderFailed:`) as the loupe does | -| LR-VIEW-SURVEY | Survey view [Classic] | P2 | ✅ | `cmd:view.survey`, `crates/ui-egui/src/panels/compare.rs` | selection tiled (≤ 48), keys, the bottom bar and the Info panel's stars and colour labels act on the active photo (ringed as in the grid), each tile's caption shows its colour label, hover × removes | +| LR-VIEW-SURVEY | Survey view [Classic] | P2 | ✅ | `cmd:view.survey`, `crates/ui-egui/src/panels/compare.rs` | selection tiled (≤ 48), keys, the bottom bar and the Info panel's stars and colour labels act on the active photo (ringed as in the grid), each tile's caption shows its colour label, hover × or ⌘/Ctrl-click removes a photo | | LR-VIEW-INFOOVERLAY | Info overlay on the photo | P1 | ✅ | `cmd:view.infoOverlay`, `cmd:view.viewOptions`, `crates/ui-egui/src/panels/detail.rs` (`info_overlay`) | off / Info 1 (by default file name, capture date, size) / Info 2 (file name, exposure, camera + lens), each three lines chosen in View Options; ⌘I cycles (I in full screen; elsewhere I stays the Info panel); the star rating and flag show under the lines when the photo has them, and in the full-screen preview on macOS the overlay starts below the window controls (issue #2460) | | LR-VIEW-SLIDESHOW | Slideshow | P2 | ✅ | `cmd:view.slideshow` | View ▸ Slideshow (⌥⌘↩): the photos in view full screen from the selected one, played with the library's playback settings (see LRC-SS-PLAYBACK: fades, random order, repeat, previews prepared ahead; `interval` / any setting overrides one run); Space pauses, ←/→ step (a cut), Esc ends; playback controls on pointer movement | | LR-VIEW-SECONDWINDOW | Second display window [Classic] | P2 | ✅ | `cmd:view.secondWindow`, `crates/ui-egui/src/panels/second.rs` | Window ▸ Second Window (⌘F11): the active photo fitted in its own native window with its own render (a floating panel where there are no native windows); loupe view only (no grid / compare / survey there) | From 3609fe846654f7b54b1c3263754fc36b1d175ca5 Mon Sep 17 00:00:00 2001 From: Fridtjof Michels Date: Sun, 1 Nov 2026 22:13:54 +0100 Subject: [PATCH 10/11] =?UTF-8?q?LR-VIEW-SURVEY:=20the=20survey=20?= =?UTF-8?q?=E2=8C=98/Ctrl-click=20test=20covers=20a=20non-active=20tile=20?= =?UTF-8?q?and=20the=20last=20photo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (#3563): a plain click on another tile makes it active, a ⌘/Ctrl-click on a selected tile that is not active removes only that one, and ⌘/Ctrl-click on the last photo leaves it in the survey and active. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests/compare.rs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/tests/compare.rs b/crates/ui-egui/src/tests/compare.rs index e4d8389749..26b069f50a 100644 --- a/crates/ui-egui/src/tests/compare.rs +++ b/crates/ui-egui/src/tests/compare.rs @@ -380,16 +380,27 @@ fn click_with(h: &mut Headless, pos: egui::Pos2, modifiers: egui::Modifiers) { h.settle(SETTLE); } -/// ⌘/Ctrl-click on a survey tile takes it out of the survey, as in Lightroom Classic; a plain -/// click only makes it active. +/// Click the centre of photo `id`'s survey tile with `modifiers`. +fn click_tile(h: &mut Headless, id: u64, modifiers: egui::Modifiers) { + let name = format!("cull:{id}"); + let pos = h.app.widgets.iter().find(|(w, _)| *w == name).map(|(_, r)| r.center()).expect("the tile is laid out"); + click_with(h, pos, modifiers); +} + +/// ⌘/Ctrl-click on a survey tile takes that photo out of the survey, as in Lightroom Classic, +/// whether or not it is the active one; a plain click only makes it active, and the last photo stays. #[test] fn survey_command_click_takes_the_photo_out() { let mut h = demo(); let [a, b, c] = survey_of_three(&mut h); - let img = h.app.image_rect.expect("the active tile is laid out"); - click_with(&mut h, img.center(), egui::Modifiers::NONE); - assert_eq!(h.app.session.selection.ids.len(), 3, "a plain click keeps the photo"); - click_with(&mut h, img.center(), egui::Modifiers::COMMAND); - let sel: Vec = h.app.session.selection.ids.iter().map(|p| p.0).collect(); - assert_eq!(sel, vec![a, c], "the ⌘/Ctrl-clicked photo left the survey (was {b})"); + let sel = |h: &Headless| h.app.session.selection.ids.iter().map(|p| p.0).collect::>(); + let active = |h: &Headless| h.app.session.selection.active.map(|p| p.0); + click_tile(&mut h, a, egui::Modifiers::NONE); + assert_eq!((sel(&h), active(&h)), (vec![a, b, c], Some(a)), "a plain click keeps the photo and makes it active"); + click_tile(&mut h, c, egui::Modifiers::COMMAND); + assert_eq!((sel(&h), active(&h)), (vec![a, b], Some(a)), "the ⌘/Ctrl-clicked photo (not the active one) left"); + click_tile(&mut h, b, egui::Modifiers::COMMAND); + assert_eq!(sel(&h), vec![a]); + click_tile(&mut h, a, egui::Modifiers::COMMAND); + assert_eq!((sel(&h), active(&h)), (vec![a], Some(a)), "the last photo stays in the survey"); } From da558f5b8fba9a46d97ae74846716b98e1a73825 Mon Sep 17 00:00:00 2001 From: Fridtjof Michels Date: Sun, 1 Nov 2026 22:34:35 +0100 Subject: [PATCH 11/11] =?UTF-8?q?LR-VIEW-SURVEY:=20test=20that=20=E2=8C=98?= =?UTF-8?q?/Ctrl-click=20on=20the=20active=20survey=20photo=20activates=20?= =?UTF-8?q?the=20last=20one=20left?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (#3563). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests/compare.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/ui-egui/src/tests/compare.rs b/crates/ui-egui/src/tests/compare.rs index 26b069f50a..f570df33a8 100644 --- a/crates/ui-egui/src/tests/compare.rs +++ b/crates/ui-egui/src/tests/compare.rs @@ -404,3 +404,15 @@ fn survey_command_click_takes_the_photo_out() { click_tile(&mut h, a, egui::Modifiers::COMMAND); assert_eq!((sel(&h), active(&h)), (vec![a], Some(a)), "the last photo stays in the survey"); } + +/// ⌘/Ctrl-click on the active survey photo while others remain: it leaves, and the last +/// remaining photo becomes active, so the keys still act on a photo on screen. +#[test] +fn survey_command_click_on_the_active_photo_activates_the_last_one() { + let mut h = demo(); + let [a, b, c] = survey_of_three(&mut h); + click_tile(&mut h, a, egui::Modifiers::NONE); + click_tile(&mut h, a, egui::Modifiers::COMMAND); + let sel: Vec = h.app.session.selection.ids.iter().map(|p| p.0).collect(); + assert_eq!((sel, h.app.session.selection.active.map(|p| p.0)), (vec![b, c], Some(c))); +}