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

Keep originals on this device, by pin and by use
Build and test / Desktop (Linux) (push) Failing after 1s
Details
Build and test / Android (aarch64) (push) Failing after 0s
Details
Build and test / Layer separation (push) Failing after 1s
Details
Traceability / Requirement traces (push) Failing after 2s
Details

Browse Source 
Fills in `image_cache`, which the previous commit's "On this device" filter
read but nothing wrote. Also carries in-flight work that shared these files:
the Android TLS root store, the settings page, and a regenerated
traceability report.

# Two populations, deliberately separate

An original is kept here for one of two reasons, and conflating them produces
the exact failure the feature exists to prevent.

**Pinned** originals were asked for. Pinning a collection before a trip is a
promise, so pinned rows are never evicted and never counted against the
budget — a cap that could silently delete a pinned trip would make pinning
worthless, because it could not be relied on without checking.

**Passively cached** originals are a side effect of working: develop already
downloads the whole file, so keeping it costs no bandwidth and saves the
entire transfer next time. This population is what the budget bounds, evicted
least-recently-used, because it otherwise grows until a day of culling fills
a disk.

Sharing one budget would let a large pin starve the passive cache, or let
browsing evict a pin. They are separate.

# What was built

`dr_catalog::cache` owns the bookkeeping — held tier, size, last use, pinned
— and writes the bytes; deciding to download stays with the caller, which is
what keeps a crate with no network out of the network's business. Files are
written to a temporary and renamed, so a dropped connection cannot leave a
truncated file recorded as a complete original. They are named by image id,
not filename: `Photos/IMG_0001.CR2` and `Trips/IMG_0001.CR2` are different
photographs, and a flat cache keyed on the name would serve one for the other.

`spawn_full_fetch` became read-through. A hit is a disk read; a miss stores
what it downloads and enforces the budget. A cache that cannot be opened is a
miss, not a failure to open the photograph.

Pinning writes intent — `tier_desired` — without downloading, so the button
responds immediately, and `spawn_pin_fetch` fills it in sequentially
afterwards. Sequential because these are tens of megabytes each: the lanes
that make the thumbnail sweep fast buy little against one connection's
bandwidth and cost a great deal of memory. A pin interrupted by a lost
connection resumes from where it stopped.

Schema v5 adds `pinned` and `path`. `pinned` is a column rather than something
inferred from `pinned_by_rule`, which is ON DELETE SET NULL and so cannot
answer for an image whose rule was deleted. A v4 catalog migrates in place;
existing rows default to unpinned, the safe direction.

The budget and "keep opened originals" come from the settings page rather than
a constant, and are applied at startup rather than only on change — a cache
capped at 2 GB last session would otherwise spend this one filling to the
default. Turning off keeping leaves what is already cached readable: those
bytes are paid for, and refusing them would re-download images sitting right
there, including pinned ones.

Also removes a doubled `#[test]` introduced in the previous commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-11 21:12:01 +02:00
co-authored by Claude Opus 5
parent cd75e5a4c6
commit fa12afed18
22 changed files with 4032 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/settings_ui.rs
+562
View File
@@ -0,0 +1,562 @@
//! TRACES: FR-EXP-1 | FR-EXP-2 | FR-EXP-3 | FR-EXP-4 | FR-EXP-6 | FR-EXP-8 | FR-NC-6a
//! Wires [`Settings`] to the Slint settings page.
//!
//! Shaped after [`launch_ui`](crate::launch_ui): the record lives in
//! `dr-types` and is tested headless, [`render`] pushes it into window
//! properties, and [`wire`] connects the callbacks. This module moves values
//! across the boundary and decides nothing about what a setting *means*.
//!
//! # Every edit saves
//!
//! There is no Save button. A settings page with one has to answer what
//! happens when the window closes with the button untouched, and every
//! available answer is bad: discarding silently loses work, prompting turns a
//! preference change into a dialogue, and saving anyway makes the button a
//! decoration. Writing on each edit removes the question — the file is a few
//! hundred bytes, written by rename, and the user's last action is always what
//! is stored.
//!
//! A failed write is surfaced rather than swallowed, because the whole
//! contract of this page is that what it shows is what is saved. If the disk
//! is full or the config directory is unwritable, a page that kept displaying
//! the new value would be lying.
use std::cell::RefCell;
use std::rc::Rc;
use dr_types::settings::budget;
use dr_types::{
CollisionPolicy, ColourSpace, ExportFormat, OutputSharpening, Settings, SizingMode,
};
use slint::ComponentHandle;
use crate::settings_store::SettingsStore;
use crate::AppWindow;
/// Shared settings state for the running window.
pub struct SettingsController {
pub settings: RefCell<Settings>,
pub store: SettingsStore,
/// Reported by the page when a write fails. Held here rather than pushed
/// straight to the window so [`render`] stays the single writer of window
/// properties.
error: RefCell<Option<String>>,
/// How much disk the cache is currently using, as a label. Supplied by
/// whoever owns the catalog — this module has no connection to query.
usage_label: RefCell<String>,
}
impl SettingsController {
pub fn new() -> Rc<Self> {
let store = SettingsStore::open();
let settings = store.load();
Rc::new(Self {
settings: RefCell::new(settings),
store,
error: RefCell::new(None),
usage_label: RefCell::new(String::new()),
})
}
/// The current settings, for whoever needs to act on them.
pub fn snapshot(&self) -> Settings {
self.settings.borrow().clone()
}
/// Show what the cache is holding. Empty hides the line.
pub fn set_usage_label(&self, label: String) {
*self.usage_label.borrow_mut() = label;
}
/// Apply an edit and persist it.
///
/// Takes a closure rather than a whole `Settings` so a caller cannot
/// accidentally write back a stale copy of the fields it was not editing —
/// with a save on every keystroke, two controls holding their own snapshots
/// would overwrite each other.
fn edit(&self, f: impl FnOnce(&mut Settings)) {
{
let mut settings = self.settings.borrow_mut();
f(&mut settings);
// The page can produce out-of-range values — a typed quality, a
// pasted number — so the same clamp the file gets on read applies
// here, before anything is stored or shown.
settings.sanitise();
}
let snapshot = self.settings.borrow().clone();
match self.store.save(&snapshot) {
Ok(()) => *self.error.borrow_mut() = None,
Err(e) => {
log::warn!("saving settings: {e}");
*self.error.borrow_mut() = Some(format!(
"Could not save to {}: {e}",
self.store.path().display()
));
}
}
}
}
/// Push the settings into the window's properties.
pub fn render(window: &AppWindow, controller: &SettingsController) {
let s = controller.settings.borrow();
// --- cache ---------------------------------------------------------
window.set_settings_original_budget(budget::label(s.cache.original_budget_bytes).into());
window.set_settings_original_unlimited(s.cache.original_budget_bytes.is_none());
window.set_settings_thumbnail_budget(budget::label(s.cache.thumbnail_budget_bytes).into());
window.set_settings_thumbnail_unlimited(s.cache.thumbnail_budget_bytes.is_none());
window.set_settings_keep_opened(s.cache.keep_opened_originals);
window.set_settings_cache_usage(controller.usage_label.borrow().clone().into());
// --- export --------------------------------------------------------
//
// Choice rows are sent as labels plus the selected index rather than as a
// model of structs: the page draws a row of chips from them and nothing
// else, so a label and an index is the whole of what it needs.
window.set_settings_format_labels(labels(ExportFormat::ALL.iter().map(|f| f.label())));
window.set_settings_format_selected(index_of(&ExportFormat::ALL, &s.export.format));
window.set_settings_quality(s.export.quality as i32);
// Disabled rather than hidden for a lossless format: a control that
// vanishes when PNG is picked reads as a bug, where a greyed one explains
// itself.
window.set_settings_quality_enabled(s.export.format.is_lossy());
window.set_settings_colour_labels(labels(ColourSpace::ALL.iter().map(|c| c.label())));
window.set_settings_colour_selected(index_of(&ColourSpace::ALL, &s.export.colour_space));
window.set_settings_sizing_labels(labels(SizingMode::CHOICES.iter().map(|m| m.label())));
// Compared by variant, not by equality: `LongEdge(900)` after the user
// typed their own number is still the "Long edge" choice, and equality
// against `CHOICES` would light nothing.
window.set_settings_sizing_selected(
SizingMode::CHOICES
.iter()
.position(|m| m.same_mode(s.export.sizing))
.unwrap_or(0) as i32,
);
window.set_settings_sizing_value(s.export.sizing.value().unwrap_or(0) as i32);
// `Original` carries no number, so the field beside the chips has nothing
// to edit and is hidden rather than shown holding a meaningless zero.
window.set_settings_sizing_has_value(s.export.sizing.value().is_some());
window.set_settings_sizing_unit(
match s.export.sizing {
SizingMode::Percentage(_) => "%",
_ => "px",
}
.into(),
);
window.set_settings_allow_upscaling(s.export.allow_upscaling);
window.set_settings_sharpening_labels(labels(OutputSharpening::ALL.iter().map(|x| x.label())));
window.set_settings_sharpening_selected(index_of(
&OutputSharpening::ALL,
&s.export.sharpening,
));
window.set_settings_filename_template(s.export.filename_template.clone().into());
window.set_settings_collision_labels(labels(CollisionPolicy::ALL.iter().map(|c| c.label())));
window.set_settings_collision_selected(index_of(&CollisionPolicy::ALL, &s.export.collision));
window.set_settings_strip_location(s.export.strip_location);
window.set_settings_destination(s.export.destination.clone().into());
window.set_settings_error(
controller
.error
.borrow()
.clone()
.unwrap_or_default()
.into(),
);
}
/// A label list as a Slint model.
fn labels<'a>(items: impl Iterator<Item = &'a str>) -> slint::ModelRc<slint::SharedString> {
let v: Vec<slint::SharedString> = items.map(slint::SharedString::from).collect();
slint::ModelRc::new(slint::VecModel::from(v))
}
/// Where `value` sits in `all`, as the index the page selects by.
fn index_of<T: PartialEq>(all: &[T], value: &T) -> i32 {
all.iter().position(|v| v == value).unwrap_or(0) as i32
}
/// Connect the page's callbacks.
///
/// `on_budget_changed` runs when a cache ceiling moves, so the caller can
/// enforce it against the catalog — this module has no connection and must not
/// grow one.
pub fn wire<F>(window: &AppWindow, controller: Rc<SettingsController>, on_budget_changed: F)
where
F: Fn(&Settings) + 'static,
{
let on_budget_changed = Rc::new(on_budget_changed);
// --- opening and closing -------------------------------------------
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_open(move || {
let Some(w) = weak.upgrade() else { return };
// Re-read from disk on open rather than trusting the copy in
// memory: another instance of the app may have written the file
// since, and showing a stale value would let this window save it
// back over the newer one.
*ctl.settings.borrow_mut() = ctl.store.load();
render(&w, &ctl);
w.set_show_settings(true);
});
}
{
let weak = window.as_weak();
window.on_settings_close(move || {
let Some(w) = weak.upgrade() else { return };
w.set_show_settings(false);
});
}
// --- cache ---------------------------------------------------------
//
// A budget arrives as typed text. An unparseable entry leaves the previous
// value in place and `render` puts the stored one back in the field, so a
// typo is visibly rejected rather than silently shrinking a cache.
{
let weak = window.as_weak();
let ctl = controller.clone();
let notify = on_budget_changed.clone();
window.on_settings_original_budget_changed(move |text| {
let Some(w) = weak.upgrade() else { return };
if let Some(bytes) = budget::from_gb(&text) {
ctl.edit(|s| s.cache.original_budget_bytes = Some(bytes));
notify(&ctl.snapshot());
} else {
log::debug!("ignoring an unusable cache budget: {text:?}");
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
let notify = on_budget_changed.clone();
window.on_settings_original_unlimited_toggled(move |unlimited| {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| {
s.cache.original_budget_bytes = if unlimited {
None
} else {
// Back to the default rather than to whatever it was
// before: the previous figure is not kept while unlimited
// is on, and inventing one would be a guess. The default is
// at least a documented number.
Some(dr_types::settings::DEFAULT_ORIGINAL_BUDGET_BYTES)
};
});
notify(&ctl.snapshot());
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
let notify = on_budget_changed.clone();
window.on_settings_thumbnail_budget_changed(move |text| {
let Some(w) = weak.upgrade() else { return };
if let Some(bytes) = budget::from_gb(&text) {
ctl.edit(|s| s.cache.thumbnail_budget_bytes = Some(bytes));
notify(&ctl.snapshot());
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
let notify = on_budget_changed.clone();
window.on_settings_thumbnail_unlimited_toggled(move |unlimited| {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| {
s.cache.thumbnail_budget_bytes = if unlimited {
None
} else {
Some(dr_types::settings::DEFAULT_THUMBNAIL_BUDGET_BYTES)
};
});
notify(&ctl.snapshot());
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_keep_opened_toggled(move |on| {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| s.cache.keep_opened_originals = on);
render(&w, &ctl);
});
}
// --- export --------------------------------------------------------
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_format_picked(move |i| {
let Some(w) = weak.upgrade() else { return };
if let Some(f) = ExportFormat::ALL.get(i as usize).copied() {
ctl.edit(|s| s.export.format = f);
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_quality_changed(move |q| {
let Some(w) = weak.upgrade() else { return };
// Cast before clamping: a negative from the control would wrap to a
// large `u8` and land on 100 instead of the floor.
ctl.edit(|s| s.export.quality = q.clamp(1, 100) as u8);
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_colour_picked(move |i| {
let Some(w) = weak.upgrade() else { return };
if let Some(c) = ColourSpace::ALL.get(i as usize).copied() {
ctl.edit(|s| s.export.colour_space = c);
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_sizing_picked(move |i| {
let Some(w) = weak.upgrade() else { return };
if let Some(mode) = SizingMode::CHOICES.get(i as usize).copied() {
// Keeps the number the user already typed when they move
// between two sized modes: switching long edge to short edge
// at 900px means 900 on the other axis, not back to 1600.
ctl.edit(|s| {
s.export.sizing = match s.export.sizing.value() {
Some(v) => mode.with_value(v),
None => mode,
};
});
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_sizing_value_changed(move |text| {
let Some(w) = weak.upgrade() else { return };
match text.trim().parse::<u32>() {
Ok(v) if v > 0 => ctl.edit(|s| s.export.sizing = s.export.sizing.with_value(v)),
_ => log::debug!("ignoring an unusable export size: {text:?}"),
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_upscaling_toggled(move |on| {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| s.export.allow_upscaling = on);
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_sharpening_picked(move |i| {
let Some(w) = weak.upgrade() else { return };
if let Some(x) = OutputSharpening::ALL.get(i as usize).copied() {
ctl.edit(|s| s.export.sharpening = x);
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_template_changed(move |text| {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| s.export.filename_template = text.to_string());
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_collision_picked(move |i| {
let Some(w) = weak.upgrade() else { return };
if let Some(c) = CollisionPolicy::ALL.get(i as usize).copied() {
ctl.edit(|s| s.export.collision = c);
}
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_strip_location_toggled(move |on| {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| s.export.strip_location = on);
render(&w, &ctl);
});
}
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_destination_changed(move |text| {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| s.export.destination = text.to_string());
render(&w, &ctl);
});
}
// --- reset ---------------------------------------------------------
{
let weak = window.as_weak();
let ctl = controller.clone();
let notify = on_budget_changed.clone();
window.on_settings_reset(move || {
let Some(w) = weak.upgrade() else { return };
ctl.edit(|s| *s = Settings::default());
notify(&ctl.snapshot());
render(&w, &ctl);
});
}
}
#[cfg(test)]
mod tests {
use super::*;
/// A controller writing to a scratch file, with no window attached.
///
/// The edit-and-persist path is what is worth testing here and it needs no
/// display server; `render` and `wire` are the parts that need a window,
/// and they only move values.
fn controller(name: &str) -> Rc<SettingsController> {
let dir = std::env::temp_dir().join(format!(
"dr-settings-ui-{name}-{}-{:?}",
std::process::id(),
std::thread::current().id()
));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let store = SettingsStore::open_at(dir.join("settings.json"));
Rc::new(SettingsController {
settings: RefCell::new(store.load()),
store,
error: RefCell::new(None),
usage_label: RefCell::new(String::new()),
})
}
#[test]
fn an_edit_reaches_the_file_without_a_save_button() {
let ctl = controller("autosave");
ctl.edit(|s| s.export.quality = 60);
assert_eq!(ctl.store.load().export.quality, 60);
}
#[test]
fn an_edit_is_sanitised_before_it_is_stored() {
let ctl = controller("sanitise");
ctl.edit(|s| s.export.quality = 200);
assert_eq!(ctl.snapshot().export.quality, 100);
assert_eq!(ctl.store.load().export.quality, 100);
}
#[test]
fn two_edits_do_not_overwrite_each_other() {
// The reason `edit` takes a closure: a caller holding a whole
// `Settings` would write back its own stale copy of every other field.
let ctl = controller("independent");
ctl.edit(|s| s.export.quality = 70);
ctl.edit(|s| s.export.format = ExportFormat::Png);
let stored = ctl.store.load();
assert_eq!(stored.export.quality, 70);
assert_eq!(stored.export.format, ExportFormat::Png);
}
#[test]
fn a_reset_restores_the_defaults_and_persists_them() {
let ctl = controller("reset");
ctl.edit(|s| {
s.export.quality = 10;
s.cache.original_budget_bytes = None;
});
ctl.edit(|s| *s = Settings::default());
assert_eq!(ctl.store.load(), Settings::default());
}
#[test]
fn switching_between_sized_modes_keeps_the_typed_number() {
let ctl = controller("sizing");
ctl.edit(|s| s.export.sizing = SizingMode::LongEdge(900));
// What `on_settings_sizing_picked` does for a sized target.
ctl.edit(|s| {
s.export.sizing = match s.export.sizing.value() {
Some(v) => SizingMode::ShortEdge(0).with_value(v),
None => SizingMode::ShortEdge(1600),
}
});
assert_eq!(ctl.snapshot().export.sizing, SizingMode::ShortEdge(900));
}
#[test]
fn a_write_failure_is_recorded_rather_than_swallowed() {
// The page's contract is that what it shows is saved, so a failed
// write has to be visible.
let ctl = controller("unwritable");
// A path whose parent is a *file* cannot be created as a directory.
let blocker = ctl.store.path().with_file_name("blocker");
std::fs::write(&blocker, b"not a directory").unwrap();
let broken = SettingsController {
settings: RefCell::new(Settings::default()),
store: SettingsStore::open_at(blocker.join("settings.json")),
error: RefCell::new(None),
usage_label: RefCell::new(String::new()),
};
broken.edit(|s| s.export.quality = 50);
assert!(broken.error.borrow().is_some(), "the failure was silent");
}
#[test]
fn a_successful_write_clears_an_earlier_error() {
let ctl = controller("clears");
*ctl.error.borrow_mut() = Some("stale".to_string());
ctl.edit(|s| s.export.quality = 80);
assert_eq!(*ctl.error.borrow(), None);
}
}