Look the lens up and say plainly whether one was found
`dr-lens` has held a complete Lensfun lookup — distortion, TCA and vignetting coefficients from a lens name, a focal length and an aperture — with no dependents anywhere in the workspace. The three corrections it feeds now exist in the graph, so this connects the two and finishes the chain. The coefficient structs stay duplicated. `dr-pipeline` is organised around having no dependencies so its codegen is testable without a device or a database (ARCH §6.5a), and `dr-lens` carries an XML parser and 5.5 MB of profile data. Neither crate can convert to the other, so the conversion goes above both, in `develop.rs`, which is the only place that sees them together. Both traits grow the same defaulted door. The optical corrections do not sit on the same side of the fetch — distortion and CA rewrite coordinates and are `Warp`s, vignetting applies a gain to the pixel already there and is an ordinary node — and fanning a profile out by which trait each happens to implement would make the caller reason about that distinction. Each correction takes its own share of the whole profile instead, and `set_lens_profile` walks both lists identically. The lookup happens in `set_source_metadata` rather than in its caller, because that is the one place a session is told which file it came from. Doing it there makes it unforgettable, in the shape `FilmRebake` already uses for the other derived thing — and, more to the point, makes *clearing* unforgettable: a session that opened a second photograph while still holding the first one's profile would correct it for the wrong optics, invisibly, in a way that looks exactly like the lens. It needs the whole shot and not just a name. Distortion is interpolated across a zoom's focal range and vignetting depends strongly on aperture — a fast prime can be two stops down in the corners wide open and clean by f/8 — so a lookup missing either returns coefficients measured for a shot nobody took. Missing any of the three refuses rather than guesses. A profile is derived, not persisted: it comes from the file's EXIF and a database, so it is not a parameter, not in the sidecar and not undoable. What is an edit is the manual trim beside it, which each correction composes with the measurement — so a photographer can lean on it, override it, or work without one. `InfoPanel` gains a lens line, and it distinguishes three cases rather than two. `dr-lens` states the rule it exists for: an automatic correction that silently did nothing is worse than one the user can see is unavailable. A session with no header draws nothing, a header naming no lens reads "Lens not recorded", and a lens the database has never heard of reads "· no profile". Collapsing the last two would send somebody hunting for a profile that was never missing — which, for third-party and adapted glass, is the ordinary case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -728,6 +728,14 @@ pub struct DevelopSession {
|
||||
/// the memory of where the pixels came from; the allowlist that turns it
|
||||
/// into something writable stays the one function in `export.rs`.
|
||||
source_meta: Option<dr_decode::Metadata>,
|
||||
/// Whether the lens in the header matched a profile in the database.
|
||||
///
|
||||
/// A separate flag rather than `graph.lens_profile().is_some()`, because
|
||||
/// the two answer different questions once the photographer starts work:
|
||||
/// the graph says what is *applied*, which a manual correction also
|
||||
/// satisfies, and this says whether a *measurement* was found. Only the
|
||||
/// second can honestly caption "no profile".
|
||||
lens_profile_found: bool,
|
||||
/// Kept so the session can build GPU resources after construction.
|
||||
///
|
||||
/// The distance fields behind a subject mask are made when a layer is
|
||||
@@ -955,6 +963,9 @@ impl DevelopSession {
|
||||
// built straight from pixels — a test, `masks_ui`'s fixture —
|
||||
// honestly has no header, and says so.
|
||||
source_meta: None,
|
||||
// Nothing has been looked up, which is not the same as "looked up
|
||||
// and not found" — `lens_summary` distinguishes them.
|
||||
lens_profile_found: false,
|
||||
ctx: ctx.clone(),
|
||||
graph,
|
||||
history,
|
||||
@@ -3479,9 +3490,106 @@ impl DevelopSession {
|
||||
/// its header together — every other way of making a session starts from
|
||||
/// pixels that never had a file behind them.
|
||||
pub fn set_source_metadata(&mut self, meta: dr_decode::Metadata) {
|
||||
// The lens profile is applied *here* rather than by the caller, and
|
||||
// that is the point of putting it in this method. This is the one
|
||||
// place a session is told which file it came from, so it is the one
|
||||
// place the lookup can be made unforgettable — the same shape
|
||||
// `FilmRebake` uses to stop a derived thing being quietly skipped.
|
||||
self.apply_lens_profile(&meta);
|
||||
self.source_meta = Some(meta);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Look this shot's lens up and hand the coefficients to the corrections.
|
||||
///
|
||||
/// Called with **every** header, including ones naming no lens: the
|
||||
/// clearing case matters as much as the setting one, because a session
|
||||
/// reused for a second photograph would otherwise correct it for the
|
||||
/// optics of the first.
|
||||
fn apply_lens_profile(&mut self, meta: &dr_decode::Metadata) {
|
||||
let found = Self::profile_for(meta);
|
||||
self.lens_profile_found = found.is_some();
|
||||
self.graph.set_lens_profile(found);
|
||||
}
|
||||
|
||||
/// The profile for one shot, converted into the pipeline's own types.
|
||||
///
|
||||
/// **The conversion lives here because nowhere else can see both sides.**
|
||||
/// `dr-lens` carries the Lensfun database and `dr-pipeline` carries the
|
||||
/// maths, and the coefficient structs are deliberately duplicated so that
|
||||
/// the dependency between them does not exist (ARCH §6.5a). This function
|
||||
/// is the seam, and it is a `match` on three optionals.
|
||||
///
|
||||
/// Every field is taken independently. The database routinely knows a
|
||||
/// lens's distortion and not its vignetting, or covers only part of a
|
||||
/// zoom's range, and a partial profile is worth applying — discarding it
|
||||
/// because one field is missing would turn a good correction into none.
|
||||
fn profile_for(meta: &dr_decode::Metadata) -> Option<dr_pipeline::LensProfile> {
|
||||
// All three are needed to ask the question at all. A lens name alone
|
||||
// does not identify a correction: distortion is interpolated across a
|
||||
// zoom's focal range and vignetting depends strongly on aperture — a
|
||||
// fast prime can be two stops down in the corners wide open and clean
|
||||
// by f/8 — so a lookup missing either would return a profile measured
|
||||
// for a shot nobody took.
|
||||
let (lens, focal, aperture) = (meta.lens.as_deref()?, meta.focal_length?, meta.aperture?);
|
||||
|
||||
let shot = dr_lens::ShotInfo::new(lens, focal, aperture);
|
||||
let found = dr_lens::lookup(&shot)?;
|
||||
if found.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(dr_pipeline::LensProfile {
|
||||
distortion: found
|
||||
.distortion
|
||||
.map(|d| dr_pipeline::ops::distortion::PtLens {
|
||||
a: d.a,
|
||||
b: d.b,
|
||||
c: d.c,
|
||||
}),
|
||||
tca: found.tca.map(|t| dr_pipeline::Tca {
|
||||
red_scale: t.red_scale,
|
||||
blue_scale: t.blue_scale,
|
||||
}),
|
||||
vignetting: found.vignetting.map(|v| dr_pipeline::ops::vignetting::Pa {
|
||||
k1: v.k1,
|
||||
k2: v.k2,
|
||||
k3: v.k3,
|
||||
}),
|
||||
})
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// What to tell the photographer about the automatic lens correction.
|
||||
///
|
||||
/// `dr-lens` states the rule this exists to satisfy: an automatic
|
||||
/// correction that silently did nothing is worse than one the user can see
|
||||
/// is unavailable. Most lenses in most photographs will not be in the
|
||||
/// database — third-party glass often reports nothing, adapted manual
|
||||
/// lenses report nothing at all — so "no profile" is the ordinary case and
|
||||
/// has to read as a fact rather than as a failure.
|
||||
pub fn lens_summary(&self) -> String {
|
||||
let Some(meta) = self.source_meta.as_ref() else {
|
||||
return String::new();
|
||||
};
|
||||
let Some(lens) = meta
|
||||
.lens
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|l| !l.is_empty())
|
||||
else {
|
||||
// Not "no profile found": nothing was looked up, because the file
|
||||
// does not say what it was taken with. Naming the wrong reason
|
||||
// would send someone hunting for a profile that was never missing.
|
||||
return "Lens not recorded".into();
|
||||
};
|
||||
if self.lens_profile_found {
|
||||
format!("{lens} · corrected")
|
||||
} else {
|
||||
format!("{lens} · no profile")
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-EXP-8
|
||||
/// The header this session was opened from, where there was one.
|
||||
///
|
||||
@@ -4814,6 +4922,136 @@ mod tests {
|
||||
.clone()
|
||||
}
|
||||
|
||||
/// A lookup needs all three of lens, focal length and aperture.
|
||||
///
|
||||
/// Not pedantry about missing fields: distortion is interpolated across a
|
||||
/// zoom's focal range and vignetting depends strongly on aperture, so a
|
||||
/// lookup done without them would return coefficients measured for a shot
|
||||
/// nobody took and apply them with full confidence. Refusing is the honest
|
||||
/// answer, and the panel says so.
|
||||
#[test]
|
||||
fn a_lookup_needs_the_whole_shot_and_not_just_the_lens() {
|
||||
let complete = dr_decode::Metadata {
|
||||
lens: Some("Nikon AF-S 50mm f/1.8G".into()),
|
||||
focal_length: Some(50.0),
|
||||
aperture: Some(1.8),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
for (name, meta) in [
|
||||
(
|
||||
"no lens",
|
||||
dr_decode::Metadata {
|
||||
lens: None,
|
||||
..complete.clone()
|
||||
},
|
||||
),
|
||||
(
|
||||
"no focal length",
|
||||
dr_decode::Metadata {
|
||||
focal_length: None,
|
||||
..complete.clone()
|
||||
},
|
||||
),
|
||||
(
|
||||
"no aperture",
|
||||
dr_decode::Metadata {
|
||||
aperture: None,
|
||||
..complete.clone()
|
||||
},
|
||||
),
|
||||
] {
|
||||
assert!(
|
||||
DevelopSession::profile_for(&meta).is_none(),
|
||||
"{name}: a partial header must not produce a confident profile"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// "Not recorded" and "no profile" are different facts.
|
||||
///
|
||||
/// Collapsing them would send someone hunting for a missing profile when
|
||||
/// the file simply never said what took the photograph — and `dr-lens`'s
|
||||
/// own rule is that the interface must be plain about which it is, because
|
||||
/// a correction that silently did nothing is worse than one visibly
|
||||
/// unavailable.
|
||||
#[test]
|
||||
fn the_lens_line_says_which_kind_of_nothing_it_found() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..8 * 8).flat_map(|_| [128u8, 128, 128, 255]).collect();
|
||||
|
||||
let session = |meta: dr_decode::Metadata| {
|
||||
let mut s = DevelopSession::open_rgb(&ctx, &rgba, 8, 8, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
s.set_source_metadata(meta);
|
||||
s
|
||||
};
|
||||
|
||||
// A session that was never given a header at all.
|
||||
let bare = DevelopSession::open_rgb(&ctx, &rgba, 8, 8, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
assert_eq!(bare.lens_summary(), "");
|
||||
|
||||
let unrecorded = session(dr_decode::Metadata::default());
|
||||
assert_eq!(unrecorded.lens_summary(), "Lens not recorded");
|
||||
|
||||
// A name no database will match. Deliberately absurd rather than a real
|
||||
// obscure lens, so the test cannot start passing for the wrong reason
|
||||
// if the bundled database grows.
|
||||
let unmatched = session(dr_decode::Metadata {
|
||||
lens: Some("Nonexistent 999mm f/0.5".into()),
|
||||
focal_length: Some(999.0),
|
||||
aperture: Some(0.5),
|
||||
..Default::default()
|
||||
});
|
||||
assert_eq!(
|
||||
unmatched.lens_summary(),
|
||||
"Nonexistent 999mm f/0.5 · no profile"
|
||||
);
|
||||
assert!(
|
||||
unmatched.graph.lens_profile().is_none(),
|
||||
"an unmatched lens must leave the corrections alone"
|
||||
);
|
||||
}
|
||||
|
||||
/// Opening a second photograph must not correct it for the first one's lens.
|
||||
///
|
||||
/// The clearing case, and the reason `apply_lens_profile` runs on every
|
||||
/// header rather than only on the ones that match something. A stale
|
||||
/// profile is invisible: the picture is simply wrong in a way that looks
|
||||
/// like the lens.
|
||||
#[test]
|
||||
fn a_second_photograph_does_not_inherit_the_first_lens_profile() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..8 * 8).flat_map(|_| [128u8, 128, 128, 255]).collect();
|
||||
let mut session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 8, 8, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
|
||||
// Stand in for a matched lens by applying a profile directly, so the
|
||||
// test does not depend on what the bundled database happens to hold.
|
||||
session
|
||||
.graph
|
||||
.set_lens_profile(Some(dr_pipeline::LensProfile {
|
||||
distortion: Some(dr_pipeline::ops::distortion::PtLens {
|
||||
a: 0.0,
|
||||
b: -0.02,
|
||||
c: 0.0,
|
||||
}),
|
||||
tca: None,
|
||||
vignetting: None,
|
||||
}));
|
||||
assert!(session.graph.lens_profile().is_some());
|
||||
|
||||
session.set_source_metadata(dr_decode::Metadata::default());
|
||||
|
||||
assert!(
|
||||
session.graph.lens_profile().is_none(),
|
||||
"a header naming no lens must clear the previous photograph's \
|
||||
correction, not leave it standing"
|
||||
);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// A gradient needs no segmentation, and until now it silently got no mask.
|
||||
///
|
||||
|
||||
@@ -1995,6 +1995,19 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
window.set_exposure(describe_exposure(&l.meta).into());
|
||||
window.set_dimensions(format!("{} × {}", l.width, l.height).into());
|
||||
|
||||
// Composed by the session rather than here, because only it
|
||||
// knows whether the lookup found anything — the header can
|
||||
// name a lens the database has never heard of, which is the
|
||||
// ordinary case for third-party and adapted glass and must
|
||||
// read as a fact rather than as a failure.
|
||||
window.set_lens(
|
||||
l.session
|
||||
.as_ref()
|
||||
.map(|s| s.lens_summary())
|
||||
.unwrap_or_default()
|
||||
.into(),
|
||||
);
|
||||
|
||||
// The panel is built from what the pipeline reports, so
|
||||
// this code names no operation (FR-DEV-3a).
|
||||
match l.session {
|
||||
@@ -2041,6 +2054,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
window.set_camera("".into());
|
||||
window.set_exposure("".into());
|
||||
window.set_dimensions("".into());
|
||||
window.set_lens("".into());
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user