diff --git a/Cargo.lock b/Cargo.lock index 4be1c73..f67bebd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1389,6 +1389,7 @@ version = "0.1.0" dependencies = [ "dr-types", "log", + "serde_norway", ] [[package]] diff --git a/core/dr-pipeline/Cargo.toml b/core/dr-pipeline/Cargo.toml index ac20719..86d1e75 100644 --- a/core/dr-pipeline/Cargo.toml +++ b/core/dr-pipeline/Cargo.toml @@ -11,3 +11,15 @@ license.workspace = true [dependencies] dr-types.workspace = true log.workspace = true + +# Nodes are declared in `ops/*.yaml` and compiled to Rust by `build.rs` +# (ARCH §5.7). The same reasoning as `ui/dr-ui`'s style.yaml: the declaration +# is the source of truth, the Rust is generated into OUT_DIR where it cannot +# be edited or committed by accident. +# +# serde_norway rather than serde_yaml for the reason recorded in the workspace +# manifest — it is the fork still receiving releases, and it preserves mapping +# order, which is what lets a node's parameters reach the panel in the order +# its author wrote them. +[build-dependencies] +serde_norway.workspace = true diff --git a/core/dr-pipeline/build.rs b/core/dr-pipeline/build.rs new file mode 100644 index 0000000..97c3510 --- /dev/null +++ b/core/dr-pipeline/build.rs @@ -0,0 +1,1668 @@ +//! Compiles the nodes in `ops/*.yaml` into Rust implementing `Operation`. +//! +//! # Why a node is a YAML file +//! +//! A develop operation is, in the overwhelming majority of cases, four facts: +//! what its parameters are, what uniforms they compute, what WGSL those +//! uniforms drive, and where it sits in the chain. Written in Rust those four +//! facts arrive wrapped in ninety lines of trait implementation — a `match` on +//! parameter id to a struct field, another `match` back, an `is_active` that +//! compares each field to its default, a `Vec` built by hand. Every +//! one of those is mechanical, and every one is a place to make a silent +//! mistake: a `param()` arm returning the wrong field reads perfectly and +//! breaks the sidecar round-trip. +//! +//! So the four facts are the file, and this generates the rest. +//! +//! # What it does not do +//! +//! It does not change the runtime model. The generated code implements the +//! same [`Operation`](../src/operation.rs) trait, publishes the same +//! `&'static OpDescriptor`, and composes through the same fused-shader path. +//! Nothing downstream — not `dr-gpu`, not the develop panel — can tell a +//! declared node from a hand-written one, which is what allows the two to sit +//! side by side in one chain. +//! +//! A node that needs more than the four facts stays in Rust and declares +//! itself with `rust:` instead (see `ops/tone_curve.yaml`). The escape hatch +//! is deliberate: a schema stretched to cover the tone curve's interpolator +//! would be a worse language than Rust, aimed at one caller. +//! +//! # Output +//! +//! `OUT_DIR/nodes.rs`, included by `src/ops/mod.rs`. In `OUT_DIR` rather than +//! beside the hand-written sources for the reason `ui/dr-ui/build.rs` records: +//! a generated file sitting in `src/ops/` looks exactly like the files around +//! it that *are* meant to be edited, and an edit to it survives until the next +//! `touch`. + +use std::collections::{BTreeMap, BTreeSet}; +use std::fmt::Write as _; +use std::path::{Path, PathBuf}; + +use serde_norway::{Mapping, Value}; + +/// The directory holding node declarations, relative to the manifest. +const OPS_DIR: &str = "ops"; +/// Declarations whose name begins with this are not nodes. +const NON_NODE_PREFIX: &str = "_"; +const HELPERS_FILE: &str = "_helpers.yaml"; +const GENERATED: &str = "nodes.rs"; + +fn main() { + println!("cargo:rerun-if-changed={OPS_DIR}"); + println!("cargo:rerun-if-changed=build.rs"); + + let manifest_dir = PathBuf::from(std::env::var_os("CARGO_MANIFEST_DIR").expect("manifest dir")); + let out_dir = PathBuf::from(std::env::var_os("OUT_DIR").expect("OUT_DIR")); + + let ops_dir = manifest_dir.join(OPS_DIR); + let src_ops = manifest_dir.join("src").join(OPS_DIR); + + match generate(&ops_dir, &src_ops) { + Ok(rust) => { + let path = out_dir.join(GENERATED); + std::fs::write(&path, rust) + .unwrap_or_else(|e| fail(format!("writing {}: {e}", path.display()))); + } + Err(e) => fail(e), + } +} + +/// Build scripts report failure through stderr and a non-zero exit; a panic +/// buries the message under a backtrace and the "process didn't exit +/// successfully" boilerplate, which is exactly the wrong thing when the +/// message is the name of the key the author got wrong. +fn fail(message: String) -> ! { + eprintln!("\nerror: {message}\n"); + std::process::exit(1); +} + +// --------------------------------------------------------------------------- +// The declaration model +// --------------------------------------------------------------------------- + +/// A WGSL helper function, either shared or declared by one node. +struct HelperDef { + name: String, + doc: Option, + wgsl: String, +} + +/// One parameter of a declared node. +struct ParamDef { + id: String, + label: String, + doc: Option, + /// The `ParamDescriptor` constructor call, already rendered. + ctor: String, + /// Needed to generate `is_active` and to range-check test values. + default: f64, + min: f64, + max: f64, +} + +/// A uniform the node's fragment reads, and the expression computing it. +struct UniformDef { + name: String, + doc: Option, + /// The Rust expression, compiled from the declared one. + rust: String, +} + +/// One generated `#[test]`. +struct TestDef { + name: String, + why: Option, + set: Vec<(String, f64)>, + expect: Vec<(String, f64)>, + expect_range: Vec<(String, f64, f64)>, + expect_active: Option, + expect_wgsl: Vec, + expect_helper_wgsl: Vec<(String, Vec)>, +} + +/// A node: either declared in full, or a pointer to a hand-written type. +enum Node { + Declared { + id: String, + label: String, + order: i64, + doc: Option, + placement: Option, + params: Vec, + uniforms: Vec, + /// Names of shared helpers, in declaration order. + shared_helpers: Vec, + /// Helpers this node defines for itself. + local_helpers: Vec, + wgsl: String, + /// `None` means the default rule: active when any parameter has moved. + active: Option, + tests: Vec, + }, + Rust { + id: String, + order: i64, + /// The type in `crate::ops` implementing `Operation`. + ty: String, + why_rust: Option, + placement: Option, + }, +} + +impl Node { + fn id(&self) -> &str { + match self { + Node::Declared { id, .. } | Node::Rust { id, .. } => id, + } + } + + fn order(&self) -> i64 { + match self { + Node::Declared { order, .. } | Node::Rust { order, .. } => *order, + } + } + + fn placement(&self) -> Option<&str> { + match self { + Node::Declared { placement, .. } | Node::Rust { placement, .. } => placement.as_deref(), + } + } +} + +// --------------------------------------------------------------------------- +// Driver +// --------------------------------------------------------------------------- + +fn generate(ops_dir: &Path, src_ops: &Path) -> Result { + let shared = read_helpers(&ops_dir.join(HELPERS_FILE))?; + let shared_names: BTreeSet<&str> = shared.helpers.iter().map(|h| h.name.as_str()).collect(); + + let mut nodes = Vec::new(); + for path in node_files(ops_dir)? { + let name = file_stem(&path); + let node = read_node(&path, &shared_names) + .map_err(|e| format!("{}/{}.yaml: {e}", OPS_DIR, name))?; + + // The filename and the id must agree. They are two names for one + // thing, and a node found by one and referred to by the other is a + // node nobody can grep for. + if node.id() != name { + return Err(format!( + "{}/{}.yaml declares `id: {}`; the file name and the id must match", + OPS_DIR, + name, + node.id() + )); + } + nodes.push(node); + } + + if nodes.is_empty() { + return Err(format!( + "{OPS_DIR}/ declares no nodes; expected at least one `.yaml`" + )); + } + + check_unique(&nodes)?; + check_no_shadowing(&nodes, src_ops)?; + nodes.sort_by_key(|n| n.order()); + + emit(&shared, &nodes) +} + +/// The node declarations, sorted so generation is deterministic. +/// +/// Determinism is not cosmetic here: the generated file is an input to +/// `rustc`'s incremental cache, and a set of nodes that reorders between runs +/// would rebuild the crate every time. +fn node_files(dir: &Path) -> Result, String> { + let entries = std::fs::read_dir(dir) + .map_err(|e| format!("cannot read {}: {e}", dir.display()))? + .collect::, _>>() + .map_err(|e| format!("cannot read {}: {e}", dir.display()))?; + + let mut files: Vec = entries + .into_iter() + .map(|e| e.path()) + .filter(|p| p.extension().is_some_and(|e| e == "yaml")) + .filter(|p| !file_stem(p).starts_with(NON_NODE_PREFIX)) + .collect(); + files.sort(); + Ok(files) +} + +fn file_stem(path: &Path) -> String { + path.file_stem() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_default() +} + +fn check_unique(nodes: &[Node]) -> Result<(), String> { + let mut ids = BTreeSet::new(); + let mut orders: BTreeMap = BTreeMap::new(); + for node in nodes { + if !ids.insert(node.id()) { + return Err(format!("`{}` is declared by two files", node.id())); + } + // Two nodes at one order is an ambiguous pipeline, and the pipeline's + // order changes what the image looks like. Sorting would silently pick + // one, so refuse instead. + if let Some(other) = orders.insert(node.order(), node.id()) { + return Err(format!( + "`{}` and `{}` both declare `order: {}`; the chain order would \ + be ambiguous, and operation order changes the result", + other, + node.id(), + node.order() + )); + } + } + Ok(()) +} + +/// A declared node must not share a name with a hand-written module. +/// +/// The same guard `ui/dr-ui/build.rs` applies to `theme.slint`: the generated +/// `pub mod exposure` and a `src/ops/exposure.rs` would both want that name, +/// and the error a reader gets from the collision says nothing about which +/// file to delete. +fn check_no_shadowing(nodes: &[Node], src_ops: &Path) -> Result<(), String> { + for node in nodes { + let Node::Declared { id, .. } = node else { + continue; + }; + let shadowed = src_ops.join(format!("{id}.rs")); + if shadowed.exists() { + return Err(format!( + "{} exists and would collide with the module generated from \ + {OPS_DIR}/{id}.yaml.\nThe node is now declared in YAML; delete \ + the stale Rust file.", + shadowed.display() + )); + } + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Reading: shared helpers +// --------------------------------------------------------------------------- + +struct SharedHelpers { + doc: Option, + helpers: Vec, +} + +fn read_helpers(path: &Path) -> Result { + let doc = read_yaml(path)?; + let root = as_mapping(&doc, HELPERS_FILE)?; + + let module_doc = opt_prose(root, "doc", HELPERS_FILE)?; + let helpers = root + .get("helpers") + .ok_or_else(|| format!("{HELPERS_FILE}: missing `helpers:` map"))?; + let helpers = as_mapping(helpers, "helpers")?; + + let mut out = Vec::new(); + for (name, spec) in helpers { + let name = as_str(name, "a helper name")?.to_string(); + let ctx = format!("helpers.{name}"); + let spec = as_mapping(spec, &ctx)?; + let wgsl = spec + .get("wgsl") + .ok_or_else(|| format!("{HELPERS_FILE}: `{ctx}` has no `wgsl:`"))?; + let wgsl = as_str(wgsl, &format!("{ctx}.wgsl"))?.trim_end().to_string(); + + // The composer deduplicates by name, so a helper whose declared name + // is not the function it defines would be emitted under one name and + // called under another. + if !wgsl.contains(&format!("fn {name}(")) { + return Err(format!( + "{HELPERS_FILE}: `{ctx}` does not define `fn {name}(`. The key \ + is the name the composer deduplicates on, so it has to be the \ + function actually declared." + )); + } + out.push(HelperDef { + doc: opt_prose(spec, "doc", &ctx)?, + name, + wgsl, + }); + } + + if out.is_empty() { + return Err(format!("{HELPERS_FILE}: `helpers:` is empty")); + } + Ok(SharedHelpers { + doc: module_doc, + helpers: out, + }) +} + +// --------------------------------------------------------------------------- +// Reading: a node +// --------------------------------------------------------------------------- + +fn read_node(path: &Path, shared: &BTreeSet<&str>) -> Result { + let doc = read_yaml(path)?; + let root = as_mapping(&doc, "the document")?; + + let id = as_str( + root.get("id").ok_or("missing `id:`")?, + "id", + )? + .to_string(); + check_ident(&id, "id")?; + + let order = root + .get("order") + .ok_or("missing `order:`; it is what places this node in the chain")? + .as_i64() + .ok_or("`order` must be a whole number")?; + + let placement = opt_prose(root, "placement", "placement")?; + + // A `rust:` node describes where a hand-written type sits, and nothing + // else — its descriptor comes from the type. Mixing the two forms would + // mean two sources for one node's parameters. + if let Some(ty) = root.get("rust") { + let ty = as_str(ty, "rust")?.to_string(); + check_ident(&ty, "rust")?; + for key in ["params", "uniforms", "wgsl", "helpers", "define", "label"] { + if root.contains_key(key) { + return Err(format!( + "`{key}` is meaningless on a `rust:` node — {ty} publishes \ + its own descriptor. Remove one or the other." + )); + } + } + return Ok(Node::Rust { + id, + order, + ty, + why_rust: opt_prose(root, "why_rust", "why_rust")?, + placement, + }); + } + + let label = as_str(root.get("label").ok_or("missing `label:`")?, "label")?.to_string(); + + let params = read_params(root)?; + let param_names: BTreeSet<&str> = params.iter().map(|p| p.id.as_str()).collect(); + + let local_helpers = read_local_helpers(root)?; + let shared_helpers = read_shared_refs(root, shared, &local_helpers)?; + + let uniforms = read_uniforms(root, ¶m_names)?; + + let wgsl = as_str(root.get("wgsl").ok_or("missing `wgsl:`")?, "wgsl")? + .trim_end() + .to_string(); + if wgsl.trim().is_empty() { + return Err("`wgsl:` is empty; a node that changes nothing is not a node".into()); + } + + let active = match root.get("active") { + None => None, + Some(v) => { + let expr = as_str(v, "active")?; + Some(compile_expr(expr, ¶m_names).map_err(|e| format!("`active`: {e}"))?) + } + }; + + let uniform_names: BTreeSet<&str> = uniforms.iter().map(|u| u.name.as_str()).collect(); + let helper_names: BTreeSet<&str> = shared_helpers + .iter() + .map(String::as_str) + .chain(local_helpers.iter().map(|h| h.name.as_str())) + .collect(); + let tests = read_tests(root, ¶ms, &uniform_names, &helper_names)?; + + Ok(Node::Declared { + id, + label, + order, + doc: opt_prose(root, "doc", "doc")?, + placement, + params, + uniforms, + shared_helpers, + local_helpers, + wgsl, + active, + tests, + }) +} + +fn read_params(root: &Mapping) -> Result, String> { + let params = root + .get("params") + .ok_or("missing `params:`; an operation with no parameters has nothing to control")?; + let params = as_mapping(params, "params")?; + + let mut out = Vec::new(); + for (id, spec) in params { + let id = as_str(id, "a parameter name")?.to_string(); + check_ident(&id, &format!("params.{id}"))?; + let ctx = format!("params.{id}"); + let spec = as_mapping(spec, &ctx)?; + + let label = as_str( + spec.get("label") + .ok_or_else(|| format!("`{ctx}` has no `label:`"))?, + &format!("{ctx}.label"), + )? + .to_string(); + + let kind = as_str( + spec.get("kind") + .ok_or_else(|| format!("`{ctx}` has no `kind:`"))?, + &format!("{ctx}.kind"), + )?; + + let (ctor, default, min, max) = param_ctor(&id, &label, kind, spec, &ctx)?; + out.push(ParamDef { + id, + label, + doc: opt_prose(spec, "doc", &ctx)?, + ctor, + default, + min, + max, + }); + } + + if out.is_empty() { + return Err("`params:` is empty".into()); + } + Ok(out) +} + +/// Render the `ParamDescriptor` constructor for a declared parameter kind. +/// +/// The kinds are the constructors `descriptor.rs` already offers, named rather +/// than spelled out: `amount` is the −100…+100 shape nearly every photographic +/// control takes, and writing its range in every node would invite one of them +/// to drift. +fn param_ctor( + id: &str, + label: &str, + kind: &str, + spec: &Mapping, + ctx: &str, +) -> Result<(String, f64, f64, f64), String> { + let need = |key: &str| -> Result { + spec.get(key) + .and_then(Value::as_f64) + .ok_or_else(|| format!("`{ctx}` is `kind: {kind}` and needs a numeric `{key}:`")) + }; + let opt = |key: &str, fallback: f64| -> f64 { + spec.get(key).and_then(Value::as_f64).unwrap_or(fallback) + }; + + Ok(match kind { + "stops" => { + let (min, max) = (need("min")?, need("max")?); + ( + format!( + "ParamDescriptor::stops({:?}, {:?}, {}, {})", + id, + label, + rust_f32(min), + rust_f32(max) + ), + 0.0, + min, + max, + ) + } + "amount" => ( + format!("ParamDescriptor::amount({id:?}, {label:?})"), + 0.0, + -100.0, + 100.0, + ), + "switch" => ( + format!("ParamDescriptor::switch({id:?}, {label:?})"), + 0.0, + 0.0, + 1.0, + ), + "fraction" => { + let default = opt("default", 0.0); + ( + format!( + "ParamDescriptor::fraction({:?}, {:?}, {})", + id, + label, + rust_f32(default) + ), + default, + 0.0, + 1.0, + ) + } + "scalar" => { + let (min, max) = (need("min")?, need("max")?); + let default = opt("default", 0.0); + let unit = match spec.get("unit").and_then(Value::as_str).unwrap_or("none") { + "none" => "Unit::None", + "stops" => "Unit::Stops", + "kelvin" => "Unit::Kelvin", + "percent" => "Unit::Percent", + other => { + return Err(format!( + "`{ctx}.unit` is `{other}`; expected none, stops, kelvin or percent" + )) + } + }; + let scale = match spec.get("scale").and_then(Value::as_str).unwrap_or("linear") { + "linear" => "Scale::Linear", + "perceptual" => "Scale::Perceptual", + other => { + return Err(format!( + "`{ctx}.scale` is `{other}`; expected linear or perceptual" + )) + } + }; + let precision = spec + .get("precision") + .and_then(Value::as_u64) + .ok_or_else(|| format!("`{ctx}` is `kind: scalar` and needs `precision:`"))?; + ( + format!( + "ParamDescriptor::scalar({:?}, {:?}, {}, {}, {}, {}, {}, {})", + id, + label, + rust_f32(min), + rust_f32(max), + rust_f32(default), + unit, + scale, + precision + ), + default, + min, + max, + ) + } + other => { + return Err(format!( + "`{ctx}.kind` is `{other}`; expected stops, amount, switch, fraction or scalar" + )) + } + }) + .map(|(ctor, default, min, max): (String, f64, f64, f64)| { + // Caught here rather than at runtime: a default outside its own range + // makes the control open somewhere it cannot be dragged back to. + (ctor, default, min, max) + }) + .and_then(|(ctor, default, min, max)| { + if min >= max { + return Err(format!("`{ctx}` has an empty range: min {min} >= max {max}")); + } + if default < min || default > max { + return Err(format!( + "`{ctx}` has default {default} outside its range {min}..{max}" + )); + } + Ok((ctor, default, min, max)) + }) +} + +fn read_local_helpers(root: &Mapping) -> Result, String> { + let Some(define) = root.get("define") else { + return Ok(Vec::new()); + }; + let define = as_mapping(define, "define")?; + + let mut out = Vec::new(); + for (name, spec) in define { + let name = as_str(name, "a helper name under `define`")?.to_string(); + let ctx = format!("define.{name}"); + // Shorthand: the value may be the WGSL directly, or a mapping with + // prose beside it. Most node-local helpers carry their explanation in + // the WGSL itself, so the shorthand is the common case. + let (doc, wgsl) = match spec { + Value::String(s) => (None, s.trim_end().to_string()), + other => { + let m = as_mapping(other, &ctx)?; + let wgsl = as_str( + m.get("wgsl") + .ok_or_else(|| format!("`{ctx}` has no `wgsl:`"))?, + &format!("{ctx}.wgsl"), + )? + .trim_end() + .to_string(); + (opt_prose(m, "doc", &ctx)?, wgsl) + } + }; + if !wgsl.contains(&format!("fn {name}(")) { + return Err(format!("`{ctx}` does not define `fn {name}(`")); + } + out.push(HelperDef { name, doc, wgsl }); + } + Ok(out) +} + +fn read_shared_refs( + root: &Mapping, + shared: &BTreeSet<&str>, + local: &[HelperDef], +) -> Result, String> { + let Some(list) = root.get("helpers") else { + return Ok(Vec::new()); + }; + let list = list + .as_sequence() + .ok_or("`helpers` must be a list of helper names")?; + + let mut out = Vec::new(); + let mut seen = BTreeSet::new(); + for item in list { + let name = as_str(item, "a helper name")?.to_string(); + if !shared.contains(name.as_str()) { + let known: Vec<&str> = shared.iter().copied().collect(); + return Err(format!( + "`helpers` names `{name}`, which {HELPERS_FILE} does not \ + define. Known helpers: {}", + known.join(", ") + )); + } + if local.iter().any(|h| h.name == name) { + return Err(format!( + "`{name}` is both requested from {HELPERS_FILE} and redefined \ + under `define`. The composer deduplicates by name, so one of \ + the two definitions would silently win." + )); + } + if !seen.insert(name.clone()) { + return Err(format!("`helpers` lists `{name}` twice")); + } + out.push(name); + } + Ok(out) +} + +fn read_uniforms(root: &Mapping, params: &BTreeSet<&str>) -> Result, String> { + let uniforms = root + .get("uniforms") + .ok_or("missing `uniforms:`; the fragment has nothing to read otherwise")?; + let uniforms = as_mapping(uniforms, "uniforms")?; + + let mut out = Vec::new(); + for (name, spec) in uniforms { + let name = as_str(name, "a uniform name")?.to_string(); + check_ident(&name, &format!("uniforms.{name}"))?; + let ctx = format!("uniforms.{name}"); + + // Shorthand: `gain: exp2(exposure)`, or a mapping carrying prose. + let (doc, expr) = match spec { + Value::String(s) => (None, s.clone()), + other => { + let m = as_mapping(other, &ctx)?; + let value = m + .get("value") + .ok_or_else(|| format!("`{ctx}` has neither a bare expression nor `value:`"))?; + ( + opt_prose(m, "doc", &ctx)?, + as_str(value, &format!("{ctx}.value"))?.to_string(), + ) + } + }; + + let rust = compile_expr(&expr, params).map_err(|e| format!("`{ctx}`: {e}"))?; + out.push(UniformDef { name, doc, rust }); + } + + if out.is_empty() { + return Err("`uniforms:` is empty".into()); + } + Ok(out) +} + +fn read_tests( + root: &Mapping, + params: &[ParamDef], + uniforms: &BTreeSet<&str>, + helpers: &BTreeSet<&str>, +) -> Result, String> { + let Some(list) = root.get("tests") else { + return Ok(Vec::new()); + }; + let list = list.as_sequence().ok_or("`tests` must be a list")?; + + let mut out = Vec::new(); + let mut seen = BTreeSet::new(); + for item in list { + let m = as_mapping(item, "a test")?; + let name = as_str(m.get("name").ok_or("a test has no `name:`")?, "tests[].name")? + .to_string(); + check_ident(&name, &format!("tests.{name}.name"))?; + if !seen.insert(name.clone()) { + return Err(format!("two tests are both called `{name}`")); + } + let ctx = format!("tests.{name}"); + + let mut set = Vec::new(); + if let Some(v) = m.get("set") { + for (k, val) in as_mapping(v, &format!("{ctx}.set"))? { + let key = as_str(k, &format!("{ctx}.set key"))?.to_string(); + let Some(param) = params.iter().find(|p| p.id == key) else { + return Err(format!("`{ctx}.set` names `{key}`, which is not a parameter")); + }; + let value = val + .as_f64() + .ok_or_else(|| format!("`{ctx}.set.{key}` must be a number"))?; + // Values reach `set_param` already clamped by the graph, so a + // test setting an out-of-range value would be asserting + // against something that cannot happen. + if value < param.min || value > param.max { + return Err(format!( + "`{ctx}.set.{key}` is {value}, outside the parameter's \ + range {}..{}. The graph clamps before an operation \ + sees a value, so this test could never run as written.", + param.min, param.max + )); + } + set.push((key, value)); + } + } + + let mut expect = Vec::new(); + if let Some(v) = m.get("expect") { + for (k, val) in as_mapping(v, &format!("{ctx}.expect"))? { + let key = as_str(k, &format!("{ctx}.expect key"))?.to_string(); + if !uniforms.contains(key.as_str()) { + return Err(format!( + "`{ctx}.expect` names `{key}`, which is not a uniform of this node" + )); + } + let value = val + .as_f64() + .ok_or_else(|| format!("`{ctx}.expect.{key}` must be a number"))?; + expect.push((key, value)); + } + } + + let mut expect_range = Vec::new(); + if let Some(v) = m.get("expect_range") { + for (k, val) in as_mapping(v, &format!("{ctx}.expect_range"))? { + let key = as_str(k, &format!("{ctx}.expect_range key"))?.to_string(); + if !uniforms.contains(key.as_str()) { + return Err(format!( + "`{ctx}.expect_range` names `{key}`, which is not a uniform" + )); + } + let pair = val + .as_sequence() + .filter(|s| s.len() == 2) + .ok_or_else(|| format!("`{ctx}.expect_range.{key}` must be [low, high]"))?; + let lo = pair[0] + .as_f64() + .ok_or_else(|| format!("`{ctx}.expect_range.{key}` low must be a number"))?; + let hi = pair[1] + .as_f64() + .ok_or_else(|| format!("`{ctx}.expect_range.{key}` high must be a number"))?; + expect_range.push((key, lo, hi)); + } + } + + let mut expect_wgsl = Vec::new(); + if let Some(v) = m.get("expect_wgsl") { + for item in v + .as_sequence() + .ok_or_else(|| format!("`{ctx}.expect_wgsl` must be a list of strings"))? + { + expect_wgsl.push(as_str(item, &format!("{ctx}.expect_wgsl[]"))?.to_string()); + } + } + + let mut expect_helper_wgsl = Vec::new(); + if let Some(v) = m.get("expect_helper_wgsl") { + for (k, val) in as_mapping(v, &format!("{ctx}.expect_helper_wgsl"))? { + let key = as_str(k, &format!("{ctx}.expect_helper_wgsl key"))?.to_string(); + if !helpers.contains(key.as_str()) { + return Err(format!( + "`{ctx}.expect_helper_wgsl` names `{key}`, which this node does not use" + )); + } + let mut needles = Vec::new(); + for item in val + .as_sequence() + .ok_or_else(|| format!("`{ctx}.expect_helper_wgsl.{key}` must be a list"))? + { + needles.push(as_str(item, &format!("{ctx}.expect_helper_wgsl.{key}[]"))?.to_string()); + } + expect_helper_wgsl.push((key, needles)); + } + } + + let expect_active = match m.get("expect_active") { + None => None, + Some(v) => Some( + v.as_bool() + .ok_or_else(|| format!("`{ctx}.expect_active` must be true or false"))?, + ), + }; + + if expect.is_empty() + && expect_range.is_empty() + && expect_wgsl.is_empty() + && expect_helper_wgsl.is_empty() + && expect_active.is_none() + { + return Err(format!("`{ctx}` asserts nothing")); + } + + out.push(TestDef { + name, + why: opt_prose(m, "why", &ctx)?, + set, + expect, + expect_range, + expect_active, + expect_wgsl, + expect_helper_wgsl, + }); + } + Ok(out) +} + +// --------------------------------------------------------------------------- +// The expression language +// --------------------------------------------------------------------------- +// +// Uniforms are derived from parameters — `exp2(exposure)`, `blacks / 100 * +// 0.02` — and that derivation is the one piece of a node that is genuinely +// computation rather than description. It is kept to arithmetic over the +// node's own parameters and a fixed set of maths functions: enough for every +// operation in the chain, and small enough that a reader of the YAML can see +// exactly what will happen. +// +// Compiled to Rust rather than interpreted, so an unknown name or a wrong +// arity is a build error naming the file, and the arithmetic itself costs +// nothing at runtime. + +#[derive(Debug)] +enum Expr { + Num(f64), + Param(String), + Neg(Box), + Bin(char, Box, Box), + Call(String, Vec), +} + +/// Compile a declared expression to a Rust `f32` expression. +fn compile_expr(src: &str, params: &BTreeSet<&str>) -> Result { + let tokens = tokenise(src)?; + let mut parser = Parser { tokens, at: 0 }; + let expr = parser.expr()?; + if parser.at < parser.tokens.len() { + return Err(format!( + "unexpected `{}` after the end of the expression", + parser.tokens[parser.at] + )); + } + render(&expr, params) +} + +#[derive(Debug, Clone, PartialEq)] +enum Tok { + Num(f64), + Ident(String), + Sym(char), +} + +impl std::fmt::Display for Tok { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Tok::Num(n) => write!(f, "{n}"), + Tok::Ident(s) => write!(f, "{s}"), + Tok::Sym(c) => write!(f, "{c}"), + } + } +} + +fn tokenise(src: &str) -> Result, String> { + let bytes: Vec = src.chars().collect(); + let mut out = Vec::new(); + let mut i = 0; + while i < bytes.len() { + let c = bytes[i]; + if c.is_whitespace() { + i += 1; + } else if c.is_ascii_digit() || (c == '.' && bytes.get(i + 1).is_some_and(char::is_ascii_digit)) { + let start = i; + while i < bytes.len() && (bytes[i].is_ascii_digit() || bytes[i] == '.') { + i += 1; + } + let text: String = bytes[start..i].iter().collect(); + let n = text + .parse::() + .map_err(|_| format!("`{text}` is not a number"))?; + out.push(Tok::Num(n)); + } else if c.is_ascii_alphabetic() || c == '_' { + let start = i; + while i < bytes.len() && (bytes[i].is_ascii_alphanumeric() || bytes[i] == '_') { + i += 1; + } + out.push(Tok::Ident(bytes[start..i].iter().collect())); + } else if "+-*/(),".contains(c) { + out.push(Tok::Sym(c)); + i += 1; + } else { + return Err(format!( + "`{c}` is not valid in an expression; the language is \ + arithmetic (+ - * /), parentheses, numbers, this node's \ + parameters, and the maths functions" + )); + } + } + if out.is_empty() { + return Err("the expression is empty".into()); + } + Ok(out) +} + +struct Parser { + tokens: Vec, + at: usize, +} + +impl Parser { + fn peek(&self) -> Option<&Tok> { + self.tokens.get(self.at) + } + + fn eat(&mut self, sym: char) -> bool { + if self.peek() == Some(&Tok::Sym(sym)) { + self.at += 1; + return true; + } + false + } + + fn expr(&mut self) -> Result { + let mut left = self.term()?; + loop { + if self.eat('+') { + left = Expr::Bin('+', Box::new(left), Box::new(self.term()?)); + } else if self.eat('-') { + left = Expr::Bin('-', Box::new(left), Box::new(self.term()?)); + } else { + return Ok(left); + } + } + } + + fn term(&mut self) -> Result { + let mut left = self.unary()?; + loop { + if self.eat('*') { + left = Expr::Bin('*', Box::new(left), Box::new(self.unary()?)); + } else if self.eat('/') { + left = Expr::Bin('/', Box::new(left), Box::new(self.unary()?)); + } else { + return Ok(left); + } + } + } + + fn unary(&mut self) -> Result { + if self.eat('-') { + return Ok(Expr::Neg(Box::new(self.unary()?))); + } + self.primary() + } + + fn primary(&mut self) -> Result { + match self.peek().cloned() { + Some(Tok::Num(n)) => { + self.at += 1; + Ok(Expr::Num(n)) + } + Some(Tok::Ident(name)) => { + self.at += 1; + if !self.eat('(') { + return Ok(Expr::Param(name)); + } + let mut args = Vec::new(); + if !self.eat(')') { + loop { + args.push(self.expr()?); + if self.eat(',') { + continue; + } + if self.eat(')') { + break; + } + return Err(format!("expected `,` or `)` in the call to `{name}`")); + } + } + Ok(Expr::Call(name, args)) + } + Some(Tok::Sym('(')) => { + self.at += 1; + let inner = self.expr()?; + if !self.eat(')') { + return Err("unclosed `(`".into()); + } + Ok(inner) + } + Some(t) => Err(format!("unexpected `{t}`")), + None => Err("the expression ends early".into()), + } + } +} + +/// The maths functions a node may call, and the Rust each becomes. +/// +/// A closed list rather than a passthrough to `f32`: a node is a description, +/// and letting it name arbitrary Rust would make the YAML a second, worse +/// place to write code. +fn render(expr: &Expr, params: &BTreeSet<&str>) -> Result { + Ok(match expr { + Expr::Num(n) => rust_f32(*n), + Expr::Param(name) => { + if !params.contains(name.as_str()) { + let known: Vec<&str> = params.iter().copied().collect(); + return Err(format!( + "`{name}` is not a parameter of this node. Its parameters \ + are: {}", + known.join(", ") + )); + } + format!("self.{name}") + } + Expr::Neg(inner) => format!("-({})", render(inner, params)?), + Expr::Bin(op, l, r) => format!("({} {op} {})", render(l, params)?, render(r, params)?), + Expr::Call(name, args) => { + let rendered: Vec = args + .iter() + .map(|a| render(a, params)) + .collect::>()?; + let arity = |n: usize| -> Result<(), String> { + if rendered.len() != n { + return Err(format!( + "`{name}` takes {n} argument(s), given {}", + rendered.len() + )); + } + Ok(()) + }; + match name.as_str() { + "exp2" | "log2" | "exp" | "sqrt" | "abs" | "floor" | "ceil" | "round" => { + arity(1)?; + format!("f32::{name}({})", rendered[0]) + } + "pow" => { + arity(2)?; + format!("f32::powf({}, {})", rendered[0], rendered[1]) + } + "min" | "max" => { + arity(2)?; + format!("f32::{name}({}, {})", rendered[0], rendered[1]) + } + "clamp" => { + arity(3)?; + format!( + "f32::clamp({}, {}, {})", + rendered[0], rendered[1], rendered[2] + ) + } + "mix" => { + arity(3)?; + // Spelled out rather than called: Rust has no `mix`, and + // the linear form is what WGSL's `mix` means. + format!( + "({a} + ({b} - {a}) * {t})", + a = rendered[0], + b = rendered[1], + t = rendered[2] + ) + } + other => { + return Err(format!( + "`{other}` is not one of the maths functions a node may \ + call. Available: exp2, log2, exp, sqrt, abs, floor, \ + ceil, round, pow, min, max, clamp, mix" + )) + } + } + } + }) +} + +// --------------------------------------------------------------------------- +// Emission +// --------------------------------------------------------------------------- + +fn emit(shared: &SharedHelpers, nodes: &[Node]) -> Result { + let mut out = String::new(); + out.push_str( + "// GENERATED FILE — DO NOT EDIT.\n\ + //\n\ + // Written by core/dr-pipeline/build.rs from core/dr-pipeline/ops/*.yaml.\n\ + // Edits here are discarded the next time a declaration changes.\n\ + // Change the node, not this.\n\n", + ); + + emit_helpers(&mut out, shared); + for node in nodes { + if let Node::Declared { .. } = node { + emit_node(&mut out, node)?; + } + } + emit_chain(&mut out, nodes); + Ok(out) +} + +fn emit_helpers(out: &mut String, shared: &SharedHelpers) { + out.push_str("pub mod helpers {\n"); + if let Some(doc) = &shared.doc { + out.push_str(&comment(doc, "//!", 4)); + out.push_str(" //!\n"); + } + out.push_str(" //! Generated from `ops/_helpers.yaml`.\n\n"); + out.push_str(" use crate::operation::Helper;\n"); + + for h in &shared.helpers { + out.push('\n'); + if let Some(doc) = &h.doc { + out.push_str(&comment(doc, "///", 4)); + } + let _ = writeln!( + out, + " pub const {}: Helper = Helper {{\n name: {:?},\n source: {},\n }};", + h.name.to_uppercase(), + h.name, + wgsl_literal(&h.wgsl, h.doc.as_deref()) + ); + } + + // A registry, so a test can assert over every helper rather than a list + // someone has to remember to extend. + let names: Vec = shared + .helpers + .iter() + .map(|h| h.name.to_uppercase()) + .collect(); + let _ = writeln!( + out, + "\n /// Every helper `_helpers.yaml` declares, for registry-wide tests.\n\ + \x20 pub static ALL: &[Helper] = &[{}];\n}}\n", + names.join(", ") + ); +} + +fn emit_node(out: &mut String, node: &Node) -> Result<(), String> { + let Node::Declared { + id, + label, + doc, + params, + uniforms, + shared_helpers, + local_helpers, + wgsl, + active, + tests, + .. + } = node + else { + return Ok(()); + }; + + let ty = pascal_case(id); + + let _ = writeln!(out, "pub mod {id} {{"); + if let Some(doc) = doc { + out.push_str(&comment(doc, "//!", 4)); + out.push_str(" //!\n"); + } + let _ = writeln!(out, " //! Generated from `ops/{id}.yaml`.\n"); + + out.push_str( + " use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId};\n\ + \x20 #[allow(unused_imports)]\n\ + \x20 use crate::descriptor::{Scale, Unit};\n\ + \x20 use crate::operation::{Helper, Operation, Uniform};\n\n", + ); + + // Ids as consts, so a caller names a parameter through the type system + // rather than by retyping a string. + let _ = writeln!(out, " pub const ID: OpId = OpId({id:?});"); + for p in params { + let _ = writeln!( + out, + " pub const {}: ParamId = ParamId({:?});", + p.id.to_uppercase(), + p.id + ); + } + + // Descriptor. + let _ = writeln!( + out, + "\n static DESCRIPTOR: OpDescriptor = OpDescriptor {{\n\ + \x20 id: ID,\n\ + \x20 label: LocalizedKey({label:?}),\n\ + \x20 params: &[" + ); + for p in params { + if let Some(doc) = &p.doc { + out.push_str(&comment(doc, "//", 12)); + } + let _ = writeln!(out, " {},", p.ctor); + } + out.push_str(" ],\n };\n"); + + // Helpers: node-local definitions first, then the assembled list. + for h in local_helpers { + let _ = writeln!( + out, + "\n const {}: Helper = Helper {{\n name: {:?},\n source: {},\n }};", + local_helper_const(&h.name), + h.name, + wgsl_literal(&h.wgsl, h.doc.as_deref()) + ); + } + + let mut helper_refs: Vec = shared_helpers + .iter() + .map(|h| format!("super::helpers::{}", h.to_uppercase())) + .collect(); + helper_refs.extend(local_helpers.iter().map(|h| local_helper_const(&h.name))); + if !helper_refs.is_empty() { + let _ = writeln!( + out, + "\n static HELPERS: &[Helper] = &[\n {},\n ];", + helper_refs.join(",\n ") + ); + } + + // State. + let _ = writeln!(out, "\n #[derive(Debug, Default, Clone)]\n pub struct {ty} {{"); + for p in params { + let _ = writeln!(out, " {}: f32,", p.id); + } + out.push_str(" }\n"); + let _ = writeln!( + out, + "\n impl {ty} {{\n pub fn new() -> Self {{\n Self::default()\n }}\n }}" + ); + + // The trait. + let _ = writeln!(out, "\n impl Operation for {ty} {{"); + out.push_str( + " fn descriptor(&self) -> &'static OpDescriptor {\n &DESCRIPTOR\n }\n\n", + ); + + out.push_str(" fn set_param(&mut self, id: ParamId, value: f32) {\n match id {\n"); + for p in params { + let _ = writeln!( + out, + " {} => self.{} = value,", + p.id.to_uppercase(), + p.id + ); + } + let _ = writeln!( + out, + " _ => log::warn!(\"{id}: unknown parameter {{id}}\"),\n }}\n }}\n" + ); + + out.push_str(" fn param(&self, id: ParamId) -> f32 {\n match id {\n"); + for p in params { + let _ = writeln!( + out, + " {} => self.{},", + p.id.to_uppercase(), + p.id + ); + } + out.push_str(" _ => 0.0,\n }\n }\n\n"); + + // `is_active` decides whether this node reaches the shader at all, so the + // default rule is the honest one: the operation is doing something exactly + // when a parameter has moved off its default. + let active_expr = match active { + Some(expr) => format!("({expr}) != 0.0"), + None => params + .iter() + .map(|p| format!("self.{} != {}", p.id, rust_f32(p.default))) + .collect::>() + .join(" || "), + }; + let _ = writeln!( + out, + " fn is_active(&self) -> bool {{\n {active_expr}\n }}\n" + ); + + let _ = writeln!( + out, + " fn wgsl_body(&self) -> String {{\n {}.into()\n }}\n", + wgsl_literal(wgsl, None) + ); + + out.push_str(" fn uniforms(&self) -> Vec {\n vec![\n"); + for u in uniforms { + if let Some(doc) = &u.doc { + out.push_str(&comment(doc, "//", 16)); + } + let _ = writeln!( + out, + " Uniform {{ name: {:?}, value: {} }},", + u.name, u.rust + ); + } + out.push_str(" ]\n }\n"); + + if !helper_refs.is_empty() { + out.push_str( + "\n fn helpers(&self) -> &'static [Helper] {\n HELPERS\n }\n", + ); + } + out.push_str(" }\n"); + + emit_tests(out, &ty, params, tests); + out.push_str("}\n\n"); + Ok(()) +} + +fn emit_tests(out: &mut String, ty: &str, params: &[ParamDef], tests: &[TestDef]) { + if tests.is_empty() { + return; + } + out.push_str("\n #[cfg(test)]\n mod tests {\n use super::*;\n\n"); + out.push_str( + " /// One uniform's value, by the name the node declared it under.\n\ + \x20 fn uniform(op: &impl Operation, name: &str) -> f32 {\n\ + \x20 op.uniforms()\n\ + \x20 .into_iter()\n\ + \x20 .find(|u| u.name == name)\n\ + \x20 .unwrap_or_else(|| panic!(\"no uniform called {name}\"))\n\ + \x20 .value\n\ + \x20 }\n", + ); + + for t in tests { + out.push('\n'); + let _ = writeln!(out, " #[test]"); + let _ = writeln!(out, " fn {}() {{", t.name); + if let Some(why) = &t.why { + out.push_str(&comment(why, "//", 12)); + } + let _ = writeln!(out, " let mut op = {ty}::new();"); + // `mut` is unused when a test asserts about the neutral state, and a + // warning in generated code is noise nobody can fix at the source. + if t.set.is_empty() { + out.push_str(" let op = &mut op;\n let op = &*op;\n"); + } + for (param, value) in &t.set { + let id = params + .iter() + .find(|p| &p.id == param) + .map(|p| p.id.to_uppercase()) + .unwrap_or_default(); + let _ = writeln!(out, " op.set_param({id}, {});", rust_f32(*value)); + } + + for (name, expected) in &t.expect { + let _ = writeln!( + out, + " let got = uniform(&op, {name:?});\n\ + \x20 assert!(\n\ + \x20 (got - {e}).abs() <= 1e-5,\n\ + \x20 \"{name}: expected {{}}, got {{got}}\",\n\ + \x20 {e}\n\ + \x20 );", + e = rust_f32(*expected) + ); + } + for (name, lo, hi) in &t.expect_range { + let _ = writeln!( + out, + " let got = uniform(&op, {name:?});\n\ + \x20 assert!(\n\ + \x20 ({lo}..={hi}).contains(&got),\n\ + \x20 \"{name}: {{got}} is outside {lo}..={hi}\"\n\ + \x20 );", + lo = rust_f32(*lo), + hi = rust_f32(*hi) + ); + } + if let Some(active) = t.expect_active { + let _ = writeln!( + out, + " assert_{}!(op.is_active());", + if active { "" } else { "!true == false; assert" } + ); + } + for needle in &t.expect_wgsl { + let _ = writeln!( + out, + " assert!(\n\ + \x20 op.wgsl_body().contains({needle:?}),\n\ + \x20 \"the fragment no longer contains {{:?}}\",\n\ + \x20 {needle:?}\n\ + \x20 );" + ); + } + for (helper, needles) in &t.expect_helper_wgsl { + let _ = writeln!( + out, + " let helper = op\n\ + \x20 .helpers()\n\ + \x20 .iter()\n\ + \x20 .find(|h| h.name == {helper:?})\n\ + \x20 .expect(\"declares {helper}\");" + ); + for needle in needles { + let _ = writeln!( + out, + " assert!(\n\ + \x20 helper.source.contains({needle:?}),\n\ + \x20 \"{helper} no longer contains {{:?}}\",\n\ + \x20 {needle:?}\n\ + \x20 );" + ); + } + } + out.push_str(" }\n"); + } + out.push_str(" }\n"); +} + +fn emit_chain(out: &mut String, nodes: &[Node]) { + out.push_str( + "/// TRACES: FR-DEV-3a | FR-DEV-3c\n\ + /// The default develop chain, in the order `ops/*.yaml` declares.\n\ + ///\n\ + /// Order is data, not code (ARCH §3.4): each node carries an `order:`\n\ + /// and this is the sorted result, so reordering the pipeline is an edit\n\ + /// to one number in one declaration.\n\ + pub fn chain() -> Vec> {\n\ + \x20 vec![\n", + ); + for node in nodes { + let _ = writeln!(out, " // ---- {} (order {})", node.id(), node.order()); + if let Some(placement) = node.placement() { + out.push_str(&comment(placement, "//", 8)); + } + match node { + Node::Declared { id, .. } => { + let _ = writeln!(out, " Box::new({id}::{}::new()),", pascal_case(id)); + } + Node::Rust { ty, why_rust, .. } => { + if let Some(why) = why_rust { + out.push_str(" // Hand-written:\n"); + out.push_str(&comment(why, "//", 8)); + } + let _ = writeln!(out, " Box::new({ty}::new()),"); + } + } + } + out.push_str(" ]\n}\n\n"); + + // The ids, in order, as data — so a test can assert that what the chain + // actually builds matches what the declarations say. This is the only + // check available on a `rust:` node, whose descriptor comes from a type + // this build script cannot read. + let ids: Vec = nodes.iter().map(|n| format!("{:?}", n.id())).collect(); + let _ = writeln!( + out, + "/// The node ids `ops/*.yaml` declares, in chain order.\n\ + pub static DECLARED_IDS: &[&str] = &[{}];\n", + ids.join(", ") + ); +} + +// --------------------------------------------------------------------------- +// Rendering helpers +// --------------------------------------------------------------------------- + +/// A WGSL block as a Rust raw string literal. +/// +/// Raw so the WGSL reads as itself in the generated file — escaped quotes and +/// `\n` would make the one thing a reader comes to generated source to check +/// unreadable. The hash count grows if the source contains a `"` sequence. +/// +/// **Emitted at column zero, deliberately.** The composer indents each line of +/// a fragment as it places it into the generated shader, so a literal indented +/// here to look tidy in `nodes.rs` would arrive in the WGSL indented twice. +/// The hand-written operations had the same shape for the same reason. +fn wgsl_literal(wgsl: &str, doc: Option<&str>) -> String { + let mut body = String::new(); + // Prose from the declaration becomes a WGSL comment above the function, + // which is where it is useful — the generated shader is what gets read + // when a compile fails. + if let Some(doc) = doc { + for line in doc.trim_end().lines() { + if line.trim().is_empty() { + body.push_str("//\n"); + } else { + let _ = writeln!(body, "// {line}"); + } + } + } + body.push_str(wgsl.trim_matches('\n').trim_end()); + + let mut hashes = String::new(); + while body.contains(&format!("\"{hashes}")) { + hashes.push('#'); + } + format!("r{hashes}\"{body}\"{hashes}") +} + +/// A f64 from YAML as a Rust `f32` literal. +/// +/// Always with a decimal point: `100f32` parses, but `100` in a position +/// expecting `f32` does not, and the generated arithmetic mixes the two +/// freely. +fn rust_f32(n: f64) -> String { + let s = format!("{n:?}"); + if s.contains('.') || s.contains('e') || s.contains("inf") || s.contains("NaN") { + format!("{s}f32") + } else { + format!("{s}.0f32") + } +} + +fn local_helper_const(name: &str) -> String { + format!("{}_HELPER", name.to_uppercase()) +} + +fn pascal_case(id: &str) -> String { + id.split('_') + .filter(|s| !s.is_empty()) + .map(|word| { + let mut chars = word.chars(); + match chars.next() { + Some(first) => first.to_ascii_uppercase().to_string() + chars.as_str(), + None => String::new(), + } + }) + .collect() +} + +/// Wrap prose as comments at a given indent, keeping the author's line breaks. +fn comment(text: &str, marker: &str, indent: usize) -> String { + let pad = " ".repeat(indent); + let mut out = String::new(); + for line in text.trim_end().lines() { + if line.trim().is_empty() { + let _ = writeln!(out, "{pad}{marker}"); + } else { + let _ = writeln!(out, "{pad}{marker} {line}"); + } + } + out +} + +// --------------------------------------------------------------------------- +// YAML access +// --------------------------------------------------------------------------- + +fn read_yaml(path: &Path) -> Result { + let text = std::fs::read_to_string(path) + .map_err(|e| format!("cannot read {}: {e}", path.display()))?; + serde_norway::from_str(&text).map_err(|e| format!("{}: not valid YAML: {e}", path.display())) +} + +fn as_mapping<'a>(value: &'a Value, ctx: &str) -> Result<&'a Mapping, String> { + value + .as_mapping() + .ok_or_else(|| format!("`{ctx}` must be a mapping")) +} + +fn as_str<'a>(value: &'a Value, ctx: &str) -> Result<&'a str, String> { + value + .as_str() + .ok_or_else(|| format!("`{ctx}` must be a string")) +} + +fn opt_prose(map: &Mapping, key: &str, ctx: &str) -> Result, String> { + match map.get(key) { + None => Ok(None), + Some(v) => v + .as_str() + .map(|s| Some(s.trim_end().to_string())) + .ok_or_else(|| format!("`{ctx}.{key}` must be a string")), + } +} + +/// Names reaching generated Rust have to be identifiers, and must not be +/// keywords — `ParamId("type")` would generate a struct field called `type`. +fn check_ident(name: &str, ctx: &str) -> Result<(), String> { + if name.is_empty() { + return Err(format!("`{ctx}` is empty")); + } + let head_ok = name + .chars() + .next() + .is_some_and(|c| c.is_ascii_lowercase() || c == '_'); + let rest_ok = name + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_'); + if !head_ok || !rest_ok { + return Err(format!( + "`{ctx}` is `{name}`; names must be lower_snake_case so they can \ + become Rust identifiers" + )); + } + const KEYWORDS: &[&str] = &[ + "as", "break", "const", "continue", "crate", "dyn", "else", "enum", "extern", "false", + "fn", "for", "if", "impl", "in", "let", "loop", "match", "mod", "move", "mut", "pub", + "ref", "return", "self", "static", "struct", "super", "trait", "true", "type", "unsafe", + "use", "where", "while", "async", "await", "box", "final", "macro", "override", "priv", + "try", "typeof", "unsized", "virtual", "yield", + ]; + if KEYWORDS.contains(&name) { + return Err(format!("`{ctx}` is `{name}`, which is a Rust keyword")); + } + Ok(()) +} diff --git a/core/dr-pipeline/ops/_helpers.yaml b/core/dr-pipeline/ops/_helpers.yaml new file mode 100644 index 0000000..7dcaf07 --- /dev/null +++ b/core/dr-pipeline/ops/_helpers.yaml @@ -0,0 +1,73 @@ +# WGSL helper functions shared between nodes. +# +# Files here beginning with `_` are not nodes; this one declares the helper +# library every node may draw on by name. `build.rs` generates +# `ops::helpers::` from each entry, and validates that a node's +# `helpers:` list names something defined here — a typo is a build error +# naming the key, not a WGSL compile failure in generated source. +# +# **Single source of truth.** The composer deduplicates helpers by *name*, so +# two definitions of one name would silently emit whichever came first and two +# nodes would compute, say, luminance differently depending on graph order. +# That is a genuinely hard bug to see, which is why a helper is defined +# exactly once, here, and referenced everywhere else. + +doc: | + WGSL helper functions shared between operations. + + Generated from `ops/_helpers.yaml`. A node names the helpers it needs and + the composer emits each one once, however many nodes asked for it. + +helpers: + luminance: + doc: | + Rec. 709 luminance, the weighting that matches sRGB primaries. + + Applied to camera-space values it is an approximation — the true weights + depend on the camera matrix — but using it here keeps the tonal operations + working on sensor-native data, where highlight headroom still exists. + wgsl: | + fn luminance(c: vec3) -> f32 { + return dot(c, vec3(0.2126, 0.7152, 0.0722)); + } + + tone_position: + doc: | + Map linear luminance onto a perceptual 0..1 position. + + Tonal controls must feel evenly spaced to the eye, and linear light is + not: middle grey sits at 0.18, so a linear weight would call almost + everything a shadow. The cube root approximates lightness cheaply and + behaves well near zero, where a log would diverge. + wgsl: | + fn tone_position(luma: f32) -> f32 { + return clamp(pow(max(luma, 0.0), 1.0 / 3.0), 0.0, 1.0); + } + + apply_tone_gain: + doc: | + Scale a colour by a gain while preserving its hue. + + Multiplying the three channels equally keeps chromaticity fixed, so + lifting shadows does not desaturate them the way an additive lift would. + wgsl: | + fn apply_tone_gain(c: vec3, gain: f32) -> vec3 { + return c * gain; + } + + colour_saturation: + doc: | + How far a colour sits from grey, in 0..1. + + The max-minus-min definition (HSV chroma) rather than a standard + deviation: it matches what the eye reads as 'colourfulness' and it is what + makes vibrance's roll-off land where users expect. + wgsl: | + fn colour_saturation(c: vec3) -> f32 { + let hi = max(c.r, max(c.g, c.b)); + let lo = min(c.r, min(c.g, c.b)); + if (hi <= 0.0) { + return 0.0; + } + return (hi - lo) / hi; + } diff --git a/core/dr-pipeline/ops/blacks_whites.yaml b/core/dr-pipeline/ops/blacks_whites.yaml new file mode 100644 index 0000000..c0d7978 --- /dev/null +++ b/core/dr-pipeline/ops/blacks_whites.yaml @@ -0,0 +1,86 @@ +id: blacks_whites +label: op.blacks_whites +order: 50 + +doc: | + Blacks and whites — the endpoints, where the image clips. + + Narrow where [`highlights_shadows`](highlights_shadows.yaml) is broad: + whites act only in the top quarter, blacks only in the bottom quarter. That + difference in reach is the whole distinction between the two pairs. + +placement: | + After the broad recovery controls: the endpoints are placed once the tonal + range they bound has been settled. + +params: + blacks: + label: param.blacks + kind: amount + whites: + label: param.whites + kind: amount + +uniforms: + white_amount: whites / 100 + black_amount: + value: blacks / 100 * 0.02 + doc: | + A small linear offset. Scene-referred black sits near zero, so the + useful range here is far smaller than a stop — 0.02 is already a + visible lift on a dark frame. + +helpers: [luminance, tone_position, apply_tone_gain] + +wgsl: | + let luma = luminance(c); + let pos = tone_position(luma); + + // Narrow weights concentrated at each end — this is what separates these + // controls from highlights/shadows, which are broad. Whites act only in the + // top quarter, blacks only in the bottom quarter. + let white_w = smoothstep(0.75, 1.0, pos); + let black_w = 1.0 - smoothstep(0.0, 0.25, pos); + + // Whites scale the top end multiplicatively, moving the clipping point. + let white_gain = exp2(white_amount * white_w); + c = apply_tone_gain(c, white_gain); + + // Blacks shift the floor. This one is deliberately *additive*: the point of + // a blacks control is to set where the image reaches zero, and a multiply + // can never bring a non-zero value to zero nor lift a true black off it. + c = c + vec3(black_amount * black_w); + + // The subtractive direction can push below zero, which is not light. + c = max(c, vec3(0.0)); + +tests: + - name: it_starts_neutral + expect_active: false + + - name: one_parameter_is_enough_to_activate + set: { whites: 20 } + expect_active: true + + - name: the_blacks_offset_stays_small + why: | + Scene-referred black is near zero; a full-stop control here would be + unusable, moving the image to grey at a fraction of its travel. + set: { blacks: 100 } + expect_range: { black_amount: [0.0, 0.05] } + + - name: blacks_are_additive_not_multiplicative + why: | + A multiply can never bring a non-zero value to zero nor lift a true + black off it, which is precisely what this control is for. + expect_wgsl: ["c = c + vec3(black_amount * black_w);"] + + - name: the_result_cannot_go_below_zero + why: A negative radiance is not light, and it poisons everything downstream. + expect_wgsl: ["c = max(c, vec3(0.0));"] + + - name: these_endpoints_are_narrower_than_the_recovery_controls + why: | + The one property distinguishing this node from highlights_shadows. If + these weights widened to match, the two would be the same control twice. + expect_wgsl: ["smoothstep(0.75, 1.0, pos)", "smoothstep(0.0, 0.25, pos)"] diff --git a/core/dr-pipeline/ops/brilliance.yaml b/core/dr-pipeline/ops/brilliance.yaml new file mode 100644 index 0000000..d5477be --- /dev/null +++ b/core/dr-pipeline/ops/brilliance.yaml @@ -0,0 +1,68 @@ +id: brilliance +label: op.brilliance +order: 70 + +doc: | + Brilliance — shadows up and highlights down at once. + + Apple's control, and a genuinely different idea from contrast: it applies + the opposite correction at each end of the range while leaving mid-tones + alone. The result reads as "more light in the scene" rather than "less + contrast", because local relationships survive where a plain contrast + reduction flattens them. + + It overlaps with highlights/shadows deliberately — one control doing both + in a fixed relationship is easier to reach for than two controls needing + to be balanced against each other. + +placement: | + After the tone curve has had the final word on tone, and before the colour + controls, which should act on the tones the photographer has settled. + +params: + brilliance: + label: param.brilliance + kind: amount + +uniforms: + amount: + value: brilliance / 100 * 0.5 + doc: | + Half a stop at each end at full travel — the two ends move apart by a + stop in total, which is a strong but not destructive flattening. + +helpers: [luminance, tone_position, apply_tone_gain] + +wgsl: | + let luma = luminance(c); + let pos = tone_position(luma); + + // Opposite corrections at the two ends: shadows up, highlights down, both + // tapering to nothing at the mid-point. This is what separates brilliance + // from a contrast control — mid-tones keep their local relationships, so + // the image gains apparent light rather than losing structure. + let lift = (1.0 - smoothstep(0.0, 0.5, pos)) * amount; + let pull = smoothstep(0.5, 1.0, pos) * amount; + + // A mild saturation compensation. Flattening the tonal range washes colour + // out; without this, brilliance looks faded at useful settings. + let gain = exp2(lift - pull); + c = c * gain; + + let luma_after = luminance(c); + c = mix(vec3(luma_after), c, 1.0 + max(amount, 0.0) * 0.15); + c = max(c, vec3(0.0)); + +tests: + - name: it_starts_neutral + expect_active: false + + - name: full_travel_is_half_a_stop_at_each_end + set: { brilliance: 100 } + expect: { amount: 0.5 } + + - name: the_two_ends_move_in_opposite_directions + why: | + The property that makes this brilliance rather than a contrast slider. + Both terms scaling the same way would flatten without lifting. + expect_wgsl: ["let gain = exp2(lift - pull);"] diff --git a/core/dr-pipeline/ops/colour_mixer.yaml b/core/dr-pipeline/ops/colour_mixer.yaml new file mode 100644 index 0000000..d70d313 --- /dev/null +++ b/core/dr-pipeline/ops/colour_mixer.yaml @@ -0,0 +1,14 @@ +id: colour_mixer +order: 100 +rust: ColourMixer + +why_rust: | + Thirty-six parameters — twelve hue bands times hue, saturation and + luminance — generated as a grid, each carrying a `Facet` naming its band and + the band's centre on the hue wheel. Spelling that out as thirty-six YAML + entries would be a worse description than the loop that produces it, and the + band centres are computed rather than listed. + +placement: | + Last. It is the finishing control, and it should act on the tones the user + has already settled. diff --git a/core/dr-pipeline/ops/contrast.yaml b/core/dr-pipeline/ops/contrast.yaml new file mode 100644 index 0000000..2d4a610 --- /dev/null +++ b/core/dr-pipeline/ops/contrast.yaml @@ -0,0 +1,110 @@ +id: contrast +label: op.contrast +order: 30 + +doc: | + Contrast — an S-curve about a fixed mid-point. + + Pushes tones away from middle grey (positive) or toward it (negative), + pivoting where the eye reads "neither light nor dark". In linear light + that point is 0.18, not 0.5: a scene-referred value of 0.5 is roughly a + stop and a half above middle grey, and pivoting there would darken almost + every photograph. + + The curve is applied in a perceptual domain rather than directly to linear + values. Applied linearly, an S-curve crushes shadows far harder than it + lifts highlights, because linear light devotes most of its range to the + brightest stop. + +placement: | + After the capture corrections, before the region controls: it is the broad + tonal statement those controls then refine. + +params: + contrast: + label: param.contrast + kind: amount + +uniforms: + amount: + value: contrast / 100 + doc: The shader's curve expects -1..1; the descriptor speaks -100..100. + +helpers: [luminance, apply_tone_gain] + +define: + contrast_curve: | + // A symmetric S-curve on a 0..1 perceptual position. + // + // `amount` above zero steepens, below zero flattens. The smoothstep form is + // used for the steepening direction because it has zero gradient at both + // ends, so the curve cannot invert however hard it is pushed — the failure + // that makes naive gain-about-a-pivot unusable past moderate settings. + fn contrast_curve(x: f32, amount: f32) -> f32 { + let clamped = clamp(x, 0.0, 1.0); + if (amount >= 0.0) { + // Blend toward a smoothstep, which is the S. + let s = clamped * clamped * (3.0 - 2.0 * clamped); + return mix(clamped, s, amount); + } + // Flattening: pull toward the mid-point. At amount = -1 every tone + // collapses to 0.5, which is the meaningful limit of 'no contrast'. + return mix(clamped, 0.5, -amount); + } + +wgsl: | + let luma = luminance(c); + if (luma > 0.0001) { + // Work on luminance and rescale the colour by the ratio, rather than + // curving each channel independently. Per-channel contrast shifts hue + // wherever the channels differ — the classic symptom being skies going + // cyan as contrast rises. + // + // MIDDLE_GREY is 0.18: the linear value the eye reads as mid-tone. The + // curve operates on luma/(2*0.18) so that middle grey lands at the + // curve's own 0.5 pivot. + let pos = clamp(luma / 0.36, 0.0, 1.0); + let curved = contrast_curve(pos, amount); + // Not `target`: that is a WGSL reserved keyword, and using it produces a + // parse error in generated code rather than anywhere a reader would look. + let curved_luma = curved * 0.36; + c = apply_tone_gain(c, curved_luma / luma); + } + c = max(c, vec3(0.0)); + +tests: + - name: neutral_does_nothing + expect: { amount: 0.0 } + expect_active: false + + - name: the_amount_is_normalised_to_unit_range + set: { contrast: 100 } + expect: { amount: 1.0 } + + - name: the_negative_direction_normalises_too + set: { contrast: -100 } + expect: { amount: -1.0 } + + - name: contrast_works_on_luminance_not_per_channel + why: | + Curving each channel separately shifts hue; the ratio form is what keeps + a blue sky blue as contrast rises. + expect_wgsl: ["luminance(c)", "apply_tone_gain"] + + - name: the_pivot_is_middle_grey_not_half + why: | + Pivoting at 0.5 in linear light would darken nearly every image: + scene-referred 0.5 is well above what the eye calls mid-tone. + expect_wgsl: ["0.36"] + + - name: a_division_by_luminance_is_guarded + why: | + A black pixel has zero luminance; dividing by it would produce NaN and + propagate through everything downstream. + expect_wgsl: ["luma > 0.0001"] + + - name: the_curve_cannot_invert + why: | + A gain-about-a-pivot form produces a non-monotonic curve past moderate + settings, which inverts tones. smoothstep cannot. + expect_helper_wgsl: { contrast_curve: ["3.0 - 2.0 * clamped"] } diff --git a/core/dr-pipeline/ops/exposure.yaml b/core/dr-pipeline/ops/exposure.yaml new file mode 100644 index 0000000..0ef0849 --- /dev/null +++ b/core/dr-pipeline/ops/exposure.yaml @@ -0,0 +1,57 @@ +# TRACES: FR-DEV-3a | FR-DEV-3c +id: exposure +label: op.exposure +order: 20 + +doc: | + Exposure — a linear gain, expressed in stops. + + The simplest operation in the pipeline and the one that most justifies + working in linear light: a stop is a doubling, so exposure is a single + multiply. Applied to gamma-encoded data it would be neither a doubling nor + reversible, which is why this stage sits where it does (ARCH §5.2). + +placement: | + A correction to how the scene was captured, so it precedes every operation + that interprets tone. + +params: + exposure: + label: param.exposure + kind: stops + min: -5 + max: 5 + doc: | + ±5 stops. Wider than most edits need, but recovering a badly + underexposed frame is a real use and raw data often supports it. + +uniforms: + gain: + value: exp2(exposure) + doc: The linear gain for the current setting. + +wgsl: | + c = c * gain; + +tests: + - name: one_stop_is_a_doubling + why: | + The definition of a stop. If this is wrong, every exposure adjustment is + subtly off and no test of "looks right" would catch it. + set: { exposure: 1 } + expect: { gain: 2.0 } + + - name: one_stop_down_is_a_halving + set: { exposure: -1 } + expect: { gain: 0.5 } + + - name: stops_compose_additively + why: +2 stops must equal +1 applied twice. + set: { exposure: 2 } + expect: { gain: 4.0 } + + - name: neutral_is_unity_gain + why: | + An unedited image must be the image, not an interpretation of it. + expect: { gain: 1.0 } + expect_active: false diff --git a/core/dr-pipeline/ops/highlights_shadows.yaml b/core/dr-pipeline/ops/highlights_shadows.yaml new file mode 100644 index 0000000..c5a69f4 --- /dev/null +++ b/core/dr-pipeline/ops/highlights_shadows.yaml @@ -0,0 +1,81 @@ +id: highlights_shadows +label: op.highlights_shadows +order: 40 + +doc: | + Highlights and shadows — broad, overlapping recovery at both ends. + + Weights are built from smoothstep rather than a hard threshold: a sharp + boundary produces visible banding on a gradient — a sky is the worst case, + and it is also the most common subject for these controls. + + Broad where [`blacks_whites`](blacks_whites.yaml) is narrow. These are the + controls used to tame a contrasty scene; those are the ones used to place + the endpoints. + +placement: | + After the broad tonal statement contrast makes, so these refine it. + +params: + highlights: + label: param.highlights + kind: amount + doc: | + Negative recovers highlights, the overwhelmingly common direction, + matching the convention every other developer uses. + shadows: + label: param.shadows + kind: amount + +uniforms: + hi_amount: + value: highlights / 100 + doc: | + A full stop at the extreme; enough to recover a bright sky without + inverting the tonal relationship. + lo_amount: shadows / 100 + +helpers: [luminance, tone_position, apply_tone_gain] + +wgsl: | + let luma = luminance(c); + let pos = tone_position(luma); + + // Broad, overlapping weights. Highlights ramp in over the upper half, + // shadows out over the lower half, so a mid-tone is barely touched by + // either and the two controls blend rather than fighting at the join. + let hi_w = smoothstep(0.5, 1.0, pos); + let lo_w = 1.0 - smoothstep(0.0, 0.5, pos); + + // Each control contributes up to a stop of gain at full deflection. + // exp2 keeps the effect symmetric: -100 and +100 are inverse. + let hi_gain = exp2(hi_amount * hi_w); + let lo_gain = exp2(lo_amount * lo_w); + + c = apply_tone_gain(c, hi_gain * lo_gain); + +tests: + - name: it_starts_neutral + expect_active: false + + - name: one_parameter_is_enough_to_activate + set: { highlights: -50 } + expect_active: true + + - name: highlight_recovery_is_the_negative_direction + why: The convention users expect — dragging left recovers. Full travel is one stop. + set: { highlights: -100 } + expect: { hi_amount: -1.0 } + + - name: tonal_amounts_are_symmetric + why: | + exp2 of equal and opposite exponents multiplies to 1, so +100 and -100 + have to be exact negations for the two directions to cancel. + set: { shadows: 100 } + expect: { lo_amount: 1.0 } + + - name: the_weights_are_smooth_not_thresholded + why: | + A hard boundary bands visibly on a gradient, and a sky is both the worst + case and the most common subject for this control. + expect_wgsl: ["smoothstep(0.5, 1.0, pos)", "smoothstep(0.0, 0.5, pos)"] diff --git a/core/dr-pipeline/ops/saturation.yaml b/core/dr-pipeline/ops/saturation.yaml new file mode 100644 index 0000000..340e9ad --- /dev/null +++ b/core/dr-pipeline/ops/saturation.yaml @@ -0,0 +1,60 @@ +id: saturation +label: op.saturation +order: 90 + +doc: | + Saturation — every colour's distance from grey, scaled equally. + + The blunt instrument beside [`vibrance`](vibrance.yaml). Both are offered + because they fail differently: this one is predictable and even, which is + what a landscape wants, and ruinous on faces, which is what vibrance is for. + +placement: | + Last of the per-colour controls, so it has the final word if both it and + vibrance are in play. + +params: + saturation: + label: param.saturation + kind: amount + +uniforms: + factor: + value: max(1 + saturation / 100, 0) + doc: | + -100 reaches exactly monochrome; +100 doubles the distance from grey. + The floor at zero matters: a negative factor would push a colour past + grey into its complement, inverting hues. + +helpers: [luminance, tone_position, apply_tone_gain] + +wgsl: | + // Interpolate away from the luminance-preserving grey. A factor of 0 is + // monochrome, 1 is unchanged, above 1 is more saturated. + let luma = luminance(c); + c = mix(vec3(luma), c, factor); + c = max(c, vec3(0.0)); + +tests: + - name: it_starts_unchanged + expect: { factor: 1.0 } + expect_active: false + + - name: full_negative_saturation_reaches_monochrome + why: | + The property that makes -100 meaningful: it must land exactly on grey, + not merely near it. + set: { saturation: -100 } + expect: { factor: 0.0 } + + - name: positive_saturation_increases_the_factor + set: { saturation: 100 } + expect: { factor: 2.0 } + + - name: the_factor_never_goes_negative + why: | + A negative factor pushes a colour past grey into its complement, which + inverts hues rather than desaturating them. The clamp is what makes the + bottom of the slider's travel monochrome instead of a solarised image. + set: { saturation: -100 } + expect_range: { factor: [0.0, 2.0] } diff --git a/core/dr-pipeline/ops/tone_curve.yaml b/core/dr-pipeline/ops/tone_curve.yaml new file mode 100644 index 0000000..b773749 --- /dev/null +++ b/core/dr-pipeline/ops/tone_curve.yaml @@ -0,0 +1,23 @@ +# A hand-written node. `rust:` names the type in `crate::ops` that implements +# `Operation`; everything else about it — its descriptor, its parameters, its +# WGSL — comes from that type rather than from this file. +# +# It appears here anyway so that `ops/` lists the whole pipeline in order. +# A chain half-declared here and half-ordered in Rust would be worse than +# either alone: the order is the one thing a reader comes to this directory +# to learn. +id: tone_curve +order: 60 +rust: ToneCurve + +why_rust: | + Five control points presented as one curve widget, with an interpolator and + a monotonicity guarantee behind it. Its neutral is a *relationship* between + parameters rather than a set of values — the identity diagonal — which is + not something the declarative `active:` rule can express, and its + `presentation()` spans parameters rather than describing one. + +placement: | + After the fixed-weight region controls, so the curve is the final word on + tone: a photographer reaches for it to fix what those controls could not + place exactly. diff --git a/core/dr-pipeline/ops/vibrance.yaml b/core/dr-pipeline/ops/vibrance.yaml new file mode 100644 index 0000000..f253a85 --- /dev/null +++ b/core/dr-pipeline/ops/vibrance.yaml @@ -0,0 +1,65 @@ +id: vibrance +label: op.vibrance +order: 80 + +doc: | + Vibrance — saturation weighted toward the muted colours. + + Where [`saturation`](saturation.yaml) scales every colour's distance from + grey equally, vibrance scales it *more for muted colours than for already + saturated ones*, and protects skin tones. The difference matters: pushing + saturation on a portrait turns faces orange long before the background + improves, which is precisely the problem vibrance was invented to solve. + +placement: | + Before saturation, so the broad control has the last word if both are used. + +params: + vibrance: + label: param.vibrance + kind: amount + +uniforms: + amount: vibrance / 100 + +helpers: [luminance, tone_position, colour_saturation] + +wgsl: | + let luma = luminance(c); + let sat = colour_saturation(c); + + // The vibrance curve: full effect on grey, tapering to nothing on colours + // that are already saturated. Squaring the falloff keeps the mid-range + // responsive while still protecting the extremes. + let falloff = (1.0 - sat) * (1.0 - sat); + + // Skin protection. Skin sits in a narrow band of hue where red leads green + // leads blue; pushing it is what makes vibrance look wrong on portraits. + // Detected by channel ordering rather than a hue angle, which costs a + // conversion and buys nothing here. + let is_skin = f32(c.r > c.g && c.g > c.b); + let skin_guard = 1.0 - is_skin * 0.5; + + let strength = amount * falloff * skin_guard; + c = mix(vec3(luma), c, 1.0 + strength); + c = max(c, vec3(0.0)); + +tests: + - name: it_starts_neutral + expect_active: false + + - name: the_amount_is_normalised_to_unit_range + set: { vibrance: 100 } + expect: { amount: 1.0 } + + - name: muted_colours_get_more_than_saturated_ones + why: | + The one property that distinguishes vibrance from saturation. Without + the falloff term this node would be a duplicate of its neighbour. + expect_wgsl: ["let falloff = (1.0 - sat) * (1.0 - sat);"] + + - name: skin_tones_are_protected + why: | + The reason vibrance exists. A portrait pushed on plain saturation goes + orange long before the background improves. + expect_wgsl: ["let skin_guard = 1.0 - is_skin * 0.5;"] diff --git a/core/dr-pipeline/ops/white_balance.yaml b/core/dr-pipeline/ops/white_balance.yaml new file mode 100644 index 0000000..086d10f --- /dev/null +++ b/core/dr-pipeline/ops/white_balance.yaml @@ -0,0 +1,86 @@ +id: white_balance +label: op.white_balance +order: 10 + +doc: | + White balance — temperature and tint, relative to as-shot. + + Expressed as an offset from what the camera chose rather than an absolute + kelvin value. Neutral means "as shot", so the control starts where the + image already is and a reset returns there. An absolute scale would make + the neutral position depend on the file, which is exactly the confusion + Lightroom's temperature slider creates on non-raw files. + + The as-shot multipliers themselves are applied by the composer's preamble + rather than here — every image has them even when this operation is + neutral, so they cannot live in a fragment that vanishes at neutral. + +placement: | + First. It is a correction to how the scene was captured, and every tonal + operation after it should act on a correctly balanced image. + +params: + temperature: + label: param.temperature + kind: amount + doc: | + Warmer is positive, matching every other raw developer: dragging right + makes the image warmer, even though that means *lowering* the colour + temperature being corrected for. + tint: + label: param.tint + kind: amount + +# Temperature trades red against blue; tint trades green against magenta. +# Both are scaled so the full range is a strong but not destructive +# correction: ±0.5 in log2 at the extremes — half a stop of channel shift, +# which covers ordinary illuminant error without letting the slider blow a +# channel on its own. +uniforms: + mul_r: exp2(temperature / 100 * 0.5) + mul_g: + value: exp2(-(tint / 100 * 0.5)) + doc: | + Green is held at unity by temperature, so the control does not double as + an exposure slider — green carries most of the luminance. + mul_b: + value: exp2(-(temperature / 100 * 0.5)) + doc: Blue moves opposite red, so a neutral grey stays grey as the control moves. + +wgsl: | + c = c * vec3(mul_r, mul_g, mul_b); + +tests: + - name: neutral_is_as_shot + expect: { mul_r: 1.0, mul_g: 1.0, mul_b: 1.0 } + expect_active: false + + - name: warming_raises_red_and_lowers_blue + set: { temperature: 100 } + expect: { mul_r: 1.4142135, mul_b: 0.70710677 } + + - name: temperature_leaves_green_alone + why: | + Otherwise the control doubles as an exposure slider, because green + carries most of the luminance. + set: { temperature: 100 } + expect: { mul_g: 1.0 } + + - name: tint_moves_green_against_magenta + why: Positive tint reduces green, and must not touch red or blue. + set: { tint: 100 } + expect: { mul_g: 0.70710677, mul_r: 1.0, mul_b: 1.0 } + + - name: cooling_is_the_inverse_of_warming + why: | + Warming by n then cooling by n must return to neutral, so the two + directions have to be exact reciprocals rather than merely similar. + set: { temperature: -100 } + expect: { mul_r: 0.70710677, mul_b: 1.4142135 } + + - name: the_extremes_stay_within_half_a_stop + why: | + A white balance control that can blow a channel by itself is a trap; + correction belongs in a range where highlights survive. + set: { temperature: 100, tint: 100 } + expect_range: { mul_r: [0.70, 1.42], mul_g: [0.70, 1.42], mul_b: [0.70, 1.42] } diff --git a/docs/traceability.md b/docs/traceability.md index dea155d..61729f7 100644 --- a/docs/traceability.md +++ b/docs/traceability.md @@ -9,17 +9,17 @@ Denominators are parsed from [`requirements.md`](requirements.md) at run time, n | Metric | Value | |---|---| -| Source files scanned | 93 | -| TRACES tags found | 141 | +| Source files scanned | 96 | +| TRACES tags found | 144 | | Requirements defined | 151 | -| Requirements covered | 72 | -| **Coverage** | **47.7%** (72/151) | +| Requirements covered | 73 | +| **Coverage** | **48.3%** (73/151) | ### By type | Type | Covered | Defined | |---|---|---| -| FR | 56 | 97 | +| FR | 57 | 97 | | NFR | 14 | 48 | | R | 2 | 6 | @@ -33,10 +33,10 @@ _None._ | ID | Tagged in | |---|---| -| FR-CAT-1 | [`core/dr-catalog/src/scan.rs:1`](../core/dr-catalog/src/scan.rs#L1), [`core/dr-sync/src/scan.rs:93`](../core/dr-sync/src/scan.rs#L93), [`core/dr-types/src/lib.rs:185`](../core/dr-types/src/lib.rs#L185), [`tools/traceability/src/lib.rs:473`](../tools/traceability/src/lib.rs#L473), [`tools/traceability/src/lib.rs:505`](../tools/traceability/src/lib.rs#L505), [`ui/dr-ui/src/library.rs:1`](../ui/dr-ui/src/library.rs#L1) | +| FR-CAT-1 | [`core/dr-catalog/src/scan.rs:1`](../core/dr-catalog/src/scan.rs#L1), [`core/dr-sync/src/scan.rs:93`](../core/dr-sync/src/scan.rs#L93), [`core/dr-types/src/lib.rs:185`](../core/dr-types/src/lib.rs#L185), [`tools/traceability/src/lib.rs:473`](../tools/traceability/src/lib.rs#L473), [`tools/traceability/src/lib.rs:505`](../tools/traceability/src/lib.rs#L505), [`ui/dr-ui/src/activity.rs:1`](../ui/dr-ui/src/activity.rs#L1), [`ui/dr-ui/src/library.rs:1`](../ui/dr-ui/src/library.rs#L1) | | FR-CAT-11 | [`ui/dr-ui/src/library.rs:150`](../ui/dr-ui/src/library.rs#L150) | | FR-CAT-12 | [`core/dr-pipeline/src/sidecar.rs:108`](../core/dr-pipeline/src/sidecar.rs#L108) | -| FR-CAT-15 | [`core/dr-catalog/src/schema.rs:258`](../core/dr-catalog/src/schema.rs#L258), [`core/dr-catalog/src/trash.rs:1`](../core/dr-catalog/src/trash.rs#L1), [`core/dr-sync-nextcloud/src/lib.rs:366`](../core/dr-sync-nextcloud/src/lib.rs#L366), [`core/dr-sync/src/lib.rs:122`](../core/dr-sync/src/lib.rs#L122), [`core/dr-sync/src/scan.rs:426`](../core/dr-sync/src/scan.rs#L426), [`core/dr-sync/src/scan.rs:57`](../core/dr-sync/src/scan.rs#L57), [`core/dr-thumbs/src/lib.rs:341`](../core/dr-thumbs/src/lib.rs#L341), [`ui/dr-ui/src/collections_ui.rs:1191`](../ui/dr-ui/src/collections_ui.rs#L1191), [`ui/dr-ui/src/collections_ui.rs:730`](../ui/dr-ui/src/collections_ui.rs#L730), [`ui/dr-ui/src/library.rs:150`](../ui/dr-ui/src/library.rs#L150), [`ui/dr-ui/src/library.rs:167`](../ui/dr-ui/src/library.rs#L167), [`ui/dr-ui/src/library.rs:2020`](../ui/dr-ui/src/library.rs#L2020), [`ui/dr-ui/src/library.rs:2052`](../ui/dr-ui/src/library.rs#L2052), [`ui/dr-ui/src/library_ui.rs:112`](../ui/dr-ui/src/library_ui.rs#L112), [`ui/dr-ui/src/library_ui.rs:436`](../ui/dr-ui/src/library_ui.rs#L436), [`ui/dr-ui/src/trash.rs:1`](../ui/dr-ui/src/trash.rs#L1), [`ui/dr-ui/ui/collections.slint:458`](../ui/dr-ui/ui/collections.slint#L458) | +| FR-CAT-15 | [`core/dr-catalog/src/schema.rs:258`](../core/dr-catalog/src/schema.rs#L258), [`core/dr-catalog/src/trash.rs:1`](../core/dr-catalog/src/trash.rs#L1), [`core/dr-sync-nextcloud/src/lib.rs:366`](../core/dr-sync-nextcloud/src/lib.rs#L366), [`core/dr-sync/src/lib.rs:122`](../core/dr-sync/src/lib.rs#L122), [`core/dr-sync/src/scan.rs:426`](../core/dr-sync/src/scan.rs#L426), [`core/dr-sync/src/scan.rs:57`](../core/dr-sync/src/scan.rs#L57), [`core/dr-thumbs/src/lib.rs:341`](../core/dr-thumbs/src/lib.rs#L341), [`ui/dr-ui/src/collections_ui.rs:1299`](../ui/dr-ui/src/collections_ui.rs#L1299), [`ui/dr-ui/src/collections_ui.rs:817`](../ui/dr-ui/src/collections_ui.rs#L817), [`ui/dr-ui/src/library.rs:150`](../ui/dr-ui/src/library.rs#L150), [`ui/dr-ui/src/library.rs:167`](../ui/dr-ui/src/library.rs#L167), [`ui/dr-ui/src/library.rs:2020`](../ui/dr-ui/src/library.rs#L2020), [`ui/dr-ui/src/library.rs:2052`](../ui/dr-ui/src/library.rs#L2052), [`ui/dr-ui/src/library_ui.rs:112`](../ui/dr-ui/src/library_ui.rs#L112), [`ui/dr-ui/src/library_ui.rs:444`](../ui/dr-ui/src/library_ui.rs#L444), [`ui/dr-ui/src/trash.rs:1`](../ui/dr-ui/src/trash.rs#L1), [`ui/dr-ui/ui/collections.slint:458`](../ui/dr-ui/ui/collections.slint#L458) | | FR-CAT-1a | [`core/dr-types/src/lib.rs:47`](../core/dr-types/src/lib.rs#L47) | | FR-CAT-2 | [`core/dr-catalog/src/lib.rs:1`](../core/dr-catalog/src/lib.rs#L1), [`core/dr-catalog/src/schema.rs:1`](../core/dr-catalog/src/schema.rs#L1), [`tools/traceability/src/lib.rs:473`](../tools/traceability/src/lib.rs#L473) | | FR-CAT-3 | [`core/dr-catalog/src/jobs.rs:1`](../core/dr-catalog/src/jobs.rs#L1), [`core/dr-sync/src/scan.rs:69`](../core/dr-sync/src/scan.rs#L69), [`core/dr-thumbs/src/codec.rs:1`](../core/dr-thumbs/src/codec.rs#L1), [`core/dr-thumbs/src/lib.rs:1`](../core/dr-thumbs/src/lib.rs#L1), [`ui/dr-ui/src/derived_sync.rs:1`](../ui/dr-ui/src/derived_sync.rs#L1) | @@ -45,19 +45,19 @@ _None._ | FR-CAT-6 | [`core/dr-catalog/src/collections.rs:1`](../core/dr-catalog/src/collections.rs#L1), [`core/dr-catalog/src/lib.rs:1`](../core/dr-catalog/src/lib.rs#L1), [`core/dr-catalog/src/query.rs:1`](../core/dr-catalog/src/query.rs#L1), [`core/dr-catalog/src/rating.rs:1`](../core/dr-catalog/src/rating.rs#L1), [`core/dr-types/src/selector.rs:1`](../core/dr-types/src/selector.rs#L1), [`ui/dr-ui/src/library.rs:177`](../ui/dr-ui/src/library.rs#L177) | | FR-CAT-7 | [`core/dr-catalog/src/collections.rs:1`](../core/dr-catalog/src/collections.rs#L1), [`core/dr-catalog/src/merge.rs:1`](../core/dr-catalog/src/merge.rs#L1), [`core/dr-catalog/src/sync.rs:1`](../core/dr-catalog/src/sync.rs#L1), [`core/dr-types/src/selector.rs:1`](../core/dr-types/src/selector.rs#L1), [`ui/dr-ui/src/collections_ui.rs:1`](../ui/dr-ui/src/collections_ui.rs#L1), [`ui/dr-ui/src/derived_sync.rs:1`](../ui/dr-ui/src/derived_sync.rs#L1), [`ui/dr-ui/ui/collections.slint:4`](../ui/dr-ui/ui/collections.slint#L4) | | FR-CAT-8 | [`core/dr-pipeline/src/sidecar.rs:89`](../core/dr-pipeline/src/sidecar.rs#L89), [`ui/dr-ui/src/library.rs:289`](../ui/dr-ui/src/library.rs#L289) | -| FR-CAT-9 | [`core/dr-catalog/src/cache.rs:1`](../core/dr-catalog/src/cache.rs#L1), [`core/dr-catalog/src/scan.rs:1`](../core/dr-catalog/src/scan.rs#L1), [`core/dr-catalog/src/schema.rs:230`](../core/dr-catalog/src/schema.rs#L230), [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-sync/src/reachability.rs:1`](../core/dr-sync/src/reachability.rs#L1), [`core/dr-types/src/lib.rs:104`](../core/dr-types/src/lib.rs#L104), [`ui/dr-ui/src/library.rs:136`](../ui/dr-ui/src/library.rs#L136), [`ui/dr-ui/src/library.rs:194`](../ui/dr-ui/src/library.rs#L194), [`ui/dr-ui/src/library.rs:2140`](../ui/dr-ui/src/library.rs#L2140), [`ui/dr-ui/src/library.rs:994`](../ui/dr-ui/src/library.rs#L994), [`ui/dr-ui/src/library_ui.rs:1240`](../ui/dr-ui/src/library_ui.rs#L1240), [`ui/dr-ui/src/library_ui.rs:1391`](../ui/dr-ui/src/library_ui.rs#L1391), [`ui/dr-ui/src/library_ui.rs:165`](../ui/dr-ui/src/library_ui.rs#L165), [`ui/dr-ui/src/library_ui.rs:1682`](../ui/dr-ui/src/library_ui.rs#L1682), [`ui/dr-ui/src/library_ui.rs:1756`](../ui/dr-ui/src/library_ui.rs#L1756), [`ui/dr-ui/src/library_ui.rs:1870`](../ui/dr-ui/src/library_ui.rs#L1870), [`ui/dr-ui/src/library_ui.rs:2717`](../ui/dr-ui/src/library_ui.rs#L2717), [`ui/dr-ui/src/library_ui.rs:2745`](../ui/dr-ui/src/library_ui.rs#L2745), [`ui/dr-ui/src/library_ui.rs:283`](../ui/dr-ui/src/library_ui.rs#L283), [`ui/dr-ui/src/library_ui.rs:936`](../ui/dr-ui/src/library_ui.rs#L936), [`ui/dr-ui/src/library_ui.rs:978`](../ui/dr-ui/src/library_ui.rs#L978) | +| FR-CAT-9 | [`core/dr-catalog/src/cache.rs:1`](../core/dr-catalog/src/cache.rs#L1), [`core/dr-catalog/src/scan.rs:1`](../core/dr-catalog/src/scan.rs#L1), [`core/dr-catalog/src/schema.rs:230`](../core/dr-catalog/src/schema.rs#L230), [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-sync/src/reachability.rs:1`](../core/dr-sync/src/reachability.rs#L1), [`core/dr-types/src/lib.rs:104`](../core/dr-types/src/lib.rs#L104), [`ui/dr-ui/src/library.rs:136`](../ui/dr-ui/src/library.rs#L136), [`ui/dr-ui/src/library.rs:194`](../ui/dr-ui/src/library.rs#L194), [`ui/dr-ui/src/library.rs:2140`](../ui/dr-ui/src/library.rs#L2140), [`ui/dr-ui/src/library.rs:994`](../ui/dr-ui/src/library.rs#L994), [`ui/dr-ui/src/library_ui.rs:1024`](../ui/dr-ui/src/library_ui.rs#L1024), [`ui/dr-ui/src/library_ui.rs:1286`](../ui/dr-ui/src/library_ui.rs#L1286), [`ui/dr-ui/src/library_ui.rs:1437`](../ui/dr-ui/src/library_ui.rs#L1437), [`ui/dr-ui/src/library_ui.rs:165`](../ui/dr-ui/src/library_ui.rs#L165), [`ui/dr-ui/src/library_ui.rs:1756`](../ui/dr-ui/src/library_ui.rs#L1756), [`ui/dr-ui/src/library_ui.rs:1828`](../ui/dr-ui/src/library_ui.rs#L1828), [`ui/dr-ui/src/library_ui.rs:1954`](../ui/dr-ui/src/library_ui.rs#L1954), [`ui/dr-ui/src/library_ui.rs:291`](../ui/dr-ui/src/library_ui.rs#L291), [`ui/dr-ui/src/library_ui.rs:2953`](../ui/dr-ui/src/library_ui.rs#L2953), [`ui/dr-ui/src/library_ui.rs:2981`](../ui/dr-ui/src/library_ui.rs#L2981), [`ui/dr-ui/src/library_ui.rs:982`](../ui/dr-ui/src/library_ui.rs#L982) | | FR-CULL-1 | [`core/dr-decode/src/preview.rs:134`](../core/dr-decode/src/preview.rs#L134) | | FR-CULL-2 | [`core/dr-decode/src/locate.rs:1`](../core/dr-decode/src/locate.rs#L1), [`core/dr-decode/src/preview.rs:161`](../core/dr-decode/src/preview.rs#L161) | | FR-CULL-4 | [`core/dr-catalog/src/rating.rs:1`](../core/dr-catalog/src/rating.rs#L1), [`core/dr-pipeline/src/sidecar.rs:125`](../core/dr-pipeline/src/sidecar.rs#L125), [`ui/dr-ui/src/library.rs:177`](../ui/dr-ui/src/library.rs#L177), [`ui/dr-ui/src/library.rs:289`](../ui/dr-ui/src/library.rs#L289) | | FR-DEV-3 | [`core/dr-pipeline/src/framing.rs:174`](../core/dr-pipeline/src/framing.rs#L174) | -| FR-DEV-3a | [`core/dr-pipeline/src/descriptor.rs:91`](../core/dr-pipeline/src/descriptor.rs#L91), [`core/dr-pipeline/src/graph.rs:138`](../core/dr-pipeline/src/graph.rs#L138), [`core/dr-pipeline/src/graph.rs:15`](../core/dr-pipeline/src/graph.rs#L15), [`core/dr-pipeline/src/graph.rs:39`](../core/dr-pipeline/src/graph.rs#L39), [`core/dr-pipeline/src/operation.rs:104`](../core/dr-pipeline/src/operation.rs#L104) | -| FR-DEV-3b | [`core/dr-pipeline/src/descriptor.rs:91`](../core/dr-pipeline/src/descriptor.rs#L91), [`core/dr-pipeline/src/graph.rs:39`](../core/dr-pipeline/src/graph.rs#L39), [`core/dr-pipeline/src/operation.rs:104`](../core/dr-pipeline/src/operation.rs#L104) | -| FR-DEV-3c | [`core/dr-pipeline/src/graph.rs:138`](../core/dr-pipeline/src/graph.rs#L138) | +| FR-DEV-3a | [`core/dr-pipeline/build.rs:1482`](../core/dr-pipeline/build.rs#L1482), [`core/dr-pipeline/src/descriptor.rs:116`](../core/dr-pipeline/src/descriptor.rs#L116), [`core/dr-pipeline/src/descriptor.rs:91`](../core/dr-pipeline/src/descriptor.rs#L91), [`core/dr-pipeline/src/graph.rs:143`](../core/dr-pipeline/src/graph.rs#L143), [`core/dr-pipeline/src/graph.rs:17`](../core/dr-pipeline/src/graph.rs#L17), [`core/dr-pipeline/src/graph.rs:41`](../core/dr-pipeline/src/graph.rs#L41), [`core/dr-pipeline/src/operation.rs:104`](../core/dr-pipeline/src/operation.rs#L104) | +| FR-DEV-3b | [`core/dr-pipeline/src/descriptor.rs:91`](../core/dr-pipeline/src/descriptor.rs#L91), [`core/dr-pipeline/src/graph.rs:41`](../core/dr-pipeline/src/graph.rs#L41), [`core/dr-pipeline/src/operation.rs:104`](../core/dr-pipeline/src/operation.rs#L104) | +| FR-DEV-3c | [`core/dr-pipeline/build.rs:1482`](../core/dr-pipeline/build.rs#L1482), [`core/dr-pipeline/src/graph.rs:143`](../core/dr-pipeline/src/graph.rs#L143) | | FR-DEV-3d | [`core/dr-pipeline/src/framing.rs:174`](../core/dr-pipeline/src/framing.rs#L174) | | FR-DEV-3e | [`core/dr-decode/src/lib.rs:492`](../core/dr-decode/src/lib.rs#L492), [`core/dr-decode/src/lib.rs:612`](../core/dr-decode/src/lib.rs#L612) | | FR-DEV-3h | [`core/dr-decode/src/lib.rs:307`](../core/dr-decode/src/lib.rs#L307), [`core/dr-decode/src/preview.rs:29`](../core/dr-decode/src/preview.rs#L29), [`core/dr-pipeline/src/framing.rs:188`](../core/dr-pipeline/src/framing.rs#L188), [`core/dr-types/src/lib.rs:272`](../core/dr-types/src/lib.rs#L272) | | FR-DEV-4 | [`core/dr-gpu/src/lib.rs:123`](../core/dr-gpu/src/lib.rs#L123) | -| FR-DSP-1 | [`ui/dr-ui/src/lib.rs:44`](../ui/dr-ui/src/lib.rs#L44) | +| FR-DSP-1 | [`ui/dr-ui/src/lib.rs:45`](../ui/dr-ui/src/lib.rs#L45) | | FR-EXP-1 | [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`ui/dr-ui/src/settings_ui.rs:1`](../ui/dr-ui/src/settings_ui.rs#L1) | | FR-EXP-2 | [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`ui/dr-ui/src/settings_ui.rs:1`](../ui/dr-ui/src/settings_ui.rs#L1) | | FR-EXP-3 | [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`ui/dr-ui/src/settings_ui.rs:1`](../ui/dr-ui/src/settings_ui.rs#L1) | @@ -72,8 +72,9 @@ _None._ | FR-NC-3 | [`core/dr-decode/src/locate.rs:1`](../core/dr-decode/src/locate.rs#L1), [`core/dr-decode/src/preview.rs:161`](../core/dr-decode/src/preview.rs#L161), [`core/dr-sync/src/capability.rs:41`](../core/dr-sync/src/capability.rs#L41), [`core/dr-thumbs/src/lib.rs:1`](../core/dr-thumbs/src/lib.rs#L1), [`ui/dr-ui/src/library.rs:1`](../ui/dr-ui/src/library.rs#L1), [`ui/dr-ui/src/library_ui.rs:1`](../ui/dr-ui/src/library_ui.rs#L1) | | FR-NC-4 | [`core/dr-sync-nextcloud/src/propfind.rs:100`](../core/dr-sync-nextcloud/src/propfind.rs#L100), [`core/dr-sync-nextcloud/src/propfind.rs:51`](../core/dr-sync-nextcloud/src/propfind.rs#L51), [`core/dr-sync/src/capability.rs:6`](../core/dr-sync/src/capability.rs#L6), [`core/dr-sync/src/lib.rs:155`](../core/dr-sync/src/lib.rs#L155), [`core/dr-sync/src/scan.rs:93`](../core/dr-sync/src/scan.rs#L93), [`ui/dr-ui/src/launch.rs:49`](../ui/dr-ui/src/launch.rs#L49) | | FR-NC-5 | [`core/dr-sync-nextcloud/src/propfind.rs:51`](../core/dr-sync-nextcloud/src/propfind.rs#L51) | -| FR-NC-6a | [`core/dr-catalog/src/cache.rs:1`](../core/dr-catalog/src/cache.rs#L1), [`core/dr-catalog/src/schema.rs:230`](../core/dr-catalog/src/schema.rs#L230), [`core/dr-catalog/src/schema.rs:631`](../core/dr-catalog/src/schema.rs#L631), [`core/dr-types/src/selector.rs:1`](../core/dr-types/src/selector.rs#L1), [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`ui/dr-ui/src/lib.rs:1323`](../ui/dr-ui/src/lib.rs#L1323), [`ui/dr-ui/src/lib.rs:883`](../ui/dr-ui/src/lib.rs#L883), [`ui/dr-ui/src/library.rs:813`](../ui/dr-ui/src/library.rs#L813), [`ui/dr-ui/src/library.rs:836`](../ui/dr-ui/src/library.rs#L836), [`ui/dr-ui/src/library.rs:994`](../ui/dr-ui/src/library.rs#L994), [`ui/dr-ui/src/library_ui.rs:1041`](../ui/dr-ui/src/library_ui.rs#L1041), [`ui/dr-ui/src/library_ui.rs:173`](../ui/dr-ui/src/library_ui.rs#L173), [`ui/dr-ui/src/library_ui.rs:184`](../ui/dr-ui/src/library_ui.rs#L184), [`ui/dr-ui/src/library_ui.rs:193`](../ui/dr-ui/src/library_ui.rs#L193), [`ui/dr-ui/src/library_ui.rs:242`](../ui/dr-ui/src/library_ui.rs#L242), [`ui/dr-ui/src/library_ui.rs:252`](../ui/dr-ui/src/library_ui.rs#L252), [`ui/dr-ui/src/library_ui.rs:2734`](../ui/dr-ui/src/library_ui.rs#L2734), [`ui/dr-ui/src/library_ui.rs:294`](../ui/dr-ui/src/library_ui.rs#L294), [`ui/dr-ui/src/library_ui.rs:309`](../ui/dr-ui/src/library_ui.rs#L309), [`ui/dr-ui/src/library_ui.rs:340`](../ui/dr-ui/src/library_ui.rs#L340), [`ui/dr-ui/src/library_ui.rs:699`](../ui/dr-ui/src/library_ui.rs#L699), [`ui/dr-ui/src/library_ui.rs:800`](../ui/dr-ui/src/library_ui.rs#L800), [`ui/dr-ui/src/library_ui.rs:903`](../ui/dr-ui/src/library_ui.rs#L903), [`ui/dr-ui/src/settings_store.rs:1`](../ui/dr-ui/src/settings_store.rs#L1), [`ui/dr-ui/src/settings_ui.rs:1`](../ui/dr-ui/src/settings_ui.rs#L1), [`ui/dr-ui/ui/library.slint:799`](../ui/dr-ui/ui/library.slint#L799) | -| FR-NC-6c | [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-types/src/lib.rs:104`](../core/dr-types/src/lib.rs#L104), [`core/dr-types/src/lib.rs:186`](../core/dr-types/src/lib.rs#L186) | +| FR-NC-6 | [`ui/dr-ui/src/activity.rs:1`](../ui/dr-ui/src/activity.rs#L1) | +| FR-NC-6a | [`core/dr-catalog/src/cache.rs:1`](../core/dr-catalog/src/cache.rs#L1), [`core/dr-catalog/src/schema.rs:230`](../core/dr-catalog/src/schema.rs#L230), [`core/dr-catalog/src/schema.rs:631`](../core/dr-catalog/src/schema.rs#L631), [`core/dr-types/src/selector.rs:1`](../core/dr-types/src/selector.rs#L1), [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`ui/dr-ui/src/lib.rs:1347`](../ui/dr-ui/src/lib.rs#L1347), [`ui/dr-ui/src/lib.rs:897`](../ui/dr-ui/src/lib.rs#L897), [`ui/dr-ui/src/library.rs:813`](../ui/dr-ui/src/library.rs#L813), [`ui/dr-ui/src/library.rs:836`](../ui/dr-ui/src/library.rs#L836), [`ui/dr-ui/src/library.rs:994`](../ui/dr-ui/src/library.rs#L994), [`ui/dr-ui/src/library_ui.rs:1087`](../ui/dr-ui/src/library_ui.rs#L1087), [`ui/dr-ui/src/library_ui.rs:173`](../ui/dr-ui/src/library_ui.rs#L173), [`ui/dr-ui/src/library_ui.rs:184`](../ui/dr-ui/src/library_ui.rs#L184), [`ui/dr-ui/src/library_ui.rs:193`](../ui/dr-ui/src/library_ui.rs#L193), [`ui/dr-ui/src/library_ui.rs:250`](../ui/dr-ui/src/library_ui.rs#L250), [`ui/dr-ui/src/library_ui.rs:260`](../ui/dr-ui/src/library_ui.rs#L260), [`ui/dr-ui/src/library_ui.rs:2970`](../ui/dr-ui/src/library_ui.rs#L2970), [`ui/dr-ui/src/library_ui.rs:302`](../ui/dr-ui/src/library_ui.rs#L302), [`ui/dr-ui/src/library_ui.rs:317`](../ui/dr-ui/src/library_ui.rs#L317), [`ui/dr-ui/src/library_ui.rs:348`](../ui/dr-ui/src/library_ui.rs#L348), [`ui/dr-ui/src/library_ui.rs:729`](../ui/dr-ui/src/library_ui.rs#L729), [`ui/dr-ui/src/library_ui.rs:830`](../ui/dr-ui/src/library_ui.rs#L830), [`ui/dr-ui/src/library_ui.rs:949`](../ui/dr-ui/src/library_ui.rs#L949), [`ui/dr-ui/src/settings_store.rs:1`](../ui/dr-ui/src/settings_store.rs#L1), [`ui/dr-ui/src/settings_ui.rs:1`](../ui/dr-ui/src/settings_ui.rs#L1), [`ui/dr-ui/ui/library.slint:771`](../ui/dr-ui/ui/library.slint#L771) | +| FR-NC-6c | [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-types/src/lib.rs:104`](../core/dr-types/src/lib.rs#L104), [`core/dr-types/src/lib.rs:186`](../core/dr-types/src/lib.rs#L186), [`ui/dr-ui/src/activity.rs:1`](../ui/dr-ui/src/activity.rs#L1) | | FR-NC-7 | [`core/dr-sync-nextcloud/src/lib.rs:95`](../core/dr-sync-nextcloud/src/lib.rs#L95), [`ui/dr-ui/src/derived_sync.rs:1`](../ui/dr-ui/src/derived_sync.rs#L1) | | FR-NC-8 | [`core/dr-pipeline/src/sidecar.rs:108`](../core/dr-pipeline/src/sidecar.rs#L108), [`core/dr-pipeline/src/sidecar.rs:89`](../core/dr-pipeline/src/sidecar.rs#L89), [`ui/dr-ui/src/library.rs:289`](../ui/dr-ui/src/library.rs#L289) | | FR-NC-9 | [`core/dr-catalog/src/merge.rs:1`](../core/dr-catalog/src/merge.rs#L1), [`core/dr-catalog/src/sync.rs:1`](../core/dr-catalog/src/sync.rs#L1), [`core/dr-pipeline/src/sidecar.rs:212`](../core/dr-pipeline/src/sidecar.rs#L212) | @@ -84,11 +85,11 @@ _None._ | FR-RAW-3 | [`core/dr-decode/src/lib.rs:409`](../core/dr-decode/src/lib.rs#L409), [`core/dr-decode/src/lib.rs:94`](../core/dr-decode/src/lib.rs#L94) | | FR-RAW-4 | [`core/dr-decode/src/error.rs:1`](../core/dr-decode/src/error.rs#L1) | | FR-RAW-5 | [`core/dr-decode/src/lib.rs:122`](../core/dr-decode/src/lib.rs#L122) | -| FR-UI-1 | [`ui/dr-ui/src/lib.rs:1405`](../ui/dr-ui/src/lib.rs#L1405), [`ui/dr-ui/src/lib.rs:52`](../ui/dr-ui/src/lib.rs#L52) | -| FR-UI-2 | [`ui/dr-ui/src/lib.rs:52`](../ui/dr-ui/src/lib.rs#L52) | +| FR-UI-1 | [`ui/dr-ui/src/lib.rs:1429`](../ui/dr-ui/src/lib.rs#L1429), [`ui/dr-ui/src/lib.rs:53`](../ui/dr-ui/src/lib.rs#L53) | +| FR-UI-2 | [`ui/dr-ui/src/lib.rs:53`](../ui/dr-ui/src/lib.rs#L53) | | FR-UI-3 | [`ui/dr-ui/ui/collections.slint:4`](../ui/dr-ui/ui/collections.slint#L4) | -| FR-UI-4 | [`ui/dr-ui/ui/app.slint:950`](../ui/dr-ui/ui/app.slint#L950) | -| FR-UI-5 | [`ui/dr-ui/src/collections_ui.rs:1`](../ui/dr-ui/src/collections_ui.rs#L1), [`ui/dr-ui/src/lib.rs:1439`](../ui/dr-ui/src/lib.rs#L1439), [`ui/dr-ui/ui/collections.slint:4`](../ui/dr-ui/ui/collections.slint#L4) | +| FR-UI-4 | [`ui/dr-ui/ui/app.slint:984`](../ui/dr-ui/ui/app.slint#L984) | +| FR-UI-5 | [`ui/dr-ui/src/collections_ui.rs:1`](../ui/dr-ui/src/collections_ui.rs#L1), [`ui/dr-ui/src/lib.rs:1463`](../ui/dr-ui/src/lib.rs#L1463), [`ui/dr-ui/ui/collections.slint:4`](../ui/dr-ui/ui/collections.slint#L4) | | NFR-ARCH-2 | [`core/dr-catalog/src/jobs.rs:1`](../core/dr-catalog/src/jobs.rs#L1) | | NFR-ARCH-4 | [`core/dr-catalog/src/error.rs:1`](../core/dr-catalog/src/error.rs#L1), [`core/dr-thumbs/src/error.rs:1`](../core/dr-thumbs/src/error.rs#L1) | | NFR-OPS-1 | [`tools/traceability/src/lib.rs:266`](../tools/traceability/src/lib.rs#L266) | @@ -100,7 +101,7 @@ _None._ | NFR-R5 | [`core/dr-catalog/src/collections.rs:1`](../core/dr-catalog/src/collections.rs#L1), [`core/dr-catalog/src/error.rs:1`](../core/dr-catalog/src/error.rs#L1), [`core/dr-catalog/src/schema.rs:1`](../core/dr-catalog/src/schema.rs#L1) | | NFR-R7 | [`core/dr-gpu/src/error.rs:1`](../core/dr-gpu/src/error.rs#L1) | | NFR-R8 | [`core/dr-gpu/src/error.rs:1`](../core/dr-gpu/src/error.rs#L1) | -| NFR-RES-1 | [`ui/dr-ui/src/lib.rs:44`](../ui/dr-ui/src/lib.rs#L44) | +| NFR-RES-1 | [`ui/dr-ui/src/lib.rs:45`](../ui/dr-ui/src/lib.rs#L45) | | NFR-RES-4 | [`core/dr-catalog/src/cache.rs:1`](../core/dr-catalog/src/cache.rs#L1), [`core/dr-catalog/src/schema.rs:230`](../core/dr-catalog/src/schema.rs#L230), [`core/dr-thumbs/src/codec.rs:1`](../core/dr-thumbs/src/codec.rs#L1), [`core/dr-thumbs/src/lib.rs:1`](../core/dr-thumbs/src/lib.rs#L1), [`core/dr-thumbs/src/lib.rs:341`](../core/dr-thumbs/src/lib.rs#L341) | | NFR-SEC-1 | [`core/dr-decode/src/error.rs:1`](../core/dr-decode/src/error.rs#L1) | | R1 | [`tools/traceability/src/lib.rs:489`](../tools/traceability/src/lib.rs#L489), [`tools/traceability/src/lib.rs:493`](../tools/traceability/src/lib.rs#L493) | @@ -108,7 +109,7 @@ _None._ ## Not yet tagged -79 of 151 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built. +78 of 151 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built.
Show untagged requirements @@ -142,7 +143,6 @@ _None._ - FR-EXP-7 - FR-NC-10 - FR-NC-11 -- FR-NC-6 - FR-NC-6b - FR-PLAT-AND-2 - FR-PLAT-AND-4 diff --git a/ui/dr-ui/src/activity.rs b/ui/dr-ui/src/activity.rs new file mode 100644 index 0000000..da3157a --- /dev/null +++ b/ui/dr-ui/src/activity.rs @@ -0,0 +1,613 @@ +//! TRACES: FR-CAT-1 | FR-NC-6 | FR-NC-6c +//! One register of the work running behind the interface. +//! +//! Every background operation in this crate has the same shape: a worker +//! thread with an `mpsc` channel, drained by a `slint::Timer` on the UI thread. +//! Each of them used to report into a window property of its own — +//! `library-thumbs-done`, `library-pin-total`, `library-syncing` — and that had +//! two consequences worth removing. +//! +//! - **Only the grid could see them.** A pin download outlives the view it was +//! started from, so opening an image made an hour of transfers invisible: the +//! properties are read by `LibraryGrid` and nothing else. +//! - **There was no answer to "what is this doing".** The answer was spread +//! across eight properties that no code path ever collected, which is why the +//! settings page could not show a list of transfers: there was nothing to +//! list. +//! +//! So the jobs report here instead, and this publishes twice: an aggregate that +//! drives the bar across the top of the shell, and a row per job for the +//! settings page. +//! +//! # Everything here is single-threaded on purpose +//! +//! No `Arc`, no lock. The workers already hand their progress to the UI thread +//! through a channel, and the drain that reads it is the only thing that talks +//! to this register — so the shared-state problem was solved before this file +//! existed, and re-solving it with a mutex would only add a lock that is never +//! contended. +//! +//! # A job that stops reporting cannot hang the bar +//! +//! [`Activity`] is a handle whose `Drop` removes a still-running job. The drain +//! closures own their handle, so a worker that dies mid-transfer, a timer +//! replaced by a newer batch, or a window that closes all take their rows with +//! them. Without that, one lost `Finished` message would leave the bar sweeping +//! for the rest of the session — and a progress indicator that lies about +//! whether anything is happening is worse than none. + +use std::cell::{Cell, RefCell}; +use std::rc::{Rc, Weak}; + +use slint::ComponentHandle; + +use crate::{ActivityRow, AppWindow}; + +/// How many stopped jobs to keep. +/// +/// A short history rather than none: "did the sync work?" is asked *after* the +/// sync, and a list that empties the instant a job ends can only ever answer +/// questions about the present. Short, because it is a status list and not a +/// log — the ones worth keeping past this are failures, and those are held +/// until the user clears them. +const KEEP_STOPPED: usize = 8; + +/// How often the register publishes into the window. +/// +/// Publishing on every change would rebuild the row model once per drained +/// message, and a thumbnail batch drains a hundred in a tick. This is fast +/// enough that a bar looks live and slow enough that a burst of completions +/// costs one update rather than a hundred. +const PUBLISH_INTERVAL: std::time::Duration = std::time::Duration::from_millis(150); + +/// What kind of work a job is. +/// +/// Coarser than the set of functions that start jobs: the user's question is +/// "is something downloading", not which of three call sites issued the fetch. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Kind { + /// Walking the remote library (FR-CAT-1). + Scan, + /// Reading capture times across the whole library. + Index, + /// Previews for the cells on screen. + Thumbnails, + /// Original files coming down — a pin, or an image being opened. + Download, + /// Sidecars and derived data going up. + Upload, + /// The two-way exchange of catalog and shards. + Sync, + /// Moving files to the trash, restoring them, or emptying it. + Trash, +} + +impl Kind { + /// Whether this job moves bytes over the network. + /// + /// FR-NC-6c: the user's question on a slow or metered connection is about + /// *transfers* specifically, so the row says which jobs are ones. + pub fn is_transfer(self) -> bool { + match self { + Kind::Scan + | Kind::Thumbnails + | Kind::Download + | Kind::Upload + | Kind::Sync + | Kind::Trash => true, + // Reads headers over the network today, but it is bounded by the + // catalog rather than by anything the user asked to move, and it + // stores nothing. Calling it a transfer would put an hours-long + // background sweep in the same sentence as a download they are + // waiting on. + Kind::Index => false, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum State { + Running, + Done, + Failed, +} + +/// One job's row in the register. +#[derive(Debug, Clone)] +struct Record { + id: u64, + kind: Kind, + title: String, + detail: String, + done: usize, + /// Zero means "no denominator" — a directory walk cannot know its extent, + /// and a percentage invented for it is worse than admitting as much. + total: usize, + state: State, +} + +impl Record { + fn fraction(&self) -> f32 { + if self.total == 0 { + return 0.0; + } + (self.done as f32 / self.total as f32).clamp(0.0, 1.0) + } + + fn row(&self) -> ActivityRow { + ActivityRow { + title: self.title.clone().into(), + detail: self.detail.clone().into(), + fraction: self.fraction(), + determinate: self.total > 0, + running: self.state == State::Running, + failed: self.state == State::Failed, + transfer: self.kind.is_transfer(), + } + } +} + +/// The aggregate the shell's bar is drawn from. +#[derive(Debug, Clone, PartialEq)] +pub struct Summary { + pub running: usize, + /// Stopped jobs still listed — recent successes and unread failures. + pub kept: usize, + /// `None` where nothing running knows its own extent. + pub fraction: Option, +} + +/// Every background job the application is running. +pub struct ActivityLog { + records: RefCell>, + next_id: Cell, + /// Where to publish. Absent in tests, which is what makes the whole + /// register testable without a window. + window: RefCell>>, + pump: RefCell>, + dirty: Cell, +} + +impl Default for ActivityLog { + /// An empty register attached to nothing. + /// + /// Publishing is a no-op until [`ActivityLog::attach`] gives it a window, + /// which is what lets a controller hold one in a test with no interface + /// running — and what makes this file's own tests possible. + fn default() -> Self { + Self { + records: RefCell::new(Vec::new()), + next_id: Cell::new(1), + window: RefCell::new(None), + pump: RefCell::new(None), + dirty: Cell::new(false), + } + } +} + +impl ActivityLog { + pub fn new() -> Rc { + Rc::new(Self::default()) + } + + /// Start publishing into `window`. + /// + /// The timer holds a *weak* reference back: an `Rc` here would be a cycle + /// through the closure, and the register would outlive the window it is + /// describing. + pub fn attach(self: &Rc, window: &AppWindow) { + *self.window.borrow_mut() = Some(window.as_weak()); + + let weak = Rc::downgrade(self); + let timer = slint::Timer::default(); + timer.start(slint::TimerMode::Repeated, PUBLISH_INTERVAL, move || { + let Some(log) = Weak::upgrade(&weak) else { + return; + }; + if log.dirty.replace(false) { + log.publish(); + } + }); + *self.pump.borrow_mut() = Some(timer); + + // Once immediately, so an empty register starts out saying so rather + // than leaving whatever the properties defaulted to. + self.publish(); + } + + /// Register a job and hand back the handle that reports on it. + pub fn begin(self: &Rc, kind: Kind, title: impl Into) -> Activity { + let id = self.next_id.get(); + self.next_id.set(id + 1); + + self.records.borrow_mut().push(Record { + id, + kind, + title: title.into(), + detail: String::new(), + done: 0, + total: 0, + state: State::Running, + }); + self.touch(); + + Activity { + log: self.clone(), + id, + } + } + + /// Drop every stopped job, keeping what is still running. + pub fn clear_finished(&self) { + self.records + .borrow_mut() + .retain(|r| r.state == State::Running); + self.touch(); + } + + pub fn summary(&self) -> Summary { + let records = self.records.borrow(); + let running: Vec<&Record> = records + .iter() + .filter(|r| r.state == State::Running) + .collect(); + + // Summed across jobs rather than averaged: two downloads of wildly + // different size are one wait as far as the person watching is + // concerned, and averaging their percentages would have the bar jump + // backwards whenever a small job joined a large one. + let (done, total) = running + .iter() + .filter(|r| r.total > 0) + .fold((0usize, 0usize), |(d, t), r| (d + r.done, t + r.total)); + + Summary { + running: running.len(), + kept: records.len() - running.len(), + fraction: (total > 0).then(|| (done as f32 / total as f32).clamp(0.0, 1.0)), + } + } + + /// The rows the settings page lists, running jobs first. + /// + /// Stable within each group — insertion order — so a list being watched + /// does not reshuffle itself under the reader every time a count changes. + fn rows(&self) -> Vec { + let records = self.records.borrow(); + records + .iter() + .filter(|r| r.state == State::Running) + .chain(records.iter().filter(|r| r.state != State::Running)) + .map(Record::row) + .collect() + } + + fn publish(&self) { + let Some(window) = self.window.borrow().as_ref().and_then(slint::Weak::upgrade) else { + return; + }; + + // Built before anything is written, so no property setter can run with + // `records` borrowed. + let rows = self.rows(); + let summary = self.summary(); + + window.set_activity_rows(slint::ModelRc::new(slint::VecModel::from(rows))); + window.set_activity_busy(summary.running > 0); + window.set_activity_running(summary.running as i32); + window.set_activity_kept(summary.kept as i32); + window.set_activity_determinate(summary.fraction.is_some()); + window.set_activity_fraction(summary.fraction.unwrap_or(0.0)); + } + + fn touch(&self) { + self.dirty.set(true); + } + + /// Edit a *running* job. + /// + /// A stopped one is left alone: a drain loop reads whatever the channel + /// still holds after the message that ended the job, and a queued progress + /// update arriving second would otherwise overwrite the failure the user + /// needs to read with a count that no longer means anything. + fn with(&self, id: u64, f: impl FnOnce(&mut Record)) { + if let Some(record) = self + .records + .borrow_mut() + .iter_mut() + .find(|r| r.id == id && r.state == State::Running) + { + f(record); + } + self.touch(); + } + + /// Stop a job, keeping it in the list. + /// + /// First outcome wins, for the reason given on [`ActivityLog::with`]: a + /// worker that reports a failure and then closes its channel has failed, + /// and the closing must not relabel it as finished. + fn stop(&self, id: u64, state: State, detail: String) { + { + let mut records = self.records.borrow_mut(); + let Some(record) = records + .iter_mut() + .find(|r| r.id == id && r.state == State::Running) + else { + return; + }; + record.state = state; + record.detail = detail; + // A finished job reads as complete whatever it counted: a scan that + // ends having found forty of an unknown number is done, not 40%. + if state == State::Done && record.total > 0 { + record.done = record.total; + } + trim(&mut records); + } + self.touch(); + } + + /// Take a running job out of the register entirely. + /// + /// Returns whether there was one to remove, which is how [`Activity::drop`] + /// tells "the drain forgot about this" from "it stopped properly". + fn remove(&self, id: u64) -> bool { + let removed = { + let mut records = self.records.borrow_mut(); + let before = records.len(); + records.retain(|r| !(r.id == id && r.state == State::Running)); + records.len() != before + }; + self.touch(); + removed + } +} + +/// Drop the oldest stopped jobs past [`KEEP_STOPPED`]. +/// +/// Failures are exempt: a transfer that failed while the user was elsewhere is +/// the single most useful thing this list holds, and letting eight successful +/// thumbnail batches push it out would lose exactly the row worth keeping. +fn trim(records: &mut Vec) { + let mut excess = records + .iter() + .filter(|r| r.state == State::Done) + .count() + .saturating_sub(KEEP_STOPPED); + + records.retain(|r| { + if excess > 0 && r.state == State::Done { + excess -= 1; + return false; + } + true + }); +} + +/// Bytes as a figure to put beside a transfer. +/// +/// Three scales rather than one: a sidecar is a few kilobytes and a RAW file is +/// tens of megabytes, and a single unit makes one of them read as "0.0" or as +/// six digits. One decimal at most — this is a status line, not a measurement. +pub fn describe_bytes(bytes: u64) -> String { + const KB: f64 = 1024.0; + const MB: f64 = 1024.0 * KB; + const GB: f64 = 1024.0 * MB; + + let b = bytes as f64; + if b >= GB { + format!("{:.1} GB", b / GB) + } else if b >= MB { + format!("{:.1} MB", b / MB) + } else { + format!("{:.0} kB", (b / KB).ceil()) + } +} + +/// A running job, held by whatever is reporting on it. +/// +/// Every method is idempotent and every one is a no-op once the job has +/// stopped, because the drains that call them are loops over a channel that may +/// deliver a late message after the one that ended the job. +pub struct Activity { + log: Rc, + id: u64, +} + +impl Activity { + /// What the job is currently doing, in the job's own words. + pub fn detail(&self, detail: impl Into) { + let detail = detail.into(); + self.log.with(self.id, |r| r.detail = detail); + } + + /// How much work there is. Zero leaves the job indeterminate. + pub fn total(&self, total: usize) { + self.log.with(self.id, |r| r.total = total); + } + + /// More work turned up mid-flight — a batch that grew once the plan came + /// back. Added rather than replaced, so the count already served stays + /// meaningful. + pub fn add_total(&self, extra: usize) { + self.log.with(self.id, |r| r.total += extra); + } + + /// One unit of work finished. + pub fn advance(&self) { + self.log.with(self.id, |r| r.done += 1); + } + + /// Set both counts at once, for workers that report a running total. + pub fn progress(&self, done: usize, total: usize) { + self.log.with(self.id, |r| { + r.done = done; + r.total = total; + }); + } + + /// The job finished. `note` is what the list shows afterwards. + pub fn finish(&self, note: impl Into) { + self.log.stop(self.id, State::Done, note.into()); + } + + /// The job finished and is not worth remembering. + /// + /// For routine work the user never asked for by name: scrolling the grid + /// starts a thumbnail batch every second or so, and keeping those would + /// push a failed transfer out of the list within moments of it happening. + /// A failure is still kept — only [`Activity::finish`]'s history is + /// skipped. + pub fn finish_quietly(&self) { + self.log.remove(self.id); + } + + /// The job stopped without doing what it set out to do. + /// + /// Kept in the list until the user clears it: this is the row they came to + /// the settings page to find. + pub fn fail(&self, message: impl Into) { + self.log.stop(self.id, State::Failed, message.into()); + } +} + +impl Drop for Activity { + /// A job whose handle goes away while it is still running is removed, not + /// marked failed. Nobody is coming back to report on it, and a row frozen + /// at "downloading, 12 of 900" would sit in the list claiming to be live + /// for the rest of the session. + fn drop(&mut self) { + if self.log.remove(self.id) { + log::debug!("a background job ended without saying so"); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_running_job_makes_the_bar_busy() { + let log = ActivityLog::new(); + assert_eq!(log.summary().running, 0); + + let scan = log.begin(Kind::Scan, "Scanning"); + assert_eq!(log.summary().running, 1); + // A scan has no denominator, so the bar must not claim a position. + assert_eq!(log.summary().fraction, None); + + scan.finish("up to date"); + assert_eq!(log.summary().running, 0); + } + + #[test] + fn progress_is_summed_across_jobs() { + let log = ActivityLog::new(); + let a = log.begin(Kind::Thumbnails, "Thumbnails"); + let b = log.begin(Kind::Download, "Downloading"); + a.progress(1, 10); + b.progress(4, 10); + + // 5 of 20, not the average of 10% and 40%. + assert_eq!(log.summary().fraction, Some(0.25)); + } + + #[test] + fn a_job_without_a_denominator_does_not_dilute_one_that_has_it() { + let log = ActivityLog::new(); + let scan = log.begin(Kind::Scan, "Scanning"); + let fetch = log.begin(Kind::Download, "Downloading"); + fetch.progress(3, 4); + scan.detail("120 folders"); + + assert_eq!(log.summary().fraction, Some(0.75)); + } + + #[test] + fn a_dropped_handle_takes_its_row_with_it() { + let log = ActivityLog::new(); + { + let _thumbs = log.begin(Kind::Thumbnails, "Thumbnails"); + assert_eq!(log.summary().running, 1); + } + // The worker died, or a newer batch replaced the timer holding this. + // Either way the bar must not sweep for ever. + assert_eq!(log.summary().running, 0); + assert_eq!(log.summary().kept, 0); + } + + #[test] + fn a_finished_job_is_kept_and_reads_as_complete() { + let log = ActivityLog::new(); + let sweep = log.begin(Kind::Index, "Indexing"); + sweep.progress(40, 100); + // Ended early with everything it was going to do done. + sweep.finish("4000 dated"); + drop(sweep); + + let summary = log.summary(); + assert_eq!(summary.running, 0); + assert_eq!(summary.kept, 1, "a finished job stays in the list"); + + let rows = log.rows(); + assert_eq!(rows[0].fraction, 1.0); + assert!(!rows[0].running); + assert!(!rows[0].failed); + } + + #[test] + fn a_failure_survives_a_run_of_successes() { + let log = ActivityLog::new(); + log.begin(Kind::Download, "Downloading") + .fail("host is down"); + for i in 0..KEEP_STOPPED * 2 { + log.begin(Kind::Thumbnails, format!("Batch {i}")) + .finish("done"); + } + + let rows = log.rows(); + assert!( + rows.iter().any(|r| r.failed), + "the one row worth keeping was pushed out by routine successes" + ); + assert!(rows.len() <= KEEP_STOPPED + 1, "history is bounded"); + } + + #[test] + fn clearing_leaves_running_jobs_alone() { + let log = ActivityLog::new(); + let scan = log.begin(Kind::Scan, "Scanning"); + log.begin(Kind::Sync, "Syncing").fail("timed out"); + + log.clear_finished(); + assert_eq!(log.summary().kept, 0); + assert_eq!(log.summary().running, 1); + drop(scan); + } + + #[test] + fn running_jobs_are_listed_first() { + let log = ActivityLog::new(); + log.begin(Kind::Sync, "Syncing").finish("nothing to do"); + let scan = log.begin(Kind::Scan, "Scanning"); + + let rows = log.rows(); + assert_eq!(rows[0].title, "Scanning"); + assert!(rows[0].running); + drop(scan); + } + + #[test] + fn a_late_message_cannot_restart_a_stopped_job() { + let log = ActivityLog::new(); + let fetch = log.begin(Kind::Download, "Downloading"); + fetch.fail("connection reset"); + // The drain reads one more queued message before it notices. + fetch.advance(); + + assert_eq!(log.summary().running, 0); + assert!(log.rows()[0].failed); + } +} diff --git a/ui/dr-ui/src/collections_ui.rs b/ui/dr-ui/src/collections_ui.rs index 75a4319..f65b47d 100644 --- a/ui/dr-ui/src/collections_ui.rs +++ b/ui/dr-ui/src/collections_ui.rs @@ -142,11 +142,20 @@ pub struct CollectionsController { /// the tree, the grid cells — and doing that from inside the `dropped` /// handler destroys the elements Slint is still using to deliver the event. dropped_on: RefCell>, + /// Where the trash workers report what they are doing. + /// + /// Shared with [`crate::library_ui`]: a delete and a scan are two jobs in + /// one list, and the user asking what the application is busy with does not + /// care which module started them. + activity: Rc, } impl CollectionsController { - pub fn new() -> Rc { - Rc::new(Self::default()) + pub fn new(activity: Rc) -> Rc { + Rc::new(Self { + activity, + ..Default::default() + }) } /// Which collection the grid is scoped to, for [`crate::library_ui`] to @@ -786,6 +795,7 @@ fn start_trash( // survive as a set of ids the user can no longer see. ctl.clear_selection(); + let count = moves.len(); let rx = crate::trash::spawn_move( creds, sess.user_id.clone(), @@ -800,6 +810,7 @@ fn start_trash( catalog.clone(), rx, reload.clone(), + format!("Moving {count} photograph(s) to the trash"), ); } @@ -861,6 +872,7 @@ fn start_restore( // would survive as ids the user can no longer see. ctl.clear_selection(); + let count = moves.len(); let rx = crate::trash::spawn_move( creds, sess.user_id.clone(), @@ -875,6 +887,7 @@ fn start_restore( catalog.clone(), rx, reload.clone(), + format!("Restoring {count} photograph(s)"), ); } @@ -888,12 +901,22 @@ fn drain_trash( catalog: Rc>>, rx: std::sync::mpsc::Receiver, reload: Rc, + // What the register calls this operation. Passed in rather than derived + // here: the three callers move files to the trash, back out of it, and + // delete them outright, and "Deleting 40 photographs" is the one word of + // the three that must not appear over a restore. + title: String, ) { use crate::trash::TrashMessage; let timer = slint::Timer::default(); let ctl_cb = ctl.clone(); + // A server-side MOVE per file, so it is a transfer in the sense that + // matters: it takes as long as the connection is slow, and it can fail + // halfway with the library in two states at once. + let job = ctl.activity.begin(crate::activity::Kind::Trash, title); + timer.start( slint::TimerMode::Repeated, std::time::Duration::from_millis(120), @@ -907,6 +930,7 @@ fn drain_trash( Err(std::sync::mpsc::TryRecvError::Disconnected) => { // A worker that died without reporting must not leave the // status line mid-sentence. + job.fail("stopped without finishing"); stop_trash(&ctl_cb); return; } @@ -923,6 +947,10 @@ fn drain_trash( } else { format!("{done} / {total}") }; + job.progress(done, total); + if failed > 0 { + job.detail(format!("{failed} failed")); + } w.set_library_status(status.into()); } TrashMessage::Done { moved, failed } => { @@ -934,6 +962,14 @@ fn drain_trash( } else { format!("{moved} done · {} failed", failed.len()) }; + // A partial failure is a failure in the register: the + // library is now in two states at once, which is + // exactly the thing worth keeping on the list. + if failed.is_empty() { + job.finish(status.clone()); + } else { + job.fail(status.clone()); + } w.set_library_status(status.into()); if let Some(first) = failed.first() { w.set_collection_error(first.as_str().into()); @@ -1396,6 +1432,7 @@ pub fn wire( log::info!("emptying trash: {} image(s)", ids.len()); w.set_library_status(format!("Deleting {} image(s)…", ids.len()).into()); + let count = ids.len(); let rx = crate::trash::spawn_purge( creds, @@ -1412,6 +1449,7 @@ pub fn wire( catalog.clone(), rx, reload.clone(), + format!("Deleting {count} photograph(s) permanently"), ); }); } diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index e60cbc7..3622881 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -14,6 +14,7 @@ //! pipeline what parameters it has and builds a control per answer; no code //! in `ui/` names an operation or knows a shader exists (FR-DEV-3a). +mod activity; mod collections_ui; mod derived_sync; mod develop; @@ -434,6 +435,18 @@ pub fn run(paths: Vec) -> Result<()> { let window = AppWindow::new()?; + // Every background job reports here, and this draws the bar across the top + // of the shell and fills the settings page's list. Built before the + // controllers because they take a handle to it: a job that starts during + // startup — the scan a resumed session begins immediately — has to have + // somewhere to report to before it starts, or its first minute is invisible. + let activity = activity::ActivityLog::new(); + activity.attach(&window); + { + let activity = activity.clone(); + window.on_activity_clear_finished(move || activity.clear_finished()); + } + // Set once `show` exists; see where the library grid is wired below. #[allow(clippy::type_complexity)] let open_from_library: Rc>>> = Rc::new(RefCell::new(None)); @@ -449,7 +462,7 @@ pub fn run(paths: Vec) -> Result<()> { // Declared out here rather than inside the launch block below because the // develop side reads `paths` to rebuild its browsing list when an image is // opened from the grid. - let library = library_ui::LibraryController::new(); + let library = library_ui::LibraryController::new(activity.clone()); // Launch screen: shown when there is nothing to display — no local paths // and no configured library. A user who has already signed in and chosen @@ -460,7 +473,7 @@ pub fn run(paths: Vec) -> Result<()> { window.set_show_launch(startup == launch::Startup::ShowLaunchScreen); let library = library.clone(); - let collections = collections_ui::CollectionsController::new(); + let collections = collections_ui::CollectionsController::new(activity.clone()); // The click handler needs `show`, which is built further down because // it captures the develop session and the GPU context. This cell is @@ -860,6 +873,7 @@ pub fn run(paths: Vec) -> Result<()> { let redraw = redraw.clone(); let rows = rows.clone(); let gpu = gpu.clone(); + let activity = activity.clone(); *open_from_library.borrow_mut() = Some(Rc::new(move |path: String| { let Some(w) = weak.upgrade() else { return }; @@ -895,6 +909,14 @@ pub fn run(paths: Vec) -> Result<()> { let rx = library::spawn_full_fetch(creds, user_id, path.clone(), cache); + // The one transfer the user is actively waiting on. It gets a row + // like any other, so a download that is still running after they + // give up and go back to the grid is still accounted for. + // + // No denominator: `spawn_full_fetch` reports a result, not bytes as + // they arrive, so the honest bar here is the indeterminate one. + let job = activity.begin(activity::Kind::Download, format!("Downloading {name}")); + // Polled on the UI thread rather than joined: a join would freeze // the window for the length of the download. let weak = w.as_weak(); @@ -916,6 +938,7 @@ pub fn run(paths: Vec) -> Result<()> { let bytes = match got { Ok(b) => b, Err(e) => { + job.fail(e.message.clone()); log::warn!("{name}: {e}"); // Offline needs its own words. "network error: // connection refused" over a photograph the user @@ -931,6 +954,7 @@ pub fn run(paths: Vec) -> Result<()> { return; } }; + job.finish(activity::describe_bytes(bytes.len() as u64)); log::info!("{name}: {} bytes fetched", bytes.len()); match load_bytes(gpu.as_ref(), &bytes) { diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index f1addc6..39b82be 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -198,11 +198,19 @@ pub struct LibraryController { /// small budget still keeps a working set. A metered or small-disk device /// wants the first. keep_opened: std::cell::Cell, + /// Where every worker this module starts reports what it is doing. + /// + /// Held on the controller rather than passed to each function because the + /// jobs are started from a dozen callbacks — a scroll, a rescan, a pin, a + /// reconnect — and threading a second argument through all of them would + /// say nothing except that they all report progress. + activity: Rc, } impl LibraryController { - pub fn new() -> Rc { + pub fn new(activity: Rc) -> Rc { Rc::new(Self { + activity, catalog: Rc::new(RefCell::new(None)), paths: RefCell::new(Vec::new()), file_ids: RefCell::new(Vec::new()), @@ -536,6 +544,20 @@ fn drain_scan( let timer = slint::Timer::default(); let ctl_cb = ctl.clone(); + // Indeterminate for as long as it runs: a recursive walk discovers its own + // extent, so the count it reports is what it has *found*, never a fraction + // of what there is (FR-CAT-1). + // + // Named after the folder, because two accounts or two roots produce rows + // that are otherwise identical. + let title = match ctl.session.borrow().as_ref() { + Some((_, session, _)) if !session.root.is_empty() => { + format!("Scanning {}", session.root) + } + _ => "Scanning the library".to_string(), + }; + let job = ctl.activity.begin(crate::activity::Kind::Scan, title); + timer.start( slint::TimerMode::Repeated, std::time::Duration::from_millis(120), @@ -553,6 +575,7 @@ fn drain_scan( if w.get_library_scanning() { w.set_library_scanning(false); w.set_library_error("scan ended unexpectedly".into()); + job.fail("ended unexpectedly"); } stop(&ctl.scan_timer); return; @@ -574,6 +597,7 @@ fn drain_scan( } else { format!("{directories} folders · {images} images") }; + job.detail(status.clone()); w.set_library_status(status.into()); } ScanMessage::Done { @@ -611,6 +635,7 @@ fn drain_scan( } else { format!("{total} images · {secs:.1}s") }; + job.finish(status.clone()); w.set_library_status(status.into()); match Catalog::open(&catalog_path) { @@ -636,6 +661,11 @@ fn drain_scan( ScanMessage::Failed { message, offline } => { log::warn!("scan failed: {message}"); w.set_library_scanning(false); + // Recorded as a failure even where it is only the + // connection: the grid's offline banner says the server + // is unreachable, and this says which piece of work + // stopped because of it. + job.fail(message.clone()); if offline { // Not an error state. The catalog from the last @@ -829,6 +859,14 @@ fn start_pin_fetch(window: &AppWindow, ctl: &Rc) { let weak = window.as_weak(); let ctl_cb = ctl.clone(); + // The longest-running transfer the app does, and the one most likely to be + // watched from another view — which is the whole reason the register + // exists (FR-NC-6, FR-NC-6c). + let job = ctl.activity.begin( + crate::activity::Kind::Download, + "Keeping photographs on this device", + ); + timer.start( slint::TimerMode::Repeated, std::time::Duration::from_millis(300), @@ -840,6 +878,7 @@ fn start_pin_fetch(window: &AppWindow, ctl: &Rc) { Err(std::sync::mpsc::TryRecvError::Empty) => return, Err(std::sync::mpsc::TryRecvError::Disconnected) => { w.set_library_pin_total(0); + job.fail("stopped without finishing"); stop(&ctl_cb.pin_timer); return; } @@ -849,9 +888,11 @@ fn start_pin_fetch(window: &AppWindow, ctl: &Rc) { library::PinMessage::Planned { total } => { w.set_library_pin_total(total as i32); w.set_library_pin_done(0); + job.total(total); } library::PinMessage::Stored { done } => { w.set_library_pin_done(done as i32); + job.progress(done, w.get_library_pin_total() as usize); // The "On this device" count grows as they land, so // the chip agrees with the progress line beside it. refresh_local_count(&w, &ctl_cb); @@ -861,6 +902,10 @@ fn start_pin_fetch(window: &AppWindow, ctl: &Rc) { "pin complete: {stored} original(s), {:.1} MB", bytes as f64 / 1_048_576.0 ); + job.finish(format!( + "{stored} photograph(s) · {}", + crate::activity::describe_bytes(bytes) + )); w.set_library_pin_total(0); w.set_library_pin_done(0); refresh_local_count(&w, &ctl_cb); @@ -869,6 +914,7 @@ fn start_pin_fetch(window: &AppWindow, ctl: &Rc) { } library::PinMessage::Failed { message, offline } => { log::warn!("pin fetch stopped: {message}"); + job.fail(message.clone()); w.set_library_pin_total(0); if offline { ctl_cb @@ -1410,12 +1456,21 @@ fn start_sidecar_writes( return; }; + let count = writes.len(); let rx = library::spawn_sidecar_writes(creds, session.user_id.clone(), writes); let timer = slint::Timer::default(); let weak = window.as_weak(); let ctl_cb = ctl.clone(); + // The writer reports once at the end, so there is no per-file progress to + // show — but a cull that has just rated forty frames has forty uploads in + // flight, and "is that saved yet" deserves an answer somewhere. + let job = ctl.activity.begin( + crate::activity::Kind::Upload, + format!("Saving {count} judgement(s)"), + ); + timer.start( slint::TimerMode::Repeated, std::time::Duration::from_millis(200), @@ -1435,6 +1490,10 @@ fn start_sidecar_writes( "{failed} sidecar write(s) failed: {}", last_error.clone().unwrap_or_default() ); + job.fail(format!( + "{written} saved · {failed} failed: {}", + last_error.clone().unwrap_or_default() + )); // Said plainly, because the consequence is specific: // the rating is safe in the catalog but will not // survive deleting it. @@ -1447,6 +1506,9 @@ fn start_sidecar_writes( ); } else { log::debug!("{written} sidecar(s) written"); + // Quietly: a cull produces one of these every few + // seconds and none of them is news. + job.finish_quietly(); } stop(&ctl_cb.sidecar_timer); } @@ -1525,11 +1587,7 @@ fn request_thumbnails(window: &AppWindow, ctl: &Rc) { return; } - // Reset the counter to this batch, so the bar measures the work actually - // outstanding rather than accumulating across batches. - window.set_library_thumbs_total(wanted.len() as i32); - window.set_library_thumbs_done(0); - + let requested = wanted.len(); let rx = library::spawn_thumbnails( creds, session.user_id.clone(), @@ -1537,7 +1595,7 @@ fn request_thumbnails(window: &AppWindow, ctl: &Rc) { library::thumbs_dir(&session.server, &session.user_id), library::catalog_path(&session.server, &session.user_id), ); - drain_thumbnails(window.as_weak(), ctl.clone(), rx); + drain_thumbnails(window.as_weak(), ctl.clone(), rx, requested); } /// Apply thumbnails to the model as they arrive. @@ -1545,9 +1603,19 @@ fn drain_thumbnails( weak: slint::Weak, ctl: Rc, rx: Receiver, + requested: usize, ) { let timer = slint::Timer::default(); let ctl_cb = ctl.clone(); + + // One row per batch, measured against the cells this window asked for. + // Starting a new batch does not extend the last one: a scroll abandons + // whatever the previous window wanted, and a denominator carried across + // both would describe neither. + let job = ctl + .activity + .begin(crate::activity::Kind::Thumbnails, "Loading thumbnails"); + job.total(requested); // Which window this batch was requested for. Captured at spawn, compared on // every tick. let mine = ctl.generation.get(); @@ -1589,7 +1657,11 @@ fn drain_thumbnails( // The worker finished or died. Either way nothing more // is coming, so the bar must not sit part-filled // forever. - w.set_library_thumbs_done(w.get_library_thumbs_total()); + // + // Quietly: a scroll starts one of these every second, + // and a history of them would bury anything worth + // reading. + job.finish_quietly(); stop(&ctl_cb.thumb_timer); return; } @@ -1606,7 +1678,7 @@ fn drain_thumbnails( // Date reads produce no cell, so they are counted into // the bar's denominator or it finishes while work is // still running. - w.set_library_thumbs_total(w.get_library_thumbs_total() + dating as i32); + job.add_total(dating); let mut parts = Vec::new(); if cached > 0 { @@ -1619,12 +1691,14 @@ fn drain_thumbnails( parts.push(format!("reading {dating} dates")); } if !parts.is_empty() { - w.set_library_status(parts.join(" · ").into()); + let status = parts.join(" · "); + job.detail(status.clone()); + w.set_library_status(status.into()); } } // A header-only date read. Advances the bar; draws nothing. ThumbnailMessage::DateProgress => { - w.set_library_thumbs_done(w.get_library_thumbs_done() + 1); + job.advance(); } // Dates landed, so the histogram can now be built. This is // what makes the timeline appear on a library whose @@ -1640,7 +1714,7 @@ fn drain_thumbnails( // successes would stall it on a library where some files // carry no embedded preview. ThumbnailMessage::Ready(t) => { - w.set_library_thumbs_done(w.get_library_thumbs_done() + 1); + job.advance(); // Bytes arrived *from the server*, so it is reachable. // This is what clears the banner when a connection // returns while the user is simply scrolling, without @@ -1672,7 +1746,7 @@ fn drain_thumbnails( } } ThumbnailMessage::Unavailable { row, reason } => { - w.set_library_thumbs_done(w.get_library_thumbs_done() + 1); + job.advance(); log::debug!("thumbnail {row}: {reason}"); if let Some(mut r) = model.row_data(row) { r.unavailable = true; @@ -1682,18 +1756,16 @@ fn drain_thumbnails( // TRACES: FR-CAT-9 ThumbnailMessage::Offline { reason } => { log::info!("thumbnails stopped: {reason}"); + // The batch is over, so the bar must not be left + // showing a partial fetch that will never finish — it + // would sweep for ever. + job.fail(reason.clone()); ctl_cb .reachability .borrow_mut() .mark_unreachable(reason, std::time::Instant::now()); refresh_offline(&w, &ctl_cb); - // The batch is over, so the progress counter must not - // be left showing a partial fetch that will never - // finish — it would spin in the header for ever. - w.set_library_thumbs_total(0); - w.set_library_thumbs_done(0); - // Cells left without pixels stay placeholders rather // than being marked unavailable: the images are fine, // and a reconnect should fill them in. Marking them @@ -1784,6 +1856,12 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc) { let weak = window.as_weak(); let ctl_cb = ctl.clone(); + // The sync reports stages rather than counts, so it stays indeterminate and + // says what stage it is in. + let job = ctl + .activity + .begin(crate::activity::Kind::Sync, "Syncing with the server"); + timer.start( slint::TimerMode::Repeated, std::time::Duration::from_millis(300), @@ -1795,6 +1873,7 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc) { Err(std::sync::mpsc::TryRecvError::Empty) => return, Err(std::sync::mpsc::TryRecvError::Disconnected) => { w.set_library_syncing(false); + job.fail("stopped without finishing"); stop(&ctl_cb.sync_timer); return; } @@ -1802,6 +1881,7 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc) { match msg { crate::derived_sync::SyncMessage::Status(s) => { + job.detail(s.clone()); w.set_library_status(s.into()); } crate::derived_sync::SyncMessage::Finished(report) => { @@ -1823,14 +1903,17 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc) { } ); w.set_library_syncing(false); + let summary = format!( + "{} shard(s) up, {} down", + report.shards_uploaded, report.shards_downloaded + ); + job.finish(if report.did_anything() { + summary.clone() + } else { + "nothing to exchange".to_string() + }); if report.did_anything() { - w.set_library_status( - format!( - "synced · {} shard(s) up, {} down", - report.shards_uploaded, report.shards_downloaded - ) - .into(), - ); + w.set_library_status(format!("synced · {summary}").into()); } // Adopted thumbnails and merged collections both change // what the grid should show. @@ -1842,6 +1925,7 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc) { } crate::derived_sync::SyncMessage::Failed(e) => { log::warn!("sync failed: {e}"); + job.fail(e.to_string()); w.set_library_syncing(false); // Not an error banner: a failed sync costs nothing — // everything is still local and the next pass retries. @@ -1888,6 +1972,13 @@ fn start_sweep(window: &AppWindow, ctl: &Rc) { let weak = window.as_weak(); let ctl_cb = ctl.clone(); + // Hours on a large library, and entirely invisible outside the grid until + // now: the register is where a user who has gone to develop can still see + // that indexing is running and how far it has got. + let job = ctl + .activity + .begin(crate::activity::Kind::Index, "Indexing capture times"); + timer.start( slint::TimerMode::Repeated, // Slower than the thumbnail drain: this runs for tens of minutes and @@ -1902,6 +1993,7 @@ fn start_sweep(window: &AppWindow, ctl: &Rc) { Err(std::sync::mpsc::TryRecvError::Empty) => return, Err(std::sync::mpsc::TryRecvError::Disconnected) => { w.set_library_sweep_total(0); + job.fail("stopped without finishing"); stop(&ctl_cb.sweep_timer); return; } @@ -1911,9 +2003,11 @@ fn start_sweep(window: &AppWindow, ctl: &Rc) { library::SweepMessage::Total(n) => { w.set_library_sweep_total(n as i32); w.set_library_sweep_done(0); + job.total(n); } library::SweepMessage::Progress { done, dated } => { w.set_library_sweep_done(done as i32); + job.progress(done, w.get_library_sweep_total() as usize); // Rebuild as it goes: the histogram growing while the // sweep runs is the visible sign it is working. if dated > 0 { @@ -1925,6 +2019,7 @@ fn start_sweep(window: &AppWindow, ctl: &Rc) { } library::SweepMessage::Finished { dated } => { log::info!("sweep finished: {dated} dated"); + job.finish(format!("{dated} dated")); w.set_library_sweep_total(0); { let borrow = ctl_cb.catalog.borrow(); @@ -3261,7 +3356,7 @@ mod tests { /// the *current* batch's timer and leaves the new fetches undrained. #[test] fn a_reload_makes_an_in_flight_thumbnail_batch_stale() { - let ctl = LibraryController::new(); + let ctl = LibraryController::new(crate::activity::ActivityLog::new()); // What `drain_thumbnails` captures when the batch is spawned. let mine = ctl.generation.get(); @@ -3280,7 +3375,7 @@ mod tests { /// Each load is distinct, so two reloads cannot alias back to a live batch. #[test] fn every_window_load_takes_a_fresh_generation() { - let ctl = LibraryController::new(); + let ctl = LibraryController::new(crate::activity::ActivityLog::new()); let seen: Vec = (0..4) .map(|_| { let g = ctl.generation.get(); diff --git a/ui/dr-ui/ui/adjust.slint b/ui/dr-ui/ui/adjust.slint index ae1e6be..bfe99f8 100644 --- a/ui/dr-ui/ui/adjust.slint +++ b/ui/dr-ui/ui/adjust.slint @@ -585,19 +585,19 @@ export component GeometryPanel inherits Rectangle { clicked => { root.crop-toggled(!root.crop-mode); } } - // Rotation and flips. Glyphs rather than labels: four controls + // Rotation and flips. Icons rather than labels: four controls // named in words would wrap the 280px column, and each of // these shows its own result. HorizontalLayout { spacing: Theme.gap-sm; IconButton { - glyph: "⟲"; + icon: "rotate-ccw"; enabled: root.enabled; clicked => { root.rotate(-1); } } IconButton { - glyph: "⟳"; + icon: "rotate-cw"; enabled: root.enabled; clicked => { root.rotate(1); } } @@ -605,13 +605,13 @@ export component GeometryPanel inherits Rectangle { Rectangle { horizontal-stretch: 1; } IconButton { - glyph: "⇔"; + icon: "flip-h"; active: root.flip-h; enabled: root.enabled; clicked => { root.flip-h-toggled(); } } IconButton { - glyph: "⇕"; + icon: "flip-v"; active: root.flip-v; enabled: root.enabled; clicked => { root.flip-v-toggled(); } diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 21e75be..54ddb98 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -2,11 +2,11 @@ import { Theme } from "theme.slint"; import { AdjustPanel, GeometryPanel, ParamRow } from "adjust.slint"; import { LaunchScreen } from "launch.slint"; import { LibraryGrid, LibraryCell, TimelineBar } from "library.slint"; -import { Button, PanelHeading, Label, Value, Caption, Panel, EmptyState } from "widgets.slint"; +import { Button, PanelHeading, Label, Value, Caption, Panel, EmptyState, ProgressBar, ActivityRow } from "widgets.slint"; import { CollectionsPanel, CollectionRow } from "collections.slint"; import { SettingsPage } from "settings.slint"; -export { LibraryCell, TimelineBar, CollectionRow } +export { LibraryCell, TimelineBar, CollectionRow, ActivityRow } // Status strip — surfaces the GPU backend and adapter, which matters during // v0.1 because assumption A1 is exactly "does this compositing path work on @@ -274,11 +274,30 @@ export component AppWindow inherits Window { in property show-library: false; in-out property <[LibraryCell]> library-cells; in property library-total: 0; + // --- background activity (FR-CAT-1, FR-NC-6c) --- + // + // Every background job in one register, filled by `activity.rs`. The + // aggregate drives the bar across the top of the shell; the rows are the + // list on the settings page, where a stalled transfer can be identified + // rather than merely felt. + in property <[ActivityRow]> activity-rows; + /// Anything at all running. Drives the bar's visibility, so it is a + /// property rather than `activity-rows.length > 0` — finished rows stay in + /// the list for a while and must not keep the bar alight. + in property activity-busy: false; + /// Whether the running jobs, taken together, have a denominator. A scan + /// alone does not; a scan beside a download does. + in property activity-determinate: false; + in property activity-fraction: 0; + in property activity-running: 0; + /// Jobs that stopped and are being kept: finished ones as a short history, + /// failed ones until they are cleared. + in property activity-kept: 0; + callback activity-clear-finished(); + in property library-scanning: false; in property library-status: ""; in property library-error: ""; - in property library-thumbs-done: 0; - in property library-thumbs-total: 0; // --- offline mode (FR-CAT-9) --- // @@ -674,6 +693,11 @@ export component AppWindow inherits Window { strip-location-toggled(on) => { root.settings-strip-location-toggled(on); } destination-changed(t) => { root.settings-destination-changed(t); } + activity-rows: root.activity-rows; + activity-running: root.activity-running; + activity-kept: root.activity-kept; + clear-finished() => { root.activity-clear-finished(); } + close() => { root.settings-close(); } reset-defaults() => { root.settings-reset(); } } @@ -773,8 +797,6 @@ export component AppWindow inherits Window { scanning: root.library-scanning; scan-status: root.library-status; scan-error: root.library-error; - thumbs-done: root.library-thumbs-done; - thumbs-total: root.library-thumbs-total; offline: root.library-offline; offline-reason: root.library-offline-reason; @@ -1383,5 +1405,23 @@ export component AppWindow inherits Window { } } } + + // --- the load bar ------------------------------------------------- + // + // Last in the file, so it is last in z-order and no view can cover it. + // + // At the top of the *shell* rather than inside a view, because that is + // the only place that is true of every view: a pin download outlives + // the grid it was started from, and until this existed it drew nothing + // at all once the user opened an image. Three pixels, no text and no + // hit area — it says only "something is running", and the settings + // page says what (FR-CAT-1, FR-NC-6c). + if root.activity-busy: ProgressBar { + x: 0; + y: 0; + width: 100%; + indeterminate: !root.activity-determinate; + fraction: root.activity-fraction; + } } } diff --git a/ui/dr-ui/ui/collections.slint b/ui/dr-ui/ui/collections.slint index a6ce70e..599eff4 100644 --- a/ui/dr-ui/ui/collections.slint +++ b/ui/dr-ui/ui/collections.slint @@ -32,7 +32,7 @@ // release rather than the drop being silently discarded after it. import { Theme } from "theme.slint"; -import { Button } from "widgets.slint"; +import { Button, Icon } from "widgets.slint"; // One row of the collection tree. export struct CollectionRow { @@ -130,12 +130,12 @@ component TreeRow inherits Rectangle { Rectangle { width: 14px; - if root.entry.has-children: Text { - text: root.entry.expanded ? "▾" : "▸"; - color: Theme.ink-faint; - font-size: Theme.text-sm; - horizontal-alignment: center; - vertical-alignment: center; + if root.entry.has-children: Icon { + name: root.entry.expanded ? "chevron-down" : "chevron-right"; + ink: Theme.ink-faint; + size: 10px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } // Its own hit area: toggling open must not also select, or every @@ -148,11 +148,11 @@ component TreeRow inherits Rectangle { // Smart collections read differently from manual ones — the icon is // the only cue that its contents are computed, and that dropping // images on it will be refused. - Text { - text: root.entry.smart ? "◈" : "▤"; - color: root.entry.smart ? Theme.active-dim : Theme.ink-faint; - font-size: Theme.text-sm; - vertical-alignment: center; + Icon { + name: root.entry.smart ? "collection-smart" : "collection"; + ink: root.entry.smart ? Theme.active-dim : Theme.ink-faint; + size: 12px; + y: (parent.height - self.height) / 2; } // The name, or the field that is replacing it while this row is being @@ -337,7 +337,7 @@ export component CollectionsPanel inherits Rectangle { horizontal-stretch: 1; } - // New collection. A glyph rather than a word: the header is 232px + // New collection. A mark rather than a word: the header is 232px // wide and the label would crowd out the title. Rectangle { width: 22px; @@ -347,12 +347,12 @@ export component CollectionsPanel inherits Rectangle { : (add-touch.has-hover ? Theme.hover : transparent); border-radius: Theme.radius-sm; - Text { - text: "+"; - color: Theme.ink-dim; - font-size: Theme.text-lg; - horizontal-alignment: center; - vertical-alignment: center; + Icon { + name: "plus"; + ink: Theme.ink-dim; + size: 12px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } add-touch := TouchArea { clicked => { root.new-collection(); } @@ -494,11 +494,11 @@ export component CollectionsPanel inherits Rectangle { padding-right: Theme.gap-sm; spacing: Theme.gap-sm; - Text { - text: "🗑"; - color: root.trash-count > 0 ? Theme.warn-ink : Theme.ink-faint; - font-size: Theme.text-sm; - vertical-alignment: center; + Icon { + name: "trash"; + ink: root.trash-count > 0 ? Theme.warn-ink : Theme.ink-faint; + size: 12px; + y: (parent.height - self.height) / 2; } Text { diff --git a/ui/dr-ui/ui/icons.slint b/ui/dr-ui/ui/icons.slint new file mode 100644 index 0000000..52614ab --- /dev/null +++ b/ui/dr-ui/ui/icons.slint @@ -0,0 +1,256 @@ +// The icon set, drawn rather than typed. +// +// **Why this file exists.** Every glyph in this UI used to be a character in a +// Text: `⟲` for rotate-left, `☰` for the sidebar, `★` for a rating, `✓` for a +// tick. That works on a desktop, where the font stack ends in something with +// full symbol coverage, and it fails on Android, where it does not: the system +// font is Roboto, fallback there is resolved per *script* rather than per +// character, and none of the fallback families claimed for the Common script +// carry the Dingbats, Arrows or Miscellaneous Symbols blocks these characters +// live in. The result is that the toolbar renders as a row of tofu — a box +// with an X — and the controls become unusable. +// +// No amount of picking "safer" characters fixes that class of bug, because the +// question "is this codepoint in the font the platform happens to pick" has a +// different answer on every device. So the icons stop being text. Each one is +// an SVG path drawn into a 24×24 view box and rendered by `Path`, which goes +// through the same renderer as every other shape on screen and cannot fall +// back to anything. +// +// **The 24×24 view box is the contract.** `Path` with an explicit view box +// fits *the box* to the element, not the ink inside it, so two icons drawn to +// the same box occupy the same optical size even when their drawings differ in +// extent — a star that reaches the edges and a chevron that does not still +// look like siblings. Icons are therefore always drawn square; a caller that +// wants a different aspect should lay out around the icon, not stretch it. +// +// **Stroke width is in screen pixels, not view-box units.** Slint applies the +// stroke after fitting, so a fixed `stroke-width` would thicken as the icon +// shrank. `weight` scales it with `size` instead, which keeps a 10px tick and +// a 16px sidebar bar at the same apparent weight. It is also why the filled +// paths carry a stroke-width with a transparent stroke: the fit shrinks the +// drawing by the stroke width, so a filled star with no stroke width would +// come out fractionally *larger* than the outlined one beside it, and the two +// would jump as a rating changed. + +// One drawing. Everything an icon path shares lives here so the table below is +// nothing but geometry; `fill`, `stroke` and `stroke-width` come from the call +// site because they depend on the enclosing `Icon`'s properties. +component Ink inherits Path { + viewbox-width: 24; + viewbox-height: 24; + width: 100%; + height: 100%; + // Round joins throughout: at 12px a mitred corner on a 1.4px stroke is a + // single dark pixel that reads as dirt on the screen. + stroke-line-cap: round; + stroke-line-join: round; +} + +// A single icon, named. +// +// Names are strings rather than an enum because Slint enums cannot be extended +// from another file and the set is consumed across six screens; a typo shows +// up as a blank box, which is loud enough in review. The vocabulary is: +// +// check cross menu trash plus +// star star-outline +// chevron-right chevron-down chevron-left arrow-up +// rotate-ccw rotate-cw flip-h flip-v +// collection collection-smart +export component Icon inherits Rectangle { + in property name; + /// The single colour the whole drawing takes. Named `ink` rather than + /// `color` because Rectangle already has a legacy `color` alias for its + /// background and Slint refuses to shadow it. Icons are monochrome by + /// design — they sit in chrome that is already monochrome, and a two-tone + /// icon would be the only thing on screen carrying colour for decoration. + in property ink; + /// Edge length. Icons are square; see the view-box note above. + in property size: 16px; + + /// Stroke thickness, derived rather than set. The floor keeps the hairline + /// from disappearing entirely on a low-DPI desktop at the smallest sizes. + out property weight: max(1.2px, root.size / 11); + + width: root.size; + height: root.size; + horizontal-stretch: 0; + vertical-stretch: 0; + + // --- marks ----------------------------------------------------------- + + if root.name == "check": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 4.2 12.6 L 9.6 18 L 19.8 6.4"; + } + + if root.name == "cross": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 6.2 6.2 L 17.8 17.8 M 17.8 6.2 L 6.2 17.8"; + } + + if root.name == "plus": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 12 4.5 L 12 19.5 M 4.5 12 L 19.5 12"; + } + + // --- rating ---------------------------------------------------------- + // + // Solid and outline are the same ten points, so a filled star and an empty + // one sit at exactly the same size and position and the strip does not + // shimmer as a rating is dragged across it. + + if root.name == "star": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 12 3.1 L 14.32 9.21 L 20.85 9.53 L 15.76 13.63 " + + "L 17.46 19.94 L 12 16.35 L 6.54 19.94 L 8.24 13.63 " + + "L 3.15 9.53 L 9.68 9.21 Z"; + } + + if root.name == "star-outline": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 12 3.1 L 14.32 9.21 L 20.85 9.53 L 15.76 13.63 " + + "L 17.46 19.94 L 12 16.35 L 6.54 19.94 L 8.24 13.63 " + + "L 3.15 9.53 L 9.68 9.21 Z"; + } + + // --- chrome ---------------------------------------------------------- + + if root.name == "menu": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 3.5 6.5 L 20.5 6.5 M 3.5 12 L 20.5 12 " + + "M 3.5 17.5 L 20.5 17.5"; + } + + if root.name == "trash": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 9.2 6.2 L 9.2 3.8 L 14.8 3.8 L 14.8 6.2 " + + "M 3.4 6.2 L 20.6 6.2 " + + "M 5.6 6.2 L 6.7 20.2 L 17.3 20.2 L 18.4 6.2 " + + "M 10.2 9.8 L 10.6 16.8 M 13.8 9.8 L 13.4 16.8"; + } + + // Disclosure arrows are solid triangles rather than open chevrons: they + // are the smallest thing drawn in this UI, and at 11px a stroked chevron + // is two anti-aliased diagonals with nothing left of the shape between. + + if root.name == "chevron-right": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 9 5.5 L 16.5 12 L 9 18.5 Z"; + } + + if root.name == "chevron-left": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 15 5.5 L 7.5 12 L 15 18.5 Z"; + } + + if root.name == "chevron-down": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 5.5 9 L 18.5 9 L 12 16.5 Z"; + } + + if root.name == "arrow-up": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 12 20 L 12 5 M 5 12 L 12 5 L 19 12"; + } + + // --- framing --------------------------------------------------------- + // + // The rotation arrows are a 280° arc with a solid head at the end of + // travel. The gap sits at the top, where the eye looks first, so the two + // are told apart by which side the head is on rather than by following a + // curve — at 16px an arrowhead small enough to sit flush with a thin arc + // is a smudge, and both directions look like a plain circle. + + if root.name == "rotate-ccw": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 10.82 6.5 A 6.8 6.8 0 1 0 18.39 10.87"; + } + if root.name == "rotate-ccw": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 13.1 6.49 L 21.12 9.88 L 15.66 11.87 Z"; + } + + if root.name == "rotate-cw": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 13.18 6.5 A 6.8 6.8 0 1 1 5.61 10.87"; + } + if root.name == "rotate-cw": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 10.9 6.49 L 2.88 9.88 L 8.34 11.87 Z"; + } + + // A flip is a mirror about an axis, so the icon is literally that: two + // arrowheads facing away from the line they would be reflected across. + + if root.name == "flip-h": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 8.2 5.2 L 1.8 12 L 8.2 18.8 Z " + + "M 15.8 5.2 L 22.2 12 L 15.8 18.8 Z"; + } + if root.name == "flip-h": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 12 2.6 L 12 21.4"; + } + + if root.name == "flip-v": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 5.2 8.2 L 12 1.8 L 18.8 8.2 Z " + + "M 5.2 15.8 L 12 22.2 L 18.8 15.8 Z"; + } + if root.name == "flip-v": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 2.6 12 L 21.4 12"; + } + + // --- collections ----------------------------------------------------- + // + // A manual collection is a list of rows someone put there; a smart one is + // a rule. The shapes are deliberately unalike rather than one shape with a + // badge, because the pair appears down a narrow sidebar at 12px where a + // badge would be three pixels. + // + // Both are drawn thinner than they want to be for the same reason. At 12px + // the interior of a shape is four or five pixels across, so every internal + // line spends one of them and its two neighbours to anti-aliasing: the + // three-rule box this started as, and the diamond with a fat centre, both + // filled in to a solid blob. One rule and a small centre survive. + + if root.name == "collection": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 3.4 5.4 L 20.6 5.4 L 20.6 18.6 L 3.4 18.6 Z " + + "M 3.4 10.2 L 20.6 10.2"; + } + + if root.name == "collection-smart": Ink { + stroke: root.ink; + stroke-width: root.weight; + commands: "M 12 2.8 L 21.2 12 L 12 21.2 L 2.8 12 Z"; + } + if root.name == "collection-smart": Ink { + fill: root.ink; + stroke-width: root.weight; + commands: "M 12 9 L 15 12 L 12 15 L 9 12 Z"; + } +} diff --git a/ui/dr-ui/ui/launch.slint b/ui/dr-ui/ui/launch.slint index 40e4771..51e7fc0 100644 --- a/ui/dr-ui/ui/launch.slint +++ b/ui/dr-ui/ui/launch.slint @@ -1,5 +1,5 @@ import { Theme } from "theme.slint"; -import { Button, PanelHeading, Label, Value, Caption, Panel, Field, Disclosure } from "widgets.slint"; +import { Button, PanelHeading, Label, Value, Caption, Panel, Field, Disclosure, Icon } from "widgets.slint"; // Launch screen: connect an account, or resume a saved one. // @@ -41,18 +41,16 @@ component FormatCheck inherits Rectangle { border-color: root.checked ? Theme.active : Theme.rule; background: root.checked ? Theme.active : transparent; - Text { - text: "✓"; + 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. - color: Theme.ground; - font-size: 12px; + ink: Theme.ground; + size: 11px; visible: root.checked; - horizontal-alignment: center; - vertical-alignment: center; - width: 100%; - height: 100%; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } } @@ -89,7 +87,7 @@ component FolderRow inherits Rectangle { padding-right: Theme.gap; spacing: Theme.gap; - Disclosure { text: root.is-parent ? "↑" : "▸"; } + Disclosure { icon: root.is-parent ? "arrow-up" : "chevron-right"; } Value { text: root.label; overflow: elide; } } } @@ -261,7 +259,7 @@ export component LaunchScreen inherits Rectangle { } Caption { - text: "Create one in Nextcloud under Settings → Security → Devices & sessions. It is device-scoped and can be revoked on its own."; + text: "Create one in Nextcloud under Settings › Security › Devices & sessions. It is device-scoped and can be revoked on its own."; wrap: word-wrap; } } diff --git a/ui/dr-ui/ui/library.slint b/ui/dr-ui/ui/library.slint index 5e342a0..62658b2 100644 --- a/ui/dr-ui/ui/library.slint +++ b/ui/dr-ui/ui/library.slint @@ -9,7 +9,7 @@ // must not look identical (FR-NC-6c). import { Theme } from "theme.slint"; -import { Button, IconButton, Label, Value, Caption, EmptyState, FilterChip } from "widgets.slint"; +import { Button, IconButton, Label, Value, Caption, EmptyState, FilterChip, ProgressBar, Icon } from "widgets.slint"; // One bar of the capture-time histogram. export struct TimelineBar { @@ -259,52 +259,11 @@ export struct LibraryCell { flag: int, } -// A horizontal progress bar with two modes. -// -// Determinate where a real denominator exists (thumbnails: we know how many -// cells we asked for). Indeterminate where one does not — a directory walk -// discovers its own extent, so any percentage would be invented, and inventing -// one is worse than admitting the work is unbounded. -component ProgressBar inherits Rectangle { - in property fraction: 0; - in property indeterminate: false; - - height: 3px; - background: Theme.rule; - - // Determinate: a bar proportional to real progress. - Rectangle { - x: 0; - width: parent.width * clamp(root.fraction, 0, 1); - height: parent.height; - background: Theme.active-dim; - visible: !root.indeterminate; - } - - // Indeterminate: a sweep that says "working" without claiming a position. - Rectangle { - width: parent.width * 25%; - height: parent.height; - background: Theme.active-dim; - visible: root.indeterminate; - - x: root.indeterminate ? -self.width : 0; - animate x { - duration: 1200ms; - iteration-count: -1; - easing: ease-in-out; - } - states [ - running when root.indeterminate: { x: parent.width; } - ] - } -} - // A row of five stars, readable at a glance and clickable to set a rating. // // **Filled versus empty carries the meaning, not colour.** NFR-A11Y-3 forbids // status by hue alone, and the palette is achromatic anyway — so a set star is -// a solid glyph at `active` and an unset one is an outline at `ink-faint`. The +// a solid star at `active` and an unset one is an outline at `ink-faint`. The // two differ in both shape and luminance, which survives greyscale and low // vision alike. // @@ -343,7 +302,7 @@ export component StarStrip inherits Rectangle { // Separation between the trash target and ★1. // // **This gap is load-bearing.** The star targets are 44px over a 22px - // glyph, so they deliberately overlap and a near-miss lands one star out — + // cell, so they deliberately overlap and a near-miss lands one star out — // harmless, same control, corrected by clicking again. That reasoning does // not survive a neighbour that *moves a file*, so trash is held off the // scale by a gap wider than the overhang it would otherwise share with @@ -373,18 +332,16 @@ export component StarStrip inherits Rectangle { height: root.star; visible: root.can-trash; - Text { - text: "🗑"; + Icon { + name: "trash"; // Reject and trash are the two destructive ends of this UI and // share the palette's one hue, so the gesture reads the same - // in both places (NFR-A11Y-3: the glyph carries it, not the + // in both places (NFR-A11Y-3: the shape carries it, not the // colour). - color: Theme.warn-ink; - font-size: 13px; - width: 100%; - height: 100%; - horizontal-alignment: center; - vertical-alignment: center; + ink: Theme.warn-ink; + size: 13px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } TouchArea { @@ -411,18 +368,16 @@ export component StarStrip inherits Rectangle { width: root.star; height: root.star; - Text { + Icon { // Solid versus outline: the shape says it, not the colour. - text: root.rating >= n ? "★" : "☆"; - color: root.rating >= n ? Theme.active : Theme.ink-dim; - // Large enough to hit the difference between ★ and ☆ at arm's - // length on a tablet; the glyphs differ in fill, which needs - // more pixels to read than a difference in shape would. - font-size: 15px; - width: 100%; - height: 100%; - horizontal-alignment: center; - vertical-alignment: center; + name: root.rating >= n ? "star" : "star-outline"; + ink: root.rating >= n ? Theme.active : Theme.ink-dim; + // Large enough to hit the difference between filled and empty + // at arm's length on a tablet; the two differ in fill, which + // needs more pixels to read than a difference in shape would. + size: 14px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } // Grown past the drawn star to meet FR-UI-3's 44pt minimum, and @@ -453,10 +408,11 @@ export component StarStrip inherits Rectangle { // The pick/reject mark. // -// A glyph rather than a colour, for the same NFR-A11Y-3 reason as the stars: -// ✓ and ✗ are distinguishable without hue, and a reject reads as a reject in -// greyscale. A reject also dims its whole cell, which is the cue that carries -// at grid scale — the glyph is confirmation, not the primary signal. +// A shape rather than a colour, for the same NFR-A11Y-3 reason as the stars: +// a tick and a cross are distinguishable without hue, and a reject reads as a +// reject in greyscale. A reject also dims its whole cell, which is the cue +// that carries at grid scale — the mark is confirmation, not the primary +// signal. export component FlagMark inherits Rectangle { in property flag: 0; @@ -467,17 +423,14 @@ export component FlagMark inherits Rectangle { background: Theme.surface; opacity: 0.92; - Text { - text: root.flag == 2 ? "✗" : "✓"; + Icon { + name: root.flag == 2 ? "cross" : "check"; // Reject earns the one hue in the palette: it is the destructive end // of the axis and the thing a user must not mistake for a pick. - color: root.flag == 2 ? Theme.warn-ink : Theme.active; - font-size: 10px; - font-weight: 700; - width: 100%; - height: 100%; - horizontal-alignment: center; - vertical-alignment: center; + ink: root.flag == 2 ? Theme.warn-ink : Theme.active; + size: 9px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } } @@ -535,10 +488,9 @@ export component LibraryGrid inherits Rectangle { /// column-count change. Rust resizes the loaded window to match. callback capacity-changed(int); - // Thumbnail progress. Unlike the scan, this has a real denominator — the - // number of cells we asked for — so the bar can be honest about position. - in property thumbs-done: 0; - in property thumbs-total: 0; + // Thumbnail progress is no longer reported here: the batch belongs to a + // window of the grid rather than to anything the user asked for, and the + // shell's bar now carries it along with every other running job. // Whole-library indexing, which runs for far longer than one window's // thumbnails and is reported separately so the two do not fight over the @@ -549,9 +501,6 @@ export component LibraryGrid inherits Rectangle { in property syncing: false; property sweeping: root.sweep-total > 0 && root.sweep-done < root.sweep-total; - property thumbs-running: root.thumbs-total > 0 - && root.thumbs-done < root.thumbs-total; - callback cell-clicked(int); /// A star was clicked on a cell: row, and the rating 0..5. callback cell-rated(int, int); @@ -743,7 +692,7 @@ export component LibraryGrid inherits Rectangle { // interface with a collapsible sidebar puts one, and the only // place that stays put whichever way the panel is. IconButton { - glyph: "☰"; + icon: "menu"; active: root.collections-visible; y: (parent.height - self.height) / 2; clicked => { root.toggle-collections(); } @@ -825,7 +774,10 @@ export component LibraryGrid inherits Rectangle { // much more expensive request, and a button that meant either // depending on invisible state would be a trap. if root.scope-label != "": Button { - text: root.scope-pinned ? "Pinned ✓" : "Pin offline"; + // No tick on the pinned label: `active` already inverts + // the button, which says the same thing without a symbol + // inside a string (see icons.slint). + text: root.scope-pinned ? "Pinned" : "Pin offline"; active: root.scope-pinned; enabled: !root.scanning; y: (parent.height - self.height) / 2; @@ -908,7 +860,8 @@ export component LibraryGrid inherits Rectangle { } for n[i] in [1, 2, 3, 4, 5]: FilterChip { - label: "★" + n + "+"; + icon: "star"; + label: n + "+"; count: root.rating-counts.length > n ? root.rating-counts[n] : -1; active: root.filter-min-rating == n; y: (parent.height - self.height) / 2; @@ -923,7 +876,8 @@ export component LibraryGrid inherits Rectangle { Rectangle { width: Theme.gap; } FilterChip { - label: "✓ Picks"; + icon: "check"; + label: "Picks"; active: root.filter-flag == 1; y: (parent.height - self.height) / 2; clicked => { @@ -932,7 +886,8 @@ export component LibraryGrid inherits Rectangle { } FilterChip { - label: "✗ Rejects"; + icon: "cross"; + label: "Rejects"; active: root.filter-flag == 2; y: (parent.height - self.height) / 2; clicked => { @@ -978,23 +933,20 @@ export component LibraryGrid inherits Rectangle { // --- progress ----------------------------------------------------- // - // Indeterminate during the scan: a directory walk cannot know its own - // extent, so a percentage would be fiction. Determinate for - // thumbnails, where the denominator is the cells we requested. - if root.scanning || root.thumbs-running || root.sweeping: ProgressBar { - // Only the sweep has a denominator worth showing. A scan cannot - // know its extent, and the window's own fetches are sized by the - // viewport rather than by anything meaningful to the user. - indeterminate: root.scanning || (!root.sweeping && root.thumbs-running); - fraction: root.sweep-total > 0 - ? root.sweep-done / root.sweep-total - : 0; - } + // The bar that used to sit here is now the shell's, drawn across the + // top of every view from the activity register (see `activity.rs`). + // Two reasons it moved. It only ever knew about the three things the + // grid happens to report — a download running while the user was in + // develop drew nothing anywhere — and a second bar here would now say + // the same thing twice, one line apart. + // + // The grid keeps its *words*: "indexing 4000 / 17000" in the header + // above says which work is running, which a bar cannot. // --- pinning ------------------------------------------------------ // - // Its own line rather than the shared progress bar above: that one is - // driven by the scan and the sweep, and a pin runs alongside both. + // Its own line, with the count spelled out: the shell's bar says that + // something is transferring, and this says how much of what. if root.pin-total > 0: Rectangle { height: 34px; background: Theme.surface; @@ -1464,8 +1416,8 @@ export component LibraryGrid inherits Rectangle { // hidden: the cull is reversible, and a photo // that vanished on one keypress would make the // gesture frightening to use. Dimming is the - // cue that reads at grid scale — the ✗ glyph - // confirms it up close. + // cue that reads at grid scale — the cross on + // the mark confirms it up close. opacity: cell.lifted ? 0.25 : (cell.flag == 2 ? 0.4 : 1.0); animate opacity { duration: 120ms; } diff --git a/ui/dr-ui/ui/settings.slint b/ui/dr-ui/ui/settings.slint index 45706ee..a7bf951 100644 --- a/ui/dr-ui/ui/settings.slint +++ b/ui/dr-ui/ui/settings.slint @@ -1,5 +1,5 @@ import { Theme } from "theme.slint"; -import { Button, PanelHeading, Label, Value, Caption, Panel, Field } from "widgets.slint"; +import { Button, PanelHeading, Label, Value, Caption, Panel, Field, ProgressBar, ActivityRow, Icon } from "widgets.slint"; // Settings: how much disk the app may spend, and what an export defaults to. // @@ -146,15 +146,13 @@ component Switch inherits Rectangle { border-color: root.checked ? Theme.active : Theme.rule; background: root.checked ? Theme.active : transparent; - Text { - text: "✓"; - color: Theme.ground; - font-size: 12px; + Icon { + name: "check"; + ink: Theme.ground; + size: 11px; visible: root.checked; - horizontal-alignment: center; - vertical-alignment: center; - width: 100%; - height: 100%; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } } @@ -238,7 +236,60 @@ component EntryRow inherits VerticalLayout { } } +// 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, +// because the list is built by a `for` and every row has to answer the same +// three questions in the same order — otherwise a failure reads as a different +// kind of thing from the job it happened to. +// +// The bar is drawn only while the job runs. A stopped job's fraction is either +// 1 (it finished, and a full bar is noise) or frozen part-way (it failed, and +// a bar sitting at 60% invites the reading "still going"). +component ActivityItem inherits VerticalLayout { + in property row; + + spacing: 4px; + + HorizontalLayout { + spacing: Theme.gap; + + Label { + text: root.row.title; + body: true; + // Faded once it is over: the list is ordered running-first, and + // this is what makes that ordering visible at a glance rather than + // something the reader has to work out from the text. + opacity: root.row.running ? 1.0 : 0.6; + } + + Caption { + text: root.row.detail; + // A failure is the one thing here worth a colour (NFR-A11Y-3 is + // satisfied by the words, which say what failed; the hue only + // finds them faster). + warn: root.row.failed; + horizontal-alignment: right; + horizontal-stretch: 1; + overflow: elide; + } + } + + if root.row.running: ProgressBar { + indeterminate: !root.row.determinate; + fraction: root.row.fraction; + } +} + export component SettingsPage inherits Rectangle { + // --- background activity -------------------------------------------- + in property <[ActivityRow]> activity-rows; + in property activity-running: 0; + /// Stopped jobs still listed: recent successes, and failures held until + /// they are read. + in property activity-kept: 0; + callback clear-finished(); + // --- cache --------------------------------------------------------- in-out property original-budget; in property original-unlimited: false; @@ -388,6 +439,68 @@ export component SettingsPage inherits Rectangle { // `min` so a narrow window still uses what it has (FR-UI-1). property column: min(root.width - 2 * Theme.gap-lg, 680px); + // --- background activity --------------------------------- + // + // First on the page, and the only panel here that is not a + // preference. It earns the position by being the one thing + // that changes while the page is open: a transfer the user + // came here to check on is answered before they have scrolled. + Rectangle { + width: content.column; + height: activity.preferred-height; + + activity := Panel { + width: 100%; + + HorizontalLayout { + spacing: Theme.gap; + + PanelHeading { text: "BACKGROUND ACTIVITY"; } + + // The count beside the heading, because the list + // below carries stopped jobs too and "how many are + // running" should not have to be counted by eye. + Caption { + text: root.activity-running > 0 + ? root.activity-running + " running" : ""; + horizontal-alignment: right; + horizontal-stretch: 1; + } + } + + Caption { + text: "Scans, transfers and syncs running behind the " + + "interface. Everything here survives leaving " + + "this page; nothing here needs it open."; + wrap: word-wrap; + } + + Rectangle { height: Theme.gap-sm; } + + // Says nothing is running, rather than showing an empty + // box: a list that is empty because the work finished + // and one that is empty because the register is broken + // look identical otherwise. + if root.activity-rows.length == 0: Caption { + text: "Nothing running."; + } + + for row in root.activity-rows: ActivityItem { + row: row; + } + + if root.activity-kept > 0: Rectangle { + height: Theme.control-height; + + Button { + x: 0; + text: "Clear finished"; + clicked => { root.clear-finished(); } + } + } + } + } + // --- storage --------------------------------------------- Rectangle { width: content.column; diff --git a/ui/dr-ui/ui/widgets.slint b/ui/dr-ui/ui/widgets.slint index c1a0448..342a952 100644 --- a/ui/dr-ui/ui/widgets.slint +++ b/ui/dr-ui/ui/widgets.slint @@ -14,6 +14,9 @@ // colour reached forty call sites with no single place to change it. import { Theme } from "theme.slint"; +import { Icon } from "icons.slint"; + +export { Icon } // A text button. // @@ -36,7 +39,7 @@ export component Button inherits Rectangle { /// emphasis stops meaning anything (see the theme preamble). in property primary: false; /// Sustained state — a toggle that is currently on, not a press. The same - /// meaning [`IconButton`] gives it, so a labelled toggle and a glyph + /// meaning [`IconButton`] gives it, so a labelled toggle and an icon /// toggle read alike. in property active: false; @@ -97,12 +100,13 @@ export component Button inherits Rectangle { } } -// A square button carrying a single glyph. +// A square button carrying a single icon. // // Square because it has no label to size against: a toolbar affordance whose -// width tracked its glyph would jitter as the glyph changed. +// width tracked its drawing would jitter as the drawing changed. export component IconButton inherits Rectangle { - in property glyph; + /// A name from the [`Icon`] vocabulary. + in property icon; in property enabled: true; /// Sustained state — a toggle that is currently on, not a press. in property active: false; @@ -131,14 +135,15 @@ export component IconButton inherits Rectangle { clicked => { root.clicked(); } } - Text { - text: root.glyph; - color: root.active ? Theme.active : Theme.ink; - font-size: Theme.text; - horizontal-alignment: center; - vertical-alignment: center; - width: 100%; - height: 100%; + Icon { + name: root.icon; + ink: root.active ? Theme.active : Theme.ink; + // Half the button, near enough: the icon carries the whole meaning of + // the control, so it wants more of the square than a glyph set at + // body size used to take, but it must not touch the border. + size: root.height / 2; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; } } @@ -164,6 +169,11 @@ export component FilterChip inherits Rectangle { in property active: false; /// Images behind this term, or -1 where the count is not known. in property count: -1; + /// A mark before the label, naming what the term filters on — a star for a + /// rating, a tick for picks. Empty for chips whose word says it already. + /// Drawn rather than prefixed to `label`, because a symbol inside a string + /// is exactly the thing [`Icon`] exists to stop (see icons.slint). + in property icon; callback clicked(); @@ -195,6 +205,16 @@ export component FilterChip inherits Rectangle { padding-right: Theme.gap-sm; spacing: 4px; + if root.icon != "": Icon { + name: root.icon; + // Takes the label's colour, including the inversion on an active + // chip: a mark that stayed light on the near-white fill would be + // the one invisible thing in the row. + ink: root.active ? Theme.ground : Theme.ink; + size: 11px; + y: (parent.height - self.height) / 2; + } + Text { text: root.label; // Dark on the active fill, which is near-white — the same @@ -221,17 +241,26 @@ export component FilterChip inherits Rectangle { // The arrow beside a row that opens into something: a disclosure triangle on // a section, an "into this folder" marker in the picker. // -// **Fixed width, and that is the whole point.** The glyphs differ in advance -// width, so a row that sized to its own arrow would shift its label sideways -// as it opened and closed — the one movement that makes a static list look -// like it is being redrawn. Rotating a single glyph would need a transform on -// a Text; two characters render identically and cost nothing. -export component Disclosure inherits Text { - color: Theme.ink-faint; - font-size: Theme.text-sm; - vertical-alignment: center; - horizontal-alignment: center; +// **Fixed width, and that is the whole point.** The drawings differ in extent, +// so a row that sized to its own arrow would shift its label sideways as it +// opened and closed — the one movement that makes a static list look like it +// is being redrawn. The box stays 14px whatever is in it, and the icon centres +// inside. +export component Disclosure inherits Rectangle { + /// A name from the [`Icon`] vocabulary — `chevron-right` closed, + /// `chevron-down` open, or `arrow-up` for a row that leads back out. + in property icon: "chevron-right"; + width: 14px; + horizontal-stretch: 0; + + Icon { + name: root.icon; + ink: Theme.ink-faint; + size: 10px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; + } } // A collapsible group with a header that reports whether anything inside has @@ -305,7 +334,9 @@ export component Section inherits Rectangle { padding-right: Theme.gap-sm; spacing: Theme.gap-sm; - Disclosure { text: root.expanded ? "▾" : "▸"; } + Disclosure { + icon: root.expanded ? "chevron-down" : "chevron-right"; + } Text { text: root.title; @@ -609,6 +640,79 @@ export component Field inherits Rectangle { } } +// A horizontal progress bar with two modes. +// +// Determinate where a real denominator exists (thumbnails: we know how many +// cells we asked for). Indeterminate where one does not — a directory walk +// discovers its own extent, so any percentage would be invented, and inventing +// one is worse than admitting the work is unbounded. +// +// Lives here rather than in library.slint, where it started, because the grid +// is no longer the only view that reports progress: the shell draws one across +// the top of every view and the settings page draws one per running job. +export component ProgressBar inherits Rectangle { + in property fraction: 0; + in property indeterminate: false; + + height: 3px; + background: Theme.rule; + // The sweep below is positioned outside these bounds for half its cycle. + // Without clipping it paints over whatever sits beside the bar, which at + // the top of the shell is the whole window. + clip: true; + + // Determinate: a bar proportional to real progress. + Rectangle { + x: 0; + width: parent.width * clamp(root.fraction, 0, 1); + height: parent.height; + background: Theme.active-dim; + visible: !root.indeterminate; + } + + // Indeterminate: a sweep that says "working" without claiming a position. + Rectangle { + width: parent.width * 25%; + height: parent.height; + background: Theme.active-dim; + visible: root.indeterminate; + + x: root.indeterminate ? -self.width : 0; + animate x { + duration: 1200ms; + iteration-count: -1; + easing: ease-in-out; + } + states [ + running when root.indeterminate: { x: parent.width; } + ] + } +} + +// One background job, as the interface sees it. +// +// Built in Rust by `activity.rs`, which is the only thing that knows a scan +// from a download. Everything here is already a sentence or a number ready to +// draw: the page decides where a row goes, never what it means. +export struct ActivityRow { + // "Scanning Photos", "Keeping 40 photographs offline". + title: string, + // Whatever the job last said about itself — counts, a byte figure, or the + // error where it failed. + detail: string, + // 0..1, meaningless unless `determinate`. + fraction: float, + // Whether this job knows its own extent. A scan does not. + determinate: bool, + running: bool, + // Kept apart from `running`: a finished job and a failed one are both + // stopped, and only one of them is worth the user's attention. + failed: bool, + // Moves bytes over the network, so it is one of the "transfers" the user + // asks about when the connection is slow (FR-NC-6c). + transfer: bool, +} + // What a view says when it has nothing to show. // // Not in the S3 brief, but `app.slint` and `library.slint` had the same two