diff --git a/core/dr-export/examples/export.rs b/core/dr-export/examples/export.rs index 9612aa6..1aeb3b2 100644 --- a/core/dr-export/examples/export.rs +++ b/core/dr-export/examples/export.rs @@ -10,7 +10,7 @@ use std::path::PathBuf; -use dr_export::{export, Frame, NameContext}; +use dr_export::{export, Frame, NameContext, SourceMetadata}; use dr_gpu::{AdjustPass, DemosaicedImage, Demosaicer, GpuContext}; use dr_pipeline::EditGraph; use dr_types::{ColourSpace, ExportFormat, ExportSettings, OutputSharpening, SizingMode}; @@ -96,6 +96,32 @@ fn main() { .map(|s| s.to_string_lossy().into_owned()) .unwrap_or_else(|| "export".into()); + // TRACES: FR-EXP-8 + // What the input said about itself, transcribed field by field into the + // allowlist `dr-export` will write from. The example passes it because + // this is the one place in the tree that produces files a person can open + // in exiftool — a unit test can prove a GPS directory is absent from a + // byte slice, but only a real export proves that a real photograph comes + // out of the far end still knowing which camera took it. + // + // The defaults apply, so the files written here carry the camera, the + // lens, the exposure and the rights statement, and carry no coordinates. + let meta = dr_decode::metadata(&bytes).unwrap_or_default(); + let source_metadata = SourceMetadata { + make: meta.make.clone(), + model: meta.model.clone(), + lens: meta.lens.clone(), + shutter: meta.shutter, + aperture: meta.aperture, + iso: meta.iso, + focal_length: meta.focal_length, + captured_at: meta.captured_at, + captured_offset: meta.captured_offset, + artist: meta.artist.clone(), + copyright: meta.copyright.clone(), + location: meta.location, + }; + // One of each format, so the run exercises every encoder that exists. for (format, sizing, sharpening) in [ ( @@ -155,7 +181,7 @@ fn main() { .expect("a free name"); let t = std::time::Instant::now(); - let out = export(&frame, &settings, name).expect("export"); + let out = export(&frame, &settings, name, Some(&source_metadata)).expect("export"); let path = out_dir.join(&out.name); std::fs::write(&path, &out.bytes).expect("write"); println!( diff --git a/core/dr-export/src/encode.rs b/core/dr-export/src/encode.rs index 01f96d2..9535a72 100644 --- a/core/dr-export/src/encode.rs +++ b/core/dr-export/src/encode.rs @@ -742,7 +742,7 @@ mod tests { artist: Some("Duncan Tourolle".into()), copyright: Some("(c) 2026 Duncan Tourolle".into()), // 48° 51' 29.52" N, 2° 17' 40.2" E. - location: dr_types::Location::new(48.8582, 2.2945, Some(35.0)), + location: dr_types::Location::new(LATITUDE, LONGITUDE, Some(35.0)), } } @@ -802,6 +802,45 @@ mod tests { bytes.windows(4).any(|w| w == LITTLE || w == BIG) } + /// TRACES: FR-EXP-8 + /// Whether the source's own coordinates appear anywhere in the bytes. + /// + /// The complement of [`has_gps_pointer`]. That one says no reader has a + /// *route* to a position; this says the numbers themselves are not in the + /// file at all — not under some other tag, not in a directory this test + /// did not think to look in, not left behind in a heap after the entry + /// pointing at it was dropped. + /// + /// The needle is the twenty-four bytes a coordinate serialises to: three + /// rationals, degrees, minutes and seconds. Specific enough that a match + /// is the coordinate rather than a coincidence, which matters because the + /// obvious cheaper needle is not: searching for the hemisphere letter as + /// `"N\0"` or `"E\0"` matches the tone curve inside the ICC profile every + /// export carries — sample 14 of the sRGB curve is 69, which is `00 45`, + /// beside a sample below 256, which is `00 xx`. A privacy test that fails + /// on the colour profile teaches nobody anything. + /// + /// Both byte orders, because `exif.rs` writes little-endian and the `tiff` + /// crate writes in the host's. + fn contains_coordinate(bytes: &[u8], degrees: f64) -> bool { + let mut little = Vec::new(); + let mut big = Vec::new(); + for (n, d) in exif::dms(degrees) { + little.extend_from_slice(&n.to_le_bytes()); + little.extend_from_slice(&d.to_le_bytes()); + big.extend_from_slice(&n.to_be_bytes()); + big.extend_from_slice(&d.to_be_bytes()); + } + bytes + .windows(little.len()) + .any(|w| w == little.as_slice() || w == big.as_slice()) + } + + /// The latitude and longitude [`source`] carries, for the two tests that + /// look for them in the bytes. + const LATITUDE: f64 = 48.8582; + const LONGITUDE: f64 = 2.2945; + #[test] fn no_export_carries_a_location_by_default() { // TRACES: FR-EXP-8 @@ -818,11 +857,16 @@ mod tests { ); let md = read_back(format, &bytes).unwrap_or_else(|| panic!("{format:?} has no EXIF")); assert_eq!(md.location, None, "{format:?} decodes to a position"); - // And the coordinates are not loose in the file under some other - // tag: the hemisphere letters a GPS directory always carries. + // And the numbers are not loose in the file with nothing pointing + // at them, which is what a scrubber that unlinked the directory + // without dropping its values would leave behind. assert!( - !bytes.windows(2).any(|w| w == b"N\0" || w == b"E\0"), - "{format:?} contains a hemisphere reference" + !contains_coordinate(&bytes, LATITUDE), + "{format:?} still contains the latitude" + ); + assert!( + !contains_coordinate(&bytes, LONGITUDE), + "{format:?} still contains the longitude" ); } } @@ -863,6 +907,17 @@ mod tests { has_gps_pointer(&bytes), "{format:?} dropped the position it was asked to keep" ); + // The control for `contains_coordinate` as well as for the + // pointer: a search that could never find the numbers would make + // the stripping test above pass without proving anything. + assert!( + contains_coordinate(&bytes, LATITUDE), + "{format:?} carries no latitude for the strip test to be about" + ); + assert!( + contains_coordinate(&bytes, LONGITUDE), + "{format:?} carries no longitude for the strip test to be about" + ); let md = read_back(format, &bytes).unwrap_or_else(|| panic!("{format:?} has no EXIF")); assert_eq!(md.make.as_deref(), Some("Canon"), "{format:?}"); assert_eq!(md.model.as_deref(), Some("Canon EOS 6D"), "{format:?}"); @@ -883,8 +938,8 @@ mod tests { let loc = md.location.unwrap_or_else(|| panic!("{format:?} lost the fix")); // Within a metre of where it started, which is finer than any // consumer receiver and far finer than the tag's own rounding. - assert!((loc.latitude - 48.8582).abs() < 1e-5, "{format:?} {loc:?}"); - assert!((loc.longitude - 2.2945).abs() < 1e-5, "{format:?} {loc:?}"); + assert!((loc.latitude - LATITUDE).abs() < 1e-5, "{format:?} {loc:?}"); + assert!((loc.longitude - LONGITUDE).abs() < 1e-5, "{format:?} {loc:?}"); assert_eq!(loc.altitude, Some(35.0), "{format:?}"); } } diff --git a/ui/dr-ui/src/export.rs b/ui/dr-ui/src/export.rs index a3e6553..2a5be24 100644 --- a/ui/dr-ui/src/export.rs +++ b/ui/dr-ui/src/export.rs @@ -617,8 +617,15 @@ fn export_one( issued: &mut HashSet, cancel: &Cancel, ) -> Option> { - let (stem, date, frame) = match source { - Source::Rendered { stem, frame } => (stem, String::new(), frame), + // TRACES: FR-EXP-8 + // The fourth element is what the photograph's own file said about itself. + // A library image is decoded here, so it has one; a frame handed over + // already rendered does not — the develop session holds pixels and an edit + // graph, not the header they came from, so an export from the develop + // button carries only what `dr-export` writes about itself until that is + // plumbed through the session. + let (stem, date, frame, source_metadata) = match source { + Source::Rendered { stem, frame } => (stem, String::new(), frame, None), Source::Library { path, cache } => { match render_from_library(request, &path, cache, cancel)? { Ok(rendered) => rendered, @@ -627,7 +634,42 @@ fn export_one( } }; - Some(place_frame(request, &stem, &date, sequence, &frame, issued)) + Some(place_frame( + request, + &stem, + &date, + sequence, + &frame, + source_metadata.as_ref(), + issued, + )) +} + +/// TRACES: FR-EXP-8 +/// What an export is allowed to carry from the file it was decoded from. +/// +/// Field by field rather than a conversion trait, and that is the point: +/// `dr_export::SourceMetadata` is an allowlist, so a tag newly parsed by +/// `dr-decode` reaches an exported file only when somebody adds a line here +/// and thereby decides, in writing, that it may leave the machine. The +/// location travels — `dr-export` is where the stripping decision is taken, +/// once, from the settings, and duplicating it here would give two places to +/// disagree. +fn carried_metadata(meta: &dr_decode::Metadata) -> dr_export::SourceMetadata { + dr_export::SourceMetadata { + make: meta.make.clone(), + model: meta.model.clone(), + lens: meta.lens.clone(), + shutter: meta.shutter, + aperture: meta.aperture, + iso: meta.iso, + focal_length: meta.focal_length, + captured_at: meta.captured_at, + captured_offset: meta.captured_offset, + artist: meta.artist.clone(), + copyright: meta.copyright.clone(), + location: meta.location, + } } /// Fetch a photograph, apply its stored edit, and render it at full size. @@ -636,7 +678,17 @@ fn render_from_library( path: &str, cache: Option, cancel: &Cancel, -) -> Option> { +) -> Option< + Result< + ( + String, + String, + dr_export::Frame, + Option, + ), + ItemError, + >, +> { let Some((creds, user_id)) = request.creds.clone() else { return Some(Err(ItemError::Fetch("no library is open".into()))); }; @@ -721,7 +773,12 @@ fn render_from_library( .map(|s| s.to_string_lossy().into_owned()) .unwrap_or_else(|| "export".into()); - Some(Ok((stem, date, frame))) + // TRACES: FR-EXP-8 + // `meta` was read at the top of this function for the orientation and the + // `{date}` token; carrying it on to the encoder is what puts the camera, + // the lens and the rights statement into the exported file. What is + // *dropped* from it is decided in `dr-export` from the settings, not here. + Some(Ok((stem, date, frame, Some(carried_metadata(&meta))))) } /// Name, encode and write one rendered frame. @@ -733,6 +790,11 @@ fn place_frame( date: &str, sequence: u32, frame: &dr_export::Frame, + // TRACES: FR-EXP-8 + // What the source file said about itself, or `None` where the caller has + // nothing to say. Handed straight through: every decision about what of it + // reaches the file is taken inside `dr-export`, from the settings. + source: Option<&dr_export::SourceMetadata>, issued: &mut HashSet, ) -> Result { // The size is resolved before the name because `{dimensions}` is one of the @@ -754,7 +816,7 @@ fn place_frame( }; let name = resolve_batch_name(&request.settings, &ctx, issued).ok_or(ItemError::NameTaken)?; - let encoded = dr_export::export(frame, &request.settings, name)?; + let encoded = dr_export::export(frame, &request.settings, name, source)?; place( &encoded,