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

Sync with integration

Browse Source
This commit is contained in:
dtourolle
2026-08-22 19:04:27 +02:00
parent 97d4bd9061 45214d3ca8
commit a9baebc396
22 changed files with 4532 additions and 316 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-catalog/src/collections.rs
+1 -1
View File
@@ -682,7 +682,7 @@ fn require_exists(conn: &Connection, id: CollectionId) -> Result<(), CatalogErro
/// same reasoning as the connector's date parsing. Version 4 layout, seeded
/// from the OS via `getrandom` through `rusqlite`'s existing dependency-free
/// path — see below.
fn new_uuid() -> String {
pub(crate) fn new_uuid() -> String {
let b = random_bytes();
// Version 4, variant 1, per RFC 4122 §4.4.
let v6 = (b[6] & 0x0F) | 0x40;
core/dr-catalog/src/error.rs
+9
View File
@@ -42,6 +42,15 @@ pub enum CatalogError {
#[error("no such collection: {0}")]
NoSuchCollection(u64),
/// A keyword the caller named is gone — deleted, or fused into another by a
/// merge while its id sat in a UI model.
///
/// Its own variant rather than a silent no-op because the two are different
/// answers to the user: a rename that quietly did nothing looks exactly like
/// a rename that did not take.
#[error("no such keyword: {0}")]
NoSuchKeyword(u64),
/// Images were dropped onto a smart collection.
///
/// A smart collection's membership *is* its selector, so member rows would
core/dr-catalog/src/keywords.rs
+1220
View File
File diff suppressed because it is too large Load Diff
core/dr-catalog/src/lib.rs
+4 -1
View File
@@ -14,9 +14,10 @@
//! - [`walk`] — those decisions driven against real storage, local or SAF
//! - [`query`] — selectors compiled to indexed SQL, windowed for the grid
//! - [`collections`] — the collection tree and membership the UI edits
//! - [`keywords`] — the keyword vocabulary and what it is assigned to
//! - [`jobs`] — the durable background work queue
//! - [`trash`] — soft delete to a folder, then permanent delete
//! - [`merge`] / [`sync`] — cross-device collection merging
//! - [`merge`] / [`sync`] — cross-device merging of collections and keywords
//!
//! # The one thing everything is designed around
//!
@@ -35,6 +36,7 @@ pub mod cache;
pub mod collections;
pub mod error;
pub mod jobs;
pub mod keywords;
pub mod merge;
pub mod query;
pub mod rating;
@@ -48,6 +50,7 @@ pub use cache::{Budget, Cache, DEFAULT_BUDGET_BYTES};
pub use collections::{Collection, CollectionKind, TreeRow};
pub use error::CatalogError;
pub use jobs::{Job, JobKind, Priority};
pub use keywords::{Coverage, Keyword, KeywordId, SelectionKeyword};
pub use merge::MergeReport;
pub use query::{Query, Sort};
pub use rating::{Judgement, MAX_RATING};
core/dr-catalog/src/merge.rs
+673 -9
View File
@@ -1,5 +1,5 @@
//! TRACES: FR-CAT-7 | FR-NC-9
//! Merging a remote catalog's collections into the local one.
//! TRACES: FR-CAT-7 | FR-CAT-5 | FR-NC-9
//! Merging a remote catalog's collections and keywords into the local one.
//!
//! # Why this is a merge and not a copy
//!
@@ -33,6 +33,35 @@
//! 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.
//!
//! # Keywords merge on the same three rules
//!
//! [`merge_keywords`] reuses all of the above rather than inventing a second
//! set of rules, because a keyword is the same shape of problem as a
//! collection: a named thing with a device-independent identity, and a
//! many-to-many join to images.
//!
//! - The **vocabulary** (`keyword_terms`) is decided per row by [`verdict`],
//! exactly as collections are.
//! - The **assignments** (`keywords`) are a set union, exactly as membership
//! is: two devices each keywording different photographs "puffin" keep both
//! sets, and two devices each keywording the *same* photograph converge on
//! one row rather than one of them winning.
//! - **Deletion** tombstones, and takes the assignments with it.
//!
//! Two things are genuinely different, and both are consequences of assignments
//! storing the *word* rather than a row id:
//!
//! 1. A tombstone deletes assignments **by name**, so a deletion still lands on
//! a device that had minted its own identity for the same word. The union
//! then refuses to readmit a word a winning tombstone has just removed —
//! without that filter, the other device's live assignments would resurrect
//! it on the very same pass.
//! 2. Two devices that independently typed the same word arrive with two uuids
//! for one keyword. [`crate::keywords::fuse_duplicates`] collapses them onto
//! the lexicographically smaller one, which both devices compute identically.
//! A unique index on the name would instead abort the merge transaction at
//! that moment, which is the ordinary case rather than a corner one.
use rusqlite::Connection;
@@ -59,18 +88,44 @@ pub struct MergeReport {
pub kept_local: usize,
pub deleted: usize,
pub members_added: usize,
// Keywords are counted separately from collections rather than summed into
// the same fields. The report is shown to the user — "3 collections, 11
// keywords" is a sentence; "14 things" is not — and a merge that went wrong
// is far easier to place when the counts say which half it went wrong in.
/// Keywords the remote had and this device did not.
pub keywords_inserted: usize,
/// Keywords the remote had renamed, or brought back from a tombstone.
pub keywords_updated: usize,
/// Keywords the remote deleted, and this device has now deleted too.
pub keywords_deleted: usize,
/// Keywords where this device's revision was at least as high.
pub keywords_kept_local: usize,
/// Redundant identities for one word, retired by
/// [`crate::keywords::fuse_duplicates`].
pub keywords_fused: usize,
/// Keyword assignments taken from the remote.
pub keywords_assigned: 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
self.inserted > 0
|| self.updated > 0
|| self.deleted > 0
|| self.members_added > 0
|| self.keywords_inserted > 0
|| self.keywords_updated > 0
|| self.keywords_deleted > 0
|| self.keywords_fused > 0
|| self.keywords_assigned > 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()
self.kept_local > 0 || self.keywords_kept_local > 0 || self.local_changed()
}
}
@@ -108,16 +163,55 @@ pub fn verdict(
}
}
/// Merge collections and membership from an attached catalog.
/// Merge everything that syncs, from an attached catalog.
///
/// The remote catalog must already be attached under the schema name
/// `remote_cat`; [`crate::Catalog::merge_attached_collections`] handles that.
/// `remote_cat`; [`crate::sync::merge_remote`] handles that.
///
/// **One transaction over both halves.** Keywords and collections are
/// independent as data, but a merge that landed the collections and then failed
/// on the keywords would leave a catalog that has already taken the remote's
/// revisions for half of itself — and the next attempt, seeing those revisions,
/// would decline to take them again. Half a merge is not a state that can be
/// resumed, so it is not a state that can be reached.
pub fn merge_all(conn: &Connection) -> Result<MergeReport, CatalogError> {
let tx = conn.unchecked_transaction()?;
let mut report = MergeReport::default();
merge_collections_within(&tx, &mut report)?;
merge_keywords_within(&tx, &mut report)?;
tx.commit()?;
Ok(report)
}
/// Merge collections and membership from an attached catalog.
///
/// The collections half of [`merge_all`], on its own. Kept as a public entry
/// point because the two halves are genuinely independent, and because the
/// rules for this one are worth being able to exercise without a keyword in
/// sight.
///
/// Runs in one transaction: a merge either lands whole or not at all.
pub fn merge_collections(conn: &Connection) -> Result<MergeReport, CatalogError> {
let tx = conn.unchecked_transaction()?;
let mut report = MergeReport::default();
merge_collections_within(&tx, &mut report)?;
tx.commit()?;
Ok(report)
}
/// Merge the keyword vocabulary and its assignments from an attached catalog.
///
/// The keywords half of [`merge_all`], on its own. See the module header for
/// the three rules and the two places keywords differ from collections.
pub fn merge_keywords(conn: &Connection) -> Result<MergeReport, CatalogError> {
let tx = conn.unchecked_transaction()?;
let mut report = MergeReport::default();
merge_keywords_within(&tx, &mut report)?;
tx.commit()?;
Ok(report)
}
fn merge_collections_within(tx: &Connection, report: &mut MergeReport) -> Result<(), CatalogError> {
// ---- collections ------------------------------------------------------
{
let mut stmt = tx.prepare(
@@ -310,8 +404,245 @@ pub fn merge_collections(conn: &Connection) -> Result<MergeReport, CatalogError>
)?;
report.members_added = added;
tx.commit()?;
Ok(report)
Ok(())
}
/// Schema name the downloaded remote catalog is attached under.
///
/// Repeated from [`crate::sync`] rather than shared, because the SQL below
/// spells it inline and a constant that only half the file used would be worse
/// than no constant at all.
const REMOTE: &str = "remote_cat";
/// The keyword half. See the module header.
fn merge_keywords_within(tx: &Connection, report: &mut MergeReport) -> Result<(), CatalogError> {
// ---- the vocabulary ---------------------------------------------------
//
// A remote written before schema v6 has no `keyword_terms` at all, and
// `remote_is_mergeable` deliberately admits it: the check is that the
// remote is not *newer* than us. So the table's absence is a normal state
// and not an error. Its assignments still merge below — those have been in
// the schema since v1 — and its words gain identities on that device the
// next time it opens the catalog and backfills.
if attached_has_table(tx, REMOTE, "keyword_terms")? {
struct Incoming {
uuid: String,
name: String,
/// What this device currently calls the same identity, if it has
/// it. A rename is applied to the assignment rows by rewriting this
/// text, so it has to be read before the term row is overwritten.
local_name: Option<String>,
created: i64,
revision: i64,
modified: i64,
verdict: MergeVerdict,
}
let rows: Vec<Incoming> = {
let mut stmt = tx.prepare(
"SELECT r.uuid, r.name, r.created, r.revision, r.modified, r.deleted,
l.name, l.revision, l.modified
FROM remote_cat.keyword_terms r
LEFT JOIN main.keyword_terms l ON l.uuid = r.uuid",
)?;
let found = stmt
.query_map([], |r| {
let deleted: i64 = r.get(5)?;
let local_rev: Option<i64> = r.get(7)?;
let local_mod: Option<i64> = r.get(8)?;
let revision: i64 = r.get(3)?;
let modified: i64 = r.get(4)?;
Ok(Incoming {
uuid: r.get(0)?,
name: r.get(1)?,
local_name: r.get(6)?,
created: r.get(2)?,
revision,
modified,
verdict: verdict(
local_rev.zip(local_mod),
(revision, modified),
deleted != 0,
),
})
})?
.collect::<Result<Vec<_>, _>>()?;
found
};
for row in rows {
match row.verdict {
MergeVerdict::KeptLocal => {
report.keywords_kept_local += 1;
}
MergeVerdict::InsertedFromRemote => {
tx.execute(
"INSERT INTO main.keyword_terms
(uuid, name, created, revision, modified, deleted)
VALUES (?1, ?2, ?3, ?4, ?5, 0)",
rusqlite::params![
row.uuid,
row.name,
row.created,
row.revision,
row.modified,
],
)?;
report.keywords_inserted += 1;
}
MergeVerdict::UpdatedFromRemote => {
// The assignments carry the *word*, so taking a new name
// for an identity we already hold means rewriting every row
// spelt the old way. Without this the vocabulary would show
// the new spelling and the search would only find the old.
if let Some(old) = row.local_name.filter(|n| *n != row.name) {
tx.execute(
"INSERT OR IGNORE INTO main.keywords(version_id, keyword)
SELECT version_id, ?2 FROM main.keywords WHERE keyword = ?1",
rusqlite::params![old, row.name],
)?;
tx.execute("DELETE FROM main.keywords WHERE keyword = ?1", [&old])?;
}
tx.execute(
"UPDATE main.keyword_terms
SET name = ?2, revision = ?3, modified = ?4, deleted = 0
WHERE uuid = ?1",
rusqlite::params![row.uuid, row.name, row.revision, row.modified],
)?;
report.keywords_updated += 1;
}
MergeVerdict::DeletedByRemote => {
// Tombstone rather than DELETE, or a third device
// reintroduces the keyword through us.
tx.execute(
"INSERT INTO main.keyword_terms
(uuid, name, created, revision, modified, deleted)
VALUES (?1, ?2, ?3, ?4, ?5, 1)
ON CONFLICT(uuid) DO UPDATE SET
deleted = 1, revision = ?4, modified = ?5",
rusqlite::params![
row.uuid,
row.name,
row.created,
row.revision,
row.modified,
],
)?;
// **By name, not by identity.** This device may well have
// minted its own uuid for the same word before the two ever
// synced, in which case deleting by uuid would tombstone a
// row that nothing is assigned to and leave every
// photograph still carrying the word.
tx.execute("DELETE FROM main.keywords WHERE keyword = ?1", [&row.name])?;
report.keywords_deleted += 1;
}
}
}
// Two devices that each typed "Iceland" now hold two identities for one
// word. Collapse them before the assignments arrive, so the vocabulary
// the user sees after a sync has one row per word.
report.keywords_fused = crate::keywords::fuse_duplicates(tx)?;
}
// ---- assignments ------------------------------------------------------
//
// Set union, and the union is the whole point: FR-NC-9's principle applied
// to metadata rather than to edit nodes. Two devices that keyworded
// different frames "puffin" both keep their work, and neither loses it to
// whichever synced second.
//
// A removal therefore does not propagate — the remote's assignment simply
// reappears. That is the same trade-off collection membership makes above,
// and for the same reason: an unwanted keyword is removed again in a
// second, and a silently lost afternoon of keywording is not recoverable at
// all. Making removal propagate needs a tombstone per assignment, which is
// a schema change and a merge rule of its own.
//
// An incoming keyword lands on the local default version, so an image that
// has not got one yet would silently drop it. That is not a rare state: the
// invariant is maintained by a backfill on open, and a scan that ran since
// has added rows it has not covered. Losing a word the user typed on
// another device, for a bookkeeping reason, would be the wrong answer —
// this is idempotent and writes nothing once the invariant holds.
crate::rating::ensure_default_versions_within(tx)?;
// Two passes rather than one statement with an `OR`, because they resolve
// *different identities* for the same photograph and each wants its own
// index. See [`ASSIGN_BY_FILE_ID`] for why there are two at all.
for sql in [ASSIGN_BY_FILE_ID, ASSIGN_BY_CONTENT_HASH] {
report.keywords_assigned += tx.execute(sql, [])?;
}
Ok(())
}
/// Take assignments for images both devices know by the server's file id.
///
/// **Preferred over the content hash**, and the reason is that `content_hash`
/// is expensive — the schema says so, and it is computed only when import
/// dedup or a reconnect asks for it, which for most libraries is never. Keying
/// keywords on it alone would mean the union quietly did nothing for the
/// ordinary image, which is the exact failure this merge exists to prevent.
///
/// `oc:fileid` is the opposite: it is recorded for every image the moment a
/// remote scan sees it, it is stable across server-side renames and moves, and
/// it is the same integer on every device pointed at the same Nextcloud — which
/// is precisely the situation where two devices are keywording one library.
///
/// The keyword lands on the local image's **default version**, not on the
/// version it came from. Version uuids do not reconcile across devices in the
/// catalog: [`crate::rating::ensure_default_versions`] mints a fresh one per
/// device, so the same photograph's default versions have different uuids on
/// two machines and a uuid-keyed join would union nothing at all. Version
/// identity is reconciled in the *sidecar* (FR-NC-8), and until a merged
/// version arrives through there, the default version is both where
/// [`crate::keywords::assign`] writes and where the panel reads — so it is the
/// one place the word can land and be seen.
const ASSIGN_BY_FILE_ID: &str = "
INSERT OR IGNORE INTO main.keywords(version_id, keyword)
SELECT lv.id, rk.keyword
FROM remote_cat.keywords rk
JOIN remote_cat.versions rv ON rv.id = rk.version_id
JOIN remote_cat.remote rr ON rr.image_id = rv.image_id
JOIN main.remote lr ON lr.file_id = rr.file_id
JOIN main.versions lv ON lv.image_id = lr.image_id AND lv.is_default = 1
WHERE NOT EXISTS (SELECT 1 FROM main.keyword_terms t
WHERE t.name = rk.keyword AND t.deleted = 1)
OR EXISTS (SELECT 1 FROM main.keyword_terms t
WHERE t.name = rk.keyword AND t.deleted = 0)";
/// The same union for a library with no server behind it.
///
/// A local-only library has no `remote` rows at all, so [`ASSIGN_BY_FILE_ID`]
/// matches nothing and this is the only identity available — and it is the one
/// collection membership already uses, so a library where membership merges
/// has keywords that merge too.
const ASSIGN_BY_CONTENT_HASH: &str = "
INSERT OR IGNORE INTO main.keywords(version_id, keyword)
SELECT lv.id, rk.keyword
FROM remote_cat.keywords rk
JOIN remote_cat.versions rv ON rv.id = rk.version_id
JOIN remote_cat.images ri ON ri.id = rv.image_id
JOIN main.images li ON li.content_hash = ri.content_hash
JOIN main.versions lv ON lv.image_id = li.id AND lv.is_default = 1
WHERE ri.content_hash IS NOT NULL
AND (NOT EXISTS (SELECT 1 FROM main.keyword_terms t
WHERE t.name = rk.keyword AND t.deleted = 1)
OR EXISTS (SELECT 1 FROM main.keyword_terms t
WHERE t.name = rk.keyword AND t.deleted = 0))";
/// Whether an attached database holds a table of this name.
///
/// The schema name and the table name are both literals from this file, never
/// user text — but they are still bound rather than formatted where SQLite
/// allows it, because the habit is what keeps the one that eventually is user
/// text from being formatted by accident.
fn attached_has_table(conn: &Connection, schema: &str, table: &str) -> Result<bool, CatalogError> {
let sql =
format!("SELECT count(*) FROM {schema}.sqlite_master WHERE type = 'table' AND name = ?1");
let n: i64 = conn.query_row(&sql, [table], |r| r.get(0))?;
Ok(n > 0)
}
#[cfg(test)]
@@ -391,12 +722,21 @@ mod tests {
// ---- integration over two real catalogs ------------------------------
fn two_catalogs() -> Connection {
attached_remote(schema::for_attached("remote_cat"))
}
/// The same pair, but with the remote stopped at v1 — a device running a
/// build from before keywords had identities.
fn two_catalogs_with_a_v1_remote() -> Connection {
attached_remote(schema::v1_for_attached("remote_cat"))
}
fn attached_remote(remote_schema: String) -> 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
}
@@ -656,6 +996,330 @@ mod tests {
assert!(!second.local_changed(), "merge must be idempotent");
}
// ---- keywords --------------------------------------------------------
/// Give an image a default version, as every write path assumes it has.
fn add_version(c: &Connection, db: &str, image: i64, uuid: &str) -> i64 {
c.execute(
&format!(
"INSERT INTO {db}.versions(image_id, uuid, name, is_default)
VALUES (?1, ?2, 'Default', 1)"
),
rusqlite::params![image, uuid],
)
.unwrap();
c.last_insert_rowid()
}
/// Put a word on an image's default version, creating the version.
///
/// The version uuid is derived from the database *and* the image, so the
/// two catalogs never accidentally agree on one — which is the real
/// situation, and the reason the assignment union cannot key on it.
fn keyword(c: &Connection, db: &str, image: i64, word: &str) {
let existing: Option<i64> = c
.query_row(
&format!("SELECT id FROM {db}.versions WHERE image_id = ?1 AND is_default = 1"),
[image],
|r| r.get(0),
)
.ok();
let version =
existing.unwrap_or_else(|| add_version(c, db, image, &format!("v-{db}-{image}")));
c.execute(
&format!("INSERT OR IGNORE INTO {db}.keywords(version_id, keyword) VALUES (?1, ?2)"),
rusqlite::params![version, word],
)
.unwrap();
}
fn add_term(c: &Connection, db: &str, uuid: &str, name: &str, rev: i64, deleted: i64) {
c.execute(
&format!(
"INSERT INTO {db}.keyword_terms(uuid, name, created, revision, modified, deleted)
VALUES (?1, ?2, 0, ?3, ?3, ?4)"
),
rusqlite::params![uuid, name, rev, deleted],
)
.unwrap();
}
/// Map an image to a server file id, as a remote scan does.
fn add_file_id(c: &Connection, db: &str, image: i64, file_id: i64) {
c.execute(
&format!("INSERT INTO {db}.remote(image_id, file_id) VALUES (?1, ?2)"),
rusqlite::params![image, file_id],
)
.unwrap();
}
/// Every word on an image locally, sorted.
fn words_on(c: &Connection, image: i64) -> Vec<String> {
let mut stmt = c
.prepare(
"SELECT DISTINCT k.keyword FROM main.keywords k
JOIN main.versions v ON v.id = k.version_id
WHERE v.image_id = ?1 ORDER BY k.keyword",
)
.unwrap();
let rows = stmt.query_map([image], |r| r.get(0)).unwrap();
rows.collect::<Result<Vec<_>, _>>().unwrap()
}
fn live_terms(c: &Connection) -> Vec<String> {
let mut stmt = c
.prepare("SELECT name FROM main.keyword_terms WHERE deleted = 0 ORDER BY name")
.unwrap();
let rows = stmt.query_map([], |r| r.get(0)).unwrap();
rows.collect::<Result<Vec<_>, _>>().unwrap()
}
#[test]
fn two_devices_keywording_different_photographs_both_survive() {
// FR-NC-9's principle applied to metadata: disjoint work merges to the
// union, and neither device loses an afternoon to whoever synced last.
let c = two_catalogs();
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
add_image(&c, db, 2, "hash-b");
}
keyword(&c, "main", 1, "puffin");
keyword(&c, "remote_cat", 2, "gannet");
merge_keywords(&c).unwrap();
assert_eq!(words_on(&c, 1), ["puffin"]);
assert_eq!(words_on(&c, 2), ["gannet"]);
}
#[test]
fn two_devices_keywording_one_photograph_keep_both_words() {
// The case the union is really for: the same frame, two different
// words, and last-writer-wins would silently drop one of them.
let c = two_catalogs();
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
}
keyword(&c, "main", 1, "puffin");
keyword(&c, "remote_cat", 1, "Iceland");
let report = merge_keywords(&c).unwrap();
assert_eq!(report.keywords_assigned, 1);
assert_eq!(words_on(&c, 1), ["Iceland", "puffin"]);
}
#[test]
fn a_word_both_devices_already_had_is_not_duplicated() {
let c = two_catalogs();
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
keyword(&c, db, 1, "puffin");
}
let report = merge_keywords(&c).unwrap();
assert_eq!(report.keywords_assigned, 0);
assert_eq!(words_on(&c, 1), ["puffin"]);
}
#[test]
fn keywords_reach_an_image_the_server_names_but_no_one_has_hashed() {
// `content_hash` is computed only when import dedup or a reconnect asks
// for it, so for most images it is NULL — and a union keyed on it alone
// would quietly do nothing for the ordinary photograph. The file id is
// recorded by every remote scan, which is exactly the situation where
// two devices are keywording one library.
let c = two_catalogs();
c.execute(
"INSERT INTO main.roots(id, kind, label) VALUES (1, 'remote', 'r')",
[],
)
.unwrap();
c.execute(
"INSERT INTO remote_cat.roots(id, kind, label) VALUES (1, 'remote', 'r')",
[],
)
.unwrap();
// Different row ids for one photograph, and no hash on either side.
c.execute(
"INSERT INTO main.images(id, root_id, source_ref, added_at)
VALUES (77, 1, 'IMG_1.CR3', 0)",
[],
)
.unwrap();
c.execute(
"INSERT INTO remote_cat.images(id, root_id, source_ref, added_at)
VALUES (3, 1, 'IMG_1.CR3', 0)",
[],
)
.unwrap();
add_file_id(&c, "main", 77, 9001);
add_file_id(&c, "remote_cat", 3, 9001);
add_version(&c, "main", 77, "v-main");
keyword(&c, "remote_cat", 3, "puffin");
merge_keywords(&c).unwrap();
assert_eq!(words_on(&c, 77), ["puffin"]);
}
#[test]
fn keywords_map_across_devices_by_content_hash_where_there_is_no_server() {
// A local-only library has no `remote` rows at all, so the hash is the
// only identity available — and it is the one membership already uses.
let c = two_catalogs();
add_image(&c, "main", 77, "same-photo");
add_image(&c, "remote_cat", 3, "same-photo");
add_version(&c, "main", 77, "v-main");
keyword(&c, "remote_cat", 3, "puffin");
merge_keywords(&c).unwrap();
assert_eq!(words_on(&c, 77), ["puffin"]);
}
#[test]
fn the_vocabulary_merges_by_uuid_and_a_skewed_clock_cannot_win() {
let c = two_catalogs();
add_term(&c, "main", "u-1", "Iceland", 9, 0);
add_term(&c, "remote_cat", "u-1", "iceland", 2, 0);
add_term(&c, "remote_cat", "u-2", "puffin", 1, 0);
let report = merge_keywords(&c).unwrap();
assert_eq!(report.keywords_kept_local, 1);
assert_eq!(report.keywords_inserted, 1);
assert_eq!(live_terms(&c), ["Iceland", "puffin"]);
}
#[test]
fn a_remote_rename_moves_this_device_s_assignments_too() {
// The failure this exists to stop: the vocabulary shows the corrected
// spelling and the search still only finds the old one.
let c = two_catalogs();
add_image(&c, "main", 1, "hash-a");
add_term(&c, "main", "u-1", "Icland", 1, 0);
keyword(&c, "main", 1, "Icland");
add_term(&c, "remote_cat", "u-1", "Iceland", 4, 0);
let report = merge_keywords(&c).unwrap();
assert_eq!(report.keywords_updated, 1);
assert_eq!(live_terms(&c), ["Iceland"]);
assert_eq!(words_on(&c, 1), ["Iceland"]);
}
#[test]
fn a_remote_deletion_takes_the_word_off_every_photograph() {
let c = two_catalogs();
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
}
add_term(&c, "main", "u-1", "blurry", 1, 0);
keyword(&c, "main", 1, "blurry");
add_term(&c, "remote_cat", "u-1", "blurry", 5, 1);
let report = merge_keywords(&c).unwrap();
assert_eq!(report.keywords_deleted, 1);
assert!(live_terms(&c).is_empty());
assert!(words_on(&c, 1).is_empty());
}
#[test]
fn a_deletion_is_not_undone_by_the_union_on_the_same_pass() {
// The remote deleted the word *and* still carries assignments for it —
// it has not yet had the chance to sweep them, or a third device put
// them there. Without the tombstone filter the union would put the word
// straight back on the photograph the deletion had just cleared.
let c = two_catalogs();
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
}
add_term(&c, "main", "u-1", "blurry", 1, 0);
keyword(&c, "main", 1, "blurry");
add_term(&c, "remote_cat", "u-1", "blurry", 5, 1);
keyword(&c, "remote_cat", 1, "blurry");
merge_keywords(&c).unwrap();
assert!(words_on(&c, 1).is_empty(), "a deleted keyword came back");
}
#[test]
fn a_deletion_lands_even_when_the_two_devices_minted_different_uuids() {
// Both typed "blurry" before they ever synced, so this device's row has
// a uuid the remote has never heard of. Deleting by identity would
// tombstone nothing and leave every photograph still carrying the word.
let c = two_catalogs();
add_image(&c, "main", 1, "hash-a");
add_term(&c, "main", "mine", "blurry", 1, 0);
keyword(&c, "main", 1, "blurry");
add_term(&c, "remote_cat", "theirs", "blurry", 5, 1);
merge_keywords(&c).unwrap();
assert!(words_on(&c, 1).is_empty());
}
#[test]
fn two_devices_that_typed_one_word_end_up_with_one_keyword() {
// Neither is wrong until they meet, which is why the name carries no
// unique index — a constraint would abort the merge at this moment.
let c = two_catalogs();
add_term(&c, "main", "zzzz", "Iceland", 3, 0);
add_term(&c, "remote_cat", "aaaa", "Iceland", 1, 0);
let report = merge_keywords(&c).unwrap();
assert_eq!(report.keywords_fused, 1);
assert_eq!(live_terms(&c), ["Iceland"]);
let survivor: String = c
.query_row("SELECT uuid FROM main.keyword_terms", [], |r| r.get(0))
.unwrap();
assert_eq!(
survivor, "aaaa",
"both devices must pick the same survivor without asking each other"
);
}
#[test]
fn merging_keywords_twice_changes_nothing_the_second_time() {
let c = two_catalogs();
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
}
add_term(&c, "remote_cat", "u-1", "puffin", 1, 0);
keyword(&c, "remote_cat", 1, "puffin");
let first = merge_keywords(&c).unwrap();
assert!(first.local_changed());
let second = merge_keywords(&c).unwrap();
assert!(!second.local_changed(), "merge must be idempotent");
}
#[test]
fn a_remote_from_before_keyword_identities_still_contributes_its_words() {
// `remote_is_mergeable` admits an older remote on purpose — the check
// is that it is not *newer* than us. A missing table is therefore a
// normal state and must not fail the merge.
let c = two_catalogs_with_a_v1_remote();
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
}
keyword(&c, "remote_cat", 1, "puffin");
let report = merge_keywords(&c).unwrap();
assert_eq!(report.keywords_assigned, 1);
assert_eq!(words_on(&c, 1), ["puffin"]);
}
#[test]
fn merge_all_lands_both_halves() {
let c = two_catalogs();
add_collection(&c, "remote_cat", 1, "u-coll", "Portugal", 1);
for db in ["main", "remote_cat"] {
add_image(&c, db, 1, "hash-a");
}
keyword(&c, "remote_cat", 1, "puffin");
let report = merge_all(&c).unwrap();
assert_eq!(report.inserted, 1);
assert_eq!(report.keywords_assigned, 1);
}
#[test]
fn keeping_local_still_marks_the_catalog_for_upload() {
// We hold something the remote does not, so the remote is stale even
core/dr-catalog/src/rating.rs
+19 -6
View File
@@ -75,6 +75,24 @@ impl Judgement {
/// The UUID is per row and generated here — it is the merge identity across
/// devices (FR-NC-8), so two images must never share one.
pub fn ensure_default_versions(conn: &Connection) -> Result<usize, CatalogError> {
// One transaction for the batch. A backfill over a 24k-image library is
// 24k inserts, and per-statement commits would make it minutes rather
// than seconds.
let tx = conn.unchecked_transaction()?;
let n = ensure_default_versions_within(&tx)?;
tx.commit()?;
Ok(n)
}
/// [`ensure_default_versions`] without opening a transaction.
///
/// Separate because SQLite has no nested `BEGIN`: [`crate::merge`] needs the
/// invariant restored *inside* the merge transaction — an incoming keyword
/// lands on a default version, so an image without one would silently drop it —
/// and calling the public form there fails at runtime with "cannot start a
/// transaction within a transaction". The same split, for the same reason, as
/// `collections::add_within`.
pub fn ensure_default_versions_within(conn: &Connection) -> Result<usize, CatalogError> {
let ids: Vec<i64> = {
let mut stmt = conn.prepare(
"SELECT i.id FROM images i
@@ -89,12 +107,8 @@ pub fn ensure_default_versions(conn: &Connection) -> Result<usize, CatalogError>
return Ok(0);
}
// One transaction for the batch. A backfill over a 24k-image library is
// 24k inserts, and per-statement commits would make it minutes rather
// than seconds.
let tx = conn.unchecked_transaction()?;
{
let mut insert = tx.prepare(
let mut insert = conn.prepare(
"INSERT INTO versions(image_id, uuid, name, is_default, rating, flag)
VALUES (?1, ?2, ?3, 1, 0, 0)",
)?;
@@ -102,7 +116,6 @@ pub fn ensure_default_versions(conn: &Connection) -> Result<usize, CatalogError>
insert.execute(rusqlite::params![id, new_uuid(), DEFAULT_VERSION_NAME])?;
}
}
tx.commit()?;
Ok(ids.len())
}
core/dr-catalog/src/schema.rs
+183 -5
View File
@@ -15,7 +15,7 @@ use rusqlite::Connection;
use crate::error::CatalogError;
/// Schema version this build writes and understands.
pub const SCHEMA_VERSION: i64 = 5;
pub const SCHEMA_VERSION: i64 = 6;
/// Apply migrations up to [`SCHEMA_VERSION`].
///
@@ -66,6 +66,12 @@ pub fn migrate(conn: &Connection) -> Result<i64, CatalogError> {
tx.pragma_update(None, "user_version", 5)?;
tx.commit()?;
}
if from < 6 {
let tx = conn.unchecked_transaction()?;
tx.execute_batch(V6)?;
tx.pragma_update(None, "user_version", 6)?;
tx.commit()?;
}
Ok(from)
}
@@ -103,6 +109,20 @@ pub fn backfill(conn: &Connection) -> Result<Vec<(&'static str, usize)>, Catalog
out.push(("default_versions", n));
}
// v6: a vocabulary row for every word some image already carries.
//
// Three ways a catalog arrives holding assignments with no term behind
// them, and all three are normal rather than exceptional: a library
// keyworded by a build that predates this table, a catalog rebuilt from
// sidecars (which carry the word and not the identity), and an import from
// Lightroom or darktable (FR-CAT-14). Without this the words are
// searchable but absent from the vocabulary list, which reads as the
// keywords having been lost.
let n = crate::keywords::adopt_orphan_terms(conn)?;
if n > 0 {
out.push(("keyword_terms", n));
}
Ok(out)
}
@@ -134,15 +154,45 @@ pub fn configure(conn: &Connection) -> Result<(), CatalogError> {
/// 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}."))
rewrite_for_attached(V1, 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.
}
/// Every table this build knows about, rewritten to target an attached
/// database.
///
/// [`v1_for_attached`] is kept alongside this rather than replaced by it: a
/// remote catalog written by an older build genuinely has only the v1 tables,
/// and the merge has to keep working against one (see
/// [`crate::merge::merge_keywords`]). Building that case in a test needs a way
/// to say "v1 and no more".
///
/// Only the migrations that *create* objects appear here. V2 through V5 are
/// `ALTER TABLE ... ADD COLUMN`, and the columns they add are local index
/// state — shadowing, trashing, cache pinning — that a merge never reads
/// across the attachment.
pub fn for_attached(schema_name: &str) -> String {
format!(
"{}\n{}",
rewrite_for_attached(V1, schema_name),
rewrite_for_attached(V6, schema_name)
)
}
/// Qualify every object a `CREATE` statement names with `schema_name`.
///
/// The rewrite is textual and therefore only as good as the naming discipline
/// in the batches it is given: every `CREATE TABLE`/`CREATE INDEX` must name
/// its object unqualified, which they do.
fn rewrite_for_attached(sql: &str, schema_name: &str) -> String {
sql.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.
}
/// Mark each JPEG that sits beside a RAW of the same name.
@@ -226,6 +276,63 @@ fn stem_of(path: &str) -> &str {
}
}
const V6: &str = r#"
-- TRACES: FR-CAT-5 | FR-CAT-6 | FR-NC-9
-- Keywords gain an identity, so that renaming and deleting one can cross
-- between devices.
--
-- The v1 `keywords` table is the *assignment*: one row per (version, word),
-- and the word is stored as text. That stays exactly as it is, and this
-- migration adds nothing to it, for a reason that is easy to get backwards.
--
-- # Why assignments keep the text rather than pointing at a row here
--
-- The catalog is a rebuildable index (ARCH §6.12). What an image is keyworded
-- with is authoritative in the sidecar and in XMP `dc:subject` (FR-CAT-13),
-- and both of those carry a *string*. Rewriting the join to reference
-- `keyword_terms(id)` would mean a catalog rebuilt from sidecars had to invent
-- term rows before it could record a single assignment, and an integer that
-- means nothing on the other device would sit where the durable fact belongs.
-- It would also break `crate::query`, which matches `kw.keyword` directly and
-- must keep hitting `keywords_term` on a 50k library (FR-CAT-6).
--
-- So the text is the fact and this table is the *identity*: it exists to give
-- a rename and a deletion something a merge can key on, and to let a keyword
-- exist in the vocabulary before any photograph carries it.
CREATE TABLE keyword_terms (
id INTEGER PRIMARY KEY,
-- Device-independent identity, as `collections.uuid` is. The integer id is
-- local and collides across devices.
uuid TEXT NOT NULL UNIQUE,
-- The word itself, and the value written into every assignment row.
name TEXT NOT NULL,
created INTEGER NOT NULL,
-- Monotonic, bumped on every local edit. `crate::merge` compares these
-- rather than timestamps, so a clock-skewed device cannot silently win.
revision INTEGER NOT NULL DEFAULT 1,
modified INTEGER NOT NULL,
-- Tombstone, so a merge against a device that still holds the keyword does
-- not resurrect it.
deleted INTEGER NOT NULL DEFAULT 0
);
-- Deliberately **not** UNIQUE.
--
-- Two devices that each type "Iceland" create two rows with two uuids, and
-- both are correct until they meet. A unique constraint would abort the merge
-- transaction at exactly that moment — the ordinary case, not a corner one.
-- Uniqueness is instead reached by convergence: `crate::keywords::create`
-- resolves an existing name locally, and `crate::keywords::fuse_duplicates`
-- collapses a cross-device pair onto the lexicographically smaller uuid, which
-- both devices compute identically without talking to each other.
--
-- Partial on `deleted = 0` because every lookup here is a live one: the
-- vocabulary list, the resolve-by-name in `create`, and the fuse pass all
-- exclude tombstones, and including them would grow the index with every
-- keyword the library has ever had rather than with the ones it has.
CREATE INDEX keyword_terms_name ON keyword_terms(name) WHERE deleted = 0;
"#;
const V5: &str = r#"
-- TRACES: FR-NC-6a | FR-CAT-9 | NFR-RES-4
-- Offline availability: what is kept, why it is kept, and where it lives.
@@ -671,6 +778,77 @@ mod tests {
assert_eq!(bytes, 100, "the existing row is untouched");
}
#[test]
fn a_v5_catalog_keeps_its_keywords_and_gains_their_identities() {
// TRACES: FR-CAT-5
// The migration case that matters here: a library keyworded by an
// import or an older build already has assignment rows, and they must
// survive into the vocabulary rather than being left searchable but
// invisible.
let c = mem();
for step in [V1, V2, V3, V4, V5] {
c.execute_batch(step).unwrap();
}
c.pragma_update(None, "user_version", 5).unwrap();
c.execute(
"INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')",
[],
)
.unwrap();
c.execute(
"INSERT INTO images(id, root_id, source_ref, added_at) VALUES (7, 1, 'IMG_7.CR3', 0)",
[],
)
.unwrap();
c.execute(
"INSERT INTO versions(id, image_id, uuid, name, is_default)
VALUES (1, 7, 'v-7', 'Default', 1)",
[],
)
.unwrap();
c.execute(
"INSERT INTO keywords(version_id, keyword) VALUES (1, 'puffin')",
[],
)
.unwrap();
assert_eq!(migrate(&c).unwrap(), 5, "migrated from v5");
assert_eq!(backfilled(&c, "keyword_terms"), 1);
let name: String = c
.query_row("SELECT name FROM keyword_terms", [], |r| r.get(0))
.unwrap();
assert_eq!(name, "puffin");
// The assignment is untouched — it is the durable fact, and the term
// row is only its identity.
let n: i64 = c
.query_row("SELECT count(*) FROM keywords", [], |r| r.get(0))
.unwrap();
assert_eq!(n, 1);
// It runs on every open, so a second pass must find nothing to do.
assert_eq!(backfilled(&c, "keyword_terms"), 0);
}
#[test]
fn two_devices_may_both_hold_a_term_of_the_same_name() {
// Deliberately not a unique index. Two devices each typing "Iceland"
// is the ordinary case, and a constraint would abort the merge
// transaction at exactly the moment they first sync.
let c = mem();
migrate(&c).unwrap();
c.execute(
"INSERT INTO keyword_terms(uuid, name, created, revision, modified)
VALUES ('a', 'Iceland', 0, 1, 1), ('b', 'Iceland', 0, 1, 1)",
[],
)
.unwrap();
let n: i64 = c
.query_row("SELECT count(*) FROM keyword_terms", [], |r| r.get(0))
.unwrap();
assert_eq!(n, 2);
}
#[test]
fn stems_ignore_directories_containing_dots() {
assert_eq!(stem_of("2026.08/IMG_1.CR2"), "IMG_1");
core/dr-catalog/src/sync.rs
+6 -5
View File
@@ -16,10 +16,11 @@
//!
//! # 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.
//! Only the *user's judgements about their library* merge: collections, and the
//! keyword vocabulary with its assignments (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 those two 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
@@ -96,7 +97,7 @@ pub fn merge_remote(conn: &Connection, remote: &Path) -> Result<MergeReport, Cat
[remote.to_string_lossy().as_ref()],
)?;
let result = merge::merge_collections(conn);
let result = merge::merge_all(conn);
// Detach even if the merge failed, or the next attempt errors with
// "database remote_cat is already in use".
core/dr-gpu/tests/tone_curve.rs
+139
View File
@@ -0,0 +1,139 @@
//! TRACES: FR-DEV-3
//! The tone curve's four curves, on a device.
//!
//! `dr-pipeline` asserts that the right WGSL is generated and `dr-gpu`'s other
//! tests assert that a shader runs; neither notices a fragment that says
//! exactly what it should and does not compile, or one that compiles and puts
//! the red curve's uniforms into the blue slot. So this renders flat grey
//! through each curve and looks at what came out.
//!
//! Flat grey because it makes every assertion a comparison between the three
//! components of one pixel: a curve that is meant to be chromatic must move
//! them apart, and one that is meant to be tonal must not.
use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext};
use dr_pipeline::ops::curve::{self, Axis, Channel};
use dr_pipeline::EditGraph;
const SIZE: u32 = 8;
fn ctx() -> Option<GpuContext> {
pollster::block_on(GpuContext::new_headless()).ok()
}
/// The centre pixel's red, green and blue, after `graph` has run over flat
/// mid-grey.
fn rendered(ctx: &GpuContext, graph: &EditGraph) -> (u8, u8, u8) {
let data: Vec<u8> = (0..SIZE * SIZE).flat_map(|_| [128, 128, 128, 255]).collect();
let source = DemosaicedImage::from_rgba8(ctx, &data, SIZE, SIZE).expect("upload");
// Composed the way the display path composes it. A curve that generates
// invalid WGSL fails at `render` below, which is the point of running this
// on a device at all.
let shader = graph.compose();
let mut adjust = AdjustPass::new(ctx);
adjust.render(&source, &shader, SIZE, SIZE).expect("render");
let pixels = adjust.export_pixels().expect("readback").0;
let at = ((SIZE / 2 * SIZE + SIZE / 2) * 4) as usize;
(pixels[at], pixels[at + 1], pixels[at + 2])
}
/// A curve with its mid-point lifted — the simplest edit that is unmistakably
/// an edit.
fn lifted(channel: Channel) -> EditGraph {
let mut graph = EditGraph::default_chain();
graph.set_param(
curve::ID,
curve::coordinate(channel, 2, Axis::Y),
0.75,
);
graph
}
#[test]
fn the_master_curve_lifts_every_component_together() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let (r0, g0, b0) = rendered(&ctx, &EditGraph::default_chain());
let (r, g, b) = rendered(&ctx, &lifted(Channel::Master));
assert!(r > r0, "the master curve did not lift the image: {r} vs {r0}");
// Grey in, grey out: the master curve is applied as a ratio over
// luminance, so it changes tone and not hue. A tolerance of one code
// value, because the components travel through the ratio separately and
// the result is quantised to eight bits.
assert!(
r.abs_diff(g) <= 1 && g.abs_diff(b) <= 1,
"the master curve tinted a neutral pixel: {r},{g},{b}"
);
assert_eq!((g0, b0), (r0, r0), "the unedited image is neutral");
}
#[test]
fn a_channel_curve_lifts_only_its_own_component() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let (r0, g0, b0) = rendered(&ctx, &EditGraph::default_chain());
for (channel, name) in [
(Channel::Red, "red"),
(Channel::Green, "green"),
(Channel::Blue, "blue"),
] {
let (r, g, b) = rendered(&ctx, &lifted(channel));
// The component the curve names moves; the other two stay exactly
// where they were. This is what catches a fragment whose uniforms are
// wired to the wrong curve — it would still lift *something*.
let (moved, still) = match channel {
Channel::Red => (r > r0, g == g0 && b == b0),
Channel::Green => (g > g0, r == r0 && b == b0),
_ => (b > b0, r == r0 && g == g0),
};
assert!(moved, "the {name} curve changed nothing: {r},{g},{b}");
assert!(
still,
"the {name} curve moved a component that was not its own: \
{r},{g},{b} from {r0},{g0},{b0}"
);
}
}
#[test]
fn the_master_and_the_channels_compose_in_one_pass() {
// All four curves at once: the case where the generated fragment is
// longest, every helper is present, and forty uniforms are in the block.
// Mostly a compile check, which is why the assertion is only that the
// result is a colour and not the one we started with.
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::P1_Y, 0.15);
graph.set_param(curve::ID, curve::P3_Y, 0.85);
for (channel, y) in [
(Channel::Red, 0.55),
(Channel::Green, 0.5),
(Channel::Blue, 0.62),
] {
graph.set_param(curve::ID, curve::coordinate(channel, 2, Axis::Y), y);
}
let (r0, _, _) = rendered(&ctx, &EditGraph::default_chain());
let (r, g, b) = rendered(&ctx, &graph);
assert!(
(r, g, b) != (r0, r0, r0),
"four active curves left the image untouched"
);
// Red and blue were pushed apart from green, which is the chromatic half
// doing its work on top of the tonal one.
assert!(b > g, "blue was lifted above green: {r},{g},{b}");
}
core/dr-pipeline/ops/README.md
+4 -3
View File
@@ -209,9 +209,10 @@ They still belong in this directory, because the pipeline's **order** is the
one thing a reader comes here to learn, and an order written half in YAML and
half in Rust would be worse than either alone.
Currently hand-written: `tone_curve` (a curve widget over five interpolated
points), `colour_mixer` (thirty-six faceted parameters from twelve computed hue
bands). `vignetting` is hand-written too but is not in the develop chain — it
Currently hand-written: `tone_curve` (one widget over four curves of five
interpolated points — master, red, green, blue — each reaching the shader only
when it has been moved), `colour_mixer` (thirty-six faceted parameters from
twelve computed hue bands). `vignetting` is hand-written too but is not in the develop chain — it
carries lens-profile coefficients that are not parameters. `distortion` and
`aberration` are `Warp`s rather than operations: they rewrite coordinates
before sampling rather than transforming a colour after it.
core/dr-pipeline/ops/tone_curve.yaml
+18 -5
View File
@@ -16,13 +16,26 @@ attributes: [tone, colour]
rust: ToneCurve
why_rust: |
Five control points presented as one curve widget, with an interpolator and
a monotonicity guarantee behind it. Its neutral is a *relationship* between
parameters rather than a set of values — the identity diagonal — which is
not something the declarative `active:` rule can express, and its
`presentation()` spans parameters rather than describing one.
Four curves — master, red, green, blue — of five control points each,
presented as one widget, with an interpolator and a monotonicity guarantee
behind them. Its neutral is a *relationship* between parameters rather than
a set of values — the identity diagonal, on every channel — which is not
something the declarative `active:` rule can express; its `presentation()`
spans forty parameters rather than describing one; and its fragment is
*assembled* rather than written, because each curve reaches the shader only
when it has been moved off the diagonal. A declared node's `wgsl:` is one
fixed block of text, which is the right shape for nearly everything here and
the wrong one for a node whose cost has to follow what the photographer
actually touched.
placement: |
After the fixed-weight region controls, so the curve is the final word on
tone: a photographer reaches for it to fix what those controls could not
place exactly.
Within the node, the master curve runs before the per-channel ones. Both
orders are visibly different images and the reasons for this one are written
out in `src/ops/curve.rs`: the master is tonal and hue-preserving, the
per-channel curves are the chromatic grade over the tones it produced, and a
point placed on a channel curve should act on the tone the photographer can
see rather than on the one the master is about to move.
core/dr-pipeline/src/ops/curve.rs
+891 -165
View File
File diff suppressed because it is too large Load Diff
core/dr-pipeline/src/sidecar.rs
+85 -1
View File
@@ -1088,7 +1088,7 @@ impl std::error::Error for ParseError {}
mod tests {
use super::*;
use crate::framing;
use crate::ops::{exposure, saturation, white_balance};
use crate::ops::{curve, exposure, saturation, white_balance};
fn edited() -> EditGraph {
let mut g = EditGraph::default_chain();
@@ -1273,6 +1273,90 @@ mod tests {
assert_eq!(once, twice);
}
/// TRACES: FR-DEV-3 | FR-CAT-8
/// A file written before the tone curve had per-channel curves.
///
/// Spelled out as literal text rather than produced by `to_text`, because
/// the claim is about *those bytes*: a sidecar generated by this build
/// would agree with this build by construction, and would go on agreeing
/// with it through a rename that broke every file on disk.
#[test]
fn a_sidecar_from_before_the_channel_curves_still_names_the_master() {
let text = "drsc 1\n\n[version u1]\nname = Default\nrevision = 4\nmodified = 9\n\
tone_curve.p1_y = 0.15\ntone_curve.p3_y = 0.85\n";
let parsed = Sidecar::parse(text).expect("valid");
let mut g = EditGraph::default_chain();
parsed.default_version().expect("a version").apply(&mut g);
// The S-curve the file describes, on the master curve and nowhere
// else.
assert_eq!(g.param(curve::ID, curve::P1_Y), Some(0.15));
assert_eq!(g.param(curve::ID, curve::P3_Y), Some(0.85));
for channel in [curve::Channel::Red, curve::Channel::Green, curve::Channel::Blue] {
for point in 0..curve::POINTS {
for axis in [curve::Axis::X, curve::Axis::Y] {
let id = curve::coordinate(channel, point, axis);
let expected = g
.capabilities()
.iter()
.find(|c| c.id == curve::ID)
.and_then(|c| c.params.iter().find(|p| p.id == id))
.map(|p| p.default);
assert_eq!(
g.param(curve::ID, id),
expected,
"{id} moved, and no line in the file mentions it"
);
}
}
}
// And writing it back produces the same two lines: the curves the file
// never mentioned are still at their defaults, so they are still
// absent (`only_non_default_values_are_written`).
let written = Sidecar::parse(&Sidecar::parse(text).expect("valid").to_text())
.expect("valid")
.to_text();
assert!(written.contains("tone_curve.p1_y = 0.15"), "{written}");
assert!(written.contains("tone_curve.p3_y = 0.85"), "{written}");
assert!(
!written.contains("tone_curve.r_"),
"an untouched channel curve was written out:\n{written}"
);
}
/// TRACES: FR-DEV-3
/// The other direction: the new curves persist like any other parameter.
#[test]
fn a_per_channel_curve_survives_the_round_trip() {
let mut g = EditGraph::default_chain();
// A faded shadow: blue lifted at the black point, red pulled down.
let blue = curve::coordinate(curve::Channel::Blue, 0, curve::Axis::Y);
let red = curve::coordinate(curve::Channel::Red, 4, curve::Axis::Y);
g.set_param(curve::ID, blue, 0.08);
g.set_param(curve::ID, red, 0.92);
g.set_param(curve::ID, curve::P2_Y, 0.55);
let mut sidecar = Sidecar::new();
sidecar.put(version_of(&g));
let text = sidecar.to_text();
// Keyed by the channel-prefixed id, which is what makes the master's
// unprefixed ones safe to leave alone.
assert!(text.contains("tone_curve.b_p0_y = 0.08"), "{text}");
let parsed = Sidecar::parse(&text).expect("valid");
let mut restored = EditGraph::default_chain();
parsed
.default_version()
.expect("a version")
.apply(&mut restored);
assert_eq!(restored.param(curve::ID, blue), Some(0.08));
assert_eq!(restored.param(curve::ID, red), Some(0.92));
assert_eq!(restored.param(curve::ID, curve::P2_Y), Some(0.55));
assert!(!restored.is_neutral());
}
#[test]
fn an_unknown_operation_survives_a_round_trip() {
// The data-loss case that matters: a device running an older build
core/dr-pipeline/tests/tone_curve.rs
+202
View File
@@ -0,0 +1,202 @@
//! TRACES: FR-DEV-3
//! The tone curve's four curves, seen from outside the crate.
//!
//! The unit tests beside the operation assert what one `ToneCurve` does. These
//! assert the two properties that only show up once it is a node in a chain
//! with a sidecar under it: that a file written before the per-channel curves
//! existed still describes the edit it described, and that the three new
//! curves cost a photograph that does not use them precisely nothing.
use dr_pipeline::ops::curve::{self, Axis, Channel};
use dr_pipeline::{EditGraph, Sidecar};
/// A version block carrying `params`, in the on-disk spelling.
fn sidecar_with(params: &[(&str, f32)]) -> String {
let mut text = String::from("drsc 1\n\n[version u1]\nname = Default\nrevision = 2\nmodified = 0\n");
for (key, value) in params {
text.push_str(&format!("{key} = {value}\n"));
}
text
}
fn apply(text: &str) -> EditGraph {
let sidecar = Sidecar::parse(text).expect("a valid sidecar");
let mut graph = EditGraph::default_chain();
sidecar
.default_version()
.expect("a default version")
.apply(&mut graph);
graph
}
/// TRACES: FR-CAT-8
/// **The compatibility guarantee, end to end.**
///
/// An edit made before this build existed has to produce the same image now.
/// Asserted on the generated shader rather than on the parameter values,
/// because that is what the photograph is actually made of: same source, same
/// uniforms, same picture.
#[test]
fn an_edit_written_before_the_channel_curves_renders_as_it_did() {
let from_file = apply(&sidecar_with(&[
("tone_curve.p1_y", 0.15),
("tone_curve.p3_y", 0.85),
]));
let mut by_hand = EditGraph::default_chain();
by_hand.set_param(curve::ID, curve::P1_Y, 0.15);
by_hand.set_param(curve::ID, curve::P3_Y, 0.85);
let restored = from_file.compose();
let expected = by_hand.compose();
assert_eq!(restored.source, expected.source);
assert_eq!(restored.uniforms, expected.uniforms);
assert_eq!(restored.structure_hash, expected.structure_hash);
}
/// **Neutral means absent, and stays absent with four times as much to be
/// neutral about.**
///
/// The curve carries forty parameters now. An image edited with an S-curve and
/// nothing else must generate the shader it generated when it carried ten:
/// three untouched curves are not three identity evaluations, they are nothing
/// at all.
#[test]
fn three_untouched_curves_cost_nothing() {
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::P2_Y, 0.62);
let shader = graph.compose();
assert!(
shader.source.contains("---- tone_curve ----"),
"the master curve must reach the shader"
);
assert!(
!shader.source.contains("channel_curve"),
"an untouched channel curve reached the shader:\n{}",
shader.source
);
for prefix in ["r_", "g_", "b_"] {
assert!(
!shader.source.contains(&format!("tone_curve_{prefix}")),
"an untouched channel curve declared uniforms:\n{}",
shader.source
);
}
}
/// And with none of them touched, the operation is not there at all — the
/// property `a_fresh_graph_is_neutral` asserts for the chain, restated for the
/// node that just quadrupled in size.
#[test]
fn a_curve_at_its_defaults_is_absent_from_the_shader() {
let graph = EditGraph::default_chain();
assert!(graph.is_neutral());
assert!(!graph.compose().source.contains("tone_curve"));
// Including when every one of the forty parameters has been explicitly
// written to its own default, which is what a sidecar round trip through
// a build with a different idea of "default" would produce.
let mut written = EditGraph::default_chain();
for cap in written.capabilities() {
if cap.id != curve::ID {
continue;
}
for p in &cap.params {
written.set_param(curve::ID, p.id, p.default);
}
}
assert!(written.is_neutral());
}
/// A grade with no tonal work is a real edit, and it must not drag the
/// luminance path in behind it.
#[test]
fn a_channel_curve_reaches_the_shader_on_its_own() {
let mut graph = EditGraph::default_chain();
graph.set_param(
curve::ID,
curve::coordinate(Channel::Blue, 0, Axis::Y),
0.08,
);
let shader = graph.compose();
assert!(shader.source.contains("c.b = channel_curve(c.b,"));
assert!(
!shader.source.contains("apply_tone_gain"),
"the identity master curve reached the shader:\n{}",
shader.source
);
// Blue's ten points, and nothing else: the other two curves are at the
// identity and contribute no slot to the uniform block.
for prefix in ["r_", "g_"] {
assert!(
!shader.source.contains(&format!("tone_curve_{prefix}")),
"an untouched channel curve declared uniforms:\n{}",
shader.source
);
}
for point in 0..curve::POINTS {
assert!(
shader.source.contains(&format!("tone_curve_b_x{point}")),
"blue's point {point} is missing from the uniform block"
);
}
}
/// Each curve is its own shader, so the pipeline cache cannot hand the red
/// curve's compiled program to an edit that moved the green one.
#[test]
fn every_curve_generates_a_distinct_shader() {
let mut hashes: Vec<u64> = Vec::new();
for channel in Channel::ALL {
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::coordinate(channel, 2, Axis::Y), 0.62);
hashes.push(graph.compose().structure_hash);
}
let before = hashes.len();
hashes.sort_unstable();
hashes.dedup();
assert_eq!(before, hashes.len(), "two curves share a compiled shader");
}
/// The forty parameters all persist, and the file names each one once.
#[test]
fn every_point_of_every_curve_round_trips_through_a_sidecar() {
let mut graph = EditGraph::default_chain();
// A different y per coordinate, so a point wired to the wrong channel
// cannot pass by holding the value it was supposed to hold anyway.
// Thirty-secondths because they survive both the decimal the file is
// written in and the binary32 it is read back into exactly — this test is
// about which parameter a value lands in, and a rounding difference here
// would fail it for an unrelated reason.
let mut step = 1;
for channel in Channel::ALL {
for point in 0..curve::POINTS {
graph.set_param(
curve::ID,
curve::coordinate(channel, point, Axis::Y),
step as f32 / 32.0,
);
step += 1;
}
}
let mut sidecar = Sidecar::new();
sidecar.put(dr_pipeline::sidecar::Version::from_graph(
"u1", "Default", &graph,
));
let restored = apply(&sidecar.to_text());
for channel in Channel::ALL {
for point in 0..curve::POINTS {
let id = curve::coordinate(channel, point, Axis::Y);
assert_eq!(
restored.param(curve::ID, id),
graph.param(curve::ID, id),
"{id} did not survive the sidecar"
);
}
}
}