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

Give the controls a vocabulary, and let a node ask for one

Browse Source 
widgets.slint set the rule — screens consume components, and a bare `Theme.*`
at a call site means a component is missing — and it set it for chrome only.
The controls never got the same treatment, so they were written wherever they
were first needed and copied from there.

**The slider was private to the develop panel.** `SliderTrack`, with the
fifty-line preamble explaining how it wrests a drag away from a Flickable,
lived inside adjust.slint and no other screen could reach it. It shows: export
quality is a 1-to-100 value, and the settings page offered a free-text box for
it, with the range written in a hint and enforced nowhere. `to-float()` answers
0 for anything it cannot parse, so a typo saved a quality of 0 and the page
displayed the 0 back as though it had been asked for.

The tick-box was written twice, in launch.slint and settings.slint, from the
same 18px box and the same handler; the second carried a comment deferring the
lift until a third caller appeared. The label-and-hint header was written three
times inside settings.slint alone.

controls.slint is the input layer beside widgets.slint's chrome layer, and the
constraint that makes it reusable is that **nothing in it knows about
`ParamRow`** — that struct is the develop panel's flattening of the capability
model, and a control that imported it could only ever be used by the develop
panel. The primitives take plain numbers; the ParamRow-shaped wrappers stay in
the panel that owns the model. 658 lines came out of the three screens.

`SliderRow` is the slider-plus-number-box ARCH §4.3 names as the pointer
presentation of a bounded scalar, and quality is its first adopter. It commits
on gesture end rather than on every movement, because the settings page saves
to disk on change and a two-second drag is a couple of hundred writes where a
text field committed once. The develop panel keeps the live stream — that is
what its pipeline is for — so `SliderTrack` now reports both.

**The other half is the descriptor.** FR-DEV-3a and ARCH §4.3a already specify
more than was built: an ordered preference list of widgets rather than one, the
demands a widget makes, and kinds beyond scalar and bool.

- `Presentation.widgets` is now a list, walked by `choose`, falling back to
  plain sliders. Falling off the end is not an error, and there is a test
  asserting an operation asking only for an unimplemented widget still yields
  one control per parameter.
- `WidgetDemand` carries what a widget inherently needs — two-dimensional
  dragging, precise pointing — and no pixels, breakpoints or platform names.
- `WidgetKind` grows to the specified set. There is deliberately no `Colour`
  *kind*: a colour is three numbers, and a value type that is not an `f32`
  would reach through the graph, the uniform block and the sidecar format to
  buy what `ColourWheel` over three scalars already describes. Every widget
  here is a hint over ordinary scalars, which is what keeps the fallback
  honest.
- `ParamKind::Enum` is the one new shape, and it fits because a variant index
  is exact in binary32. `kind: enum` with a `variants:` list works in
  `ops/*.yaml`, so a node declaring one gets a segmented control with no UI
  file edited — which is the promise ops/mod.rs already makes.

The panel's dispatch was duplicated: a lone parameter and a grouped one each
wrote out their own list of kinds, so `enum` would have had to be added twice
and a kind added to one would appear or vanish depending on how many parameters
its operation happened to declare. `ParamControl` is now the only such chain.

`rows_from` is free-standing rather than a method, which is what lets the
FR-DEV-3c acceptance test requirements.md asks for actually be written: an
operation the frontend has never heard of, appearing in a generated panel, with
no GPU in sight.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-16 23:21:35 +02:00
co-authored by Claude Opus 5
parent e7130ff891
commit 0a331c717e
10 changed files with 1662 additions and 910 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/develop.rs
+366 -168
View File
@@ -84,183 +84,250 @@ impl DevelopSession {
/// Built entirely from the capability list. The `kind` string chooses the
/// widget; nothing switches on a parameter's identity.
pub fn rows(&self) -> Vec<ParamRow> {
let mut rows = Vec::new();
for (op_index, op) in self.graph.capabilities().iter().enumerate() {
// Framing has a panel of its own.
rows_from(&self.graph.capabilities())
}
}
/// Whether this frontend has an implementation of `widget`.
///
/// The single source of truth for what the panel can draw. A function rather
/// than a constant list so that a kind whose availability depends on something
/// — a canvas-hosted widget needs a canvas — has one obvious place to say so.
pub(crate) fn draws(widget: WidgetKind) -> bool {
match widget {
WidgetKind::ToneCurve => true,
// Not implemented here. The `is_on_canvas` kinds additionally need a
// host the panel does not have: the canvas draws those, and the
// panel's job is the affordance that turns them on.
WidgetKind::ColourWheel
| WidgetKind::CropOverlay
| WidgetKind::GradientHandle
| WidgetKind::BrushMask
| WidgetKind::WhitePoint => false,
}
}
/// The panel model for a set of capabilities.
///
/// Free-standing rather than a method, and that is the point: it needs no GPU,
/// no decoded image and no session, so the whole descriptor-to-panel path can
/// be exercised against a hand-built capability list. That is what the
/// 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.
pub(crate) fn rows_from(caps: &[OpCapability]) -> Vec<ParamRow> {
let mut rows = Vec::new();
for (op_index, op) in caps.iter().enumerate() {
// Framing has a panel of its own.
//
// The one place this side names a stage, and the exception proves
// the rule: every *other* operation is rendered from its
// descriptor alone. Framing is skipped because its parameters are
// not sliders in any useful sense — four crop edges are dragged on
// the photograph and a quarter turn is a button — so it is
// presented by `GeometryPanel` instead of generated here. Emitting
// both would show the same eight values twice, in one good control
// surface and one bad one.
if op.id == dr_pipeline::framing::ID {
continue;
}
// Where this operation's rows begin. The panel groups by walking
// back to it, so it has to be taken before any row is pushed.
let group_head = rows.len();
// An operation may ask for one widget spanning several
// parameters. Honouring it is optional — dropping this block
// renders the same parameters as ordinary sliders, and the edit
// still works — which is exactly why the hint is a hint.
if let Some(presentation) = &op.presentation {
// **The widget registry, and the only one.**
//
// The one place this side names a stage, and the exception proves
// the rule: every *other* operation is rendered from its
// descriptor alone. Framing is skipped because its parameters are
// not sliders in any useful sense — four crop edges are dragged on
// the photograph and a quarter turn is a button — so it is
// presented by `GeometryPanel` instead of generated here. Emitting
// both would show the same eight values twice, in one good control
// surface and one bad one.
if op.id == dr_pipeline::framing::ID {
// `choose` walks the operation's preference list and hands
// back the first entry this frontend implements (ARCH §4.3a).
// Everything not listed here is unimplemented by definition,
// so an operation asking for a colour wheel gets sliders
// rather than an error — which is the designed behaviour, not
// a gap: every parameter is an individually addressable
// scalar, so the edit still works.
//
// The `match` inside is exhaustive on purpose. Adding a
// `WidgetKind` to the core stops this compiling until someone
// has decided, here, whether this frontend draws it.
let row = presentation.choose(draws).and_then(|widget| match widget {
WidgetKind::ToneCurve => curve_row(op_index, group_head, op, presentation),
// Named but not drawn here. Listed rather than caught
// by a wildcard so that the next kind added to the
// core surfaces as a compile error in this file.
WidgetKind::ColourWheel
| WidgetKind::CropOverlay
| WidgetKind::GradientHandle
| WidgetKind::BrushMask
| WidgetKind::WhitePoint => None,
});
if let Some(row) = row {
rows.push(row);
continue;
}
// Where this operation's rows begin. The panel groups by walking
// back to it, so it has to be taken before any row is pushed.
let group_head = rows.len();
// An operation may ask for one widget spanning several
// parameters. Honouring it is optional — dropping this block
// renders the same parameters as ordinary sliders, and the edit
// still works — which is exactly why the hint is a hint.
if let Some(presentation) = &op.presentation {
// A `match` rather than an `if let`: when a second widget
// kind is added, this stops compiling until it is handled,
// rather than silently falling through to sliders.
let row = match presentation.widget {
WidgetKind::Curve => self.curve_row(op_index, group_head, op, presentation),
};
if let Some(row) = row {
rows.push(row);
continue;
}
}
// Whether anything in this operation has been touched, aggregated
// before the rows are built so every row of the group can carry
// the same answer — the panel's heading is one of them and cannot
// see the others.
//
// Derived here rather than asked of the core: a group is a
// composition this side invented, so whether one is modified is
// this side's question to answer (ARCH §4.3a).
let group_modified = op.params.iter().any(|p| p.value != p.default);
let group_len = op.params.len() as i32;
// The aspect the previous row belonged to, so a run can be told
// from its continuation. Reset per operation: two operations that
// happened to facet on the same key are still two groups.
let mut previous_aspect: Option<&str> = None;
for param_index in presentation_order(&op.params) {
let p = &op.params[param_index];
let (kind, min, max, precision, unit) = match &p.kind {
ParamKind::Scalar {
min,
max,
unit,
precision,
..
} => (
"scalar",
*min,
*max,
i32::from(*precision),
unit_suffix(*unit),
),
ParamKind::Bool => ("bool", 0.0, 1.0, 0, ""),
};
// A faceted parameter is named by its *subject* — the band —
// because its aspect is already written above the run it sits
// in. Unfaceted parameters keep their own label, which is
// every operation but the mixer.
let param_label = match &p.facet {
Some(f) => labels::resolve(f.subject.0),
None => labels::resolve(p.label.0),
};
let aspect = p.facet.as_ref().map(|f| f.aspect.0);
let starts_facet = aspect.is_some() && aspect != previous_aspect;
previous_aspect = aspect;
rows.push(ParamRow {
op_index: op_index as i32,
param_index: param_index as i32,
op_label: labels::resolve(op.label.0).into(),
param_label: param_label.into(),
facet_label: aspect.map(labels::resolve).unwrap_or_default().into(),
starts_facet,
// -1 rather than an `Option`, which a Slint struct cannot
// carry: 0° is red, so no value in range can stand for
// "no swatch".
swatch_hue: p.facet.as_ref().and_then(|f| f.subject_hue).unwrap_or(-1.0),
group_head: group_head as i32,
group_len,
group_modified,
kind: kind.into(),
value: p.value,
default_value: p.default,
minimum: min,
maximum: max,
precision,
unit: unit.into(),
// Only curve rows carry points.
points: slint::ModelRc::new(slint::VecModel::from(Vec::<f32>::new())),
});
}
}
rows
// Whether anything in this operation has been touched, aggregated
// before the rows are built so every row of the group can carry
// the same answer — the panel's heading is one of them and cannot
// see the others.
//
// Derived here rather than asked of the core: a group is a
// composition this side invented, so whether one is modified is
// this side's question to answer (ARCH §4.3a).
let group_modified = op.params.iter().any(|p| p.value != p.default);
let group_len = op.params.len() as i32;
// The aspect the previous row belonged to, so a run can be told
// from its continuation. Reset per operation: two operations that
// happened to facet on the same key are still two groups.
let mut previous_aspect: Option<&str> = None;
for param_index in presentation_order(&op.params) {
let p = &op.params[param_index];
// Empty for every kind but `Enum`, which is what the panel
// keys on to build a segmented control rather than a slider.
let mut choices: Vec<slint::SharedString> = Vec::new();
let (kind, min, max, precision, unit) = match &p.kind {
ParamKind::Scalar {
min,
max,
unit,
precision,
..
} => (
"scalar",
*min,
*max,
i32::from(*precision),
unit_suffix(*unit),
),
ParamKind::Bool => ("bool", 0.0, 1.0, 0, ""),
// The value is a variant index, so the range is the list's
// own bounds and the precision is whole numbers. Labels are
// resolved here, against this crate's catalogue, because
// the core deals in localisation keys only (NFR-A11Y-1).
ParamKind::Enum { variants } => {
choices = variants
.iter()
.map(|v| labels::resolve(v.0).into())
.collect();
("enum", 0.0, variants.len().saturating_sub(1) as f32, 0, "")
}
};
// A faceted parameter is named by its *subject* — the band —
// because its aspect is already written above the run it sits
// in. Unfaceted parameters keep their own label, which is
// every operation but the mixer.
let param_label = match &p.facet {
Some(f) => labels::resolve(f.subject.0),
None => labels::resolve(p.label.0),
};
let aspect = p.facet.as_ref().map(|f| f.aspect.0);
let starts_facet = aspect.is_some() && aspect != previous_aspect;
previous_aspect = aspect;
rows.push(ParamRow {
op_index: op_index as i32,
param_index: param_index as i32,
op_label: labels::resolve(op.label.0).into(),
param_label: param_label.into(),
facet_label: aspect.map(labels::resolve).unwrap_or_default().into(),
starts_facet,
// -1 rather than an `Option`, which a Slint struct cannot
// carry: 0° is red, so no value in range can stand for
// "no swatch".
swatch_hue: p.facet.as_ref().and_then(|f| f.subject_hue).unwrap_or(-1.0),
group_head: group_head as i32,
group_len,
group_modified,
kind: kind.into(),
value: p.value,
default_value: p.default,
minimum: min,
maximum: max,
precision,
unit: unit.into(),
// Only curve rows carry points.
points: slint::ModelRc::new(slint::VecModel::from(Vec::<f32>::new())),
choices: slint::ModelRc::new(slint::VecModel::from(choices)),
});
}
}
rows
}
/// One row standing for a whole curve.
///
/// 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.
fn curve_row(
op_index: usize,
group_head: usize,
op: &OpCapability,
presentation: &Presentation,
) -> 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;
}
/// One row standing for a whole curve.
///
/// 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.
fn curve_row(
&self,
op_index: usize,
group_head: usize,
op: &OpCapability,
presentation: &Presentation,
) -> 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);
// The widget addresses points by offset from the first, so they must
// be contiguous in the capability list.
let base = op
.params
.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;
}
// The widget addresses points by offset from the first, so they must
// be contiguous in the capability list.
let base = op
.params
.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,
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
// grid, so it heads no run and carries no swatch.
facet_label: String::new().into(),
starts_facet: false,
swatch_hue: -1.0,
group_head: group_head as i32,
// One widget standing for every parameter of the operation, so
// the group it heads is itself and nothing else.
group_len: 1,
group_modified: op.params.iter().any(|p| p.value != p.default),
kind: "curve".into(),
value: 0.0,
default_value: 0.0,
minimum: 0.0,
maximum: 1.0,
precision: 4,
unit: String::new().into(),
points: slint::ModelRc::new(slint::VecModel::from(points)),
})
}
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,
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
// grid, so it heads no run and carries no swatch.
facet_label: String::new().into(),
starts_facet: false,
swatch_hue: -1.0,
group_head: group_head as i32,
// One widget standing for every parameter of the operation, so
// the group it heads is itself and nothing else.
group_len: 1,
group_modified: op.params.iter().any(|p| p.value != p.default),
kind: "curve".into(),
value: 0.0,
default_value: 0.0,
minimum: 0.0,
maximum: 1.0,
precision: 4,
unit: String::new().into(),
points: slint::ModelRc::new(slint::VecModel::from(points)),
// A curve is not a choice between named alternatives.
choices: slint::ModelRc::new(slint::VecModel::from(Vec::<slint::SharedString>::new())),
})
}
impl DevelopSession {
/// The curve's shape, sampled for drawing.
///
/// Evaluated with `dr_pipeline`'s own spline, so the line the user drags
@@ -1091,6 +1158,135 @@ mod tests {
/// A widget hint only collapses an operation to one row when it is
/// *honoured*; `rows` falls back to sliders otherwise, and mirroring that
/// here is what keeps the test honest when a hint stops applying.
/// TRACES: FR-DEV-3c
/// An operation this file has never heard of, appearing in the panel.
///
/// The acceptance test requirements.md names for FR-DEV-3c: "a test
/// operation added to the registry appears in a generated panel with no
/// frontend change". Built as a capability rather than a real node so it
/// costs the pipeline nothing — what is being asserted is the mapping from
/// descriptor to control, and that mapping does not care whether a shader
/// exists behind it.
#[test]
fn an_operation_the_frontend_has_never_heard_of_gets_controls() {
use dr_pipeline::{LocalizedKey, ParamCapability};
let invented = OpCapability {
id: OpId("invented"),
label: LocalizedKey("op.invented"),
active: false,
presentation: None,
params: vec![
ParamCapability {
id: ParamId("strength"),
label: LocalizedKey("param.invented.strength"),
kind: ParamKind::Scalar {
min: -100.0,
max: 100.0,
scale: dr_pipeline::Scale::Linear,
unit: Unit::Percent,
precision: 0,
},
default: 0.0,
value: 25.0,
facet: None,
},
ParamCapability {
id: ParamId("method"),
label: LocalizedKey("param.invented.method"),
kind: ParamKind::Enum {
variants: &[
LocalizedKey("param.invented.method.fast"),
LocalizedKey("param.invented.method.exact"),
],
},
default: 0.0,
value: 1.0,
facet: None,
},
],
};
let rows = rows_from(&[invented]);
assert_eq!(rows.len(), 2, "each parameter should become one row");
// The scalar becomes a slider carrying its declared range and unit.
assert_eq!(rows[0].kind, "scalar");
assert_eq!(rows[0].minimum, -100.0);
assert_eq!(rows[0].maximum, 100.0);
assert_eq!(rows[0].value, 25.0);
// The enum becomes a choice, with its range spanning the variant
// indices and the variant names resolved for drawing. Nothing in this
// file names the operation or either parameter to make that happen.
assert_eq!(rows[1].kind, "enum");
assert_eq!(rows[1].minimum, 0.0);
assert_eq!(rows[1].maximum, 1.0);
assert_eq!(rows[1].precision, 0);
assert_eq!(slint::Model::row_count(&rows[1].choices), 2);
// The value is the selected index, which is what the segmented control
// reads — an enum needs no separate selection field.
assert_eq!(rows[1].value, 1.0);
}
#[test]
fn an_unimplemented_widget_falls_back_to_sliders_rather_than_vanishing() {
// ARCH §4.3a: falling off the end of the preference list is not an
// error. An operation asking only for a widget this frontend does not
// draw must still yield one control per parameter, or declaring a
// preference would be a way to make an edit unreachable.
use dr_pipeline::{LocalizedKey, ParamCapability, WidgetDemand};
let wheel = OpCapability {
id: OpId("grading"),
label: LocalizedKey("op.grading"),
active: false,
presentation: Some(Presentation {
widgets: &[WidgetKind::ColourWheel],
demand: WidgetDemand {
two_dimensional: true,
precise_pointing: false,
},
params: &[ParamId("hue"), ParamId("strength")],
}),
params: vec![
ParamCapability {
id: ParamId("hue"),
label: LocalizedKey("param.grading.hue"),
kind: ParamKind::Scalar {
min: 0.0,
max: 360.0,
scale: dr_pipeline::Scale::Linear,
unit: Unit::None,
precision: 0,
},
default: 0.0,
value: 0.0,
facet: None,
},
ParamCapability {
id: ParamId("strength"),
label: LocalizedKey("param.grading.strength"),
kind: ParamKind::Scalar {
min: 0.0,
max: 1.0,
scale: dr_pipeline::Scale::Linear,
unit: Unit::None,
precision: 2,
},
default: 0.0,
value: 0.0,
facet: None,
},
],
};
assert!(!draws(WidgetKind::ColourWheel), "precondition");
let rows = rows_from(&[wheel]);
assert_eq!(rows.len(), 2, "both parameters must remain reachable");
assert!(rows.iter().all(|r| r.kind == "scalar"));
}
fn rows_of(caps: &[OpCapability]) -> Vec<(usize, usize)> {
let mut rows = Vec::new();
for op in caps {
@@ -1381,7 +1577,9 @@ mod tests {
.presentation
.as_ref()
.expect("the curve declares a widget");
assert_eq!(presentation.widget, WidgetKind::Curve);
// Asked the way the panel asks it: the first preference this frontend
// implements, not a fixed single kind.
assert_eq!(presentation.choose(draws), Some(WidgetKind::ToneCurve));
// Every parameter is owned by the widget, so none is left over to be
// rendered as a stray slider.
assert_eq!(presentation.params.len(), curve_cap.params.len());