diff --git a/core/dr-pipeline/build.rs b/core/dr-pipeline/build.rs index 3718f5f..3f67af8 100644 --- a/core/dr-pipeline/build.rs +++ b/core/dr-pipeline/build.rs @@ -586,9 +586,51 @@ fn param_ctor( max, ) } + // A fixed list of named alternatives. The value is the chosen index, + // so the range is the list's own bounds and a node declaring one needs + // no `min:`/`max:` of its own. + // + // Variants are localisation keys, like every other label in a node — + // the core never holds a display string (NFR-A11Y-1). The default is + // always the first, so `reset` means the same thing here as everywhere + // else; a node whose neutral choice is not first has listed them in + // the wrong order. + "enum" => { + let variants = spec + .get("variants") + .and_then(Value::as_sequence) + .ok_or_else(|| format!("`{ctx}` is `kind: enum` and needs a `variants:` list"))?; + if variants.len() < 2 { + return Err(format!( + "`{ctx}.variants` lists {} choice(s); a control the user \ + cannot change is not a control", + variants.len() + )); + } + let keys = variants + .iter() + .enumerate() + .map(|(i, v)| { + as_str(v, &format!("{ctx}.variants[{i}]")) + .map(|k| format!("LocalizedKey({k:?})")) + }) + .collect::, _>>()?; + ( + format!( + "ParamDescriptor::choice({:?}, {:?}, &[{}])", + id, + label, + keys.join(", ") + ), + 0.0, + 0.0, + (variants.len() - 1) as f64, + ) + } other => { return Err(format!( - "`{ctx}.kind` is `{other}`; expected stops, amount, switch, fraction or scalar" + "`{ctx}.kind` is `{other}`; expected stops, amount, switch, \ + fraction, scalar or enum" )) } }) diff --git a/core/dr-pipeline/src/descriptor.rs b/core/dr-pipeline/src/descriptor.rs index 4d1cd23..c8ffe62 100644 --- a/core/dr-pipeline/src/descriptor.rs +++ b/core/dr-pipeline/src/descriptor.rs @@ -59,23 +59,91 @@ pub enum Unit { /// A control that does not reduce to a slider or a switch. /// -/// The core names the *kind* of widget; `dr-widgets` owns what it looks like -/// and how it behaves (ARCH §3.3). This is deliberately a small closed -/// enum rather than an open string: a UI must be able to match exhaustively -/// and know it has covered everything the core can ask for. +/// The core names the *kind* of widget; the frontend owns what it looks like +/// and how it behaves (ARCH §3.3). This is deliberately a small closed enum +/// rather than an open string: a UI must be able to match exhaustively and +/// know it has covered everything the core can ask for. +/// +/// **Every one of these is a hint over ordinary scalar parameters**, never a +/// new kind of value. A curve is its point coordinates, a colour wheel is +/// three numbers, a crop is four edges — all [`ParamKind::Scalar`], all +/// individually addressable, all persisted by the sidecar with no special +/// case. That is what makes the fallback in [`Presentation::widgets`] honest: +/// a frontend that implements none of these still renders every parameter as +/// a slider and the edit works, merely more tediously. +/// +/// It is also why there is no `Colour` *kind*. A colour is three or four +/// numbers, and introducing a value type that is not an `f32` would reach +/// through the graph, the uniform block and the sidecar format to buy a +/// control that [`ColourWheel`](WidgetKind::ColourWheel) already describes. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum WidgetKind { /// A tone curve, edited by dragging points on a grid. /// - /// The underlying parameters are ordinary [`ParamKind::Scalar`]s — the - /// point coordinates — so a UI that does not implement the curve widget - /// can still present them as sliders and remain fully functional. That - /// fallback is the reason the points are scalars rather than an opaque - /// blob. - Curve, + /// The underlying parameters are the point coordinates, x and y + /// interleaved. + ToneCurve, + /// Colour grading wheels: a hue-and-strength pad per tonal range. + ColourWheel, + /// On-canvas crop and straighten handles. Parameters are the four crop + /// edges as fractions, and the straighten angle. + CropOverlay, + /// On-canvas placement of a linear or radial mask. + GradientHandle, + /// On-canvas brush strokes. + BrushMask, + /// An eyedropper bound to the canvas, setting white balance from a pixel. + WhitePoint, +} + +impl WidgetKind { + /// TRACES: FR-UI-7 + /// Whether this widget is manipulated on the photograph rather than in a + /// panel. + /// + /// A property of the widget itself, not of the screen: a crop is dragged + /// on the image wherever the image is, and that is true of a phone and a + /// workstation alike. Where the panel then *puts* the affordance that + /// turns it on, and how large its handles are, stay frontend decisions + /// (ARCH §4.3a). + pub fn is_on_canvas(self) -> bool { + matches!( + self, + Self::CropOverlay | Self::GradientHandle | Self::BrushMask | Self::WhitePoint + ) + } +} + +/// TRACES: FR-DEV-3a +/// What a widget inherently needs in order to be usable. +/// +/// **Demands describe the control, not the screen** (ARCH §4.3a). A curve +/// needs two-dimensional pointing and a certain amount of room to be worth +/// drawing at all; those are facts about curves. Whether *this* window has +/// that room, at what breakpoint, on what platform, is the frontend's +/// question, and a demand carrying pixels or a platform name would be the core +/// answering it — a core that reasons about pixels will eventually be wrong +/// about a display it never saw. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct WidgetDemand { + /// Needs the value to be dragged in two dimensions at once. A frontend + /// with only a keyboard, or a strictly linear input, should skip it. + pub two_dimensional: bool, + /// Needs pointing accurate to a small fraction of the control. A frontend + /// driving a television with a remote should skip it; touch is fine, since + /// hit regions grow to the modality (FR-UI-7). + pub precise_pointing: bool, } /// The shape of a parameter's value. +/// +/// **Every variant is carried as an `f32`.** That is not an implementation +/// detail to be tidied away later — it is what lets one storage path serve +/// every parameter: the sidecar writes a number, the uniform block takes a +/// number, and `set_param` is one function rather than one per shape. A +/// variant that needed a richer value would reach through all three, which is +/// why a colour is a [`WidgetKind::ColourWheel`] over three scalars rather +/// than a kind of its own. #[derive(Debug, Clone, PartialEq)] pub enum ParamKind { Scalar { @@ -86,6 +154,24 @@ pub enum ParamKind { precision: u8, }, Bool, + /// TRACES: FR-DEV-3a + /// One of a short, fixed list of named alternatives. + /// + /// The value is the chosen variant's **index**, held as an `f32` like + /// everything else — small integers are exact in binary32, so this costs + /// nothing in fidelity and keeps the parameter on the ordinary storage + /// path. + /// + /// Distinct from a `Scalar` running 0..n because the numbers are not on a + /// scale: interpolating between two of them is meaningless, dragging + /// through them is not a gesture anyone wants, and the labels are the + /// whole point. A UI that treated this as a scalar would render a slider + /// reading "2" where the user needs to see "Bicubic". + Enum { + /// In index order. The label is a localisation key, resolved by the + /// frontend — `core/` must not depend on a localiser (NFR-A11Y-1). + variants: &'static [LocalizedKey], + }, } /// TRACES: FR-DEV-3a | FR-DEV-3b @@ -105,14 +191,44 @@ pub enum ParamKind { /// operations, which want plain sliders, say nothing at all. #[derive(Debug, Clone, PartialEq)] pub struct Presentation { - pub widget: WidgetKind, - /// The parameters this widget owns, in the order it expects them. + /// Widgets that would draw these parameters, **in descending order of + /// preference** (ARCH §4.3a). + /// + /// The frontend walks the list and takes the first it both implements and + /// can afford. Falling off the end is not an error: every parameter + /// remains an individually addressable scalar, so plain sliders are always + /// the final fallback and the edit still works. + /// + /// A list rather than one kind because the alternatives are real. A colour + /// grading operation is best as a wheel, acceptable as a hue-and-strength + /// pair of sliders, and an operation that can say so gets a good control + /// on a workstation and a usable one on a phone without the core knowing + /// which it is talking to. + pub widgets: &'static [WidgetKind], + /// What the preferred widget needs in order to be worth drawing. + /// + /// Applies to the list as a whole rather than per entry: a frontend that + /// cannot meet the demand skips to plain sliders, which is the same answer + /// it gives for a widget it has not implemented. + pub demand: WidgetDemand, + /// The parameters these widgets own, in the order they expect them. /// /// Parameters absent from this list are presented normally, so an /// operation can pair a curve with an ordinary strength slider. pub params: &'static [ParamId], } +impl Presentation { + /// The first widget in the preference list that `supported` accepts. + /// + /// The walk lives here rather than in each frontend so that "first + /// supported, else sliders" is written once and cannot drift between a + /// desktop UI, a test harness and whatever consumes capabilities next. + pub fn choose(&self, supported: impl Fn(WidgetKind) -> bool) -> Option { + self.widgets.iter().copied().find(|w| supported(*w)) + } +} + /// TRACES: FR-DEV-3a /// A parameter's place in an operation whose parameters form a grid. /// @@ -206,6 +322,26 @@ impl ParamDescriptor { } } + /// One of a fixed list of alternatives, defaulting to the first. + /// + /// The first rather than a caller-chosen index, so the reset contract + /// holds the way it does for every other kind: index 0 is the neutral + /// choice, and an operation whose default is not its first variant has + /// listed them in the wrong order. + pub const fn choice( + id: &'static str, + label: &'static str, + variants: &'static [LocalizedKey], + ) -> Self { + Self { + id: ParamId(id), + label: LocalizedKey(label), + kind: ParamKind::Enum { variants }, + default: 0.0, + facet: None, + } + } + /// A 0…1 fraction — a proportion of something, rather than an amount. /// /// Its own constructor because the crop rect needs four of them and the @@ -299,6 +435,19 @@ impl ParamDescriptor { 0.0 } } + // Rounded before clamping, because the value arriving here is an + // `f32` that has been through a sidecar and possibly a slider: an + // index of 1.9999 is variant 2, and truncating it to 1 would + // silently select the wrong option. A non-finite index falls back + // to the default for the same reason a scalar does. + ParamKind::Enum { variants } => { + if value.is_finite() { + let last = variants.len().saturating_sub(1) as f32; + value.round().clamp(0.0, last) + } else { + self.default + } + } } } } diff --git a/core/dr-pipeline/src/graph.rs b/core/dr-pipeline/src/graph.rs index 9d0abea..1ecca06 100644 --- a/core/dr-pipeline/src/graph.rs +++ b/core/dr-pipeline/src/graph.rs @@ -447,6 +447,24 @@ mod tests { ); } ParamKind::Bool => {} + ParamKind::Enum { variants } => { + // An empty list is a control with nothing to pick, and + // a one-entry list is a control that cannot be + // changed — both are declaration mistakes rather than + // states a UI should try to render. + assert!( + variants.len() > 1, + "{}.{} offers fewer than two choices", + cap.id, + p.id + ); + assert!( + p.default >= 0.0 && p.default < variants.len() as f32, + "{}.{} defaults to a variant that does not exist", + cap.id, + p.id + ); + } } } } @@ -536,6 +554,13 @@ mod tests { c.label.0, p.label.0, p.value ), ParamKind::Bool => format!("{}/{}: switch", c.label.0, p.label.0), + ParamKind::Enum { variants } => format!( + "{}/{}: choice of {} = {}", + c.label.0, + p.label.0, + variants.len(), + p.value + ), }) }) .collect(); diff --git a/core/dr-pipeline/src/lib.rs b/core/dr-pipeline/src/lib.rs index 1256ff5..a9a15fe 100644 --- a/core/dr-pipeline/src/lib.rs +++ b/core/dr-pipeline/src/lib.rs @@ -41,7 +41,7 @@ pub mod sidecar; pub use descriptor::{ Facet, LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId, ParamKind, Presentation, - Scale, Unit, WidgetKind, + Scale, Unit, WidgetDemand, WidgetKind, }; pub use framing::{CropRect, Framing}; pub use graph::{EditGraph, OpCapability, ParamCapability}; @@ -81,6 +81,11 @@ mod tests { } } ParamKind::Bool => 1.0 - p.default, + // The last variant, so the choice differs from the + // default whatever the list holds. A single-variant enum + // cannot be moved off its default and is correctly left + // where it is. + ParamKind::Enum { variants } => variants.len().saturating_sub(1) as f32, }; g.set_param(desc.id, p.id, v); } diff --git a/core/dr-pipeline/src/ops/curve.rs b/core/dr-pipeline/src/ops/curve.rs index 91d0e94..ca0294e 100644 --- a/core/dr-pipeline/src/ops/curve.rs +++ b/core/dr-pipeline/src/ops/curve.rs @@ -33,7 +33,7 @@ use crate::descriptor::{ LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId, Presentation, Scale, Unit, - WidgetKind, + WidgetDemand, WidgetKind, }; use crate::operation::{Helper, Operation, Uniform}; use crate::ops::helpers; @@ -293,7 +293,16 @@ impl Operation for ToneCurve { fn presentation(&self) -> Option { Some(Presentation { - widget: WidgetKind::Curve, + // One entry: there is no second way to draw a tone curve that is + // better than the sliders the frontend falls back to anyway. + widgets: &[WidgetKind::ToneCurve], + demand: WidgetDemand { + // A point is dragged in x and y together — that is what a + // curve *is*, and a frontend that can only move one axis at a + // time is better off with the point coordinates as sliders. + two_dimensional: true, + precise_pointing: true, + }, params: &CURVE_PARAMS, }) } @@ -614,7 +623,14 @@ mod tests { // If the presentation misses one, that slider appears twice: once in // the curve and once as a stray control beneath it. let presentation = ToneCurve::new().presentation().expect("declares a widget"); - assert_eq!(presentation.widget, WidgetKind::Curve); + assert_eq!(presentation.widgets, &[WidgetKind::ToneCurve]); + // A frontend that implements the curve gets it; one that implements + // nothing falls through to sliders rather than to an error. + assert_eq!( + presentation.choose(|w| w == WidgetKind::ToneCurve), + Some(WidgetKind::ToneCurve) + ); + assert_eq!(presentation.choose(|_| false), None); assert_eq!(presentation.params.len(), DESCRIPTOR.params.len()); for p in DESCRIPTOR.params { assert!( diff --git a/ui/dr-ui/src/develop.rs b/ui/dr-ui/src/develop.rs index 4dc281a..9074529 100644 --- a/ui/dr-ui/src/develop.rs +++ b/ui/dr-ui/src/develop.rs @@ -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 { - 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 { + 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::::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 = 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::::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 { + // 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 { - // 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 = 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 = 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::::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()); diff --git a/ui/dr-ui/ui/adjust.slint b/ui/dr-ui/ui/adjust.slint index bfe99f8..96cc1f7 100644 --- a/ui/dr-ui/ui/adjust.slint +++ b/ui/dr-ui/ui/adjust.slint @@ -9,6 +9,7 @@ import { Theme } from "theme.slint"; import { PanelHeading, Label, Value, Caption, Button, IconButton, Swatch } from "widgets.slint"; +import { SliderTrack, ControlRow, CurveEditor, Segmented } from "controls.slint"; // One parameter, flattened for Slint's model system. // @@ -43,7 +44,7 @@ export struct ParamRow { // Which control to build. Mirrors ParamKind, plus the widget kinds an // operation can request through its presentation. - kind: string, // "scalar" | "bool" | "curve" + kind: string, // "scalar" | "bool" | "enum" | "curve" // A run of rows inside a group, for an operation whose parameters form a // grid rather than a list. @@ -83,6 +84,14 @@ export struct ParamRow { // `param-index` on a curve row is the index of the *first* point // parameter, so a drag routes back by offsetting from it. points: [float], + + // Enum rows only: the variant names, in index order. + // + // Resolved in Rust against the UI's catalogue, like every other label + // here — the core publishes localisation keys and never a display string. + // `value` on such a row is the chosen index, which is why an enum needs no + // separate selection field. + choices: [string], } // The name of a group of controls, and what can be done to the group. @@ -151,165 +160,6 @@ component GroupHeading inherits Rectangle { } } -// **The** slider. One track, one hit area, one set of gesture rules. -// -// This exists because there used to be two of these, written out separately — -// one reading its geometry from a `ParamRow` for the generated panel, one -// taking plain numbers for the straighten angle — with a comment claiming the -// duplication was safe because "the track behaviour is the same and -// deliberately so". It was not safe and did not stay the same: the moment the -// touch arbitration below was fixed in one copy, the two sliders in the same -// sidebar started behaving differently, and which one you got depended on which -// panel you happened to be dragging in. The wrappers below now differ only in -// where their numbers come from. -component SliderTrack inherits Rectangle { - in property value; - in property default-value; - in property minimum; - in property maximum; - - callback changed(float); - callback reset(); - /// The pointer is on this control. A scrolling ancestor listens so it can - /// stand down — see the note on `engaged` below. - callback engaged-changed(bool); - - height: Theme.touch-target / 2; - - // Guarded, because a descriptor with a zero range would otherwise divide - // by nothing and put every position at infinity. - property span: max(0.000001, root.maximum - root.minimum); - - // **Why hover, and not the drag itself.** - // - // A Flickable does not merely compete for a gesture, it *withholds* the - // press: `DelayForwarding` holds it back for 100ms and only delivers it if - // nothing has claimed the gesture by then. A finger that starts moving - // inside that window therefore leaves this TouchArea never pressed at all — - // so `moved` never fires, and any handler keyed on the drag having started - // can never run. That is why the panel kept taking sliders away from a - // finger while a tap worked perfectly: a tap's release arrives before the - // 100ms is up, so press and release are delivered together, and only a - // *drag* falls in the hole. - // - // Hover is the one signal that does get through. Move events are dispatched - // to children even while the press is withheld, so the moment a finger - // lands on a track and travels a pixel this goes true, the panel sets - // `interactive: false`, and the Flickable stops arbitrating before it can - // capture anything. - // - // The cost is that a drag *starting* on a track no longer scrolls the - // panel. The label above each track and the padding around it still do, and - // the wheel is unaffected — a Flickable handles wheel events whether or not - // it is interactive. - property engaged: area.has-hover || area.claimed; - changed engaged => { root.engaged-changed(root.engaged); } - - // The rail. - Rectangle { - y: (parent.height - 3px) / 2; - height: 3px; - background: Theme.surface-raised; - border-radius: 1.5px; - } - - // The default position, drawn only when it is not at an end. Hand-built - // rather than using the standard Slint slider so this can be marked at - // all — a symmetric control needs to show where zero is. - if root.minimum < root.default-value && root.default-value < root.maximum: Rectangle { - x: (root.default-value - root.minimum) / root.span * parent.width - 1px; - y: (parent.height - 9px) / 2; - width: 2px; - height: 9px; - background: Theme.rule; - } - - // Fill from the default to the current value, so the control shows the - // size and direction of the adjustment rather than an absolute magnitude. - Rectangle { - property default-x: - (root.default-value - root.minimum) / root.span * parent.width; - property value-x: - (root.value - root.minimum) / root.span * parent.width; - - x: min(self.default-x, self.value-x); - width: abs(self.value-x / 1px - self.default-x / 1px) * 1px; - y: (parent.height - 3px) / 2; - height: 3px; - // The fill is the engaged part of the control — the span the - // photographer has actually moved — so it takes `active` rather than - // the ink the rest of the track is drawn in. - background: Theme.active; - border-radius: 1.5px; - } - - Rectangle { - x: (root.value - root.minimum) / root.span * parent.width - 6px; - y: (parent.height - 12px) / 2; - width: 12px; - height: 12px; - border-radius: 6px; - background: area.has-hover || area.pressed ? Theme.ink : Theme.ink-dim; - } - - area := TouchArea { - // Explicitly fill the track. A TouchArea with no geometry collapses to - // zero and only reports the events that happen to land on it, which - // shows up as a slider that clicks but does not drag. - width: 100%; - height: 100%; - - // Whether this gesture has been claimed as a slider drag. - // - // A scrolling panel acts vertically and this control acts - // horizontally, so the axis of the movement says which was meant. - // Committing on press-down instead — the obvious approach — makes - // every attempt to scroll from a slider jump its value first, which is - // destructive and happens constantly given how much of a panel is - // sliders. - property claimed: false; - - function value-at(px: length) -> float { - return clamp( - root.minimum + (px / self.width) * root.span, - root.minimum, - root.maximum); - } - - moved => { - // `moved` fires only while pressed, so this is a drag. - if (!self.claimed - && abs(self.mouse-x - self.pressed-x) - > abs(self.mouse-y - self.pressed-y)) { - self.claimed = true; - } - if (self.claimed) { - root.changed(self.value-at(self.mouse-x)); - } - } - pointer-event(ev) => { - if (ev.kind == PointerEventKind.up - || ev.kind == PointerEventKind.cancel) { - self.claimed = false; - } - // Right-click resets, alongside double-click. - if (ev.kind == PointerEventKind.down - && ev.button == PointerEventButton.right) { - root.reset(); - } - } - clicked => { - // A press with no meaningful drag: jump to it. Handled on release - // rather than on press so it cannot fire during a scroll that - // merely started here. - if (!self.claimed) { - root.changed(self.value-at(self.mouse-x)); - } - } - double-clicked => { root.reset(); } - } -} - // A parameter's value, at the precision its descriptor declares. // // A global rather than the same ternary written into every control that shows @@ -327,6 +177,13 @@ global Readout { } // One generated parameter: a label, a readout, and the track above. +// +// The readout is *not* editable, where the settings page's `SliderRow` pairs +// the same track with a number box. That is a considered difference rather than +// an inconsistency: this column is 280px wide and the colour mixer alone puts +// thirty-six of these in it, so a text box per row would be most of the width +// and a keyboard target nobody is aiming for. The number is still reachable — +// the track resets on double-click and right-click. component ParamSlider inherits Rectangle { in property data; callback changed(float); @@ -336,27 +193,13 @@ component ParamSlider inherits Rectangle { height: 46px; - VerticalLayout { - spacing: 2px; - - HorizontalLayout { - Label { - text: root.data.param-label; - emphasised: root.data.value != root.data.default-value; - } - - Rectangle { horizontal-stretch: 1; } - - Value { - text: Readout.of(root.data); - // The one readout in the panel that has moved off its default - // is what the eye is hunting for, and `modified` is the only - // thing left to say it with once hue is gone. - modified: root.data.value != root.data.default-value; - placeholder: root.data.value == root.data.default-value; - compact: true; - } - } + ControlRow { + label: root.data.param-label; + readout: Readout.of(root.data); + // The one readout in the panel that has moved off its default is what + // the eye is hunting for, and `modified` is the only thing left to say + // it with once hue is gone. + modified: root.data.value != root.data.default-value; SliderTrack { value: root.data.value; @@ -486,24 +329,10 @@ component PlainSlider inherits Rectangle { height: 46px; - VerticalLayout { - spacing: 2px; - - HorizontalLayout { - Label { - text: root.label; - emphasised: root.value != root.default-value; - } - - Rectangle { horizontal-stretch: 1; } - - Value { - text: (Math.round(root.value * 10) / 10) + root.unit; - modified: root.value != root.default-value; - placeholder: root.value == root.default-value; - compact: true; - } - } + ControlRow { + label: root.label; + readout: (Math.round(root.value * 10) / 10) + root.unit; + modified: root.value != root.default-value; SliderTrack { value: root.value; @@ -518,6 +347,97 @@ component PlainSlider inherits Rectangle { } } +// **The control registry: one row in, one control out.** +// +// Slint cannot instantiate a component from a runtime string, so mapping a +// declared kind to a control is necessarily a chain of `if`s. The thing worth +// insisting on is that there is exactly *one* such chain. There were two — the +// panel draws a lone parameter bare and a group under a heading, and each +// branch wrote out its own list of kinds — so `enum` would have had to be added +// in both, and a kind added to only one would appear or vanish depending on how +// many parameters its operation happened to declare. +// +// Everything below routes back through `param-changed` by index. This component +// knows a curve point spans two parameters and a swatch names a hue band; it +// knows nothing about which operation it is drawing, which is the property that +// makes it a registry rather than a panel. +component ParamControl inherits Rectangle { + in property data; + /// Curve rows only; ignored by every other kind. + in property <[float]> curve-samples; + + callback param-changed(int, int, float); + callback param-reset(int, int); + callback curve-reset(int); + callback drag-changed(bool); + + height: layout.preferred-height; + + layout := VerticalLayout { + spacing: 0px; + alignment: start; + + // A row whose subject is a colour is identified by that colour; every + // other scalar keeps its name. The two differ only in what stands in + // for the label — the track, the gestures and the routing are the same + // underneath. + if root.data.kind == "scalar" && root.data.swatch-hue >= 0: SwatchSlider { + data: root.data; + drag-changed(on) => { root.drag-changed(on); } + changed(v) => { + root.param-changed(root.data.op-index, root.data.param-index, v); + } + reset => { + root.param-reset(root.data.op-index, root.data.param-index); + } + } + + if root.data.kind == "scalar" && root.data.swatch-hue < 0: ParamSlider { + data: root.data; + drag-changed(on) => { root.drag-changed(on); } + changed(v) => { + root.param-changed(root.data.op-index, root.data.param-index, v); + } + reset => { + root.param-reset(root.data.op-index, root.data.param-index); + } + } + + // A fixed list of alternatives. The value *is* the index, so picking + // one is an ordinary parameter change and needs no separate route. + // + // Chips rather than a dropdown for the same reason the settings page + // uses them: these lists are short, and a collapsed menu hides the + // alternatives behind a click. ARCH §4.3 names the dropdown as the + // pointer presentation of the same kind, so this is where that choice + // will be made when the modality switch lands. + if root.data.kind == "enum": Segmented { + label: root.data.param-label; + options: root.data.choices; + selected: Math.round(root.data.value); + picked(i) => { + root.param-changed(root.data.op-index, root.data.param-index, i); + } + } + + 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); + } + reset => { root.curve-reset(root.data.op-index); } + } + } +} + // Crop, rotation, flips and straightening — the framing controls. // // **Why this is hand-built when the rest of the panel is generated.** The @@ -632,165 +552,6 @@ export component GeometryPanel inherits Rectangle { } } -// A tone curve editor: a square grid with draggable control points. -// -// The curve *line* is drawn from `samples`, which Rust evaluates with the -// same spline the shader uses. Reimplementing the interpolation here would -// mean two curves that could disagree — the drawn one and the applied one — -// which is the worst possible failure for a control whose whole job is to -// show you what it is doing. -component CurveEditor inherits Rectangle { - in property data; - // Polyline of the curve, y values sampled at even x. 0..1, y up. - in property <[float]> samples; - // Which point is being dragged, or -1. - in-out property active-point: -1; - - callback point-moved(int, float, float); - callback reset(); - /// The pointer is on a control point. The panel stands its Flickable down - /// while it is, for the reason spelled out on `ParamSlider`'s `engaged` — - /// and more acutely here, because a curve point is dragged *vertically*, - /// which is the Flickable's own axis and so is contested every time. - callback drag-changed(bool); - - property point-count: root.data.points.length / 2; - - // Hover, not the drag: `active-point` is set on press, and the press is - // exactly what a Flickable withholds. Only the plot's grab targets count, - // so the rest of the plot still scrolls the panel. - property engaged: root.active-point >= 0 || root.hovered-point >= 0; - changed engaged => { root.drag-changed(root.engaged); } - - // The point the pointer is over, or -1. Set by the grab targets below, - // and used only to highlight the marker. - in-out property hovered-point: -1; - - // Square: a tone curve is read as a deviation from the 45° diagonal, and - // that reading only works if the axes share a scale. - height: self.width; - - plot := Rectangle { - background: Theme.ground; - border-width: 1px; - border-color: Theme.rule; - - // Quarter gridlines and the identity diagonal, so the shape of the - // edit is legible at a glance. - for i in [1, 2, 3]: Rectangle { - x: parent.width * i / 4; - width: 1px; - background: Theme.rule; - opacity: 0.5; - } - for i in [1, 2, 3]: Rectangle { - y: parent.height * i / 4; - height: 1px; - background: Theme.rule; - opacity: 0.5; - } - - // The curve. One thin rectangle per sample: Slint has no polyline - // primitive, and at this size the segments are sub-pixel anyway. - for s[i] in root.samples: Rectangle { - property next: i + 1 < root.samples.length - ? root.samples[i + 1] : s; - x: parent.width * i / max(root.samples.length - 1, 1); - width: parent.width / max(root.samples.length - 1, 1) + 1px; - // Span the segment vertically, so a steep section stays joined. - y: parent.height * (1.0 - max(s, self.next)); - height: max(parent.height * abs(self.next - s), 1.5px); - background: Theme.active; - } - - // Control points. - for idx in [0, 1, 2, 3, 4]: Rectangle { - property exists: idx < root.point-count; - property px: root.data.points[idx * 2]; - property py: root.data.points[idx * 2 + 1]; - - visible: self.exists; - x: parent.width * self.px - 5px; - y: parent.height * (1.0 - self.py) - 5px; - width: 10px; - height: 10px; - border-radius: 5px; - // Grown and tinted when grabbable, so it is obvious where the - // curve takes the gesture and where the panel scrolls instead. - property live: root.active-point == idx - || root.hovered-point == idx; - background: self.live ? Theme.active : Theme.ink; - border-width: 1px; - border-color: Theme.ground; - } - - // **One grab target per point, and nothing covering the rest.** - // - // Three constraints meet here, and only this arrangement satisfies - // all of them: - // - // 1. The panel scrolls, and Slint cannot hand back a press once - // taken — so an area spanning the plot would swallow every scroll - // gesture beginning over the curve. Small targets leave the rest - // of the plot free. - // 2. `enabled: false` does not work as a gate: a disabled TouchArea - // recognises *no* events at all, hover included, so it cannot - // report where the pointer is in order to decide. - // 3. A target positioned by its own point would slide out from under - // the pointer on the first movement, stalling the drag. So while - // a point is being dragged its target **freezes** at the press - // position and grows to cover the plot, keeping the pointer - // inside it however far the point travels. - for idx in [0, 1, 2, 3, 4]: TouchArea { - property exists: idx < root.point-count; - property dragging: root.active-point == idx; - - // Frozen and expanded while dragging; tracking the point - // otherwise. - x: self.dragging ? 0px - : parent.width * root.data.points[idx * 2] - 14px; - y: self.dragging ? 0px - : parent.height * (1.0 - root.data.points[idx * 2 + 1]) - 14px; - width: self.dragging ? parent.width : 28px; - height: self.dragging ? parent.height : 28px; - visible: self.exists; - mouse-cursor: pointer; - - // Highlights the marker, so it is visible where the curve takes - // the gesture and where the panel scrolls instead. - changed has-hover => { - if (self.has-hover) { - root.hovered-point = idx; - } else if (root.hovered-point == idx) { - root.hovered-point = -1; - } - } - - pointer-event(ev) => { - if (ev.kind == PointerEventKind.down - && ev.button == PointerEventButton.left) { - root.active-point = idx; - } - if (ev.kind == PointerEventKind.up - || ev.kind == PointerEventKind.cancel) { - root.active-point = -1; - } - } - moved => { - if (self.dragging) { - // Coordinates are relative to this area, which is the - // whole plot while dragging — so no offset is needed. - root.point-moved( - idx, - clamp(self.mouse-x / parent.width, 0.0, 1.0), - clamp(1.0 - self.mouse-y / parent.height, 0.0, 1.0)); - } - } - double-clicked => { root.reset(); } - } - } -} - // The panel: a heading per multi-parameter operation, a control per parameter. // // **Why the loop is shaped the way it is.** `rows` is flat, and Slint can @@ -906,33 +667,15 @@ export component AdjustPanel inherits Rectangle { // // `row` is the entry here — the group's head is its only // member — so there is nothing to index back into. - if row.group-head == i && row.group-len == 1: VerticalLayout { - spacing: 0px; - - if row.kind == "scalar": ParamSlider { - data: row; - drag-changed(on) => { root.slider-dragging = on; } - changed(v) => { - root.param-changed( - row.op-index, row.param-index, v); - } - reset => { - root.param-reset(row.op-index, row.param-index); - } - } - - if row.kind == "curve": CurveEditor { - data: row; - samples: root.curve-samples; - drag-changed(on) => { root.slider-dragging = on; } - point-moved(point, x, y) => { - root.param-changed( - row.op-index, row.param-index + point * 2, x); - root.param-changed( - row.op-index, row.param-index + point * 2 + 1, y); - } - reset => { root.curve-reset(row.op-index); } + if row.group-head == i && row.group-len == 1: ParamControl { + data: row; + curve-samples: root.curve-samples; + 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); } } // `if` rather than a zero height: a hidden-but-present @@ -975,52 +718,17 @@ export component AdjustPanel inherits Rectangle { title: entry.facet-label; } - // A row whose subject is a colour is identified by - // that colour; every other scalar keeps its name. - // The two differ only in what stands in for the - // label — the track, the gestures and the routing - // are the same underneath. - if entry.kind == "scalar" && entry.swatch-hue >= 0: SwatchSlider { + ParamControl { data: entry; + curve-samples: root.curve-samples; drag-changed(on) => { root.slider-dragging = on; } - changed(v) => { - root.param-changed( - entry.op-index, entry.param-index, v); + param-changed(op, param, v) => { + root.param-changed(op, param, v); } - reset => { - root.param-reset(entry.op-index, entry.param-index); - } - } - - if entry.kind == "scalar" && entry.swatch-hue < 0: ParamSlider { - data: entry; - drag-changed(on) => { root.slider-dragging = on; } - changed(v) => { - root.param-changed( - entry.op-index, entry.param-index, v); - } - reset => { - root.param-reset(entry.op-index, entry.param-index); - } - } - - if entry.kind == "curve": CurveEditor { - data: entry; - samples: root.curve-samples; - drag-changed(on) => { root.slider-dragging = 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( - entry.op-index, entry.param-index + point * 2, x); - root.param-changed( - entry.op-index, entry.param-index + point * 2 + 1, y); - } - reset => { - root.curve-reset(entry.op-index); + param-reset(op, param) => { + root.param-reset(op, param); } + curve-reset(op) => { root.curve-reset(op); } } } } diff --git a/ui/dr-ui/ui/controls.slint b/ui/dr-ui/ui/controls.slint new file mode 100644 index 0000000..f095afb --- /dev/null +++ b/ui/dr-ui/ui/controls.slint @@ -0,0 +1,870 @@ +// The control vocabulary: everything that takes input. +// +// **Why this is a second file rather than more of widgets.slint.** That file +// establishes the rule — screens consume components, and a bare `Theme.*` at a +// call site means a component is missing — and it establishes it for *chrome*: +// buttons, panels, headings, the text roles. Chrome is drawn and read. What is +// below is dragged, typed into and toggled, which is a different kind of thing +// with a different set of concerns: gesture arbitration, validation, hit +// targets, and what a control does when the value moves under it. Keeping them +// apart is what lets either file stay readable. +// +// **The rule this file establishes.** A control's *behaviour* is written once, +// here, and its numbers come from the caller. Before this file the slider lived +// inside the develop panel and was reachable from nowhere else, so the settings +// page — which has a 1-to-100 quality and three other bounded numbers — offered +// a free-text box instead, and `to-float()` silently turned a typo into zero. A +// primitive that only one screen can reach is not a primitive. +// +// **Nothing here knows about `ParamRow`.** That struct is the develop panel's +// flattening of the capability model and lives in adjust.slint; a control that +// imported it would drag the whole develop model into the settings page. The +// primitives take plain numbers and strings, and the ParamRow-shaped wrappers +// stay in the panel that owns the model. This is the constraint that makes the +// file reusable, so it is worth stating rather than merely observing. + +import { Theme } from "theme.slint"; +import { Icon, Label, Value, Caption, Field } from "widgets.slint"; + +// --- the slider --------------------------------------------------------- + +// **The** slider track. One track, one hit area, one set of gesture rules. +// +// This exists because there used to be two of these, written out separately — +// one reading its geometry from a `ParamRow` for the generated panel, one +// taking plain numbers for the straighten angle — with a comment claiming the +// duplication was safe because "the track behaviour is the same and +// deliberately so". It was not safe and did not stay the same: the moment the +// touch arbitration below was fixed in one copy, the two sliders in the same +// sidebar started behaving differently, and which one you got depended on which +// panel you happened to be dragging in. The wrappers now differ only in where +// their numbers come from. +// +// It moved here from adjust.slint unchanged. The argument above is the reason +// the move matters: a track private to one panel is a track the next screen +// reimplements, which is the same failure one level up. +export component SliderTrack inherits Rectangle { + in property value; + in property default-value; + in property minimum; + in property maximum; + + /// Live, once per movement. For anything that should follow the drag: a + /// readout, a preview, the image itself. + callback changed(float); + /// The gesture is over and this is the value to keep. + /// + /// **Two callbacks because there are two costs.** Re-rendering a + /// photograph on every movement is the entire point of a develop slider, + /// and the pipeline is built for it. Writing a settings file on every + /// movement is not: a two-second drag is a couple of hundred serialise- + /// and-save round trips where a text field committed once, and on Android + /// that goes through the Storage Access Framework. So a caller whose + /// handler is cheap listens to `changed`, and one whose handler is + /// expensive listens to this — rather than every such caller inventing its + /// own debounce, which is how two of them come to disagree about when an + /// edit is finished. + callback committed(float); + callback reset(); + /// The pointer is on this control. A scrolling ancestor listens so it can + /// stand down — see the note on `engaged` below. + callback engaged-changed(bool); + + height: Theme.touch-target / 2; + + // Guarded, because a descriptor with a zero range would otherwise divide + // by nothing and put every position at infinity. + property span: max(0.000001, root.maximum - root.minimum); + + // **Why hover, and not the drag itself.** + // + // A Flickable does not merely compete for a gesture, it *withholds* the + // press: `DelayForwarding` holds it back for 100ms and only delivers it if + // nothing has claimed the gesture by then. A finger that starts moving + // inside that window therefore leaves this TouchArea never pressed at all — + // so `moved` never fires, and any handler keyed on the drag having started + // can never run. That is why the panel kept taking sliders away from a + // finger while a tap worked perfectly: a tap's release arrives before the + // 100ms is up, so press and release are delivered together, and only a + // *drag* falls in the hole. + // + // Hover is the one signal that does get through. Move events are dispatched + // to children even while the press is withheld, so the moment a finger + // lands on a track and travels a pixel this goes true, the panel sets + // `interactive: false`, and the Flickable stops arbitrating before it can + // capture anything. + // + // The cost is that a drag *starting* on a track no longer scrolls the + // panel. The label above each track and the padding around it still do, and + // the wheel is unaffected — a Flickable handles wheel events whether or not + // it is interactive. + property engaged: area.has-hover || area.claimed; + changed engaged => { root.engaged-changed(root.engaged); } + + // The rail. + Rectangle { + y: (parent.height - 3px) / 2; + height: 3px; + background: Theme.surface-raised; + border-radius: 1.5px; + } + + // The default position, drawn only when it is not at an end. Hand-built + // rather than using the standard Slint slider so this can be marked at + // all — a symmetric control needs to show where zero is. + if root.minimum < root.default-value && root.default-value < root.maximum: Rectangle { + x: (root.default-value - root.minimum) / root.span * parent.width - 1px; + y: (parent.height - 9px) / 2; + width: 2px; + height: 9px; + background: Theme.rule; + } + + // Fill from the default to the current value, so the control shows the + // size and direction of the adjustment rather than an absolute magnitude. + Rectangle { + property default-x: + (root.default-value - root.minimum) / root.span * parent.width; + property value-x: + (root.value - root.minimum) / root.span * parent.width; + + x: min(self.default-x, self.value-x); + width: abs(self.value-x / 1px - self.default-x / 1px) * 1px; + y: (parent.height - 3px) / 2; + height: 3px; + // The fill is the engaged part of the control — the span the + // photographer has actually moved — so it takes `active` rather than + // the ink the rest of the track is drawn in. + background: Theme.active; + border-radius: 1.5px; + } + + Rectangle { + x: (root.value - root.minimum) / root.span * parent.width - 6px; + y: (parent.height - 12px) / 2; + width: 12px; + height: 12px; + border-radius: 6px; + background: area.has-hover || area.pressed ? Theme.ink : Theme.ink-dim; + } + + area := TouchArea { + // Explicitly fill the track. A TouchArea with no geometry collapses to + // zero and only reports the events that happen to land on it, which + // shows up as a slider that clicks but does not drag. + width: 100%; + height: 100%; + + // Whether this gesture has been claimed as a slider drag. + // + // A scrolling panel acts vertically and this control acts + // horizontally, so the axis of the movement says which was meant. + // Committing on press-down instead — the obvious approach — makes + // every attempt to scroll from a slider jump its value first, which is + // destructive and happens constantly given how much of a panel is + // sliders. + property claimed: false; + + // The last value this gesture emitted, held until it is committed. + // + // Kept here rather than read back from `root.value` at release: the + // parent may or may not have fed the live value back down, and a + // commit that reported the *old* number on a caller who ignored + // `changed` would silently save the wrong thing. + property pending-value; + property pending: false; + + function value-at(px: length) -> float { + return clamp( + root.minimum + (px / self.width) * root.span, + root.minimum, + root.maximum); + } + + function emit(v: float) { + self.pending-value = v; + self.pending = true; + root.changed(v); + } + + // Idempotent, because both the pointer-up and the `clicked` that + // follows it are legitimate places to notice a gesture has ended and + // only one of them should commit. + function commit() { + if (self.pending) { + self.pending = false; + root.committed(self.pending-value); + } + } + + moved => { + // `moved` fires only while pressed, so this is a drag. + if (!self.claimed + && abs(self.mouse-x - self.pressed-x) + > abs(self.mouse-y - self.pressed-y)) { + self.claimed = true; + } + if (self.claimed) { + self.emit(self.value-at(self.mouse-x)); + } + } + pointer-event(ev) => { + if (ev.kind == PointerEventKind.up + || ev.kind == PointerEventKind.cancel) { + self.claimed = false; + // A drag ending outside the track never produces `clicked`, so + // the commit cannot wait for one. + self.commit(); + } + // Right-click resets, alongside double-click. + if (ev.kind == PointerEventKind.down + && ev.button == PointerEventButton.right) { + root.reset(); + } + } + clicked => { + // A press with no meaningful drag: jump to it. Handled on release + // rather than on press so it cannot fire during a scroll that + // merely started here. + if (!self.claimed) { + self.emit(self.value-at(self.mouse-x)); + } + self.commit(); + } + double-clicked => { root.reset(); } + } +} + +// --- numeric entry ------------------------------------------------------ + +// A number typed rather than dragged, held to a declared range and precision. +// +// **The parsing is the point.** `Field` plus `to-float()` — which is what the +// settings page did before this existed — cannot tell a rejected entry from a +// deliberate zero, because `to-float()` answers 0 for both. So a mistyped +// export quality silently became 0, was saved, and the page then displayed the +// 0 as though the user had asked for it. `is-float()` is the missing question, +// and asking it is the whole reason this is a component and not a `Field` with +// a call-site handler. +// +// **A rejected entry reverts rather than erroring.** There is nowhere to put a +// validation message on a settings row that would not push every control below +// it down the page, and a control that rejects input by snapping back to the +// last good value has already said what happened. Out-of-range is different +// from unparseable and is *not* rejected: 500 in a 1-to-100 field is a clear +// intention, so it clamps to 100 and shows the 100, which is both what the +// pipeline received and an answer to "why did that not take". +export component NumberField inherits Rectangle { + in property value; + in property minimum; + in property maximum; + /// Decimal places shown, and the precision an entry is held to. Zero for a + /// count, two for a value in stops — the same figure a parameter + /// descriptor declares. + in property precision: 0; + in property enabled: true; + + callback changed(float); + + width: 72px; + height: Theme.touch-target; + horizontal-stretch: 0; + opacity: root.enabled ? 1.0 : 0.4; + + property factor: Math.pow(10, root.precision); + + pure function shown(v: float) -> string { + return Math.round(v * root.factor) / root.factor + ""; + } + + // The text in the box, deliberately *not* a binding on `value`. + // + // A binding would be broken permanently the first time the TextInput wrote + // through it — Slint drops a binding on assignment — so the box would + // follow the value until the first keystroke and never again. Seeded on + // `init` and rewritten from the two events where rewriting is correct: + // when the value moves under the box (a slider drag, a reset elsewhere), + // and when an entry is committed. + in-out property text; + init => { root.text = root.shown(root.value); } + changed value => { root.text = root.shown(root.value); } + + function commit() { + if (root.text.is-float()) { + root.changed(clamp(root.text.to-float(), root.minimum, root.maximum)); + } + // Rewritten either way. A valid entry is echoed back clamped and at the + // declared precision, so the box always shows the number the pipeline + // actually holds; an invalid one is discarded and the last good value + // returns. + root.text = root.shown(root.value); + } + + field := Field { + width: 100%; + height: 100%; + text <=> root.text; + // Committed on Enter *and* on losing focus, matching `TextRow`: Enter + // alone loses the edit the moment the user clicks the next control, + // which on a page that saves continuously reads as the setting not + // having taken. + accepted => { root.commit(); } + } + + property focused: field.has-focus; + changed focused => { + if (!self.focused) { + root.commit(); + } + } +} + +// --- booleans ----------------------------------------------------------- + +// A tick-box and its label: one setting that is either on or off. +// +// Written twice before this — `FormatCheck` in launch.slint and `Switch` in +// settings.slint — from the same 18px box with the same `active` fill and the +// same toggle handler. The second copy carried a comment saying the lift +// belonged here "once a third caller appears", which was the right instinct +// and the wrong threshold: the two copies had already drifted in height, and +// the third caller is this file establishing what a boolean looks like. +// +// **A ticked box fills rather than merely outlining.** `active` is the token +// for an engaged control, and it is what the slider fill and the focus border +// take, so a checked box reads as the same kind of state as those. +export component Check inherits Rectangle { + in property label; + /// Why the default is what it is, for settings where the consequence is + /// not obvious from the name — upscaling being off, location being + /// stripped. Empty on a box whose label says it already; a page of bare + /// switches makes the user guess at what each one costs. + in property hint; + in-out property checked; + + callback toggled(bool); + + height: max(row.preferred-height, Theme.control-height); + + touch := TouchArea { + // FR-UI-3: the drawn row is shorter than a touch target, so the target + // grows past its own bounds rather than the ink growing. + height: max(parent.height, Theme.touch-target); + y: (parent.height - self.height) / 2; + mouse-cursor: pointer; + clicked => { + root.checked = !root.checked; + root.toggled(root.checked); + } + } + + row := HorizontalLayout { + spacing: Theme.gap; + alignment: start; + + Rectangle { + width: 18px; + height: 18px; + y: (parent.height - self.height) / 2; + border-radius: Theme.radius-sm; + border-width: 1px; + border-color: root.checked ? Theme.active : Theme.rule; + background: root.checked ? Theme.active : transparent; + + Icon { + name: "check"; + // Dark on the fill: `active` is near-white, and the white tick + // this carried against a saturated accent is invisible on it. + ink: Theme.ground; + size: 11px; + visible: root.checked; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; + } + } + + VerticalLayout { + spacing: 1px; + alignment: center; + + Label { text: root.label; body: true; emphasised: touch.has-hover; } + Caption { text: root.hint; visible: root.hint != ""; wrap: word-wrap; } + } + } +} + +// --- one choice out of several ------------------------------------------ + +// One term in a `Segmented`. +// +// Not `FilterChip`, which widgets.slint keeps for the library's rating filter: +// that one carries a count and has on-off semantics, where this is +// single-selection over a fixed list. They look alike and behave differently, +// which is exactly the case for two components rather than one with a flag. +export component ChoiceChip inherits Rectangle { + in property label; + in property selected: false; + in property enabled: true; + + callback clicked(); + + height: Theme.control-height; + // Wide enough that a one-word label is still a comfortable target, which + // is what `control-min-width` exists for — but chips sit several to a row, + // so they take their own narrower floor rather than the button's. + min-width: 64px; + border-radius: Theme.radius; + border-width: 1px; + border-color: root.selected ? Theme.active : Theme.rule; + background: !root.enabled ? transparent + : (root.selected ? Theme.active + : (touch.pressed ? Theme.pressed + : (touch.has-hover ? Theme.hover : transparent))); + opacity: root.enabled ? 1.0 : 0.4; + + touch := TouchArea { + height: max(parent.height, Theme.touch-target); + y: (parent.height - self.height) / 2; + enabled: root.enabled; + mouse-cursor: pointer; + clicked => { root.clicked(); } + } + + Text { + text: root.label; + // Dark on the fill: `active` is near-white and ink on it is invisible. + color: root.selected ? Theme.ground : Theme.ink; + font-size: Theme.text; + horizontal-alignment: center; + vertical-alignment: center; + width: 100%; + height: 100%; + } +} + +// --- the labelled-row scaffold ------------------------------------------ + +// A control's name, with an aside about it on the same line. +// +// Written out three times inside settings.slint before this — once each in the +// choice row, the entry row and the switch — as a body `Label` beside a +// right-aligned elided `Caption`. Three copies of a two-element layout is how +// the hint ends up aligned differently from one row to the next. +// +// The name sits *above* its control rather than beside it: the control rows +// are wide, and a left-hand label column would either crush them or leave the +// page half empty at narrow widths. Stacked, every row uses the full width at +// any window size (FR-UI-1). +export component FieldRow inherits HorizontalLayout { + in property label; + in property hint; + + spacing: Theme.gap; + + Label { text: root.label; body: true; } + + Caption { + text: root.hint; + horizontal-alignment: right; + horizontal-stretch: 1; + overflow: elide; + } +} + +// A labelled row of chips: one choice out of a short list. +// +// Not a dropdown. Every choice set on the settings page is short and the +// options are worth reading side by side — a photographer picking an output +// colour space benefits from seeing that ProPhoto exists next to sRGB, which a +// collapsed menu hides behind a click. ARCH §4.3 names the dropdown as the +// *pointer* presentation of the same `Enum`; this is the segmented one, and +// when the modality switch lands it will sit beside a dropdown rather than be +// replaced by one. +export component Segmented inherits VerticalLayout { + in property label; + in property hint; + in property <[string]> options; + in property selected: 0; + in property enabled: true; + + callback picked(int); + + spacing: 4px; + + FieldRow { label: root.label; hint: root.hint; } + + HorizontalLayout { + spacing: Theme.gap-sm; + alignment: start; + + for option[i] in root.options: ChoiceChip { + label: option; + selected: i == root.selected; + enabled: root.enabled; + clicked => { root.picked(i); } + } + } +} + +// A text entry with its label above and an optional unit after it. +// +// `Field` is `touch-target` tall and stretches, which is right for a server URL +// on the launch screen and wrong for a filename template — so this constrains +// the width rather than restyling the field. +export component TextRow inherits VerticalLayout { + in property label; + in property hint; + in-out property text; + in property unit; + in property placeholder; + in property enabled: true; + in property field-width: 140px; + + callback accepted(string); + + spacing: 4px; + + FieldRow { label: root.label; hint: root.hint; } + + HorizontalLayout { + spacing: Theme.gap-sm; + alignment: start; + + Rectangle { + width: root.field-width; + height: field.preferred-height; + opacity: root.enabled ? 1.0 : 0.4; + + field := Field { + width: 100%; + text <=> root.text; + placeholder: root.placeholder; + // Committed on Enter *and* on losing focus. Enter alone loses + // an edit the moment the user clicks the next control, which + // on a page that saves continuously reads as the setting not + // having taken. + accepted(t) => { root.accepted(t); } + } + + // `Field` reports focus but does not signal losing it, so the + // change is watched here. + property focused: field.has-focus; + changed focused => { + if (!self.focused) { + root.accepted(root.text); + } + } + } + + Label { + text: root.unit; + visible: root.unit != ""; + vertical-alignment: center; + height: Theme.touch-target; + } + } +} + +// --- sliders that carry their own readout ------------------------------- + +// A control with its name and current value on the line above it. +// +// The develop panel's shape, generalised: a parameter's name on the left, what +// it currently reads on the right, and the control itself beneath. The control +// arrives as `@children` rather than being built here, so the same header +// serves a track today and whatever else wants one later without this +// component learning what a track is. +// +// **`modified` drives both halves.** A value differing from its default is the +// single thing a photographer scans a panel for, and with hue gone from the +// palette the only signal left is luminance — so the label lights and the +// readout changes ink together, from one fact, rather than two call sites each +// deciding. +export component ControlRow inherits VerticalLayout { + in property label; + /// Already formatted. Precision belongs to whoever owns the value — a + /// descriptor declares it — and a control that rounded on its own would + /// show the same parameter two ways in the same panel. + in property readout; + in property modified: false; + + spacing: 2px; + + HorizontalLayout { + Label { text: root.label; emphasised: root.modified; } + + Rectangle { horizontal-stretch: 1; } + + Value { + text: root.readout; + modified: root.modified; + placeholder: !root.modified; + compact: true; + } + } + + @children +} + +// A slider and an editable number box for the same value. +// +// **The pointer presentation of a bounded scalar** (ARCH §4.3): the track for +// choosing a value by eye, the box for saying one exactly. Neither alone is +// enough — a track cannot express "exactly 90", and a bare number box makes +// the user guess what the range is until they exceed it. +// +// This is what the settings page's bounded numbers should have been. Export +// quality is 1-to-100 and was a free-text field, so the range was written in a +// hint and enforced nowhere, and `to-float()` turned a typo into zero. +// +// Distinct from `ControlRow` + `SliderTrack`, which is what the develop panel +// uses: there the readout is *not* editable, because that column is 280px wide +// and holds thirty-six of these in the colour mixer alone — a text box per row +// would be most of the width and a keyboard target nobody is aiming for. Two +// presentations of one idea, and which is right depends on how many are on +// screen at once. +export component SliderRow inherits VerticalLayout { + in property label; + in property hint; + in property value; + in property default-value; + in property minimum; + in property maximum; + in property precision: 0; + in property enabled: true; + + /// Fires once per completed gesture, not once per movement. + /// + /// This row exists for settings-shaped values, whose handlers persist — + /// so it takes `SliderTrack`'s `committed` rather than its `changed` and + /// spares every call site the debounce. A caller that genuinely wants the + /// live stream, as the develop panel does, composes `ControlRow` with a + /// bare track instead. + callback changed(float); + callback reset(); + + spacing: 4px; + + // What the controls draw while a drag is in flight. + // + // The committed value only arrives at the end of the gesture, so without + // this the handle would sit still under the finger for the whole drag and + // jump at release. Seeded and re-seeded imperatively rather than bound: + // Slint drops a binding on the first assignment, so a bound property would + // follow `value` until the first drag and never again. + property live: root.value; + init => { root.live = root.value; } + changed value => { root.live = root.value; } + + // A declared precision is a declared *step*, not merely a display format. + // + // Without this the track hands out the raw position under the finger, so a + // quality of 89.6 reads as "90" in the box — `NumberField` rounds for + // display — and arrives at a caller storing whole numbers as 89. The box + // and the stored value would disagree by one, visibly, on release. Snapping + // here makes the two the same number by construction. + property step-factor: Math.pow(10, root.precision); + pure function quantise(v: float) -> float { + return Math.round(v * root.step-factor) / root.step-factor; + } + + FieldRow { label: root.label; hint: root.hint; } + + HorizontalLayout { + spacing: Theme.gap; + opacity: root.enabled ? 1.0 : 0.4; + + SliderTrack { + horizontal-stretch: 1; + // Centred against the number box, which is a full touch target + // tall where the track is half of one. + y: (parent.height - self.height) / 2; + + value: root.live; + default-value: root.default-value; + minimum: root.minimum; + maximum: root.maximum; + + changed(v) => { root.live = root.quantise(v); } + committed(v) => { root.changed(root.quantise(v)); } + reset => { root.reset(); } + } + + NumberField { + // Follows the drag, so the number and the handle never disagree. + value: root.live; + minimum: root.minimum; + maximum: root.maximum; + precision: root.precision; + enabled: root.enabled; + // A typed entry is already a completed gesture. + changed(v) => { root.changed(v); } + } + } +} + +// --- two-dimensional controls ------------------------------------------- + +// A tone curve editor: a square grid with draggable control points. +// +// The curve *line* is drawn from `samples`, which Rust evaluates with the same +// spline the shader uses. Reimplementing the interpolation here would mean two +// curves that could disagree — the drawn one and the applied one — which is the +// worst possible failure for a control whose whole job is to show you what it +// is doing. +// +// Takes `points` directly rather than the `ParamRow` it used to read, for the +// reason given in this file's preamble: a primitive that knows the develop +// panel's model can only ever be used by the develop panel. +export component CurveEditor inherits Rectangle { + /// Control point coordinates, x and y interleaved, each 0..1 with y up. + in property <[float]> points; + /// Polyline of the curve, y sampled at even x. 0..1, y up. + in property <[float]> samples; + /// Which point is being dragged, or -1. + in-out property active-point: -1; + + callback point-moved(int, float, float); + callback reset(); + /// The pointer is on a control point. The panel stands its Flickable down + /// while it is, for the reason spelled out on `SliderTrack`'s `engaged` — + /// and more acutely here, because a curve point is dragged *vertically*, + /// which is the Flickable's own axis and so is contested every time. + callback drag-changed(bool); + + property point-count: root.points.length / 2; + + // Hover, not the drag: `active-point` is set on press, and the press is + // exactly what a Flickable withholds. Only the plot's grab targets count, + // so the rest of the plot still scrolls the panel. + property engaged: root.active-point >= 0 || root.hovered-point >= 0; + changed engaged => { root.drag-changed(root.engaged); } + + // The point the pointer is over, or -1. Set by the grab targets below, and + // used only to highlight the marker. + in-out property hovered-point: -1; + + // Square: a tone curve is read as a deviation from the 45° diagonal, and + // that reading only works if the axes share a scale. + height: self.width; + + plot := Rectangle { + background: Theme.ground; + border-width: 1px; + border-color: Theme.rule; + + // Quarter gridlines and the identity diagonal, so the shape of the + // edit is legible at a glance. + for i in [1, 2, 3]: Rectangle { + x: parent.width * i / 4; + width: 1px; + background: Theme.rule; + opacity: 0.5; + } + for i in [1, 2, 3]: Rectangle { + y: parent.height * i / 4; + height: 1px; + background: Theme.rule; + opacity: 0.5; + } + + // The curve. One thin rectangle per sample: Slint has no polyline + // primitive, and at this size the segments are sub-pixel anyway. + for s[i] in root.samples: Rectangle { + property next: i + 1 < root.samples.length + ? root.samples[i + 1] : s; + x: parent.width * i / max(root.samples.length - 1, 1); + width: parent.width / max(root.samples.length - 1, 1) + 1px; + // Span the segment vertically, so a steep section stays joined. + y: parent.height * (1.0 - max(s, self.next)); + height: max(parent.height * abs(self.next - s), 1.5px); + background: Theme.active; + } + + // Control points. + for idx in [0, 1, 2, 3, 4]: Rectangle { + property exists: idx < root.point-count; + property px: root.points[idx * 2]; + property py: root.points[idx * 2 + 1]; + + visible: self.exists; + x: parent.width * self.px - 5px; + y: parent.height * (1.0 - self.py) - 5px; + width: 10px; + height: 10px; + border-radius: 5px; + // Grown and tinted when grabbable, so it is obvious where the + // curve takes the gesture and where the panel scrolls instead. + property live: root.active-point == idx + || root.hovered-point == idx; + background: self.live ? Theme.active : Theme.ink; + border-width: 1px; + border-color: Theme.ground; + } + + // **One grab target per point, and nothing covering the rest.** + // + // Three constraints meet here, and only this arrangement satisfies all + // of them: + // + // 1. The panel scrolls, and Slint cannot hand back a press once + // taken — so an area spanning the plot would swallow every scroll + // gesture beginning over the curve. Small targets leave the rest of + // the plot free. + // 2. `enabled: false` does not work as a gate: a disabled TouchArea + // recognises *no* events at all, hover included, so it cannot + // report where the pointer is in order to decide. + // 3. A target positioned by its own point would slide out from under + // the pointer on the first movement, stalling the drag. So while a + // point is being dragged its target **freezes** at the press + // position and grows to cover the plot, keeping the pointer inside + // it however far the point travels. + for idx in [0, 1, 2, 3, 4]: TouchArea { + property exists: idx < root.point-count; + property dragging: root.active-point == idx; + + // Frozen and expanded while dragging; tracking the point + // otherwise. + x: self.dragging ? 0px + : parent.width * root.points[idx * 2] - 14px; + y: self.dragging ? 0px + : parent.height * (1.0 - root.points[idx * 2 + 1]) - 14px; + width: self.dragging ? parent.width : 28px; + height: self.dragging ? parent.height : 28px; + visible: self.exists; + mouse-cursor: pointer; + + // Highlights the marker, so it is visible where the curve takes + // the gesture and where the panel scrolls instead. + changed has-hover => { + if (self.has-hover) { + root.hovered-point = idx; + } else if (root.hovered-point == idx) { + root.hovered-point = -1; + } + } + + pointer-event(ev) => { + if (ev.kind == PointerEventKind.down + && ev.button == PointerEventButton.left) { + root.active-point = idx; + } + if (ev.kind == PointerEventKind.up + || ev.kind == PointerEventKind.cancel) { + root.active-point = -1; + } + } + moved => { + if (self.dragging) { + // Coordinates are relative to this area, which is the whole + // plot while dragging — so no offset is needed. + root.point-moved( + idx, + clamp(self.mouse-x / parent.width, 0.0, 1.0), + clamp(1.0 - self.mouse-y / parent.height, 0.0, 1.0)); + } + } + double-clicked => { root.reset(); } + } + } +} diff --git a/ui/dr-ui/ui/launch.slint b/ui/dr-ui/ui/launch.slint index 51e7fc0..13edb9e 100644 --- a/ui/dr-ui/ui/launch.slint +++ b/ui/dr-ui/ui/launch.slint @@ -1,5 +1,6 @@ import { Theme } from "theme.slint"; -import { Button, PanelHeading, Label, Value, Caption, Panel, Field, Disclosure, Icon } from "widgets.slint"; +import { Button, PanelHeading, Label, Value, Caption, Panel, Field, Disclosure } from "widgets.slint"; +import { Check } from "controls.slint"; // Launch screen: connect an account, or resume a saved one. // @@ -7,61 +8,6 @@ import { Button, PanelHeading, Label, Value, Caption, Panel, Field, Disclosure, // with no library configured, and the place they return to in order to sign // out or switch account (FR-NC-1, FR-NC-4). -// One tick-box in the format selection. -component FormatCheck inherits Rectangle { - in property label; - in-out property checked; - callback toggled(bool); - - // FR-UI-3: 44pt minimum hit target under touch. The drawn row is - // shorter, so the touch area extends beyond the visible bounds. - height: 32px; - - touch := TouchArea { - height: max(parent.height, Theme.touch-target); - y: (parent.height - self.height) / 2; - clicked => { - root.checked = !root.checked; - root.toggled(root.checked); - } - } - - HorizontalLayout { - spacing: Theme.gap; - alignment: start; - - Rectangle { - width: 18px; - height: 18px; - y: (parent.height - self.height) / 2; - border-radius: 3px; - border-width: 1px; - // A ticked box is an engaged control, so it fills with `active` - // — the same token the slider fill and the focus border take. - border-color: root.checked ? Theme.active : Theme.rule; - background: root.checked ? Theme.active : transparent; - - Icon { - name: "check"; - // Dark on the fill: `active` is near-white, and the white - // tick this carried against a saturated accent is invisible - // against it. - ink: Theme.ground; - size: 11px; - visible: root.checked; - x: (parent.width - self.width) / 2; - y: (parent.height - self.height) / 2; - } - } - - Label { - text: root.label; - emphasised: touch.has-hover; - body: true; - } - } -} - // This screen's buttons are form actions in a single stacked column, not // chrome beside a photograph — they are given the full `touch-target` height // rather than the denser `control-height` the toolbars use. `Button` draws @@ -395,7 +341,7 @@ export component LaunchScreen inherits Rectangle { PanelHeading { text: "SCAN FOR"; } - for label[i] in root.format-labels: FormatCheck { + for label[i] in root.format-labels: Check { label: label; checked: root.format-checked[i]; toggled(on) => { root.format-toggled(i, on); } diff --git a/ui/dr-ui/ui/settings.slint b/ui/dr-ui/ui/settings.slint index 65e3a7e..abc4ba1 100644 --- a/ui/dr-ui/ui/settings.slint +++ b/ui/dr-ui/ui/settings.slint @@ -1,5 +1,6 @@ import { Theme } from "theme.slint"; -import { Button, PanelHeading, Label, Value, Caption, Panel, Field, ProgressBar, ActivityRow, Icon } from "widgets.slint"; +import { Button, PanelHeading, Label, Value, Caption, Panel, ProgressBar, ActivityRow } from "widgets.slint"; +import { Segmented, TextRow, Check, SliderRow } from "controls.slint"; // Settings: how much disk the app may spend, and what an export defaults to. // @@ -15,227 +16,6 @@ import { Button, PanelHeading, Label, Value, Caption, Panel, Field, ProgressBar, // switches views this way — launch, library, develop — and an overlay would be // a fourth mechanism for the same job. -// One choice out of several, drawn as a row of chips. -// -// Not a dropdown. Every choice set on this page is short and the options are -// worth reading side by side — a photographer picking an output colour space -// benefits from seeing that ProPhoto exists next to sRGB, which a collapsed -// menu hides behind a click. `FilterChip` in widgets.slint is the same idea for -// the library's rating filter, but it carries a count and a filter's -// on-off semantics; this is single-selection over a fixed list, so the -// behaviour differs where it matters. -component ChoiceChip inherits Rectangle { - in property label; - in property selected: false; - in property enabled: true; - callback clicked(); - - height: Theme.control-height; - // Wide enough that a one-word label is still a comfortable target, which - // is what `control-min-width` exists for — but chips sit several to a row, - // so they take their own narrower floor rather than the button's. - min-width: 64px; - border-radius: Theme.radius; - border-width: 1px; - border-color: root.selected ? Theme.active : Theme.rule; - // The selected chip fills, matching the checked box and the slider fill: - // `active` is the token for an engaged control, and this is the one chip in - // the row that is engaged. - background: !root.enabled ? transparent - : (root.selected ? Theme.active - : (touch.pressed ? Theme.pressed - : (touch.has-hover ? Theme.hover : transparent))); - opacity: root.enabled ? 1.0 : 0.4; - - touch := TouchArea { - // FR-UI-3: the drawn chip is `control-height`, so the target grows - // past its own bounds rather than the ink growing. - height: max(parent.height, Theme.touch-target); - y: (parent.height - self.height) / 2; - enabled: root.enabled; - mouse-cursor: pointer; - clicked => { root.clicked(); } - } - - Text { - text: root.label; - // Dark on the fill: `active` is near-white and ink on it is invisible. - color: root.selected ? Theme.ground : Theme.ink; - font-size: Theme.text; - horizontal-alignment: center; - vertical-alignment: center; - width: 100%; - height: 100%; - } -} - -// A labelled row of chips, with the label above rather than beside. -// -// Above, because the chip rows are wide and a left-hand label column would -// either crush them or leave the page half empty at narrow widths. Stacked, -// every row uses the full width at any window size (FR-UI-1). -component ChoiceRow inherits VerticalLayout { - in property label; - in property hint; - in property <[string]> options; - in property selected: 0; - in property enabled: true; - callback picked(int); - - spacing: 4px; - - HorizontalLayout { - spacing: Theme.gap; - Label { text: root.label; body: true; } - Caption { - text: root.hint; - horizontal-alignment: right; - horizontal-stretch: 1; - overflow: elide; - } - } - - HorizontalLayout { - spacing: Theme.gap-sm; - alignment: start; - - for option[i] in root.options: ChoiceChip { - label: option; - selected: i == root.selected; - enabled: root.enabled; - clicked => { root.picked(i); } - } - } -} - -// A switch: one setting that is either on or off. -// -// The tick-box shape is `FormatCheck`'s from launch.slint, which is the -// established idiom for a boolean in this codebase. Reproduced rather than -// shared because that one is private to the launch screen and lives inside its -// format list; lifting it into widgets.slint would be the better move once a -// third caller appears, and doing it for the second is how a component ends up -// with parameters for every caller's variation. -component Switch inherits Rectangle { - in property label; - in property hint; - in-out property checked; - callback toggled(bool); - - height: max(row.preferred-height, Theme.control-height); - - touch := TouchArea { - height: max(parent.height, Theme.touch-target); - y: (parent.height - self.height) / 2; - clicked => { - root.checked = !root.checked; - root.toggled(root.checked); - } - } - - row := HorizontalLayout { - spacing: Theme.gap; - alignment: start; - - Rectangle { - width: 18px; - height: 18px; - y: (parent.height - self.height) / 2; - border-radius: Theme.radius-sm; - border-width: 1px; - border-color: root.checked ? Theme.active : Theme.rule; - background: root.checked ? Theme.active : transparent; - - Icon { - name: "check"; - ink: Theme.ground; - size: 11px; - visible: root.checked; - x: (parent.width - self.width) / 2; - y: (parent.height - self.height) / 2; - } - } - - VerticalLayout { - spacing: 1px; - alignment: center; - Label { text: root.label; body: true; emphasised: touch.has-hover; } - // The hint carries *why* a default is what it is, for the settings - // where that is not obvious from the name — upscaling being off, - // location being stripped. A page of bare switches makes the user - // guess at the consequence of each. - Caption { text: root.hint; visible: root.hint != ""; wrap: word-wrap; } - } - } -} - -// A text entry with its label above and an optional unit after it. -// -// `Field` is `touch-target` tall and stretches, which is right for a server -// URL on the launch screen and wrong for a byte count — so this constrains the -// width rather than restyling the field. -component EntryRow inherits VerticalLayout { - in property label; - in property hint; - in-out property text; - in property unit; - in property placeholder; - in property enabled: true; - in property field-width: 140px; - callback accepted(string); - - spacing: 4px; - - HorizontalLayout { - spacing: Theme.gap; - Label { text: root.label; body: true; } - Caption { - text: root.hint; - horizontal-alignment: right; - horizontal-stretch: 1; - overflow: elide; - } - } - - HorizontalLayout { - spacing: Theme.gap-sm; - alignment: start; - - Rectangle { - width: root.field-width; - height: field.preferred-height; - opacity: root.enabled ? 1.0 : 0.4; - - field := Field { - width: 100%; - text <=> root.text; - placeholder: root.placeholder; - // Committed on Enter *and* on losing focus. Enter alone loses - // an edit the moment the user clicks the next control, which - // on a page that saves continuously reads as the setting not - // having taken. - accepted(t) => { root.accepted(t); } - } - - // `Field` reports focus but does not signal losing it, so the - // change is watched here. - property focused: field.has-focus; - changed focused => { - if (!self.focused) { - root.accepted(root.text); - } - } - } - - Label { - text: root.unit; - visible: root.unit != ""; - vertical-alignment: center; - height: Theme.touch-target; - } - } -} - // One background job: what it is, how far along, and what it last said. // // A row rather than a `Value`/`Caption` pair written out at the call site, @@ -533,7 +313,7 @@ export component SettingsPage inherits Rectangle { Rectangle { height: Theme.gap-sm; } - EntryRow { + TextRow { label: "Cached originals"; hint: "evicted oldest-first when full"; text <=> root.original-budget; @@ -542,7 +322,7 @@ export component SettingsPage inherits Rectangle { accepted(t) => { root.original-budget-changed(t); } } - Switch { + Check { label: "No limit on cached originals"; hint: "Nothing is ever evicted for space. " + "Pinned photographs are kept regardless."; @@ -552,7 +332,7 @@ export component SettingsPage inherits Rectangle { Rectangle { height: Theme.gap-sm; } - EntryRow { + TextRow { label: "Thumbnails and previews"; hint: "what the grid draws from"; text <=> root.thumbnail-budget; @@ -561,7 +341,7 @@ export component SettingsPage inherits Rectangle { accepted(t) => { root.thumbnail-budget-changed(t); } } - Switch { + Check { label: "No limit on thumbnails"; checked: root.thumbnail-unlimited; toggled(on) => { root.thumbnail-unlimited-toggled(on); } @@ -569,7 +349,7 @@ export component SettingsPage inherits Rectangle { Rectangle { height: Theme.gap-sm; } - Switch { + Check { label: "Keep originals after opening them"; hint: "The file was downloaded anyway, so keeping it " + "costs no bandwidth and saves the transfer next time."; @@ -596,26 +376,39 @@ export component SettingsPage inherits Rectangle { wrap: word-wrap; } - ChoiceRow { + Segmented { label: "Format"; options: root.format-labels; selected: root.format-selected; picked(i) => { root.format-picked(i); } } - EntryRow { + // A bounded number, so it gets the control for one. + // + // This was a free-text field: the range lived in the + // hint and was enforced nowhere, and `to-float()` + // answers 0 for anything unparseable — so a typo saved + // a quality of 0 and the page then showed the 0 back as + // though it had been asked for. The track carries the + // range and the box refuses what it cannot read. + SliderRow { label: "Quality"; // Says why it is greyed rather than leaving the // user to work out that PNG has no quality. hint: root.quality-enabled ? "1 to 100" : "the chosen format is lossless"; - text: root.quality; + value: root.quality; + // No meaningful neutral: quality has a sensible + // default but not a *zero*, and a default marker + // partway along a track reads as one. + default-value: 1; + minimum: 1; + maximum: 100; enabled: root.quality-enabled; - field-width: 90px; - accepted(t) => { root.quality-changed(t.to-float()); } + changed(v) => { root.quality-changed(v); } } - ChoiceRow { + Segmented { label: "Colour space"; hint: "profile embedded on export"; options: root.colour-labels; @@ -623,7 +416,7 @@ export component SettingsPage inherits Rectangle { picked(i) => { root.colour-picked(i); } } - ChoiceRow { + Segmented { label: "Size"; options: root.sizing-labels; selected: root.sizing-selected; @@ -633,7 +426,7 @@ export component SettingsPage inherits Rectangle { // Only where the chosen mode carries a number: // "Original" has none, and a field showing 0 beside it // would invite the reading "zero pixels". - if root.sizing-has-value: EntryRow { + if root.sizing-has-value: TextRow { label: "Size value"; text: root.sizing-value; unit: root.sizing-unit; @@ -641,7 +434,7 @@ export component SettingsPage inherits Rectangle { accepted(t) => { root.sizing-value-changed(t); } } - Switch { + Check { label: "Allow upscaling"; hint: "Off, a request larger than the source exports " + "at source size rather than failing."; @@ -649,7 +442,7 @@ export component SettingsPage inherits Rectangle { toggled(on) => { root.upscaling-toggled(on); } } - ChoiceRow { + Segmented { label: "Output sharpening"; hint: "scaled by the resize factor"; options: root.sharpening-labels; @@ -674,7 +467,7 @@ export component SettingsPage inherits Rectangle { PanelHeading { text: "FILES AND METADATA"; } - EntryRow { + TextRow { label: "Filename template"; hint: "{name} {seq} {date} {dimensions} {preset}"; text <=> root.filename-template; @@ -683,7 +476,7 @@ export component SettingsPage inherits Rectangle { accepted(t) => { root.template-changed(t); } } - ChoiceRow { + Segmented { label: "If the file exists"; options: root.collision-labels; selected: root.collision-selected; @@ -695,14 +488,14 @@ export component SettingsPage inherits Rectangle { // the Storage Access Framework at all, and on any // platform a server destination is reached over a // network that may not be there. - ChoiceRow { + Segmented { label: "Export to"; options: root.target-labels; selected: root.target-selected; picked(i) => { root.target-picked(i); } } - EntryRow { + TextRow { label: "Destination"; hint: "empty asks each time"; text <=> root.destination; @@ -711,7 +504,7 @@ export component SettingsPage inherits Rectangle { accepted(t) => { root.destination-changed(t); } } - Switch { + Check { label: "Strip location and personal metadata"; hint: "On. An export is usually the copy that leaves " + "this machine, and a location embedded in a "