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

Merge integration into wip/ingest

Browse Source 
Second pass, against the detail-stage and thumbnail work that has landed since
the first. Resolved and verified here rather than in the shared merge worktree,
so what goes back is a fast-forward.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

# Conflicts:
#	docs/traceability.md
This commit is contained in:
dtourolle
2026-08-22 19:44:42 +02:00
parent d91f1ec277 42f1f55fd3
commit 56dd3187f1
47 changed files with 11460 additions and 395 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/develop.rs
+399 -50
View File
@@ -273,6 +273,20 @@ pub struct DevelopSession {
/// attributes leaves it at — the tabs are the interface's idea, not the
/// core's, and nothing breaks without them (ARCH §4.3a).
active_tab: Option<dr_pipeline::Attribute>,
/// TRACES: FR-DEV-3
/// Which of the curve widget's subjects the panel is plotting.
///
/// The tone curve is four curves — one over tone and one per colour
/// channel — and one square plot draws one of them at a time. The index
/// is into the subjects the operation's parameters are faceted on, in the
/// order it declares them, so nothing here knows that "red" exists.
///
/// **Interface state, not part of the edit.** It changes no pixel, so it
/// is not a parameter, it is not in the graph, it is not in the sidecar
/// and it is not on the undo stack — the same standing as which tab is
/// open. One value rather than one per operation, for the same reason
/// `curve_samples` is one polyline: the panel draws one curve.
curve_channel: usize,
}
impl DevelopSession {
@@ -339,6 +353,7 @@ impl DevelopSession {
active_mask: None,
show_overlay: false,
active_tab: None,
curve_channel: 0,
}
}
@@ -349,8 +364,12 @@ impl DevelopSession {
pub fn rows(&self) -> Vec<ParamRow> {
let caps = self.scoped_capabilities();
match self.active_tab {
Some(attribute) => rows_filtered(&caps, |op| op.attributes.contains(&attribute)),
None => rows_from(&caps),
Some(attribute) => rows_filtered(
&caps,
|op| op.attributes.contains(&attribute),
self.curve_channel,
),
None => rows_filtered(&caps, |_| true, self.curve_channel),
}
}
@@ -384,8 +403,10 @@ impl DevelopSession {
.into_iter()
.filter(|a| *a != Attribute::Geometry)
.filter(|a| {
caps.iter()
.any(|c| c.attributes.contains(a) && !rows_filtered(&caps, |o| o.attributes.contains(a)).is_empty())
caps.iter().any(|c| {
c.attributes.contains(a)
&& !rows_filtered(&caps, |o| o.attributes.contains(a), 0).is_empty()
})
})
.map(|a| (a, crate::labels::resolve(a.label().0)))
.collect()
@@ -494,8 +515,13 @@ pub(crate) fn supported(widget: WidgetKind) -> bool {
/// FR-DEV-3c acceptance test asks for — an operation the frontend has never
/// heard of appearing in a generated panel — and it cannot be asserted at all
/// if generating a row requires a device.
///
/// `#[cfg(test)]` since the panel began passing the selected curve down: the
/// session always has one to pass, and a wrapper that quietly picked the first
/// would be a second answer to a question the session already answers.
#[cfg(test)]
pub(crate) fn rows_from(caps: &[OpCapability]) -> Vec<ParamRow> {
rows_filtered(caps, |_| true)
rows_filtered(caps, |_| true, 0)
}
/// The panel model for the capabilities `keep` accepts.
@@ -508,9 +534,16 @@ pub(crate) fn rows_from(caps: &[OpCapability]) -> Vec<ParamRow> {
///
/// Getting that backwards is how a slider ends up driving a different
/// operation, which is the kind of fault that looks like a rendering bug.
///
/// `curve_channel` is which subject a multi-subject widget is showing — the
/// tone curve's four curves are one plot with a selector over it. It is passed
/// in rather than read from anywhere because this function is deliberately
/// free-standing: the descriptor-to-panel path has to be exercisable against a
/// hand-built capability list with no session behind it.
pub(crate) fn rows_filtered(
caps: &[OpCapability],
keep: impl Fn(&OpCapability) -> bool,
curve_channel: usize,
) -> Vec<ParamRow> {
let mut rows = Vec::new();
for (op_index, op) in caps.iter().enumerate() {
@@ -558,7 +591,9 @@ pub(crate) fn rows_filtered(
// to the core stops this compiling until someone has decided,
// here, whether the panel draws it.
let row = match widget {
WidgetKind::ToneCurve => curve_row(op_index, group_head, op, presentation),
WidgetKind::ToneCurve => {
curve_row(op_index, group_head, op, presentation, curve_channel)
}
// Canvas-hosted kinds returned above; the rest are not
// implemented and reached sliders via `choose`.
WidgetKind::ColourWheel
@@ -671,8 +706,76 @@ pub(crate) fn rows_filtered(
rows
}
/// One run of a curve widget's parameters: the points of a single curve.
///
/// A widget may span several curves — the tone curve is one plot over a master
/// curve and three colour channels — and it says so the way the colour mixer
/// says it has twelve bands: by faceting each parameter with the *subject* it
/// acts on. Consecutive parameters sharing a subject are one curve.
struct CurveRun {
/// The subject's localisation key, or `None` where the widget's parameters
/// carry no facet at all and are therefore a single unnamed curve.
subject: Option<&'static str>,
/// Where this run's points begin in the operation's parameter list. What
/// a drag routes back through, so it must be a position in `op.params`
/// and not in the presentation's list.
base: usize,
/// How many coordinates it holds.
len: usize,
}
/// TRACES: FR-DEV-3a
/// The curves a curve widget spans, in the order the operation declares them.
///
/// **This is the whole of the panel's knowledge of colour channels: none.** It
/// groups by whatever subject the parameters carry, so an operation offering a
/// master curve and three channels gets a four-way selector, one offering a
/// single unfaceted curve gets no selector at all, and one that grows a fifth
/// curve tomorrow needs no change here.
///
/// Returns `None` where the parameters do not look like point coordinates —
/// an odd count, a run that is not contiguous in the capability list — in
/// which case the caller falls back to sliders rather than drawing a widget
/// over a layout it has guessed at.
fn curve_runs(op: &OpCapability, presentation: &Presentation) -> Option<Vec<CurveRun>> {
// Points are x/y pairs, so an odd count means the operation and this code
// disagree about the layout.
if presentation.params.len() < 2 || !presentation.params.len().is_multiple_of(2) {
log::warn!("{}: curve widget needs an even parameter count", op.id);
return None;
}
let mut runs: Vec<CurveRun> = Vec::new();
for id in presentation.params {
// The widget addresses points by offset from the first of its run, so
// a run has to be contiguous in the capability list.
let at = op.params.iter().position(|p| p.id == *id)?;
let subject = op.params[at].facet.as_ref().map(|f| f.subject.0);
match runs.last_mut() {
Some(run) if run.subject == subject && run.base + run.len == at => run.len += 1,
_ => runs.push(CurveRun {
subject,
base: at,
len: 1,
}),
}
}
if runs.iter().any(|r| !r.len.is_multiple_of(2)) {
log::warn!("{}: a curve's points are not contiguous", op.id);
return None;
}
Some(runs)
}
/// One row standing for a whole curve.
///
/// `channel` picks which of the widget's curves is plotted; it is clamped
/// rather than validated, because the selection is interface state that
/// outlives a change of photograph and the new image's operation may have
/// fewer curves than the old one's.
///
/// Returns `None` if the operation's parameters do not look like point
/// coordinates, in which case the caller falls back to sliders rather than
/// rendering a broken widget.
@@ -681,38 +784,21 @@ fn curve_row(
group_head: usize,
op: &OpCapability,
presentation: &Presentation,
channel: usize,
) -> Option<ParamRow> {
// Points are x/y pairs, so an odd count means the operation and this
// code disagree about the layout.
if presentation.params.len() < 2 || !presentation.params.len().is_multiple_of(2) {
log::warn!("{}: curve widget needs an even parameter count", op.id);
return None;
}
let runs = curve_runs(op, presentation)?;
let run = runs.get(channel.min(runs.len().saturating_sub(1)))?;
// The widget addresses points by offset from the first, so they must
// be contiguous in the capability list.
let base = op
.params
let points: Vec<f32> = op.params[run.base..run.base + run.len]
.iter()
.position(|p| p.id == presentation.params[0])?;
for (i, id) in presentation.params.iter().enumerate() {
if op.params.get(base + i).map(|p| p.id) != Some(*id) {
log::warn!("{}: curve parameters are not contiguous", op.id);
return None;
}
}
let points: Vec<f32> = presentation
.params
.iter()
.filter_map(|id| op.params.iter().find(|p| p.id == *id))
.map(|p| p.value)
.collect();
Some(ParamRow {
op_index: op_index as i32,
// The first point parameter; the widget offsets from here.
param_index: base as i32,
// The first point parameter *of the curve on show*; the widget offsets
// from here, so switching curve is what re-points the drag.
param_index: run.base as i32,
op_label: labels::resolve(op.label.0).into(),
param_label: String::new().into(),
// A widget spanning a whole operation is not a row in anyone's
@@ -733,21 +819,97 @@ fn curve_row(
precision: 4,
unit: String::new().into(),
points: slint::ModelRc::new(slint::VecModel::from(points)),
// A curve is not a choice between named alternatives.
// A curve is not a choice between named alternatives. The curves it
// can switch between are named on the panel rather than on the row —
// see `DevelopSession::curve_channels` for why they cannot ride here.
choices: no_choices(),
})
}
impl DevelopSession {
/// The curve's shape, sampled for drawing.
/// TRACES: FR-DEV-3
/// The names of the curves the widget can switch between.
///
/// Empty where there is only one, which is also the answer for a frontend
/// with no curve at all: a selector over a single choice is a row of
/// nothing.
///
/// **Derived from the facets, so nothing here names a colour channel.**
/// The operation says its forty points are one control applied to four
/// subjects and publishes a localisation key for each; this resolves the
/// keys and hands over four words. An operation that grew a fifth curve
/// would appear here on its own.
///
/// A panel property rather than a field on the curve's `ParamRow`, and the
/// reason is Slint's: a row's models are compared by identity, so a fresh
/// list of names built on every parameter event would make the row look
/// changed every time, and rewriting a row rebuilds the repeater item
/// underneath it — destroying the `TouchArea` holding the drag in
/// progress. The same hazard `rows`'s in-place point update exists to
/// avoid. Nothing in this list is a drag target, so up here it is safe to
/// replace wholesale, exactly as [`Self::curve_samples`] is.
pub fn curve_channels(&self) -> Vec<String> {
for op in &self.scoped_capabilities() {
let Some(presentation) = &op.presentation else {
continue;
};
if presentation.choose(supported) != Some(WidgetKind::ToneCurve) {
continue;
}
let Some(runs) = curve_runs(op, presentation) else {
continue;
};
if runs.len() < 2 {
continue;
}
return runs
.iter()
.map(|r| r.subject.map(labels::resolve).unwrap_or_default())
.collect();
}
Vec::new()
}
/// Which curve the widget is plotting, as an index into
/// [`Self::curve_channels`].
pub fn curve_channel(&self) -> i32 {
self.curve_channel as i32
}
/// Plot a different one of the operation's curves.
///
/// Out-of-range indices are ignored rather than clamped: the only thing
/// that can send one is a stale interface event, and quietly moving the
/// selection somewhere the user did not point is worse than doing nothing.
pub fn set_curve_channel(&mut self, index: i32) {
let Ok(index) = usize::try_from(index) else {
return;
};
if index < self.curve_channels().len() {
self.curve_channel = index;
}
}
/// The plotted curve's shape, sampled for drawing.
///
/// Evaluated with `dr_pipeline`'s own spline, so the line the user drags
/// is the line the shader applies. The alternative — reading the curve
/// back off the GPU — is the round-trip ARCH §6.1 forbids, to draw a
/// polyline.
///
/// The line drawn is the *selected* curve's own shape, not the composition
/// of it with the master. Two curves overlaid on one grid is a plot of two
/// things, and the one being dragged has to be the one whose points are
/// under the pointer.
pub fn curve_samples(&self) -> Vec<f32> {
const SAMPLES: usize = 96;
// The selection is an index over the subjects the panel found, which
// for this operation is its channel order. Clamped rather than
// trusted: a selection made on one photograph outlives the change to
// the next.
let channel = curve::Channel::ALL[self.curve_channel.min(curve::CHANNELS - 1)];
let mut xs = [0.0f32; curve::POINTS];
let mut ys = [0.0f32; curve::POINTS];
let mut found = false;
@@ -757,16 +919,18 @@ impl DevelopSession {
continue;
}
found = true;
for (i, p) in cap.params.iter().enumerate() {
let point = i / 2;
if point >= curve::POINTS {
break;
}
if i % 2 == 0 {
xs[point] = p.value;
} else {
ys[point] = p.value;
}
// By id rather than by position, so which curve is plotted is
// decided by naming it and not by arithmetic over the parameter
// list.
let value = |id| {
cap.params
.iter()
.find(|p| p.id == id)
.map_or(0.0, |p| p.value)
};
for i in 0..curve::POINTS {
xs[i] = value(curve::coordinate(channel, i, curve::Axis::X));
ys[i] = value(curve::coordinate(channel, i, curve::Axis::Y));
}
}
if !found {
@@ -891,11 +1055,30 @@ impl DevelopSession {
/// The mask array is rasterised in source space at proxy size and sampled
/// through the framing map, so one array is correct at every output size:
/// a 256px thumbnail and a 24 MP export bind the same texture.
///
/// **And the detail stage with it.** The neighbourhood operations — noise
/// reduction, capture sharpening, and the rest of FR-DEV-3's kernels —
/// cannot be fused into the single dispatch, so an edit using one composes
/// a fused pass that hands on *linear* values and a chain of passes that
/// finishes the job (see `dr_pipeline::detail`). Those two halves must be
/// composed from one graph and dispatched together, or the fused shader's
/// storage format does not match the texture bound to it; going through
/// `render_detailed` here is what makes that true of every path at once.
/// It falls through to the plain render when the chain is empty, which is
/// almost every edit, so this costs nothing to the frames that do not
/// need it.
///
/// `space` has to be the space `shader` was composed for. It is the last
/// pass of the detail chain that performs the output transform when there
/// is one, so the two would otherwise be free to disagree about which
/// primaries the file is in — and the result would be a correctly
/// labelled file with the wrong colours in it (FR-EXP-2).
fn render_with_masks(
&mut self,
shader: &dr_pipeline::operation::ComposedShader,
w: u32,
h: u32,
space: dr_types::ColourSpace,
) -> Result<(), String> {
let ctx = self.ctx.clone();
self.ensure_subject_fields(&ctx);
@@ -905,8 +1088,32 @@ impl DevelopSession {
.then(|| self.masks.as_ref().and_then(|p| p.array()))
.flatten();
// The neighbourhood stage, composed at the size actually being drawn.
//
// It has to be composed *per render* rather than cached with the edit,
// because a kernel is the one thing in this pipeline that is not
// scale-free: a sharpening radius is stated in source pixels and the
// develop view renders at whatever the viewport needs (FR-DSP-1), so
// the conversion is different for the canvas, the thumbnail and the
// export. `render_scale` works the ratio out from the framing, so a
// crop and a zoom are already accounted for, and zooming to 1:1
// restores an exact preview with no second render path to maintain.
//
// Empty for every edit with no active neighbourhood operation — which
// is almost all of them — and `render_detailed` then falls straight
// through to the single masked dispatch this used to call.
let scale = self.graph.render_scale(self.demosaiced.size(), (w, h));
let detail = self.graph.compose_detail_for(scale, space);
// Detail passes read what the colour pass wrote, so the key they are
// cached against is the colour key: moving a sharpening slider re-runs
// this stage and not the fused one (FR-DEV-3d).
let colour_key = self
.graph
.invalidation()
.through(dr_pipeline::Affects::Colour);
self.adjust
.render_masked(&self.demosaiced, shader, w, h, masks)
.render_detailed(&self.demosaiced, shader, w, h, masks, &detail, colour_key)
.map(|_| ())
.map_err(|e| e.to_string())
}
@@ -1605,7 +1812,7 @@ impl DevelopSession {
// Rasterise the masks first: the shader addresses array slices by
// index, so the array has to describe *this* stack before it is bound.
self.render_with_masks(&shader, w, h)?;
self.render_with_masks(&shader, w, h, dr_types::ColourSpace::Srgb)?;
let texture = self.adjust.output().ok_or("nothing was rendered")?;
// The import is fallible on format and usage only, and both are fixed
@@ -1729,7 +1936,7 @@ impl DevelopSession {
let (w, h) = self.graph.output_size(sw, sh);
let shader = self.graph.compose_for(space);
self.render_with_masks(&shader, w, h)?;
self.render_with_masks(&shader, w, h, space)?;
let (pixels, rw, rh) = self.adjust.export_pixels().map_err(|e| e.to_string())?;
dr_export::Frame::in_space(rw, rh, pixels, space).map_err(|e| e.to_string())
@@ -1756,7 +1963,7 @@ impl DevelopSession {
let (w, h) = fit(fw, fh, edge.max(1), edge.max(1));
let shader = self.graph.compose_for(dr_types::ColourSpace::Srgb);
self.render_with_masks(&shader, w, h)?;
self.render_with_masks(&shader, w, h, dr_types::ColourSpace::Srgb)?;
let (pixels, rw, rh) = self.adjust.export_pixels().map_err(|e| e.to_string())?;
Ok((rw, rh, pixels))
@@ -3419,9 +3626,9 @@ mod tests {
#[test]
fn the_curve_collapses_to_a_single_row() {
// Ten point parameters must appear as one curve control, not ten
// sliders — otherwise the widget and the sliders both render and the
// panel shows the same values twice.
// Every point parameter — all four curves' worth — must appear as one
// curve control, not as forty sliders. Otherwise the widget and the
// sliders both render and the panel shows the same values twice.
let graph = EditGraph::default_chain();
let curve_cap = graph
.capabilities()
@@ -3429,7 +3636,11 @@ mod tests {
.find(|c| c.id == curve::ID)
.expect("the chain includes a tone curve");
assert_eq!(curve_cap.params.len(), curve::POINTS * 2);
assert_eq!(
curve_cap.params.len(),
curve::CHANNELS * curve::POINTS * 2,
"a master curve and one per colour channel"
);
let presentation = curve_cap
.presentation
.as_ref()
@@ -3469,6 +3680,144 @@ mod tests {
}
}
/// The panel's whole knowledge of colour channels, asserted to be none.
///
/// It groups the widget's parameters by the subject the *operation* put on
/// them and finds four curves; nothing below says "red", and an operation
/// that grew a fifth curve would arrive here on its own.
#[test]
fn a_curve_widget_offers_one_run_per_subject() {
let graph = EditGraph::default_chain();
let cap = graph
.capabilities()
.into_iter()
.find(|c| c.id == curve::ID)
.expect("tone curve present");
let presentation = cap.presentation.as_ref().expect("declares a widget");
let runs = curve_runs(&cap, presentation).expect("a curve-shaped operation");
assert_eq!(runs.len(), curve::CHANNELS);
for (i, run) in runs.iter().enumerate() {
assert_eq!(run.len, curve::POINTS * 2, "run {i} is not five points");
assert_eq!(run.base, i * curve::POINTS * 2);
assert!(run.subject.is_some(), "run {i} is unnamed");
}
}
#[test]
fn switching_curve_repoints_the_row() {
use slint::Model as _;
// What a drag routes through. The row's `param_index` is the base of
// the curve *on show*, so picking a different one must move it — if it
// did not, dragging a point on the red curve would write to the
// master's.
let graph = EditGraph::default_chain();
let caps = graph.capabilities();
let curve_at = caps
.iter()
.position(|c| c.id == curve::ID)
.expect("tone curve present");
let mut bases = Vec::new();
for channel in 0..curve::CHANNELS {
let rows = rows_filtered(&caps, |_| true, channel);
let row = rows
.iter()
.find(|r| r.op_index as usize == curve_at)
.expect("the curve has a row");
assert_eq!(row.kind, "curve");
assert_eq!(
row.points.row_count(),
curve::POINTS * 2,
"one curve's points, not all four curves'"
);
bases.push(row.param_index);
}
assert_eq!(
bases,
(0..curve::CHANNELS)
.map(|i| (i * curve::POINTS * 2) as i32)
.collect::<Vec<_>>()
);
}
#[test]
fn a_selection_the_operation_cannot_honour_falls_back_to_its_last_curve() {
// The selection outlives the photograph it was made on, and the next
// image's operation may offer fewer curves. Clamping keeps a plot on
// the grid; the alternative is a curve row that vanishes, which reads
// as the tone curve having disappeared from the panel.
let graph = EditGraph::default_chain();
let caps = graph.capabilities();
let rows = rows_filtered(&caps, |_| true, 99);
let row = rows
.iter()
.find(|r| r.kind == "curve")
.expect("the curve still has a row");
assert_eq!(
row.param_index,
((curve::CHANNELS - 1) * curve::POINTS * 2) as i32
);
}
#[test]
fn an_operation_whose_points_are_unfaceted_is_one_curve() {
// A curve widget that spans a single unnamed curve — which is what
// this operation was before the channels arrived, and what any other
// node declaring a `tone_curve` widget over ten scalars would be.
// It must draw, and it must offer no choice.
use dr_pipeline::{LocalizedKey, ParamCapability, WidgetDemand};
use slint::Model as _;
static IDS: [ParamId; 4] = [
ParamId("p0_x"),
ParamId("p0_y"),
ParamId("p1_x"),
ParamId("p1_y"),
];
let param = |id: ParamId| ParamCapability {
id,
label: LocalizedKey("param.point"),
kind: ParamKind::Scalar {
min: 0.0,
max: 1.0,
scale: dr_pipeline::Scale::Linear,
unit: Unit::None,
precision: 4,
},
default: 0.0,
value: 0.0,
facet: None,
};
let plain = OpCapability {
id: OpId("invented_curve"),
label: LocalizedKey("op.invented_curve"),
active: false,
presentation: Some(Presentation {
widgets: &[WidgetKind::ToneCurve],
demand: WidgetDemand {
two_dimensional: true,
precise_pointing: true,
},
params: &IDS,
}),
params: IDS.iter().map(|id| param(*id)).collect(),
attributes: &[dr_pipeline::Attribute::Tone],
};
let presentation = plain.presentation.as_ref().expect("declares a widget");
let runs = curve_runs(&plain, presentation).expect("curve-shaped");
assert_eq!(runs.len(), 1, "one unnamed curve");
assert_eq!(runs[0].subject, None);
let rows = rows_from(&[plain]);
assert_eq!(rows.len(), 1);
assert_eq!(rows[0].kind, "curve");
assert_eq!(rows[0].points.row_count(), IDS.len());
}
#[test]
fn curve_samples_start_on_the_diagonal() {
// A fresh curve is the identity, so the drawn line must be the 45°
ui/dr-ui/src/export.rs
+68 -6
View File
@@ -617,8 +617,15 @@ fn export_one(
issued: &mut HashSet<String>,
cancel: &Cancel,
) -> Option<Result<Placed, ItemError>> {
let (stem, date, frame) = match source {
Source::Rendered { stem, frame } => (stem, String::new(), frame),
// TRACES: FR-EXP-8
// The fourth element is what the photograph's own file said about itself.
// A library image is decoded here, so it has one; a frame handed over
// already rendered does not — the develop session holds pixels and an edit
// graph, not the header they came from, so an export from the develop
// button carries only what `dr-export` writes about itself until that is
// plumbed through the session.
let (stem, date, frame, source_metadata) = match source {
Source::Rendered { stem, frame } => (stem, String::new(), frame, None),
Source::Library { path, cache } => {
match render_from_library(request, &path, cache, cancel)? {
Ok(rendered) => rendered,
@@ -627,7 +634,42 @@ fn export_one(
}
};
Some(place_frame(request, &stem, &date, sequence, &frame, issued))
Some(place_frame(
request,
&stem,
&date,
sequence,
&frame,
source_metadata.as_ref(),
issued,
))
}
/// TRACES: FR-EXP-8
/// What an export is allowed to carry from the file it was decoded from.
///
/// Field by field rather than a conversion trait, and that is the point:
/// `dr_export::SourceMetadata` is an allowlist, so a tag newly parsed by
/// `dr-decode` reaches an exported file only when somebody adds a line here
/// and thereby decides, in writing, that it may leave the machine. The
/// location travels — `dr-export` is where the stripping decision is taken,
/// once, from the settings, and duplicating it here would give two places to
/// disagree.
fn carried_metadata(meta: &dr_decode::Metadata) -> dr_export::SourceMetadata {
dr_export::SourceMetadata {
make: meta.make.clone(),
model: meta.model.clone(),
lens: meta.lens.clone(),
shutter: meta.shutter,
aperture: meta.aperture,
iso: meta.iso,
focal_length: meta.focal_length,
captured_at: meta.captured_at,
captured_offset: meta.captured_offset,
artist: meta.artist.clone(),
copyright: meta.copyright.clone(),
location: meta.location,
}
}
/// Fetch a photograph, apply its stored edit, and render it at full size.
@@ -636,7 +678,17 @@ fn render_from_library(
path: &str,
cache: Option<crate::library::CacheContext>,
cancel: &Cancel,
) -> Option<Result<(String, String, dr_export::Frame), ItemError>> {
) -> Option<
Result<
(
String,
String,
dr_export::Frame,
Option<dr_export::SourceMetadata>,
),
ItemError,
>,
> {
let Some((creds, user_id)) = request.creds.clone() else {
return Some(Err(ItemError::Fetch("no library is open".into())));
};
@@ -721,7 +773,12 @@ fn render_from_library(
.map(|s| s.to_string_lossy().into_owned())
.unwrap_or_else(|| "export".into());
Some(Ok((stem, date, frame)))
// TRACES: FR-EXP-8
// `meta` was read at the top of this function for the orientation and the
// `{date}` token; carrying it on to the encoder is what puts the camera,
// the lens and the rights statement into the exported file. What is
// *dropped* from it is decided in `dr-export` from the settings, not here.
Some(Ok((stem, date, frame, Some(carried_metadata(&meta)))))
}
/// Name, encode and write one rendered frame.
@@ -733,6 +790,11 @@ fn place_frame(
date: &str,
sequence: u32,
frame: &dr_export::Frame,
// TRACES: FR-EXP-8
// What the source file said about itself, or `None` where the caller has
// nothing to say. Handed straight through: every decision about what of it
// reaches the file is taken inside `dr-export`, from the settings.
source: Option<&dr_export::SourceMetadata>,
issued: &mut HashSet<String>,
) -> Result<Placed, ItemError> {
// The size is resolved before the name because `{dimensions}` is one of the
@@ -754,7 +816,7 @@ fn place_frame(
};
let name = resolve_batch_name(&request.settings, &ctx, issued).ok_or(ItemError::NameTaken)?;
let encoded = dr_export::export(frame, &request.settings, name)?;
let encoded = dr_export::export(frame, &request.settings, name, source)?;
place(
&encoded,
ui/dr-ui/src/labels.rs
+30
View File
@@ -30,6 +30,13 @@ pub fn resolve(key: &str) -> String {
"op.vibrance" => "Vibrance".into(),
"op.saturation" => "Saturation".into(),
"op.colour_mixer" => "Colour Mixer".into(),
// "Sharpening" rather than what `derive` would make of the id. The id
// says *capture* sharpening to separate it from the output sharpening
// an export applies (FR-EXP-4), which is a distinction about where in
// the pipeline it sits; in the develop panel there is only one, and
// "Capture Sharpen" would name a distinction the photographer cannot
// see from there.
"op.capture_sharpen" => "Sharpening".into(),
"op.framing" => "Crop & Rotate".into(),
// Parameters
@@ -54,6 +61,19 @@ pub fn resolve(key: &str) -> String {
"param.channel.sat" => "Saturation".into(),
"param.channel.lum" => "Luminance".into(),
// The tone curve's four curves, which its points are *subject* to.
//
// Catalogued rather than derived because the master curve's key would
// otherwise read "Rgb": these are the terms of a four-way choice, and
// one of them miscapitalised is the one the eye goes to. The three
// colours would derive correctly and are written out beside it anyway,
// since a list where one entry is translated and three are guessed is
// the shape a half-finished translation takes.
"channel.rgb" => "RGB".into(),
"channel.red" => "Red".into(),
"channel.green" => "Green".into(),
"channel.blue" => "Blue".into(),
// The hue bands, which a faceted row is *subject* to.
//
// Catalogued even where `derive` would produce the same word, because
@@ -121,6 +141,16 @@ mod tests {
fn catalogued_keys_resolve_to_their_label() {
assert_eq!(resolve("op.white_balance"), "White Balance");
assert_eq!(resolve("param.highlights"), "Highlights");
// Catalogued precisely because `derive` would get it wrong: the id
// carries a distinction ("capture", as against an export's output
// sharpening) that belongs in the pipeline and not on a panel.
assert_eq!(resolve("op.capture_sharpen"), "Sharpening");
// Its parameters are the opposite case — the derived words are the
// right words, so they are left uncatalogued and shared with whatever
// asks for an amount or a radius next.
assert_eq!(resolve("param.amount"), "Amount");
assert_eq!(resolve("param.radius"), "Radius");
assert_eq!(resolve("param.threshold"), "Threshold");
}
#[test]
ui/dr-ui/src/lib.rs
+34
View File
@@ -585,8 +585,24 @@ pub(crate) fn sync_rows(
// curve must be drawn whatever shape it is in.
rows.set_vec(current);
curve_moved = true;
// The curves the widget can switch between, named. They can only
// change with the operation set, which is what this branch means, so
// the walk that derives them is not on the parameter-event path.
let channels: Vec<slint::SharedString> = match session.borrow().as_ref() {
Some(s) => s.curve_channels().into_iter().map(Into::into).collect(),
None => Vec::new(),
};
window.set_curve_channels(slint::ModelRc::new(slint::VecModel::from(channels)));
}
// Which curve is plotted, on every pass. Picking one that happens to be
// shaped like the last — two untouched curves are both the diagonal —
// moves no point, so this cannot ride on the resample below: the chips
// would go on highlighting the curve the user just navigated away from.
let channel = session.borrow().as_ref().map_or(0, |s| s.curve_channel());
window.set_curve_channel(channel);
if !curve_moved {
return;
}
@@ -1861,6 +1877,24 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
redraw(&w);
});
}
{
// Which of the curve's curves the plot is showing. **No redraw**, and
// that is the whole character of this control: it changes no
// parameter, so the photograph is already correct on screen and
// recomputing it would be a frame spent to produce the same pixels.
// For the same reason it records no history step — there is nothing
// to undo — and the sidecar never hears about it.
let weak = window.as_weak();
let session = session.clone();
let rows = rows.clone();
window.on_curve_channel_picked(move |index| {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
s.set_curve_channel(index);
}
sync_rows(&w, &rows, &session);
});
}
// ---- undo and redo (FR-DEV-5) ---------------------------------------
//
ui/dr-ui/src/library_ui.rs
+264 -1
View File
@@ -22,7 +22,7 @@ use dr_types::FormatFilter;
use slint::{ComponentHandle, Model as _};
use crate::library::{self, ScanMessage, ThumbnailMessage};
use crate::{AppWindow, LibraryCell, TimelineBar};
use crate::{AppWindow, KeywordRow, LibraryCell, TimelineBar};
/// Window size before the grid has reported its geometry.
///
@@ -2124,6 +2124,170 @@ fn refresh_rating_counts(window: &AppWindow, catalog: &Catalog) {
window.set_library_local_count(library::local_original_count(catalog).unwrap_or(0) as i32);
}
// --- keywords (FR-CAT-5, FR-CAT-6) ---------------------------------------
//
// `dr_catalog::keywords` owns the data rules — the vocabulary, the many-to-many
// join, what a rename does to the assignments. This part owns the *interaction*:
// which photographs the sheet is acting on, and keeping what it draws honest
// about what actually landed.
/// Redraw the keywording sheet against whatever is selected now.
///
/// Called when the sheet opens and after every assignment, rather than on every
/// selection change: the selection moves on each arrow key and the sheet is shut
/// for almost all of them, so computing coverage over a forty-image selection
/// on each one would be work nobody is looking at.
///
/// Re-read from the catalog rather than patched in place after a write. A word
/// applied to a selection that partly already had it moves from "3 of 12" to
/// "12 of 12", and a model updated by hand would have to reproduce the rule
/// that decides that — which is exactly the rule the catalog has just applied.
fn refresh_keywords(window: &AppWindow, ctl: &Rc<LibraryController>, images: &[dr_types::ImageId]) {
let borrow = ctl.catalog.borrow();
let Some(catalog) = borrow.as_ref() else {
return;
};
let rows = match dr_catalog::keywords::for_images(catalog.connection(), images) {
Ok(rows) => rows,
Err(e) => {
// The grid is entirely usable without the sheet, so this is logged
// rather than surfaced: a keyword read that failed must not put an
// error banner over a library the user is browsing.
log::debug!("reading keywords: {e}");
return;
}
};
let model: Vec<KeywordRow> = rows
.into_iter()
.map(|row| KeywordRow {
id: row.keyword.id.0 as i32,
name: row.keyword.name.into(),
coverage: match row.coverage {
dr_catalog::Coverage::None => 0,
dr_catalog::Coverage::Some => 1,
dr_catalog::Coverage::All => 2,
},
selected_count: row.selected_count as i32,
image_count: row.keyword.image_count as i32,
})
.collect();
window.set_library_keywords(slint::ModelRc::new(slint::VecModel::from(model)));
}
/// Put a keyword on the selection, or take it off.
///
/// # Why this does not write a sidecar
///
/// Every other judgement in this file — a star, a flag — is written to the
/// catalog and then queued to the image's sidecar, because the sidecar is what
/// makes it survive a catalog rebuild (ARCH §6.12). A keyword has no place in
/// the sidecar format yet: `dr_pipeline::sidecar::Version` carries `rating` and
/// `flag` and nothing else that is not an edit-graph parameter.
///
/// So a keyword is, for now, catalog state that reaches the user's other
/// devices through the *catalog* merge ([`dr_catalog::merge`]) rather than
/// through the sidecar. That is a real limitation and not a silent one: a
/// deleted catalog loses keywords where it would keep ratings, until the
/// sidecar gains a `dc:subject` field (FR-CAT-13) and this grows the same
/// queued write the stars have.
fn apply_keyword(window: &AppWindow, ctl: &Rc<LibraryController>, word: &str, assigning: bool) {
let Some(coll) = ctl.coll_ctl.borrow().as_ref().and_then(|c| c.upgrade()) else {
return;
};
let images = coll.selected();
// The word as it will be *stored*, resolved before anything is written.
// The status line below quotes it back, and quoting what was typed would
// report a leading space the catalog is about to drop — leaving the user to
// wonder whether it mattered.
//
// This is also where a blank keyword is caught, which is why it happens
// before the selection check: "you typed nothing" is a better answer than
// "select an image first" to someone who pressed return on an empty field.
let word = match dr_catalog::keywords::normalise(word) {
Ok(word) => word,
Err(e) => {
// `BadName` carries text written to be read by the user rather than
// by a developer, so it is shown as it is.
window.set_library_error(format!("{e}").into());
return;
}
};
// Assigning with nothing selected still means something — it puts the word
// in the vocabulary, ready for the photographs it was typed for — so only
// the removal half needs a selection to act on.
if images.is_empty() && !assigning {
window.set_library_status("Select an image first".into());
return;
}
let outcome = {
let borrow = ctl.catalog.borrow();
let Some(catalog) = borrow.as_ref() else {
return;
};
let conn = catalog.connection();
if assigning {
dr_catalog::keywords::assign(conn, &images, &word)
} else {
dr_catalog::keywords::unassign(conn, &images, &word)
}
};
let n = match outcome {
Ok(n) => n,
Err(e) => {
window.set_library_error(format!("{e}").into());
return;
}
};
window.set_library_error(slint::SharedString::new());
window.set_library_status(keyword_summary(&word, n, images.len(), assigning).into());
refresh_keywords(window, ctl, &images);
// A filtered grid may no longer hold what was just keyworded — taking
// "puffin" off an image while showing only puffins means it belongs
// elsewhere now. The same reasoning as a rating that falls below the star
// filter.
if !ctl.filter.borrow().is_unfiltered() {
load_window(window, ctl);
}
}
/// What the status line says about a keyword that just landed.
///
/// The honest count, not the requested one: "added to 3 of 12" is what
/// happened when nine of them already carried the word, and a message that
/// claimed twelve would be teaching the user that the counts are decorative.
fn keyword_summary(word: &str, changed: usize, selected: usize, assigning: bool) -> String {
if selected == 0 {
return format!("Added “{word}” to the keyword list");
}
let verb = if assigning { "Added" } else { "Removed" };
let preposition = if assigning { "to" } else { "from" };
if changed == 0 {
return if assigning {
format!("Every selected photograph already had “{word}”")
} else {
format!("None of the selected photographs had “{word}”")
};
}
if changed == selected {
let what = if selected == 1 {
"1 photograph".to_string()
} else {
format!("{selected} photographs")
};
return format!("{verb} “{word}” {preposition} {what}");
}
format!("{verb} “{word}” {preposition} {changed} of {selected}")
}
/// Apply a judgement to a set of images: catalog first, then sidecars.
///
/// # Order matters
@@ -4319,6 +4483,40 @@ pub fn wire<F>(
});
}
// --- keywords (FR-CAT-5, FR-CAT-6) ------------------------------------
//
// Three callbacks and no state of their own: the sheet's open/shut is local
// to the `.slint` file, and what a keyword applies to is the grid selection
// the collections controller already owns. A second copy of either here is
// a second thing that can disagree with the first.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let coll_for_keywords = coll_ctl.clone();
window.on_library_keywords_opened(move || {
let Some(w) = weak.upgrade() else { return };
refresh_keywords(&w, &ctl, &coll_for_keywords.selected());
});
}
{
let weak = window.as_weak();
let ctl = ctl.clone();
window.on_library_assign_keyword(move |word| {
let Some(w) = weak.upgrade() else { return };
apply_keyword(&w, &ctl, word.as_str(), true);
});
}
{
let weak = window.as_weak();
let ctl = ctl.clone();
window.on_library_unassign_keyword(move |word| {
let Some(w) = weak.upgrade() else { return };
apply_keyword(&w, &ctl, word.as_str(), false);
});
}
// --- the filter bar ---------------------------------------------------
//
// Each of these narrows what the grid *queries*, so all three reset the
@@ -5184,6 +5382,71 @@ mod tests {
assert_eq!(paths, vec!["c.CR2", "a.CR2"]);
}
// --- what the status line says about a keyword (FR-CAT-5) -------------
//
// Split out from the callback for the same reason `decide_drop` is: the
// sheet cannot be driven from a test, and this is the part that can
// actually mislead someone.
/// TRACES: FR-CAT-5
#[test]
fn a_partly_applied_keyword_reports_the_honest_count() {
// Nine of the twelve already had it. Claiming twelve is how a user
// learns that the counts are decorative.
assert_eq!(
keyword_summary("puffin", 3, 12, true),
"Added “puffin” to 3 of 12"
);
}
/// TRACES: FR-CAT-5
#[test]
fn a_keyword_that_changed_nothing_says_so_rather_than_claiming_success() {
assert_eq!(
keyword_summary("puffin", 0, 12, true),
"Every selected photograph already had “puffin”"
);
assert_eq!(
keyword_summary("puffin", 0, 12, false),
"None of the selected photographs had “puffin”"
);
}
/// TRACES: FR-CAT-5
#[test]
fn one_photograph_is_singular() {
// "Added to 1 photographs" is the kind of small wrongness that makes
// the rest of the interface look unfinished.
assert_eq!(
keyword_summary("puffin", 1, 1, true),
"Added “puffin” to 1 photograph"
);
assert_eq!(
keyword_summary("puffin", 2, 2, true),
"Added “puffin” to 2 photographs"
);
}
/// TRACES: FR-CAT-5
#[test]
fn removing_a_keyword_reads_as_removal() {
assert_eq!(
keyword_summary("blurry", 4, 4, false),
"Removed “blurry” from 4 photographs"
);
}
/// TRACES: FR-CAT-5
#[test]
fn typing_a_word_with_nothing_selected_says_what_it_did_do() {
// It builds the vocabulary, which is a legitimate thing to do ahead of
// a shoot — so it must not report itself as having keyworded nothing.
assert_eq!(
keyword_summary("puffin", 0, 0, true),
"Added “puffin” to the keyword list"
);
}
/// TRACES: FR-EXP-7
#[test]
fn a_selection_outside_the_loaded_window_still_resolves() {