Merge branch 'worktree-agent-a89309a856c8f4947' into integration

This commit is contained in:
2026-08-22 19:00:56 +02:00
11 changed files with 1807 additions and 233 deletions
+139
View File
@@ -0,0 +1,139 @@
//! TRACES: FR-DEV-3
//! The tone curve's four curves, on a device.
//!
//! `dr-pipeline` asserts that the right WGSL is generated and `dr-gpu`'s other
//! tests assert that a shader runs; neither notices a fragment that says
//! exactly what it should and does not compile, or one that compiles and puts
//! the red curve's uniforms into the blue slot. So this renders flat grey
//! through each curve and looks at what came out.
//!
//! Flat grey because it makes every assertion a comparison between the three
//! components of one pixel: a curve that is meant to be chromatic must move
//! them apart, and one that is meant to be tonal must not.
use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext};
use dr_pipeline::ops::curve::{self, Axis, Channel};
use dr_pipeline::EditGraph;
const SIZE: u32 = 8;
fn ctx() -> Option<GpuContext> {
pollster::block_on(GpuContext::new_headless()).ok()
}
/// The centre pixel's red, green and blue, after `graph` has run over flat
/// mid-grey.
fn rendered(ctx: &GpuContext, graph: &EditGraph) -> (u8, u8, u8) {
let data: Vec<u8> = (0..SIZE * SIZE).flat_map(|_| [128, 128, 128, 255]).collect();
let source = DemosaicedImage::from_rgba8(ctx, &data, SIZE, SIZE).expect("upload");
// Composed the way the display path composes it. A curve that generates
// invalid WGSL fails at `render` below, which is the point of running this
// on a device at all.
let shader = graph.compose();
let mut adjust = AdjustPass::new(ctx);
adjust.render(&source, &shader, SIZE, SIZE).expect("render");
let pixels = adjust.export_pixels().expect("readback").0;
let at = ((SIZE / 2 * SIZE + SIZE / 2) * 4) as usize;
(pixels[at], pixels[at + 1], pixels[at + 2])
}
/// A curve with its mid-point lifted — the simplest edit that is unmistakably
/// an edit.
fn lifted(channel: Channel) -> EditGraph {
let mut graph = EditGraph::default_chain();
graph.set_param(
curve::ID,
curve::coordinate(channel, 2, Axis::Y),
0.75,
);
graph
}
#[test]
fn the_master_curve_lifts_every_component_together() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let (r0, g0, b0) = rendered(&ctx, &EditGraph::default_chain());
let (r, g, b) = rendered(&ctx, &lifted(Channel::Master));
assert!(r > r0, "the master curve did not lift the image: {r} vs {r0}");
// Grey in, grey out: the master curve is applied as a ratio over
// luminance, so it changes tone and not hue. A tolerance of one code
// value, because the components travel through the ratio separately and
// the result is quantised to eight bits.
assert!(
r.abs_diff(g) <= 1 && g.abs_diff(b) <= 1,
"the master curve tinted a neutral pixel: {r},{g},{b}"
);
assert_eq!((g0, b0), (r0, r0), "the unedited image is neutral");
}
#[test]
fn a_channel_curve_lifts_only_its_own_component() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let (r0, g0, b0) = rendered(&ctx, &EditGraph::default_chain());
for (channel, name) in [
(Channel::Red, "red"),
(Channel::Green, "green"),
(Channel::Blue, "blue"),
] {
let (r, g, b) = rendered(&ctx, &lifted(channel));
// The component the curve names moves; the other two stay exactly
// where they were. This is what catches a fragment whose uniforms are
// wired to the wrong curve — it would still lift *something*.
let (moved, still) = match channel {
Channel::Red => (r > r0, g == g0 && b == b0),
Channel::Green => (g > g0, r == r0 && b == b0),
_ => (b > b0, r == r0 && g == g0),
};
assert!(moved, "the {name} curve changed nothing: {r},{g},{b}");
assert!(
still,
"the {name} curve moved a component that was not its own: \
{r},{g},{b} from {r0},{g0},{b0}"
);
}
}
#[test]
fn the_master_and_the_channels_compose_in_one_pass() {
// All four curves at once: the case where the generated fragment is
// longest, every helper is present, and forty uniforms are in the block.
// Mostly a compile check, which is why the assertion is only that the
// result is a colour and not the one we started with.
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::P1_Y, 0.15);
graph.set_param(curve::ID, curve::P3_Y, 0.85);
for (channel, y) in [
(Channel::Red, 0.55),
(Channel::Green, 0.5),
(Channel::Blue, 0.62),
] {
graph.set_param(curve::ID, curve::coordinate(channel, 2, Axis::Y), y);
}
let (r0, _, _) = rendered(&ctx, &EditGraph::default_chain());
let (r, g, b) = rendered(&ctx, &graph);
assert!(
(r, g, b) != (r0, r0, r0),
"four active curves left the image untouched"
);
// Red and blue were pushed apart from green, which is the chromatic half
// doing its work on top of the tonal one.
assert!(b > g, "blue was lifted above green: {r},{g},{b}");
}
+4 -3
View File
@@ -209,9 +209,10 @@ They still belong in this directory, because the pipeline's **order** is the
one thing a reader comes here to learn, and an order written half in YAML and
half in Rust would be worse than either alone.
Currently hand-written: `tone_curve` (a curve widget over five interpolated
points), `colour_mixer` (thirty-six faceted parameters from twelve computed hue
bands). `vignetting` is hand-written too but is not in the develop chain — it
Currently hand-written: `tone_curve` (one widget over four curves of five
interpolated points — master, red, green, blue — each reaching the shader only
when it has been moved), `colour_mixer` (thirty-six faceted parameters from
twelve computed hue bands). `vignetting` is hand-written too but is not in the develop chain — it
carries lens-profile coefficients that are not parameters. `distortion` and
`aberration` are `Warp`s rather than operations: they rewrite coordinates
before sampling rather than transforming a colour after it.
+18 -5
View File
@@ -16,13 +16,26 @@ attributes: [tone, colour]
rust: ToneCurve
why_rust: |
Five control points presented as one curve widget, with an interpolator and
a monotonicity guarantee behind it. Its neutral is a *relationship* between
parameters rather than a set of values — the identity diagonal — which is
not something the declarative `active:` rule can express, and its
`presentation()` spans parameters rather than describing one.
Four curves — master, red, green, blue — of five control points each,
presented as one widget, with an interpolator and a monotonicity guarantee
behind them. Its neutral is a *relationship* between parameters rather than
a set of values — the identity diagonal, on every channel — which is not
something the declarative `active:` rule can express; its `presentation()`
spans forty parameters rather than describing one; and its fragment is
*assembled* rather than written, because each curve reaches the shader only
when it has been moved off the diagonal. A declared node's `wgsl:` is one
fixed block of text, which is the right shape for nearly everything here and
the wrong one for a node whose cost has to follow what the photographer
actually touched.
placement: |
After the fixed-weight region controls, so the curve is the final word on
tone: a photographer reaches for it to fix what those controls could not
place exactly.
Within the node, the master curve runs before the per-channel ones. Both
orders are visibly different images and the reasons for this one are written
out in `src/ops/curve.rs`: the master is tonal and hue-preserving, the
per-channel curves are the chromatic grade over the tones it produced, and a
point placed on a channel curve should act on the tone the photographer can
see rather than on the one the master is about to move.
File diff suppressed because it is too large Load Diff
+85 -1
View File
@@ -1088,7 +1088,7 @@ impl std::error::Error for ParseError {}
mod tests {
use super::*;
use crate::framing;
use crate::ops::{exposure, saturation, white_balance};
use crate::ops::{curve, exposure, saturation, white_balance};
fn edited() -> EditGraph {
let mut g = EditGraph::default_chain();
@@ -1273,6 +1273,90 @@ mod tests {
assert_eq!(once, twice);
}
/// TRACES: FR-DEV-3 | FR-CAT-8
/// A file written before the tone curve had per-channel curves.
///
/// Spelled out as literal text rather than produced by `to_text`, because
/// the claim is about *those bytes*: a sidecar generated by this build
/// would agree with this build by construction, and would go on agreeing
/// with it through a rename that broke every file on disk.
#[test]
fn a_sidecar_from_before_the_channel_curves_still_names_the_master() {
let text = "drsc 1\n\n[version u1]\nname = Default\nrevision = 4\nmodified = 9\n\
tone_curve.p1_y = 0.15\ntone_curve.p3_y = 0.85\n";
let parsed = Sidecar::parse(text).expect("valid");
let mut g = EditGraph::default_chain();
parsed.default_version().expect("a version").apply(&mut g);
// The S-curve the file describes, on the master curve and nowhere
// else.
assert_eq!(g.param(curve::ID, curve::P1_Y), Some(0.15));
assert_eq!(g.param(curve::ID, curve::P3_Y), Some(0.85));
for channel in [curve::Channel::Red, curve::Channel::Green, curve::Channel::Blue] {
for point in 0..curve::POINTS {
for axis in [curve::Axis::X, curve::Axis::Y] {
let id = curve::coordinate(channel, point, axis);
let expected = g
.capabilities()
.iter()
.find(|c| c.id == curve::ID)
.and_then(|c| c.params.iter().find(|p| p.id == id))
.map(|p| p.default);
assert_eq!(
g.param(curve::ID, id),
expected,
"{id} moved, and no line in the file mentions it"
);
}
}
}
// And writing it back produces the same two lines: the curves the file
// never mentioned are still at their defaults, so they are still
// absent (`only_non_default_values_are_written`).
let written = Sidecar::parse(&Sidecar::parse(text).expect("valid").to_text())
.expect("valid")
.to_text();
assert!(written.contains("tone_curve.p1_y = 0.15"), "{written}");
assert!(written.contains("tone_curve.p3_y = 0.85"), "{written}");
assert!(
!written.contains("tone_curve.r_"),
"an untouched channel curve was written out:\n{written}"
);
}
/// TRACES: FR-DEV-3
/// The other direction: the new curves persist like any other parameter.
#[test]
fn a_per_channel_curve_survives_the_round_trip() {
let mut g = EditGraph::default_chain();
// A faded shadow: blue lifted at the black point, red pulled down.
let blue = curve::coordinate(curve::Channel::Blue, 0, curve::Axis::Y);
let red = curve::coordinate(curve::Channel::Red, 4, curve::Axis::Y);
g.set_param(curve::ID, blue, 0.08);
g.set_param(curve::ID, red, 0.92);
g.set_param(curve::ID, curve::P2_Y, 0.55);
let mut sidecar = Sidecar::new();
sidecar.put(version_of(&g));
let text = sidecar.to_text();
// Keyed by the channel-prefixed id, which is what makes the master's
// unprefixed ones safe to leave alone.
assert!(text.contains("tone_curve.b_p0_y = 0.08"), "{text}");
let parsed = Sidecar::parse(&text).expect("valid");
let mut restored = EditGraph::default_chain();
parsed
.default_version()
.expect("a version")
.apply(&mut restored);
assert_eq!(restored.param(curve::ID, blue), Some(0.08));
assert_eq!(restored.param(curve::ID, red), Some(0.92));
assert_eq!(restored.param(curve::ID, curve::P2_Y), Some(0.55));
assert!(!restored.is_neutral());
}
#[test]
fn an_unknown_operation_survives_a_round_trip() {
// The data-loss case that matters: a device running an older build
+202
View File
@@ -0,0 +1,202 @@
//! TRACES: FR-DEV-3
//! The tone curve's four curves, seen from outside the crate.
//!
//! The unit tests beside the operation assert what one `ToneCurve` does. These
//! assert the two properties that only show up once it is a node in a chain
//! with a sidecar under it: that a file written before the per-channel curves
//! existed still describes the edit it described, and that the three new
//! curves cost a photograph that does not use them precisely nothing.
use dr_pipeline::ops::curve::{self, Axis, Channel};
use dr_pipeline::{EditGraph, Sidecar};
/// A version block carrying `params`, in the on-disk spelling.
fn sidecar_with(params: &[(&str, f32)]) -> String {
let mut text = String::from("drsc 1\n\n[version u1]\nname = Default\nrevision = 2\nmodified = 0\n");
for (key, value) in params {
text.push_str(&format!("{key} = {value}\n"));
}
text
}
fn apply(text: &str) -> EditGraph {
let sidecar = Sidecar::parse(text).expect("a valid sidecar");
let mut graph = EditGraph::default_chain();
sidecar
.default_version()
.expect("a default version")
.apply(&mut graph);
graph
}
/// TRACES: FR-CAT-8
/// **The compatibility guarantee, end to end.**
///
/// An edit made before this build existed has to produce the same image now.
/// Asserted on the generated shader rather than on the parameter values,
/// because that is what the photograph is actually made of: same source, same
/// uniforms, same picture.
#[test]
fn an_edit_written_before_the_channel_curves_renders_as_it_did() {
let from_file = apply(&sidecar_with(&[
("tone_curve.p1_y", 0.15),
("tone_curve.p3_y", 0.85),
]));
let mut by_hand = EditGraph::default_chain();
by_hand.set_param(curve::ID, curve::P1_Y, 0.15);
by_hand.set_param(curve::ID, curve::P3_Y, 0.85);
let restored = from_file.compose();
let expected = by_hand.compose();
assert_eq!(restored.source, expected.source);
assert_eq!(restored.uniforms, expected.uniforms);
assert_eq!(restored.structure_hash, expected.structure_hash);
}
/// **Neutral means absent, and stays absent with four times as much to be
/// neutral about.**
///
/// The curve carries forty parameters now. An image edited with an S-curve and
/// nothing else must generate the shader it generated when it carried ten:
/// three untouched curves are not three identity evaluations, they are nothing
/// at all.
#[test]
fn three_untouched_curves_cost_nothing() {
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::P2_Y, 0.62);
let shader = graph.compose();
assert!(
shader.source.contains("---- tone_curve ----"),
"the master curve must reach the shader"
);
assert!(
!shader.source.contains("channel_curve"),
"an untouched channel curve reached the shader:\n{}",
shader.source
);
for prefix in ["r_", "g_", "b_"] {
assert!(
!shader.source.contains(&format!("tone_curve_{prefix}")),
"an untouched channel curve declared uniforms:\n{}",
shader.source
);
}
}
/// And with none of them touched, the operation is not there at all — the
/// property `a_fresh_graph_is_neutral` asserts for the chain, restated for the
/// node that just quadrupled in size.
#[test]
fn a_curve_at_its_defaults_is_absent_from_the_shader() {
let graph = EditGraph::default_chain();
assert!(graph.is_neutral());
assert!(!graph.compose().source.contains("tone_curve"));
// Including when every one of the forty parameters has been explicitly
// written to its own default, which is what a sidecar round trip through
// a build with a different idea of "default" would produce.
let mut written = EditGraph::default_chain();
for cap in written.capabilities() {
if cap.id != curve::ID {
continue;
}
for p in &cap.params {
written.set_param(curve::ID, p.id, p.default);
}
}
assert!(written.is_neutral());
}
/// A grade with no tonal work is a real edit, and it must not drag the
/// luminance path in behind it.
#[test]
fn a_channel_curve_reaches_the_shader_on_its_own() {
let mut graph = EditGraph::default_chain();
graph.set_param(
curve::ID,
curve::coordinate(Channel::Blue, 0, Axis::Y),
0.08,
);
let shader = graph.compose();
assert!(shader.source.contains("c.b = channel_curve(c.b,"));
assert!(
!shader.source.contains("apply_tone_gain"),
"the identity master curve reached the shader:\n{}",
shader.source
);
// Blue's ten points, and nothing else: the other two curves are at the
// identity and contribute no slot to the uniform block.
for prefix in ["r_", "g_"] {
assert!(
!shader.source.contains(&format!("tone_curve_{prefix}")),
"an untouched channel curve declared uniforms:\n{}",
shader.source
);
}
for point in 0..curve::POINTS {
assert!(
shader.source.contains(&format!("tone_curve_b_x{point}")),
"blue's point {point} is missing from the uniform block"
);
}
}
/// Each curve is its own shader, so the pipeline cache cannot hand the red
/// curve's compiled program to an edit that moved the green one.
#[test]
fn every_curve_generates_a_distinct_shader() {
let mut hashes: Vec<u64> = Vec::new();
for channel in Channel::ALL {
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::coordinate(channel, 2, Axis::Y), 0.62);
hashes.push(graph.compose().structure_hash);
}
let before = hashes.len();
hashes.sort_unstable();
hashes.dedup();
assert_eq!(before, hashes.len(), "two curves share a compiled shader");
}
/// The forty parameters all persist, and the file names each one once.
#[test]
fn every_point_of_every_curve_round_trips_through_a_sidecar() {
let mut graph = EditGraph::default_chain();
// A different y per coordinate, so a point wired to the wrong channel
// cannot pass by holding the value it was supposed to hold anyway.
// Thirty-secondths because they survive both the decimal the file is
// written in and the binary32 it is read back into exactly — this test is
// about which parameter a value lands in, and a rounding difference here
// would fail it for an unrelated reason.
let mut step = 1;
for channel in Channel::ALL {
for point in 0..curve::POINTS {
graph.set_param(
curve::ID,
curve::coordinate(channel, point, Axis::Y),
step as f32 / 32.0,
);
step += 1;
}
}
let mut sidecar = Sidecar::new();
sidecar.put(dr_pipeline::sidecar::Version::from_graph(
"u1", "Default", &graph,
));
let restored = apply(&sidecar.to_text());
for channel in Channel::ALL {
for point in 0..curve::POINTS {
let id = curve::coordinate(channel, point, Axis::Y);
assert_eq!(
restored.param(curve::ID, id),
graph.param(curve::ID, id),
"{id} did not survive the sidecar"
);
}
}
}
+352 -46
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 {
@@ -3419,9 +3583,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 +3593,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 +3637,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°
+13
View File
@@ -54,6 +54,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
+34
View File
@@ -583,8 +583,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;
}
@@ -1804,6 +1820,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) ---------------------------------------
//
+59 -13
View File
@@ -365,10 +365,17 @@ component ParamControl inherits Rectangle {
in property <ParamRow> data;
/// Curve rows only; ignored by every other kind.
in property <[float]> curve-samples;
/// The curves this widget can plot, named. Empty, or one entry, where
/// there is nothing to choose between — see `curve-channel-picked`.
in property <[string]> curve-channels;
/// Which of `curve-channels` is on the grid.
in property <int> curve-channel;
callback param-changed(int, int, float);
callback param-reset(int, int);
callback curve-reset(int);
/// Plot a different one of the operation's curves.
callback curve-channel-picked(int);
callback drag-changed(bool);
height: layout.preferred-height;
@@ -420,20 +427,49 @@ component ParamControl inherits Rectangle {
}
}
if root.data.kind == "curve": CurveEditor {
points: root.data.points;
samples: root.curve-samples;
drag-changed(on) => { root.drag-changed(on); }
// A point carries two parameters, so the parameter index is the
// row's base plus the point's offset. This component still knows
// nothing about which operation it belongs to.
point-moved(point, x, y) => {
root.param-changed(
root.data.op-index, root.data.param-index + point * 2, x);
root.param-changed(
root.data.op-index, root.data.param-index + point * 2 + 1, y);
// A curve, and — where the operation offers more than one — the choice
// of which curve is on the grid.
//
// One plot rather than four stacked ones: the curves are read against
// the diagonal and against each other, which needs the grid large, and
// four grids at a quarter of the width would each be too small to
// place a point in. So the selector switches the subject of a single
// plot, and the names in it come from the core — this file does not
// know that a colour channel is what is being chosen between, only
// that the widget said it spans several named things.
if root.data.kind == "curve": VerticalLayout {
spacing: Theme.gap-sm;
if root.curve-channels.length > 1: Segmented {
// The operation's own name, which nothing else in this row
// draws: a curve row heads no group, so without this the plot
// would sit in the panel unlabelled.
label: root.data.op-label;
options: root.curve-channels;
selected: root.curve-channel;
picked(i) => { root.curve-channel-picked(i); }
}
CurveEditor {
points: root.data.points;
samples: root.curve-samples;
drag-changed(on) => { root.drag-changed(on); }
// A point carries two parameters, so the parameter index is
// the row's base plus the point's offset. The base is the
// first point of the curve *on show*, so switching curve
// re-points the drag and this component still knows nothing
// about which operation — or which curve — it is drawing.
point-moved(point, x, y) => {
root.param-changed(
root.data.op-index, root.data.param-index + point * 2, x);
root.param-changed(
root.data.op-index, root.data.param-index + point * 2 + 1, y);
}
// Resetting a curve resets the operation, which is all four of
// them — a photographer who double-clicks to start again means
// the control, not the curve that happens to be on show.
reset => { root.curve-reset(root.data.op-index); }
}
reset => { root.curve-reset(root.data.op-index); }
}
}
}
@@ -825,9 +861,16 @@ export component AdjustPanel inherits Rectangle {
/// The tone curve's sampled shape, evaluated in Rust by the same spline
/// the shader runs so the drawn line cannot disagree with the applied one.
in property <[float]> curve-samples;
/// The curves the tone curve widget can plot, named by the core. Fewer
/// than two of them means there is nothing to choose and no selector.
in property <[string]> curve-channels;
/// Which of them `curve-samples` and the row's points describe.
in property <int> curve-channel;
callback param-changed(int, int, float);
callback param-reset(int, int);
callback curve-reset(int);
/// Plot a different one of the curve's curves.
callback curve-channel-picked(int);
/// Return every parameter of one operation to its default — the reset on
/// a section's own header, beside the panel-wide one.
callback op-reset(int);
@@ -986,12 +1029,15 @@ export component AdjustPanel inherits Rectangle {
ParamControl {
data: row;
curve-samples: root.curve-samples;
curve-channels: root.curve-channels;
curve-channel: root.curve-channel;
drag-changed(on) => { root.slider-dragging = on; }
param-changed(op, param, v) => {
root.param-changed(op, param, v);
}
param-reset(op, param) => { root.param-reset(op, param); }
curve-reset(op) => { root.curve-reset(op); }
curve-channel-picked(i) => { root.curve-channel-picked(i); }
}
}
}
+10
View File
@@ -716,9 +716,14 @@ export component AppWindow inherits Window {
// The tone curve's sampled shape, evaluated by the core so the drawn
// line and the applied one cannot disagree.
in property <[float]> curve-samples;
// The curves that widget can plot, named by the core, and which of them
// `curve-samples` describes. Fewer than two means nothing to choose.
in property <[string]> curve-channels;
in property <int> curve-channel;
callback param-changed(int, int, float);
callback param-reset(int, int);
callback curve-reset(int);
callback curve-channel-picked(int);
callback reset-all();
// --- copying settings between photographs (FR-DEV-6) ---
@@ -2177,6 +2182,11 @@ in property <bool> panel-visible: true;
enabled: root.adjust-enabled;
scope: root.adjust-scope;
curve-samples: root.curve-samples;
curve-channels: root.curve-channels;
curve-channel: root.curve-channel;
curve-channel-picked(i) => {
root.curve-channel-picked(i);
}
param-changed(op, param, value) => {
root.param-changed(op, param, value);
}