diff --git a/Cargo.lock b/Cargo.lock index f59a50a..fd3c095 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1166,6 +1166,16 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d8b14ccef22fc6f5a8f4d7d768562a182c04ce9a3b3157b91390b52ddfdf1a76" +[[package]] +name = "dr-catalog" +version = "0.1.0" +dependencies = [ + "dr-types", + "log", + "rusqlite", + "thiserror 2.0.20", +] + [[package]] name = "dr-decode" version = "0.1.0" @@ -1209,6 +1219,7 @@ dependencies = [ "dr-types", "log", "thiserror 2.0.20", + "tokio", ] [[package]] @@ -1455,6 +1466,18 @@ dependencies = [ "zune-inflate", ] +[[package]] +name = "fallible-iterator" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" + +[[package]] +name = "fallible-streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" + [[package]] name = "fastrand" version = "2.5.0" @@ -2069,6 +2092,15 @@ dependencies = [ "foldhash 0.2.0", ] +[[package]] +name = "hashlink" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32069d97bb81e38fa67eab65e3393bf804bb85969f2bc06bf13f64aef5aba248" +dependencies = [ + "hashbrown 0.17.1", +] + [[package]] name = "heck" version = "0.5.0" @@ -3239,6 +3271,17 @@ dependencies = [ "redox_syscall 0.9.1", ] +[[package]] +name = "libsqlite3-sys" +version = "0.38.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1d20bef17f513b9b3004532233187769cd072d790971f4e4da0e346eb6401e8" +dependencies = [ + "cc", + "pkg-config", + "vcpkg", +] + [[package]] name = "libudev-sys" version = "0.1.4" @@ -4923,6 +4966,31 @@ dependencies = [ "unicode-width 0.2.2", ] +[[package]] +name = "rsqlite-vfs" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c51c9ae4df8a7fba42103df5c621fa3c37eccf3a3c650879e90fc48b11cc192c" +dependencies = [ + "hashbrown 0.16.1", + "thiserror 2.0.20", +] + +[[package]] +name = "rusqlite" +version = "0.40.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23f2a97da3e3873c73cb2a2e71b35c40ff95e0b1eefa8d72d8499a6928c3b5b3" +dependencies = [ + "bitflags 2.13.1", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "smallvec", + "sqlite-wasm-rs", +] + [[package]] name = "rustc-demangle" version = "0.1.28" @@ -5568,6 +5636,18 @@ dependencies = [ "bitflags 2.13.1", ] +[[package]] +name = "sqlite-wasm-rs" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc3efc0da82635d7e1ced0053bbbfa8c7ab9645d0bf36ceb4f7127bb85315d75" +dependencies = [ + "cc", + "js-sys", + "rsqlite-vfs", + "wasm-bindgen", +] + [[package]] name = "stable_deref_trait" version = "1.2.1" @@ -6333,6 +6413,12 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "vcpkg" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" + [[package]] name = "version_check" version = "0.9.5" diff --git a/Cargo.toml b/Cargo.toml index 6c58e38..716a936 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,7 @@ resolver = "2" members = [ "core/dr-types", + "core/dr-catalog", "core/dr-decode", "core/dr-gpu", "core/dr-pipeline", @@ -22,6 +23,7 @@ repository = "https://github.com/dtourolle/DarkRoom" [workspace.dependencies] # Internal dr-types = { path = "core/dr-types" } +dr-catalog = { path = "core/dr-catalog" } dr-decode = { path = "core/dr-decode" } dr-gpu = { path = "core/dr-gpu" } dr-pipeline = { path = "core/dr-pipeline" } @@ -63,6 +65,15 @@ base64 = "0.23" # Decode. rawler is the pure-Rust decoder (D2); zune-jpeg decodes the # embedded previews rawler extracts. +# Catalog. `bundled` compiles SQLite from source rather than linking the +# system library — the same cross-compilation reasoning as the TLS choice +# above: no system dependency to satisfy under the Android NDK. +# +# `backup` is not optional in practice: it is what takes a consistent snapshot +# of a live WAL database for upload. A filesystem copy of `catalog.sqlite` +# while a `-wal` exists beside it uploads a torn file. +rusqlite = { version = "0.40", features = ["bundled", "backup"] } + rawler = "0.7" zune-jpeg = "0.4.21" bytemuck = { version = "1", features = ["derive"] } diff --git a/core/dr-catalog/Cargo.toml b/core/dr-catalog/Cargo.toml new file mode 100644 index 0000000..7137e36 --- /dev/null +++ b/core/dr-catalog/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "dr-catalog" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true + +[dependencies] +dr-types.workspace = true +rusqlite.workspace = true +thiserror.workspace = true +log.workspace = true diff --git a/core/dr-catalog/src/error.rs b/core/dr-catalog/src/error.rs new file mode 100644 index 0000000..080ae0e --- /dev/null +++ b/core/dr-catalog/src/error.rs @@ -0,0 +1,41 @@ +//! TRACES: NFR-ARCH-4 | NFR-R5 +//! Catalog errors. +//! +//! Typed and attached to the affected subject rather than panicking — a +//! corrupt row or a failed job marks one image and lets the batch continue. + +/// Something went wrong talking to the catalog. +#[derive(Debug, thiserror::Error)] +pub enum CatalogError { + #[error("sqlite: {0}")] + Sqlite(#[from] rusqlite::Error), + + /// The catalog was written by a newer build. + /// + /// Opening it read-write would corrupt state this build cannot represent, + /// so the app refuses and says so (NFR-R5). + #[error("catalog schema v{found} is newer than this build supports (v{supported})")] + SchemaTooNew { found: i64, supported: i64 }, + + /// A scan could not reach a root at all. + /// + /// Distinct from "files are missing": this aborts the scan *before* the + /// deletion sweep, because every folder would look unreached and the sweep + /// would delete the whole library (FR-CAT-9). + #[error("root {0} is unreachable; scan aborted without pruning")] + RootUnreachable(u64), + + /// A smart collection whose selector references itself, directly or via + /// another collection. + #[error("collection {0} would form a cycle")] + CollectionCycle(u64), + + #[error("no such collection: {0}")] + NoSuchCollection(u64), + + #[error("malformed stored selector: {0}")] + BadSelector(String), + + #[error("io: {0}")] + Io(String), +} diff --git a/core/dr-catalog/src/jobs.rs b/core/dr-catalog/src/jobs.rs new file mode 100644 index 0000000..80ddad5 --- /dev/null +++ b/core/dr-catalog/src/jobs.rs @@ -0,0 +1,392 @@ +//! TRACES: FR-CAT-3 | NFR-ARCH-2 | FR-PLAT-AND-3 +//! The background work queue. +//! +//! Jobs live in the catalog, so they survive process death — routine on +//! Android rather than exceptional (FR-PLAT-AND-3). Two properties carry the +//! design: +//! +//! - **Coalescing.** `UNIQUE(kind, subject_id)` makes enqueueing idempotent, +//! so every code path that notices a change can just enqueue and let the +//! table absorb the redundancy. +//! - **Priority shared with the GPU scheduler** (ARCH §5.3), so one notion of +//! urgency governs the whole app and visible work always preempts bulk work. + +use rusqlite::Connection; + +use crate::error::CatalogError; + +/// What a job does. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(i64)] +pub enum JobKind { + /// Recursive incremental scan from a folder (§scan). + ScanFolder = 0, + /// Promote an image from stat-only to full EXIF. + ExtractMetadata = 1, + /// Build or rebuild a thumbnail. + Thumbnail = 2, + /// A sidecar on disk is newer than what the catalog read. + ReadSidecar = 3, + /// Flush a local edit to its sidecar. Debounced, never per slider tick. + WriteSidecar = 4, + /// Whole-file hash. On demand only — import dedup, reconnect-by-hash. + ContentHash = 5, + /// Range-extract an embedded preview from a remote file (FR-NC-3). + FetchPreview = 6, + /// Fetch a full original: pinned by rule, or explicitly asked for. + FetchOriginal = 7, +} + +impl JobKind { + fn from_i64(v: i64) -> Option { + Some(match v { + 0 => JobKind::ScanFolder, + 1 => JobKind::ExtractMetadata, + 2 => JobKind::Thumbnail, + 3 => JobKind::ReadSidecar, + 4 => JobKind::WriteSidecar, + 5 => JobKind::ContentHash, + 6 => JobKind::FetchPreview, + 7 => JobKind::FetchOriginal, + _ => return None, + }) + } + + /// Whether this job transfers over the network, and so is subject to the + /// metered-connection and charging constraints in FR-NC-6. + pub fn is_network(self) -> bool { + matches!(self, JobKind::FetchPreview | JobKind::FetchOriginal) + } +} + +/// Scheduling class, matching the GPU tile scheduler (ARCH §5.3). +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[repr(i64)] +pub enum Priority { + /// Bulk work: metadata sweeps, rule-driven fetches, hashing. + Background = 0, + /// Just outside the viewport; the next image in culling. + Prefetch = 1, + /// Visible cells, and the image currently open. + /// + /// Strictly preempts background work. Without this, scrolling during a + /// bulk thumbnail pass misses its frame budget — the common case, not an + /// edge case (NFR-ARCH-2). + Interactive = 2, +} + +/// Lifecycle state. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(i64)] +pub enum JobState { + Pending = 0, + Running = 1, + Failed = 2, +} + +/// A job ready to run. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Job { + pub id: i64, + pub kind: JobKind, + pub subject_id: Option, + pub priority: Priority, + pub attempts: i64, + pub payload: Option, +} + +/// Give up after this many attempts and attach the error to the subject. +/// +/// One corrupt file must not stall the queue behind endless retries +/// (FR-RAW-4). +pub const MAX_ATTEMPTS: i64 = 5; + +/// Backoff before retrying a failed job, in seconds. +/// +/// Exponential, capped — a server that is down for an hour should not be +/// retried every second, and a transient decode failure should not wait an +/// hour. +pub fn backoff_seconds(attempts: i64) -> i64 { + const CAP: i64 = 300; + match attempts { + a if a <= 0 => 0, + a if a >= 9 => CAP, + a => (1i64 << (a - 1)).min(CAP), + } +} + +/// Enqueue work, coalescing with any identical pending job. +/// +/// Re-requesting at a higher priority *promotes* the existing row rather than +/// duplicating it, which is what lets the grid shout "this one is visible now" +/// about a job already queued in the background. +pub fn enqueue( + conn: &Connection, + kind: JobKind, + subject_id: Option, + priority: Priority, + payload: Option<&str>, +) -> Result<(), CatalogError> { + conn.execute( + "INSERT INTO jobs(kind, subject_id, priority, state, payload) + VALUES (?1, ?2, ?3, 0, ?4) + ON CONFLICT(kind, subject_id) DO UPDATE SET + priority = max(jobs.priority, excluded.priority), + -- A job that failed and is being re-requested deserves a fresh + -- start: the file may well have changed since it failed. + state = CASE WHEN jobs.state = 2 THEN 0 ELSE jobs.state END, + attempts = CASE WHEN jobs.state = 2 THEN 0 ELSE jobs.attempts END, + not_before = CASE WHEN jobs.state = 2 THEN 0 ELSE jobs.not_before END", + rusqlite::params![kind as i64, subject_id, priority as i64, payload], + )?; + Ok(()) +} + +/// Claim the next runnable job, highest priority first. +/// +/// `now` is passed rather than read from the clock so backoff is testable. +/// Claiming marks the row `Running` in the same transaction as the read, so +/// two workers cannot take the same job. +pub fn claim_next(conn: &Connection, now: i64) -> Result, CatalogError> { + let tx = conn.unchecked_transaction()?; + + let job = tx + .query_row( + "SELECT id, kind, subject_id, priority, attempts, payload + FROM jobs + WHERE state = 0 AND not_before <= ?1 + ORDER BY priority DESC, id ASC + LIMIT 1", + [now], + |r| { + Ok(( + r.get::<_, i64>(0)?, + r.get::<_, i64>(1)?, + r.get::<_, Option>(2)?, + r.get::<_, i64>(3)?, + r.get::<_, i64>(4)?, + r.get::<_, Option>(5)?, + )) + }, + ) + .ok(); + + let Some((id, kind, subject_id, priority, attempts, payload)) = job else { + return Ok(None); + }; + + tx.execute( + "UPDATE jobs SET state = 1, attempts = attempts + 1 WHERE id = ?1", + [id], + )?; + tx.commit()?; + + Ok(Some(Job { + id, + kind: JobKind::from_i64(kind).unwrap_or(JobKind::ExtractMetadata), + subject_id, + priority: match priority { + 2 => Priority::Interactive, + 1 => Priority::Prefetch, + _ => Priority::Background, + }, + attempts: attempts + 1, + payload, + })) +} + +/// Job finished successfully. +pub fn complete(conn: &Connection, id: i64) -> Result<(), CatalogError> { + conn.execute("DELETE FROM jobs WHERE id = ?1", [id])?; + Ok(()) +} + +/// Job failed. Reschedules with backoff, or gives up past [`MAX_ATTEMPTS`]. +pub fn fail(conn: &Connection, job: &Job, now: i64, err: &str) -> Result<(), CatalogError> { + if job.attempts >= MAX_ATTEMPTS { + conn.execute( + "UPDATE jobs SET state = 2, last_error = ?2 WHERE id = ?1", + rusqlite::params![job.id, err], + )?; + } else { + conn.execute( + "UPDATE jobs SET state = 0, not_before = ?2, last_error = ?3 WHERE id = ?1", + rusqlite::params![job.id, now + backoff_seconds(job.attempts), err], + )?; + } + Ok(()) +} + +/// Recover jobs orphaned by process death. +/// +/// A row left `Running` has no owner — the process that claimed it is gone. +/// Called at startup, before any worker begins (FR-PLAT-AND-3). +pub fn recover_orphaned(conn: &Connection) -> Result { + let n = conn.execute("UPDATE jobs SET state = 0 WHERE state = 1", [])?; + Ok(n) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::schema; + + fn db() -> Connection { + let c = Connection::open_in_memory().unwrap(); + schema::configure(&c).unwrap(); + schema::migrate(&c).unwrap(); + c + } + + #[test] + fn repeated_enqueue_coalesces() { + let c = db(); + for _ in 0..10 { + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + } + let n: i64 = c + .query_row("SELECT count(*) FROM jobs", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 1); + } + + #[test] + fn re_enqueueing_at_higher_priority_promotes() { + let c = db(); + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + // The grid scrolls this image into view. + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Interactive, None).unwrap(); + + let p: i64 = c + .query_row("SELECT priority FROM jobs", [], |r| r.get(0)) + .unwrap(); + assert_eq!(p, Priority::Interactive as i64); + } + + #[test] + fn priority_never_regresses() { + let c = db(); + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Interactive, None).unwrap(); + // A background sweep must not demote work the user is waiting on. + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + + let p: i64 = c + .query_row("SELECT priority FROM jobs", [], |r| r.get(0)) + .unwrap(); + assert_eq!(p, Priority::Interactive as i64); + } + + #[test] + fn claim_takes_highest_priority_first() { + let c = db(); + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + enqueue(&c, JobKind::Thumbnail, Some(2), Priority::Interactive, None).unwrap(); + enqueue(&c, JobKind::Thumbnail, Some(3), Priority::Prefetch, None).unwrap(); + + let first = claim_next(&c, 0).unwrap().unwrap(); + assert_eq!(first.subject_id, Some(2)); + let second = claim_next(&c, 0).unwrap().unwrap(); + assert_eq!(second.subject_id, Some(3)); + } + + #[test] + fn a_claimed_job_is_not_claimed_twice() { + let c = db(); + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + assert!(claim_next(&c, 0).unwrap().is_some()); + assert!(claim_next(&c, 0).unwrap().is_none()); + } + + #[test] + fn failure_backs_off_then_becomes_claimable_again() { + let c = db(); + enqueue( + &c, + JobKind::FetchPreview, + Some(1), + Priority::Background, + None, + ) + .unwrap(); + let job = claim_next(&c, 100).unwrap().unwrap(); + fail(&c, &job, 100, "network down").unwrap(); + + // Still backing off. + assert!(claim_next(&c, 100).unwrap().is_none()); + // Past the backoff. + assert!(claim_next(&c, 100 + backoff_seconds(job.attempts)) + .unwrap() + .is_some()); + } + + #[test] + fn a_persistently_failing_job_stops_retrying() { + let c = db(); + enqueue( + &c, + JobKind::ExtractMetadata, + Some(1), + Priority::Background, + None, + ) + .unwrap(); + + let mut now = 0; + for _ in 0..MAX_ATTEMPTS { + let job = claim_next(&c, now).unwrap().expect("should be claimable"); + fail(&c, &job, now, "corrupt file").unwrap(); + now += backoff_seconds(job.attempts); + } + + // One corrupt file must not stall the queue forever (FR-RAW-4). + assert!(claim_next(&c, now + 100_000).unwrap().is_none()); + let state: i64 = c + .query_row("SELECT state FROM jobs", [], |r| r.get(0)) + .unwrap(); + assert_eq!(state, JobState::Failed as i64); + } + + #[test] + fn re_requesting_a_failed_job_gives_it_a_fresh_start() { + let c = db(); + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + let mut now = 0; + for _ in 0..MAX_ATTEMPTS { + let job = claim_next(&c, now).unwrap().unwrap(); + fail(&c, &job, now, "boom").unwrap(); + now += backoff_seconds(job.attempts); + } + // The file changed on disk, so the old failure says nothing about it. + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Interactive, None).unwrap(); + let job = claim_next(&c, now).unwrap().expect("retryable again"); + assert_eq!(job.attempts, 1); + } + + #[test] + fn orphaned_jobs_return_to_pending_on_restart() { + let c = db(); + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + claim_next(&c, 0).unwrap().unwrap(); + // Process dies here. Android does this routinely. + assert_eq!(recover_orphaned(&c).unwrap(), 1); + assert!(claim_next(&c, 0).unwrap().is_some()); + } + + #[test] + fn backoff_grows_then_caps() { + assert_eq!(backoff_seconds(0), 0); + assert_eq!(backoff_seconds(1), 1); + assert_eq!(backoff_seconds(3), 4); + assert_eq!(backoff_seconds(100), 300); + } + + #[test] + fn network_jobs_are_identifiable_for_metered_gating() { + // FR-NC-6: transfers respect unmetered-network and charging + // constraints; local work must not be gated by them. + assert!(JobKind::FetchOriginal.is_network()); + assert!(JobKind::FetchPreview.is_network()); + assert!(!JobKind::Thumbnail.is_network()); + assert!(!JobKind::ExtractMetadata.is_network()); + } +} diff --git a/core/dr-catalog/src/lib.rs b/core/dr-catalog/src/lib.rs new file mode 100644 index 0000000..587bc62 --- /dev/null +++ b/core/dr-catalog/src/lib.rs @@ -0,0 +1,374 @@ +//! TRACES: FR-CAT-2 | FR-CAT-4 | FR-CAT-6 | NFR-P1 +//! The catalog: a rebuildable index over the library. +//! +//! Not a source of truth. Sidecars next to the images hold the authoritative +//! edit state (ARCH §6.12), and this file is deletable at any time — rebuilt +//! by rescanning sources and reading sidecars. That inversion is deliberate: +//! darktable maintains both a database and sidecars while achieving the +//! reliability of neither. +//! +//! # What lives here +//! +//! - [`schema`] — tables and forward-only migrations +//! - [`scan`] — incremental discovery that prunes unchanged directories +//! - [`query`] — selectors compiled to indexed SQL, windowed for the grid +//! - [`jobs`] — the durable background work queue +//! - [`merge`] / [`sync`] — cross-device collection merging +//! +//! # The one thing everything is designed around +//! +//! **Work is proportional to what changed, or to what the user is looking at — +//! never to library size.** A 50k-image library that has not changed costs one +//! metadata probe per folder to verify (§scan), no thumbnails to regenerate +//! (§jobs coalescing), and no rule evaluation per grid cell (materialised +//! `tier_desired`). + +use std::path::Path; + +use dr_types::{Availability, ImageId}; +use rusqlite::Connection; + +pub mod error; +pub mod jobs; +pub mod merge; +pub mod query; +pub mod scan; +pub mod schema; +pub mod sync; + +pub use error::CatalogError; +pub use jobs::{Job, JobKind, Priority}; +pub use merge::MergeReport; +pub use query::{Query, Sort}; +pub use scan::{DirAction, DirState, EntryAction, ScanOutcome}; + +/// One row of the library grid. +/// +/// Exactly what a cell draws and nothing more — no join per cell, and +/// availability reads a materialised column rather than evaluating cache rules +/// (ARCH §9.5). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GridRow { + pub id: ImageId, + pub name: String, + pub availability: Availability, + /// UTC seconds. `None` until EXIF has been read. + pub captured_at: Option, + /// Minutes east of UTC, for rendering the photographer's local time. + pub captured_offset: Option, + /// 0 = nothing, 1 = stat-only, 2 = full EXIF. + pub metadata_state: u8, +} + +/// A count of images in one time bucket, for the timeline scrubber. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct TimeBucket { + /// UTC seconds at the bucket's start. + pub start: i64, + pub count: u32, +} + +/// Time bucket size. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Granularity { + Year, + Month, + Day, + Hour, +} + +impl Granularity { + /// SQLite `strftime` format that collapses a timestamp to this bucket. + /// + /// Applied to **local** time, not UTC: "everything from 3 August" means + /// the photographer's 3 August, which is why `captured_offset` is stored + /// alongside the UTC timestamp. + fn strftime(self) -> &'static str { + match self { + Granularity::Year => "%Y", + Granularity::Month => "%Y-%m", + Granularity::Day => "%Y-%m-%d", + Granularity::Hour => "%Y-%m-%dT%H", + } + } + + /// A sensible bucket size for a span of seconds, so the UI need not guess. + pub fn for_span(seconds: i64) -> Self { + const DAY: i64 = 86_400; + match seconds { + s if s > 5 * 365 * DAY => Granularity::Year, + s if s > 90 * DAY => Granularity::Month, + s if s > 2 * DAY => Granularity::Day, + _ => Granularity::Hour, + } + } +} + +/// A connection to the catalog. +pub struct Catalog { + conn: Connection, +} + +impl Catalog { + /// Open or create a catalog, migrating it forward if needed. + pub fn open(path: &Path) -> Result { + let conn = Connection::open(path)?; + schema::configure(&conn)?; + schema::migrate(&conn)?; + Ok(Catalog { conn }) + } + + /// An in-memory catalog, for tests and for a throwaway import preview. + pub fn in_memory() -> Result { + let conn = Connection::open_in_memory()?; + schema::configure(&conn)?; + schema::migrate(&conn)?; + Ok(Catalog { conn }) + } + + /// Escape hatch for modules that need raw access. Not part of the UI-facing + /// surface. + pub fn connection(&self) -> &Connection { + &self.conn + } + + /// How many images match. + /// + /// Returned alongside the first window so the grid can size its scrollbar + /// and paint in one round trip. + pub fn count(&self, q: &Query, now: i64) -> Result { + let c = query::compile(&q.filter, now); + let sql = query::count_sql(&c); + let n: i64 = + self.conn + .query_row(&sql, rusqlite::params_from_iter(c.params.iter()), |r| { + r.get(0) + })?; + Ok(n as usize) + } + + /// Fetch one window of results. + /// + /// Never returns the whole catalog: FR-CAT-4 requires memory bounded + /// independently of library size. + pub fn window( + &self, + q: &Query, + range: std::ops::Range, + now: i64, + ) -> Result, CatalogError> { + let c = query::compile(&q.filter, now); + let sql = query::window_sql(q, &c); + + let mut params = c.params.clone(); + params.push(rusqlite::types::Value::Integer(range.len() as i64)); + params.push(rusqlite::types::Value::Integer(range.start as i64)); + + let mut stmt = self.conn.prepare(&sql)?; + let rows = stmt + .query_map(rusqlite::params_from_iter(params.iter()), |r| { + let source_ref: String = r.get(1)?; + let avail: i64 = r.get(2)?; + Ok(GridRow { + id: ImageId(r.get::<_, i64>(0)? as u64), + name: source_ref + .rsplit(['/', ':']) + .next() + .unwrap_or(&source_ref) + .to_string(), + availability: decode_availability(avail), + captured_at: r.get(3)?, + captured_offset: r.get::<_, Option>(4)?.map(|v| v as i32), + metadata_state: r.get::<_, i64>(5)? as u8, + }) + })? + .collect::, _>>()?; + Ok(rows) + } + + /// Counts per time bucket, for the timeline scrubber. + /// + /// One grouped aggregate over the `images_captured` index — not 50k rows + /// handed to the UI to bucket itself. + pub fn timeline( + &self, + q: &Query, + g: Granularity, + now: i64, + ) -> Result, CatalogError> { + let c = query::compile(&q.filter, now); + // Bucketed in local time: captured_offset is minutes east of UTC, and + // NULL falls back to UTC rather than dropping the row. + let sql = format!( + "SELECT min(captured_at) AS start, + count(*) AS n + FROM images + WHERE {} AND captured_at IS NOT NULL + GROUP BY strftime('{}', captured_at + coalesce(captured_offset, 0) * 60, + 'unixepoch') + ORDER BY start ASC", + c.where_sql, + g.strftime() + ); + + let mut stmt = self.conn.prepare(&sql)?; + let rows = stmt + .query_map(rusqlite::params_from_iter(c.params.iter()), |r| { + Ok(TimeBucket { + start: r.get(0)?, + count: r.get::<_, i64>(1)? as u32, + }) + })? + .collect::, _>>()?; + Ok(rows) + } + + /// Merge a downloaded remote catalog's collections into this one. + /// + /// See [`sync`] for why only collections cross over. + pub fn merge_remote_catalog(&self, remote: &Path) -> Result { + sync::merge_remote(&self.conn, remote) + } + + /// Write a consistent snapshot ready to upload. + pub fn snapshot_for_upload(&self, dest: &Path) -> Result<(), CatalogError> { + sync::snapshot_for_upload(&self.conn, dest) + } +} + +fn decode_availability(v: i64) -> Availability { + match v { + 1 => Availability::Preview, + 2 => Availability::Original, + 3 => Availability::Offline, + _ => Availability::MetadataOnly, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use dr_types::Selector; + + fn seeded() -> Catalog { + let cat = Catalog::in_memory().unwrap(); + let c = cat.connection(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')", + [], + ) + .unwrap(); + // Three images across two days, one with no EXIF read yet. + for (id, name, captured, state) in [ + (1i64, "a.CR3", Some(1_000_000i64), 2i64), + (2, "b.CR3", Some(1_100_000), 2), + (3, "c.CR3", None, 1), + ] { + c.execute( + "INSERT INTO images(id, root_id, source_ref, captured_at, metadata_state, added_at) + VALUES (?1, 1, ?2, ?3, ?4, 0)", + rusqlite::params![id, name, captured, state], + ) + .unwrap(); + } + cat + } + + #[test] + fn count_and_window_agree() { + let cat = seeded(); + let q = Query::default(); + assert_eq!(cat.count(&q, 0).unwrap(), 3); + assert_eq!(cat.window(&q, 0..10, 0).unwrap().len(), 3); + } + + #[test] + fn window_is_bounded_by_the_requested_range() { + // FR-CAT-4: memory independent of catalog size. + let cat = seeded(); + let rows = cat.window(&Query::default(), 0..2, 0).unwrap(); + assert_eq!(rows.len(), 2); + } + + #[test] + fn paging_covers_every_row_exactly_once() { + let cat = seeded(); + let q = Query::default(); + let mut seen = Vec::new(); + for start in (0..3).step_by(2) { + seen.extend(cat.window(&q, start..start + 2, 0).unwrap()); + } + let mut ids: Vec = seen.iter().map(|r| r.id.0).collect(); + ids.sort_unstable(); + assert_eq!(ids, vec![1, 2, 3]); + } + + #[test] + fn an_image_without_capture_time_sorts_last_not_first() { + // Otherwise a freshly scanned library leads with whatever has not been + // read yet, which looks like corruption to the user. + let cat = seeded(); + let rows = cat.window(&Query::default(), 0..10, 0).unwrap(); + assert_eq!(rows.last().unwrap().id, ImageId(3)); + } + + #[test] + fn metadata_state_reaches_the_grid() { + // The grid needs it to distinguish "no photos on this date" from + // "EXIF not read yet" (FR-NC-6c's honesty principle). + let cat = seeded(); + let rows = cat.window(&Query::default(), 0..10, 0).unwrap(); + let pending = rows.iter().find(|r| r.id == ImageId(3)).unwrap(); + assert_eq!(pending.metadata_state, 1); + } + + #[test] + fn a_filter_narrows_the_count() { + let cat = seeded(); + let q = Query { + filter: Selector::Text("a.CR3".into()), + ..Default::default() + }; + assert_eq!(cat.count(&q, 0).unwrap(), 1); + } + + #[test] + fn timeline_buckets_and_skips_unread_images() { + let cat = seeded(); + let buckets = cat + .timeline(&Query::default(), Granularity::Day, 0) + .unwrap(); + // Two images with timestamps, one day apart in UTC; the third has no + // capture time and cannot be placed on a timeline at all. + let total: u32 = buckets.iter().map(|b| b.count).sum(); + assert_eq!(total, 2); + } + + #[test] + fn timeline_granularity_follows_the_span() { + const DAY: i64 = 86_400; + assert_eq!(Granularity::for_span(10 * 365 * DAY), Granularity::Year); + assert_eq!(Granularity::for_span(120 * DAY), Granularity::Month); + assert_eq!(Granularity::for_span(10 * DAY), Granularity::Day); + assert_eq!(Granularity::for_span(3600), Granularity::Hour); + } + + #[test] + fn names_are_derived_for_both_paths_and_saf_ids() { + let cat = Catalog::in_memory().unwrap(); + let c = cat.connection(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'saf', 'tree')", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES (1, 1, 'primary:DCIM/Camera/IMG_1.CR3', 0)", + [], + ) + .unwrap(); + let rows = cat.window(&Query::default(), 0..10, 0).unwrap(); + assert_eq!(rows[0].name, "IMG_1.CR3"); + } +} diff --git a/core/dr-catalog/src/merge.rs b/core/dr-catalog/src/merge.rs new file mode 100644 index 0000000..3a236e6 --- /dev/null +++ b/core/dr-catalog/src/merge.rs @@ -0,0 +1,527 @@ +//! TRACES: FR-CAT-7 | FR-NC-9 +//! Merging a remote catalog's collections into the local one. +//! +//! # Why this is a merge and not a copy +//! +//! The catalog file syncs to Nextcloud, and a device that finds a newer remote +//! copy must not simply replace its own — whichever device synced second would +//! lose everything the first did not have. So the remote file is downloaded to +//! a side path, `ATTACH`ed, and merged table by table. +//! +//! Row-level merging needs identities that are stable across devices, and +//! `collections.id INTEGER PRIMARY KEY` is not: two devices independently +//! allocate id 1 for different collections. Hence `collections.uuid`, which is +//! what everything here keys on. The integer id stays local and is never +//! compared across catalogs. +//! +//! # Conflict rule +//! +//! Per collection, by `revision` — a monotonic counter bumped on every local +//! edit — with `modified` timestamp only as a tiebreak. Comparing revisions +//! rather than mtimes means a device with a skewed clock cannot silently win +//! (the failure mode FR-NC-9 avoids for sidecars, applied here). +//! +//! Membership merges as a **set union**, not last-writer-wins: two devices +//! each adding different images to the same collection keep both sets. That +//! is almost always what the user meant, and the exception — a removal racing +//! an addition — resolves in favour of the addition, which is recoverable by +//! removing it again. Silently losing an addition is not. +//! +//! # Deletion +//! +//! A deleted collection leaves a tombstone (`deleted = 1`), because a merge +//! against a device that still holds it would otherwise resurrect it. The +//! tombstone carries a revision like any other edit, so deletion competes on +//! the same footing as a rename. + +use rusqlite::Connection; + +use crate::error::CatalogError; + +/// How a collection differed between the two catalogs. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MergeVerdict { + /// Present only remotely — insert it. + InsertedFromRemote, + /// Remote revision is higher — take its fields. + UpdatedFromRemote, + /// Local revision is at least as high — keep ours. + KeptLocal, + /// Remote says deleted, and wins on revision. + DeletedByRemote, +} + +/// What a merge did, for logging and for telling the user. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct MergeReport { + pub inserted: usize, + pub updated: usize, + pub kept_local: usize, + pub deleted: usize, + pub members_added: usize, +} + +impl MergeReport { + /// Whether the local catalog changed, and so needs re-uploading. + pub fn local_changed(&self) -> bool { + self.inserted > 0 || self.updated > 0 || self.deleted > 0 || self.members_added > 0 + } + + /// Whether the local catalog holds anything the remote did not, and so + /// must be uploaded even if nothing was taken from the remote. + pub fn should_upload(&self) -> bool { + self.kept_local > 0 || self.local_changed() + } +} + +/// Decide one collection, given both sides' revisions. +/// +/// Split out from the SQL so the rule is testable on its own — it is the part +/// that decides whether a user loses a collection. +pub fn verdict( + local: Option<(i64, i64)>, // (revision, modified) + remote: (i64, i64), + remote_deleted: bool, +) -> MergeVerdict { + let (r_rev, r_mod) = remote; + match local { + None if remote_deleted => { + // A tombstone for something we never had. Recording it still + // matters: without it, a third device could reintroduce the + // collection through us. + MergeVerdict::DeletedByRemote + } + None => MergeVerdict::InsertedFromRemote, + Some((l_rev, l_mod)) => { + // Revision first; timestamp only to break an exact tie. Equal + // revisions with equal timestamps keep local, so a merge that + // changes nothing is stable and repeatable. + let remote_wins = r_rev > l_rev || (r_rev == l_rev && r_mod > l_mod); + if !remote_wins { + MergeVerdict::KeptLocal + } else if remote_deleted { + MergeVerdict::DeletedByRemote + } else { + MergeVerdict::UpdatedFromRemote + } + } + } +} + +/// Merge collections and membership from an attached catalog. +/// +/// The remote catalog must already be attached under the schema name +/// `remote_cat`; [`crate::Catalog::merge_attached_collections`] handles that. +/// +/// Runs in one transaction: a merge either lands whole or not at all. +pub fn merge_collections(conn: &Connection) -> Result { + let tx = conn.unchecked_transaction()?; + let mut report = MergeReport::default(); + + // ---- collections ------------------------------------------------------ + { + let mut stmt = tx.prepare( + "SELECT r.uuid, r.name, r.parent_id, r.kind, r.selector_json, + r.created, r.revision, r.modified, r.deleted, + l.revision, l.modified + FROM remote_cat.collections r + LEFT JOIN main.collections l ON l.uuid = r.uuid", + )?; + + struct Incoming { + uuid: String, + name: String, + kind: i64, + selector_json: Option, + created: i64, + revision: i64, + modified: i64, + // No `deleted` field: the verdict already encodes it, and keeping + // both invites the two disagreeing. + verdict: MergeVerdict, + } + + let rows: Vec = stmt + .query_map([], |r| { + let deleted: i64 = r.get(8)?; + let local_rev: Option = r.get(9)?; + let local_mod: Option = r.get(10)?; + let revision: i64 = r.get(6)?; + let modified: i64 = r.get(7)?; + Ok(Incoming { + uuid: r.get(0)?, + name: r.get(1)?, + kind: r.get(3)?, + selector_json: r.get(4)?, + created: r.get(5)?, + revision, + modified, + verdict: verdict(local_rev.zip(local_mod), (revision, modified), deleted != 0), + }) + })? + .collect::>()?; + + for row in rows { + match row.verdict { + MergeVerdict::KeptLocal => { + report.kept_local += 1; + } + MergeVerdict::InsertedFromRemote => { + tx.execute( + "INSERT INTO main.collections + (uuid, name, parent_id, kind, selector_json, + created, revision, modified, deleted) + VALUES (?1, ?2, NULL, ?3, ?4, ?5, ?6, ?7, 0)", + rusqlite::params![ + row.uuid, + row.name, + row.kind, + row.selector_json, + row.created, + row.revision, + row.modified, + ], + )?; + report.inserted += 1; + } + MergeVerdict::UpdatedFromRemote => { + tx.execute( + "UPDATE main.collections + SET name = ?2, kind = ?3, selector_json = ?4, + revision = ?5, modified = ?6, deleted = 0 + WHERE uuid = ?1", + rusqlite::params![ + row.uuid, + row.name, + row.kind, + row.selector_json, + row.revision, + row.modified, + ], + )?; + report.updated += 1; + } + MergeVerdict::DeletedByRemote => { + // Tombstone rather than DELETE: the row must outlive the + // deletion or a third device reintroduces it. + tx.execute( + "INSERT INTO main.collections + (uuid, name, kind, created, revision, modified, deleted) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, 1) + ON CONFLICT(uuid) DO UPDATE SET + deleted = 1, revision = ?5, modified = ?6", + rusqlite::params![ + row.uuid, + row.name, + row.kind, + row.created, + row.revision, + row.modified, + ], + )?; + tx.execute( + "DELETE FROM main.collection_members + WHERE collection_id = (SELECT id FROM main.collections WHERE uuid = ?1)", + [&row.uuid], + )?; + report.deleted += 1; + } + } + } + } + + // ---- membership ------------------------------------------------------- + // + // Set union, keyed on (collection uuid, image content hash). The hash + // rather than the image id, for the same reason collections use a uuid: + // image ids are local. An image the remote has and we do not is skipped — + // it will join when a scan or sync catalogues it, and the next merge picks + // it up. + // + // Tombstoned collections are excluded, or a merge would repopulate a + // collection it had just deleted. + let added = tx.execute( + "INSERT OR IGNORE INTO main.collection_members(collection_id, image_id, position, added) + SELECT lc.id, li.id, rm.position, rm.added + FROM remote_cat.collection_members rm + JOIN remote_cat.collections rc ON rc.id = rm.collection_id + JOIN main.collections lc ON lc.uuid = rc.uuid AND lc.deleted = 0 + JOIN remote_cat.images ri ON ri.id = rm.image_id + JOIN main.images li ON li.content_hash = ri.content_hash + WHERE ri.content_hash IS NOT NULL", + [], + )?; + report.members_added = added; + + tx.commit()?; + Ok(report) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::schema; + + #[test] + fn a_collection_we_lack_is_taken_from_remote() { + assert_eq!( + verdict(None, (1, 100), false), + MergeVerdict::InsertedFromRemote + ); + } + + #[test] + fn higher_remote_revision_wins() { + assert_eq!( + verdict(Some((3, 100)), (4, 50), false), + MergeVerdict::UpdatedFromRemote + ); + } + + #[test] + fn a_skewed_clock_cannot_beat_a_higher_local_revision() { + // The remote's timestamp is far in the future, but it has seen fewer + // edits. Revision decides, so the skewed device does not silently + // overwrite real work. + assert_eq!( + verdict(Some((9, 100)), (2, 999_999), false), + MergeVerdict::KeptLocal + ); + } + + #[test] + fn equal_revisions_break_on_timestamp() { + assert_eq!( + verdict(Some((3, 100)), (3, 200), false), + MergeVerdict::UpdatedFromRemote + ); + assert_eq!( + verdict(Some((3, 200)), (3, 100), false), + MergeVerdict::KeptLocal + ); + } + + #[test] + fn an_identical_collection_is_stable() { + // Merging twice must not oscillate or report spurious changes. + assert_eq!( + verdict(Some((3, 100)), (3, 100), false), + MergeVerdict::KeptLocal + ); + } + + #[test] + fn deletion_competes_on_revision_like_any_other_edit() { + // Remote deleted it at revision 5; we renamed it at revision 4. The + // deletion is newer, so it wins. + assert_eq!( + verdict(Some((4, 100)), (5, 100), true), + MergeVerdict::DeletedByRemote + ); + // But a stale deletion does not undo a newer local edit. + assert_eq!( + verdict(Some((6, 100)), (5, 100), true), + MergeVerdict::KeptLocal + ); + } + + #[test] + fn a_tombstone_for_something_we_never_had_is_recorded() { + // Otherwise this device could reintroduce the collection to a third. + assert_eq!(verdict(None, (2, 100), true), MergeVerdict::DeletedByRemote); + } + + // ---- integration over two real catalogs ------------------------------ + + fn two_catalogs() -> Connection { + let c = Connection::open_in_memory().unwrap(); + schema::configure(&c).unwrap(); + schema::migrate(&c).unwrap(); + // A second in-memory database standing in for the downloaded remote. + c.execute_batch("ATTACH ':memory:' AS remote_cat").unwrap(); + let remote_schema = super::super::schema::v1_for_attached("remote_cat"); + c.execute_batch(&remote_schema).unwrap(); + c + } + + fn add_image(c: &Connection, db: &str, id: i64, hash: &str) { + c.execute( + &format!( + "INSERT INTO {db}.roots(id, kind, label) VALUES (1, 'local', 'r') + ON CONFLICT(id) DO NOTHING" + ), + [], + ) + .unwrap(); + c.execute( + &format!( + "INSERT INTO {db}.images(id, root_id, source_ref, content_hash, added_at) + VALUES (?1, 1, ?2, ?3, 0)" + ), + rusqlite::params![id, format!("img{id}.CR3"), hash], + ) + .unwrap(); + } + + fn add_collection(c: &Connection, db: &str, id: i64, uuid: &str, name: &str, rev: i64) { + c.execute( + &format!( + "INSERT INTO {db}.collections(id, uuid, name, kind, created, revision, modified) + VALUES (?1, ?2, ?3, 0, 0, ?4, ?4)" + ), + rusqlite::params![id, uuid, name, rev], + ) + .unwrap(); + } + + #[test] + fn disjoint_collections_from_two_devices_both_survive() { + // The property the whole design exists for: neither device loses work. + let c = two_catalogs(); + add_collection(&c, "main", 1, "uuid-local", "Iceland", 1); + add_collection(&c, "remote_cat", 1, "uuid-remote", "Portugal", 1); + + let report = merge_collections(&c).unwrap(); + assert_eq!(report.inserted, 1); + + let names: Vec = c + .prepare("SELECT name FROM main.collections ORDER BY name") + .unwrap() + .query_map([], |r| r.get(0)) + .unwrap() + .collect::>() + .unwrap(); + assert_eq!(names, vec!["Iceland", "Portugal"]); + } + + #[test] + fn membership_unions_rather_than_replacing() { + // Two devices each added a different image to the same collection. + let c = two_catalogs(); + add_collection(&c, "main", 1, "shared", "Trip", 1); + add_collection(&c, "remote_cat", 1, "shared", "Trip", 1); + add_image(&c, "main", 1, "hash-a"); + add_image(&c, "main", 2, "hash-b"); + add_image(&c, "remote_cat", 1, "hash-b"); + + c.execute( + "INSERT INTO main.collection_members(collection_id, image_id, added) VALUES (1, 1, 0)", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO remote_cat.collection_members(collection_id, image_id, added) + VALUES (1, 1, 0)", + [], + ) + .unwrap(); + + merge_collections(&c).unwrap(); + + let n: i64 = c + .query_row("SELECT count(*) FROM main.collection_members", [], |r| { + r.get(0) + }) + .unwrap(); + assert_eq!(n, 2, "both devices' additions survive"); + } + + #[test] + fn membership_maps_across_devices_by_content_hash() { + // The same photograph carries different integer ids on each device. + // Keying on the id would attach the wrong image. + let c = two_catalogs(); + add_collection(&c, "main", 1, "shared", "Trip", 1); + add_collection(&c, "remote_cat", 1, "shared", "Trip", 1); + add_image(&c, "main", 77, "same-photo"); + add_image(&c, "remote_cat", 3, "same-photo"); + + c.execute( + "INSERT INTO remote_cat.collection_members(collection_id, image_id, added) + VALUES (1, 3, 0)", + [], + ) + .unwrap(); + + merge_collections(&c).unwrap(); + + let img: i64 = c + .query_row("SELECT image_id FROM main.collection_members", [], |r| { + r.get(0) + }) + .unwrap(); + assert_eq!(img, 77, "resolved to the local id for the same photo"); + } + + #[test] + fn an_image_we_do_not_have_yet_is_skipped_not_errored() { + let c = two_catalogs(); + add_collection(&c, "main", 1, "shared", "Trip", 1); + add_collection(&c, "remote_cat", 1, "shared", "Trip", 1); + add_image(&c, "remote_cat", 1, "not-here-yet"); + c.execute( + "INSERT INTO remote_cat.collection_members(collection_id, image_id, added) + VALUES (1, 1, 0)", + [], + ) + .unwrap(); + + let report = merge_collections(&c).unwrap(); + assert_eq!(report.members_added, 0); + // It joins on a later merge, once a scan has catalogued the file. + } + + #[test] + fn a_remote_deletion_does_not_resurrect_via_membership() { + let c = two_catalogs(); + add_collection(&c, "main", 1, "doomed", "Old", 1); + add_image(&c, "main", 1, "hash-a"); + add_image(&c, "remote_cat", 1, "hash-a"); + c.execute( + "INSERT INTO remote_cat.collections(id, uuid, name, kind, created, revision, modified, deleted) + VALUES (1, 'doomed', 'Old', 0, 0, 5, 5, 1)", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO remote_cat.collection_members(collection_id, image_id, added) + VALUES (1, 1, 0)", + [], + ) + .unwrap(); + + let report = merge_collections(&c).unwrap(); + assert_eq!(report.deleted, 1); + let n: i64 = c + .query_row("SELECT count(*) FROM main.collection_members", [], |r| { + r.get(0) + }) + .unwrap(); + assert_eq!(n, 0, "membership must not repopulate a deleted collection"); + } + + #[test] + fn merging_twice_changes_nothing_the_second_time() { + let c = two_catalogs(); + add_collection(&c, "remote_cat", 1, "uuid-r", "Portugal", 1); + + let first = merge_collections(&c).unwrap(); + assert!(first.local_changed()); + + let second = merge_collections(&c).unwrap(); + assert!(!second.local_changed(), "merge must be idempotent"); + } + + #[test] + fn keeping_local_still_marks_the_catalog_for_upload() { + // We hold something the remote does not, so the remote is stale even + // though we took nothing from it. + let c = two_catalogs(); + add_collection(&c, "main", 1, "shared", "Renamed here", 5); + add_collection(&c, "remote_cat", 1, "shared", "Old name", 2); + + let report = merge_collections(&c).unwrap(); + assert_eq!(report.kept_local, 1); + assert!(report.should_upload()); + } +} diff --git a/core/dr-catalog/src/query.rs b/core/dr-catalog/src/query.rs new file mode 100644 index 0000000..574489a --- /dev/null +++ b/core/dr-catalog/src/query.rs @@ -0,0 +1,511 @@ +//! TRACES: FR-CAT-4 | FR-CAT-6 +//! Compiling a [`Selector`] into indexed SQL, and windowing the result. +//! +//! The UI never assembles SQL — it hands over a [`Query`] and receives a +//! window. Two properties matter: +//! +//! 1. **Nothing user-supplied is interpolated into SQL text.** Every value +//! binds as a parameter; `LIKE` patterns have their wildcards escaped. +//! 2. **Predicates hit indices.** Filtering 50k images must stay interactive +//! (FR-CAT-6), which means no expression over a column that would defeat +//! its index. + +use dr_types::{Availability, ColourLabel, DateSelector, FlagState, Selector}; +use rusqlite::types::Value; + +/// What to show, and in what order. +#[derive(Debug, Clone)] +pub struct Query { + pub filter: Selector, + pub sort: Sort, + pub descending: bool, +} + +impl Default for Query { + fn default() -> Self { + Query { + filter: Selector::All, + sort: Sort::CapturedAt, + descending: true, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Sort { + CapturedAt, + Added, + FileName, + Rating, + /// Manual order within a collection. Falls back to capture time where the + /// query is not scoped to one collection, since position is meaningless + /// outside it. + CollectionPosition, +} + +impl Sort { + /// The ORDER BY fragment. Fixed strings — never user input. + /// + /// Capture time sorts NULLs last regardless of direction: an image whose + /// EXIF has not been read yet (metadata_state 1) should not lead the grid + /// simply because its timestamp is unknown. + fn sql(self, descending: bool) -> &'static str { + match (self, descending) { + (Sort::CapturedAt, false) => { + "ORDER BY images.captured_at IS NULL, images.captured_at ASC, images.id ASC" + } + (Sort::CapturedAt, true) => { + "ORDER BY images.captured_at IS NULL, images.captured_at DESC, images.id DESC" + } + (Sort::Added, false) => "ORDER BY images.added_at ASC, images.id ASC", + (Sort::Added, true) => "ORDER BY images.added_at DESC, images.id DESC", + (Sort::FileName, false) => "ORDER BY images.source_ref ASC, images.id ASC", + (Sort::FileName, true) => "ORDER BY images.source_ref DESC, images.id DESC", + (Sort::Rating, false) => "ORDER BY v.rating ASC, images.id ASC", + (Sort::Rating, true) => "ORDER BY v.rating DESC, images.id DESC", + (Sort::CollectionPosition, false) => { + "ORDER BY cm.position IS NULL, cm.position ASC, images.captured_at ASC" + } + (Sort::CollectionPosition, true) => { + "ORDER BY cm.position IS NULL, cm.position DESC, images.captured_at DESC" + } + } + } + + /// Whether this sort needs the default-version join. + fn needs_version(self) -> bool { + matches!(self, Sort::Rating) + } + + /// Whether this sort needs a collection-membership join. + fn needs_membership(self) -> bool { + matches!(self, Sort::CollectionPosition) + } +} + +/// A compiled WHERE clause plus its bound parameters. +/// +/// Kept separate from the statement so `count` and `window` can share one +/// compilation. +#[derive(Debug, Default)] +pub struct Compiled { + pub where_sql: String, + pub params: Vec, + /// True if the filter depends on capture time, and therefore on EXIF that + /// a freshly scanned library may not have read yet. The UI surfaces this + /// rather than silently under-reporting. + pub needs_capture_time: bool, +} + +/// Compile a selector to SQL against the `images` table. +/// +/// `now` is passed rather than read from the clock so a rolling window is +/// reproducible in tests and consistent across one query. +pub fn compile(filter: &Selector, now: i64) -> Compiled { + let mut params = Vec::new(); + let sql = if filter.is_unfiltered() { + "1".to_string() + } else { + emit(filter, now, &mut params) + }; + Compiled { + where_sql: sql, + params, + needs_capture_time: filter.needs_capture_time(), + } +} + +fn emit(s: &Selector, now: i64, p: &mut Vec) -> String { + match s { + Selector::All => "1".into(), + + Selector::Collection(id) => { + p.push(Value::Integer(id.0 as i64)); + format!( + "EXISTS (SELECT 1 FROM collection_members m + WHERE m.image_id = images.id AND m.collection_id = ?{})", + p.len() + ) + } + + Selector::Folder { + root, + path, + recursive, + } => { + p.push(Value::Integer(root.0 as i64)); + let root_ix = p.len(); + if *recursive { + // Prefix match on the folder path. `like_prefix` escapes the + // pattern metacharacters, so a folder literally named "50%" + // matches itself and not everything. + p.push(Value::Text(like_prefix(path))); + format!( + "images.folder_id IN ( + SELECT id FROM folders + WHERE root_id = ?{root_ix} + AND (path = ?{p} OR path LIKE ?{p} || '/%' ESCAPE '\\'))", + p = p.len() + ) + } else { + p.push(Value::Text(path.clone())); + format!( + "images.folder_id IN ( + SELECT id FROM folders WHERE root_id = ?{root_ix} AND path = ?{})", + p.len() + ) + } + } + + Selector::DateRange(d) => emit_date(d, now, p), + + Selector::Rating { min } => { + p.push(Value::Integer(*min as i64)); + format!("{} >= ?{}", default_version_scalar("rating"), p.len()) + } + + Selector::Label(l) => { + p.push(Value::Integer(label_code(*l))); + format!("{} = ?{}", default_version_scalar("label"), p.len()) + } + + Selector::Flag(f) => { + p.push(Value::Integer(flag_code(*f))); + format!("{} = ?{}", default_version_scalar("flag"), p.len()) + } + + Selector::Keyword(k) => { + p.push(Value::Text(k.clone())); + format!( + "EXISTS (SELECT 1 FROM keywords kw + JOIN versions kv ON kv.id = kw.version_id + WHERE kv.image_id = images.id AND kw.keyword = ?{})", + p.len() + ) + } + + Selector::Camera(c) => { + p.push(Value::Text(c.clone())); + format!("images.camera = ?{}", p.len()) + } + + Selector::Lens(l) => { + p.push(Value::Text(l.clone())); + format!("images.lens = ?{}", p.len()) + } + + Selector::IsoRange { min, max } => { + p.push(Value::Integer(*min as i64)); + let lo = p.len(); + p.push(Value::Integer(*max as i64)); + format!("images.iso BETWEEN ?{lo} AND ?{}", p.len()) + } + + Selector::Availability(a) => { + p.push(Value::Integer(availability_code(*a))); + format!("images.availability = ?{}", p.len()) + } + + Selector::Text(t) => { + // Substring over filename and keywords. A LIKE scan is adequate at + // 50k; if free text over title and description becomes a real + // workflow, FTS5 is the answer and it is additive. + p.push(Value::Text(format!("%{}%", escape_like(t)))); + let ix = p.len(); + format!( + "(images.source_ref LIKE ?{ix} ESCAPE '\\' + OR EXISTS (SELECT 1 FROM keywords kw + JOIN versions kv ON kv.id = kw.version_id + WHERE kv.image_id = images.id + AND kw.keyword LIKE ?{ix} ESCAPE '\\'))" + ) + } + + // An empty conjunction is vacuously true; an empty disjunction matches + // nothing. Both arise from a UI that lets every term be cleared, and + // conflating them would show the whole library when the user meant the + // opposite. + Selector::All_(v) if v.is_empty() => "1".into(), + Selector::Any(v) if v.is_empty() => "0".into(), + + Selector::All_(v) => join(v, " AND ", now, p), + Selector::Any(v) => join(v, " OR ", now, p), + Selector::Not(inner) => format!("NOT ({})", emit(inner, now, p)), + } +} + +fn join(items: &[Selector], op: &str, now: i64, p: &mut Vec) -> String { + let parts: Vec = items.iter().map(|s| emit(s, now, p)).collect(); + format!("({})", parts.join(op)) +} + +fn emit_date(d: &DateSelector, now: i64, p: &mut Vec) -> String { + match d { + DateSelector::Between { from, to } => { + p.push(Value::Integer(*from)); + let lo = p.len(); + p.push(Value::Integer(*to)); + // Half-open, so adjacent ranges neither overlap nor gap. + format!( + "(images.captured_at >= ?{lo} AND images.captured_at < ?{})", + p.len() + ) + } + DateSelector::Rolling { days } => { + let from = now - (*days as i64) * 86_400; + p.push(Value::Integer(from)); + format!("images.captured_at >= ?{}", p.len()) + } + DateSelector::CollectionSpan(id) => { + p.push(Value::Integer(id.0 as i64)); + let ix = p.len(); + format!( + "images.captured_at BETWEEN + (SELECT min(i2.captured_at) FROM images i2 + JOIN collection_members m2 ON m2.image_id = i2.id + WHERE m2.collection_id = ?{ix}) + AND (SELECT max(i2.captured_at) FROM images i2 + JOIN collection_members m2 ON m2.image_id = i2.id + WHERE m2.collection_id = ?{ix})" + ) + } + } +} + +/// Rating, label, and flag live on the *default* version, not the image. +/// +/// A correlated subquery rather than a join, so these compose inside `OR` and +/// `NOT` without the join multiplying rows. +fn default_version_scalar(col: &str) -> String { + format!( + "(SELECT dv.{col} FROM versions dv + WHERE dv.image_id = images.id AND dv.is_default = 1 LIMIT 1)" + ) +} + +/// Escape LIKE metacharacters so a literal `%` or `_` in user text matches +/// itself. Paired with `ESCAPE '\'` in every LIKE that uses it. +fn escape_like(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for c in s.chars() { + if matches!(c, '%' | '_' | '\\') { + out.push('\\'); + } + out.push(c); + } + out +} + +fn like_prefix(path: &str) -> String { + escape_like(path.trim_end_matches('/')) +} + +fn label_code(l: ColourLabel) -> i64 { + match l { + ColourLabel::Red => 1, + ColourLabel::Yellow => 2, + ColourLabel::Green => 3, + ColourLabel::Blue => 4, + ColourLabel::Purple => 5, + } +} + +fn flag_code(f: FlagState) -> i64 { + match f { + FlagState::Unflagged => 0, + FlagState::Pick => 1, + FlagState::Reject => 2, + } +} + +fn availability_code(a: Availability) -> i64 { + match a { + Availability::MetadataOnly => 0, + Availability::Preview => 1, + Availability::Original => 2, + Availability::Offline => 3, + } +} + +/// Build the full SELECT for a window of results. +/// +/// Joins are added only where the sort needs them, so an unsorted-by-rating +/// grid query touches one table. +pub fn window_sql(q: &Query, compiled: &Compiled) -> String { + let mut joins = String::new(); + if q.sort.needs_version() { + joins.push_str(" LEFT JOIN versions v ON v.image_id = images.id AND v.is_default = 1"); + } + if q.sort.needs_membership() { + // Only meaningful when the filter scopes to one collection; elsewhere + // position is NULL and the sort falls through to capture time. + joins.push_str(" LEFT JOIN collection_members cm ON cm.image_id = images.id"); + } + format!( + "SELECT images.id, images.source_ref, images.availability, images.captured_at, \ + images.captured_offset, images.metadata_state \ + FROM images{joins} WHERE {} {} LIMIT ? OFFSET ?", + compiled.where_sql, + q.sort.sql(q.descending) + ) +} + +/// Build the COUNT for the same filter. +pub fn count_sql(compiled: &Compiled) -> String { + format!("SELECT count(*) FROM images WHERE {}", compiled.where_sql) +} + +#[cfg(test)] +mod tests { + use super::*; + use dr_types::{CollectionId, RootId}; + + #[test] + fn unfiltered_compiles_to_a_constant() { + let c = compile(&Selector::All, 0); + assert_eq!(c.where_sql, "1"); + assert!(c.params.is_empty()); + } + + #[test] + fn empty_conjunction_and_disjunction_differ() { + // The distinction that decides whether clearing a filter shows + // everything or nothing. + assert_eq!(compile(&Selector::All_(vec![]), 0).where_sql, "1"); + assert_eq!(compile(&Selector::Any(vec![]), 0).where_sql, "0"); + } + + #[test] + fn values_bind_rather_than_interpolate() { + // The injection guard: a hostile keyword must appear in params, never + // in SQL text. + let evil = "'; DROP TABLE images; --"; + let c = compile(&Selector::Keyword(evil.into()), 0); + assert!(!c.where_sql.contains("DROP")); + assert_eq!(c.params, vec![Value::Text(evil.into())]); + } + + #[test] + fn like_metacharacters_are_escaped() { + // A search for "50%" must not match everything containing "50". + let c = compile(&Selector::Text("50%".into()), 0); + assert_eq!(c.params, vec![Value::Text("%50\\%%".into())]); + assert!(c.where_sql.contains("ESCAPE")); + } + + #[test] + fn a_backslash_in_search_text_is_itself_escaped() { + let c = compile(&Selector::Text("a\\b".into()), 0); + assert_eq!(c.params, vec![Value::Text("%a\\\\b%".into())]); + } + + #[test] + fn rolling_window_resolves_against_supplied_now() { + // Passed in rather than read from the clock, so the window is stable + // across one query and reproducible in a test. + let now = 1_000_000i64; + let c = compile( + &Selector::DateRange(DateSelector::Rolling { days: 90 }), + now, + ); + assert_eq!(c.params, vec![Value::Integer(now - 90 * 86_400)]); + } + + #[test] + fn between_is_half_open() { + let c = compile( + &Selector::DateRange(DateSelector::Between { from: 10, to: 20 }), + 0, + ); + // Half-open so adjacent day buckets neither overlap nor leave a gap. + assert!(c.where_sql.contains(">= ?1")); + assert!(c.where_sql.contains("< ?2")); + } + + #[test] + fn nested_composition_numbers_parameters_in_order() { + let s = Selector::All_(vec![ + Selector::Rating { min: 4 }, + Selector::Any(vec![ + Selector::Camera("X-T5".into()), + Selector::Not(Box::new(Selector::Lens("XF 35".into()))), + ]), + ]); + let c = compile(&s, 0); + assert_eq!( + c.params, + vec![ + Value::Integer(4), + Value::Text("X-T5".into()), + Value::Text("XF 35".into()), + ] + ); + assert!(c.where_sql.contains("?1")); + assert!(c.where_sql.contains("?2")); + assert!(c.where_sql.contains("?3")); + } + + #[test] + fn recursive_folder_matches_the_folder_itself_and_below() { + let c = compile( + &Selector::Folder { + root: RootId(1), + path: "2026/08".into(), + recursive: true, + }, + 0, + ); + // Both branches: the folder's own images and those in subfolders. + assert!(c.where_sql.contains("path = ?2")); + assert!(c.where_sql.contains("|| '/%'")); + } + + #[test] + fn collection_span_binds_its_id_once_and_reuses_it() { + let c = compile( + &Selector::DateRange(DateSelector::CollectionSpan(CollectionId(7))), + 0, + ); + assert_eq!(c.params, vec![Value::Integer(7)]); + } + + #[test] + fn capture_time_dependency_is_reported() { + let c = compile(&Selector::DateRange(DateSelector::Rolling { days: 7 }), 0); + assert!(c.needs_capture_time); + let c = compile(&Selector::Rating { min: 5 }, 0); + assert!(!c.needs_capture_time); + } + + #[test] + fn capture_sort_puts_unknown_timestamps_last_in_both_directions() { + // An image whose EXIF has not been read yet must not lead the grid + // just because its timestamp is NULL. + assert!(Sort::CapturedAt.sql(true).contains("IS NULL")); + assert!(Sort::CapturedAt.sql(false).contains("IS NULL")); + } + + #[test] + fn window_sql_joins_only_when_the_sort_needs_it() { + let c = compile(&Selector::All, 0); + let plain = window_sql( + &Query { + filter: Selector::All, + sort: Sort::CapturedAt, + descending: true, + }, + &c, + ); + assert!(!plain.contains("JOIN")); + + let rated = window_sql( + &Query { + filter: Selector::All, + sort: Sort::Rating, + descending: true, + }, + &c, + ); + assert!(rated.contains("JOIN versions")); + } +} diff --git a/core/dr-catalog/src/scan.rs b/core/dr-catalog/src/scan.rs new file mode 100644 index 0000000..e1414ce --- /dev/null +++ b/core/dr-catalog/src/scan.rs @@ -0,0 +1,254 @@ +//! TRACES: FR-CAT-1 | FR-CAT-9 | NFR-P1 +//! Incremental scan: the local analogue of ETag pruning. +//! +//! Nextcloud propagates ETags up the tree, so one request proves a whole +//! library unchanged (ARCH §8.4). A filesystem offers no such guarantee — a +//! directory's mtime moves when its *direct* entries change and not when a +//! grandchild does, so there is no cheap "did anything below here change" +//! probe. +//! +//! Local scan therefore prunes at each level rather than at the root: one +//! metadata probe per directory when nothing changed, instead of one per file. +//! A 50k-image library in ~2k folders costs 2k probes, which is the difference +//! between meeting and missing NFR-P1 on SAF. +//! +//! This module holds the decision logic and the deletion-sweep rules; walking +//! an actual directory belongs to the platform layer, which supplies +//! [`DirState`] and [`DirEntry`]. + +use dr_types::FormatFilter; + +/// What a directory looked like when last scanned, and what it looks like now. +/// +/// Both fields are cheap to obtain: one `stat` locally, one +/// `DocumentsContract` metadata query on SAF. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct DirState { + pub mtime: i64, + /// Direct children, files and directories alike. + /// + /// mtime alone misses a delete-and-create inside one timestamp tick, and + /// coarse-granularity providers widen that window. The count does not + /// close the hole — a paired add and remove moves neither — but a bare add + /// or remove moves the count, and those are far commoner. + pub entry_count: u32, +} + +/// One entry from a directory listing. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DirEntry { + pub name: String, + pub is_dir: bool, + pub size: u64, + pub mtime: i64, +} + +/// What the scanner should do with a directory, before listing it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DirAction { + /// Contents unchanged. Skip the listing, but still recurse into known + /// children — without upward propagation, a deep change is invisible from + /// here. + RecurseOnly, + /// List and reconcile, then recurse. + ListAndRecurse, +} + +/// Decide whether a directory needs listing. +pub fn classify_dir(stored: Option, current: DirState) -> DirAction { + match stored { + Some(s) if s == current => DirAction::RecurseOnly, + _ => DirAction::ListAndRecurse, + } +} + +/// What reconciling one listed entry against the catalog implies. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EntryAction { + /// Not catalogued. Insert at `metadata_state = 1` and queue EXIF. + Insert, + /// Catalogued and unchanged. The common case, and it must cost nothing. + Unchanged, + /// Size or mtime moved: re-read metadata, rebuild the thumbnail, and drop + /// the content hash, which is no longer valid. + Changed, + /// Recognised but not a format the user asked to scan for. + Ignored, +} + +/// What the catalog already holds for a source. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct KnownFile { + pub size: u64, + pub mtime: i64, +} + +/// Classify one listed file. +pub fn classify_entry( + entry: &DirEntry, + known: Option, + formats: &FormatFilter, +) -> EntryAction { + if !formats.allows_name(&entry.name) { + return EntryAction::Ignored; + } + match known { + None => EntryAction::Insert, + Some(k) if k.size == entry.size && k.mtime == entry.mtime => EntryAction::Unchanged, + Some(_) => EntryAction::Changed, + } +} + +/// Outcome of a scan, which decides whether pruning may run. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScanOutcome { + /// Every reachable folder was visited. + Complete, + /// The user cancelled. Partial state is valid — jobs are resumable — but + /// unvisited folders must not be read as deleted. + Cancelled, + /// The root itself could not be opened: drive unplugged, SAF grant + /// revoked, share unmounted. + RootUnreachable, + /// Some subtree failed while the root was fine. + PartialFailure, +} + +impl ScanOutcome { + /// Whether the deletion sweep may run. + /// + /// **The most dangerous decision in the catalog.** The sweep deletes every + /// folder not reached by this scan's generation. After an incomplete scan + /// that is most of the library, so it runs only on `Complete`. + /// + /// FR-CAT-9 draws exactly this line: a source *proven absent* may leave + /// the catalog; a source merely *unreachable* is marked offline and kept, + /// with its ratings and edits intact. + pub fn may_prune(self) -> bool { + matches!(self, ScanOutcome::Complete) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use dr_types::Format; + + const A: DirState = DirState { + mtime: 100, + entry_count: 5, + }; + + #[test] + fn unchanged_directory_is_not_listed() { + assert_eq!(classify_dir(Some(A), A), DirAction::RecurseOnly); + } + + #[test] + fn a_never_seen_directory_is_listed() { + assert_eq!(classify_dir(None, A), DirAction::ListAndRecurse); + } + + #[test] + fn changed_mtime_forces_a_listing() { + let now = DirState { mtime: 101, ..A }; + assert_eq!(classify_dir(Some(A), now), DirAction::ListAndRecurse); + } + + #[test] + fn entry_count_catches_what_mtime_misses() { + // A file added within the same timestamp tick: mtime is unchanged, so + // mtime alone would skip this directory and lose the new image. + let now = DirState { + mtime: 100, + entry_count: 6, + }; + assert_eq!(classify_dir(Some(A), now), DirAction::ListAndRecurse); + } + + #[test] + fn unchanged_file_costs_nothing() { + let e = DirEntry { + name: "IMG_0001.CR3".into(), + is_dir: false, + size: 30_000_000, + mtime: 500, + }; + let known = KnownFile { + size: 30_000_000, + mtime: 500, + }; + assert_eq!( + classify_entry(&e, Some(known), &FormatFilter::all()), + EntryAction::Unchanged + ); + } + + #[test] + fn a_resaved_file_is_reprocessed() { + let e = DirEntry { + name: "IMG_0001.CR3".into(), + is_dir: false, + size: 30_000_001, + mtime: 900, + }; + let known = KnownFile { + size: 30_000_000, + mtime: 500, + }; + assert_eq!( + classify_entry(&e, Some(known), &FormatFilter::all()), + EntryAction::Changed + ); + } + + #[test] + fn format_filter_excludes_unwanted_types() { + let jpeg = DirEntry { + name: "IMG_0001.JPG".into(), + is_dir: false, + size: 1, + mtime: 1, + }; + assert_eq!( + classify_entry(&jpeg, None, &FormatFilter::raw_only()), + EntryAction::Ignored + ); + assert_eq!( + classify_entry(&jpeg, None, &FormatFilter::all()), + EntryAction::Insert + ); + } + + #[test] + fn a_placeholder_is_catalogued_as_the_image_it_stands_for() { + // 121,785 of these in a real synced folder (ARCH §9.0). Each must + // enter the catalog as a CR2 marked offline, not be skipped as an + // unknown ".nextcloud" type. + let stub = DirEntry { + name: "_MG_4130.CR2.nextcloud".into(), + is_dir: false, + size: 1, + mtime: 1, + }; + assert_eq!( + classify_entry(&stub, None, &FormatFilter::from_formats([Format::Cr2])), + EntryAction::Insert + ); + } + + #[test] + fn pruning_requires_a_complete_scan() { + assert!(ScanOutcome::Complete.may_prune()); + } + + #[test] + fn an_unreachable_root_never_prunes() { + // The guard that stops an unplugged drive from deleting the library: + // every folder would look unreached, so the sweep would take all of + // them (FR-CAT-9). + assert!(!ScanOutcome::RootUnreachable.may_prune()); + assert!(!ScanOutcome::Cancelled.may_prune()); + assert!(!ScanOutcome::PartialFailure.may_prune()); + } +} diff --git a/core/dr-catalog/src/schema.rs b/core/dr-catalog/src/schema.rs new file mode 100644 index 0000000..a52c559 --- /dev/null +++ b/core/dr-catalog/src/schema.rs @@ -0,0 +1,353 @@ +//! TRACES: FR-CAT-2 | NFR-R5 +//! Schema definition and forward-only migrations. +//! +//! The catalog is an *index*, not a source of truth (ARCH §6.12) — it is +//! deletable and rebuildable from sources plus sidecars. That is what makes +//! migration failure survivable, and why the recovery path is the normal +//! mechanism rather than a last resort. +//! +//! Migrations are forward-only, transactional, and idempotent on retry +//! (NFR-R5). The app refuses to open a catalog newer than it understands +//! rather than corrupting it. + +use rusqlite::Connection; + +use crate::error::CatalogError; + +/// Schema version this build writes and understands. +pub const SCHEMA_VERSION: i64 = 1; + +/// Apply migrations up to [`SCHEMA_VERSION`]. +/// +/// Returns the version migrated from, so callers can log or back up before a +/// real migration (NFR-R2 requires a backup before schema change). +pub fn migrate(conn: &Connection) -> Result { + let from: i64 = conn.query_row("PRAGMA user_version", [], |r| r.get(0))?; + + if from > SCHEMA_VERSION { + return Err(CatalogError::SchemaTooNew { + found: from, + supported: SCHEMA_VERSION, + }); + } + if from == SCHEMA_VERSION { + return Ok(from); + } + + // Each step runs in its own transaction so a failure leaves the catalog + // at a coherent version rather than half-migrated. + if from < 1 { + let tx = conn.unchecked_transaction()?; + tx.execute_batch(V1)?; + tx.pragma_update(None, "user_version", 1)?; + tx.commit()?; + } + + Ok(from) +} + +/// Connection setup applied on every open, migration or not. +/// +/// WAL is required by NFR-R1: it survives power loss without corruption, and +/// it lets a background job write while the grid reads. +pub fn configure(conn: &Connection) -> Result<(), CatalogError> { + conn.pragma_update(None, "journal_mode", "WAL")?; + // NORMAL rather than FULL: with WAL this is durable across process death + // (which is what FR-PLAT-AND-3 cares about) and only risks the last + // transaction on power loss. The catalog is rebuildable; the sidecars are + // not, and they are written separately with their own fsync discipline. + conn.pragma_update(None, "synchronous", "NORMAL")?; + conn.pragma_update(None, "foreign_keys", true)?; + // A scan touching thousands of rows is transient; let SQLite spill to + // memory rather than materialising temp b-trees on disk. + conn.pragma_update(None, "temp_store", "MEMORY")?; + Ok(()) +} + +/// The v1 schema rewritten to target an attached database. +/// +/// Needed because a downloaded remote catalog is `ATTACH`ed under its own +/// schema name before merging, and tests build one from scratch. SQLite has no +/// "create these tables over there" form, so the names are rewritten. +/// +/// The rewrite is textual and therefore only as good as the naming discipline +/// in [`V1`]: every `CREATE TABLE`/`CREATE INDEX` must name its object +/// unqualified, which they do. +pub fn v1_for_attached(schema_name: &str) -> String { + V1.replace("CREATE TABLE ", &format!("CREATE TABLE {schema_name}.")) + .replace("CREATE INDEX ", &format!("CREATE INDEX {schema_name}.")) + .replace( + "CREATE UNIQUE INDEX ", + &format!("CREATE UNIQUE INDEX {schema_name}."), + ) + // REFERENCES within an attached schema resolve to that schema already, + // so foreign keys need no rewriting — but the ON clause of an index + // does, and `CREATE INDEX x.name ON table` is the correct form. +} + +const V1: &str = r#" +-- Roots ------------------------------------------------------------------- +CREATE TABLE roots ( + id INTEGER PRIMARY KEY, + kind TEXT NOT NULL, -- 'local' | 'saf' | 'remote' + grant_blob BLOB, -- SAF persisted permission; NULL on Linux + label TEXT NOT NULL, + last_seen INTEGER, + -- Bumped once per completed scan. Folders record the generation they were + -- reached in; anything older was not reached and no longer exists. + scan_generation INTEGER NOT NULL DEFAULT 0 +); + +-- Folders: the unit of change detection, local and remote alike ----------- +CREATE TABLE folders ( + id INTEGER PRIMARY KEY, + root_id INTEGER NOT NULL REFERENCES roots(id) ON DELETE CASCADE, + parent_id INTEGER REFERENCES folders(id) ON DELETE CASCADE, + path TEXT NOT NULL, + -- Remote: the propagating ETag that makes a no-op sync one request. + etag TEXT, + -- Local: directory mtime plus direct-entry count. mtime alone misses a + -- paired create+delete inside one timestamp tick; the count narrows that. + mtime INTEGER, + entry_count INTEGER, + scanned_generation INTEGER NOT NULL DEFAULT 0, + UNIQUE(root_id, path) +); +CREATE INDEX folders_parent ON folders(parent_id); + +-- Images ------------------------------------------------------------------ +CREATE TABLE images ( + id INTEGER PRIMARY KEY, + root_id INTEGER NOT NULL REFERENCES roots(id) ON DELETE CASCADE, + folder_id INTEGER REFERENCES folders(id) ON DELETE CASCADE, + source_ref TEXT NOT NULL, + -- Expensive: requires reading the whole file. Computed only when + -- something needs it (import dedup, reconnect-by-hash), never in a scan. + content_hash TEXT, + format TEXT, + w INTEGER, + h INTEGER, + -- UTC seconds. NULL until EXIF is read, or if the file carries none. + captured_at INTEGER, + -- Minutes east of UTC. A photograph's timestamp is local to where it was + -- taken; storing UTC alone makes a Tokyo shoot span two days in Paris. + captured_offset INTEGER, + camera TEXT, + lens TEXT, + iso INTEGER, + aperture REAL, + shutter REAL, + availability INTEGER NOT NULL DEFAULT 0, + file_size INTEGER, + file_mtime INTEGER, + -- 0 = nothing, 1 = stat-only, 2 = full EXIF. The grid is usable at 1. + metadata_state INTEGER NOT NULL DEFAULT 0, + sidecar_mtime INTEGER, + added_at INTEGER NOT NULL, + UNIQUE(root_id, source_ref) +); +CREATE INDEX images_captured ON images(captured_at); +CREATE INDEX images_folder ON images(folder_id); +-- Partial: content_hash is NULL for most rows most of the time, and the +-- non-NULL subset is exactly what reconnect and dedup query. +CREATE INDEX images_hash ON images(content_hash) WHERE content_hash IS NOT NULL; + +-- Versions ---------------------------------------------------------------- +CREATE TABLE versions ( + id INTEGER PRIMARY KEY, + image_id INTEGER NOT NULL REFERENCES images(id) ON DELETE CASCADE, + uuid TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + is_default INTEGER NOT NULL DEFAULT 0, + graph_hash TEXT, + rating INTEGER NOT NULL DEFAULT 0, + label INTEGER, + flag INTEGER NOT NULL DEFAULT 0 +); +CREATE INDEX versions_image ON versions(image_id); + +CREATE TABLE keywords ( + version_id INTEGER NOT NULL REFERENCES versions(id) ON DELETE CASCADE, + keyword TEXT NOT NULL, + PRIMARY KEY(version_id, keyword) +); +CREATE INDEX keywords_term ON keywords(keyword); + +-- Remote mapping ---------------------------------------------------------- +CREATE TABLE remote ( + image_id INTEGER PRIMARY KEY REFERENCES images(id) ON DELETE CASCADE, + -- oc:fileid — stable across server-side rename and move, so a move is not + -- a re-download of 80 MB. + file_id INTEGER NOT NULL, + etag TEXT, + sync_state INTEGER NOT NULL DEFAULT 0, + remote_path TEXT +); +CREATE UNIQUE INDEX remote_file ON remote(file_id); + +-- Collections ------------------------------------------------------------- +CREATE TABLE collections ( + id INTEGER PRIMARY KEY, + -- Device-independent identity. The integer id is local and collides + -- across devices; the UUID is what a cross-device merge keys on. + uuid TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + parent_id INTEGER REFERENCES collections(id) ON DELETE CASCADE, + kind INTEGER NOT NULL, -- 0 = manual, 1 = smart + selector_json TEXT, -- smart only + created INTEGER NOT NULL, + -- Monotonic per collection, bumped on every local edit. Merge compares + -- these rather than file mtimes, so a clock-skewed device cannot silently + -- win. + revision INTEGER NOT NULL DEFAULT 1, + modified INTEGER NOT NULL, + -- Tombstone. A deleted collection must outlive its deletion, or a merge + -- with a device that still has it would resurrect it. + deleted INTEGER NOT NULL DEFAULT 0 +); + +CREATE TABLE collection_members ( + collection_id INTEGER NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + image_id INTEGER NOT NULL REFERENCES images(id) ON DELETE CASCADE, + position INTEGER, -- manual ordering; NULL = by capture time + added INTEGER NOT NULL, + PRIMARY KEY(collection_id, image_id) +); +CREATE INDEX members_image ON collection_members(image_id); + +-- Cache ------------------------------------------------------------------- +CREATE TABLE cache ( + id INTEGER PRIMARY KEY, + version_id INTEGER REFERENCES versions(id) ON DELETE CASCADE, + image_id INTEGER REFERENCES images(id) ON DELETE CASCADE, + kind INTEGER NOT NULL, -- thumbnail | proxy | original + resolution INTEGER, + graph_hash TEXT, + path TEXT NOT NULL, + bytes INTEGER NOT NULL, + last_used INTEGER NOT NULL +); +CREATE INDEX cache_lru ON cache(last_used); + +CREATE TABLE cache_rules ( + id INTEGER PRIMARY KEY, + selector_json TEXT NOT NULL, + tier INTEGER NOT NULL, + priority INTEGER NOT NULL DEFAULT 0, + enabled INTEGER NOT NULL DEFAULT 1 +); + +CREATE TABLE image_cache ( + image_id INTEGER PRIMARY KEY REFERENCES images(id) ON DELETE CASCADE, + tier_actual INTEGER NOT NULL DEFAULT 0, + -- Materialised rather than recomputed, so the grid can draw availability + -- badges without evaluating every rule for every visible cell. + tier_desired INTEGER NOT NULL DEFAULT 0, + bytes INTEGER NOT NULL DEFAULT 0, + last_used INTEGER, + pinned_by_rule INTEGER REFERENCES cache_rules(id) ON DELETE SET NULL +); + +-- Jobs -------------------------------------------------------------------- +CREATE TABLE jobs ( + id INTEGER PRIMARY KEY, + kind INTEGER NOT NULL, + subject_id INTEGER, + priority INTEGER NOT NULL DEFAULT 0, + state INTEGER NOT NULL DEFAULT 0, -- 0=pending 1=running 2=failed + attempts INTEGER NOT NULL DEFAULT 0, + not_before INTEGER NOT NULL DEFAULT 0, + payload TEXT, + last_error TEXT, + -- Coalescing. Enqueueing the same work twice updates one row rather than + -- queueing it twice, which is what makes "enqueue on any change" safe to + -- call liberally. + UNIQUE(kind, subject_id) +); +CREATE INDEX jobs_ready ON jobs(state, priority DESC, not_before); +"#; + +#[cfg(test)] +mod tests { + use super::*; + + fn mem() -> Connection { + let c = Connection::open_in_memory().unwrap(); + configure(&c).unwrap(); + c + } + + #[test] + fn migrate_creates_schema_at_current_version() { + let c = mem(); + assert_eq!(migrate(&c).unwrap(), 0); + let v: i64 = c + .query_row("PRAGMA user_version", [], |r| r.get(0)) + .unwrap(); + assert_eq!(v, SCHEMA_VERSION); + } + + #[test] + fn migrate_is_idempotent() { + let c = mem(); + migrate(&c).unwrap(); + // Re-running must not error or duplicate anything — NFR-R5 requires + // idempotency on retry, since a migration can be interrupted. + assert_eq!(migrate(&c).unwrap(), SCHEMA_VERSION); + } + + #[test] + fn refuses_a_catalog_from_a_newer_build() { + let c = mem(); + migrate(&c).unwrap(); + c.pragma_update(None, "user_version", SCHEMA_VERSION + 1) + .unwrap(); + // Opening it read-write would corrupt data this build cannot + // represent. Refusing is the specified behaviour (NFR-R5). + assert!(matches!( + migrate(&c), + Err(CatalogError::SchemaTooNew { .. }) + )); + } + + #[test] + fn foreign_keys_cascade_from_root_to_image() { + let c = mem(); + migrate(&c).unwrap(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'test')", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) VALUES (1, 1, 'a.CR3', 0)", + [], + ) + .unwrap(); + c.execute("DELETE FROM roots WHERE id = 1", []).unwrap(); + + let n: i64 = c + .query_row("SELECT count(*) FROM images", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 0, "images must not outlive their root"); + } + + #[test] + fn job_uniqueness_coalesces_rather_than_duplicating() { + let c = mem(); + migrate(&c).unwrap(); + for _ in 0..5 { + c.execute( + "INSERT INTO jobs(kind, subject_id, priority) VALUES (1, 42, 0) + ON CONFLICT(kind, subject_id) + DO UPDATE SET priority = max(priority, excluded.priority)", + [], + ) + .unwrap(); + } + let n: i64 = c + .query_row("SELECT count(*) FROM jobs", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 1, "five enqueues of the same work is one job"); + } +} diff --git a/core/dr-catalog/src/sync.rs b/core/dr-catalog/src/sync.rs new file mode 100644 index 0000000..7cabf36 --- /dev/null +++ b/core/dr-catalog/src/sync.rs @@ -0,0 +1,252 @@ +//! TRACES: FR-CAT-7 | FR-NC-9 | NFR-R1 +//! Preparing the catalog file for upload, and taking in a remote one. +//! +//! # The hazard this module exists to handle +//! +//! A WAL-mode SQLite database is not one file. Committed transactions can live +//! in `catalog.sqlite-wal` with the main file lagging behind, so copying +//! `catalog.sqlite` alone uploads a **torn snapshot**: internally consistent as +//! of some older point, missing everything since. Worse, a naive copy taken +//! while a writer is mid-transaction can be structurally corrupt. +//! +//! So an upload never copies the live file. It runs a TRUNCATE checkpoint to +//! fold the WAL back into the main file, then uses SQLite's own backup API to +//! take a consistent snapshot — which serialises correctly against concurrent +//! writers rather than racing them. +//! +//! # What is actually synced +//! +//! Only collections merge (see [`crate::merge`]). The rest of the catalog is a +//! *local index* of *local* storage — folder mtimes, cache paths, job rows — +//! and copying another device's version of those in would be actively wrong. +//! The remote file is read for its collections and then discarded. +//! +//! This is why the catalog remains disposable in the ARCH §6.12 sense: nothing +//! here makes the local database authoritative for anything a rebuild could +//! not recover. + +use std::path::{Path, PathBuf}; + +use rusqlite::Connection; + +use crate::error::CatalogError; +use crate::merge::{self, MergeReport}; + +/// Schema name the downloaded remote catalog is attached under. +const REMOTE_SCHEMA: &str = "remote_cat"; + +/// Fold the WAL into the main database file. +/// +/// TRUNCATE rather than PASSIVE: passive checkpointing gives up when a reader +/// holds the WAL open, which would leave recent commits out of the snapshot +/// without saying so. +pub fn checkpoint(conn: &Connection) -> Result<(), CatalogError> { + conn.pragma_update(None, "wal_checkpoint", "TRUNCATE")?; + Ok(()) +} + +/// Write a consistent snapshot of the catalog to `dest`, ready to upload. +/// +/// Uses the backup API rather than a filesystem copy so the snapshot is +/// coherent even with writers active. Callers should still prefer a quiet +/// moment — this competes with background jobs for the write lock. +pub fn snapshot_for_upload(conn: &Connection, dest: &Path) -> Result<(), CatalogError> { + checkpoint(conn)?; + + let mut out = Connection::open(dest)?; + let backup = rusqlite::backup::Backup::new(conn, &mut out)?; + // SQLite's own "copy everything" sentinel is -1, but rusqlite asserts a + // positive page count, so ask for more pages than a catalog will ever + // have. The effect is the same: one step, no interleaved writers, no + // progress callback. A 50k-image catalog is tens of megabytes. + backup.run_to_completion(i32::MAX, std::time::Duration::ZERO, None)?; + Ok(()) +} + +/// Whether a downloaded remote catalog is worth merging. +/// +/// Cheap guard before attaching: a remote written by a newer build may contain +/// tables and columns this one cannot read, and attempting the merge would +/// fail mid-transaction rather than declining cleanly. +pub fn remote_is_mergeable(remote: &Path) -> Result { + let conn = Connection::open_with_flags( + remote, + rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY | rusqlite::OpenFlags::SQLITE_OPEN_NO_MUTEX, + )?; + let v: i64 = conn.query_row("PRAGMA user_version", [], |r| r.get(0))?; + Ok(v <= crate::schema::SCHEMA_VERSION) +} + +/// Attach a downloaded remote catalog, merge its collections, detach. +/// +/// The remote file is opened **read-only** — this device never writes to +/// another device's catalog, it only reads collections out of it. +pub fn merge_remote(conn: &Connection, remote: &Path) -> Result { + if !remote_is_mergeable(remote)? { + return Err(CatalogError::SchemaTooNew { + found: -1, + supported: crate::schema::SCHEMA_VERSION, + }); + } + + // Path binds as a parameter; ATTACH accepts one, so a path containing a + // quote cannot break out into SQL. + conn.execute( + &format!("ATTACH DATABASE ?1 AS {REMOTE_SCHEMA}"), + [remote.to_string_lossy().as_ref()], + )?; + + let result = merge::merge_collections(conn); + + // Detach even if the merge failed, or the next attempt errors with + // "database remote_cat is already in use". + let detach = conn.execute(&format!("DETACH DATABASE {REMOTE_SCHEMA}"), []); + if let Err(e) = detach { + log::warn!("failed to detach remote catalog: {e}"); + } + + result +} + +/// Where the catalog snapshot and the downloaded remote live. +/// +/// Both are transient working files, not the catalog itself, so they belong in +/// the cache directory rather than beside the live database. +#[derive(Debug, Clone)] +pub struct SyncPaths { + pub upload_snapshot: PathBuf, + pub downloaded_remote: PathBuf, +} + +impl SyncPaths { + pub fn in_dir(cache_dir: &Path) -> Self { + SyncPaths { + upload_snapshot: cache_dir.join("catalog-upload.sqlite"), + downloaded_remote: cache_dir.join("catalog-remote.sqlite"), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::schema; + + fn seeded(path: &Path) -> Connection { + let c = Connection::open(path).unwrap(); + schema::configure(&c).unwrap(); + schema::migrate(&c).unwrap(); + c + } + + #[test] + fn snapshot_captures_committed_data() { + let dir = tempdir(); + let live = dir.join("catalog.sqlite"); + let snap = dir.join("snap.sqlite"); + + let c = seeded(&live); + c.execute( + "INSERT INTO collections(uuid, name, kind, created, revision, modified) + VALUES ('u1', 'Iceland', 0, 0, 1, 1)", + [], + ) + .unwrap(); + + snapshot_for_upload(&c, &snap).unwrap(); + + // The snapshot must hold the row even though it was written after the + // database was created — the torn-file failure this guards against. + let s = Connection::open(&snap).unwrap(); + let name: String = s + .query_row("SELECT name FROM collections", [], |r| r.get(0)) + .unwrap(); + assert_eq!(name, "Iceland"); + } + + #[test] + fn a_remote_from_a_newer_build_is_declined_not_attempted() { + let dir = tempdir(); + let remote = dir.join("remote.sqlite"); + let r = seeded(&remote); + r.pragma_update(None, "user_version", schema::SCHEMA_VERSION + 1) + .unwrap(); + drop(r); + + assert!(!remote_is_mergeable(&remote).unwrap()); + + let local = seeded(&dir.join("local.sqlite")); + assert!(matches!( + merge_remote(&local, &remote), + Err(CatalogError::SchemaTooNew { .. }) + )); + } + + #[test] + fn merge_remote_round_trips_a_collection() { + let dir = tempdir(); + let remote_path = dir.join("remote.sqlite"); + { + let r = seeded(&remote_path); + r.execute( + "INSERT INTO collections(uuid, name, kind, created, revision, modified) + VALUES ('u-remote', 'Portugal', 0, 0, 1, 1)", + [], + ) + .unwrap(); + checkpoint(&r).unwrap(); + } + + let local = seeded(&dir.join("local.sqlite")); + local + .execute( + "INSERT INTO collections(uuid, name, kind, created, revision, modified) + VALUES ('u-local', 'Iceland', 0, 0, 1, 1)", + [], + ) + .unwrap(); + + let report = merge_remote(&local, &remote_path).unwrap(); + assert_eq!(report.inserted, 1); + + let n: i64 = local + .query_row("SELECT count(*) FROM collections", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 2); + } + + #[test] + fn the_remote_can_be_merged_twice_without_attach_conflict() { + // Detach must happen even on the failure path, or the second attempt + // errors with "database remote_cat is already in use". + let dir = tempdir(); + let remote_path = dir.join("remote.sqlite"); + { + let r = seeded(&remote_path); + r.execute( + "INSERT INTO collections(uuid, name, kind, created, revision, modified) + VALUES ('u-remote', 'Portugal', 0, 0, 1, 1)", + [], + ) + .unwrap(); + checkpoint(&r).unwrap(); + } + let local = seeded(&dir.join("local.sqlite")); + + merge_remote(&local, &remote_path).unwrap(); + let second = merge_remote(&local, &remote_path).unwrap(); + assert!(!second.local_changed()); + } + + /// A scratch directory that cleans up with the test. + fn tempdir() -> PathBuf { + let base = std::env::temp_dir().join(format!( + "dr-catalog-test-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&base); + std::fs::create_dir_all(&base).unwrap(); + base + } +} diff --git a/core/dr-gpu/src/adjust.rs b/core/dr-gpu/src/adjust.rs index 22f629b..f91be06 100644 --- a/core/dr-gpu/src/adjust.rs +++ b/core/dr-gpu/src/adjust.rs @@ -494,102 +494,58 @@ mod tests { let mut pass = AdjustPass::new(&ctx); let img = grey_image(&ctx, 4000); - // A name paired with the edit that activates one operation. Boxed - // closures rather than a type alias: the list is read top to bottom - // as a table of what is covered. - #[allow(clippy::type_complexity)] - let cases: Vec<(&str, Box)> = vec![ - ( - "white_balance.temperature", - Box::new(|g: &mut EditGraph| { - g.set_param(white_balance::ID, white_balance::TEMPERATURE, 60.0) - }), - ), - ( - "white_balance.tint", - Box::new(|g: &mut EditGraph| { - g.set_param(white_balance::ID, white_balance::TINT, -40.0) - }), - ), - ( - "exposure", - Box::new(|g: &mut EditGraph| g.set_param(exposure::ID, exposure::EXPOSURE, 1.5)), - ), - ( - "highlights", - Box::new(|g: &mut EditGraph| { - g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::HIGHLIGHTS, -70.0) - }), - ), - ( - "shadows", - Box::new(|g: &mut EditGraph| { - g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::SHADOWS, 70.0) - }), - ), - ( - "blacks", - Box::new(|g: &mut EditGraph| { - g.set_param(tone::BLACKS_WHITES_ID, tone::BLACKS, -50.0) - }), - ), - ( - "whites", - Box::new(|g: &mut EditGraph| { - g.set_param(tone::BLACKS_WHITES_ID, tone::WHITES, 50.0) - }), - ), - ( - "brilliance", - Box::new(|g: &mut EditGraph| { - g.set_param(colour::BRILLIANCE_ID, colour::BRILLIANCE, 60.0) - }), - ), - ( - "vibrance", - Box::new(|g: &mut EditGraph| { - g.set_param(colour::VIBRANCE_ID, colour::VIBRANCE, 60.0) - }), - ), - ( - "saturation", - Box::new(|g: &mut EditGraph| { - g.set_param(colour::SATURATION_ID, colour::SATURATION, 60.0) - }), - ), - ]; - - for (name, apply) in cases { - let mut g = EditGraph::default_chain(); - apply(&mut g); - let shader = g.compose(); - pass.render(&img, &shader, 16, 16) - .unwrap_or_else(|e| panic!("{name} generated invalid WGSL:\n{e}")); + // Cases derived from the chain itself rather than a hand-written + // list: every parameter of every operation is exercised, and adding + // an operation extends the coverage automatically instead of + // silently going untested. + let probe = EditGraph::default_chain(); + for cap in probe.capabilities() { + for p in &cap.params { + let dr_pipeline::ParamKind::Scalar { min, max, .. } = p.kind else { + continue; + }; + // Both extremes: a fragment can be valid at one end of its + // range and not the other. + for value in [min, max] { + let mut g = EditGraph::default_chain(); + g.set_param(cap.id, p.id, value); + let shader = g.compose(); + pass.render(&img, &shader, 16, 16).unwrap_or_else(|e| { + panic!( + "{}.{} at {value} generated invalid WGSL:\n{e}", + cap.id, p.id + ) + }); + } + } } } #[test] fn the_whole_chain_at_once_compiles() { // Individually-valid fragments can still collide when combined — - // duplicate helpers, clashing locals, a malformed uniform block. + // duplicate helpers, clashing locals, a malformed uniform block. With + // every operation active this is the largest shader the pipeline can + // generate. let Some(ctx) = ctx() else { return }; let mut pass = AdjustPass::new(&ctx); let img = grey_image(&ctx, 4000); let mut g = EditGraph::default_chain(); - g.set_param(white_balance::ID, white_balance::TEMPERATURE, 30.0); - g.set_param(white_balance::ID, white_balance::TINT, -20.0); - g.set_param(exposure::ID, exposure::EXPOSURE, 0.8); - g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::HIGHLIGHTS, -60.0); - g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::SHADOWS, 40.0); - g.set_param(tone::BLACKS_WHITES_ID, tone::BLACKS, -25.0); - g.set_param(tone::BLACKS_WHITES_ID, tone::WHITES, 35.0); - g.set_param(colour::BRILLIANCE_ID, colour::BRILLIANCE, 45.0); - g.set_param(colour::VIBRANCE_ID, colour::VIBRANCE, 55.0); - g.set_param(colour::SATURATION_ID, colour::SATURATION, 15.0); + for cap in EditGraph::default_chain().capabilities() { + for p in &cap.params { + if let dr_pipeline::ParamKind::Scalar { max, .. } = p.kind { + g.set_param(cap.id, p.id, max * 0.6); + } + } + } let shader = g.compose(); - assert_eq!(shader.source.matches("---- ").count(), 7); + assert_eq!( + shader.source.matches("---- ").count(), + g.descriptors().len(), + "every operation should be active" + ); pass.render(&img, &shader, 32, 32) .expect("the full chain must compile"); } diff --git a/core/dr-pipeline/src/descriptor.rs b/core/dr-pipeline/src/descriptor.rs index b7a9174..79f0217 100644 --- a/core/dr-pipeline/src/descriptor.rs +++ b/core/dr-pipeline/src/descriptor.rs @@ -113,6 +113,37 @@ impl ParamDescriptor { } } + /// A toggle. Neutral when off, so the reset contract still holds. + pub const fn switch(id: &'static str, label: &'static str) -> Self { + Self { + id: ParamId(id), + label: LocalizedKey(label), + kind: ParamKind::Bool, + default: 0.0, + } + } + + /// A 0…1 fraction — a proportion of something, rather than an amount. + /// + /// Its own constructor because the crop rect needs four of them and the + /// default differs per edge: an origin starts at 0 and an extent at 1. + /// Precision of 4 because at 6000px a step of 0.0001 is under a pixel, + /// and a coarser one would make a crop edge unplaceable. + pub const fn fraction(id: &'static str, label: &'static str, default: f32) -> Self { + Self { + id: ParamId(id), + label: LocalizedKey(label), + kind: ParamKind::Scalar { + min: 0.0, + max: 1.0, + scale: Scale::Linear, + unit: Unit::None, + precision: 4, + }, + default, + } + } + /// A general scalar with an explicit range and default. // // Eight arguments, and a builder would be the usual answer — but this has diff --git a/core/dr-pipeline/src/framing.rs b/core/dr-pipeline/src/framing.rs new file mode 100644 index 0000000..7d5d99e --- /dev/null +++ b/core/dr-pipeline/src/framing.rs @@ -0,0 +1,943 @@ +//! Framing — crop, straighten, rotate, flip (FR-DEV-3, ARCH §5.2). +//! +//! # Why this is not an `Operation`, and not a `Warp` either +//! +//! An [`crate::operation::Operation`] is a function from colour to colour. By +//! the time one runs, the colour has been sampled and the question framing +//! asks — *which* source pixel does this output pixel come from — has already +//! been answered. And a crop changes the output's dimensions and aspect +//! ratio, which no colour fragment can express. +//! +//! [`crate::warp::Warp`] is closer: it also rewrites coordinates before the +//! fetch. But a warp is a *correction to the optics* — distortion and CA are +//! properties of the lens, defined about the optical axis, over the whole +//! frame the lens projected. Framing is a decision about *composition*, made +//! afterwards. The order matters and is not a preference: +//! +//! ```text +//! output pixel → framing → warp (lens) → sample → colour ops → output +//! ``` +//! +//! Reading forward, the lens is corrected on the full frame and the crop +//! then selects from the corrected result. Correcting distortion on an +//! already-cropped frame would place the optical centre in the wrong spot and +//! bend the image about a point the lens never saw. +//! +//! So framing runs **first** in the coordinate chain, and hands the warp +//! chain exactly the space it documents: normalised, centred, `r == 1` at the +//! corner. Neither stage needs to know the other exists. +//! +//! # Why sampling changes with the angle +//! +//! At 90° steps and flips, output pixels land exactly on source pixels, so +//! the map is a permutation and an integer `textureLoad` is both correct and +//! lossless. At any other angle it is not, and nearest-neighbour sampling +//! makes a straightened horizon visibly stair-step — the commonest use of +//! this stage, and where the artefact is most obvious. Free angles therefore +//! need interpolation, which is what [`Framing::needs_interpolation`] tells +//! the composer. Paying for it only when the angle demands it keeps the +//! common case exact rather than merely close. + +use std::f32::consts::PI; +use std::fmt::Write as _; + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId, Scale, Unit}; +use crate::operation::Affects; + +pub const ID: OpId = OpId("framing"); + +pub const ANGLE: ParamId = ParamId("angle"); +pub const ROTATION: ParamId = ParamId("rotation"); +pub const FLIP_H: ParamId = ParamId("flip_h"); +pub const FLIP_V: ParamId = ParamId("flip_v"); +pub const CROP_X: ParamId = ParamId("crop_x"); +pub const CROP_Y: ParamId = ParamId("crop_y"); +pub const CROP_W: ParamId = ParamId("crop_w"); +pub const CROP_H: ParamId = ParamId("crop_h"); + +/// Widest straightening the control offers, in degrees either way. +/// +/// Straightening a horizon is a small correction; a gross reorientation is +/// what the 90° steps are for. Bounding it keeps the slider's travel where +/// the edits actually are. +pub const MAX_STRAIGHTEN: f32 = 45.0; + +static DESCRIPTOR: OpDescriptor = OpDescriptor { + id: ID, + label: LocalizedKey("op.framing"), + params: &[ + // Straightening. Degrees rather than a normalised amount because a + // photographer reading "-1.4°" off a horizon knows what it means. + ParamDescriptor::scalar( + "angle", + "param.angle", + -MAX_STRAIGHTEN, + MAX_STRAIGHTEN, + 0.0, + Unit::None, + Scale::Linear, + 2, + ), + // Quarter turns, 0..3. Separate from `angle` because these are exact + // and lossless, and because reorienting a frame is a different + // gesture from nudging a horizon. + ParamDescriptor::scalar( + "rotation", + "param.rotation", + 0.0, + 3.0, + 0.0, + Unit::None, + Scale::Linear, + 0, + ), + ParamDescriptor::switch("flip_h", "param.flip_h"), + ParamDescriptor::switch("flip_v", "param.flip_v"), + // The crop rect, in fractions of the source. Normalised rather than + // in pixels so a crop survives being applied to a proxy, a full + // resolution render, or an export at another size — the same reason + // the viewport renders at display resolution (FR-DSP-1). + ParamDescriptor::fraction("crop_x", "param.crop_x", 0.0), + ParamDescriptor::fraction("crop_y", "param.crop_y", 0.0), + ParamDescriptor::fraction("crop_w", "param.crop_w", 1.0), + ParamDescriptor::fraction("crop_h", "param.crop_h", 1.0), + ], +}; + +/// A normalised crop rectangle, in fractions of the source image. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct CropRect { + pub x: f32, + pub y: f32, + pub width: f32, + pub height: f32, +} + +impl Default for CropRect { + fn default() -> Self { + Self { + x: 0.0, + y: 0.0, + width: 1.0, + height: 1.0, + } + } +} + +impl CropRect { + /// Smallest crop the rect may be reduced to, as a fraction of the source. + /// + /// A zero-extent crop produces a zero-sized output texture, which is a + /// device error rather than a visibly silly image. Bounding it here means + /// no caller has to defend against it. + pub const MIN_EXTENT: f32 = 0.01; + + /// Whether this rect selects the whole image. + pub fn is_full(&self) -> bool { + self.x == 0.0 && self.y == 0.0 && self.width == 1.0 && self.height == 1.0 + } + + /// Clamp into the unit square, keeping the rect non-degenerate. + /// + /// The origin is clamped first and the extent fitted to what remains, so + /// a rect dragged past an edge slides rather than inverting. + pub fn normalised(self) -> Self { + let x = finite(self.x, 0.0).clamp(0.0, 1.0 - Self::MIN_EXTENT); + let y = finite(self.y, 0.0).clamp(0.0, 1.0 - Self::MIN_EXTENT); + let width = finite(self.width, 1.0).clamp(Self::MIN_EXTENT, 1.0 - x); + let height = finite(self.height, 1.0).clamp(Self::MIN_EXTENT, 1.0 - y); + Self { + x, + y, + width, + height, + } + } +} + +/// Replace a non-finite value with a fallback. +/// +/// A NaN reaching the crop rect would propagate into the output *dimensions*, +/// not merely the pixels — `NaN as u32` is 0, and a zero-sized texture is a +/// device error. The same defence as `ParamDescriptor::clamp`, one level up. +fn finite(v: f32, fallback: f32) -> f32 { + if v.is_finite() { + v + } else { + fallback + } +} + +/// Crop, straighten, rotation and flips for one image. +/// +/// Holds no GPU state: like the rest of the graph this is CPU-side, so a lost +/// device is recovered by re-composing rather than by re-deriving the edit +/// (ARCH §6.10). +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct Framing { + /// Straightening, in degrees. Positive rotates the image clockwise. + angle: f32, + /// Quarter turns clockwise, 0..=3. + quarter_turns: u8, + flip_h: bool, + flip_v: bool, + crop: CropRect, +} + +impl Default for Framing { + fn default() -> Self { + Self { + angle: 0.0, + quarter_turns: 0, + flip_h: false, + flip_v: false, + crop: CropRect::default(), + } + } +} + +impl Framing { + pub fn new() -> Self { + Self::default() + } + + pub fn descriptor(&self) -> &'static OpDescriptor { + &DESCRIPTOR + } + + /// What this stage affects, for invalidation scoping (FR-DEV-3d). + pub fn affects(&self) -> Affects { + Affects::Geometry + } + + pub fn crop(&self) -> CropRect { + self.crop + } + + pub fn set_crop(&mut self, rect: CropRect) { + self.crop = rect.normalised(); + } + + pub fn angle(&self) -> f32 { + self.angle + } + + pub fn quarter_turns(&self) -> u8 { + self.quarter_turns + } + + pub fn flips(&self) -> (bool, bool) { + (self.flip_h, self.flip_v) + } + + /// Add quarter turns, wrapping. The rotate-left/right buttons. + pub fn rotate_quarters(&mut self, turns: i32) { + self.quarter_turns = (i32::from(self.quarter_turns) + turns).rem_euclid(4) as u8; + } + + /// Whether this stage currently changes the image. + /// + /// The same contract the operations honour: neutral framing contributes + /// nothing to the generated shader, so an uncropped image reads its + /// pixels through the identity map exactly as it did before this existed. + pub fn is_active(&self) -> bool { + self.angle != 0.0 + || self.quarter_turns != 0 + || self.flip_h + || self.flip_v + || !self.crop.is_full() + } + + /// Whether the axes are swapped — a 90° or 270° turn. + fn swaps_axes(&self) -> bool { + self.quarter_turns % 2 == 1 + } + + /// Whether the map puts output pixels between source pixels. + /// + /// False for quarter turns and flips, which are permutations with an + /// exact answer. True once a free angle is involved. The composer reads + /// this to decide between an integer load and a filtered sample — and a + /// warp being active forces interpolation regardless, which is the + /// composer's call to make rather than this stage's. + pub fn needs_interpolation(&self) -> bool { + self.angle != 0.0 + } + + pub fn set_param(&mut self, id: ParamId, value: f32) { + match id { + ANGLE => self.angle = finite(value, 0.0), + // Descriptor-clamped to 0..3, so the cast cannot wrap. + ROTATION => self.quarter_turns = finite(value, 0.0).round().clamp(0.0, 3.0) as u8, + FLIP_H => self.flip_h = value != 0.0, + FLIP_V => self.flip_v = value != 0.0, + CROP_X => { + self.crop = CropRect { + x: value, + ..self.crop + } + .normalised() + } + CROP_Y => { + self.crop = CropRect { + y: value, + ..self.crop + } + .normalised() + } + CROP_W => { + self.crop = CropRect { + width: value, + ..self.crop + } + .normalised() + } + CROP_H => { + self.crop = CropRect { + height: value, + ..self.crop + } + .normalised() + } + _ => log::warn!("framing: unknown parameter {id}"), + } + } + + pub fn param(&self, id: ParamId) -> f32 { + match id { + ANGLE => self.angle, + ROTATION => f32::from(self.quarter_turns), + FLIP_H => f32::from(u8::from(self.flip_h)), + FLIP_V => f32::from(u8::from(self.flip_v)), + CROP_X => self.crop.x, + CROP_Y => self.crop.y, + CROP_W => self.crop.width, + CROP_H => self.crop.height, + _ => 0.0, + } + } + + pub fn reset(&mut self) { + *self = Self::default(); + } + + /// The output size this framing produces from a source of `(w, h)`. + /// + /// The rendered aspect ratio follows from here, which is why this is the + /// one piece of framing both the UI and the GPU pass need before any + /// pixel is shaded: the output texture is allocated from it. + /// + /// A free angle does **not** change the output size. The rotated image is + /// sampled into the crop rect as it stands, so straightening a horizon + /// leaves the frame where the user put it and may pull in undefined area + /// at the corners — see [`Self::max_inscribed_crop`] for the rect that + /// avoids that. + pub fn output_size(&self, width: u32, height: u32) -> (u32, u32) { + let (w, h) = if self.swaps_axes() { + (height, width) + } else { + (width, height) + }; + // Round rather than truncate: half of a 101px axis should be 51, and + // truncation biases every crop smaller. + let cw = ((w as f32 * self.crop.width).round() as u32).max(1); + let ch = ((h as f32 * self.crop.height).round() as u32).max(1); + (cw, ch) + } + + /// The largest centred crop, at the current angle, containing no + /// undefined area. + /// + /// Rotating a rectangle inside its own bounds exposes the corners: there + /// is no source pixel there, and the shader renders it black. This is the + /// rect that avoids it — what a "straighten and auto-crop" gesture would + /// apply, and what the crop overlay should offer as its bound. + /// + /// The standard largest-inscribed-rectangle result for a rotated + /// rectangle of the same aspect ratio. + pub fn max_inscribed_crop(&self, width: u32, height: u32) -> CropRect { + if self.angle == 0.0 || width == 0 || height == 0 { + return CropRect::default(); + } + + let (w, h) = if self.swaps_axes() { + (height as f32, width as f32) + } else { + (width as f32, height as f32) + }; + + let a = (self.angle * PI / 180.0).abs(); + let (sin, cos) = (a.sin(), a.cos()); + + // Longer and shorter side, so the two cases below stay symmetric. + let (long, short) = if w >= h { (w, h) } else { (h, w) }; + + let (bw, bh) = if short <= 2.0 * sin * cos * long || (sin - cos).abs() < 1e-6 { + // Half-constrained: the shorter side alone limits the rectangle. + let half = 0.5 * short; + if w >= h { + (half / sin, half / cos) + } else { + (half / cos, half / sin) + } + } else { + // Fully constrained by both sides. + let cos2 = cos * cos - sin * sin; + ((w * cos - h * sin) / cos2, (h * cos - w * sin) / cos2) + }; + + // Back to fractions of the (possibly axis-swapped) frame, centred. + let fw = (bw / w).clamp(CropRect::MIN_EXTENT, 1.0); + let fh = (bh / h).clamp(CropRect::MIN_EXTENT, 1.0); + CropRect { + x: (1.0 - fw) * 0.5, + y: (1.0 - fh) * 0.5, + width: fw, + height: fh, + } + .normalised() + } + + /// Uniform values the generated prologue reads. + /// + /// A fixed-size block in a fixed slot, like the camera matrix: the + /// prologue is emitted whether or not any operation is active, so its + /// uniforms cannot be positioned by the op loop. + /// + /// The angle reaches the shader as sin/cos rather than degrees — a trig + /// call per pixel would recover a value constant across the dispatch. + pub fn uniforms(&self) -> [f32; FRAMING_UNIFORM_FIELDS] { + let rad = self.angle * PI / 180.0; + [ + self.crop.x, + self.crop.y, + self.crop.width, + self.crop.height, + rad.sin(), + rad.cos(), + 0.0, + 0.0, + ] + } + + /// The WGSL mapping an output pixel to a **normalised centred** source + /// position, ready for the warp chain. + /// + /// Leaves the result in `p`: centre `(0, 0)`, `r == 1` at the corner — + /// exactly the space [`crate::warp`] documents, so lens correction + /// composes on top of this without either stage naming the other. + /// + /// `aspect` is left in scope alongside it, since the warp chain and the + /// sampler both need it to return to texture coordinates. + pub fn wgsl_prologue(&self) -> String { + let mut s = String::new(); + + s.push_str( + " // ---- framing ---- + // Output pixel -> source position, in the normalised centred space the + // warp chain expects: the centre is (0, 0) and the radius is 1 at the + // corner. Working here rather than in pixels is what makes the map + // independent of the resolution being rendered at. + let src_dims = textureDimensions(source); + let aspect = vec2(f32(src_dims.x) / f32(src_dims.y), 1.0); + var uv = (vec2(gid.xy) + vec2(0.5)) / vec2(dims); +", + ); + + if !self.is_active() { + // Neutral framing still has to produce `p`, since the warp chain + // and the sampler read it either way. It is only the crop, + // rotation and flip steps that vanish. + s.push_str( + " + // Framing is neutral: the whole frame, unrotated. + var p = (uv - vec2(0.5)) * aspect; +", + ); + return s; + } + + s.push_str( + " + // Into the crop rect. + uv = u.crop_rect.xy + uv * u.crop_rect.zw; + var p = (uv - vec2(0.5)) * aspect; +", + ); + + if self.angle != 0.0 { + // Done in the aspect-corrected space, which is the whole reason + // `p` is scaled by `aspect` above: a rotation applied to raw 0..1 + // coordinates on a non-square image shears it rather than + // turning it, and that reads as a rendering fault. + s.push_str( + " + // Straighten, about the frame centre. + p = vec2( + p.x * u.framing_angle.y - p.y * u.framing_angle.x, + p.x * u.framing_angle.x + p.y * u.framing_angle.y, + ); +", + ); + } + + if self.quarter_turns != 0 { + // An exact coordinate permutation rather than a rotation through + // the matrix above, which would resample a transform that has an + // exact answer. Applied to `p`, so the aspect scaling has to be + // undone and reapplied across the swap. + let permutation = match self.quarter_turns { + 1 => " p = vec2(p.y * aspect.x, -p.x / aspect.x);", + 2 => " p = -p;", + _ => " p = vec2(-p.y * aspect.x, p.x / aspect.x);", + }; + let _ = write!( + s, + " + // {}° clockwise — an exact permutation, so nothing is resampled. +{permutation} +", + u32::from(self.quarter_turns) * 90 + ); + } + + if self.flip_h { + s.push_str(" p.x = -p.x;\n"); + } + if self.flip_v { + s.push_str(" p.y = -p.y;\n"); + } + + s + } + + /// Identifies this framing's *structure* — which branches the prologue + /// generates, not the values it reads. + /// + /// Deliberately coarse, for the reason the operation hash is: dragging + /// the crop handles or the straighten slider must reuse the compiled + /// pipeline and upload uniforms only. Only the presence of each + /// transform, never its magnitude, may enter this. + pub fn structure_key(&self) -> u64 { + u64::from(!self.crop.is_full()) + | u64::from(self.angle != 0.0) << 1 + | u64::from(self.flip_h) << 2 + | u64::from(self.flip_v) << 3 + | u64::from(self.quarter_turns) << 4 + } +} + +/// Floats the framing block occupies in the generated uniform struct. +/// +/// Two `vec4`s: the crop rect, and the angle's sin/cos with padding. +pub const FRAMING_UNIFORM_FIELDS: usize = 8; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_fresh_framing_is_neutral() { + // The invariant behind "opening an image shows the image". + let f = Framing::new(); + assert!(!f.is_active()); + assert!(!f.needs_interpolation()); + assert_eq!(f.output_size(6000, 4000), (6000, 4000)); + } + + #[test] + fn neutral_framing_still_produces_a_position_for_the_warp_chain() { + // The prologue always defines `p` and `aspect`, active or not — the + // warp chain and the sampler read them either way, so a neutral + // framing that skipped them would fail to compile rather than + // rendering an unframed image. + let src = Framing::new().wgsl_prologue(); + assert!(src.contains("var p ="), "{src}"); + assert!(src.contains("let aspect ="), "{src}"); + // ...but none of the transform steps. + assert!(!src.contains("crop_rect")); + assert!(!src.contains("framing_angle")); + } + + #[test] + fn an_active_framing_reads_the_crop_rect() { + let mut f = Framing::new(); + f.set_crop(CropRect { + x: 0.1, + y: 0.1, + width: 0.5, + height: 0.5, + }); + assert!(f.wgsl_prologue().contains("u.crop_rect")); + } + + #[test] + fn cropping_changes_the_output_size() { + let mut f = Framing::new(); + f.set_crop(CropRect { + x: 0.25, + y: 0.25, + width: 0.5, + height: 0.5, + }); + assert!(f.is_active()); + assert_eq!(f.output_size(1000, 800), (500, 400)); + } + + #[test] + fn a_quarter_turn_swaps_the_output_axes() { + // What makes a landscape frame come out portrait: the output is + // genuinely taller than it is wide. + let mut f = Framing::new(); + f.rotate_quarters(1); + assert_eq!(f.output_size(6000, 4000), (4000, 6000)); + + f.rotate_quarters(1); + assert_eq!(f.output_size(6000, 4000), (6000, 4000)); + } + + #[test] + fn crop_applies_within_the_rotated_frame() { + // Half of a rotated frame must be half of the *rotated* dimensions, + // or a crop drawn on screen after a rotation lands somewhere else. + let mut f = Framing::new(); + f.rotate_quarters(1); + f.set_crop(CropRect { + x: 0.0, + y: 0.0, + width: 0.5, + height: 1.0, + }); + assert_eq!(f.output_size(6000, 4000), (2000, 6000)); + } + + #[test] + fn quarter_turns_wrap_in_both_directions() { + let mut f = Framing::new(); + f.rotate_quarters(-1); + assert_eq!(f.quarter_turns(), 3); + f.rotate_quarters(1); + assert_eq!(f.quarter_turns(), 0); + f.rotate_quarters(7); + assert_eq!(f.quarter_turns(), 3); + } + + #[test] + fn a_crop_cannot_be_driven_degenerate() { + // A zero-extent crop produces a zero-sized texture, which is a device + // error rather than a visibly silly image. + let mut f = Framing::new(); + f.set_crop(CropRect { + x: 0.5, + y: 0.5, + width: 0.0, + height: 0.0, + }); + let (w, h) = f.output_size(1000, 1000); + assert!(w >= 1 && h >= 1); + assert!(f.crop().width >= CropRect::MIN_EXTENT); + } + + #[test] + fn a_crop_pushed_past_the_edge_stays_inside() { + let mut f = Framing::new(); + f.set_crop(CropRect { + x: 0.8, + y: 0.9, + width: 0.5, + height: 0.5, + }); + let c = f.crop(); + assert!(c.x + c.width <= 1.0 + 1e-6, "{c:?} extends past the edge"); + assert!(c.y + c.height <= 1.0 + 1e-6, "{c:?} extends past the edge"); + } + + #[test] + fn a_nan_crop_falls_back_rather_than_producing_a_zero_texture() { + // Worse than a wrong image: `NaN as u32` is 0, and a zero-sized + // texture is a device error. + let mut f = Framing::new(); + f.set_param(CROP_W, f32::NAN); + f.set_param(CROP_X, f32::INFINITY); + let (w, h) = f.output_size(1000, 1000); + assert!(w >= 1 && h >= 1); + assert!(f.crop().width.is_finite() && f.crop().x.is_finite()); + } + + #[test] + fn output_size_rounds_rather_than_truncating() { + // Truncation biases every crop smaller; half of 101 should be 51. + let mut f = Framing::new(); + f.set_crop(CropRect { + x: 0.0, + y: 0.0, + width: 0.5, + height: 0.5, + }); + assert_eq!(f.output_size(101, 101), (51, 51)); + } + + #[test] + fn quarter_turns_and_flips_need_no_interpolation() { + // Why 90° steps are handled apart from the free angle: they have an + // exact answer and must not be resampled. + let mut f = Framing::new(); + f.rotate_quarters(1); + f.set_param(FLIP_H, 1.0); + assert!(f.is_active()); + assert!(!f.needs_interpolation()); + } + + #[test] + fn a_free_angle_needs_interpolation() { + let mut f = Framing::new(); + f.set_param(ANGLE, 1.5); + assert!(f.needs_interpolation()); + assert!(f.wgsl_prologue().contains("u.framing_angle")); + } + + #[test] + fn a_quarter_turn_corrects_for_aspect_across_the_swap() { + // `p` is scaled by the source aspect, so a permutation that exchanges + // the axes has to undo and reapply it. Without that a 90° turn on a + // 3:2 frame comes out stretched. + let mut f = Framing::new(); + f.rotate_quarters(1); + assert!(f.wgsl_prologue().contains("aspect.x")); + } + + #[test] + fn the_structure_key_ignores_magnitudes() { + // What the pipeline cache depends on: dragging the straighten slider + // or the crop handles must not recompile. + let mut a = Framing::new(); + a.set_param(ANGLE, 1.0); + let mut b = Framing::new(); + b.set_param(ANGLE, 4.0); + assert_eq!(a.structure_key(), b.structure_key()); + assert_eq!(a.wgsl_prologue(), b.wgsl_prologue()); + assert_ne!(a.uniforms(), b.uniforms()); + + let mut c = Framing::new(); + c.set_crop(CropRect { + x: 0.1, + y: 0.1, + width: 0.5, + height: 0.5, + }); + let mut d = Framing::new(); + d.set_crop(CropRect { + x: 0.2, + y: 0.2, + width: 0.4, + height: 0.4, + }); + assert_eq!(c.structure_key(), d.structure_key()); + assert_eq!(c.wgsl_prologue(), d.wgsl_prologue()); + } + + #[test] + fn different_transforms_take_different_structure_keys() { + // The other half of the cache contract: framing that generates + // different code must not reuse another's pipeline. + let mut seen = std::collections::BTreeSet::new(); + seen.insert(Framing::new().structure_key()); + + let mut cropped = Framing::new(); + cropped.set_crop(CropRect { + x: 0.1, + y: 0.1, + width: 0.5, + height: 0.5, + }); + assert!(seen.insert(cropped.structure_key())); + + let mut angled = Framing::new(); + angled.set_param(ANGLE, 2.0); + assert!(seen.insert(angled.structure_key())); + + let mut flipped = Framing::new(); + flipped.set_param(FLIP_H, 1.0); + assert!(seen.insert(flipped.structure_key())); + + for turns in 1..=3 { + let mut f = Framing::new(); + f.rotate_quarters(turns); + assert!(seen.insert(f.structure_key()), "{turns} quarter turns"); + } + } + + #[test] + fn parameters_round_trip() { + let mut f = Framing::new(); + for (id, v) in [ + (ANGLE, 2.5), + (ROTATION, 2.0), + (FLIP_H, 1.0), + (FLIP_V, 1.0), + (CROP_X, 0.1), + (CROP_Y, 0.2), + (CROP_W, 0.5), + (CROP_H, 0.4), + ] { + f.set_param(id, v); + assert_eq!(f.param(id), v, "{id} did not round-trip"); + } + } + + #[test] + fn every_default_leaves_the_stage_neutral() { + // The same contract the operations honour, checked against the + // descriptor rather than a literal. + let mut f = Framing::new(); + for p in DESCRIPTOR.params { + f.set_param(p.id, p.default); + } + assert!(!f.is_active(), "descriptor defaults must be neutral"); + } + + #[test] + fn every_default_is_within_its_declared_range() { + for p in DESCRIPTOR.params { + assert_eq!(p.clamp(p.default), p.default, "{} is out of range", p.id); + } + } + + #[test] + fn no_parameter_is_declared_twice() { + let mut ids: Vec<&str> = DESCRIPTOR.params.iter().map(|p| p.id.0).collect(); + let before = ids.len(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(before, ids.len(), "framing has a duplicate parameter"); + } + + #[test] + fn reset_returns_to_neutral() { + let mut f = Framing::new(); + f.set_param(ANGLE, 3.0); + f.rotate_quarters(1); + f.set_crop(CropRect { + x: 0.1, + y: 0.1, + width: 0.3, + height: 0.3, + }); + assert!(f.is_active()); + + f.reset(); + assert!(!f.is_active()); + assert_eq!(f.wgsl_prologue(), Framing::new().wgsl_prologue()); + } + + #[test] + fn uniforms_carry_the_angle_as_sin_and_cos() { + // The shader never sees degrees: converting here keeps a trig call + // out of every pixel. + let mut f = Framing::new(); + f.set_param(ANGLE, 90.0); + let u = f.uniforms(); + assert!( + (u[4] - 1.0).abs() < 1e-6, + "sin(90°) should be 1, got {}", + u[4] + ); + assert!(u[5].abs() < 1e-6, "cos(90°) should be 0, got {}", u[5]); + } + + #[test] + fn uniforms_are_always_finite() { + // One NaN in the uniform block blanks every pixel. + let mut f = Framing::new(); + f.set_param(ANGLE, f32::NAN); + f.set_param(CROP_W, f32::NAN); + assert!( + f.uniforms().iter().all(|v| v.is_finite()), + "{:?}", + f.uniforms() + ); + } + + #[test] + fn the_uniform_block_is_vec4_aligned() { + // Emitted as whole `vec4`s; a size not divisible by four would + // misalign every operation uniform that follows it. + assert_eq!(FRAMING_UNIFORM_FIELDS % 4, 0); + assert_eq!(Framing::new().uniforms().len(), FRAMING_UNIFORM_FIELDS); + } + + #[test] + fn the_inscribed_crop_of_an_unrotated_image_is_the_whole_frame() { + assert!(Framing::new().max_inscribed_crop(6000, 4000).is_full()); + } + + #[test] + fn the_inscribed_crop_shrinks_as_the_angle_grows() { + // Straightening further must cut in further; anything else leaves + // undefined corners inside the frame. + let mut small = Framing::new(); + small.set_param(ANGLE, 2.0); + let mut large = Framing::new(); + large.set_param(ANGLE, 10.0); + + let a = small.max_inscribed_crop(6000, 4000); + let b = large.max_inscribed_crop(6000, 4000); + assert!(a.width > b.width, "{} should exceed {}", a.width, b.width); + assert!(a.width < 1.0, "a rotated frame cannot keep its full width"); + } + + #[test] + fn the_inscribed_crop_is_centred_and_inside_the_frame() { + for angle in [1.0f32, 5.0, 15.0, 30.0, 45.0, -7.5] { + let mut f = Framing::new(); + f.set_param(ANGLE, angle); + for (w, h) in [(6000u32, 4000u32), (4000, 6000), (3000, 3000)] { + let c = f.max_inscribed_crop(w, h); + assert!( + c.width > 0.0 && c.height > 0.0, + "{angle}° on {w}x{h}: {c:?} is degenerate" + ); + assert!( + c.x + c.width <= 1.0 + 1e-4 && c.y + c.height <= 1.0 + 1e-4, + "{angle}° on {w}x{h}: {c:?} extends past the frame" + ); + assert!( + ((c.x + c.width * 0.5) - 0.5).abs() < 1e-4, + "{angle}° on {w}x{h}: {c:?} is not centred" + ); + } + } + } + + #[test] + fn the_inscribed_crop_contains_no_undefined_area() { + // The property the derivation exists for, checked directly: every + // corner of the inscribed rect, mapped through the same transform the + // shader applies, must land inside the source. + for angle in [1.0f32, 5.0, 15.0, 30.0, 45.0, -12.0] { + let mut f = Framing::new(); + f.set_param(ANGLE, angle); + let (w, h) = (6000.0f32, 4000.0f32); + let c = f.max_inscribed_crop(6000, 4000); + + let rad = angle * PI / 180.0; + let (sn, cs) = (rad.sin(), rad.cos()); + let aspect = w / h; + + for (fx, fy) in [ + (c.x, c.y), + (c.x + c.width, c.y), + (c.x, c.y + c.height), + (c.x + c.width, c.y + c.height), + ] { + let (px, py) = ((fx - 0.5) * aspect, fy - 0.5); + let (rx, ry) = (px * cs - py * sn, px * sn + py * cs); + let (ux, uy) = (rx / aspect + 0.5, ry + 0.5); + assert!( + (-1e-3..=1.0 + 1e-3).contains(&ux) && (-1e-3..=1.0 + 1e-3).contains(&uy), + "{angle}°: corner ({fx}, {fy}) maps to ({ux}, {uy}), outside the source" + ); + } + } + } +} diff --git a/core/dr-pipeline/src/graph.rs b/core/dr-pipeline/src/graph.rs index b4b0f69..df16f1e 100644 --- a/core/dr-pipeline/src/graph.rs +++ b/core/dr-pipeline/src/graph.rs @@ -8,7 +8,8 @@ //! so reordering the pipeline needs no code change. use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamId, ParamKind}; -use crate::operation::{compose, ComposedShader, Operation}; +use crate::framing::{CropRect, Framing}; +use crate::operation::{compose_with_framing, ComposedShader, Operation}; use crate::ops; /// TRACES: FR-DEV-3a @@ -52,9 +53,16 @@ impl ParamCapability { } } -/// An ordered pipeline of operations. +/// An ordered pipeline of operations, plus how the result is framed. pub struct EditGraph { ops: Vec>, + /// Crop, straighten, rotation and flips. + /// + /// Held apart from `ops` rather than in the list because it is not one: + /// an operation transforms a colour, and framing decides which source + /// pixel that colour is read from — and changes the output's dimensions, + /// which no colour operation can do. See [`crate::framing`]. + framing: Framing, } impl EditGraph { @@ -70,17 +78,49 @@ impl EditGraph { ops: vec![ Box::new(ops::WhiteBalance::new()), Box::new(ops::Exposure::new()), + Box::new(ops::Contrast::new()), Box::new(ops::HighlightsShadows::new()), Box::new(ops::BlacksWhites::new()), Box::new(ops::Brilliance::new()), Box::new(ops::Vibrance::new()), Box::new(ops::Saturation::new()), + // The mixer comes last: it is the finishing control, and it + // should act on the tones the user has already settled. + Box::new(ops::ColourMixer::new()), ], + framing: Framing::new(), } } - /// Descriptors for every operation, in order. Drives panel generation - /// (FR-DEV-3a). + /// The framing — crop, straighten, rotation and flips. + /// + /// Reached directly rather than through `set_param` because the crop is a + /// rectangle, and driving one through four independent scalars makes an + /// interactive drag four clamps that can disagree. The parameter route + /// still exists for the sidecar, which has only scalars to work with. + pub fn framing(&self) -> &Framing { + &self.framing + } + + pub fn framing_mut(&mut self) -> &mut Framing { + &mut self.framing + } + + /// The size this graph renders to, given a source of `(w, h)`. + /// + /// Cropping and quarter turns change it, so the caller allocating the + /// output texture must ask rather than assume the source size. + pub fn output_size(&self, width: u32, height: u32) -> (u32, u32) { + self.framing.output_size(width, height) + } + + /// Descriptors for every operation, in order. + /// + /// Operations only — framing is not one, and is reached through + /// [`Self::framing`] or the capability list. The distinction matters here + /// because this is what the codegen tests count `---- ` shader blocks + /// against, and framing generates a prologue rather than a colour block. + /// A UI wanting everything should read [`Self::capabilities`] (FR-DEV-3a). pub fn descriptors(&self) -> Vec<&'static OpDescriptor> { self.ops.iter().map(|o| o.descriptor()).collect() } @@ -100,28 +140,47 @@ impl EditGraph { /// step, and so reopening an edited image shows where the sliders /// actually are. pub fn capabilities(&self) -> Vec { - self.ops - .iter() - .map(|op| { - let desc = op.descriptor(); - OpCapability { - id: desc.id, - label: desc.label, - active: op.is_active(), - params: desc - .params - .iter() - .map(|p| ParamCapability { - id: p.id, - label: p.label, - kind: p.kind.clone(), - default: p.default, - value: op.param(p.id), - }) - .collect(), - } - }) - .collect() + let ops = self.ops.iter().map(|op| { + let desc = op.descriptor(); + OpCapability { + id: desc.id, + label: desc.label, + active: op.is_active(), + params: desc + .params + .iter() + .map(|p| ParamCapability { + id: p.id, + label: p.label, + kind: p.kind.clone(), + default: p.default, + value: op.param(p.id), + }) + .collect(), + } + }); + + // Framing last, matching where it sits in the pipeline: the crop is + // decided after the image looks right, not before. + let desc = self.framing.descriptor(); + let framing = OpCapability { + id: desc.id, + label: desc.label, + active: self.framing.is_active(), + params: desc + .params + .iter() + .map(|p| ParamCapability { + id: p.id, + label: p.label, + kind: p.kind.clone(), + default: p.default, + value: self.framing.param(p.id), + }) + .collect(), + }; + + ops.chain(std::iter::once(framing)).collect() } /// Set a parameter, clamping to the descriptor's declared range. @@ -130,6 +189,15 @@ impl EditGraph { /// has to defend against an out-of-range value, and a corrupt sidecar /// cannot reach a shader. pub fn set_param(&mut self, op: OpId, param: ParamId, value: f32) { + if op == crate::framing::ID { + let Some(desc) = self.framing.descriptor().param(param) else { + log::warn!("unknown parameter {param} on {op}; ignoring"); + return; + }; + self.framing.set_param(param, desc.clamp(value)); + return; + } + let Some(operation) = self.ops.iter_mut().find(|o| o.descriptor().id == op) else { // A sidecar naming an operation this build does not have. The // rest of the edit must still apply. @@ -148,29 +216,51 @@ impl EditGraph { /// Read a parameter back. pub fn param(&self, op: OpId, param: ParamId) -> Option { + if op == crate::framing::ID { + return self + .framing + .descriptor() + .param(param) + .map(|_| self.framing.param(param)); + } self.ops .iter() .find(|o| o.descriptor().id == op) .map(|o| o.param(param)) } - /// Reset every parameter of every operation to its default. + /// Reset every parameter of every operation, and the framing, to default. pub fn reset(&mut self) { for op in &mut self.ops { for p in op.descriptor().params { op.set_param(p.id, p.default); } } + self.framing.reset(); } - /// Whether any operation currently changes the image. + /// Set the crop rectangle. Clamped to keep it inside the frame. + pub fn set_crop(&mut self, rect: CropRect) { + self.framing.set_crop(rect); + } + + pub fn crop(&self) -> CropRect { + self.framing.crop() + } + + /// Rotate by quarter turns, wrapping. The rotate-left/right buttons. + pub fn rotate_quarters(&mut self, turns: i32) { + self.framing.rotate_quarters(turns); + } + + /// Whether any operation, or the framing, currently changes the image. pub fn is_neutral(&self) -> bool { - !self.ops.iter().any(|o| o.is_active()) + !self.ops.iter().any(|o| o.is_active()) && !self.framing.is_active() } /// Generate the fused shader for the current state. pub fn compose(&self) -> ComposedShader { - compose(&self.ops) + compose_with_framing(&self.ops, &self.framing) } } @@ -406,8 +496,16 @@ mod tests { }) .collect(); - assert_eq!(rendered.len(), 10, "seven operations, ten parameters"); + // Counted from the chain, not a literal: this test must not need + // editing when an operation is added, or it would be asserting the + // opposite of what it claims. + let expected: usize = g.capabilities().iter().map(|c| c.params.len()).sum(); + assert_eq!(rendered.len(), expected); assert!(rendered.iter().all(|r| !r.is_empty())); + assert!( + expected > 40, + "the chain should now carry the mixer's 36 parameters too" + ); } #[test] diff --git a/core/dr-pipeline/src/lib.rs b/core/dr-pipeline/src/lib.rs index db81a8b..313af98 100644 --- a/core/dr-pipeline/src/lib.rs +++ b/core/dr-pipeline/src/lib.rs @@ -32,6 +32,7 @@ //! data neither would be physically meaningful (ARCH §5.2). pub mod descriptor; +pub mod framing; pub mod graph; pub mod operation; pub mod ops; @@ -39,8 +40,11 @@ pub mod ops; pub use descriptor::{ LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId, ParamKind, Scale, Unit, }; +pub use framing::{CropRect, Framing}; pub use graph::{EditGraph, OpCapability, ParamCapability}; -pub use operation::{compose, Affects, ComposedShader, Helper, Operation, Uniform}; +pub use operation::{ + compose, compose_with_framing, Affects, ComposedShader, Helper, Operation, Uniform, +}; #[cfg(test)] mod tests { @@ -67,10 +71,12 @@ mod tests { let g = fully_active(); assert!(!g.is_neutral()); let shader = g.compose(); + // Counted against the chain rather than a literal, so adding an + // operation does not require editing this test. assert_eq!( shader.source.matches("---- ").count(), - 7, - "all seven operations should appear" + g.descriptors().len(), + "every operation in the chain should appear" ); } @@ -148,16 +154,106 @@ mod tests { #[test] fn generated_uniform_names_are_valid_wgsl_identifiers() { let shader = fully_active().compose(); - for line in shader.source.lines() { + + // Only the `struct Params` block. Scanning the whole source picks up + // helper *signatures* such as `fn contrast_curve(x: f32, ...)`, whose + // parameters are not uniform declarations at all. + let body = shader + .source + .split_once("struct Params {") + .expect("a uniform struct is always generated") + .1 + .split_once('}') + .expect("the struct is closed") + .0; + + let mut checked = 0; + for line in body.lines() { let trimmed = line.trim(); - let Some((name, _)) = trimmed.split_once(": f32,") else { + if trimmed.starts_with("//") { + continue; + } + let Some((name, _)) = trimmed.split_once(':') else { continue; }; + let name = name.trim(); assert!( - name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') + !name.is_empty() + && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') && !name.starts_with(|c: char| c.is_ascii_digit()), "{name} is not a valid WGSL identifier" ); + checked += 1; + } + assert!(checked > 0, "the struct should declare fields"); + } + + #[test] + fn no_fragment_declares_a_wgsl_reserved_keyword() { + // Caught the hard way: `let target = ...` in the contrast fragment + // failed to compile with "name `target` is a reserved keyword", and + // the error pointed at generated source rather than at the operation + // that wrote it. Checking here names the culprit directly. + // + // Not the full reserved list — the ones a colour operation would + // plausibly reach for. + const RESERVED: &[&str] = &[ + "target", + "sample", + "filter", + "texture", + "buffer", + "binding", + "const", + "enum", + "mat", + "vec", + "ptr", + "ref", + "shared", + "static", + "typedef", + "union", + "unless", + "handle", + "layout", + "packed", + "premerge", + "regardless", + "typedef", + "active", + "do", + "enum", + "input", + "output", + "private", + "resource", + "restrict", + "self", + "std", + "where", + ]; + + let g = EditGraph::default_chain(); + for cap in g.capabilities() { + // Activate the whole operation so its fragment is emitted. + let mut probe = EditGraph::default_chain(); + for p in &cap.params { + if let ParamKind::Scalar { max, .. } = p.kind { + probe.set_param(cap.id, p.id, max * 0.5); + } + } + let source = probe.compose().source; + + for keyword in RESERVED { + let declaration = format!("let {keyword} "); + let var_declaration = format!("var {keyword} "); + assert!( + !source.contains(&declaration) && !source.contains(&var_declaration), + "{} declares `{keyword}`, which is a WGSL reserved keyword", + cap.id + ); + } } } diff --git a/core/dr-pipeline/src/operation.rs b/core/dr-pipeline/src/operation.rs index 42ff530..a58f0ab 100644 --- a/core/dr-pipeline/src/operation.rs +++ b/core/dr-pipeline/src/operation.rs @@ -23,6 +23,7 @@ use std::fmt::Write as _; use crate::descriptor::{OpDescriptor, ParamId}; +use crate::framing::{Framing, FRAMING_UNIFORM_FIELDS}; /// What an operation's parameters affect, for cache invalidation scoping. /// @@ -131,7 +132,19 @@ const BASE_UNIFORM_FIELDS: usize = 16; /// /// Inactive operations are skipped entirely — they contribute no code, no /// uniforms, and nothing to the structure hash. +/// +/// Equivalent to [`compose_with_framing`] with neutral framing. pub fn compose(ops: &[Box]) -> ComposedShader { + compose_with_framing(ops, &Framing::new()) +} + +/// Compose operations and framing into a single compute shader. +/// +/// Framing generates the shader's **prologue** — the map from an output pixel +/// back to a source position — where [`compose`] would emit a fixed identity +/// scale. The fused-dispatch property is unaffected: a cropped, straightened +/// edit with three adjustments is still one dispatch, one read, one write. +pub fn compose_with_framing(ops: &[Box], framing: &Framing) -> ComposedShader { let active: Vec<&dyn Operation> = ops .iter() .map(|o| o.as_ref()) @@ -157,6 +170,18 @@ pub fn compose(ops: &[Box]) -> ComposedShader { ); uniform_values.resize(BASE_UNIFORM_FIELDS, 0.0); + // Framing's block follows the base one at a fixed offset, for the same + // reason: the prologue is emitted whether or not any operation is active, + // so these slots cannot be positioned by the op loop below. + uniform_fields.push_str( + " // Framing: the crop rect (origin, extent) and the straightening\n\ + \x20 // angle as sin/cos — a trig call per pixel would recompute a\n\ + \x20 // value that is constant across the dispatch.\n\ + \x20 crop_rect: vec4,\n\ + \x20 framing_angle: vec4,\n", + ); + uniform_values.extend_from_slice(&framing.uniforms()); + for op in &active { let id = op.descriptor().id.0; let prefix = sanitise(id); @@ -206,6 +231,19 @@ pub fn compose(ops: &[Box]) -> ComposedShader { let _ = writeln!(helper_src, "{}\n", h.source.trim_end()); } + // The coordinate stage: output pixel -> source position -> colour. Emitted + // ahead of the operation fragments, which receive the sampled `c`. + let prologue = format!( + "{}\n{}", + framing.wgsl_prologue(), + sample_source(framing.needs_interpolation()) + ); + let sampler_helper = if framing.needs_interpolation() { + BILINEAR_HELPER + } else { + "" + }; + let source = format!( "// GENERATED — do not edit. // @@ -221,7 +259,7 @@ struct Params {{ @group(0) @binding(1) var u: Params; @group(0) @binding(2) var output: texture_storage_2d; -{helper_src}// Linear sRGB to the display transfer function. +{sampler_helper}{helper_src}// Linear sRGB to the display transfer function. // // The one place quantisation happens: everything above runs in linear f16, // and this is the final encode (ARCH §5.2). @@ -238,14 +276,7 @@ fn main(@builtin(global_invocation_id) gid: vec3) {{ return; }} - // Source is the demosaiced image: linear, scene-referred, camera space. - let src_dims = textureDimensions(source); - let coord = vec2( - i32(gid.x * src_dims.x / dims.x), - i32(gid.y * src_dims.y / dims.y), - ); - var c = textureLoad(source, coord, 0).rgb; - +{prologue} // As-shot white balance. Applied unconditionally, before any operation, // because it is part of *interpreting* the sensor rather than an edit: a // Bayer sensor's green photosites collect far more signal than its red @@ -271,7 +302,10 @@ fn main(@builtin(global_invocation_id) gid: vec3) {{ active.len() ); - let structure_hash = hash_structure(&active); + // Framing enters the hash by structure only — which branches its prologue + // generated, never how far a slider moved. Dragging the crop handles must + // reuse the compiled pipeline and re-upload uniforms. + let structure_hash = mix(hash_structure(&active), framing.structure_key()); ComposedShader { source, @@ -280,6 +314,83 @@ fn main(@builtin(global_invocation_id) gid: vec3) {{ } } +/// The WGSL turning the framed source position `p` into the colour `c`. +/// +/// Split out because it is the join between the coordinate stage and the +/// colour stage, and because the choice it makes — an exact integer load, or +/// a filtered sample — is the one thing the free-angle case changes. +fn sample_source(interpolate: bool) -> &'static str { + if interpolate { + " // Back to texture coordinates. + let uv_src = p / aspect + vec2(0.5); + + // Outside the source there is no pixel. A straightened frame exposes its + // corners; render them black rather than clamping, which would smear an + // edge pixel across them. + if (any(uv_src < vec2(0.0)) || any(uv_src >= vec2(1.0))) { + textureStore(output, vec2(gid.xy), vec4(0.0, 0.0, 0.0, 1.0)); + return; + } + + // A free angle puts output pixels between source pixels. Nearest-neighbour + // here is what makes a straightened horizon stair-step, so interpolate. + var c = sample_bilinear(uv_src, src_dims); +" + } else { + " // Back to texture coordinates. + let uv_src = p / aspect + vec2(0.5); + + // Outside the source there is no pixel — possible once the frame has been + // transformed at all. Render it black rather than clamping, which would + // smear an edge pixel across the gap. + if (any(uv_src < vec2(0.0)) || any(uv_src >= vec2(1.0))) { + textureStore(output, vec2(gid.xy), vec4(0.0, 0.0, 0.0, 1.0)); + return; + } + + // Every output pixel lands on a source pixel, so load it directly: exact, + // and with no interpolation to soften detail. + let coord = min(vec2(uv_src * vec2(src_dims)), vec2(src_dims) - vec2(1)); + var c = textureLoad(source, coord, 0).rgb; +" + } +} + +/// Bilinear sampling against an unfiltered `texture_2d`. +/// +/// Hand-rolled rather than done with a sampler: the source is bound as a plain +/// texture, and adding a sampler for the straightening case alone would change +/// a bind group layout that every pass shares. +const BILINEAR_HELPER: &str = "fn sample_bilinear(uv: vec2, dims: vec2) -> vec3 { + let last = vec2(dims) - vec2(1); + + // Half-texel offset: sample positions are texel *centres*. Without it the + // image shifts by half a pixel and every rotation comes out slightly soft. + let q = uv * vec2(dims) - vec2(0.5); + let base = floor(q); + let f = q - base; + let i0 = clamp(vec2(base), vec2(0), last); + let i1 = min(i0 + vec2(1), last); + + let c00 = textureLoad(source, vec2(i0.x, i0.y), 0).rgb; + let c10 = textureLoad(source, vec2(i1.x, i0.y), 0).rgb; + let c01 = textureLoad(source, vec2(i0.x, i1.y), 0).rgb; + let c11 = textureLoad(source, vec2(i1.x, i1.y), 0).rgb; + + return mix(mix(c00, c10, f.x), mix(c01, c11, f.x), f.y); +} + +"; + +/// Fold a value into a hash. FNV-1a's mixing step, over eight bytes. +fn mix(mut h: u64, value: u64) -> u64 { + for byte in value.to_le_bytes() { + h ^= u64::from(byte); + h = h.wrapping_mul(0x100_0000_01b3); + } + h +} + /// Hash the op-set and order — the structure, not the values. /// /// Two edits with the same operations at different slider positions produce @@ -305,13 +416,29 @@ fn hash_structure(active: &[&dyn Operation]) -> u64 { /// /// Whole-word matching matters: an operation with uniforms `amount` and /// `amount_hi` must not have the first rewrite corrupt the second. -fn rewrite_uniform(src: &str, name: &str, replacement: &str) -> String { +/// +/// Shared with [`crate::warp`], which prefixes its uniforms by the same rule +/// and must not diverge from it. +/// Comments are skipped. A fragment explaining what `factor` does should not +/// have its prose rewritten to `u.saturation_factor` — the generated source +/// is meant to be read when a shader fails to compile, and mangled comments +/// make that harder rather than easier. +pub(crate) fn rewrite_uniform(src: &str, name: &str, replacement: &str) -> String { let mut out = String::with_capacity(src.len()); let bytes = src.as_bytes(); let mut i = 0; + // Tracks whether the cursor sits inside a `//` comment. WGSL fragments + // use line comments only, so this needs no block-comment handling. + let mut in_comment = false; while i < src.len() { - if src[i..].starts_with(name) { + if bytes[i] == b'\n' { + in_comment = false; + } else if !in_comment && src[i..].starts_with("//") { + in_comment = true; + } + + if !in_comment && src[i..].starts_with(name) { let before_ok = i == 0 || !is_ident_byte(bytes[i - 1]); let after = i + name.len(); let after_ok = after >= src.len() || !is_ident_byte(bytes[after]); @@ -511,6 +638,33 @@ mod tests { assert_eq!(got, "x = u.p_amount + amount_hi;"); } + #[test] + fn rewriting_leaves_comments_alone() { + // Found in generated source: a comment reading "A factor of 0 is + // monochrome" came out as "A u.saturation_factor of 0 is monochrome". + // The generated source is what gets read when a shader fails to + // compile, so mangling it works against the one time it matters. + let got = rewrite_uniform( + "// A factor of 0 is monochrome\nc = c * factor;", + "factor", + "u.op_factor", + ); + assert_eq!(got, "// A factor of 0 is monochrome\nc = c * u.op_factor;"); + } + + #[test] + fn rewriting_resumes_after_a_comment_ends() { + let got = rewrite_uniform( + "// factor here is prose\nlet x = factor;\n// factor again\n", + "factor", + "u.p", + ); + assert_eq!( + got, + "// factor here is prose\nlet x = u.p;\n// factor again\n" + ); + } + #[test] fn rewriting_leaves_substrings_alone() { let got = rewrite_uniform("total_amount = 1.0;", "amount", "u.a"); diff --git a/core/dr-pipeline/src/ops/colour_mixer.rs b/core/dr-pipeline/src/ops/colour_mixer.rs new file mode 100644 index 0000000..baefacb --- /dev/null +++ b/core/dr-pipeline/src/ops/colour_mixer.rs @@ -0,0 +1,611 @@ +//! The colour mixer — twelve hue bands, each with hue, saturation and +//! luminance. +//! +//! The control photographers mean by "per-colour adjustment": pick a colour +//! range, then shift its hue, deepen or mute it, or lighten it, without +//! touching the rest of the image. Thirty-six parameters in one operation. +//! +//! # Why bands overlap +//! +//! Each band has a centre hue and influences colours near it with a weight +//! that falls smoothly to zero at its neighbours' centres. A hard assignment +//! — "this pixel is orange, that one is yellow" — puts a visible seam through +//! any gradient crossing a boundary, and skies and skin are exactly where +//! that shows. Overlapping weights mean adjacent bands blend, and a colour +//! halfway between two centres receives half of each. +//! +//! # Why the weights are normalised +//! +//! With overlap, a pixel's weights sum to more than one, so applying each +//! band's gain independently would compound them. The shader normalises, so +//! setting every band's saturation to +100 gives the same result as setting +//! the global saturation to +100 rather than something far stronger. + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId}; +use crate::operation::{Helper, Operation, Uniform}; +use crate::ops::helpers; + +pub const ID: OpId = OpId("colour_mixer"); + +/// The twelve bands, in hue order starting at red. +/// +/// Twelve rather than Lightroom's eight: the extra bands fall between the +/// primaries and secondaries, which is where skin (orange-to-red) and +/// foliage (yellow-to-green) actually sit, and where eight bands force a +/// compromise. +pub struct Band { + /// Stable id fragment, used to build parameter ids. + pub key: &'static str, + /// Centre hue in degrees. + pub hue: f32, +} + +pub static BANDS: [Band; 12] = [ + Band { + key: "red", + hue: 0.0, + }, + Band { + key: "orange", + hue: 30.0, + }, + Band { + key: "yellow", + hue: 60.0, + }, + Band { + key: "chartreuse", + hue: 90.0, + }, + Band { + key: "green", + hue: 120.0, + }, + Band { + key: "spring", + hue: 150.0, + }, + Band { + key: "cyan", + hue: 180.0, + }, + Band { + key: "azure", + hue: 210.0, + }, + Band { + key: "blue", + hue: 240.0, + }, + Band { + key: "violet", + hue: 270.0, + }, + Band { + key: "magenta", + hue: 300.0, + }, + Band { + key: "rose", + hue: 330.0, + }, +]; + +/// The three adjustments each band carries. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Channel { + Hue, + Saturation, + Luminance, +} + +impl Channel { + pub const ALL: [Channel; 3] = [Channel::Hue, Channel::Saturation, Channel::Luminance]; + + pub const fn suffix(self) -> &'static str { + match self { + Channel::Hue => "hue", + Channel::Saturation => "sat", + Channel::Luminance => "lum", + } + } +} + +// Parameter descriptors, one per band per channel. Written out rather than +// generated because `ParamDescriptor` must be `const` to live in a `static`, +// and a const loop cannot build a slice. The macro keeps it honest. +macro_rules! band_params { + ($($key:literal),* $(,)?) => { + &[ + $( + ParamDescriptor::amount( + concat!($key, "_hue"), + concat!("param.mixer.", $key, ".hue"), + ), + ParamDescriptor::amount( + concat!($key, "_sat"), + concat!("param.mixer.", $key, ".sat"), + ), + ParamDescriptor::amount( + concat!($key, "_lum"), + concat!("param.mixer.", $key, ".lum"), + ), + )* + ] + }; +} + +static DESCRIPTOR: OpDescriptor = OpDescriptor { + id: ID, + label: LocalizedKey("op.colour_mixer"), + params: band_params![ + "red", + "orange", + "yellow", + "chartreuse", + "green", + "spring", + "cyan", + "azure", + "blue", + "violet", + "magenta", + "rose", + ], +}; + +static MIXER_HELPERS: &[Helper] = &[ + helpers::LUMINANCE, + Helper { + name: "rgb_to_hcl", + source: "\ +// Hue (degrees), chroma, and the max channel, in one pass. +// +// Not a full HSL conversion: the mixer needs hue to weight the bands and +// chroma to know how much colour there is to adjust, and computing lightness +// separately from Rec. 709 luminance gives a better-behaved result than +// HSL's (max+min)/2. +fn rgb_to_hcl(c: vec3) -> vec3 { + let hi = max(c.r, max(c.g, c.b)); + let lo = min(c.r, min(c.g, c.b)); + let chroma = hi - lo; + + var hue = 0.0; + if (chroma > 0.00001) { + if (hi == c.r) { + // fract handles the wrap from -60 to 300 without a branch. + hue = 60.0 * fract(((c.g - c.b) / chroma) / 6.0 + 1.0) * 6.0 / 6.0; + hue = 60.0 * (((c.g - c.b) / chroma) % 6.0); + if (hue < 0.0) { hue = hue + 360.0; } + } else if (hi == c.g) { + hue = 60.0 * (((c.b - c.r) / chroma) + 2.0); + } else { + hue = 60.0 * (((c.r - c.g) / chroma) + 4.0); + } + } + return vec3(hue, chroma, hi); +}", + }, + Helper { + name: "band_weight", + source: "\ +// How strongly a hue belongs to a band centred at `centre`. +// +// Cosine falloff over +/-60 degrees, so a band reaches zero exactly at its +// neighbours' centres and adjacent weights sum to one across the gap. A +// narrower window would leave hues between bands unreachable; a wider one +// would make every adjustment affect the whole wheel. +fn band_weight(hue: f32, centre: f32) -> f32 { + // Shortest angular distance, accounting for the wrap at 360. + var d = abs(hue - centre); + if (d > 180.0) { d = 360.0 - d; } + if (d >= 60.0) { return 0.0; } + // cos ramp: 1 at the centre, 0 at 60 degrees. + return 0.5 + 0.5 * cos(d * 3.14159265 / 60.0); +}", + }, + Helper { + name: "hue_to_rgb_scale", + source: "\ +// Rebuild a colour after shifting its hue, preserving chroma and level. +// +// Reconstructing from HSV rather than rotating in RGB: an RGB rotation +// matrix desaturates as it turns, which is visible as colours going pale +// mid-shift. +fn hue_to_rgb_scale(hue: f32, chroma: f32, hi: f32) -> vec3 { + let h = fract(hue / 360.0) * 6.0; + let x = chroma * (1.0 - abs((h % 2.0) - 1.0)); + var rgb = vec3(0.0); + if (h < 1.0) { rgb = vec3(chroma, x, 0.0); } + else if (h < 2.0) { rgb = vec3(x, chroma, 0.0); } + else if (h < 3.0) { rgb = vec3(0.0, chroma, x); } + else if (h < 4.0) { rgb = vec3(0.0, x, chroma); } + else if (h < 5.0) { rgb = vec3(x, 0.0, chroma); } + else { rgb = vec3(chroma, 0.0, x); } + return rgb + vec3(hi - chroma); +}", + }, +]; + +/// Twelve hue bands, each with hue, saturation and luminance. +#[derive(Debug, Clone)] +pub struct ColourMixer { + /// `[band][channel]`, matching [`BANDS`] and [`Channel::ALL`]. + values: [[f32; 3]; 12], +} + +impl Default for ColourMixer { + fn default() -> Self { + Self { + values: [[0.0; 3]; 12], + } + } +} + +impl ColourMixer { + pub fn new() -> Self { + Self::default() + } + + /// The parameter id for one band and channel. + /// + /// Ids are `"_"`, matching the descriptors above. + fn index_of(id: ParamId) -> Option<(usize, usize)> { + let (band, channel) = id.0.rsplit_once('_')?; + let b = BANDS.iter().position(|x| x.key == band)?; + let c = Channel::ALL.iter().position(|x| x.suffix() == channel)?; + Some((b, c)) + } + + /// Whether any band has a non-zero setting. + fn any_set(&self) -> bool { + self.values.iter().flatten().any(|v| *v != 0.0) + } +} + +impl Operation for ColourMixer { + fn descriptor(&self) -> &'static OpDescriptor { + &DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match Self::index_of(id) { + Some((b, c)) => self.values[b][c] = value, + None => log::warn!("colour_mixer: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + Self::index_of(id).map_or(0.0, |(b, c)| self.values[b][c]) + } + + fn is_active(&self) -> bool { + self.any_set() + } + + fn wgsl_body(&self) -> String { + // Only the bands the user actually touched contribute code. A single + // adjusted band therefore costs one weight evaluation rather than + // twelve — the composition property applied within an operation. + let mut lines = String::from( + "\ +let hcl = rgb_to_hcl(c); +let hue = hcl.x; +let chroma = hcl.y; +let hi = hcl.z; + +// Achromatic pixels have no hue to match, and adjusting them would tint +// neutrals — the most visible way a mixer can go wrong. +if (chroma > 0.0001) { + var w_total = 0.0; + var d_hue = 0.0; + var d_sat = 0.0; + var d_lum = 0.0; +", + ); + + for (b, band) in BANDS.iter().enumerate() { + let v = self.values[b]; + if v.iter().all(|x| *x == 0.0) { + continue; + } + let key = band.key; + lines.push_str(&format!( + "\n // {key}\n {{\n let w = band_weight(hue, {:.1});\n w_total = w_total + w;\n", + band.hue + )); + if v[0] != 0.0 { + lines.push_str(&format!(" d_hue = d_hue + w * {key}_hue;\n")); + } + if v[1] != 0.0 { + lines.push_str(&format!(" d_sat = d_sat + w * {key}_sat;\n")); + } + if v[2] != 0.0 { + lines.push_str(&format!(" d_lum = d_lum + w * {key}_lum;\n")); + } + lines.push_str(" }\n"); + } + + lines.push_str( + " + // Normalise by the total weight, so overlapping bands blend rather than + // compound. Without this, a hue sitting between two adjusted bands would + // receive roughly twice the intended adjustment. + if (w_total > 0.0001) { + d_hue = d_hue / w_total; + d_sat = d_sat / w_total; + d_lum = d_lum / w_total; + + // Hue: up to 30 degrees at full travel. Enough to move foliage from + // yellow-green to green, not enough to turn it blue by accident. + let new_hue = hue + d_hue * 30.0; + + // Saturation scales chroma; luminance scales the whole colour. + let new_chroma = clamp(chroma * (1.0 + d_sat), 0.0, hi); + c = hue_to_rgb_scale(new_hue, new_chroma, hi); + c = c * exp2(d_lum); + } +} +c = max(c, vec3(0.0));", + ); + + lines + } + + fn uniforms(&self) -> Vec { + // Only the bands that contributed code declare uniforms, and in the + // same order the fragment references them. + let mut out = Vec::new(); + for b in 0..BANDS.len() { + let v = self.values[b]; + if v.iter().all(|x| *x == 0.0) { + continue; + } + // Names must match those the fragment emitted. + if v[0] != 0.0 { + out.push(Uniform { + name: HUE_NAMES[b], + value: v[0] / 100.0, + }); + } + if v[1] != 0.0 { + out.push(Uniform { + name: SAT_NAMES[b], + value: v[1] / 100.0, + }); + } + if v[2] != 0.0 { + out.push(Uniform { + name: LUM_NAMES[b], + // Up to half a stop per band. + value: v[2] / 100.0 * 0.5, + }); + } + } + out + } + + fn helpers(&self) -> &'static [Helper] { + MIXER_HELPERS + } +} + +// Uniform names must be `&'static str`, and they are built from the band +// keys. Declared as tables rather than formatted at runtime, so the fragment +// and the uniform list cannot disagree. +static HUE_NAMES: [&str; 12] = [ + "red_hue", + "orange_hue", + "yellow_hue", + "chartreuse_hue", + "green_hue", + "spring_hue", + "cyan_hue", + "azure_hue", + "blue_hue", + "violet_hue", + "magenta_hue", + "rose_hue", +]; +static SAT_NAMES: [&str; 12] = [ + "red_sat", + "orange_sat", + "yellow_sat", + "chartreuse_sat", + "green_sat", + "spring_sat", + "cyan_sat", + "azure_sat", + "blue_sat", + "violet_sat", + "magenta_sat", + "rose_sat", +]; +static LUM_NAMES: [&str; 12] = [ + "red_lum", + "orange_lum", + "yellow_lum", + "chartreuse_lum", + "green_lum", + "spring_lum", + "cyan_lum", + "azure_lum", + "blue_lum", + "violet_lum", + "magenta_lum", + "rose_lum", +]; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn there_are_twelve_bands_with_thirty_six_parameters() { + assert_eq!(BANDS.len(), 12); + assert_eq!(DESCRIPTOR.params.len(), 36); + } + + #[test] + fn bands_are_evenly_spaced_around_the_wheel() { + // Uneven spacing would leave some hues weakly covered, since the + // weight window is a fixed 60 degrees. + for (i, band) in BANDS.iter().enumerate() { + assert!( + (band.hue - i as f32 * 30.0).abs() < 1e-6, + "{} is at {}, expected {}", + band.key, + band.hue, + i as f32 * 30.0 + ); + } + } + + #[test] + fn every_descriptor_id_resolves_to_a_band_and_channel() { + // The link between the descriptor list and the value array. A + // mismatch would make a slider silently adjust nothing. + for p in DESCRIPTOR.params { + assert!( + ColourMixer::index_of(p.id).is_some(), + "{} does not map to a band", + p.id + ); + } + } + + #[test] + fn every_band_and_channel_has_a_descriptor() { + // The reverse direction: a band with no descriptor is unreachable + // from the UI. + for band in BANDS.iter() { + for ch in Channel::ALL { + let id = format!("{}_{}", band.key, ch.suffix()); + assert!( + DESCRIPTOR.params.iter().any(|p| p.id.0 == id), + "{id} has no descriptor" + ); + } + } + } + + #[test] + fn the_uniform_name_tables_match_the_band_keys() { + // Three parallel tables and a band list; if they drift, the fragment + // references a uniform that was never declared and the shader fails + // to compile. + for (i, band) in BANDS.iter().enumerate() { + assert_eq!(HUE_NAMES[i], format!("{}_hue", band.key)); + assert_eq!(SAT_NAMES[i], format!("{}_sat", band.key)); + assert_eq!(LUM_NAMES[i], format!("{}_lum", band.key)); + } + } + + #[test] + fn a_fresh_mixer_is_inactive() { + assert!(!ColourMixer::new().is_active()); + } + + #[test] + fn setting_any_band_activates_it() { + let mut m = ColourMixer::new(); + m.set_param(ParamId("blue_sat"), 40.0); + assert!(m.is_active()); + assert_eq!(m.param(ParamId("blue_sat")), 40.0); + } + + #[test] + fn only_adjusted_bands_reach_the_shader() { + // The composition property applied within an operation: adjusting + // one band must not cost twelve weight evaluations. + let mut m = ColourMixer::new(); + m.set_param(ParamId("blue_sat"), 40.0); + let body = m.wgsl_body(); + + assert!(body.contains("blue_sat"), "the adjusted band must appear"); + assert!(!body.contains("red_sat"), "untouched bands must not"); + assert_eq!( + body.matches("band_weight(").count(), + 1, + "one adjusted band means one weight evaluation" + ); + } + + #[test] + fn only_adjusted_channels_within_a_band_reach_the_shader() { + let mut m = ColourMixer::new(); + m.set_param(ParamId("green_lum"), -25.0); + let body = m.wgsl_body(); + assert!(body.contains("green_lum")); + assert!(!body.contains("green_hue")); + assert!(!body.contains("green_sat")); + } + + #[test] + fn the_fragment_and_uniforms_agree_on_names() { + // The failure this prevents is a compile error in generated code, + // which is far harder to read than a failed assertion here. + let mut m = ColourMixer::new(); + m.set_param(ParamId("orange_hue"), 20.0); + m.set_param(ParamId("orange_sat"), -30.0); + m.set_param(ParamId("azure_lum"), 15.0); + + let body = m.wgsl_body(); + for u in m.uniforms() { + assert!( + body.contains(u.name), + "uniform {} is declared but never used", + u.name + ); + } + // And nothing referenced without being declared. + let declared: Vec<&str> = m.uniforms().iter().map(|u| u.name).collect(); + for band in BANDS.iter() { + for ch in Channel::ALL { + let name = format!("{}_{}", band.key, ch.suffix()); + if body.contains(&name) { + assert!( + declared.contains(&name.as_str()), + "{name} is used but not declared" + ); + } + } + } + } + + #[test] + fn achromatic_pixels_are_excluded() { + // Adjusting a hue-less pixel would tint neutrals, which is the most + // visible way a mixer misbehaves. + let mut m = ColourMixer::new(); + m.set_param(ParamId("red_sat"), 50.0); + assert!(m.wgsl_body().contains("chroma > 0.0001")); + } + + #[test] + fn overlapping_weights_are_normalised() { + // Without normalising, a hue between two adjusted bands gets roughly + // double the intended adjustment. + let mut m = ColourMixer::new(); + m.set_param(ParamId("red_sat"), 50.0); + m.set_param(ParamId("orange_sat"), 50.0); + let body = m.wgsl_body(); + assert!(body.contains("d_sat / w_total")); + } + + #[test] + fn unknown_parameters_are_ignored() { + let mut m = ColourMixer::new(); + m.set_param(ParamId("puce_sat"), 50.0); + m.set_param(ParamId("malformed"), 50.0); + assert!(!m.is_active()); + } + + #[test] + fn luminance_travel_is_bounded_to_half_a_stop() { + let mut m = ColourMixer::new(); + m.set_param(ParamId("blue_lum"), 100.0); + let v = m.uniforms()[0].value; + assert!((v - 0.5).abs() < 1e-6, "got {v}"); + } +} diff --git a/core/dr-pipeline/src/ops/contrast.rs b/core/dr-pipeline/src/ops/contrast.rs new file mode 100644 index 0000000..b97e649 --- /dev/null +++ b/core/dr-pipeline/src/ops/contrast.rs @@ -0,0 +1,213 @@ +//! 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. + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId}; +use crate::operation::{Helper, Operation, Uniform}; +use crate::ops::helpers; + +pub const ID: OpId = OpId("contrast"); +pub const CONTRAST: ParamId = ParamId("contrast"); + +static DESCRIPTOR: OpDescriptor = OpDescriptor { + id: ID, + label: LocalizedKey("op.contrast"), + params: &[ParamDescriptor::amount("contrast", "param.contrast")], +}; + +/// The helpers this operation needs, including its own S-curve. +static CONTRAST_HELPERS: &[Helper] = &[ + helpers::LUMINANCE, + helpers::APPLY_TONE_GAIN, + Helper { + name: "contrast_curve", + source: "\ +// 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); +}", + }, +]; + +#[derive(Debug, Default, Clone)] +pub struct Contrast { + amount: f32, +} + +impl Contrast { + pub fn new() -> Self { + Self::default() + } +} + +impl Operation for Contrast { + fn descriptor(&self) -> &'static OpDescriptor { + &DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + CONTRAST => self.amount = value, + _ => log::warn!("contrast: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + CONTRAST => self.amount, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.amount != 0.0 + } + + fn wgsl_body(&self) -> String { + "\ +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));" + .into() + } + + fn uniforms(&self) -> Vec { + vec![Uniform { + name: "amount", + value: self.amount / 100.0, + }] + } + + fn helpers(&self) -> &'static [Helper] { + CONTRAST_HELPERS + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::operation::compose; + + #[test] + fn neutral_does_nothing() { + let c = Contrast::new(); + assert!(!c.is_active()); + assert_eq!(c.uniforms()[0].value, 0.0); + } + + #[test] + fn the_amount_is_normalised_to_unit_range() { + // The shader's curve expects -1..1; the descriptor speaks -100..100. + let mut c = Contrast::new(); + c.set_param(CONTRAST, 100.0); + assert!((c.uniforms()[0].value - 1.0).abs() < 1e-6); + c.set_param(CONTRAST, -100.0); + assert!((c.uniforms()[0].value + 1.0).abs() < 1e-6); + } + + #[test] + fn contrast_works_on_luminance_not_per_channel() { + // Curving each channel separately shifts hue; the ratio form is what + // keeps a blue sky blue as contrast rises. + let mut c = Contrast::new(); + c.set_param(CONTRAST, 50.0); + let body = c.wgsl_body(); + assert!(body.contains("luminance(c)")); + assert!( + body.contains("apply_tone_gain"), + "the colour must be scaled by a ratio, not curved per channel" + ); + } + + #[test] + fn the_pivot_is_middle_grey_not_half() { + // 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. + let c = Contrast::new(); + assert!( + c.wgsl_body().contains("0.36"), + "the curve must pivot about middle grey (0.18, doubled to place \ + it at the curve's own midpoint)" + ); + } + + #[test] + fn the_curve_cannot_invert() { + // A gain-about-a-pivot form produces a non-monotonic curve past + // moderate settings, which inverts tones. smoothstep cannot. + let helper = CONTRAST_HELPERS + .iter() + .find(|h| h.name == "contrast_curve") + .expect("declares its curve"); + assert!(helper.source.contains("3.0 - 2.0 * clamped")); + } + + #[test] + fn it_composes_with_the_other_tonal_operations() { + // Contrast, highlights/shadows and brilliance all want `luminance`; + // the composer must emit it once. + let ops: Vec> = vec![ + Box::new({ + let mut o = Contrast::new(); + o.set_param(CONTRAST, 40.0); + o + }), + Box::new({ + let mut o = crate::ops::HighlightsShadows::new(); + o.set_param(crate::ops::tone::HIGHLIGHTS, -30.0); + o + }), + ]; + let shader = compose(&ops); + assert_eq!(shader.source.matches("fn luminance(").count(), 1); + assert_eq!(shader.source.matches("fn apply_tone_gain(").count(), 1); + assert_eq!(shader.source.matches("fn contrast_curve(").count(), 1); + } + + #[test] + fn a_division_by_luminance_is_guarded() { + // A black pixel has zero luminance; dividing by it would produce NaN + // and propagate through everything downstream. + assert!( + Contrast::new().wgsl_body().contains("luma > 0.0001"), + "the ratio must be guarded against black pixels" + ); + } +} diff --git a/core/dr-pipeline/src/ops/distortion.rs b/core/dr-pipeline/src/ops/distortion.rs new file mode 100644 index 0000000..7c70f0b --- /dev/null +++ b/core/dr-pipeline/src/ops/distortion.rs @@ -0,0 +1,336 @@ +//! Geometric distortion correction. +//! +//! Straightens the lines a lens bends: barrel distortion on wide angles, +//! pincushion on telephotos. A [`crate::warp::Warp`] rather than an +//! [`crate::operation::Operation`], because it changes *where* a pixel is read +//! from rather than what its value becomes. +//! +//! # The model +//! +//! Lensfun's `ptlens` model, matched deliberately so a lens profile from the +//! Lensfun database applies with no conversion: +//! +//! ```text +//! r_d = r_u · (a·r_u³ + b·r_u² + c·r_u + 1 − a − b − c) +//! ``` +//! +//! The `1 − a − b − c` term is not decoration: it forces the polynomial to +//! equal 1 at `r_u = 1`, pinning the image corner in place. Without it every +//! coefficient change would also rescale the frame, so the distortion slider +//! would double as a zoom and no setting would leave the framing alone. +//! +//! `a` and `b` are the higher-order terms that describe a lens's real, +//! slightly wavy profile; `c` alone gives the simple barrel/pincushion shape. +//! The manual control drives `c` only — a single slider cannot meaningfully +//! set three correlated coefficients, and hand-correcting a lens with no +//! profile is a "make the horizon straight" task, which one term does well. +//! The full triple is reachable by loading a profile. + +use crate::descriptor::{ + LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId, Scale, Unit, +}; +use crate::operation::{Helper, Uniform}; +use crate::warp::Warp; + +pub const ID: OpId = OpId("distortion"); +pub const AMOUNT: ParamId = ParamId("amount"); + +static DESCRIPTOR: OpDescriptor = OpDescriptor { + id: ID, + label: LocalizedKey("op.distortion"), + // ±100 maps to a ±0.25 cubic coefficient. That covers an uncorrected + // fisheye at one end and strong pincushion at the other; beyond it the + // inverse mapping stops being single-valued near the corners and the + // correction folds the image over itself. + params: &[ParamDescriptor::scalar( + "amount", + "param.distortion.amount", + -100.0, + 100.0, + 0.0, + Unit::None, + Scale::Linear, + 0, + )], +}; + +/// The cubic coefficient at full slider travel. +const MAX_COEFF: f32 = 0.25; + +#[derive(Debug, Default, Clone)] +pub struct Distortion { + amount: f32, + /// Profile coefficients, when a lens profile is loaded. `None` means the + /// manual slider drives `c` alone. + profile: Option, +} + +/// The three `ptlens` coefficients, as Lensfun stores them. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct PtLens { + pub a: f32, + pub b: f32, + pub c: f32, +} + +impl Distortion { + pub fn new() -> Self { + Self::default() + } + + /// Apply a lens profile's coefficients. + /// + /// The manual slider then acts as a *trim* on top: photographers routinely + /// find a profile slightly over- or under-corrects on their copy of a + /// lens, and having to choose between "profile" and "manual" would make + /// that untunable. + pub fn set_profile(&mut self, profile: Option) { + self.profile = profile; + } + + /// The effective coefficients: profile plus manual trim. + fn coefficients(&self) -> PtLens { + let trim = self.amount / 100.0 * MAX_COEFF; + match self.profile { + Some(p) => PtLens { + a: p.a, + b: p.b, + c: p.c + trim, + }, + None => PtLens { + a: 0.0, + b: 0.0, + c: trim, + }, + } + } +} + +impl Warp for Distortion { + fn descriptor(&self) -> &'static OpDescriptor { + &DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + AMOUNT => self.amount = value, + _ => log::warn!("distortion: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + AMOUNT => self.amount, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + // A loaded profile corrects even with the slider at zero — that is + // the whole point of a profile. + let c = self.coefficients(); + c.a != 0.0 || c.b != 0.0 || c.c != 0.0 + } + + fn wgsl_body(&self) -> String { + // Written against `p`, which is already normalised and centred. + "\ +let r = length(p); +p = p * ptlens_scale(r, dist_a, dist_b, dist_c);" + .into() + } + + fn uniforms(&self) -> Vec { + let c = self.coefficients(); + vec![ + Uniform { + name: "dist_a", + value: c.a, + }, + Uniform { + name: "dist_b", + value: c.b, + }, + Uniform { + name: "dist_c", + value: c.c, + }, + ] + } + + fn helpers(&self) -> &'static [Helper] { + PTLENS + } +} + +static PTLENS: &[Helper] = &[Helper { + name: "ptlens_scale", + source: "\ +// The `ptlens` radial polynomial (Lensfun's model). +// +// Returns the factor mapping an undistorted radius to the distorted radius +// it should be sampled from. The trailing `1 - a - b - c` normalises the +// polynomial to 1 at r = 1, which pins the corner and stops a coefficient +// change from also rescaling the frame. +fn ptlens_scale(r: f32, a: f32, b: f32, c: f32) -> f32 { + let d = 1.0 - a - b - c; + return ((a * r + b) * r + c) * r + d; +}", +}]; + +#[cfg(test)] +mod tests { + use super::*; + + /// The scale factor the shader would compute, mirrored on the CPU so the + /// maths is testable without a device (ARCH §6.5a). + fn scale(c: PtLens, r: f32) -> f32 { + let d = 1.0 - c.a - c.b - c.c; + ((c.a * r + c.b) * r + c.c) * r + d + } + + #[test] + fn neutral_does_nothing() { + let d = Distortion::new(); + assert!(!d.is_active()); + let c = d.coefficients(); + assert_eq!((c.a, c.b, c.c), (0.0, 0.0, 0.0)); + } + + #[test] + fn a_neutral_polynomial_is_the_identity() { + // Every radius must map to itself when no correction is set, + // otherwise opening an image would resample it for nothing. + let c = Distortion::new().coefficients(); + for r in [0.0, 0.25, 0.5, 0.75, 1.0] { + assert!((scale(c, r) - 1.0).abs() < 1e-6, "r={r} was rescaled"); + } + } + + #[test] + fn the_corner_is_pinned_whatever_the_coefficients() { + // The property the `1 - a - b - c` term exists for: correction must + // not silently zoom the frame. If this fails, the distortion slider + // doubles as a crop and no setting leaves framing untouched. + for amount in [-100.0, -50.0, -1.0, 1.0, 50.0, 100.0] { + let mut d = Distortion::new(); + d.set_param(AMOUNT, amount); + let s = scale(d.coefficients(), 1.0); + assert!( + (s - 1.0).abs() < 1e-5, + "amount {amount} moved the corner by {}", + s - 1.0 + ); + } + } + + #[test] + fn the_centre_never_moves() { + // r = 0 is the optical axis; a radial model must leave it fixed, and + // `p * scale` does so for any finite scale. + let mut d = Distortion::new(); + d.set_param(AMOUNT, 100.0); + assert!(scale(d.coefficients(), 0.0).is_finite()); + } + + #[test] + fn positive_amounts_correct_barrel_distortion() { + // Barrel distortion pushes detail outward, so correcting it must + // sample from further out at mid radii — an inverse map (see the + // `warp` module docs), which is why "correct barrel" magnifies. + let mut d = Distortion::new(); + d.set_param(AMOUNT, 100.0); + let s = scale(d.coefficients(), 0.5); + assert!(s < 1.0, "mid-radius scale was {s}, expected < 1"); + } + + #[test] + fn negative_amounts_go_the_other_way() { + let mut pin = Distortion::new(); + pin.set_param(AMOUNT, -100.0); + let mut bar = Distortion::new(); + bar.set_param(AMOUNT, 100.0); + assert!(scale(pin.coefficients(), 0.5) > scale(bar.coefficients(), 0.5)); + } + + #[test] + fn the_mapping_stays_monotonic_across_the_whole_range() { + // If radius stops increasing with radius, the correction folds the + // image over itself and produces a mirrored ring. This is what bounds + // the slider at ±100, so it is worth asserting rather than trusting. + for amount in [-100.0, -50.0, 0.0, 50.0, 100.0] { + let mut d = Distortion::new(); + d.set_param(AMOUNT, amount); + let c = d.coefficients(); + let mut prev = 0.0; + for i in 1..=100 { + let r = i as f32 / 100.0; + let mapped = r * scale(c, r); + assert!( + mapped > prev, + "amount {amount}: mapping folded at r={r} ({mapped} <= {prev})" + ); + prev = mapped; + } + } + } + + #[test] + fn a_profile_corrects_with_the_slider_at_zero() { + // Loading a lens profile must do something on its own; requiring the + // user to also move a slider would make profiles pointless. + let mut d = Distortion::new(); + assert!(!d.is_active()); + d.set_profile(Some(PtLens { + a: 0.0168, + b: -0.0320, + c: -0.0287, + })); + assert!(d.is_active()); + assert_eq!(d.param(AMOUNT), 0.0); + } + + #[test] + fn the_slider_trims_a_loaded_profile_rather_than_replacing_it() { + // A profile that over-corrects on this copy of the lens must stay + // tunable, so the manual control adds to `c` and leaves a and b. + let profile = PtLens { + a: 0.01, + b: -0.02, + c: 0.03, + }; + let mut d = Distortion::new(); + d.set_profile(Some(profile)); + d.set_param(AMOUNT, 100.0); + + let c = d.coefficients(); + assert_eq!(c.a, profile.a, "the profile's a must survive a trim"); + assert_eq!(c.b, profile.b); + assert!((c.c - (profile.c + MAX_COEFF)).abs() < 1e-6); + } + + #[test] + fn a_profile_can_be_cleared() { + let mut d = Distortion::new(); + d.set_profile(Some(PtLens { + a: 0.01, + b: 0.0, + c: 0.0, + })); + assert!(d.is_active()); + d.set_profile(None); + assert!(!d.is_active(), "clearing a profile must return to neutral"); + } + + #[test] + fn the_wgsl_body_reads_its_declared_uniforms() { + // The composer rewrites bare names; a body naming something it did + // not declare would compile to a reference to a nonexistent field. + let mut d = Distortion::new(); + d.set_param(AMOUNT, 50.0); + let body = d.wgsl_body(); + for u in d.uniforms() { + assert!(body.contains(u.name), "{} is declared but unused", u.name); + } + } +} diff --git a/core/dr-pipeline/src/ops/mod.rs b/core/dr-pipeline/src/ops/mod.rs index d5f10c1..38e51fc 100644 --- a/core/dr-pipeline/src/ops/mod.rs +++ b/core/dr-pipeline/src/ops/mod.rs @@ -6,12 +6,16 @@ //! shader to edit, no UI change (FR-DEV-3c). pub mod colour; +pub mod colour_mixer; +pub mod contrast; pub mod exposure; pub mod helpers; pub mod tone; pub mod white_balance; pub use colour::{Brilliance, Saturation, Vibrance}; +pub use colour_mixer::ColourMixer; +pub use contrast::Contrast; pub use exposure::Exposure; pub use tone::{BlacksWhites, HighlightsShadows}; pub use white_balance::WhiteBalance; diff --git a/core/dr-pipeline/src/warp.rs b/core/dr-pipeline/src/warp.rs new file mode 100644 index 0000000..29fb1a1 --- /dev/null +++ b/core/dr-pipeline/src/warp.rs @@ -0,0 +1,314 @@ +//! Coordinate-domain operations — the geometry half of the pipeline. +//! +//! # Why this is not `Operation` +//! +//! Every [`crate::operation::Operation`] is a function from colour to colour: +//! `wgsl_body` receives `c: vec3` and produces one. That shape cannot +//! express lens correction, and the reason is worth stating precisely because +//! it is what justifies a second trait rather than an extension of the first. +//! +//! Distortion does not change a pixel's value; it changes **which pixel you +//! read**. Chromatic aberration is worse still: lateral CA is a per-channel +//! radial magnification, so red, green and blue must be fetched from three +//! *different* coordinates. No function of an already-fetched `vec3` can +//! recover that — by the time a colour reaches an `Operation`, the three +//! channels have been sampled together and the information is gone. +//! +//! So a warp runs **before** the fetch, and composes into the generated +//! shader ahead of it (ARCH §5.2 places lens corrections in the geometry +//! half of the chain). +//! +//! # Inverse mapping +//! +//! A warp declares where an output pixel's colour **came from**, not where an +//! input pixel goes. This is not a stylistic choice: +//! +//! - A forward map is a *scatter* — each input pixel writes somewhere. In a +//! compute shader that needs atomics, leaves holes where the map expands, +//! and races where it contracts. +//! - An inverse map is a *gather* — each output pixel reads somewhere. One +//! dispatch, one write per pixel, no contention, and hole-free by +//! construction. +//! +//! So `undistort` is expressed as "given this output position, which source +//! position feeds it?". For a barrel-distorting lens that means the warp +//! *magnifies* the radius, which reads backwards until you remember the +//! direction is inverse. +//! +//! # Coordinate space +//! +//! Warps work in **normalised centred** coordinates: the image centre is +//! `(0, 0)`, and the radius is scaled so that `r == 1` at the corner. Both +//! properties matter. +//! +//! Centring is what makes the polynomial meaningful — lens distortion is +//! radially symmetric about the optical axis, so a formula written about any +//! other origin would need cross terms to say the same thing. +//! +//! Corner normalisation is what makes a coefficient **portable across +//! resolutions and aspect ratios**: the same value describes the lens whether +//! applied to a full-resolution export, a 512px thumbnail, or a cropped +//! frame. Normalising to the shorter edge instead — the other obvious choice +//! — would make a coefficient mean different things on a 3:2 and a 16:9 body +//! wearing the same lens, which defeats the point of a lens profile. + +use std::fmt::Write as _; + +use crate::descriptor::{OpDescriptor, ParamId}; +use crate::operation::{Helper, Uniform}; + +/// A coordinate-domain operation, applied before the source is sampled. +/// +/// Object-safe for the same reason [`crate::operation::Operation`] is: the +/// graph holds `Box` in order, so the geometry chain is data. +pub trait Warp: Send + Sync { + /// Static description, driving UI generation exactly as for an operation. + fn descriptor(&self) -> &'static OpDescriptor; + + /// Set a parameter. Values arrive already clamped to the descriptor. + fn set_param(&mut self, id: ParamId, value: f32); + + /// Read a parameter back. + fn param(&self, id: ParamId) -> f32; + + /// Whether this warp currently moves any pixel. + /// + /// A warp at neutral is omitted from the shader entirely — and if *every* + /// warp is neutral the generated shader keeps its integer `textureLoad` + /// path rather than paying for a bilinear sample it does not need. + fn is_active(&self) -> bool; + + /// The WGSL body of this warp's inverse coordinate transform. + /// + /// Receives `p` (a `vec2`, normalised and centred per the module + /// docs) and must leave the **source** position in `p`. + /// + /// A warp needing per-channel divergence writes `p_r` and `p_b` as well; + /// they enter the block equal to `p` and are carried out of it. A warp + /// that ignores them costs nothing — the composer drops the per-channel + /// path when no active warp declares [`Self::splits_channels`]. + /// + /// Uniforms are addressed by their bare declared names, as for an + /// operation; the composer rewrites them to their prefixed fields. + fn wgsl_body(&self) -> String; + + /// Uniform values this warp's body reads. + fn uniforms(&self) -> Vec; + + /// Whether this warp moves the channels independently. + /// + /// True only for chromatic aberration. When no active warp declares it, + /// the composer emits a single sample instead of three — a 3× saving in + /// texture bandwidth for the common case of distortion alone, which at + /// 24 MP is the difference the tile budget is measured in. + fn splits_channels(&self) -> bool { + false + } + + /// Any WGSL helper functions the body calls. + fn helpers(&self) -> &'static [Helper] { + &[] + } +} + +/// The composed geometry stage: WGSL, uniforms, and what it needs from the +/// sampler. +#[derive(Debug, Clone, PartialEq, Default)] +pub struct ComposedWarp { + /// The WGSL block computing source coordinates, or empty when no warp is + /// active. + pub body: String, + /// Helper functions the body calls. + pub helpers: Vec, + /// Uniform declarations, to be appended to the generated struct. + pub uniform_fields: String, + /// Uniform values, in declaration order. + pub uniforms: Vec, + /// Whether any active warp samples the channels separately. + pub splits_channels: bool, +} + +impl ComposedWarp { + /// Whether any warp is active. When false the shader samples with an + /// integer `textureLoad` and no interpolation at all. + pub fn is_active(&self) -> bool { + !self.body.is_empty() + } +} + +/// Compose the active warps into one coordinate transform. +/// +/// Warps chain in order: each receives the position the previous one produced, +/// so correcting distortion and then CA composes as a single expression with +/// no intermediate buffer. +pub fn compose_warps(warps: &[Box]) -> ComposedWarp { + let active: Vec<&dyn Warp> = warps + .iter() + .map(|w| w.as_ref()) + .filter(|w| w.is_active()) + .collect(); + + if active.is_empty() { + return ComposedWarp::default(); + } + + let mut out = ComposedWarp { + splits_channels: active.iter().any(|w| w.splits_channels()), + ..Default::default() + }; + + for warp in &active { + let id = warp.descriptor().id.0; + let prefix = sanitise(id); + + let warp_uniforms = warp.uniforms(); + if !warp_uniforms.is_empty() { + let _ = writeln!(out.uniform_fields, " // {id}"); + } + for u in &warp_uniforms { + let _ = writeln!(out.uniform_fields, " {prefix}_{}: f32,", u.name); + out.uniforms.push(u.value); + } + + for h in warp.helpers() { + if !out.helpers.iter().any(|e| e.name == h.name) { + out.helpers.push(*h); + } + } + + let mut fragment = warp.wgsl_body(); + for u in &warp_uniforms { + fragment = crate::operation::rewrite_uniform( + &fragment, + u.name, + &format!("u.{prefix}_{}", u.name), + ); + } + + let _ = writeln!(out.body, "\n // ---- warp: {id} ----"); + let _ = writeln!(out.body, " {{"); + for line in fragment.lines() { + let _ = writeln!(out.body, " {line}"); + } + let _ = writeln!(out.body, " }}"); + } + + out +} + +fn sanitise(id: &str) -> String { + id.chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor}; + + static DESC_A: OpDescriptor = OpDescriptor { + id: OpId("warp_a"), + label: LocalizedKey("a"), + params: &[ParamDescriptor::amount("amount", "a.amount")], + }; + static DESC_B: OpDescriptor = OpDescriptor { + id: OpId("warp_b"), + label: LocalizedKey("b"), + params: &[ParamDescriptor::amount("amount", "b.amount")], + }; + + struct Fake { + desc: &'static OpDescriptor, + amount: f32, + splits: bool, + } + + impl Warp for Fake { + fn descriptor(&self) -> &'static OpDescriptor { + self.desc + } + fn set_param(&mut self, _id: ParamId, value: f32) { + self.amount = value; + } + fn param(&self, _id: ParamId) -> f32 { + self.amount + } + fn is_active(&self) -> bool { + self.amount != 0.0 + } + fn wgsl_body(&self) -> String { + "p = p * amount;".into() + } + fn uniforms(&self) -> Vec { + vec![Uniform { + name: "amount", + value: self.amount, + }] + } + fn splits_channels(&self) -> bool { + self.splits + } + } + + fn fake(desc: &'static OpDescriptor, amount: f32, splits: bool) -> Box { + Box::new(Fake { + desc, + amount, + splits, + }) + } + + #[test] + fn no_active_warp_composes_to_nothing() { + // The property that keeps the common case free: an image with no lens + // correction must not pay for a bilinear sample. + let composed = compose_warps(&[fake(&DESC_A, 0.0, false)]); + assert!(!composed.is_active()); + assert!(composed.uniforms.is_empty()); + assert!(!composed.splits_channels); + } + + #[test] + fn an_active_warp_appears_once() { + let composed = compose_warps(&[fake(&DESC_A, 2.0, false)]); + assert!(composed.is_active()); + assert!(composed.body.contains("---- warp: warp_a ----")); + assert!(composed.body.contains("u.warp_a_amount")); + } + + #[test] + fn uniforms_are_prefixed_so_warps_cannot_collide() { + // Both fakes declare `amount`; without prefixing the generated struct + // would carry a duplicate field and fail to compile. + let composed = compose_warps(&[fake(&DESC_A, 1.0, false), fake(&DESC_B, 2.0, false)]); + assert!(composed.uniform_fields.contains("warp_a_amount: f32")); + assert!(composed.uniform_fields.contains("warp_b_amount: f32")); + assert_eq!(composed.uniforms, vec![1.0, 2.0]); + } + + #[test] + fn channel_splitting_is_requested_by_any_active_warp() { + // One CA warp among several must switch the whole stage to the + // three-sample path. + let composed = compose_warps(&[fake(&DESC_A, 1.0, false), fake(&DESC_B, 1.0, true)]); + assert!(composed.splits_channels); + } + + #[test] + fn an_inactive_splitting_warp_does_not_force_three_samples() { + // CA present but at neutral must cost nothing — otherwise every image + // with the panel visible pays triple bandwidth. + let composed = compose_warps(&[fake(&DESC_A, 1.0, false), fake(&DESC_B, 0.0, true)]); + assert!(composed.is_active()); + assert!(!composed.splits_channels); + } + + #[test] + fn warps_compose_in_order() { + let composed = compose_warps(&[fake(&DESC_A, 1.0, false), fake(&DESC_B, 1.0, false)]); + let a = composed.body.find("warp_a").expect("a present"); + let b = composed.body.find("warp_b").expect("b present"); + assert!(a < b, "warps must chain in graph order"); + } +} diff --git a/core/dr-sync-nextcloud/examples/connect.rs b/core/dr-sync-nextcloud/examples/connect.rs index 0681f03..009992c 100644 --- a/core/dr-sync-nextcloud/examples/connect.rs +++ b/core/dr-sync-nextcloud/examples/connect.rs @@ -1,9 +1,13 @@ //! Exercise the connector against a real Nextcloud server. //! //! ```text -//! cargo run -p dr-sync-nextcloud --example connect -- https://cloud.example [remote/path] +//! cargo run -p dr-sync-nextcloud --example connect -- [path] [--raw|--formats cr2,nef] //! ``` //! +//! With no path it lists the account root so a folder can be chosen. Given a +//! path it scans recursively for images matching the format filter, which is +//! the library-setup flow (FR-CAT-1). +//! //! **Strictly read-only.** PROPFIND, ETag probes, range GETs and preview //! requests only — no PUT, MOVE or DELETE — so it cannot alter a live library. //! @@ -15,6 +19,7 @@ //! not re-authenticate. That location is for *testing convenience* only; //! FR-NC-2 requires the real app to use platform secure storage. +use std::collections::HashMap; use std::path::PathBuf; use std::time::Instant; @@ -30,7 +35,25 @@ async fn main() { eprintln!("usage: connect [remote/path]"); std::process::exit(2); }; - let start_path = args.next().unwrap_or_default(); + let rest: Vec = args.collect(); + let start_path = rest + .iter() + .find(|a| !a.starts_with("--")) + .cloned() + .unwrap_or_default(); + + // Format selection — the tick-boxes, as a CLI flag. + let filter = if let Some(i) = rest.iter().position(|a| a == "--formats") { + let list = rest.get(i + 1).cloned().unwrap_or_default(); + dr_types::FormatFilter::from_formats( + list.split(',') + .filter_map(|s| dr_types::Format::from_extension(s.trim())), + ) + } else if rest.iter().any(|a| a == "--raw") { + dr_types::FormatFilter::raw_only() + } else { + dr_types::FormatFilter::all() + }; let creds = match load_cached(&server) { Some(c) => { @@ -63,126 +86,134 @@ async fn main() { println!("cheap no-op sync: {}", strategy.has_cheap_noop()); let root = RemotePath::new(&start_path); - let mut failures = 0; - // ---- 1. listing ---------------------------------------------------- - println!("\n[1] PROPFIND Depth:1 on /{start_path}"); - let t = Instant::now(); - let entries = match backend.list(&root, None).await { - Ok(e) => { - println!( - " {} entries in {:.0}ms", - e.len(), - t.elapsed().as_secs_f64() * 1000.0 - ); - e + // No path given: list this level so the user can pick a folder. + if start_path.is_empty() { + println!("\n[browse] PROPFIND Depth:1 on the account root"); + let t = Instant::now(); + match backend.list(&root, None).await { + Ok(entries) => { + println!( + " {} entries in {:.0}ms\n", + entries.len(), + t.elapsed().as_secs_f64() * 1000.0 + ); + let mut dirs: Vec<_> = entries + .iter() + .filter(|e| e.kind == dr_sync::EntryKind::Directory) + .collect(); + dirs.sort_by_key(|e| e.path.name().to_ascii_lowercase()); + for d in &dirs { + println!(" {}/", d.path.name()); + } + println!( + "\n Re-run with a folder to scan it, e.g.:\n … {} \"{}\" --raw", + server, + dirs.first().map(|d| d.path.name()).unwrap_or("Photos") + ); + } + Err(e) => { + eprintln!(" FAILED: {e}"); + std::process::exit(1); + } } + return; + } + + // A path was given: scan it recursively for the selected formats. + println!("\n[scan] {} under /{start_path}", describe_filter(&filter)); + let t = Instant::now(); + let result = match dr_sync::scan(&backend, &root, &filter, &HashMap::new(), |p| { + if p.directories_listed % 25 == 0 && p.directories_listed > 0 { + print!( + "\r {} dirs, {} images…", + p.directories_listed, p.images_found + ); + let _ = std::io::Write::flush(&mut std::io::stdout()); + } + }) + .await + { + Ok(r) => r, Err(e) => { - println!(" FAILED: {e}"); - failures += 1; - Vec::new() + eprintln!("\n FAILED: {e}"); + std::process::exit(1); } }; + let scan_ms = t.elapsed().as_secs_f64() * 1000.0; - for e in entries.iter().take(5) { + println!( + "\r {} images in {} directories, {:.1}s", + result.images.len(), + result.progress.directories_listed, + scan_ms / 1000.0 + ); + + let mut by_ext: std::collections::BTreeMap = Default::default(); + for i in &result.images { + let ext = i + .path + .name() + .rsplit_once('.') + .map(|(_, e)| e.to_ascii_uppercase()) + .unwrap_or_default(); + *by_ext.entry(ext).or_default() += 1; + } + for (ext, n) in &by_ext { + println!(" {ext:<6} {n}"); + } + + // Prove the fast path on a real RAW: metadata from a header range alone. + let raw = result.images.iter().find(|i| { + i.path + .name() + .rsplit_once('.') + .and_then(|(_, e)| dr_types::Format::from_extension(&e.to_ascii_lowercase())) + .is_some_and(|f| f.is_raw()) + && i.size > 300_000 + }); + + if let Some(f) = raw { println!( - " {:?} {:<40} {:>10} id={:?}{}", - e.kind, - truncate(e.path.name(), 40), - human(e.size), - e.id, - if e.has_preview { " preview" } else { "" } - ); - } - if entries.len() > 5 { - println!(" … {} more", entries.len() - 5); - } - - // ---- 2. the pruning probe ------------------------------------------ - println!("\n[2] PROPFIND Depth:0 — the ETag pruning probe (FR-NC-4)"); - let t = Instant::now(); - match backend.dir_validator(&root).await { - Ok(v) => println!( - " etag {} in {:.0}ms — one request proves the tree unchanged", - v.as_str(), - t.elapsed().as_secs_f64() * 1000.0 - ), - Err(e) => { - println!(" FAILED: {e}"); - failures += 1; - } - } - - // ---- 3. range read ------------------------------------------------- - // The mechanism the whole mobile story rests on (FR-NC-3). - let file = entries - .iter() - .find(|e| e.kind == dr_sync::EntryKind::File && e.size > 300_000); - - if let Some(f) = file { - println!( - "\n[3] Range GET — first 256KB of {} ({})", + "\n[range] first 256KB of {} ({})", f.path.name(), human(f.size) ); let id = RemoteId::Path(f.path.clone()); - let t = Instant::now(); match backend.get(&id, Some(0..262_144)).await { Ok(bytes) => { let ms = t.elapsed().as_secs_f64() * 1000.0; let pct = (bytes.len() as f64 / f.size as f64) * 100.0; println!( - " got {} in {ms:.0}ms ({pct:.1}% of the file)", + " {} in {ms:.0}ms — {pct:.2}% of the file", human(bytes.len() as u64) ); - - if bytes.len() as u64 >= f.size { - println!(" WARNING: whole file returned — server ignored Range"); - failures += 1; - } else { - println!(" range requests work — remote browsing is viable"); - } - - if let Some(fmt) = dr_decode::probe(&bytes) { - println!(" probe: {fmt:?}"); - } match dr_decode::metadata(&bytes) { Ok(m) => println!( - " metadata from the range alone: {} {}", - m.make.unwrap_or_default(), - m.model.unwrap_or_default() + " metadata from that range alone: {} {} | {}", + m.make.unwrap_or_default().trim(), + m.model.unwrap_or_default().trim(), + m.iso.map(|i| format!("ISO {i}")).unwrap_or_default() ), Err(e) => println!(" metadata: {e}"), } } - Err(e) => { - println!(" FAILED: {e}"); - failures += 1; - } - } - - // ---- 4. server preview ------------------------------------------ - println!("\n[4] Server preview (ARCH §6.7 expects none for RAW)"); - match backend.thumbnail(&f.id, 256).await { - Ok(Some(b)) => println!(" {} returned", human(b.len() as u64)), - Ok(None) => println!(" none — as expected; local extraction is the path"), - Err(e) => println!(" error: {e}"), + Err(e) => println!(" FAILED: {e}"), } } else { - println!("\n[3] skipped — no file over 300KB at this path"); + println!("\n[range] skipped — no RAW over 300KB found"); } - println!( - "\n{}", - if failures == 0 { - "all checks passed" - } else { - "FAILURES" - } - ); - if failures > 0 { - std::process::exit(1); + println!("\nscan complete"); +} + +fn describe_filter(f: &dr_types::FormatFilter) -> String { + let names: Vec<&str> = f.iter().map(|x| x.label()).collect(); + if names.len() >= 9 { + "all supported formats".into() + } else { + names.join(", ") } } @@ -271,11 +302,3 @@ fn human(bytes: u64) -> String { b => format!("{b}B"), } } - -fn truncate(s: &str, n: usize) -> String { - if s.chars().count() <= n { - s.to_string() - } else { - format!("{}…", s.chars().take(n - 1).collect::()) - } -} diff --git a/core/dr-sync/Cargo.toml b/core/dr-sync/Cargo.toml index 4837125..1b9fdf2 100644 --- a/core/dr-sync/Cargo.toml +++ b/core/dr-sync/Cargo.toml @@ -10,3 +10,6 @@ dr-types.workspace = true async-trait.workspace = true thiserror.workspace = true log.workspace = true + +[dev-dependencies] +tokio = { workspace = true } diff --git a/core/dr-sync/src/lib.rs b/core/dr-sync/src/lib.rs index f6d76c7..10f2665 100644 --- a/core/dr-sync/src/lib.rs +++ b/core/dr-sync/src/lib.rs @@ -22,10 +22,12 @@ use async_trait::async_trait; pub mod capability; pub mod error; +pub mod scan; pub mod types; pub use capability::{Capabilities, ChangeDetection, ChunkConstraints, ServerPreviews}; pub use error::RemoteError; +pub use scan::{scan, ScanProgress, ScanResult}; pub use types::{ Cursor, EntryKind, Identity, Precondition, RemoteChange, RemoteEntry, RemoteId, RemotePath, Validator, diff --git a/core/dr-sync/src/scan.rs b/core/dr-sync/src/scan.rs new file mode 100644 index 0000000..00ec5a6 --- /dev/null +++ b/core/dr-sync/src/scan.rs @@ -0,0 +1,464 @@ +//! Recursive discovery of images under a chosen remote folder. +//! +//! The library-setup path: the user picks a folder, ticks the formats they +//! shoot, and this walks the tree finding matching files (FR-CAT-1, M-5). +//! +//! Depth:1 per directory, never `Depth: infinity` — the latter is frequently +//! disabled and prohibitively expensive where it is not (ARCH §8.4). Where the +//! backend propagates directory ETags, an unchanged subtree is skipped whole, +//! which is what keeps a re-scan proportional to what changed rather than to +//! library size. + +use std::collections::HashMap; + +use dr_types::FormatFilter; + +use crate::{ + Capabilities, ChangeDetection, EntryKind, RemoteBackend, RemoteEntry, RemoteError, RemotePath, + Validator, +}; + +/// Progress during a scan, so the UI can show something on a large library. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct ScanProgress { + pub directories_listed: usize, + /// Directories skipped because their ETag was unchanged. The value of + /// pruning, made visible. + pub directories_pruned: usize, + pub images_found: usize, +} + +/// The result of a scan. +#[derive(Debug, Clone, Default)] +pub struct ScanResult { + /// Files matching the format filter. + pub images: Vec, + /// Every directory visited, with its ETag, so the next scan can prune. + /// + /// **Must be persisted.** Without stored folder ETags there is nothing to + /// compare against and every scan is a full walk (ARCH §6.6). + pub directories: Vec<(RemotePath, Validator)>, + pub progress: ScanProgress, +} + +/// How deep to recurse before giving up. +/// +/// A symlink loop or a pathological tree would otherwise walk forever. Real +/// photo libraries are nowhere near this deep. +const MAX_DEPTH: usize = 32; + +/// TRACES: FR-CAT-1 | FR-NC-4 | M-5 | M-7 +/// Walk `root` recursively, collecting files the filter accepts. +/// +/// `known` maps previously seen directories to their ETags. Pass an empty map +/// for a first scan; pass the stored ETags to prune unchanged subtrees. +/// +/// `on_progress` is called after each directory so a long scan can report +/// rather than appear hung. +pub async fn scan( + backend: &B, + root: &RemotePath, + filter: &FormatFilter, + known: &HashMap, + mut on_progress: F, +) -> Result +where + B: RemoteBackend + ?Sized, + F: FnMut(ScanProgress), +{ + let prunable = supports_pruning(backend.capabilities()); + let mut result = ScanResult::default(); + + // Explicit stack rather than recursion: an async recursive fn needs + // boxing, and a deep tree could overflow. + let mut stack = vec![(root.clone(), 0usize)]; + + while let Some((dir, depth)) = stack.pop() { + if depth > MAX_DEPTH { + log::warn!("scan: depth limit at {dir}, not descending further"); + continue; + } + + // Prune: if the directory's ETag is unchanged, nothing anywhere + // beneath it changed either, because Nextcloud propagates upward. + if prunable { + if let Some(previous) = known.get(&dir) { + match backend.dir_validator(&dir).await { + Ok(current) if ¤t == previous => { + result.progress.directories_pruned += 1; + on_progress(result.progress); + continue; + } + Ok(_) => {} + // A probe failure is not fatal — fall through to listing, + // which is correct, just not free. + Err(e) => log::debug!("scan: validator probe failed for {dir}: {e}"), + } + } + } + + let entries = match backend.list(&dir, None).await { + Ok(e) => e, + Err(RemoteError::NotFound(_)) => { + // Deleted between listing its parent and reaching it. + log::debug!("scan: {dir} vanished during the walk"); + continue; + } + Err(e) => return Err(e), + }; + + result.progress.directories_listed += 1; + + for entry in entries { + match entry.kind { + EntryKind::Directory => { + result + .directories + .push((entry.path.clone(), entry.validator.clone())); + stack.push((entry.path, depth + 1)); + } + EntryKind::File => { + if filter.allows_name(entry.path.name()) { + result.images.push(entry); + result.progress.images_found += 1; + } + } + } + } + + on_progress(result.progress); + } + + // Sort so a scan is reproducible and the grid has a stable order. + result.images.sort_by(|a, b| a.path.cmp(&b.path)); + result.directories.sort_by(|a, b| a.0.cmp(&b.0)); + Ok(result) +} + +/// Whether pruning is worth attempting against this backend. +/// +/// Only propagating ETags make an unchanged parent prove an unchanged +/// subtree. With per-entry ETags the probe costs a request and proves +/// nothing about children, so it is pure overhead. +fn supports_pruning(caps: &Capabilities) -> bool { + matches!(caps.change_detection, ChangeDetection::PropagatingEtags) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{RemoteId, ServerPreviews}; + use async_trait::async_trait; + use std::cell::RefCell; + use std::ops::Range; + + /// A backend over an in-memory tree, counting requests so tests can + /// assert that pruning actually avoids work. + struct FakeBackend { + tree: HashMap>, + etags: HashMap, + caps: Capabilities, + lists: RefCell, + probes: RefCell, + } + + // The fake is single-threaded; tests never share it across threads. + unsafe impl Sync for FakeBackend {} + + fn dir(path: &str, etag: &str) -> RemoteEntry { + RemoteEntry { + id: RemoteId::Path(RemotePath::new(path)), + path: RemotePath::new(path), + kind: EntryKind::Directory, + validator: Validator::new(etag), + size: 0, + modified: None, + has_preview: false, + } + } + + fn file(path: &str) -> RemoteEntry { + RemoteEntry { + id: RemoteId::Path(RemotePath::new(path)), + path: RemotePath::new(path), + kind: EntryKind::File, + validator: Validator::new("f"), + size: 1000, + modified: None, + has_preview: false, + } + } + + impl FakeBackend { + /// Photos/{2025/{a.CR2,b.jpg}, 2026/{c.NEF,notes.txt}} + fn sample(change_detection: ChangeDetection) -> Self { + let mut tree = HashMap::new(); + tree.insert( + "Photos".into(), + vec![dir("Photos/2025", "e2025"), dir("Photos/2026", "e2026")], + ); + tree.insert( + "Photos/2025".into(), + vec![file("Photos/2025/a.CR2"), file("Photos/2025/b.jpg")], + ); + tree.insert( + "Photos/2026".into(), + vec![file("Photos/2026/c.NEF"), file("Photos/2026/notes.txt")], + ); + + let mut etags = HashMap::new(); + etags.insert("Photos".to_string(), "root"); + etags.insert("Photos/2025".to_string(), "e2025"); + etags.insert("Photos/2026".to_string(), "e2026"); + + Self { + tree, + etags, + caps: Capabilities { + change_detection, + stable_ids: true, + range_reads: true, + chunked_upload: None, + bulk_upload: false, + conditional_write: true, + server_previews: ServerPreviews::None, + }, + lists: RefCell::new(0), + probes: RefCell::new(0), + } + } + } + + #[async_trait] + impl RemoteBackend for FakeBackend { + fn capabilities(&self) -> &Capabilities { + &self.caps + } + fn name(&self) -> &str { + "fake" + } + async fn list( + &self, + dir: &RemotePath, + _since: Option<&Validator>, + ) -> Result, RemoteError> { + *self.lists.borrow_mut() += 1; + Ok(self.tree.get(dir.as_str()).cloned().unwrap_or_default()) + } + async fn dir_validator(&self, dir: &RemotePath) -> Result { + *self.probes.borrow_mut() += 1; + self.etags + .get(dir.as_str()) + .map(|e| Validator::new(*e)) + .ok_or_else(|| RemoteError::NotFound(dir.to_string())) + } + async fn delta( + &self, + _c: &crate::Cursor, + ) -> Result<(Vec, crate::Cursor), RemoteError> { + Err(RemoteError::Unsupported("fake")) + } + async fn get( + &self, + _id: &RemoteId, + _r: Option>, + ) -> Result, RemoteError> { + Ok(Vec::new()) + } + async fn put( + &self, + _p: &RemotePath, + _b: Vec, + _pc: Option, + ) -> Result { + Err(RemoteError::Unsupported("fake")) + } + async fn delete( + &self, + _id: &RemoteId, + _pc: Option, + ) -> Result<(), RemoteError> { + Err(RemoteError::Unsupported("fake")) + } + } + + #[tokio::test] + async fn finds_images_recursively() { + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &HashMap::new(), + |_| {}, + ) + .await + .unwrap(); + + // notes.txt is not an image; the other three are. + assert_eq!(r.images.len(), 3); + assert_eq!(r.progress.images_found, 3); + assert_eq!(r.progress.directories_listed, 3); + } + + #[tokio::test] + async fn the_format_filter_is_applied() { + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::from_formats([dr_types::Format::Cr2]), + &HashMap::new(), + |_| {}, + ) + .await + .unwrap(); + + assert_eq!(r.images.len(), 1); + assert_eq!(r.images[0].path.name(), "a.CR2"); + } + + #[tokio::test] + async fn unchanged_subtrees_are_pruned() { + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let mut known = HashMap::new(); + known.insert(RemotePath::new("Photos/2025"), Validator::new("e2025")); + + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &known, + |_| {}, + ) + .await + .unwrap(); + + // 2025 was proven unchanged by a single probe, so it was never listed + // and its files were not re-enumerated. + assert_eq!(r.progress.directories_pruned, 1); + assert_eq!(r.progress.directories_listed, 2); + assert!(r.images.iter().all(|i| !i.path.as_str().contains("2025"))); + } + + #[tokio::test] + async fn a_changed_etag_defeats_pruning() { + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let mut known = HashMap::new(); + known.insert(RemotePath::new("Photos/2025"), Validator::new("stale")); + + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &known, + |_| {}, + ) + .await + .unwrap(); + + assert_eq!(r.progress.directories_pruned, 0); + assert_eq!(r.images.len(), 3); + } + + #[tokio::test] + async fn pruning_is_not_attempted_without_propagating_etags() { + // Per-entry ETags say nothing about children, so probing would cost a + // request and prove nothing. + let b = FakeBackend::sample(ChangeDetection::LocalEtags); + let mut known = HashMap::new(); + known.insert(RemotePath::new("Photos/2025"), Validator::new("e2025")); + + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &known, + |_| {}, + ) + .await + .unwrap(); + + assert_eq!(*b.probes.borrow(), 0, "must not probe"); + assert_eq!(r.progress.directories_pruned, 0); + assert_eq!(r.images.len(), 3); + } + + #[tokio::test] + async fn directory_etags_are_returned_for_persistence() { + // Without these the next scan has nothing to compare and prunes + // nothing (ARCH §6.6). + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &HashMap::new(), + |_| {}, + ) + .await + .unwrap(); + + assert_eq!(r.directories.len(), 2); + assert!(r + .directories + .iter() + .any(|(p, v)| p.as_str() == "Photos/2025" && v.as_str() == "e2025")); + } + + #[tokio::test] + async fn results_are_ordered_reproducibly() { + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &HashMap::new(), + |_| {}, + ) + .await + .unwrap(); + + let paths: Vec<&str> = r.images.iter().map(|i| i.path.as_str()).collect(); + let mut sorted = paths.clone(); + sorted.sort(); + assert_eq!(paths, sorted); + } + + #[tokio::test] + async fn progress_is_reported_per_directory() { + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let mut updates = Vec::new(); + scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &HashMap::new(), + |p| updates.push(p), + ) + .await + .unwrap(); + + // One per directory visited, so a long scan never looks hung. + assert_eq!(updates.len(), 3); + assert_eq!(updates.last().unwrap().images_found, 3); + } + + #[tokio::test] + async fn an_empty_filter_finds_nothing_but_still_walks() { + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags); + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::from_formats([]), + &HashMap::new(), + |_| {}, + ) + .await + .unwrap(); + + assert!(r.images.is_empty()); + // The walk still happened, so directory ETags are still collected. + assert_eq!(r.directories.len(), 2); + } +} diff --git a/core/dr-types/src/lib.rs b/core/dr-types/src/lib.rs index cbc8102..dca189d 100644 --- a/core/dr-types/src/lib.rs +++ b/core/dr-types/src/lib.rs @@ -4,9 +4,14 @@ //! else in `core/` builds on these types, so anything added here is paid for //! everywhere. +use std::collections::BTreeSet; use std::fmt; use std::ops::Range; +pub mod selector; + +pub use selector::{ColourLabel, DateSelector, FlagState, Selector, Tier}; + /// Identifies a granted library location — a directory on Linux, a persisted /// document tree on Android. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -20,6 +25,14 @@ pub struct ImageId(pub u64); #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct VersionId(pub u64); +/// Identifies a user-defined collection. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct CollectionId(pub u64); + +/// Identifies a folder within a root. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct FolderId(pub u64); + /// TRACES: FR-CAT-1a | FR-PLAT-AND-1 /// An opaque, re-resolvable reference to source image data. /// @@ -92,7 +105,9 @@ impl fmt::Display for SourceRef { /// /// Recognition is by extension only; whether a decoder can actually handle the /// file is a separate question answered by `dr-decode`. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +/// `Ord` so a [`FormatFilter`] can hold these in a `BTreeSet` — which keeps +/// iteration order stable, and therefore keeps a serialised filter diffable. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum Format { Cr2, Cr3, @@ -106,6 +121,34 @@ pub enum Format { } impl Format { + /// Every format the catalog recognises, in the FR-RAW-1 launch order. + pub const ALL: &'static [Format] = &[ + Format::Cr2, + Format::Cr3, + Format::Nef, + Format::Arw, + Format::Raf, + Format::Rw2, + Format::Orf, + Format::Dng, + Format::Jpeg, + ]; + + /// Label for the format tick-boxes. + pub fn label(self) -> &'static str { + match self { + Format::Cr2 => "Canon CR2", + Format::Cr3 => "Canon CR3", + Format::Nef => "Nikon NEF", + Format::Arw => "Sony ARW", + Format::Raf => "Fujifilm RAF", + Format::Rw2 => "Panasonic RW2", + Format::Orf => "Olympus ORF", + Format::Dng => "Adobe DNG", + Format::Jpeg => "JPEG", + } + } + /// Recognise from a lowercase extension. pub fn from_extension(ext: &str) -> Option { Some(match ext { @@ -128,7 +171,75 @@ impl Format { } } +/// TRACES: FR-CAT-1 | FR-RAW-1 | M-9 /// TRACES: FR-NC-6c +/// A user-selected set of formats to scan for. +/// +/// Backs the format tick-boxes at library setup: a photographer shooting one +/// body has no reason to pay for scanning formats they never produce, and on +/// a large remote library that saves real time. +/// +/// Defaults to every supported format, so an unconfigured scan finds +/// everything rather than silently missing files. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FormatFilter { + allowed: BTreeSet, +} + +impl Default for FormatFilter { + fn default() -> Self { + Self::all() + } +} + +impl FormatFilter { + /// Every supported format. + pub fn all() -> Self { + Self { + allowed: Format::ALL.iter().copied().collect(), + } + } + + /// RAW formats only, excluding JPEG. + pub fn raw_only() -> Self { + Self { + allowed: Format::ALL.iter().copied().filter(|f| f.is_raw()).collect(), + } + } + + /// An explicit set. An empty set matches nothing, which is a legitimate + /// (if useless) user choice and not silently rewritten to "everything". + pub fn from_formats(formats: impl IntoIterator) -> Self { + Self { + allowed: formats.into_iter().collect(), + } + } + + pub fn allows(&self, f: Format) -> bool { + self.allowed.contains(&f) + } + + /// Whether a filename should be scanned. + /// + /// Looks through a VFS placeholder suffix, so a dehydrated + /// `IMG.CR2.nextcloud` is matched as the CR2 it stands for. + pub fn allows_name(&self, name: &str) -> bool { + let name = name.strip_suffix(PLACEHOLDER_SUFFIX).unwrap_or(name); + name.rsplit_once('.') + .map(|(_, ext)| ext.to_ascii_lowercase()) + .and_then(|e| Format::from_extension(&e)) + .is_some_and(|f| self.allows(f)) + } + + pub fn is_empty(&self) -> bool { + self.allowed.is_empty() + } + + pub fn iter(&self) -> impl Iterator + '_ { + self.allowed.iter().copied() + } +} + /// How much of an image is available locally (FR-NC-6c). /// /// Surfaced in the UI so a user always knows what they have — the failure @@ -247,6 +358,59 @@ mod tests { assert_eq!(m.extension().as_deref(), Some("cr2")); } + #[test] + fn default_filter_matches_everything() { + // An unconfigured scan must find every supported file rather than + // silently missing formats. + let f = FormatFilter::default(); + assert!(f.allows_name("IMG.CR2")); + assert!(f.allows_name("IMG.jpg")); + assert!(!f.allows_name("notes.txt")); + } + + #[test] + fn raw_only_excludes_jpeg() { + let f = FormatFilter::raw_only(); + assert!(f.allows_name("IMG.CR2")); + assert!(f.allows_name("IMG.NEF")); + assert!(!f.allows_name("IMG.jpg")); + } + + #[test] + fn a_chosen_subset_excludes_the_rest() { + // The tick-box case: one body, one format. + let f = FormatFilter::from_formats([Format::Cr2]); + assert!(f.allows_name("IMG.cr2")); + assert!(!f.allows_name("IMG.NEF")); + assert!(!f.allows_name("IMG.jpg")); + } + + #[test] + fn an_empty_selection_matches_nothing() { + // Useless but legitimate; must not be silently rewritten to "all". + let f = FormatFilter::from_formats([]); + assert!(f.is_empty()); + assert!(!f.allows_name("IMG.CR2")); + } + + #[test] + fn filter_sees_through_placeholder_suffixes() { + // A dehydrated file is still a CR2 the user owns (ARCH §9.0). + let f = FormatFilter::raw_only(); + assert!(f.allows_name("IMG.CR2.nextcloud")); + assert!(!f.allows_name("notes.txt.nextcloud")); + } + + #[test] + fn every_format_is_selectable_and_labelled() { + // ALL drives the tick-box list; a format missing from it would be + // unselectable and therefore never scanned. + assert_eq!(Format::ALL.len(), 9); + assert!(Format::ALL.iter().all(|f| !f.label().is_empty())); + assert!(Format::ALL.contains(&Format::Cr2)); + assert!(Format::ALL.contains(&Format::Jpeg)); + } + #[test] fn formats_round_trip_and_classify() { assert_eq!(Format::from_extension("cr3"), Some(Format::Cr3)); diff --git a/core/dr-types/src/selector.rs b/core/dr-types/src/selector.rs new file mode 100644 index 0000000..4cd14fe --- /dev/null +++ b/core/dr-types/src/selector.rs @@ -0,0 +1,205 @@ +//! TRACES: FR-CAT-6 | FR-CAT-7 | FR-NC-6a +//! Image set selection — one predicate language, three uses. +//! +//! The same [`Selector`] expresses a library filter (what the grid shows), a +//! smart collection (a saved filter), and a cache rule (what is kept locally, +//! at which tier). Three near-identical predicate languages is a well-trodden +//! way for a catalog to rot, so there is one. +//! +//! The useful consequence: any filter the user has narrowed to can be saved as +//! a collection, and any collection can be pinned offline, with no conversion +//! between representations. +//! +//! Lives in `dr-types` rather than `dr-catalog` so `dr-sync`'s cache rules can +//! use it without either crate depending on the other. + +use crate::{Availability, CollectionId, RootId}; + +/// A predicate over images. +/// +/// Compiles to indexed SQL in `dr-catalog`; evaluated against catalog state +/// for cache rules. Deliberately data — no closures, so it serialises into a +/// saved collection or a sync'd cache rule. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Selector { + /// Everything. The empty filter, and the implicit default cache rule. + All, + Collection(CollectionId), + Folder { + root: RootId, + path: String, + recursive: bool, + }, + DateRange(DateSelector), + Rating { + min: u8, + }, + Label(ColourLabel), + Flag(FlagState), + Keyword(String), + Camera(String), + Lens(String), + IsoRange { + min: u32, + max: u32, + }, + /// What is actually available right now. + /// + /// The most useful filter on a tablet ("what can I edit on this train"), + /// and the natural thing to pin — "everything flagged that isn't local + /// yet". + Availability(Availability), + /// Substring over filename and keywords. + Text(String), + /// Boolean composition. Named with a trailing underscore because `All` is + /// already taken by the empty filter, and renaming that would read worse. + All_(Vec), + Any(Vec), + Not(Box), +} + +impl Selector { + /// Whether this selector matches every image without inspection. + /// + /// Lets a caller skip compiling a WHERE clause entirely for the common + /// unfiltered grid. + pub fn is_unfiltered(&self) -> bool { + match self { + Selector::All => true, + // An empty conjunction is vacuously true; an empty disjunction is + // not. Both arise from a UI that lets the user clear every term. + Selector::All_(v) => v.iter().all(Selector::is_unfiltered), + _ => false, + } + } + + /// Whether evaluating this requires capture time, and therefore full EXIF + /// (`metadata_state` 2). + /// + /// A freshly scanned library has not finished extracting metadata, so a + /// date filter is incomplete until it does. The UI says so rather than + /// silently under-reporting (FR-NC-6c's principle, applied to metadata). + pub fn needs_capture_time(&self) -> bool { + match self { + Selector::DateRange(_) => true, + Selector::All_(v) | Selector::Any(v) => v.iter().any(Selector::needs_capture_time), + Selector::Not(s) => s.needs_capture_time(), + _ => false, + } + } + + /// Every collection this selector references, directly or nested. + /// + /// Used to detect cycles before a smart collection referencing another + /// collection is saved. + pub fn collections(&self, out: &mut Vec) { + match self { + Selector::Collection(id) => out.push(*id), + Selector::All_(v) | Selector::Any(v) => v.iter().for_each(|s| s.collections(out)), + Selector::Not(s) => s.collections(out), + _ => {} + } + } +} + +/// A date constraint, absolute or moving. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DateSelector { + /// UTC seconds, half-open: `from <= t < to`. + Between { from: i64, to: i64 }, + /// "The last 90 days" — the window moves with the clock, so the set stays + /// current without the user touching it. + Rolling { days: u32 }, + /// Bounded by a collection's own capture range: "this trip". + CollectionSpan(CollectionId), +} + +/// Colour labels, matching the conventional set. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum ColourLabel { + Red, + Yellow, + Green, + Blue, + Purple, +} + +/// The pick/reject axis, independent of star rating. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum FlagState { + /// Not yet judged. What "filter to unjudged" selects (FR-CULL-4). + Unflagged, + Pick, + Reject, +} + +/// How much of an image a cache rule asks to keep locally. +/// +/// Ordered so the most generous matching rule wins (ARCH §9.3). +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum Tier { + /// Catalog rows and sidecars only. Never evicted — authoritative and tiny. + Metadata, + /// Enough to browse and cull. + Preview, + /// The full source. Never bulk-synced by default (FR-NC-6). + Original, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unfiltered_recognises_empty_conjunction() { + assert!(Selector::All.is_unfiltered()); + assert!(Selector::All_(vec![]).is_unfiltered()); + assert!(Selector::All_(vec![Selector::All]).is_unfiltered()); + assert!(!Selector::Rating { min: 5 }.is_unfiltered()); + } + + #[test] + fn an_empty_disjunction_is_not_unfiltered() { + // `Any([])` matches nothing, not everything. Treating it as + // unfiltered would show the whole library when the user cleared + // every term of an "or" filter. + assert!(!Selector::Any(vec![]).is_unfiltered()); + } + + #[test] + fn capture_time_dependency_is_found_when_nested() { + let s = Selector::All_(vec![ + Selector::Rating { min: 4 }, + Selector::Not(Box::new(Selector::DateRange(DateSelector::Rolling { + days: 90, + }))), + ]); + assert!(s.needs_capture_time()); + + let s = Selector::Any(vec![ + Selector::Rating { min: 4 }, + Selector::Camera("X".into()), + ]); + assert!(!s.needs_capture_time()); + } + + #[test] + fn nested_collection_references_are_collected() { + let s = Selector::Any(vec![ + Selector::Collection(CollectionId(1)), + Selector::Not(Box::new(Selector::Collection(CollectionId(2)))), + ]); + let mut found = Vec::new(); + s.collections(&mut found); + assert_eq!(found, vec![CollectionId(1), CollectionId(2)]); + } + + #[test] + fn tiers_order_by_generosity() { + // ARCH §9.3: where rules disagree, the most generous wins, which is + // `max` over this ordering. + assert!(Tier::Original > Tier::Preview); + assert!(Tier::Preview > Tier::Metadata); + assert_eq!(Tier::Preview.max(Tier::Original), Tier::Original); + } +} diff --git a/docs/catalog.md b/docs/catalog.md new file mode 100644 index 0000000..3e7fb43 --- /dev/null +++ b/docs/catalog.md @@ -0,0 +1,529 @@ +# DarkRoom — Catalog, library view, and background work + +**Status:** Draft v0.1 · 2026-08-09 +**Companion to:** [requirements.md](requirements.md), [architecture.md](architecture.md) + +Specifies `dr-catalog`: the index the library view queries, how it stays current without rescanning +everything, and how thumbnails get made. [architecture.md §6.2](architecture.md) sketches the schema +in eight lines; this expands it to the point of implementability and fills the two gaps that sketch +leaves open — **incremental local scan** and **the job queue**. + +Sync's remote side is already designed ([architecture.md §8](architecture.md)): ETag pruning turns a +no-op sync of 50k images into one request. Nothing equivalent existed for a local root, which is the +central problem this document solves. + +--- + +## 1. What this must not do + +Stated first because every design choice below follows from it. + +| Must not | Why | +|---|---| +| Stat 50k files to open the catalog | NFR-P1: catalog open < 2 s desktop, < 4 s Android. SAF `DocumentsContract` queries are far slower than `stat` (spike S10). | +| Re-derive thumbnails for unchanged images | NFR-P3 throughput is for *new* work; redoing it on every connect makes first paint unbounded. | +| Fetch previews for remote images nobody looks at | A 50k remote library at 1–3 MB per range-extract is 50–150 GB. FR-NC-6 forbids bulk transfer by default. | +| Evaluate cache rules per grid cell | ARCH §9.5 already answers this: `tier_desired` is materialised. | +| Block the UI executor on any of it | NFR-P9, NFR-ARCH-1. | + +The unifying principle: **work is proportional to what changed, or to what the user is looking at — +never to library size.** + +--- + +## 2. Schema + +Extends [architecture.md §6.2](architecture.md). Additions beyond that sketch are marked ⊕. + +```sql +-- Roots ----------------------------------------------------------------- +roots( + id INTEGER PRIMARY KEY, + kind TEXT, -- 'local' | 'saf' | 'remote' + grant_blob BLOB, -- SAF persisted permission; NULL on Linux + label TEXT, + last_seen INTEGER, + scan_generation INTEGER -- ⊕ bumped per completed scan; see §3.4 +); + +-- Folders: the unit of change detection, local and remote alike --------- +folders( + id INTEGER PRIMARY KEY, + root_id INTEGER NOT NULL REFERENCES roots(id), + parent_id INTEGER REFERENCES folders(id), + path TEXT NOT NULL, + etag TEXT, -- remote: propagating ETag (ARCH §8.4) + mtime INTEGER, -- ⊕ local: directory mtime + entry_count INTEGER, -- ⊕ local: direct children, mtime's blind spot + scanned_generation INTEGER, -- ⊕ deletion sweep; see §3.4 + UNIQUE(root_id, path) +); + +-- Images ---------------------------------------------------------------- +images( + id INTEGER PRIMARY KEY, + root_id INTEGER NOT NULL REFERENCES roots(id), + folder_id INTEGER REFERENCES folders(id), -- ⊕ folder filter without LIKE + source_ref TEXT NOT NULL, + content_hash TEXT, -- NULL until hashed; see §3.5 + format TEXT, + w INTEGER, h INTEGER, + captured_at INTEGER, -- UTC seconds; NULL if EXIF absent + captured_offset INTEGER, -- ⊕ minutes east of UTC; see §4.2 + camera TEXT, lens TEXT, + iso INTEGER, aperture REAL, shutter REAL, + availability INTEGER, + file_size INTEGER, -- ⊕ cheap change signal alongside mtime + file_mtime INTEGER, -- ⊕ + metadata_state INTEGER, -- ⊕ 0=none 1=stat-only 2=full EXIF; §3.5 + sidecar_mtime INTEGER, + UNIQUE(root_id, source_ref) +); + +-- Versions, keywords, remote, cache: per ARCH §6.2, unchanged ----------- + +-- Collections ⊕ --------------------------------------------------------- +collections( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL, + parent_id INTEGER REFERENCES collections(id), -- collection sets + kind INTEGER NOT NULL, -- 0 = manual, 1 = smart + selector_json TEXT, -- smart only; the §5 Selector + created INTEGER +); + +collection_members( + collection_id INTEGER NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + image_id INTEGER NOT NULL REFERENCES images(id) ON DELETE CASCADE, + position INTEGER, -- manual ordering; NULL = by capture time + PRIMARY KEY(collection_id, image_id) +); + +-- Jobs ⊕ ---------------------------------------------------------------- +jobs( + id INTEGER PRIMARY KEY, + kind INTEGER NOT NULL, + subject_id INTEGER, -- image or folder, per kind + priority INTEGER NOT NULL, + state INTEGER NOT NULL, -- 0=pending 1=running 2=failed + attempts INTEGER NOT NULL DEFAULT 0, + not_before INTEGER, -- retry backoff + payload TEXT, + UNIQUE(kind, subject_id) -- coalescing; see §6.2 +); +``` + +Indices that exist for a stated query, not speculatively: + +```sql +CREATE INDEX images_captured ON images(captured_at); -- §4 timeline +CREATE INDEX images_folder ON images(folder_id); +CREATE INDEX images_hash ON images(content_hash) WHERE content_hash IS NOT NULL; +CREATE INDEX folders_parent ON folders(parent_id); +CREATE INDEX jobs_ready ON jobs(state, priority DESC, not_before); +CREATE INDEX versions_image ON versions(image_id); +CREATE INDEX members_image ON collection_members(image_id); +``` + +`content_hash` is indexed *partially*. It is NULL for most rows most of the time (§3.5), and a +partial index over the non-NULL subset is both smaller and what FR-CAT-9's reconnection-by-hash +and FR-CAT-11's duplicate detection actually query. + +--- + +## 3. Incremental scan + +### 3.1 The local analogue of ETag pruning + +Nextcloud propagates ETags up the tree, so one request proves a whole library unchanged +([architecture.md §8.4](architecture.md)). A filesystem offers no such guarantee — a directory's +mtime changes when its *direct* entries change, and not when a grandchild does. There is no +cheap "did anything below here change" probe. + +So local scan prunes at each level rather than at the root: + +``` +scan(folder): + (mtime, count) = stat(folder) + if (mtime, count) == stored: + # This directory's own entries are unchanged. Its files need no + # examination at all — but subdirectories may still have changed + # internally, so recurse into known children without listing. + for child in stored_children(folder): + scan(child) + else: + entries = list(folder) # the expensive call + reconcile(folder, entries) # §3.3 + for child in entries.dirs: scan(child) + mark scanned(folder, current_generation) +``` + +Cost is **one `stat` per directory** when nothing changed, versus one per *file*. A 50k-image +library in ~2k folders costs 2k stats — a few milliseconds locally, and the difference between +meeting and missing NFR-P1 on SAF. + +The recursion into unchanged directories is not redundant: it is what makes a change to one deep +file detectable at all, given no upward propagation. What it avoids is the *listing* — on SAF a +`DocumentsContract` query returning 200 rows costs far more than a metadata probe on the directory +itself. + +### 3.2 Why entry-count as well as mtime + +Directory mtime alone misses a real case: delete one file and create another within the same +timestamp granularity, and mtime can be unchanged while contents differ. Some filesystems and most +SAF providers report coarse timestamps, which widens the window. + +Storing `(mtime, entry_count)` closes the common form of this — a paired add and remove changes +neither, but that is rarer than a bare add or remove, and both of those move the count. It is a +cheap narrowing, not a proof. + +**Where correctness must not depend on it,** the user gets an explicit *Rescan folder* action +(FR-CAT-1), and reconnection matches by content hash (FR-CAT-9). Sync's remote path is unaffected — +ETags are authoritative there. + +### 3.3 Reconciling a changed directory + +For each entry in a listing: + +| Situation | Action | +|---|---| +| Not in catalog | Insert with `metadata_state = 1`; enqueue `ExtractMetadata` | +| In catalog, `(size, mtime)` match | Nothing — the common case | +| In catalog, `(size, mtime)` differ | Re-enqueue `ExtractMetadata` and `Thumbnail`; clear `content_hash` | +| In catalog, absent from listing | Deletion candidate — §3.4 | +| Placeholder (`*.nextcloud`) | Catalogue as the image it stands for; `Availability::Offline` (ARCH §9.0) | + +Sidecars are examined in the same pass: a `.drsc` whose mtime exceeds `images.sidecar_mtime` enqueues +a `ReadSidecar` job. This is how an edit made on another device — landed by the Nextcloud client, +not by us — reaches the catalog. + +### 3.4 Deletion without a full sweep + +A file removed outside the app appears only as an *absence*, which a pruned scan cannot see: the +folder it vanished from has a changed mtime and is listed, but a folder never visited is never +compared. + +Generation counting handles this without a full pass. Each scan bumps `roots.scan_generation`, and +every folder reached — whether listed or skipped — records it. After the walk: + +```sql +-- Folders never reached: their parent no longer lists them. +DELETE FROM folders + WHERE root_id = ?1 AND scanned_generation < ?2; +``` + +Images under a deleted folder cascade. Images missing from a *listed* folder are caught directly in +§3.3. Together these cover deletion with no additional traversal. + +Deletion here means **removing the catalog row for a source proven absent**, which FR-CAT-9 sharply +distinguishes from a source merely unreachable. A root that fails to open at all — unplugged drive, +revoked SAF grant — aborts the scan and marks the root offline. It never runs the sweep, because +every folder would look unreached and the sweep would delete the entire library. + +That guard is the single most dangerous line in this design, and it is stated as an invariant: +**the deletion sweep runs only after a scan that completed without a root-level access error.** + +### 3.5 Metadata in two passes + +Full EXIF extraction requires opening and parsing each file. At 50k images that is minutes, and it +must not stand between the user and a usable grid. + +`metadata_state` records how far each image has got: + +| State | Holds | Cost | +|---|---|---| +| 0 — none | Row exists, nothing read | — | +| 1 — stat-only | Name, size, mtime, format from extension | Free, from the listing | +| 2 — full | EXIF: capture time, camera, lens, exposure, dimensions | One open + parse | + +The grid is usable at state 1: it can show filenames, sort by filename or file mtime, and display +placeholder cells. Promotion to state 2 runs as background jobs, prioritised by what is on screen +(§6.3), so visible images get real capture times within a frame or two of being scrolled to. + +**Capture-time filtering (§4) needs state 2**, so a freshly scanned library's timeline is incomplete +until the pass finishes. The UI states this plainly — a progress affordance on the timeline, not a +silently wrong filter. Which is the FR-NC-6c principle applied to metadata rather than pixels: say +what you actually have. + +`content_hash` is a *third*, still lazier tier. It requires reading the whole file, so it is computed +only when something needs it: import duplicate detection (FR-CAT-11), or reconnecting a moved source +(FR-CAT-9). Never during a routine scan. + +--- + +## 4. The library view + +### 4.1 Query model + +The UI never assembles SQL. It hands the catalog a `Query` and receives a stable, windowable result: + +```rust +pub struct Query { + pub filter: Selector, // §5 — same type cache rules use + pub sort: Sort, + pub descending: bool, +} + +pub enum Sort { + CapturedAt, + Added, + FileName, + Rating, + /// Manual order within a collection; falls back to CapturedAt elsewhere. + CollectionPosition, +} +``` + +Results are fetched by window, never wholesale — FR-CAT-4 requires memory bounded independently of +catalog size: + +```rust +impl Catalog { + fn count(&self, q: &Query) -> Result; + fn window(&self, q: &Query, range: Range) -> Result, CatalogError>; +} +``` + +`GridRow` carries exactly what a cell draws — id, thumbnail key, availability, rating, flag, capture +time — and nothing that would require a join per cell. Availability badges read `tier_desired` +directly (ARCH §9.5), so no rule evaluation happens on the render path. + +A `LIMIT/OFFSET` window degrades at high offsets, since SQLite must walk the skipped rows. Scrolling +is overwhelmingly *sequential*, so the catalog keeps a keyset cursor for forward and backward paging +and falls back to OFFSET only for a scrollbar jump. Jumps are rare and single; scrolling is +continuous. + +### 4.2 Time + +Capture time is the spine of a photo library, and it has one persistent trap: **a photograph's +timestamp is local to where it was taken.** Store UTC alone and a shoot that ran 09:00–17:00 in +Tokyo displays as spanning two days in Paris. Store local time alone and ordering across a timezone +change is wrong. + +So both: `captured_at` in UTC for ordering, `captured_offset` in minutes for display and for +day-bucketing. EXIF `OffsetTimeOriginal` supplies it where present; where absent — common on older +bodies — the offset is NULL and the catalog falls back to the library's configured display timezone, +flagged so the UI can show it as inferred. + +Day, month, and year buckets are computed against **local** time. "Everything from 3 August" means +the photographer's 3 August. + +The timeline affordance is a histogram of counts per bucket, which the grid uses for scrubbing: + +```rust +pub enum Granularity { Year, Month, Day, Hour } + +pub struct TimeBucket { + pub start: i64, // UTC seconds, bucket start + pub count: u32, +} + +fn timeline(&self, q: &Query, g: Granularity) -> Result, CatalogError>; +``` + +This is one grouped aggregate over the `images_captured` index, not 50k rows into the UI. It is what +makes "drag across two years to find the trip" work, and it is the cheapest useful thing a library +view can offer over a flat grid. + +### 4.3 Filtering interactively + +FR-CAT-6 requires filter results to update interactively on 50k images. Three things make that hold: + +1. **Filters compile to indexed predicates.** A `Selector` becomes a WHERE clause over indexed + columns. Keyword and collection membership become `EXISTS` subqueries against their own indices. +2. **Count and first window are one round trip.** The grid needs a row count to size its scrollbar + and the first screenful to paint; the catalog returns both together. +3. **A filter change cancels the one in flight.** Typing in a search box issues a query per + keystroke; each supersedes the last (NFR-ARCH-3). Without this the UI queues work it will discard. + +--- + +## 5. Selectors: one type, three uses + +[architecture.md §9.2](architecture.md) defines `Selector` for cache rules. The same type expresses +library filters and smart collections. This is deliberate and worth stating as a design decision, +because three near-identical predicate languages is a classic way for a catalog to rot. + +| Use | Meaning | +|---|---| +| Library filter | What the grid shows now | +| Smart collection | A saved, named filter (FR-CAT-7) | +| Cache rule | What is kept locally, at which tier (FR-NC-6a) | + +One consequence is directly useful: any filter the user has narrowed to can be saved as a smart +collection, and any collection can be pinned offline, with no conversion step. "Show me 5-star images +from the last 90 days" → save as a collection → pin it for the trip. Three features, one mechanism. + +`Selector` moves to `dr-types` so `dr-catalog` and `dr-sync` share it without either depending on the +other. It gains variants the cache-rule sketch did not need: + +```rust +pub enum Selector { + All, // ⊕ the empty filter + Collection(CollectionId), + Folder { root: RootId, path: String, recursive: bool }, + DateRange(DateSelector), + Rating { min: u8 }, + Label(ColourLabel), + Flag(FlagState), + Keyword(String), + Camera(String), // ⊕ FR-CAT-6 indexed field + Lens(String), // ⊕ + IsoRange { min: u32, max: u32 }, // ⊕ + Availability(Availability), // ⊕ "what can I edit right now" + Text(String), // ⊕ filename/keyword substring + All_(Vec), + Any(Vec), + Not(Box), +} +``` + +`Availability` as a selector earns its place: on a tablet the most useful filter is often "what do I +actually have here", and it is also the natural thing to *pin* — "keep everything I've flagged that +isn't already local". + +Compilation is a straightforward recursive walk producing SQL with bound parameters. **Nothing +user-supplied is ever interpolated into SQL text.** `Text` becomes a bound `LIKE` pattern with `%`, +`_`, and the escape character escaped. + +--- + +## 6. Background work + +### 6.1 Job kinds + +```rust +pub enum JobKind { + ScanFolder, // §3, recursive from a folder + ExtractMetadata, // state 1 → 2 + Thumbnail, // §7 + ReadSidecar, // external sidecar change detected + WriteSidecar, // local edit → disk, debounced (ARCH §6.1) + ContentHash, // on demand only + FetchPreview, // remote range-extract (FR-NC-3) + FetchOriginal, // pinned or explicitly requested +} +``` + +### 6.2 Coalescing is the point + +`UNIQUE(kind, subject_id)` on `jobs` means enqueueing is idempotent: an image touched five times +during a scan has one thumbnail job, not five. Enqueue is +`INSERT … ON CONFLICT DO UPDATE SET priority = max(priority, excluded.priority)`, so a re-request at +higher priority promotes the existing row rather than duplicating it. + +This is what makes "regenerate on update" safe to call liberally. Every code path that notices a +change can just enqueue; the table absorbs the redundancy. + +### 6.3 Priority + +Reuses the existing GPU scheduler classes ([architecture.md §5.3](architecture.md)) so one notion of +priority governs the whole app: + +| Class | Jobs | Preempts | +|---|---|---| +| `Interactive` | Metadata and thumbnails for visible cells; preview for the open image | everything | +| `Prefetch` | The scroll margin; next image in culling | Background | +| `Background` | Bulk metadata, rule-driven fetches, hashing | — | + +Visible-cell work is enqueued by the grid as it scrolls, at `Interactive`. The effect is that a +freshly scanned library fills in *where the user is looking* first, and grinds through the rest +behind them. + +### 6.4 Durability and failure + +Jobs live in the catalog, so they survive process death — which on Android is routine, not +exceptional (FR-PLAT-AND-3). On startup, rows in state `running` revert to `pending`: the process +that owned them is gone. + +Failures increment `attempts` and set `not_before` to an exponential backoff. After a bounded retry +count the job is marked failed and attached to its image as a typed error (NFR-ARCH-4) — one +corrupt file does not stall the queue, and the user can see which files failed and why. + +**A job runner never touches the UI executor**, and `Interactive` work runs on the decode pool with +the I/O pool behind it (ARCH §7.1). + +--- + +## 7. Thumbnails + +### 7.1 When + +Not "on first connect" as a bulk operation. Thumbnails are generated: + +- **On demand**, for cells entering the viewport plus the prefetch margin — at `Interactive` +- **On change**, when §3.3 sees a differing `(size, mtime)` +- **On rule**, for images a cache rule pins at `Preview` or above — at `Background` +- **Never** for a remote image nobody has looked at and no rule covers + +For a local library this converges on "everything, eventually", because scrolling reaches everything +and the background pass has nothing else to do. For a remote library it converges on "what you +actually browsed", which is the difference between a few hundred megabytes and a hundred gigabytes. + +### 7.2 How, by availability + +| Availability | Source | Cost | +|---|---|---| +| `Original`, local | Embedded JPEG via `dr-decode` preview path | ~200 KB read, no demosaic | +| `Original`, no embedded preview | Full decode, downscale | Expensive — `Background` only | +| Remote | Range-extract embedded JPEG (FR-NC-3) | 1–3 MB vs 25–100 MB | +| Placeholder / `Offline` | None — render the offline affordance | 0 | + +The remote path deliberately does **not** ask the Nextcloud client to hydrate the file. ARCH §9.0 +established hydration is whole-file, so it costs ~100× what the range extract does. Hydration stays +reserved for the original tier, where the user has asked for the actual image. + +Server previews (`/core/preview`) are tried only where PROPFIND reported `nc:has-preview`. ARCH §6.7 +verified stock Nextcloud ships no RAW preview provider, so for RAW this is nearly always absent — it +is an opportunistic saving, never the mechanism. + +### 7.3 Storage + +Thumbnails are content-addressed by `(content_hash | source_ref, size_class)` and stored as files +under the platform cache directory, with the `cache` table holding the index. Files, not BLOBs: +SQLite handles small blobs well but a 50k-image thumbnail cache is gigabytes, and mixing it into the +catalog would bloat the file the app must open in under two seconds. + +Two size classes at v1 — grid (256px) and filmstrip/loupe (1024px) — both long-edge, both JPEG. The +cache is LRU-capped per NFR-RES-4, and thumbnails evict before proxies and long after sidecars, +which never evict at all (FR-NC-6b). + +--- + +## 8. What this document does not settle + +- **FTS.** `Selector::Text` is a `LIKE` scan over filename and keywords. Adequate at 50k; if free + text over description and title becomes a real workflow, an FTS5 table is the answer, and it is + additive. +- **Smart collection materialisation.** Currently evaluated on read. If a smart collection's + membership needs to be *stable* — for manual ordering, or for a pinned set that must not shift + under the user — it needs materialising with an invalidation rule. Deferred until there is a + concrete need. +- **Multi-root capture-time collisions.** FR-CAT-11 detects duplicates on import; the same image + catalogued under two roots is a related but distinct case, not yet specified. +- **Timeline granularity selection.** Which bucket size the UI picks for a given zoom is a UI + concern, but the catalog should probably suggest one from the query's date span rather than have + the UI guess. + +--- + +## 9. Requirements touched + +| ID | How this document addresses it | +|---|---| +| FR-CAT-1 | §3 incremental scan, cancellable and resumable via §6 jobs | +| FR-CAT-3 | §7 thumbnail pyramid, two size classes, embedded-preview fast path | +| FR-CAT-4 | §4.1 windowed queries, memory independent of catalog size | +| FR-CAT-5 | §3.5 two-pass metadata | +| FR-CAT-6 | §4.3 indexed filter compilation, §5 selectors | +| FR-CAT-7 | §2 collections schema, §5 manual and smart | +| FR-CAT-9 | §3.4 the offline/deleted distinction and the sweep guard | +| FR-CAT-11 | §3.5 lazy content hashing | +| FR-NC-3 | §7.2 range-extract for remote thumbnails | +| FR-NC-6a | §5 shared selector type | +| FR-NC-6c | §3.5 metadata honesty, §7.2 availability-driven sourcing | +| NFR-P1 | §3.1 one stat per directory, not per file | +| NFR-P3 | §7.1 on-demand generation | +| NFR-ARCH-2 | §6.3 priority classes shared with the GPU scheduler | +| NFR-ARCH-3 | §4.3 query cancellation, §6 job cancellation | +| NFR-RES-4 | §7.3 LRU cap, eviction order |