From 733a033274d3ca31cf0ec6a8f01fee78ea35ad84 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 24 Sep 2026 20:56:23 -0400 Subject: [PATCH] Test that a stub decoder reaches the scan, the ladder and export FR-RAW-2's "without changing callers" needs a test that would fail if a caller named the concrete decoder; passing a real RAW through rawler cannot tell the two apart, because both routes give the same answer. The decoder_seam tests hand a stub decoder, for a container no real decoder reads, to the catalog scan (read_metadata_only over a folder backend), the preview ladder (the remote two-stage fetch, an import's thumbnail and the viewer's no-GPU fallback) and export (open_for_export, skipped without an adapter). Each assertion is on something only the stub produces: its camera and date, a header fetched at its 64-byte budget rather than HEADER_BYTES, preview and sensor sizes turned by its orientation. Switching collect_metadata or make_thumbnail back to the free functions fails two of the three tests. The develop test_support module is widened to the crate so the export test shares the one headless GPU context the other tests use. The requirements note for FR-RAW-2 now records the trait as built and the second decoder as not. --- docs/dev/requirements.md | 12 +- ui/dr-ui/src/decoder_seam.rs | 277 +++++++++++++++++++++++++++++++++++ ui/dr-ui/src/develop/mod.rs | 4 +- ui/dr-ui/src/lib.rs | 2 + 4 files changed, 290 insertions(+), 5 deletions(-) create mode 100644 ui/dr-ui/src/decoder_seam.rs diff --git a/docs/dev/requirements.md b/docs/dev/requirements.md index fe4d4d4..d27f67c 100644 --- a/docs/dev/requirements.md +++ b/docs/dev/requirements.md @@ -243,9 +243,15 @@ read metadata, and `import.rs` fetches exactly that range through `Storage::read calling `dr_decode::metadata`. The decoder states its requirement and the storage layer satisfies it; a decoder holding its own `SourceRef` would have had to implement the range policy itself. -What is genuinely not built is the trait. There is one decoder, reached through free functions, so -"without changing callers" is a claim nothing yet tests. The clause stands as written and is -outstanding work, not a satisfied one. +*Status (2026-09-24).* The trait is built: `dr_decode::Decoder`, over bytes — `header_bytes`, +`metadata`, `orientation`, `locate_preview`, `preview` and `decode` — with `dr_decode::Rawler` as +its one implementation, delegating to the free functions that were there before. The catalog scan, +the thumbnail ladder, import, the viewer, export, merge and repairs take a `&dyn Decoder`; only the +places that start a job name `dr_decode::default()`. "Without changing callers" is tested by +`dr-ui`'s `decoder_seam` tests, which hand a stub decoder for a container no real decoder reads to +the scan, the ladder and export, and fail if any of them reaches past the trait. Nothing in the +trait takes a path or a `SourceRef`. The second decoder itself (LibRaw, D2) is not built; S7 (#48) +is what would say when it is needed. **FR-RAW-3 — Sensor data handling.** Correctly apply per-camera black/white levels, CFA pattern identification, and camera-native colour matrices. Demosaic quality shall be selectable, with at diff --git a/ui/dr-ui/src/decoder_seam.rs b/ui/dr-ui/src/decoder_seam.rs new file mode 100644 index 0000000..33c3de0 --- /dev/null +++ b/ui/dr-ui/src/decoder_seam.rs @@ -0,0 +1,277 @@ +//! TRACES: FR-RAW-2 +//! A second decoder behind the trait, and the callers none the wiser. +//! +//! FR-RAW-2 says a decoder may be added for broader camera coverage without +//! changing callers (D2: LibRaw, for bodies rawler lacks). These tests are +//! that claim, made checkable: [`Stub`] reads a container no real decoder +//! knows, and the catalog scan, the preview ladder and export are each handed +//! it through the same `&dyn Decoder` the application passes rawler through. +//! +//! Every assertion is on something only the stub could have produced — a +//! camera name, a preview size, a header budget, a turn — so a caller that +//! reached past the trait to `dr_decode::metadata` or its siblings would get +//! rawler's refusal of these bytes and fail here, rather than pass on a real +//! file where the two decoders happen to agree. + +use std::sync::Mutex; + +use dr_decode::{ + BaseCurve, CfaPattern, CropRect, DecodeError, Decoder, Metadata, Preview, PreviewLocation, + PreviewSize, RawImage, +}; +use dr_types::Orientation; + +/// What every stub file starts with. Not a signature any real format uses. +const MAGIC: &[u8] = b"STUB-RAW"; + +/// The stub's header budget: far smaller than `dr_decode::HEADER_BYTES`, so a +/// caller that fetched the shipped decoder's budget instead is visible. +const HEADER: u64 = 64; + +/// The size of the preview [`Decoder::preview`] cuts, before orientation. +const PREVIEW: (u32, u32) = (24, 12); + +/// The size of the JPEG embedded for the remote ladder, before orientation. +const EMBEDDED: (u32, u32) = (40, 20); + +/// The sensor [`Decoder::decode`] returns. +const SENSOR: (u32, u32) = (32, 16); + +/// EXIF 6: one clockwise quarter turn, so every size above comes out swapped. +fn turned() -> Orientation { + Orientation::from_exif(6) +} + +/// A decoder for the stub container, recording the length of every header it +/// is handed so a test can see how much the caller fetched. +#[derive(Debug, Default)] +struct Stub { + headers: Mutex>, +} + +impl Stub { + fn is_ours(bytes: &[u8]) -> bool { + bytes.starts_with(MAGIC) + } + + fn refuse() -> DecodeError { + DecodeError::Unsupported("not a stub file".into()) + } +} + +impl Decoder for Stub { + fn header_bytes(&self) -> u64 { + HEADER + } + + fn metadata(&self, bytes: &[u8]) -> Result { + self.headers.lock().unwrap().push(bytes.len()); + if !Self::is_ours(bytes) { + return Err(Self::refuse()); + } + Ok(Metadata { + make: Some("Stubco".into()), + model: Some("Stubco One".into()), + iso: Some(321), + captured_at: Some(1_234_567_890), + orientation: Some(turned()), + ..Default::default() + }) + } + + fn orientation(&self, header: &[u8]) -> Option { + Self::is_ours(header).then(turned) + } + + fn locate_preview(&self, header: &[u8], file_len: u64) -> Option { + if !Self::is_ours(header) { + return None; + } + let word = |at: usize| u32::from_le_bytes(header[at..at + 4].try_into().unwrap()) as u64; + let (start, len) = (word(8), word(12)); + (start + len <= file_len).then_some(PreviewLocation { + range: start..start + len, + width: Some(EMBEDDED.0), + height: Some(EMBEDDED.1), + }) + } + + fn preview(&self, bytes: &[u8], _size: PreviewSize) -> Result { + if !Self::is_ours(bytes) { + return Err(Self::refuse()); + } + Ok(Preview { + width: PREVIEW.0, + height: PREVIEW.1, + rgba: vec![200; (PREVIEW.0 * PREVIEW.1 * 4) as usize], + }) + } + + fn decode(&self, bytes: &[u8]) -> Result { + if !Self::is_ours(bytes) { + return Err(Self::refuse()); + } + let (width, height) = SENSOR; + Ok(RawImage { + width, + height, + data: vec![2048; (width * height) as usize], + cfa_pattern: CfaPattern::Rggb, + black_level: [0; 4], + white_level: 4095, + wb_coeffs: [1.0; 4], + color_matrix: None, + base_curve: BaseCurve::IDENTITY, + crop: CropRect { + x: 0, + y: 0, + width, + height, + }, + samples_per_pixel: 1, + profile: None, + make: "Stubco".into(), + model: "Stubco One".into(), + }) + } +} + +/// A stub file: the magic, where the embedded JPEG sits, padding to the +/// header budget, then the JPEG — kept well past [`HEADER`] so a header read +/// and a whole-file read differ in length. +fn stub_file() -> Vec { + let (w, h) = EMBEDDED; + let jpeg = dr_thumbs::encode_rgba(w, h, &vec![90; (w * h * 4) as usize]).expect("encodes"); + let mut file = MAGIC.to_vec(); + file.extend_from_slice(&(HEADER as u32).to_le_bytes()); + file.extend_from_slice(&(jpeg.len() as u32).to_le_bytes()); + file.resize(HEADER as usize, 0); + file.extend_from_slice(&jpeg); + file +} + +const NAME: &str = "IMG_0001.STB"; + +/// The stub file in a folder of its own, served by the folder backend — the +/// same `RemoteBackend` the scan and the ladder use against a real library. +/// `tag` keeps two tests running at once out of each other's folder. +fn library(tag: &str) -> (std::path::PathBuf, dr_sync_folder::FolderBackend, u64) { + let dir = std::env::temp_dir().join(format!("dr-decoder-seam-{tag}-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let file = stub_file(); + std::fs::write(dir.join(NAME), &file).unwrap(); + let backend = dr_sync_folder::FolderBackend::new(&dir).expect("folder opens"); + (dir, backend, file.len() as u64) +} + +fn request(size: u64) -> crate::library::ThumbnailRequest { + crate::library::ThumbnailRequest { + row: 0, + path: NAME.into(), + file_id: Some(1), + size, + image_id: 7, + thumb_size: dr_thumbs::ThumbSize::Grid, + needs_metadata: true, + full_resolution: false, + } +} + +fn swapped((w, h): (u32, u32)) -> (u32, u32) { + (h, w) +} + +/// TRACES: FR-RAW-2 | FR-CAT-5 +/// The catalog scan dates an image through the decoder it is given, and +/// fetches the header that decoder asked for rather than the shipped one's. +#[test] +fn the_catalog_scan_reads_headers_through_the_trait() { + let (dir, backend, size) = library("scan"); + let stub = Stub::default(); + let mut found = Vec::new(); + + let rt = crate::net_runtime::build().unwrap(); + let reached = rt.block_on(crate::library::read_metadata_only( + &backend, + &stub, + &request(size), + &mut found, + )); + + assert!(reached); + assert_eq!(found.len(), 1, "the stub's header was read"); + assert_eq!(found[0].image_id, 7); + assert_eq!(found[0].camera.as_deref(), Some("Stubco One")); + assert_eq!(found[0].captured_at, Some(1_234_567_890)); + assert_eq!(found[0].iso, Some(321)); + assert_eq!( + *stub.headers.lock().unwrap(), + vec![HEADER as usize], + "one header, of the size the decoder declared" + ); + let _ = std::fs::remove_dir_all(dir); +} + +/// TRACES: FR-RAW-2 | FR-CULL-2 | FR-NC-3 +/// Every rung of the preview ladder takes its preview and its orientation +/// from the decoder it is given: the remote two-stage fetch, the import's +/// thumbnail, and the viewer's fallback when there is no GPU. +#[test] +fn the_preview_ladder_cuts_previews_through_the_trait() { + let (dir, backend, size) = library("ladder"); + let stub = Stub::default(); + let bytes = stub_file(); + + // Remote: header, locate, fetch the range, decode, turn. + let rt = crate::net_runtime::build().unwrap(); + let mut found = Vec::new(); + let outcome = rt.block_on(crate::library::fetch_preview( + &backend, + &stub, + &request(size), + &mut found, + )); + match outcome { + crate::library::PreviewOutcome::Ready(p) => { + assert_eq!((p.width, p.height), swapped(EMBEDDED)) + } + crate::library::PreviewOutcome::Unavailable(r) + | crate::library::PreviewOutcome::Offline(r) => panic!("no preview: {r}"), + } + assert_eq!(found.len(), 1, "the same header dated the image"); + + // An import's thumbnail, from bytes already in hand. + let thumb = crate::import::make_thumbnail(&stub, &bytes).expect("a thumbnail"); + assert_eq!((thumb.width, thumb.height), swapped(PREVIEW)); + + // The viewer with no GPU, which shows the embedded preview read-only. + let loaded = crate::load_bytes(None, &stub, &bytes).expect("a preview to show"); + assert!(loaded.session.is_none()); + assert_eq!((loaded.width, loaded.height), swapped(PREVIEW)); + assert_eq!(loaded.meta.model.as_deref(), Some("Stubco One")); + + let _ = std::fs::remove_dir_all(dir); +} + +/// TRACES: FR-RAW-2 | FR-EXP-8 | FR-EXP-9 +/// Export opens a photograph for its full-size render through the decoder it +/// is given: the header it carries and the sensor it renders are the stub's. +#[test] +fn export_decodes_through_the_trait() { + let Some(gpu) = crate::develop::test_support::headless() else { + eprintln!("no GPU adapter; skipping"); + return; + }; + let stub = Stub::default(); + + let (meta, session) = + crate::export::open_for_export(&gpu, &stub, &stub_file()).expect("the stub opens"); + + assert_eq!(meta.make.as_deref(), Some("Stubco")); + assert_eq!( + session.source_metadata().and_then(|m| m.model.as_deref()), + Some("Stubco One"), + "the session remembers the header it was opened from" + ); + assert_eq!(session.source_size(), swapped(SENSOR)); +} diff --git a/ui/dr-ui/src/develop/mod.rs b/ui/dr-ui/src/develop/mod.rs index 9627a04..ff18048 100644 --- a/ui/dr-ui/src/develop/mod.rs +++ b/ui/dr-ui/src/develop/mod.rs @@ -48,7 +48,7 @@ pub(super) const SEGMENT_PROXY_EDGE: u32 = 1600; /// the code they exercise left these three needed in most of the resulting /// files, so they live here once instead of being copied. #[cfg(test)] -pub(super) mod test_support { +pub(crate) mod test_support { use super::*; use dr_gpu::GpuContext; @@ -142,7 +142,7 @@ pub(super) mod test_support { /// `OnceLock` rather than `lazy_static`: the initialiser runs once however /// many threads arrive together, and the losers block until it is done — /// which is precisely the property that was missing. - pub(super) fn headless() -> Option { + pub(crate) fn headless() -> Option { static SHARED: std::sync::OnceLock> = std::sync::OnceLock::new(); SHARED .get_or_init(|| pollster::block_on(dr_gpu::GpuContext::new_headless()).ok()) diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index 62ba4c3..4f1ae96 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -22,6 +22,8 @@ mod activity; mod bursts; mod collections_ui; +#[cfg(test)] +mod decoder_seam; mod derived_sync; mod develop; mod develop_ui;