Remember where the photographer was
Opening the application was always a fresh arrival at the beginning of the library, whatever you had been doing when you closed it. What is written down is the view, the scope, the rating filter and the photograph on screen -- the open one in develop, the first visible one in the grid. Not just a scroll position: a position without the filter that produced it names a row of a list that no longer exists. Restoring them has an order for the same reason -- scope, then filter, then position, then the view -- because each step changes what an ordinal *means*. Addressed by remote path and collection UUID, never by an ordinal or a row id. `images.id` and `collections.id` are local to one catalog, and a grid ordinal is local to one ordering; a record naming either would land somewhere arbitrary on a second device and after any filter change on this one. Where the ordinal is needed, `library::ordinal_of_path` computes it through the grid's own `ORDER BY`, taken verbatim by a window function rather than spelled a second time as an inequality -- which is the mistake `grid_order_for` already warns about, and which a manually ordered collection would make unreadable. Every failure degrades rather than reports. A collection this device has not merged leaves the scope at the whole library; a photograph that has since been deleted falls back to when it was taken, which puts the grid in the right week; a torn file yields no place and the library opens at the top. Reopening develop is the one thing that requires an exact match, because a canvas on a path that no longer resolves is a filename over an empty frame. The record lives in `dr-types` beside `Settings` and the store lives here beside `SettingsStore`, for the reason `dr-types`' manifest gives: a JSON serialiser in `core/` would be paid for by every crate there. Two files and two lifetimes, though -- resetting preferences must not forget where you were. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -1069,6 +1069,26 @@ pub fn catalog_path(account: &Account) -> PathBuf {
|
||||
data_root().join(account.namespace()).join("catalog.sqlite")
|
||||
}
|
||||
|
||||
/// TRACES: FR-UI-8
|
||||
/// Where this library's last position is remembered.
|
||||
///
|
||||
/// Beside the catalog, under the same account namespace, for the reason
|
||||
/// `catalog_path` gives: a place belongs to one library, and two folders on one
|
||||
/// disk are two libraries with two positions.
|
||||
///
|
||||
/// **The name matches the file that travels.** The copy on the server is
|
||||
/// `place.json` under `.darkroom-derived/`, and the exchange between them is a
|
||||
/// straight newest-wins swap of the same bytes — so calling the local one
|
||||
/// anything else would be one more thing to keep in step for no gain.
|
||||
///
|
||||
/// In the data directory rather than the cache one. The consequence is milder
|
||||
/// here than for the sidecars `data_root` was moved for — losing a place costs
|
||||
/// a scroll, not a day of culling — but a file the system is free to delete is
|
||||
/// one that would rarely survive long enough to be read.
|
||||
pub fn place_path(account: &Account) -> PathBuf {
|
||||
data_root().join(account.namespace()).join("place.json")
|
||||
}
|
||||
|
||||
/// The directory every account's data hangs off.
|
||||
///
|
||||
/// **Not the cache directory, and on Android that distinction is the whole
|
||||
@@ -4230,6 +4250,89 @@ pub fn read_ids_span(
|
||||
Ok(ids)
|
||||
}
|
||||
|
||||
/// TRACES: FR-UI-8
|
||||
/// Where one photograph sits in the grid, by its remote path.
|
||||
///
|
||||
/// The inverse of [`read_ids_span`], and it exists for the same reason that one
|
||||
/// does: an ordinal only names a photograph relative to an ordering, so it has
|
||||
/// to be *computed* through the ordering the cells are drawn with rather than
|
||||
/// guessed at. A restored place that landed on a row read through a different
|
||||
/// `ORDER BY` would open the library at a photograph the user has never seen,
|
||||
/// which looks exactly like the position having been forgotten.
|
||||
///
|
||||
/// # Why a window function rather than a count
|
||||
///
|
||||
/// The obvious implementation is "count the rows that sort before this one",
|
||||
/// and it would need this file to spell the ordering out a second time — as an
|
||||
/// inequality, with its own handling of the `captured_at IS NULL` term and its
|
||||
/// own tie-break. [`grid_order_for`] already warns what a second spelling
|
||||
/// costs, and a manually ordered collection makes it worse: that ordering is a
|
||||
/// correlated subquery, and an inequality over it is not something anyone
|
||||
/// should have to read.
|
||||
///
|
||||
/// `row_number() OVER ({order})` takes the ordering *verbatim* from the same
|
||||
/// function the window read uses, so the two cannot disagree by construction.
|
||||
/// It is a full pass over the scope rather than an index seek, which is the
|
||||
/// cost of that guarantee — and it is paid once, at launch, against a query the
|
||||
/// grid runs several times per screenful of scrolling.
|
||||
///
|
||||
/// `Ok(None)` means the path is not in this grid: deleted, trashed, filtered
|
||||
/// out, or in a collection the place did not name. The caller falls back to
|
||||
/// when the photograph was taken, which is what makes a place survive its
|
||||
/// subject.
|
||||
///
|
||||
/// # Binding order
|
||||
///
|
||||
/// The window's parameters come first, unlike in [`read_ids_span`]. SQLite
|
||||
/// binds anonymous `?` by their position **in the SQL text**, and here the
|
||||
/// `OVER (...)` clause is in the select list — ahead of the `WHERE` the scope
|
||||
/// narrows. Swapping the two silently looks up a collection by an image id.
|
||||
pub fn ordinal_of_path(
|
||||
catalog: &Catalog,
|
||||
scope: Option<dr_types::CollectionId>,
|
||||
filter: &RatingFilter,
|
||||
trash: bool,
|
||||
path: &str,
|
||||
) -> Result<Option<usize>, dr_catalog::CatalogError> {
|
||||
let (inner, mut params) = if trash {
|
||||
(
|
||||
format!(
|
||||
"SELECT i.source_ref AS sref,
|
||||
row_number() OVER ({TRASH_ORDER}) - 1 AS ord
|
||||
FROM images i
|
||||
WHERE {TRASHED}"
|
||||
),
|
||||
Vec::new(),
|
||||
)
|
||||
} else {
|
||||
let (clause, scope_params) = scope_clause(catalog, scope)?;
|
||||
let rated = filter.sql();
|
||||
let folded = uncollapsed("i");
|
||||
let (order, mut params) = grid_order_for(catalog, scope);
|
||||
// The window first, then the scope — see the note above.
|
||||
params.extend(scope_params);
|
||||
(
|
||||
format!(
|
||||
"SELECT i.source_ref AS sref,
|
||||
row_number() OVER ({order}) - 1 AS ord
|
||||
FROM images i
|
||||
WHERE {VISIBLE}{rated}{folded}{clause}"
|
||||
),
|
||||
params,
|
||||
)
|
||||
};
|
||||
params.push(rusqlite::types::Value::Text(path.to_string()));
|
||||
|
||||
let mut stmt = catalog
|
||||
.connection()
|
||||
.prepare(&format!("SELECT ord FROM ({inner}) WHERE sref = ?"))?;
|
||||
let mut rows = stmt.query(rusqlite::params_from_iter(params.iter()))?;
|
||||
match rows.next()? {
|
||||
Some(r) => Ok(Some(r.get::<_, i64>(0)?.max(0) as usize)),
|
||||
None => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
/// The columns every windowed read selects, in the order [`row_to_cell`] reads
|
||||
/// them.
|
||||
///
|
||||
@@ -4830,6 +4933,177 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// The fixture the ordinal tests share: a mix of dated, undated, shadowed
|
||||
/// and trashed rows, which is what makes the grid's ordering non-obvious.
|
||||
fn a_small_library() -> Catalog {
|
||||
let catalog = Catalog::in_memory().unwrap();
|
||||
let c = catalog.connection();
|
||||
c.execute(
|
||||
"INSERT INTO roots(id, kind, label) VALUES (1, 'remote', 'lib')",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
for (id, name, captured, shadow, trashed) in [
|
||||
(1i64, "2019/a.CR2", Some(100i64), None, None),
|
||||
(2, "2019/b.CR2", Some(200), None, None),
|
||||
// Shadowed by its RAW sibling: never a row of the grid.
|
||||
(3, "2019/b.JPG", Some(200), Some(2i64), None),
|
||||
(4, "2020/c.CR2", Some(300), None, None),
|
||||
// Undated sorts last, whatever its name.
|
||||
(5, "2018/d.CR2", None, None, None),
|
||||
// Trashed: out of the library, and the only row of the trash.
|
||||
(6, "2020/e.CR2", Some(400), None, Some(9i64)),
|
||||
] {
|
||||
c.execute(
|
||||
"INSERT INTO images(id, root_id, source_ref, captured_at, shadowed_by,
|
||||
trashed_at, added_at)
|
||||
VALUES (?1, 1, ?2, ?3, ?4, ?5, 0)",
|
||||
rusqlite::params![id, name, captured, shadow, trashed],
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
catalog
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_cell_reports_the_ordinal_it_is_drawn_at() {
|
||||
// TRACES: FR-UI-8
|
||||
// The invariant the whole restore rests on, stated the strongest way
|
||||
// there is: walk the window the grid actually draws and ask for each
|
||||
// cell's ordinal by path. Anything less — a spot check, or a hand-built
|
||||
// expectation — would pass while `ordinal_of_path` and `read_cells`
|
||||
// disagreed about undated rows, shadowed rows or the tie-break, which
|
||||
// is precisely where an ordering drifts.
|
||||
let catalog = a_small_library();
|
||||
let filter = RatingFilter::default();
|
||||
let cells = read_cells(&catalog, 0, 100).unwrap();
|
||||
assert_eq!(cells.len(), 4, "shadowed and trashed are not rows");
|
||||
|
||||
for (i, cell) in cells.iter().enumerate() {
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &filter, false, &cell.remote_path).unwrap(),
|
||||
Some(i),
|
||||
"{} is drawn at row {i}",
|
||||
cell.remote_path
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_photograph_that_is_not_in_the_grid_reports_nothing() {
|
||||
// Deleted, never scanned, or hidden behind its RAW. All three are the
|
||||
// same answer, and the caller falls back to the capture time — which is
|
||||
// only reachable if this says `None` rather than guessing.
|
||||
let catalog = a_small_library();
|
||||
let filter = RatingFilter::default();
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &filter, false, "2019/gone.CR2").unwrap(),
|
||||
None,
|
||||
"never heard of it"
|
||||
);
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &filter, false, "2019/b.JPG").unwrap(),
|
||||
None,
|
||||
"shadowed by its RAW"
|
||||
);
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &filter, false, "2020/e.CR2").unwrap(),
|
||||
None,
|
||||
"trashed"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_trash_is_ordinalled_through_its_own_list() {
|
||||
// The trash orders by deletion time and lists exactly what the library
|
||||
// excludes, so an ordinal taken through the library's ordering would
|
||||
// name a different photograph — or, here, nothing at all.
|
||||
let catalog = a_small_library();
|
||||
let filter = RatingFilter::default();
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &filter, true, "2020/e.CR2").unwrap(),
|
||||
Some(0)
|
||||
);
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &filter, true, "2019/a.CR2").unwrap(),
|
||||
None,
|
||||
"a live photograph is not in the trash"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_filter_moves_the_ordinal_with_the_grid() {
|
||||
// TRACES: FR-UI-8
|
||||
// A place carries the filter it was recorded under *and* the position,
|
||||
// and the second is only meaningful under the first. Restoring them in
|
||||
// the wrong order — position, then filter — would land on a row of a
|
||||
// list that no longer exists, which is the bug this pairing exists to
|
||||
// rule out.
|
||||
let catalog = a_small_library();
|
||||
let c = catalog.connection();
|
||||
// Three stars on the third photograph only.
|
||||
c.execute(
|
||||
"INSERT INTO versions(id, image_id, uuid, name, is_default, rating)
|
||||
VALUES (1, 4, 'u4', 'default', 1, 3)",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let unfiltered = RatingFilter::default();
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &unfiltered, false, "2020/c.CR2").unwrap(),
|
||||
Some(2)
|
||||
);
|
||||
|
||||
let starred = RatingFilter {
|
||||
min_rating: 3,
|
||||
..RatingFilter::default()
|
||||
};
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &starred, false, "2020/c.CR2").unwrap(),
|
||||
Some(0),
|
||||
"the only survivor of the filter is the first row of it"
|
||||
);
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, None, &starred, false, "2019/a.CR2").unwrap(),
|
||||
None,
|
||||
"filtered out, so it has no position in this grid"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_collection_is_ordinalled_through_its_own_scope() {
|
||||
// The window's parameters are bound ahead of the scope's — see the note
|
||||
// on `ordinal_of_path`. Getting that order wrong looks up a collection
|
||||
// by an image id, which fails silently as "not in this grid".
|
||||
let catalog = a_small_library();
|
||||
let c = catalog.connection();
|
||||
let coll =
|
||||
dr_catalog::collections::create(c, "Trip", None, dr_catalog::CollectionKind::Manual)
|
||||
.unwrap();
|
||||
// The second and third photographs, out of order, so position matters.
|
||||
dr_catalog::collections::add_images(c, coll, &[dr_types::ImageId(4), dr_types::ImageId(2)])
|
||||
.unwrap();
|
||||
|
||||
let filter = RatingFilter::default();
|
||||
let cells = read_cells_scoped(&catalog, Some(coll), &filter, 0, 100).unwrap();
|
||||
assert_eq!(cells.len(), 2);
|
||||
for (i, cell) in cells.iter().enumerate() {
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, Some(coll), &filter, false, &cell.remote_path).unwrap(),
|
||||
Some(i),
|
||||
"{} is drawn at row {i} of the collection",
|
||||
cell.remote_path
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
ordinal_of_path(&catalog, Some(coll), &filter, false, "2019/a.CR2").unwrap(),
|
||||
None,
|
||||
"not a member"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_thumbnail_pass_asks_only_for_what_is_missing() {
|
||||
// The work list is the whole point of the pass being resumable and of
|
||||
|
||||
Reference in New Issue
Block a user