Merge: the optical corrections, connected at last
Four files' worth of lens correction existed, was tested, and had never touched a photograph. `compose_warps` had no callers, `dr-lens` had no dependents, and `ops/vignetting.rs` had no declaration in `ops/` — so `Attribute::Optics` was a category the tab strip could only ever filter out for having no rows in it. Distortion, chromatic aberration and lens vignetting now render, carry their parameters through the sidecar and the undo stack, and take their coefficients from the Lensfun database when the file names a lens it knows. The panel says which of "no lens recorded" and "no profile for this lens" it is, because an automatic correction that silently did nothing is worse than one visibly unavailable. One real bug on the way: `lens.rs` and `framing.rs` both documented the shader's `p` as corner-normalised, and it is not — its length at the corner is `0.5 * length(aspect)`, about 0.901 on a 3:2 frame. Every Lensfun polynomial would have been evaluated short of where it was fitted, by a factor varying with the aspect ratio, which reads as a correction that is merely too weak. The category vocabulary moved with it. `Attribute::Geometry` is `Compose` — named for the photographer's decision rather than for the maths it shares with the lens corrections — and a film stock stopped claiming to be both tone and colour, which had put "Kodachrome" in two groups it belongs to neither of.
This commit is contained in:
Generated
+1
@@ -1658,6 +1658,7 @@ dependencies = [
|
||||
"dr-film",
|
||||
"dr-gpu",
|
||||
"dr-ingest",
|
||||
"dr-lens",
|
||||
"dr-pipeline",
|
||||
"dr-plat",
|
||||
"dr-preset-xmp",
|
||||
|
||||
@@ -335,6 +335,7 @@ fn render_stack(
|
||||
ColourSpace::Srgb,
|
||||
stack,
|
||||
&SpotSet::new(),
|
||||
&[],
|
||||
);
|
||||
adjust
|
||||
.render_masked(source, &shader, ow, oh, Some(array))
|
||||
|
||||
@@ -1845,7 +1845,21 @@ mod tests {
|
||||
fused_blocks += usize::from(point);
|
||||
}
|
||||
|
||||
// The count the loop above accumulated, plus framing — which emits a
|
||||
// The lens corrections, which are the third kind of block. They are
|
||||
// not in `descriptors` — they rewrite coordinates rather than
|
||||
// transform a colour, so they are not operations — and they run ahead
|
||||
// of the fetch rather than in either stage the loop above sorts into.
|
||||
let mut warp_blocks = 0;
|
||||
for desc in g.warp_descriptors() {
|
||||
let id = desc.id.0;
|
||||
assert!(
|
||||
shader.source.contains(&format!("---- warp: {id} ----")),
|
||||
"{id} was armed above and did not reach the shader"
|
||||
);
|
||||
warp_blocks += 1;
|
||||
}
|
||||
|
||||
// The counts the loops above accumulated, plus framing — which emits a
|
||||
// stage of its own rather than an operation block and is not in
|
||||
// `descriptors`. Asserted as well as the per-operation exclusive-or
|
||||
// because the two catch different faults: the XOR catches an operation
|
||||
@@ -1853,7 +1867,7 @@ mod tests {
|
||||
// in the chain asked for.
|
||||
assert_eq!(
|
||||
shader.source.matches("---- ").count(),
|
||||
fused_blocks + 1,
|
||||
fused_blocks + warp_blocks + 1,
|
||||
"the fused shader carries a block nothing in the chain asked for"
|
||||
);
|
||||
assert!(
|
||||
|
||||
@@ -72,6 +72,7 @@ fn render_at(
|
||||
ColourSpace::Srgb,
|
||||
stack,
|
||||
&SpotSet::new(),
|
||||
&[],
|
||||
);
|
||||
|
||||
let mut masks = MaskPass::new(ctx).expect("mask pass");
|
||||
|
||||
@@ -90,6 +90,7 @@ fn render_at(ctx: &GpuContext, stack: &MaskStack, out: u32) -> Vec<u8> {
|
||||
ColourSpace::Srgb,
|
||||
stack,
|
||||
&SpotSet::new(),
|
||||
&[],
|
||||
);
|
||||
let mut adjust = AdjustPass::new(ctx);
|
||||
adjust
|
||||
@@ -108,6 +109,7 @@ fn render_unmasked(ctx: &GpuContext, stack: &MaskStack, out: u32) -> Vec<u8> {
|
||||
ColourSpace::Srgb,
|
||||
stack,
|
||||
&SpotSet::new(),
|
||||
&[],
|
||||
);
|
||||
let mut adjust = AdjustPass::new(ctx);
|
||||
adjust.render(&source, &shader, out, out).expect("render");
|
||||
@@ -230,6 +232,7 @@ fn render_gradient(ctx: &GpuContext, stack: &MaskStack, w: u32, h: u32) -> Vec<u
|
||||
ColourSpace::Srgb,
|
||||
stack,
|
||||
&SpotSet::new(),
|
||||
&[],
|
||||
);
|
||||
let mut adjust = AdjustPass::new(ctx);
|
||||
adjust
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
id: film_sim
|
||||
order: 25
|
||||
attributes: [tone, colour]
|
||||
# Effect, not tone-and-colour. It declared both, which put "Kodachrome" in
|
||||
# the Light group beside exposure and again in Colour beside white balance —
|
||||
# two places, neither of which is where anyone looks for it, and both of which
|
||||
# it crowded. A stock is `Effect`'s own definition: applied rather than
|
||||
# corrected, a look and not a fix. That it moves tone and colour is true of
|
||||
# every look and is not what the attribute is for.
|
||||
attributes: [effect]
|
||||
rust: FilmSim
|
||||
|
||||
why_rust: |
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
# A hand-written node, and the first thing in `Attribute::Optics`.
|
||||
#
|
||||
# Written, tested and unreferenced until now: `src/ops/vignetting.rs` has
|
||||
# existed with a full descriptor and a working polynomial, and without an entry
|
||||
# here it was never in `chain()` — so it reached no photograph and no panel.
|
||||
# This file is the whole of what was missing, which is the point of `ops/`
|
||||
# being the one place the pipeline's order is written down.
|
||||
id: vignetting
|
||||
order: 5
|
||||
|
||||
attributes: [optics]
|
||||
rust: Vignetting
|
||||
|
||||
why_rust: |
|
||||
It carries a lens profile's `pa` coefficients, which are not parameters: they
|
||||
come from the body and lens that took the photograph, not from the
|
||||
photographer, and no `uniforms:` expression could produce them. The one
|
||||
parameter that *is* theirs — the manual trim — is composed with `k1` in Rust,
|
||||
because the profile and the trim have to reach the shader as a single
|
||||
polynomial rather than as two the fragment would have to add up.
|
||||
|
||||
placement: |
|
||||
First in the chain, ahead of white balance and exposure.
|
||||
|
||||
Vignetting is what the lens did to the light before the sensor measured it,
|
||||
so undoing it belongs with reading the file rather than with editing the
|
||||
picture — everything downstream is then working on the frame the lens would
|
||||
have delivered had it been even.
|
||||
|
||||
The ordering is load-bearing rather than tidy. Correcting a corner means
|
||||
*dividing* by an attenuation below one, which pushes those pixels up: a fast
|
||||
prime wide open needs about two stops there. Run after the tonal stages, that
|
||||
recovery happens once the highlights have already been rolled off and
|
||||
clipped, so it lifts values that no longer have anywhere to go and the
|
||||
corners posterise instead of brightening. Run here, the headroom to hold them
|
||||
still exists (ARCH §5.2).
|
||||
|
||||
Before distortion and chromatic aberration in intent, though those are
|
||||
`lens::Warp`s rather than nodes and compose ahead of the fetch, so no `order:`
|
||||
relates the two.
|
||||
@@ -120,7 +120,7 @@ pub enum Attr {
|
||||
Colour,
|
||||
Detail,
|
||||
Optics,
|
||||
Geometry,
|
||||
Compose,
|
||||
Effect,
|
||||
}
|
||||
|
||||
@@ -132,7 +132,7 @@ impl Attr {
|
||||
Attr::Colour => "colour",
|
||||
Attr::Detail => "detail",
|
||||
Attr::Optics => "optics",
|
||||
Attr::Geometry => "geometry",
|
||||
Attr::Compose => "compose",
|
||||
Attr::Effect => "effect",
|
||||
}
|
||||
}
|
||||
@@ -144,7 +144,7 @@ impl Attr {
|
||||
Attr::Colour,
|
||||
Attr::Detail,
|
||||
Attr::Optics,
|
||||
Attr::Geometry,
|
||||
Attr::Compose,
|
||||
Attr::Effect,
|
||||
];
|
||||
}
|
||||
|
||||
@@ -350,7 +350,7 @@ fn attribute(a: decl::Attr) -> Attribute {
|
||||
decl::Attr::Colour => Attribute::Colour,
|
||||
decl::Attr::Detail => Attribute::Detail,
|
||||
decl::Attr::Optics => Attribute::Optics,
|
||||
decl::Attr::Geometry => Attribute::Geometry,
|
||||
decl::Attr::Compose => Attribute::Compose,
|
||||
decl::Attr::Effect => Attribute::Effect,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -580,8 +580,15 @@ pub enum Attribute {
|
||||
/// Corrections for the lens that took the photograph: distortion,
|
||||
/// chromatic aberration, vignetting.
|
||||
Optics,
|
||||
/// The shape of the frame: crop, straighten, rotation, flips.
|
||||
Geometry,
|
||||
/// How the frame is composed: crop, straighten, rotation, flips.
|
||||
///
|
||||
/// Named for the decision rather than for the maths. The lens corrections
|
||||
/// are geometry too — distortion moves pixels exactly as a straighten
|
||||
/// does — and lumping the two together would file a correction the
|
||||
/// photographer never asked for beside a choice that is the whole reason
|
||||
/// they opened the photograph. [`Self::Optics`] is what the lens did;
|
||||
/// this is what they decided.
|
||||
Compose,
|
||||
/// Applied rather than corrected — a look, not a fix.
|
||||
Effect,
|
||||
}
|
||||
@@ -598,7 +605,7 @@ impl Attribute {
|
||||
Attribute::Colour,
|
||||
Attribute::Detail,
|
||||
Attribute::Optics,
|
||||
Attribute::Geometry,
|
||||
Attribute::Compose,
|
||||
Attribute::Effect,
|
||||
];
|
||||
|
||||
@@ -613,7 +620,7 @@ impl Attribute {
|
||||
Self::Colour => "attr.colour",
|
||||
Self::Detail => "attr.detail",
|
||||
Self::Optics => "attr.optics",
|
||||
Self::Geometry => "attr.geometry",
|
||||
Self::Compose => "attr.compose",
|
||||
Self::Effect => "attr.effect",
|
||||
})
|
||||
}
|
||||
@@ -630,7 +637,7 @@ impl Attribute {
|
||||
Self::Colour => "colour",
|
||||
Self::Detail => "detail",
|
||||
Self::Optics => "optics",
|
||||
Self::Geometry => "geometry",
|
||||
Self::Compose => "compose",
|
||||
Self::Effect => "effect",
|
||||
}
|
||||
}
|
||||
@@ -642,7 +649,15 @@ impl Attribute {
|
||||
"colour" => Self::Colour,
|
||||
"detail" => Self::Detail,
|
||||
"optics" => Self::Optics,
|
||||
"geometry" => Self::Geometry,
|
||||
"compose" => Self::Compose,
|
||||
// The name this attribute was persisted under before it was
|
||||
// called Compose. Settings written by an older build carry it in
|
||||
// `develop.copy_attributes`, and `from_name` returning `None`
|
||||
// there does not fail loudly — `presets::scope_for` logs and drops
|
||||
// the entry, silently narrowing what a paste carries. Accepted on
|
||||
// the way in only; `name` writes the current spelling, so a
|
||||
// settings file rewrites itself the first time it is saved.
|
||||
"geometry" => Self::Compose,
|
||||
"effect" => Self::Effect,
|
||||
_ => return None,
|
||||
})
|
||||
|
||||
@@ -989,6 +989,7 @@ mod tests {
|
||||
dr_types::ColourSpace::Srgb,
|
||||
&crate::mask::MaskStack::new(),
|
||||
&crate::spot::SpotSet::new(),
|
||||
&[],
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -78,7 +78,7 @@ static DESCRIPTOR: LazyLock<Arc<OpDescriptor>> = LazyLock::new(|| {
|
||||
Arc::new(OpDescriptor {
|
||||
// The shape of the frame, and the only operation that changes the
|
||||
// output's dimensions.
|
||||
attributes: vec![Attribute::Geometry],
|
||||
attributes: vec![Attribute::Compose],
|
||||
id: ID,
|
||||
label: LocalizedKey("op.framing"),
|
||||
params: vec![
|
||||
@@ -893,9 +893,16 @@ impl Framing {
|
||||
/// The WGSL mapping an output pixel to a **normalised centred** source
|
||||
/// position, ready for the warp chain.
|
||||
///
|
||||
/// Leaves the result in `p`: centre `(0, 0)`, `r == 1` at the corner —
|
||||
/// exactly the space [`crate::lens`] documents, so lens correction
|
||||
/// composes on top of this without either stage naming the other.
|
||||
/// Leaves the result in `p`: centre `(0, 0)`, spanning `±0.5 * aspect` —
|
||||
/// the space [`crate::lens`] documents, so lens correction composes on top
|
||||
/// of this without either stage naming the other.
|
||||
///
|
||||
/// **`p` is centred but not corner-normalised**, and this used to claim it
|
||||
/// was. Its length at the corner is `0.5 * length(aspect)`, not 1. The
|
||||
/// corner-normalised radius the radial corrections need is published
|
||||
/// separately as `radius` by `operation::sample_source`, which divides by
|
||||
/// exactly that; anything reading `length(p)` as a lens radius is off by
|
||||
/// an aspect-dependent factor.
|
||||
///
|
||||
/// `aspect` is left in scope alongside it, since the warp chain and the
|
||||
/// sampler both need it to return to texture coordinates.
|
||||
@@ -2312,7 +2319,7 @@ mod tests {
|
||||
// And the sampler's last step, which lives in `operation.rs` and is
|
||||
// the half of the map this file does not emit.
|
||||
assert!(
|
||||
crate::operation::sample_source(false).contains("p / aspect + vec2<f32>(0.5)"),
|
||||
crate::operation::sample_source(false, false).contains("p / aspect + vec2<f32>(0.5)"),
|
||||
"the sampler's return to texture coordinates moved"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ use crate::descriptor::{
|
||||
Attribute, Facet, LocalizedKey, OpDescriptor, OpId, ParamId, ParamKind, Presentation,
|
||||
};
|
||||
use crate::framing::{CropRect, Framing};
|
||||
use crate::lens::LensProfile;
|
||||
use crate::mask::MaskStack;
|
||||
use crate::operation::{compose_full, ComposedShader, Operation};
|
||||
use crate::ops;
|
||||
@@ -120,6 +121,33 @@ pub struct EditGraph {
|
||||
/// *where* an adjustment applies, and a spot says where a piece of the
|
||||
/// photograph comes from.
|
||||
spots: SpotSet,
|
||||
/// The lens corrections that rewrite coordinates: distortion and lateral
|
||||
/// chromatic aberration (`docs/architecture.md` §5.2).
|
||||
///
|
||||
/// Apart from `ops` for the fourth time, and this one is not about shape
|
||||
/// but about direction. Every [`Operation`] is a function from colour to
|
||||
/// colour, and these run *before* a colour exists: they decide which
|
||||
/// source pixel is read, and CA decides it three times over. See
|
||||
/// [`crate::lens`] for why that cannot be expressed as an operation.
|
||||
///
|
||||
/// Beside [`Self::framing`] in every way that matters — the two compose
|
||||
/// into one coordinate map, and neither can be applied without the other's
|
||||
/// result — but held separately because framing also changes the output's
|
||||
/// dimensions, which a warp never does.
|
||||
warps: Vec<Box<dyn crate::lens::Warp>>,
|
||||
/// The lens profile the corrections above were given, if any.
|
||||
///
|
||||
/// Kept as well as fanned out, because the corrections hold it in a form
|
||||
/// nothing can read back: each has folded its own share of the profile
|
||||
/// into private coefficients. Something has to be able to answer "is this
|
||||
/// photograph corrected from a measurement, or by hand?" — the interface
|
||||
/// is required to say so plainly rather than let an automatic correction
|
||||
/// silently do nothing — and this is the only place that can.
|
||||
///
|
||||
/// Not in [`EditState`], not in the sidecar, and not undoable: it is
|
||||
/// derived from the file's EXIF and a database, exactly as the film's
|
||||
/// baked tables are derived from a stock's id.
|
||||
lens_profile: Option<LensProfile>,
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3f
|
||||
@@ -164,6 +192,17 @@ impl EditGraph {
|
||||
masks: Arc::new(MaskStack::new()),
|
||||
film: None,
|
||||
spots: SpotSet::new(),
|
||||
// Distortion first, then CA, and the order is the correction's
|
||||
// rather than a preference. Each warp receives the position the
|
||||
// previous one produced, and lateral CA is a magnification about
|
||||
// the optical axis of the *undistorted* frame — measured on a
|
||||
// barrel-distorted one it would be fitted to a radius the lens
|
||||
// profile does not describe.
|
||||
warps: vec![
|
||||
Box::new(crate::ops::Distortion::new()),
|
||||
Box::new(crate::ops::Aberration::new()),
|
||||
],
|
||||
lens_profile: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -248,6 +287,53 @@ impl EditGraph {
|
||||
self.ops.iter().map(|o| o.descriptor()).collect()
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Apply a lens profile's measured coefficients to every correction that
|
||||
/// wants some, or clear them all with `None`.
|
||||
///
|
||||
/// **Derived state, not an edit.** A profile comes from the file's EXIF
|
||||
/// plus a database this crate does not link, so it is not a parameter, is
|
||||
/// not in the sidecar, and is not undoable. What *is* an edit is the manual
|
||||
/// trim beside it: each correction composes the profile with its own
|
||||
/// slider, so a photographer can lean on the measurement, override it, or
|
||||
/// work without one.
|
||||
///
|
||||
/// **Clearing matters as much as setting.** Opening a photograph from an
|
||||
/// unrecognised lens must pass `None` rather than simply not calling this:
|
||||
/// a graph reused across images would otherwise correct this frame for the
|
||||
/// optics of the last one, which is both wrong and invisible.
|
||||
///
|
||||
/// Fans out over the warps and the operations alike. Which trait a
|
||||
/// correction implements is a fact about where it sits relative to the
|
||||
/// fetch, and no business of the caller's — see
|
||||
/// [`crate::Operation::set_lens_profile`].
|
||||
pub fn set_lens_profile(&mut self, profile: Option<LensProfile>) {
|
||||
self.lens_profile = profile;
|
||||
for warp in &mut self.warps {
|
||||
warp.set_profile(profile.as_ref());
|
||||
}
|
||||
for op in &mut self.ops {
|
||||
op.set_lens_profile(profile.as_ref());
|
||||
}
|
||||
}
|
||||
|
||||
/// The profile currently applied, if any.
|
||||
pub fn lens_profile(&self) -> Option<&LensProfile> {
|
||||
self.lens_profile.as_ref()
|
||||
}
|
||||
|
||||
/// Descriptors for the coordinate-domain lens corrections, in order.
|
||||
///
|
||||
/// The counterpart to [`Self::descriptors`] and split from it for the same
|
||||
/// reason framing is absent there: a warp emits its own block in the
|
||||
/// generated shader rather than a colour fragment, so the codegen tests
|
||||
/// that count `---- ` markers have to know which kind they are counting.
|
||||
/// A UI wanting everything still reads [`Self::capabilities`], where all
|
||||
/// three kinds arrive together and indistinguishably (FR-DEV-3a).
|
||||
pub fn warp_descriptors(&self) -> Vec<Arc<OpDescriptor>> {
|
||||
self.warps.iter().map(|w| w.descriptor()).collect()
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3a | FR-DEV-3c
|
||||
/// Everything a UI needs to build its controls.
|
||||
///
|
||||
@@ -286,6 +372,39 @@ impl EditGraph {
|
||||
}
|
||||
});
|
||||
|
||||
// The lens corrections first, matching where they sit in the shader:
|
||||
// they rewrite the coordinate before any colour is fetched, so nothing
|
||||
// below them can be judged until they are right. It also puts the
|
||||
// three optical corrections together in the panel — these two and the
|
||||
// vignetting node, which is an ordinary operation and arrives above
|
||||
// through `ops`.
|
||||
let warps = self.warps.iter().map(|w| {
|
||||
let desc = w.descriptor();
|
||||
OpCapability {
|
||||
id: desc.id,
|
||||
label: desc.label,
|
||||
active: w.is_active(),
|
||||
params: desc
|
||||
.params
|
||||
.iter()
|
||||
.map(|p| ParamCapability {
|
||||
id: p.id,
|
||||
label: p.label,
|
||||
kind: p.kind.clone(),
|
||||
default: p.default,
|
||||
value: w.param(p.id),
|
||||
facet: p.facet,
|
||||
})
|
||||
.collect(),
|
||||
// No preferred widget. A distortion amount and the two CA
|
||||
// scales are ordinary scalars, and plain sliders — the
|
||||
// fallback every frontend implements — are the right control
|
||||
// for them.
|
||||
presentation: None,
|
||||
attributes: desc.attributes.clone(),
|
||||
}
|
||||
});
|
||||
|
||||
// Framing last, matching where it sits in the pipeline: the crop is
|
||||
// decided after the image looks right, not before.
|
||||
let desc = self.framing.descriptor();
|
||||
@@ -314,7 +433,7 @@ impl EditGraph {
|
||||
attributes: desc.attributes.clone(),
|
||||
};
|
||||
|
||||
ops.chain(std::iter::once(framing)).collect()
|
||||
warps.chain(ops).chain(std::iter::once(framing)).collect()
|
||||
}
|
||||
|
||||
/// Set a parameter, clamping to the descriptor's declared range.
|
||||
@@ -362,6 +481,15 @@ impl EditGraph {
|
||||
// nothing to register (FR-DEV-3c).
|
||||
ops: _,
|
||||
framing: _,
|
||||
// Reached through `capabilities` with the other two. A warp's
|
||||
// parameters are ordinary scalars once they are in that list, so
|
||||
// the sidecar, the clipboard and the undo stack carry them with
|
||||
// nothing registered anywhere (FR-DEV-3c).
|
||||
warps: _,
|
||||
// Derived from the file and a database, so it is rebuilt on open
|
||||
// rather than restored — the same reason the film's tables travel
|
||||
// as an id and not as numbers.
|
||||
lens_profile: _,
|
||||
masks,
|
||||
film,
|
||||
spots,
|
||||
@@ -440,6 +568,20 @@ impl EditGraph {
|
||||
return;
|
||||
}
|
||||
|
||||
// The warps, before the operations. Their ids cannot collide with an
|
||||
// operation's — `ops/` and the warp list are disjoint by construction,
|
||||
// and `declared_parity` asserts the chain is exactly what `ops/`
|
||||
// declares — so the order is for readability rather than precedence.
|
||||
if let Some(warp) = self.warps.iter_mut().find(|w| w.descriptor().id == op) {
|
||||
let descriptor = warp.descriptor();
|
||||
let Some(desc) = descriptor.param(param) else {
|
||||
log::warn!("unknown parameter {param} on {op}; ignoring");
|
||||
return;
|
||||
};
|
||||
warp.set_param(param, desc.clamp(value));
|
||||
return;
|
||||
}
|
||||
|
||||
let Some(operation) = self.ops.iter_mut().find(|o| o.descriptor().id == op) else {
|
||||
// A sidecar naming an operation this build does not have. The
|
||||
// rest of the edit must still apply.
|
||||
@@ -465,6 +607,9 @@ impl EditGraph {
|
||||
.param(param)
|
||||
.map(|_| self.framing.param(param));
|
||||
}
|
||||
if let Some(warp) = self.warps.iter().find(|w| w.descriptor().id == op) {
|
||||
return warp.descriptor().param(param).map(|_| warp.param(param));
|
||||
}
|
||||
self.ops
|
||||
.iter()
|
||||
.find(|o| o.descriptor().id == op)
|
||||
@@ -478,6 +623,11 @@ impl EditGraph {
|
||||
op.set_param(p.id, p.default);
|
||||
}
|
||||
}
|
||||
for warp in &mut self.warps {
|
||||
for p in &warp.descriptor().params {
|
||||
warp.set_param(p.id, p.default);
|
||||
}
|
||||
}
|
||||
self.framing.reset();
|
||||
// Masks go too, and this is why `apply` can be a replacement rather
|
||||
// than an overlay: a sidecar with no mask blocks means an edit with no
|
||||
@@ -542,7 +692,14 @@ impl EditGraph {
|
||||
/// graph renders to the screen and to a file in the same breath, and the
|
||||
/// two want different answers.
|
||||
pub fn compose_for(&self, output: dr_types::ColourSpace) -> ComposedShader {
|
||||
compose_full(&self.ops, &self.framing, output, &self.masks, &self.spots)
|
||||
compose_full(
|
||||
&self.ops,
|
||||
&self.framing,
|
||||
output,
|
||||
&self.masks,
|
||||
&self.spots,
|
||||
&self.warps,
|
||||
)
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-1
|
||||
@@ -637,6 +794,22 @@ impl EditGraph {
|
||||
u64::from(crate::operation::canonical_bits(self.framing.param(p.id))),
|
||||
);
|
||||
}
|
||||
// The warps belong to the geometry key, not the colour one: they decide
|
||||
// which source pixel a colour is read from, so a cached *result* of
|
||||
// this stage is wrong the moment one moves. Hashed by id as well as by
|
||||
// value, so two warps swapping their amounts is not the same edit.
|
||||
for warp in &self.warps {
|
||||
let descriptor = warp.descriptor();
|
||||
geometry = hash_bytes(geometry, descriptor.id.0.as_bytes());
|
||||
for p in &descriptor.params {
|
||||
geometry = hash_bytes(geometry, p.id.0.as_bytes());
|
||||
geometry = mix(
|
||||
geometry,
|
||||
u64::from(crate::operation::canonical_bits(warp.param(p.id))),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// The view rect is not a parameter and not in the structure key — it
|
||||
// is not an edit (see `Framing::view`). It is still an input to every
|
||||
// rendered pixel, so a cache that ignored it would show the wrong part
|
||||
@@ -823,20 +996,166 @@ mod tests {
|
||||
assert_ne!(first.uniforms, second.uniforms);
|
||||
}
|
||||
|
||||
/// The whole point of putting the warps in `capabilities`: everything that
|
||||
/// walks that list carries them, with nothing registered anywhere.
|
||||
#[test]
|
||||
fn a_warp_is_carried_by_the_machinery_it_never_told_about_itself() {
|
||||
use crate::ops::{aberration, distortion};
|
||||
|
||||
let mut g = EditGraph::default_chain();
|
||||
g.set_param(distortion::ID, distortion::AMOUNT, 40.0);
|
||||
g.set_param(aberration::ID, aberration::RED, 25.0);
|
||||
|
||||
assert_eq!(g.param(distortion::ID, distortion::AMOUNT), Some(40.0));
|
||||
assert_eq!(g.param(aberration::ID, aberration::RED), Some(25.0));
|
||||
|
||||
// Through the same capture/apply the sidecar, the clipboard and the
|
||||
// undo stack all use.
|
||||
let state = g.state();
|
||||
let mut restored = EditGraph::default_chain();
|
||||
// No film in this graph, so nothing is owed; the result is asserted
|
||||
// rather than dropped because ignoring it elsewhere would leave a
|
||||
// photograph rendering without its stock.
|
||||
assert_eq!(restored.set_state(&state), FilmRebake::NotNeeded);
|
||||
|
||||
assert_eq!(
|
||||
restored.param(distortion::ID, distortion::AMOUNT),
|
||||
Some(40.0),
|
||||
"a distortion correction did not survive a state round trip, so \
|
||||
reopening the photograph would silently drop it"
|
||||
);
|
||||
assert_eq!(restored.param(aberration::ID, aberration::RED), Some(25.0));
|
||||
}
|
||||
|
||||
/// A profile has to reach all three corrections, across both traits.
|
||||
///
|
||||
/// The failure this guards is the quiet one: a profile that reached the
|
||||
/// warps and not the vignetting node would correct the geometry and leave
|
||||
/// the corners dark, which looks like an under-corrected lens rather than
|
||||
/// like a wiring fault.
|
||||
#[test]
|
||||
fn a_lens_profile_reaches_every_correction_that_wants_one() {
|
||||
use crate::lens::{LensProfile, Tca};
|
||||
use crate::ops::{aberration, distortion, vignetting};
|
||||
|
||||
let mut g = EditGraph::default_chain();
|
||||
for id in [distortion::ID, aberration::ID, vignetting::ID] {
|
||||
assert_eq!(
|
||||
g.param(id, ParamId("amount")).unwrap_or(0.0),
|
||||
0.0,
|
||||
"{id} should start neutral"
|
||||
);
|
||||
}
|
||||
|
||||
g.set_lens_profile(Some(LensProfile {
|
||||
distortion: Some(distortion::PtLens {
|
||||
a: 0.0,
|
||||
b: -0.012,
|
||||
c: 0.0,
|
||||
}),
|
||||
tca: Some(Tca {
|
||||
red_scale: 1.000_32,
|
||||
blue_scale: 0.999_93,
|
||||
}),
|
||||
vignetting: Some(vignetting::Pa {
|
||||
k1: -0.42,
|
||||
k2: 0.05,
|
||||
k3: 0.0,
|
||||
}),
|
||||
}));
|
||||
|
||||
// Every correction is now doing something, with every slider still at
|
||||
// its default — which is the whole point of a profile.
|
||||
let source = g.compose().source;
|
||||
for marker in [
|
||||
"---- warp: distortion ----",
|
||||
"---- warp: aberration ----",
|
||||
"---- vignetting ----",
|
||||
] {
|
||||
assert!(
|
||||
source.contains(marker),
|
||||
"a profile did not reach {marker}: {source}"
|
||||
);
|
||||
}
|
||||
|
||||
// And clearing it puts the photograph back, which is what opening an
|
||||
// image from an unrecognised lens has to do.
|
||||
g.set_lens_profile(None);
|
||||
let cleared = g.compose().source;
|
||||
assert!(!cleared.contains("---- warp: "));
|
||||
assert!(!cleared.contains("---- vignetting ----"));
|
||||
assert!(g.lens_profile().is_none());
|
||||
}
|
||||
|
||||
/// A warp is an edit, so `reset` has to reach it. It did not until the
|
||||
/// loop was added: a reset that left the lens corrections standing would
|
||||
/// mean "back to the file as it is" quietly did not mean that.
|
||||
#[test]
|
||||
fn resetting_the_graph_neutralises_the_warps() {
|
||||
use crate::ops::distortion;
|
||||
|
||||
let mut g = EditGraph::default_chain();
|
||||
g.set_param(distortion::ID, distortion::AMOUNT, 40.0);
|
||||
g.reset();
|
||||
|
||||
assert_eq!(g.param(distortion::ID, distortion::AMOUNT), Some(0.0));
|
||||
}
|
||||
|
||||
/// The warps belong to the geometry key, not the colour one.
|
||||
///
|
||||
/// A tile cache keyed on geometry holds the *result* of the coordinate
|
||||
/// stage. Moving a distortion slider changes which source pixel every
|
||||
/// output pixel reads, so a cache that did not notice would keep drawing
|
||||
/// the previous correction — visibly, and only where it had already
|
||||
/// cached.
|
||||
#[test]
|
||||
fn a_warp_moves_the_geometry_key_and_leaves_the_others_alone() {
|
||||
use crate::operation::Affects;
|
||||
use crate::ops::distortion;
|
||||
|
||||
let mut g = EditGraph::default_chain();
|
||||
let before = g.invalidation();
|
||||
g.set_param(distortion::ID, distortion::AMOUNT, 40.0);
|
||||
let after = g.invalidation();
|
||||
|
||||
assert_ne!(
|
||||
before.of(Affects::Geometry),
|
||||
after.of(Affects::Geometry),
|
||||
"a distortion change must invalidate the geometry stage"
|
||||
);
|
||||
assert_eq!(
|
||||
before.of(Affects::Colour),
|
||||
after.of(Affects::Colour),
|
||||
"and must not invalidate the colour stage, which it does not touch"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn capabilities_describe_every_operation_and_parameter() {
|
||||
// The UI builds its whole panel from this. Anything missing here is
|
||||
// something the UI would have to hardcode.
|
||||
let g = EditGraph::default_chain();
|
||||
let caps = g.capabilities();
|
||||
// Every operation, plus framing — which is not an operation and so
|
||||
// is absent from `descriptors`, but must still reach the panel.
|
||||
assert_eq!(caps.len(), g.descriptors().len() + 1);
|
||||
// Every operation, plus everything that is *not* an operation and so
|
||||
// is absent from `descriptors`: the framing, and the coordinate-domain
|
||||
// lens corrections. All of them have parameters a photographer sets,
|
||||
// so all of them have to reach the panel — and the panel is forbidden
|
||||
// from naming any of them (FR-DEV-3a), which leaves this list as the
|
||||
// only way they can arrive.
|
||||
assert_eq!(caps.len(), g.descriptors().len() + g.warps.len() + 1);
|
||||
assert!(
|
||||
caps.iter().any(|c| c.id == crate::framing::ID),
|
||||
"framing must appear in the capability list, or the UI cannot \
|
||||
build a crop control without naming it"
|
||||
);
|
||||
for warp in &g.warps {
|
||||
let id = warp.descriptor().id;
|
||||
assert!(
|
||||
caps.iter().any(|c| c.id == id),
|
||||
"{id} must appear in the capability list, or its correction is \
|
||||
in the shader with no control anywhere that can reach it"
|
||||
);
|
||||
}
|
||||
|
||||
for cap in &caps {
|
||||
assert!(!cap.params.is_empty(), "{} exposes no parameters", cap.id);
|
||||
|
||||
@@ -41,6 +41,12 @@
|
||||
//! `(0, 0)`, and the radius is scaled so that `r == 1` at the corner. Both
|
||||
//! properties matter.
|
||||
//!
|
||||
//! Note which variable carries which. The shader's `p` supplies the centring
|
||||
//! and spans `±0.5 * aspect`; the corner normalisation is applied on top of it
|
||||
//! by `operation::sample_source`, which publishes the result as `radius`. A
|
||||
//! warp reading `length(p)` and calling it `r` would be evaluating its
|
||||
//! polynomial short of where the profile was fitted.
|
||||
//!
|
||||
//! Centring is what makes the polynomial meaningful — lens distortion is
|
||||
//! radially symmetric about the optical axis, so a formula written about any
|
||||
//! other origin would need cross terms to say the same thing.
|
||||
@@ -58,6 +64,32 @@ use std::sync::Arc;
|
||||
use crate::descriptor::{OpDescriptor, ParamId};
|
||||
use crate::operation::{Helper, Uniform};
|
||||
|
||||
/// Lateral chromatic aberration, as a per-channel radial scale.
|
||||
#[derive(Debug, Clone, Copy, PartialEq)]
|
||||
pub struct Tca {
|
||||
pub red_scale: f32,
|
||||
pub blue_scale: f32,
|
||||
}
|
||||
|
||||
/// What a lens profile says about one shot, in the pipeline's own types.
|
||||
///
|
||||
/// **Deliberately a mirror of `dr_lens::LensProfile` rather than that type
|
||||
/// itself.** The dependency would have to run the wrong way: `dr-lens` carries
|
||||
/// an XML parser and 5.5 MB of Lensfun data, and this crate is organised around
|
||||
/// having no dependencies so that its codegen is testable without a GPU or a
|
||||
/// database (ARCH §6.5a). Whatever sits above both does the conversion; it is
|
||||
/// nine fields and a `match`.
|
||||
///
|
||||
/// Every field is independently optional because the database is: it commonly
|
||||
/// carries distortion for a lens and no vignetting, or covers only part of a
|
||||
/// zoom. A partial profile is useful and must not be discarded wholesale.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Default)]
|
||||
pub struct LensProfile {
|
||||
pub distortion: Option<crate::ops::distortion::PtLens>,
|
||||
pub tca: Option<Tca>,
|
||||
pub vignetting: Option<crate::ops::vignetting::Pa>,
|
||||
}
|
||||
|
||||
/// A coordinate-domain operation, applied before the source is sampled.
|
||||
///
|
||||
/// Object-safe for the same reason [`crate::operation::Operation`] is: the
|
||||
@@ -112,6 +144,20 @@ pub trait Warp: Send + Sync {
|
||||
fn helpers(&self) -> &'static [Helper] {
|
||||
&[]
|
||||
}
|
||||
|
||||
/// Take whatever this warp needs from a lens profile.
|
||||
///
|
||||
/// Handed the *whole* profile rather than its own slice of it, so that the
|
||||
/// graph fanning one out does not have to know which correction wants
|
||||
/// which coefficients — the same reason an operation is handed a
|
||||
/// [`ParamId`] rather than a field. `None` clears any profile in place,
|
||||
/// which is what opening a photograph from an unrecognised lens must do:
|
||||
/// leaving the previous one standing would correct this frame for the
|
||||
/// optics of the last one.
|
||||
///
|
||||
/// Defaulted, because a warp need not be profile-driven. Nothing about the
|
||||
/// coordinate stage requires a database behind it.
|
||||
fn set_profile(&mut self, _profile: Option<&LensProfile>) {}
|
||||
}
|
||||
|
||||
/// The composed geometry stage: WGSL, uniforms, and what it needs from the
|
||||
|
||||
@@ -61,7 +61,7 @@ pub use detail::{
|
||||
pub use framing::{CropRect, Framing};
|
||||
pub use graph::{EditGraph, OpCapability, ParamCapability};
|
||||
pub use history::{Edit, Entry as HistoryEntry, History, Step};
|
||||
pub use lens::{compose_warps, ComposedWarp, Warp};
|
||||
pub use lens::{compose_warps, ComposedWarp, LensProfile, Tca, Warp};
|
||||
pub use operation::{
|
||||
compose, compose_with_framing, Affects, ComposedShader, Helper, Invalidation, Operation,
|
||||
OutputMode, Uniform, BASE_CURVE_POINTS, BASE_CURVE_UNIFORM_OFFSET, RESERVED_UNIFORM_FIELDS,
|
||||
|
||||
@@ -55,7 +55,7 @@ use std::fmt::Write as _;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::coverage::Coverage;
|
||||
use crate::descriptor::{OpDescriptor, ParamId};
|
||||
use crate::descriptor::{Attribute, OpDescriptor, ParamId};
|
||||
use crate::operation::Operation;
|
||||
use crate::ops;
|
||||
|
||||
@@ -777,8 +777,8 @@ pub struct MaskLayer {
|
||||
pub ops: Vec<Box<dyn Operation>>,
|
||||
}
|
||||
|
||||
/// The chain a mask layer holds: every point operation, and none of the
|
||||
/// neighbourhood ones.
|
||||
/// The chain a mask layer holds: every point operation, and neither the
|
||||
/// neighbourhood ones nor the optical corrections.
|
||||
///
|
||||
/// A layer's adjustments are fused into the colour dispatch and multiplied by
|
||||
/// the mask afterwards, which is exactly why a layer needs no per-operation
|
||||
@@ -795,10 +795,26 @@ pub struct MaskLayer {
|
||||
/// Filtering here means a local sharpening or denoise control simply does not
|
||||
/// appear until there is a stage that can honour it, which is the honest
|
||||
/// state of affairs.
|
||||
/// **The optical corrections are excluded on their own grounds**, which are
|
||||
/// not the neighbourhood argument above: they would work perfectly and mean
|
||||
/// nothing.
|
||||
///
|
||||
/// `Attribute::Optics` describes what the *lens* did to the whole frame. Its
|
||||
/// corrections are radial about the optical axis, and the `radius` they read
|
||||
/// is the distance from the centre of the photograph — a layer's mask does not
|
||||
/// and cannot move it. So a vignetting slider inside a mask would not correct
|
||||
/// the falloff within the selected region; it would lay a frame-centred radial
|
||||
/// ramp over the picture and then multiply it by the mask. That is a
|
||||
/// well-defined operation nobody would ever want, and worse, it is one whose
|
||||
/// name promises something else entirely.
|
||||
///
|
||||
/// The general rule the two exclusions share: a layer holds an operation only
|
||||
/// where restricting it to a region is a thing a photographer could mean.
|
||||
fn layer_chain() -> Vec<Box<dyn Operation>> {
|
||||
ops::chain()
|
||||
.into_iter()
|
||||
.filter(|o| o.detail().is_none())
|
||||
.filter(|o| !o.descriptor().attributes.contains(&Attribute::Optics))
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -1463,33 +1479,50 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn a_layer_offers_only_the_operations_it_can_actually_apply() {
|
||||
// Two exclusions, for two different reasons, and the test states both
|
||||
// because they fail in different ways.
|
||||
//
|
||||
// A layer's adjustments are fused into the colour dispatch and then
|
||||
// multiplied by the mask. A neighbourhood operation cannot take that
|
||||
// route: it is a dispatch of its own, run after the fused pass and
|
||||
// after the masks are already applied, so there is nowhere to hand it
|
||||
// one layer's mask.
|
||||
// one layer's mask. Left in, it moves and nothing happens.
|
||||
//
|
||||
// An optical correction *can* take that route, and that is the
|
||||
// problem. It is radial about the frame's optical axis, which a mask
|
||||
// cannot move, so it would lay a frame-centred ramp over the whole
|
||||
// picture and multiply it by the mask. Left in, it moves and the wrong
|
||||
// thing happens — under a name that promises the right one.
|
||||
//
|
||||
// The panel builds itself from `capabilities()` and names no
|
||||
// operation, so anything left in this chain becomes a control. One
|
||||
// that cannot work is worse than one that is missing: it moves, the
|
||||
// picture does not change, and nothing says why.
|
||||
// operation, so anything left in this chain becomes a control.
|
||||
let layer = lit_layer("m1", 1.0);
|
||||
let ids: Vec<&str> = layer.capabilities().iter().map(|c| c.id.0).collect();
|
||||
|
||||
let global = crate::ops::chain();
|
||||
let local_means_something = |op: &dyn Operation| {
|
||||
op.detail().is_none()
|
||||
&& !op
|
||||
.descriptor()
|
||||
.attributes
|
||||
.contains(&crate::descriptor::Attribute::Optics)
|
||||
};
|
||||
|
||||
for op in &global {
|
||||
let id = op.descriptor().id.0;
|
||||
assert_eq!(
|
||||
ids.contains(&id),
|
||||
op.detail().is_none(),
|
||||
"{id} is offered as a local adjustment but cannot be one, \
|
||||
or is a point operation and has gone missing from a layer"
|
||||
local_means_something(op.as_ref()),
|
||||
"{id} is offered as a local adjustment but cannot meaningfully \
|
||||
be one, or is an ordinary point operation and has gone \
|
||||
missing from a layer"
|
||||
);
|
||||
}
|
||||
assert!(
|
||||
ids.len() < global.len() || global.iter().all(|o| o.detail().is_none()),
|
||||
ids.len() < global.len()
|
||||
|| global.iter().all(|op| local_means_something(op.as_ref())),
|
||||
"the filter dropped nothing, so either it is not running or the \
|
||||
chain has no neighbourhood operation left to drop"
|
||||
chain has nothing left that a layer cannot carry"
|
||||
);
|
||||
|
||||
// And a clone must rebuild the same chain: it copies parameters across
|
||||
|
||||
@@ -377,6 +377,20 @@ pub trait Operation: Send + Sync {
|
||||
fn presentation(&self) -> Option<Presentation> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Take whatever this operation needs from a lens profile.
|
||||
///
|
||||
/// The counterpart of [`crate::lens::Warp::set_profile`], and it exists on
|
||||
/// this trait as well because the optical corrections do not all live on
|
||||
/// the same side of the fetch. Distortion and CA rewrite coordinates and
|
||||
/// are warps; vignetting applies a gain to the pixel already there and is
|
||||
/// an ordinary node. Splitting the fan-out by which trait a correction
|
||||
/// happens to implement would make the caller reason about that, so both
|
||||
/// traits carry the same door and `EditGraph::set_lens_profile` walks
|
||||
/// both lists the same way.
|
||||
///
|
||||
/// Defaulted: fourteen of the fifteen operations have nothing to take.
|
||||
fn set_lens_profile(&mut self, _profile: Option<&crate::lens::LensProfile>) {}
|
||||
}
|
||||
|
||||
/// A named WGSL helper function, deduplicated across operations.
|
||||
@@ -510,6 +524,11 @@ pub fn compose_with_framing(
|
||||
output,
|
||||
&MaskStack::new(),
|
||||
&crate::spot::SpotSet::new(),
|
||||
// No lens corrections. This entry point exists for callers that have
|
||||
// an operation chain and nothing else — the codegen tests, and the
|
||||
// export path before it grew a graph — and a warp is not something a
|
||||
// caller can hold without one.
|
||||
&[],
|
||||
)
|
||||
}
|
||||
|
||||
@@ -531,7 +550,13 @@ pub fn compose_full(
|
||||
output: ColourSpace,
|
||||
masks: &MaskStack,
|
||||
spots: &crate::spot::SpotSet,
|
||||
warps: &[Box<dyn crate::lens::Warp>],
|
||||
) -> ComposedShader {
|
||||
// The lens corrections, composed into one coordinate transform. Beside
|
||||
// `framing` because they are the other half of the same stage: framing
|
||||
// says which part of the source this output pixel comes from, and a warp
|
||||
// says where the lens put it once it got there.
|
||||
let warp = crate::lens::compose_warps(warps);
|
||||
// Active *point* operations. A neighbourhood operation is filtered out
|
||||
// here rather than asked for a fragment it cannot write: it reads pixels
|
||||
// it is not writing, so it belongs to the detail stage that runs after
|
||||
@@ -636,6 +661,19 @@ pub fn compose_full(
|
||||
);
|
||||
uniform_values.extend_from_slice(&framing.uniforms());
|
||||
|
||||
// The warps, immediately after framing and before any operation, matching
|
||||
// where they sit in the shader. Slot order is emission order and nothing
|
||||
// addresses a slot by number, so this only has to be consistent with
|
||||
// itself — but keeping it in pipeline order is what makes the generated
|
||||
// struct readable next to the generated body.
|
||||
uniform_fields.push_str(&warp.uniform_fields);
|
||||
uniform_values.extend_from_slice(&warp.uniforms);
|
||||
for h in &warp.helpers {
|
||||
if !helpers.iter().any(|existing| existing.name == h.name) {
|
||||
helpers.push(*h);
|
||||
}
|
||||
}
|
||||
|
||||
for op in &active {
|
||||
let id = op.descriptor().id.0;
|
||||
let prefix = sanitise(id);
|
||||
@@ -702,17 +740,35 @@ pub fn compose_full(
|
||||
|
||||
// The coordinate stage: output pixel -> source position -> colour. Emitted
|
||||
// ahead of the operation fragments, which receive the sampled `c`.
|
||||
let prologue = format!(
|
||||
"{}\n{}",
|
||||
framing.wgsl_prologue(),
|
||||
sample_source(framing.needs_interpolation())
|
||||
);
|
||||
let sampler_helper = if framing.needs_interpolation() {
|
||||
BILINEAR_HELPER
|
||||
// A warp puts an output pixel between source pixels exactly as a free
|
||||
// angle does, so either one forces the interpolating sampler. Asking
|
||||
// framing alone — which is what this did before the warps existed — would
|
||||
// have nearest-neighboured a distortion correction on an unstraightened
|
||||
// frame, and the aliasing would have looked like a bad profile.
|
||||
let interpolate = framing.needs_interpolation() || warp.is_active();
|
||||
|
||||
// Declared ahead of the warp block, which assigns to them. They enter
|
||||
// equal to `p` so that a chain mixing a splitting warp with a
|
||||
// non-splitting one still carries every earlier correction into the red
|
||||
// and blue paths — a distortion correction must move all three channels,
|
||||
// and only the CA that follows it may move them apart.
|
||||
let channel_positions = if warp.splits_channels {
|
||||
"\n // Per-channel source positions, for the lateral CA correction.\n\
|
||||
\x20 var p_r = p;\n\
|
||||
\x20 var p_b = p;\n"
|
||||
} else {
|
||||
""
|
||||
};
|
||||
|
||||
let prologue = format!(
|
||||
"{}{}{}\n{}",
|
||||
framing.wgsl_prologue(),
|
||||
channel_positions,
|
||||
warp.body,
|
||||
sample_source(interpolate, warp.splits_channels)
|
||||
);
|
||||
let sampler_helper = if interpolate { BILINEAR_HELPER } else { "" };
|
||||
|
||||
// The tail, and it is the whole of the difference between the two output
|
||||
// modes. Everything above — the prologue, the fragments, the mask layers,
|
||||
// the camera matrix — is emitted identically either way, so an operation
|
||||
@@ -1049,7 +1105,51 @@ fn encode_output(c: vec3<f32>) -> vec3<f32> {{
|
||||
/// Split out because it is the join between the coordinate stage and the
|
||||
/// colour stage, and because the choice it makes — an exact integer load, or
|
||||
/// a filtered sample — is the one thing the free-angle case changes.
|
||||
pub(crate) fn sample_source(interpolate: bool) -> &'static str {
|
||||
pub(crate) fn sample_source(interpolate: bool, splits_channels: bool) -> &'static str {
|
||||
if splits_channels {
|
||||
// Lateral chromatic aberration is a per-channel radial magnification,
|
||||
// so red and blue are fetched from positions green is not — which is
|
||||
// the whole reason a warp is not an `Operation`. By the time a colour
|
||||
// reaches an operation the three channels have been sampled together
|
||||
// and the divergence is gone.
|
||||
//
|
||||
// Green never moves. It is the reference the other two are scaled
|
||||
// about, so a correction that is wrong still leaves one channel sharp
|
||||
// rather than softening all three.
|
||||
return " // Back to texture coordinates, once per channel.
|
||||
let uv_src = p / aspect + vec2<f32>(0.5);
|
||||
let uv_r = p_r / aspect + vec2<f32>(0.5);
|
||||
let uv_b = p_b / aspect + vec2<f32>(0.5);
|
||||
|
||||
// Tested on green alone, not on all three.
|
||||
//
|
||||
// The three positions differ by a fraction of a pixel at any correction a
|
||||
// real lens needs, so testing each would only let the outermost row of the
|
||||
// frame disagree with itself about whether it exists — which draws a
|
||||
// coloured fringe along the edge, the exact artefact this is here to
|
||||
// remove. `sample_bilinear` clamps its own texel indices, so red and blue
|
||||
// land on the edge pixel rather than out of bounds.
|
||||
if (any(uv_src < vec2<f32>(0.0)) || any(uv_src >= vec2<f32>(1.0))) {
|
||||
textureStore(output, vec2<i32>(gid.xy), vec4<f32>(0.0, 0.0, 0.0, 1.0));
|
||||
return;
|
||||
}
|
||||
|
||||
// TRACES: FR-DEV-3f
|
||||
// Where this pixel sits on the *source*, in source pixels. Green's
|
||||
// position, since that is the one that did not move.
|
||||
let source_px = uv_src * vec2<f32>(src_dims);
|
||||
let radius = length(p) / (0.5 * length(aspect));
|
||||
|
||||
// Three fetches, one channel kept from each. Two thirds of the work is
|
||||
// discarded, which is why `Warp::splits_channels` exists: with no CA in
|
||||
// the chain the single-sample path below is emitted instead.
|
||||
var c = vec3<f32>(
|
||||
sample_bilinear(uv_r, src_dims).r,
|
||||
sample_bilinear(uv_src, src_dims).g,
|
||||
sample_bilinear(uv_b, src_dims).b,
|
||||
);
|
||||
";
|
||||
}
|
||||
if interpolate {
|
||||
" // Back to texture coordinates.
|
||||
let uv_src = p / aspect + vec2<f32>(0.5);
|
||||
@@ -1074,6 +1174,23 @@ pub(crate) fn sample_source(interpolate: bool) -> &'static str {
|
||||
let source_px = uv_src * vec2<f32>(src_dims);
|
||||
// A free angle puts output pixels between source pixels. Nearest-neighbour
|
||||
// here is what makes a straightened horizon stair-step, so interpolate.
|
||||
// Distance from the optical axis, normalised so the corner is exactly 1.
|
||||
//
|
||||
// Published beside `source_px` and for the same reason: a fragment is
|
||||
// handed a colour with no way back to a coordinate, and the radial
|
||||
// corrections need one. Derived from `p` after the whole coordinate stage,
|
||||
// so it measures the *source* frame — which is what a lens profile is
|
||||
// calibrated against, and why an off-centre crop still gets the falloff
|
||||
// its corner actually had rather than one centred on the crop.
|
||||
//
|
||||
// **The division is the part that is easy to leave out.** `p` spans
|
||||
// `+/-0.5 * aspect`, so at the corner its length is `0.5 * length(aspect)`
|
||||
// -- about 0.901 on a 3:2 frame, not 1. Lensfun's polynomials are fitted
|
||||
// against a corner radius of 1, so passing `length(p)` straight in
|
||||
// evaluates every one of them short of where it was measured, and by an
|
||||
// amount that changes with the aspect ratio. It reads as a correction that
|
||||
// is simply too weak, which is indistinguishable from a bad profile.
|
||||
let radius = length(p) / (0.5 * length(aspect));
|
||||
var c = sample_bilinear(uv_src, src_dims);
|
||||
"
|
||||
} else {
|
||||
@@ -1101,6 +1218,23 @@ pub(crate) fn sample_source(interpolate: bool) -> &'static str {
|
||||
// put, and its *amount* is handled separately by how much film a pixel
|
||||
// covers -- see `dr_film::Grain`.
|
||||
let source_px = uv_src * vec2<f32>(src_dims);
|
||||
// Distance from the optical axis, normalised so the corner is exactly 1.
|
||||
//
|
||||
// Published beside `source_px` and for the same reason: a fragment is
|
||||
// handed a colour with no way back to a coordinate, and the radial
|
||||
// corrections need one. Derived from `p` after the whole coordinate stage,
|
||||
// so it measures the *source* frame — which is what a lens profile is
|
||||
// calibrated against, and why an off-centre crop still gets the falloff
|
||||
// its corner actually had rather than one centred on the crop.
|
||||
//
|
||||
// **The division is the part that is easy to leave out.** `p` spans
|
||||
// `+/-0.5 * aspect`, so at the corner its length is `0.5 * length(aspect)`
|
||||
// -- about 0.901 on a 3:2 frame, not 1. Lensfun's polynomials are fitted
|
||||
// against a corner radius of 1, so passing `length(p)` straight in
|
||||
// evaluates every one of them short of where it was measured, and by an
|
||||
// amount that changes with the aspect ratio. It reads as a correction that
|
||||
// is simply too weak, which is indistinguishable from a bad profile.
|
||||
let radius = length(p) / (0.5 * length(aspect));
|
||||
var c = textureLoad(source, coord, 0).rgb;
|
||||
"
|
||||
}
|
||||
@@ -1393,6 +1527,139 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
use crate::lens::Warp as _;
|
||||
|
||||
/// A neutral warp list must leave the shader exactly as it was.
|
||||
///
|
||||
/// The property the whole `is_active` filter exists for: an unedited
|
||||
/// photograph keeps the integer `textureLoad` path, and pays nothing —
|
||||
/// not a bilinear fetch, not a uniform slot, not a recompile — for
|
||||
/// corrections nobody has asked for.
|
||||
#[test]
|
||||
fn warps_at_neutral_change_nothing_at_all() {
|
||||
let ops = crate::ops::chain();
|
||||
let framing = Framing::new();
|
||||
|
||||
let without = compose_full(
|
||||
&ops,
|
||||
&framing,
|
||||
ColourSpace::Srgb,
|
||||
&MaskStack::new(),
|
||||
&crate::spot::SpotSet::new(),
|
||||
&[],
|
||||
);
|
||||
let with_neutral = compose_full(
|
||||
&ops,
|
||||
&framing,
|
||||
ColourSpace::Srgb,
|
||||
&MaskStack::new(),
|
||||
&crate::spot::SpotSet::new(),
|
||||
&[
|
||||
Box::new(crate::ops::Distortion::new()) as Box<dyn crate::lens::Warp>,
|
||||
Box::new(crate::ops::Aberration::new()),
|
||||
],
|
||||
);
|
||||
|
||||
assert_eq!(without.source, with_neutral.source);
|
||||
assert_eq!(without.structure_hash, with_neutral.structure_hash);
|
||||
assert_eq!(without.uniforms, with_neutral.uniforms);
|
||||
assert!(
|
||||
!without.source.contains("sample_bilinear"),
|
||||
"an unwarped, unstraightened frame must keep the integer load path"
|
||||
);
|
||||
}
|
||||
|
||||
/// Distortion alone samples once; chromatic aberration samples three times.
|
||||
///
|
||||
/// `splits_channels` is the whole reason for this test. Lateral CA fetches
|
||||
/// red and blue from positions green is not, and paying that everywhere
|
||||
/// would triple the texture bandwidth of the common case — a distortion
|
||||
/// correction with no CA, which is most lens profiles.
|
||||
#[test]
|
||||
fn only_chromatic_aberration_splits_the_channels() {
|
||||
let compose_with = |warps: Vec<Box<dyn crate::lens::Warp>>| {
|
||||
compose_full(
|
||||
&crate::ops::chain(),
|
||||
&Framing::new(),
|
||||
ColourSpace::Srgb,
|
||||
&MaskStack::new(),
|
||||
&crate::spot::SpotSet::new(),
|
||||
&warps,
|
||||
)
|
||||
.source
|
||||
};
|
||||
|
||||
let mut distortion = crate::ops::Distortion::new();
|
||||
distortion.set_param(crate::ops::distortion::AMOUNT, 40.0);
|
||||
let only_distortion = compose_with(vec![Box::new(distortion)]);
|
||||
|
||||
assert!(
|
||||
only_distortion.contains("---- warp: distortion ----"),
|
||||
"an active distortion must reach the shader"
|
||||
);
|
||||
assert!(
|
||||
only_distortion.contains("sample_bilinear"),
|
||||
"a warp puts output pixels between source pixels, so it forces \
|
||||
the interpolating sampler even on an unstraightened frame"
|
||||
);
|
||||
assert!(
|
||||
!only_distortion.contains("var p_r"),
|
||||
"distortion moves all three channels together and must not pay \
|
||||
for the per-channel path"
|
||||
);
|
||||
|
||||
let mut ca = crate::ops::Aberration::new();
|
||||
ca.set_param(crate::ops::aberration::RED, 25.0);
|
||||
let with_ca = compose_with(vec![Box::new(ca)]);
|
||||
|
||||
assert!(
|
||||
with_ca.contains("var p_r"),
|
||||
"CA needs per-channel positions"
|
||||
);
|
||||
assert_eq!(
|
||||
with_ca.matches("sample_bilinear(").count(),
|
||||
// Three fetches in the body, plus the helper's own definition.
|
||||
4,
|
||||
"CA must fetch each channel from its own position"
|
||||
);
|
||||
}
|
||||
|
||||
/// An active warp is a different shader and must not reuse the cached one.
|
||||
///
|
||||
/// Covered by `hash_source` rather than by anything warp-specific — the
|
||||
/// body is written into the source — but asserted because the alternative
|
||||
/// failure is silent: the correction would simply never appear, exactly as
|
||||
/// a zoom did before `Framing::structure_key` gained its last bit.
|
||||
#[test]
|
||||
fn arming_a_warp_recompiles_but_moving_its_slider_does_not() {
|
||||
let compose_at = |amount: f32| {
|
||||
let mut d = crate::ops::Distortion::new();
|
||||
d.set_param(crate::ops::distortion::AMOUNT, amount);
|
||||
compose_full(
|
||||
&crate::ops::chain(),
|
||||
&Framing::new(),
|
||||
ColourSpace::Srgb,
|
||||
&MaskStack::new(),
|
||||
&crate::spot::SpotSet::new(),
|
||||
&[Box::new(d) as Box<dyn crate::lens::Warp>],
|
||||
)
|
||||
};
|
||||
|
||||
let neutral = compose_at(0.0);
|
||||
let armed = compose_at(40.0);
|
||||
let further = compose_at(70.0);
|
||||
|
||||
assert_ne!(
|
||||
neutral.structure_hash, armed.structure_hash,
|
||||
"arming a warp adds a block to the shader and must recompile"
|
||||
);
|
||||
assert_eq!(
|
||||
armed.structure_hash, further.structure_hash,
|
||||
"its magnitude is a uniform, so a drag must not recompile"
|
||||
);
|
||||
assert_ne!(armed.uniforms, further.uniforms);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn structure_hash_ignores_values_but_tracks_the_op_set() {
|
||||
// The property the shader cache depends on: moving a slider must not
|
||||
|
||||
@@ -141,6 +141,17 @@ impl Warp for Aberration {
|
||||
r != 1.0 || b != 1.0
|
||||
}
|
||||
|
||||
fn set_profile(&mut self, profile: Option<&crate::lens::LensProfile>) {
|
||||
// The inherent `set_profile` taking just this correction's own
|
||||
// coefficients, not this trait method: an inherent method wins over a
|
||||
// trait one of the same name, so this is a narrowing and not a loop.
|
||||
self.set_profile(
|
||||
profile
|
||||
.and_then(|p| p.tca)
|
||||
.map(|t| (t.red_scale, t.blue_scale)),
|
||||
);
|
||||
}
|
||||
|
||||
fn wgsl_body(&self) -> String {
|
||||
// `p_r` and `p_b` enter equal to `p` and are carried out of the block.
|
||||
// Green is deliberately absent: it is the reference and never moves.
|
||||
|
||||
@@ -136,6 +136,13 @@ impl Warp for Distortion {
|
||||
c.a != 0.0 || c.b != 0.0 || c.c != 0.0
|
||||
}
|
||||
|
||||
fn set_profile(&mut self, profile: Option<&crate::lens::LensProfile>) {
|
||||
// The inherent `set_profile` taking just this correction's own
|
||||
// coefficients, not this trait method: an inherent method wins over a
|
||||
// trait one of the same name, so this is a narrowing and not a loop.
|
||||
self.set_profile(profile.and_then(|p| p.distortion));
|
||||
}
|
||||
|
||||
fn wgsl_body(&self) -> String {
|
||||
// Written against `p`, which is already normalised and centred.
|
||||
"\
|
||||
|
||||
@@ -54,6 +54,14 @@
|
||||
//! than `Operation`, because they rewrite *coordinates* before the source is
|
||||
//! sampled rather than transforming a colour after it. They are not part of
|
||||
//! the develop chain and do not appear in `ops/`.
|
||||
//!
|
||||
//! [`vignetting`] is the exception, and the reason the split is drawn at
|
||||
//! coordinates rather than at "lens correction": it applies a gain to the
|
||||
//! pixel already fetched, so it is an ordinary node in `ops/` like any other.
|
||||
//! What it needs that a colour fragment is not otherwise given is the pixel's
|
||||
//! distance from the optical axis, which the sampler publishes as `radius`
|
||||
//! — corner-normalised there, because the profile coefficients are fitted
|
||||
//! against a corner radius of 1 and the prologue's `p` is not.
|
||||
|
||||
// Hand-written nodes. Each is listed in `ops/` with `rust:`, which is what
|
||||
// places it in the chain; these are the implementations that entry points at.
|
||||
|
||||
@@ -133,6 +133,13 @@ impl Operation for Vignetting {
|
||||
c.k1 != 0.0 || c.k2 != 0.0 || c.k3 != 0.0
|
||||
}
|
||||
|
||||
fn set_lens_profile(&mut self, profile: Option<&crate::lens::LensProfile>) {
|
||||
// The inherent `set_profile` taking just this correction's own
|
||||
// coefficients, not this trait method: an inherent method wins over a
|
||||
// trait one of the same name, so this is a narrowing and not a loop.
|
||||
self.set_profile(profile.and_then(|p| p.vignetting));
|
||||
}
|
||||
|
||||
fn wgsl_body(&self) -> String {
|
||||
// `radius` comes from the prologue: the pixel's distance from the
|
||||
// optical axis, normalised so the corner is 1.
|
||||
|
||||
@@ -59,7 +59,7 @@ use crate::graph::EditGraph;
|
||||
///
|
||||
/// It also dissolves a special case. Framing used to be excluded by an
|
||||
/// explicit test against one operation's id; it is now excluded because
|
||||
/// [`Attribute::Geometry`] is not in the default set, and the argument below
|
||||
/// [`Attribute::Compose`] is not in the default set, and the argument below
|
||||
/// is a statement about a kind of edit rather than about a particular node.
|
||||
///
|
||||
/// # Why geometry is out by default
|
||||
@@ -116,7 +116,7 @@ impl Scope {
|
||||
/// its shape. The default.
|
||||
pub fn adjustments() -> Self {
|
||||
Self {
|
||||
bits: Self::all_bits() & !Self::bit(Attribute::Geometry),
|
||||
bits: Self::all_bits() & !Self::bit(Attribute::Compose),
|
||||
carries_unclassified: true,
|
||||
}
|
||||
}
|
||||
@@ -278,7 +278,7 @@ impl Preset {
|
||||
pub fn touches_framing(&self) -> bool {
|
||||
self.params
|
||||
.keys()
|
||||
.any(|(op, _)| attributes_of(op).is_some_and(|a| a.contains(&Attribute::Geometry)))
|
||||
.any(|(op, _)| attributes_of(op).is_some_and(|a| a.contains(&Attribute::Compose)))
|
||||
}
|
||||
|
||||
/// How many operations this preset touches, in scope.
|
||||
@@ -1254,8 +1254,8 @@ mod tests {
|
||||
.into_iter()
|
||||
.filter(|a| !default_scope.has(*a))
|
||||
.collect();
|
||||
assert_eq!(excluded, vec![Attribute::Geometry]);
|
||||
assert!(Scope::everything().has(Attribute::Geometry));
|
||||
assert_eq!(excluded, vec![Attribute::Compose]);
|
||||
assert!(Scope::everything().has(Attribute::Compose));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -46,7 +46,7 @@ fn the_groups_are_derivable_from_the_chain() {
|
||||
);
|
||||
assert!(present.contains(&Attribute::Colour));
|
||||
assert!(
|
||||
present.contains(&Attribute::Geometry),
|
||||
present.contains(&Attribute::Compose),
|
||||
"framing is in the capability list and is geometry"
|
||||
);
|
||||
|
||||
|
||||
@@ -402,6 +402,7 @@ fn every_declared_node_is_checked() {
|
||||
"noise_reduction",
|
||||
"texture",
|
||||
"tone_curve",
|
||||
"vignetting",
|
||||
],
|
||||
"the set of hand-written nodes changed; if that is deliberate, update \
|
||||
this list and the module documentation above"
|
||||
|
||||
+44
-44
File diff suppressed because one or more lines are too long
@@ -34,6 +34,12 @@ dr-sync-nextcloud.workspace = true
|
||||
dr-export.workspace = true
|
||||
dr-ingest.workspace = true
|
||||
dr-film.workspace = true
|
||||
# The lens profile database, here for the same reason dr-film is: dr-pipeline
|
||||
# knows the maths of lens correction and deliberately has no dependency with
|
||||
# which to find out which coefficients belong to which lens. The conversion
|
||||
# between the two crates' mirrored coefficient types happens in `develop.rs`,
|
||||
# because it is the only place that can see both.
|
||||
dr-lens.workspace = true
|
||||
dr-pipeline.workspace = true
|
||||
dr-catalog.workspace = true
|
||||
# The face pipeline, with the ONNX runtime: this is the layer that actually
|
||||
|
||||
+245
-7
@@ -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,
|
||||
@@ -1021,15 +1032,15 @@ impl DevelopSession {
|
||||
/// (FR-DEV-3a). An attribute nothing carries is left out rather than
|
||||
/// offered as a tab that opens onto nothing.
|
||||
///
|
||||
/// Geometry is excluded: its one operation prefers an on-canvas widget and
|
||||
/// is skipped by the row builder, so a Geometry tab would be empty of rows
|
||||
/// while `GeometryPanel` holds the real controls.
|
||||
/// Compose is excluded: its one operation prefers an on-canvas widget and
|
||||
/// is skipped by the row builder, so a Compose tab would be empty of rows
|
||||
/// while `ComposePanel` holds the real controls.
|
||||
pub fn tabs(&self) -> Vec<(dr_pipeline::Attribute, String)> {
|
||||
use dr_pipeline::Attribute;
|
||||
let caps = self.scoped_capabilities();
|
||||
Attribute::ALL
|
||||
.into_iter()
|
||||
.filter(|a| *a != Attribute::Geometry)
|
||||
.filter(|a| *a != Attribute::Compose)
|
||||
.filter(|a| {
|
||||
caps.iter().any(|c| {
|
||||
c.attributes.contains(a)
|
||||
@@ -1171,7 +1182,7 @@ pub(crate) fn supported(widget: WidgetKind) -> bool {
|
||||
// Drawn in the panel.
|
||||
WidgetKind::ToneCurve => true,
|
||||
// Hosted on the canvas: the overlay is drawn over the photograph and
|
||||
// the panel contributes `GeometryPanel`, the affordance that turns it
|
||||
// the panel contributes `ComposePanel`, the affordance that turns it
|
||||
// on.
|
||||
WidgetKind::CropOverlay => true,
|
||||
// Not implemented. Listed rather than caught by a wildcard so the next
|
||||
@@ -1254,7 +1265,7 @@ pub(crate) fn rows_filtered(
|
||||
// terms, with nothing named.
|
||||
//
|
||||
// Skipped rather than rendered as an affordance row, because
|
||||
// the affordance is `GeometryPanel` — a bespoke control for a
|
||||
// the affordance is `ComposePanel` — a bespoke control for a
|
||||
// known stage, which is a thing the interface is entitled to
|
||||
// build (ARCH §4.3a draws the line at the *generated* panel
|
||||
// naming stages, not at the interface having hand-made
|
||||
@@ -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.
|
||||
///
|
||||
@@ -5748,7 +5986,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn framing_is_not_generated_as_sliders() {
|
||||
// `GeometryPanel` presents crop, rotation, flips and straightening as
|
||||
// `ComposePanel` presents crop, rotation, flips and straightening as
|
||||
// the gestures they are. If the generic path emitted them too the
|
||||
// sidebar would carry both — including four "Crop Left/Top/Width/
|
||||
// Height" sliders no one can compose a photograph with.
|
||||
|
||||
+15
-1
@@ -127,7 +127,7 @@ fn catalogued(key: &str) -> Option<&'static str> {
|
||||
"attr.colour" => "Colour",
|
||||
"attr.detail" => "Detail",
|
||||
"attr.optics" => "Optics",
|
||||
"attr.geometry" => "Geometry",
|
||||
"attr.compose" => "Compose",
|
||||
"attr.effect" => "Effects",
|
||||
|
||||
"op.white_balance" => "White Balance",
|
||||
@@ -146,6 +146,20 @@ fn catalogued(key: &str) -> Option<&'static str> {
|
||||
// see from there.
|
||||
"op.capture_sharpen" => "Sharpening",
|
||||
"op.framing" => "Crop & Rotate",
|
||||
// "Lens Vignetting", not the "Vignetting" `derive` would produce, and
|
||||
// the qualifier is doing real work. This operation corrects the corner
|
||||
// falloff a lens imposed; the creative vignette that darkens corners on
|
||||
// purpose is a different operation, belongs to `Effect` rather than
|
||||
// `Optics`, and will want the plain word when it exists. Naming this
|
||||
// one for what it corrects means the two can sit in the same panel
|
||||
// without either having to be renamed to make room.
|
||||
"op.vignetting" => "Lens Vignetting",
|
||||
// "Chromatic Aberration", not the "Aberration" `derive` would give.
|
||||
// The bare word names a whole family of lens defects — coma, spherical,
|
||||
// astigmatism — and this operation corrects exactly one of them. Under
|
||||
// it sit sliders called "Red" and "Blue", which only make sense once
|
||||
// the heading has said what is being separated.
|
||||
"op.aberration" => "Chromatic Aberration",
|
||||
|
||||
// Parameters
|
||||
"param.temperature" => "Temperature",
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
@@ -210,7 +210,7 @@ fn label_for(attribute: dr_pipeline::Attribute) -> &'static str {
|
||||
Attribute::Colour => "Colour",
|
||||
Attribute::Detail => "Detail",
|
||||
Attribute::Optics => "Optics",
|
||||
Attribute::Geometry => "Geometry",
|
||||
Attribute::Compose => "Compose",
|
||||
Attribute::Effect => "Effect",
|
||||
}
|
||||
}
|
||||
@@ -548,7 +548,7 @@ pub fn render(
|
||||
// holding no crop loses nothing to the setting, and saying so anyway would
|
||||
// train the user to ignore the line.
|
||||
window.set_settings_framing_withheld(
|
||||
clipboard.holds_framing() && !scope.has(dr_pipeline::Attribute::Geometry),
|
||||
clipboard.holds_framing() && !scope.has(dr_pipeline::Attribute::Compose),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -535,7 +535,7 @@ component ParamControl inherits Rectangle {
|
||||
// This does not weaken ARCH §4.3: nothing here reads a parameter *value* out
|
||||
// of a descriptor or routes by index. It calls named session actions, which is
|
||||
// what a bespoke widget for a known stage is entitled to do.
|
||||
export component GeometryPanel inherits Rectangle {
|
||||
export component ComposePanel inherits Rectangle {
|
||||
in property <bool> enabled: true;
|
||||
in property <float> angle: 0.0;
|
||||
in property <float> max-straighten: 45.0;
|
||||
@@ -586,7 +586,7 @@ export component GeometryPanel inherits Rectangle {
|
||||
// naming the group, flagging that it holds an edit, and offering the
|
||||
// reset — without the hiding.
|
||||
GroupHeading {
|
||||
title: "GEOMETRY";
|
||||
title: "COMPOSE";
|
||||
modified: root.modified;
|
||||
has-reset: root.modified;
|
||||
reset => { root.reset(); }
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { Theme } from "theme.slint";
|
||||
import { AdjustPanel, GeometryPanel, GroupStrip, ParamRow, TransferPanel, ViewMode } from "adjust.slint";
|
||||
import { AdjustPanel, ComposePanel, GroupStrip, ParamRow, TransferPanel, ViewMode } from "adjust.slint";
|
||||
import { CategoryRow, GradientHandle, GradientHandles, HandleRole, MaskPanel, MaskRow, SubjectRow } from "masks.slint";
|
||||
import { SpotHandle, SpotHandles, SpotPanel, SpotRole } from "spots.slint";
|
||||
import { CropOverlay } from "crop.slint";
|
||||
@@ -65,6 +65,12 @@ export component AppWindow inherits Window {
|
||||
// Current image, for the status strip and empty state.
|
||||
in property <string> filename: "";
|
||||
in property <string> camera: "";
|
||||
/// TRACES: FR-DEV-3
|
||||
/// The lens the file names, and whether a correction profile was found for
|
||||
/// it. Composed in Rust because the sentence depends on a database lookup
|
||||
/// this file cannot see, and because "not recorded" and "no profile" are
|
||||
/// different facts that must not be collapsed into one.
|
||||
in property <string> lens: "";
|
||||
in property <string> exposure: "";
|
||||
in property <string> dimensions: "";
|
||||
in property <int> index: 0;
|
||||
@@ -2416,6 +2422,7 @@ in property <bool> panel-visible: true;
|
||||
column := VerticalLayout {
|
||||
if !root.local-mode && !root.repairing: InfoPanel {
|
||||
camera: root.camera;
|
||||
lens: root.lens;
|
||||
exposure: root.exposure;
|
||||
dimensions: root.dimensions;
|
||||
}
|
||||
@@ -2471,7 +2478,7 @@ in property <bool> panel-visible: true;
|
||||
// made rather than how it is applied: the frame is decided
|
||||
// by eye first and the pipeline runs it last (see
|
||||
// `dr_pipeline::framing` on the coordinate order).
|
||||
if !root.local-mode && !root.repairing: GeometryPanel {
|
||||
if !root.local-mode && !root.repairing: ComposePanel {
|
||||
enabled: root.adjust-enabled;
|
||||
angle: root.straighten;
|
||||
max-straighten: root.max-straighten;
|
||||
|
||||
@@ -217,6 +217,17 @@ export component StatusBar inherits Rectangle {
|
||||
// wanted, and the per-group lids only stood between the user and the controls.
|
||||
export component InfoPanel inherits Rectangle {
|
||||
in property <string> camera;
|
||||
/// TRACES: FR-DEV-3
|
||||
/// The lens, and whether it matched a correction profile.
|
||||
///
|
||||
/// Here rather than beside the optical sliders, and the reason is what the
|
||||
/// line is *for*. `dr-lens` states the rule — an automatic correction that
|
||||
/// silently did nothing is worse than one the user can see is unavailable
|
||||
/// — and the fact it reports is a fact about the file: which lens took
|
||||
/// this photograph, and whether the database has heard of it. That is the
|
||||
/// same kind of thing as the body and the exposure, and it wants reading
|
||||
/// once on opening rather than hunting for under a group filter.
|
||||
in property <string> lens;
|
||||
in property <string> exposure;
|
||||
in property <string> dimensions;
|
||||
|
||||
@@ -239,6 +250,16 @@ export component InfoPanel inherits Rectangle {
|
||||
|
||||
Label { text: root.exposure; }
|
||||
|
||||
// Dim, like the dimensions below it: this is something the file says,
|
||||
// not something the photographer chose. An empty string draws nothing
|
||||
// — a session built from pixels with no header has no lens to report,
|
||||
// and an empty row is honest where "Unknown" would be noise.
|
||||
Caption {
|
||||
text: root.lens;
|
||||
visible: root.lens != "";
|
||||
wrap: word-wrap;
|
||||
}
|
||||
|
||||
Caption { text: root.dimensions; }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -121,7 +121,7 @@ export component HistoryPanel inherits Rectangle {
|
||||
spacing: Theme.gap-sm;
|
||||
alignment: start;
|
||||
|
||||
// A heading, not a collapsible. `GeometryPanel` carries the argument
|
||||
// A heading, not a collapsible. `ComposePanel` carries the argument
|
||||
// and it holds here: the list is last in the column, so what a lid
|
||||
// would save is scrolling past nothing.
|
||||
HorizontalLayout {
|
||||
|
||||
Reference in New Issue
Block a user