dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Let a preset name its film, and let a look reach only what it names

Browse Source 
A preset could not choose a film stock. The stock is a choice of material
rather than a parameter, so `Preset` — a map of `op.param = value` — had
nowhere to hold it, and "Portra 400, printed" could not be saved, copied
or shipped as a look. Worse, the film node's own sliders *were*
parameters: a paste moved one stock's exposure and push onto whatever
stock the target was on, and left the target's tables baked from the
values it had just replaced.

A preset now carries a `FilmRef` beside its parameters. It travels under
whichever scope carries the film node, so the stock and its sliders are
never split, and by the replacement rule every other parameter follows:
applied at that scope, a preset without a film develops the target
without one. `Preset::apply` returns the `FilmRebake` it owes, as
`EditGraph::set_state` already did, because this crate cannot bake a
stock; the develop session pays it before recording the step, and the
batch paste writes the stock into each sidecar through `film_for`. The
library file spells it `film =` / `film_print =`, as a sidecar does, and
an older build keeps those lines as ones it does not understand.

`EditState` keeps the film in its own field only: the parameters it
captures leave it out, so one edit has one place to say which stock it
is on.

Second, a preset now has a reach. Replacement is right for a copy of a
whole edit — "make these match" — and wrong for a look: a stock-only
"Portra 400" applied that way would put the photograph's exposure, white
balance and noise reduction back to default. `Reach::Named` replaces only
the operations a preset names (whole operations, so a look that sets the
blacks resets the whites beside them) and the film only if it names one.
Saved edits and the clipboard keep `Reach::Whole`; the line `reach =
named` is written only for the other, so existing libraries write the
same bytes.
This commit is contained in:
dtourolle
2026-09-26 13:44:32 -04:00
parent c6d4c1ba62
commit 90c0695c05
11 changed files with 503 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-pipeline/src/graph.rs
+8 -2
View File
@@ -623,7 +623,9 @@ impl EditGraph {
} = self;
EditState {
params: Preset::capture(self),
// Without the film: it has a field of its own below, and one
// edit must not have two places to disagree about its stock.
params: Preset::capture_params(self),
// A refcount bump. See `EditState::masks` for why that matters on
// a path called once a frame.
masks: Arc::clone(masks),
@@ -663,7 +665,11 @@ impl EditGraph {
// At full scope. `Scope` is a question about what a paste carries
// *between* photographs; this is one photograph's own edit being put
// back, so there is nothing to leave behind.
params.apply(self, Scope::everything());
//
// What the parameters say about the film is discarded: the stock is
// `film`'s to decide, below, and clearing it is what happens there
// either way.
let _ = params.apply(self, Scope::everything());
self.masks = Arc::clone(masks);
self.spots = spots.clone();
core/dr-pipeline/src/lib.rs
+1 -1
View File
@@ -69,7 +69,7 @@ pub use operation::{
OutputMode, Uniform, BASE_CURVE_POINTS, BASE_CURVE_UNIFORM_OFFSET, CLIP_ONSET,
RESERVED_UNIFORM_FIELDS, SAMPLE_CACHE_UNIFORM_OFFSET,
};
pub use preset::{LibraryParseError, NameError, Preset, PresetLibrary, Scope};
pub use preset::{LibraryParseError, NameError, Preset, PresetLibrary, Reach, Scope};
pub use sidecar::{Sidecar, Version};
pub use spot::{Spot, SpotMode, SpotSet};
pub use state::{EditState, FilmRebake, FilmRef};
core/dr-pipeline/src/preset.rs
+437 -23
View File
@@ -44,6 +44,7 @@ use std::fmt::Write as _;
use crate::descriptor::{Attribute, OpId, ParamId};
use crate::graph::EditGraph;
use crate::state::{FilmRebake, FilmRef};
/// Which parts of an edit a copy carries.
///
@@ -173,6 +174,15 @@ impl Scope {
self.bits == 0
}
/// Whether this scope carries the film — the stock as well as its sliders.
///
/// Asked of the film node's own classification rather than decided here,
/// so the stock travels under exactly the scopes its exposure slider
/// does. Splitting the two would put one stock's exposure on another.
pub fn carries_film(self) -> bool {
self.covers(crate::ops::film_sim::ID.0)
}
fn bit(attribute: Attribute) -> u8 {
1 << Attribute::ALL
.iter()
@@ -216,6 +226,17 @@ fn attributes_of(op: &str) -> Option<&'static [Attribute]> {
TABLE.get(op).map(Vec::as_slice)
}
/// How much of the target an applied preset replaces. See the module note.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub enum Reach {
/// Everything in scope: absence means default. A copy, a saved edit.
#[default]
Whole,
/// Only the operations the preset names, and the film if it names one.
/// Everything else in the target is left as it was. A look.
Named,
}
/// A set of non-default parameter values, ready to apply elsewhere.
///
/// Ordered, so two captures of the same edit compare equal and a caller can
@@ -223,6 +244,10 @@ fn attributes_of(op: &str) -> Option<&'static [Attribute]> {
#[derive(Debug, Clone, PartialEq, Default)]
pub struct Preset {
params: BTreeMap<(String, String), f32>,
reach: Reach,
/// TRACES: FR-DEV-3f
/// The stock this edit develops on, by id. See the module note.
film: Option<FilmRef>,
}
impl Preset {
@@ -237,6 +262,21 @@ impl Preset {
/// have to be re-copied to change one's mind about framing, and a preset
/// that had already discarded the crop could never grow it back.
pub fn capture(graph: &EditGraph) -> Self {
Self {
film: graph.film().map(|f| FilmRef {
stock: f.stock.clone(),
print: f.print.clone(),
}),
..Self::capture_params(graph)
}
}
/// The parameters alone, without the film.
///
/// For [`EditGraph::state`], which carries the film in a field of its own
/// ([`crate::EditState::film`]). Capturing it twice would give one edit
/// two places to disagree about its stock.
pub(crate) fn capture_params(graph: &EditGraph) -> Self {
let mut params = BTreeMap::new();
for cap in graph.capabilities() {
for p in &cap.params {
@@ -245,12 +285,59 @@ impl Preset {
}
}
}
Self { params }
Self::from_params(params)
}
/// Build from an already-captured parameter map — a sidecar's, typically.
pub fn from_params(params: BTreeMap<(String, String), f32>) -> Self {
Self { params }
Self {
params,
reach: Reach::Whole,
film: None,
}
}
/// The same preset, reaching as far as `reach` says.
pub fn with_reach(self, reach: Reach) -> Self {
Self { reach, ..self }
}
/// How much of the target this preset replaces.
pub fn reach(&self) -> Reach {
self.reach
}
/// Whether applying this preset replaces `op` — before any scope is asked.
fn reaches(&self, op: &str) -> bool {
match self.reach {
Reach::Whole => true,
Reach::Named => {
self.params.keys().any(|(o, _)| o == op)
|| (self.film.is_some() && op == crate::ops::film_sim::ID.0)
}
}
}
/// The same preset, developed on `film`.
pub fn with_film(self, film: Option<FilmRef>) -> Self {
Self { film, ..self }
}
/// The stock this preset develops on, if it names one.
pub fn film(&self) -> Option<&FilmRef> {
self.film.as_ref()
}
/// What applying at `scope` does to the target's film.
///
/// `None` when the scope leaves the film alone; `Some(None)` when it
/// develops the target without one. The same two levels the sidecar
/// writer takes, which is who asks: the batch path amends files rather
/// than graphs, and has to know whether the film line is being written at
/// all before it knows what to write.
pub fn film_for(&self, scope: Scope) -> Option<Option<&FilmRef>> {
(scope.carries_film() && self.reaches(crate::ops::film_sim::ID.0))
.then_some(self.film.as_ref())
}
/// The parameters, for a caller that stores them.
@@ -270,7 +357,7 @@ impl Preset {
/// answers is whether there is a clipboard to offer, which is why the UI
/// asks it before enabling a paste.
pub fn is_empty(&self) -> bool {
self.params.is_empty()
self.params.is_empty() && self.film.is_none()
}
/// How many parameters were captured.
@@ -296,10 +383,14 @@ impl Preset {
/// rather than parameters because thirty-six mixer sliders is a number
/// about the mixer's shape, not about how much was copied.
pub fn op_count(&self, scope: Scope) -> usize {
// A stock with every film slider at default is still the film node
// at work, and a preset holding nothing else is not "Neutral".
let film = self.film.is_some().then_some(crate::ops::film_sim::ID.0);
let mut ops: Vec<&str> = self
.params
.keys()
.map(|(op, _)| op.as_str())
.chain(film)
.filter(|op| scope.covers(op))
.collect();
ops.sort_unstable();
@@ -319,7 +410,13 @@ impl Preset {
/// to a fitted view mid-comparison, which reads as the paste having
/// navigated somewhere. This mirrors `DevelopSession::reset_framing`, and
/// lives here so every caller inherits it rather than each remembering.
pub fn apply(&self, graph: &mut EditGraph, scope: Scope) {
///
/// The **film** is replaced when the scope carries it, and handed back as
/// a [`FilmRebake`] because this crate cannot bake a stock — the same debt
/// [`EditGraph::set_state`] returns, for the same reason. It is cleared
/// even when the preset names the stock already on the graph: the tables
/// were baked from the film sliders this call has just replaced.
pub fn apply(&self, graph: &mut EditGraph, scope: Scope) -> FilmRebake {
let view = graph.framing().view();
// Clear the scope first, so absence means default (see the module
@@ -328,7 +425,7 @@ impl Preset {
let clears: Vec<(OpId, ParamId, f32)> = graph
.capabilities()
.iter()
.filter(|cap| scope.covers(cap.id.0))
.filter(|cap| scope.covers(cap.id.0) && self.reaches(cap.id.0))
.flat_map(|cap| cap.params.iter().map(|p| (cap.id, p.id, p.default)))
.collect();
for (op, param, default) in clears {
@@ -347,6 +444,17 @@ impl Preset {
}
graph.framing_mut().set_view(view);
match self.film_for(scope) {
None => FilmRebake::NotNeeded,
Some(film) => {
graph.set_film(None);
match film {
None => FilmRebake::NotNeeded,
Some(film) => FilmRebake::Wanted(film.clone()),
}
}
}
}
/// Apply to a parameter map — the sidecar of an image that is not open.
@@ -361,8 +469,11 @@ impl Preset {
/// Same replacement rule as [`Self::apply`]: the target's in-scope keys go,
/// the preset's arrive, and out-of-scope keys — the target's own crop, on
/// the default scope — are left exactly as they were.
///
/// A parameter map has no film in it, so the stock is not written here:
/// the caller asks [`Self::film_for`] and writes it beside the map.
pub fn amend(&self, target: &mut BTreeMap<(String, String), f32>, scope: Scope) {
target.retain(|(op, _), _| !scope.covers(op));
target.retain(|(op, _), _| !(scope.covers(op) && self.reaches(op)));
for ((op, param), value) in &self.params {
if scope.covers(op) {
target.insert((op.clone(), param.clone()), *value);
@@ -556,6 +667,14 @@ impl PresetLibrary {
self.presets.is_empty()
}
/// How many lines were kept without being understood.
///
/// For a file this build wrote itself, or shipped, that should be none: a
/// misspelt key is preserved faithfully and does nothing.
pub fn unread_lines(&self) -> usize {
self.unknown.values().map(Vec::len).sum()
}
/// Serialise to the on-disk form.
///
/// Deterministic, like the sidecar's: the same library always produces the
@@ -567,6 +686,22 @@ impl PresetLibrary {
for ((op, param), value) in preset.params() {
let _ = writeln!(out, "{op}.{param} = {}", format_value(*value));
}
// TRACES: FR-DEV-3f
// Spelled as the sidecar spells them, so a block can still be
// pasted from one file into the other. A build that predates
// these lines reads them as lines it does not understand and
// writes them back untouched, which is the promise below.
// Only when it differs from the default, so every library written
// before looks existed still writes the same bytes.
if preset.reach() == Reach::Named {
let _ = writeln!(out, "reach = named");
}
if let Some(film) = preset.film() {
let _ = writeln!(out, "film = {}", film.stock);
if let Some(print) = &film.print {
let _ = writeln!(out, "film_print = {print}");
}
}
for line in self.unknown.get(name).into_iter().flatten() {
let _ = writeln!(out, "{line}");
}
@@ -597,6 +732,8 @@ impl PresetLibrary {
let mut library = Self::default();
let mut current: Option<String> = None;
let mut params: BTreeMap<(String, String), f32> = BTreeMap::new();
let mut film: Option<FilmRef> = None;
let mut reach = Reach::Whole;
for line in lines {
let line = line.trim();
@@ -609,9 +746,16 @@ impl PresetLibrary {
.and_then(|l| l.strip_suffix(']'))
{
if let Some(name) = current.take() {
library.presets.insert(name, Preset::from_params(params));
params = BTreeMap::new();
library.presets.insert(
name,
Preset::from_params(std::mem::take(&mut params))
.with_film(film.take())
.with_reach(reach),
);
}
params.clear();
film = None;
reach = Reach::Whole;
// A name the writer should never have produced is dropped
// rather than taken: accepting it would mean writing a file
// back out that no longer parses as this one.
@@ -631,6 +775,23 @@ impl PresetLibrary {
};
match line.split_once('=') {
// TRACES: FR-DEV-3f
// Not checked against the installed stocks: this crate does
// not link them, and a preset naming a stock this device
// lacks must survive being stored here. Whoever bakes it
// reports the miss — the sidecar's rule, for its reason.
// `get_or_insert_with` because a hand-edited block may name
// the paper first.
Some((key, value)) if key.trim() == "reach" && value.trim() == "named" => {
reach = Reach::Named;
}
Some((key, value)) if key.trim() == "film" && !value.trim().is_empty() => {
film.get_or_insert_with(FilmRef::default).stock = value.trim().to_string();
}
Some((key, value)) if key.trim() == "film_print" && !value.trim().is_empty() => {
film.get_or_insert_with(FilmRef::default).print =
Some(value.trim().to_string());
}
Some((key, value)) => {
let key = key.trim();
let value = value.trim();
@@ -654,7 +815,21 @@ impl PresetLibrary {
}
if let Some(name) = current {
library.presets.insert(name, Preset::from_params(params));
library.presets.insert(
name,
Preset::from_params(params)
.with_film(film)
.with_reach(reach),
);
}
// A paper with no film named beside it is a print of nothing. Dropped
// rather than kept, since baking it would have no stock to start from.
for preset in library.presets.values_mut() {
if preset.film.as_ref().is_some_and(|f| f.stock.is_empty()) {
log::warn!("preset library: a paper without a film; ignoring it");
preset.film = None;
}
}
// A block whose every line was unreadable still produced a preset, and
@@ -803,7 +978,9 @@ mod tests {
};
target.set_crop(target_crop);
preset.apply(&mut target, Scope::adjustments());
preset
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(
target.param(exposure::ID, exposure::EXPOSURE),
@@ -835,7 +1012,9 @@ mod tests {
fn pasting_everything_carries_the_composition_too() {
let preset = Preset::capture(&edited());
let mut target = EditGraph::default_chain();
preset.apply(&mut target, Scope::everything());
preset
.apply(&mut target, Scope::everything())
.expect_no_film();
assert_eq!(target.param(framing::ID, framing::ANGLE), Some(-2.0));
assert_eq!(target.param(framing::ID, framing::KEYSTONE_V), Some(40.0));
@@ -857,7 +1036,9 @@ mod tests {
target.set_param(exposure::ID, exposure::EXPOSURE, 2.0);
target.set_param(saturation::ID, saturation::SATURATION, -50.0);
neutral.apply(&mut target, Scope::adjustments());
neutral
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(target.param(exposure::ID, exposure::EXPOSURE), Some(0.0));
assert_eq!(
@@ -876,7 +1057,9 @@ mod tests {
let mut target = EditGraph::default_chain();
target.set_param(framing::ID, framing::ANGLE, 3.5);
neutral.apply(&mut target, Scope::adjustments());
neutral
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(target.param(framing::ID, framing::ANGLE), Some(3.5));
}
@@ -894,7 +1077,9 @@ mod tests {
height: 0.25,
});
preset.apply(&mut target, Scope::everything());
preset
.apply(&mut target, Scope::everything())
.expect_no_film();
assert!(
target.framing().is_zoomed(),
"the paste threw away the viewport: {:?}",
@@ -911,7 +1096,9 @@ mod tests {
let mut sideways = EditGraph::default_chain();
sideways.set_orientation(dr_types::Orientation::from_exif(6));
preset.apply(&mut sideways, Scope::everything());
preset
.apply(&mut sideways, Scope::everything())
.expect_no_film();
assert_eq!(
sideways.framing().baseline(),
@@ -928,7 +1115,9 @@ mod tests {
params.insert(("exposure".to_string(), "exposure".to_string()), 1.25);
let mut target = EditGraph::default_chain();
Preset::from_params(params).apply(&mut target, Scope::adjustments());
Preset::from_params(params)
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(target.param(exposure::ID, exposure::EXPOSURE), Some(1.25));
}
@@ -939,7 +1128,9 @@ mod tests {
params.insert(("exposure".to_string(), "exposure".to_string()), 99.0);
let mut target = EditGraph::default_chain();
Preset::from_params(params).apply(&mut target, Scope::adjustments());
Preset::from_params(params)
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(target.param(exposure::ID, exposure::EXPOSURE), Some(5.0));
}
@@ -1001,11 +1192,13 @@ mod tests {
let mut target_map = Preset::capture(&target_graph).into_params();
preset.apply(&mut target_graph, scope);
preset.apply(&mut target_graph, scope).expect_no_film();
preset.amend(&mut target_map, scope);
let mut rebuilt = EditGraph::default_chain();
Preset::from_params(target_map).apply(&mut rebuilt, Scope::everything());
Preset::from_params(target_map)
.apply(&mut rebuilt, Scope::everything())
.expect_no_film();
for cap in target_graph.capabilities() {
for p in &cap.params {
@@ -1030,7 +1223,9 @@ mod tests {
let preset = Preset::capture(&source);
let mut target = EditGraph::default_chain();
preset.apply(&mut target, Scope::everything());
preset
.apply(&mut target, Scope::everything())
.expect_no_film();
for cap in source.capabilities() {
for p in &cap.params {
@@ -1068,6 +1263,188 @@ mod tests {
assert_eq!(excluded, vec![framing::ID.0]);
}
// --- the film ------------------------------------------------------------
/// A graph developing on `stock`. The tables are invented; what is under
/// test is whether the *choice* travels.
fn on_film(stock: &str) -> EditGraph {
let mut g = edited();
g.set_film(Some(crate::graph::Film {
stock: stock.to_string(),
print: Some("kodak_portra_endura".to_string()),
tables: crate::ops::FilmTables {
exposure_matrix: [[1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 1.0]],
curves: vec![[0.5, 0.5, 0.5]; crate::ops::film_sim::CURVE_SAMPLES],
curve_log_min: -3.0,
curve_log_max: 1.0,
lut: vec![[0.5, 0.5, 0.5]; 8],
density_max: 2.0,
lut_size: 2,
grain_particles: [0.0; 3],
grain_density_max: [2.0; 3],
grain_uniformity: 1.0,
},
}));
g
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
#[test]
fn a_copy_carries_the_stock_it_was_developed_on() {
let preset = Preset::capture(&on_film("kodak_portra_400"));
let film = preset.film().expect("the stock was captured");
assert_eq!(film.stock, "kodak_portra_400");
assert_eq!(film.print.as_deref(), Some("kodak_portra_endura"));
assert!(Preset::capture(&edited()).film().is_none());
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
/// The graph's own state keeps the film in one place only.
#[test]
fn an_edit_state_does_not_carry_the_film_twice() {
let state = on_film("kodak_portra_400").state();
assert!(state.film.is_some());
assert_eq!(state.params.film(), None);
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
/// Applying a preset that names a stock asks for it to be baked, and
/// clears the tables it found — they came from the sliders it replaced.
#[test]
fn applying_a_film_preset_asks_for_its_stock() {
let preset = Preset::capture(&on_film("kodak_portra_400"));
let mut target = on_film("ilford_hp5");
let rebake = preset.apply(&mut target, Scope::adjustments());
assert_eq!(
rebake.wanted().map(|f| f.stock.as_str()),
Some("kodak_portra_400")
);
assert!(target.film().is_none(), "the old tables were left standing");
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
/// Replacement, as for every parameter: a preset with no stock, at a
/// scope that carries the film, develops the target without one.
#[test]
fn a_preset_without_a_film_clears_the_targets() {
let mut target = on_film("ilford_hp5");
Preset::capture(&edited())
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert!(target.film().is_none());
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
/// A scope that leaves the film node behind leaves the stock behind too,
/// so an exposure never lands on a stock it was not set for.
#[test]
fn a_scope_without_the_film_leaves_the_stock_alone() {
let preset = Preset::capture(&on_film("kodak_portra_400"));
let mut target = on_film("ilford_hp5");
let tone = Scope::of([Attribute::Tone]);
assert!(!tone.carries_film());
preset.apply(&mut target, tone).expect_no_film();
assert_eq!(target.film().map(|f| f.stock.as_str()), Some("ilford_hp5"));
assert_eq!(preset.film_for(tone), None);
assert!(preset.film_for(Scope::adjustments()).is_some());
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
#[test]
fn a_stock_alone_is_not_a_neutral_preset() {
let preset = Preset::default().with_film(Some(FilmRef {
stock: "kodak_portra_400".into(),
print: None,
}));
assert!(!preset.is_empty());
assert_eq!(preset.op_count(Scope::adjustments()), 1);
assert_eq!(preset.op_count(Scope::of([Attribute::Tone])), 0);
}
// --- reach ---------------------------------------------------------------
/// A look: a stock and a contrast, and nothing else.
fn look() -> Preset {
let mut params = BTreeMap::new();
params.insert(("contrast".to_string(), "contrast".to_string()), 20.0);
Preset::from_params(params)
.with_film(Some(FilmRef {
stock: "kodak_portra_400".into(),
print: None,
}))
.with_reach(Reach::Named)
}
/// TRACES: FR-DEV-6
/// The reason `Reach` exists: a look applied over a corrected photograph
/// keeps the correction.
#[test]
fn a_look_leaves_what_it_does_not_name_alone() {
let mut target = EditGraph::default_chain();
target.set_param(exposure::ID, exposure::EXPOSURE, 1.25);
target.set_param(saturation::ID, saturation::SATURATION, -40.0);
let rebake = look().apply(&mut target, Scope::adjustments());
assert_eq!(target.param(exposure::ID, exposure::EXPOSURE), Some(1.25));
assert_eq!(
target.param(saturation::ID, saturation::SATURATION),
Some(-40.0)
);
assert_eq!(
rebake.wanted().map(|f| f.stock.as_str()),
Some("kodak_portra_400")
);
}
/// TRACES: FR-DEV-6
/// A look without a film leaves the target's film alone, where a whole
/// edit without one would clear it.
#[test]
fn a_look_without_a_film_keeps_the_targets() {
let mut target = on_film("ilford_hp5");
let only_contrast = look().with_film(None);
assert_eq!(only_contrast.film_for(Scope::adjustments()), None);
only_contrast
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(target.film().map(|f| f.stock.as_str()), Some("ilford_hp5"));
}
/// TRACES: FR-DEV-6
/// The batch path agrees: a look amends only the operations it names.
#[test]
fn a_look_amends_only_what_it_names() {
let mut target = BTreeMap::new();
target.insert(("exposure".to_string(), "exposure".to_string()), 1.25);
target.insert(("contrast".to_string(), "contrast".to_string()), -50.0);
look().amend(&mut target, Scope::adjustments());
assert_eq!(
target.get(&("exposure".to_string(), "exposure".to_string())),
Some(&1.25)
);
assert_eq!(
target.get(&("contrast".to_string(), "contrast".to_string())),
Some(&20.0)
);
}
/// TRACES: FR-DEV-6
#[test]
fn a_look_survives_the_library_round_trip() {
let mut lib = named();
lib.insert("Look", look()).unwrap();
let text = lib.to_text();
assert!(text.contains("reach = named\n"), "{text}");
let back = PresetLibrary::parse(&text).unwrap();
assert_eq!(back, lib);
// And the default is not written, so an existing library's bytes are
// unchanged by this format ever having grown the line.
assert_eq!(named().to_text().matches("reach").count(), 0);
}
// -----------------------------------------------------------------------
// Named presets
// -----------------------------------------------------------------------
@@ -1147,6 +1524,37 @@ mod tests {
assert!(out.contains("not_an_op.not_a_param = 0.25"), "{out}");
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
#[test]
fn a_film_survives_the_library_round_trip() {
let mut lib = named();
lib.insert("Portra", Preset::capture(&on_film("kodak_portra_400")))
.unwrap();
let text = lib.to_text();
assert!(text.contains("film = kodak_portra_400\n"), "{text}");
assert!(
text.contains("film_print = kodak_portra_endura\n"),
"{text}"
);
assert_eq!(PresetLibrary::parse(&text).unwrap(), lib);
}
/// TRACES: FR-DEV-6 | FR-DEV-3f
/// The paper may come first in a hand-edited block, and a paper with no
/// film is dropped rather than baked from nothing.
#[test]
fn film_lines_read_in_either_order_and_a_lone_paper_is_dropped() {
let text = format!(
"drpl {LIBRARY_FORMAT_VERSION}\n\n[preset A]\nfilm_print = p\nfilm = s\n\n\
[preset B]\nfilm_print = p\nexposure.exposure = 1\n"
);
let lib = PresetLibrary::parse(&text).unwrap();
let a = lib.get("A").unwrap().film().unwrap();
assert_eq!((a.stock.as_str(), a.print.as_deref()), ("s", Some("p")));
assert_eq!(lib.get("B").unwrap().film(), None);
assert_eq!(lib.get("B").unwrap().len(), 1);
}
#[test]
fn a_file_from_a_newer_build_is_refused_rather_than_guessed_at() {
let text = format!("drpl {}\n", LIBRARY_FORMAT_VERSION + 1);
@@ -1234,7 +1642,9 @@ mod tests {
let preset = stored.get("Warm portrait").unwrap();
let mut target = EditGraph::default_chain();
preset.apply(&mut target, Scope::adjustments());
preset
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(target.param(exposure::ID, exposure::EXPOSURE), Some(0.75));
// The target keeps its own framing on the default scope.
assert_eq!(target.param(framing::ID, framing::ANGLE), Some(0.0));
@@ -1268,7 +1678,9 @@ mod tests {
fn a_hand_picked_scope_carries_only_the_kinds_it_names() {
let preset = Preset::capture(&edited());
let mut target = EditGraph::default_chain();
preset.apply(&mut target, Scope::of([Attribute::Tone]));
preset
.apply(&mut target, Scope::of([Attribute::Tone]))
.expect_no_film();
// Tone was picked, so the exposure travelled.
assert_eq!(
@@ -1333,7 +1745,9 @@ mod tests {
let mut target = EditGraph::default_chain();
target.set_param(exposure::ID, exposure::EXPOSURE, -1.25);
Preset::capture(&edited()).apply(&mut target, empty);
Preset::capture(&edited())
.apply(&mut target, empty)
.expect_no_film();
assert_eq!(
target.param(exposure::ID, exposure::EXPOSURE),
Some(-1.25),
core/dr-pipeline/src/starter.rs
+3 -1
View File
@@ -169,7 +169,9 @@ mod tests {
assert!(!preset.is_empty(), "{name} carries nothing");
let mut graph = EditGraph::default_chain();
preset.apply(&mut graph, Scope::adjustments());
preset
.apply(&mut graph, Scope::adjustments())
.expect_no_film();
assert_ne!(
Preset::capture(&graph),
Preset::default(),
core/dr-preset-xmp/src/lib.rs
+4 -1
View File
@@ -472,7 +472,10 @@ mod tests {
// second kind of thing with a second apply path.
let import = read_xmp(ATTRIBUTE_FORM).unwrap();
let mut graph = EditGraph::default_chain();
import.preset.apply(&mut graph, Scope::adjustments());
import
.preset
.apply(&mut graph, Scope::adjustments())
.expect_no_film();
assert_eq!(
graph.param(
dr_pipeline::ops::exposure::ID,
docs/dev/traceability.md
+28 -28
View File
File diff suppressed because one or more lines are too long
ui/dr-ui/src/develop/render.rs
+3 -1
View File
@@ -547,7 +547,9 @@ impl DevelopSession {
/// photograph nobody asked for, so every caller restores from a
/// [`EditGraph::state`] taken first.
pub(super) fn strip_adjustments(&mut self) {
Preset::default().apply(&mut self.graph, Scope::adjustments());
// The empty preset names no stock, so what it owes is nothing; the
// film is cleared through the session below.
let _ = Preset::default().apply(&mut self.graph, Scope::adjustments());
*self.graph.masks_mut() = dr_pipeline::mask::MaskStack::new();
*self.graph.spots_mut() = dr_pipeline::SpotSet::new();
// Through the session rather than the graph: the baked tables live on
ui/dr-ui/src/develop/session.rs
+6 -1
View File
@@ -584,7 +584,12 @@ impl DevelopSession {
/// The panel must be rebuilt from [`Self::rows`] afterwards: a paste moves
/// values the sliders are showing, and nothing here pushes them.
pub fn apply_settings(&mut self, preset: &Preset, scope: Scope) {
preset.apply(&mut self.graph, scope);
let rebake = preset.apply(&mut self.graph, scope);
// TRACES: FR-DEV-3f
// Before the step is recorded, so the history holds the photograph as
// it now renders — on the preset's stock, baked from the film sliders
// that just arrived with it.
self.pay_film_debt(&rebake);
// A paste is undoable, and is the action most in need of it: it
// replaces everything in scope at once, so getting it wrong costs more
// than any single control can.
ui/dr-ui/src/library/sidecar.rs
+5 -3
View File
@@ -47,11 +47,13 @@ pub enum Amendment {
preset: dr_pipeline::Preset,
scope: dr_pipeline::Scope,
/// TRACES: FR-DEV-3f
/// The film stock, when this is an image's own edit being written back.
/// The film stock, when this write concerns it: an image's own edit
/// being written back, or a paste at a scope that carries the film
/// (`Preset::film_for`).
///
/// Two levels of `Option`, and both are load-bearing. The outer says
/// whether this write concerns the film at all — a paste does not,
/// exactly as it carries no masks. The inner is the choice itself, and
/// whether this write concerns the film at all — a paste that leaves
/// the film node behind does not. The inner is the choice itself, and
/// `Some(None)` is a real edit: "develop this normally again". Without
/// the distinction, clearing a film could never be saved.
film: Option<Option<dr_pipeline::sidecar::FilmRef>>,
ui/dr-ui/src/library_ui/ratings_keywords.rs
+5 -5
View File
@@ -592,11 +592,11 @@ fn collect_settings_writes(
// against one photograph and describes nothing on another.
// The target keeps whatever local adjustments it already had.
masks: None,
// A paste carries no film either, and for a plainer reason
// than the masks: a preset is a parameter map, and a stock is
// not a parameter. Pasting one would be pasting a choice the
// clipboard never captured.
film: None,
// TRACES: FR-DEV-3f
// The film travels where its sliders do: a stock is a look,
// not a fact about one frame, and a preset carrying one is
// how "Portra, printed" is applied to forty photographs.
film: preset.film_for(scope).map(|f| f.cloned()),
// Nor snapshots: they are states of the photograph they were
// taken on, and mean nothing on another.
snapshots: None,
ui/dr-ui/src/presets.rs
+3 -1
View File
@@ -1581,7 +1581,9 @@ mod tests {
let preset = presets.get("Warm").expect("stored");
let mut target = EditGraph::default_chain();
preset.apply(&mut target, Scope::adjustments());
preset
.apply(&mut target, Scope::adjustments())
.expect_no_film();
assert_eq!(
target.param(
dr_pipeline::ops::exposure::ID,