Ship presets with the application instead of seeding them
The six starter presets were copied into the photographer's own library on a first run and were theirs from then on. That cannot grow into a real collection: a copy is frozen at the release that wrote it, so an improved preset reaches nobody who had the old one, and re-seeding would overwrite a preset someone had tuned. `dr_pipeline::bundled` now holds the shipped presets as `.drpl` files compiled into the binary, in sections — Essentials (the former six) and three sections of film presets, one per measured stock in dr-film, printed on the paper its profile names — and never writes them to the user's file. Every shipped preset is a look (`Reach::Named`), so applying one keeps the corrections a photograph already has. A name links a photographer's copy to a shipped preset. Saving over a shipped name makes their version the one that name applies; it is listed in the shipped section, marked as changed, and deleting it reverts to the shipped one. Renaming it makes it one of their own and the shipped preset reappears. Keyed on the name because that is what the photographer sees and chooses by. Copies an older first run seeded are forgotten on load where they are still exactly as seeded — otherwise all six would list as changed and stay frozen at their old values. A tuned one is kept and now overrides. The sheet lists "Yours" first, then each shipped section, with headings. Shipped rows apply and nothing else; a changed row offers Revert where the photographer's own offer Delete. A dr-ui test checks every shipped film names a stock this build can bake, on that stock's own paper, because dr-pipeline does not link the profile database. The film presets name stocks by id; the measurements behind them are spektrafilm's (CC BY-SA 4.0), attributed in each file as in dr-film.
This commit is contained in:
+142
-80
@@ -739,26 +739,19 @@ impl NamedPresets {
|
||||
|
||||
fn at(store: PresetStore) -> Rc<Self> {
|
||||
// TRACES: FR-DEV-6
|
||||
// Seed the starter set on a device that has never had a library, and
|
||||
// only then. See `preset_starter` for why this is not a merge:
|
||||
// re-adding on every start would resurrect one deleted on purpose.
|
||||
//
|
||||
// "Never had one" is the *file* not existing rather than the library
|
||||
// being empty — a photographer who deleted all six must not be handed
|
||||
// them again on the next launch.
|
||||
let seeding = !store.path().exists();
|
||||
let library = if seeding {
|
||||
let starters = dr_pipeline::starter::library();
|
||||
if let Err(e) = store.save(&starters) {
|
||||
// Not fatal. The presets are in memory and usable this
|
||||
// session; what is lost is their persistence, and refusing to
|
||||
// start over it would be absurd.
|
||||
log::warn!("could not write the starter presets: {e}");
|
||||
}
|
||||
starters
|
||||
} else {
|
||||
store.load()
|
||||
};
|
||||
// Nothing is seeded any more: the shipped presets are read from the
|
||||
// binary every time (`dr_pipeline::bundled`). What a first run of an
|
||||
// older build copied into this file is forgotten here where it is
|
||||
// still exactly as copied — otherwise all six would list as changed
|
||||
// and stay frozen at their old values. In memory only; the next save
|
||||
// writes the file without them, and until then nothing is lost,
|
||||
// because each one is identical to the preset now shipped in its
|
||||
// place.
|
||||
let mut library = store.load();
|
||||
let forgotten = dr_pipeline::bundled::forget_unchanged_copies(&mut library);
|
||||
if forgotten > 0 {
|
||||
log::info!("{forgotten} seeded preset copies are now shipped presets");
|
||||
}
|
||||
Rc::new(Self {
|
||||
store,
|
||||
library: RefCell::new(library),
|
||||
@@ -808,14 +801,45 @@ impl NamedPresets {
|
||||
message
|
||||
}
|
||||
|
||||
/// The stored names, in the order they are written.
|
||||
/// The photographer's own stored names, in the order they are written.
|
||||
#[cfg(test)]
|
||||
pub fn names(&self) -> Vec<String> {
|
||||
self.library.borrow().names().map(String::from).collect()
|
||||
}
|
||||
|
||||
/// The preset stored under `name`.
|
||||
/// The preset `name` applies: the photographer's if they have one under
|
||||
/// that name, the shipped one otherwise.
|
||||
pub fn get(&self, name: &str) -> Option<Preset> {
|
||||
self.library.borrow().get(name).cloned()
|
||||
dr_pipeline::bundled::lookup(&self.library.borrow(), name)
|
||||
}
|
||||
|
||||
/// Every row the sheet lists, headings included.
|
||||
///
|
||||
/// The photographer's own section is left out while it is empty: a
|
||||
/// heading over nothing reads as a list that failed to load.
|
||||
pub fn rows(&self) -> Vec<crate::PresetRow> {
|
||||
use dr_pipeline::bundled::Origin;
|
||||
let mut rows = Vec::new();
|
||||
for section in dr_pipeline::bundled::listing(&self.library.borrow()) {
|
||||
if section.rows.is_empty() {
|
||||
continue;
|
||||
}
|
||||
rows.push(crate::PresetRow {
|
||||
heading: section.title.into(),
|
||||
name: Default::default(),
|
||||
origin: crate::PresetOrigin::Shipped,
|
||||
});
|
||||
rows.extend(section.rows.into_iter().map(|row| crate::PresetRow {
|
||||
heading: Default::default(),
|
||||
name: row.name.into(),
|
||||
origin: match row.origin {
|
||||
Origin::Yours => crate::PresetOrigin::Yours,
|
||||
Origin::Shipped => crate::PresetOrigin::Shipped,
|
||||
Origin::Changed => crate::PresetOrigin::Changed,
|
||||
},
|
||||
}));
|
||||
}
|
||||
rows
|
||||
}
|
||||
|
||||
/// Whether a preset is stored under `name`.
|
||||
@@ -898,10 +922,9 @@ impl SaveError {
|
||||
}
|
||||
}
|
||||
|
||||
/// Push the stored names onto the window.
|
||||
/// Push the listing onto the window.
|
||||
pub fn render_named(window: &AppWindow, named: &Rc<NamedPresets>) {
|
||||
let names: Vec<slint::SharedString> = named.names().into_iter().map(Into::into).collect();
|
||||
window.set_preset_names(slint::ModelRc::new(slint::VecModel::from(names)));
|
||||
window.set_preset_rows(slint::ModelRc::new(slint::VecModel::from(named.rows())));
|
||||
}
|
||||
|
||||
/// Wire saving, applying, renaming and deleting named presets.
|
||||
@@ -1452,69 +1475,112 @@ mod tests {
|
||||
// Named presets
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/// A library that has *already been started*, so the starter presets are
|
||||
/// not in the way.
|
||||
///
|
||||
/// Writing an empty library first is what makes the file exist, which is
|
||||
/// the condition seeding tests — see `NamedPresets::at`. The seeding
|
||||
/// itself has its own tests below rather than being switched off with a
|
||||
/// flag that only tests would set.
|
||||
/// A library in a fresh directory of its own.
|
||||
fn named(name: &str) -> (Rc<NamedPresets>, PathBuf) {
|
||||
let dir = tempdir(name);
|
||||
let path = dir.join("presets.drpl");
|
||||
PresetStore::open_at(path.clone())
|
||||
.save(&PresetLibrary::default())
|
||||
.unwrap();
|
||||
(NamedPresets::open_at(path), dir)
|
||||
}
|
||||
|
||||
/// A device that has never had a preset library.
|
||||
fn fresh(name: &str) -> (Rc<NamedPresets>, PathBuf) {
|
||||
let dir = tempdir(name);
|
||||
(NamedPresets::open_at(dir.join("presets.drpl")), dir)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_first_run_is_handed_the_starter_presets() {
|
||||
// A sheet that opens on "No presets yet" teaches the photographer that
|
||||
// the feature is homework.
|
||||
let (presets, _dir) = fresh("first-run");
|
||||
assert_eq!(presets.names().len(), dr_pipeline::starter::library().len());
|
||||
assert!(!presets.names().is_empty());
|
||||
/// The rows under the heading `title`, as (name, origin).
|
||||
fn section(presets: &NamedPresets, title: &str) -> Vec<(String, crate::PresetOrigin)> {
|
||||
presets
|
||||
.rows()
|
||||
.into_iter()
|
||||
.skip_while(|r| r.heading != title)
|
||||
.skip(1)
|
||||
.take_while(|r| r.heading.is_empty())
|
||||
.map(|r| (r.name.to_string(), r.origin))
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_starters_are_written_out_so_they_survive_a_restart() {
|
||||
let (_presets, dir) = fresh("starters-persist");
|
||||
let path = dir.join("presets.drpl");
|
||||
assert!(path.exists(), "the starter library was never written");
|
||||
assert!(!PresetStore::open_at(path).load().is_empty());
|
||||
fn a_first_run_lists_the_shipped_presets_and_writes_nothing() {
|
||||
// A sheet that opens on "No presets yet" teaches the photographer
|
||||
// that the feature is homework — and a copy written into their file
|
||||
// would be frozen at this release's values.
|
||||
let (presets, dir) = named("first-run");
|
||||
assert!(presets.names().is_empty());
|
||||
assert!(!section(&presets, "Essentials").is_empty());
|
||||
assert!(presets.get("Punch").is_some());
|
||||
assert!(!dir.join("presets.drpl").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_starter_the_photographer_deleted_stays_deleted() {
|
||||
// The reason seeding keys on the file existing rather than on the
|
||||
// library being empty: deleting all six must not hand them back on the
|
||||
// next launch.
|
||||
let dir = tempdir("deleted-starters");
|
||||
let path = dir.join("presets.drpl");
|
||||
{
|
||||
let presets = NamedPresets::open_at(path.clone());
|
||||
for name in presets.names() {
|
||||
let mut library = presets.library.borrow_mut();
|
||||
library.remove(&name);
|
||||
}
|
||||
presets.persist(|_| {}).unwrap();
|
||||
}
|
||||
|
||||
let again = NamedPresets::open_at(path);
|
||||
assert!(
|
||||
again.names().is_empty(),
|
||||
"the starters came back: {:?}",
|
||||
again.names()
|
||||
fn an_empty_section_of_your_own_is_not_listed() {
|
||||
let (presets, _dir) = named("no-yours");
|
||||
assert!(presets.rows().iter().all(|r| r.heading != "Yours"));
|
||||
presets.insert("Mine", Preset::default()).unwrap();
|
||||
assert_eq!(
|
||||
section(&presets, "Yours"),
|
||||
vec![("Mine".to_string(), crate::PresetOrigin::Yours)]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_seeded_copies_of_an_older_build_become_the_shipped_presets() {
|
||||
// A library as an older first run wrote it: the six, verbatim.
|
||||
let dir = tempdir("seeded-copies");
|
||||
let path = dir.join("presets.drpl");
|
||||
let essentials = dr_pipeline::bundled::sections()
|
||||
.into_iter()
|
||||
.find(|s| s.id == "essentials")
|
||||
.unwrap()
|
||||
.presets;
|
||||
let mut seeded = PresetLibrary::default();
|
||||
for (name, preset) in essentials.iter() {
|
||||
seeded
|
||||
.insert(name, preset.clone().with_reach(dr_pipeline::Reach::Whole))
|
||||
.unwrap();
|
||||
}
|
||||
PresetStore::open_at(path.clone()).save(&seeded).unwrap();
|
||||
|
||||
let presets = NamedPresets::open_at(path);
|
||||
assert!(presets.names().is_empty(), "{:?}", presets.names());
|
||||
assert!(section(&presets, "Essentials")
|
||||
.iter()
|
||||
.all(|(_, origin)| *origin == crate::PresetOrigin::Shipped));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saving_over_a_shipped_name_changes_it_and_deleting_reverts() {
|
||||
let (presets, _dir) = named("override");
|
||||
let shipped = presets.get("Punch").unwrap();
|
||||
|
||||
presets.insert("Punch", Preset::capture(&edited())).unwrap();
|
||||
assert_eq!(presets.get("Punch"), Some(Preset::capture(&edited())));
|
||||
assert!(section(&presets, "Essentials")
|
||||
.contains(&("Punch".to_string(), crate::PresetOrigin::Changed)));
|
||||
|
||||
presets.library.borrow_mut().remove("Punch");
|
||||
assert_eq!(presets.get("Punch"), Some(shipped));
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-6 | FR-DEV-3f
|
||||
/// Every stock a shipped preset names is one this build can bake, on the
|
||||
/// paper the preset says.
|
||||
///
|
||||
/// Here rather than beside the presets: `dr-pipeline` does not link the
|
||||
/// profile database, and a misspelt stock there would ship a preset that
|
||||
/// logs a warning and develops without a film. The paper is checked too,
|
||||
/// because the session prints on a stock's *own* paper and reads only
|
||||
/// whether one was asked for — a preset naming another would say one
|
||||
/// thing and render another.
|
||||
#[test]
|
||||
fn every_shipped_film_is_a_stock_this_build_can_bake() {
|
||||
for section in dr_pipeline::bundled::sections() {
|
||||
for (name, preset) in section.presets.iter() {
|
||||
let Some(film) = preset.film() else { continue };
|
||||
let profile = dr_film::find(&film.stock)
|
||||
.unwrap_or_else(|| panic!("{name}: no stock {:?}", film.stock));
|
||||
if let Some(print) = &film.print {
|
||||
let paper = dr_film::default_print(profile)
|
||||
.unwrap_or_else(|| panic!("{name}: {} has no paper", film.stock));
|
||||
assert_eq!(&paper.stock, print, "{name}");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn importing_a_folder_stores_what_it_finds() {
|
||||
let (presets, dir) = named("import-folder");
|
||||
@@ -1613,11 +1679,7 @@ mod tests {
|
||||
|
||||
let presets = NamedPresets::open_at(blocked.join("presets.drpl"));
|
||||
assert!(presets.insert("Warm", Preset::default()).is_err());
|
||||
// Not "the library is empty": the starter set is held in memory even
|
||||
// on a device whose config directory cannot be written, which is the
|
||||
// right behaviour — the presets are usable this session and only their
|
||||
// persistence is lost. What must not survive is the preset whose write
|
||||
// failed.
|
||||
// What must not survive is the preset whose write failed.
|
||||
assert!(
|
||||
!presets.contains("Warm"),
|
||||
"the failed save left a preset behind: {:?}",
|
||||
|
||||
@@ -14,7 +14,7 @@ import { CollectionsPanel, CollectionRow, Collections, OfflinePrompt, Collection
|
||||
MembershipSheet, MembershipRow } from "collections.slint";
|
||||
import { HistogramPanel, HistogramView, Levels } from "histogram.slint";
|
||||
import { RecoveryPrompt } from "recovery.slint";
|
||||
import { CopySheet, PresetSheet, ScopeChips, ScopeKind } from "presets.slint";
|
||||
import { CopySheet, PresetOrigin, PresetRow, PresetSheet, ScopeChips, ScopeKind } from "presets.slint";
|
||||
import { ExportOptions, ExportSheet } from "export.slint";
|
||||
import { FocusMarks, FocusPanel, Peaking } from "peaking.slint";
|
||||
import { SettingsPage } from "settings.slint";
|
||||
@@ -499,7 +499,7 @@ export component AppWindow inherits Window {
|
||||
// where there is an edit to capture, and applied most often in the grid,
|
||||
// where there is a selection to apply it to.
|
||||
in-out property <bool> presets-open: false;
|
||||
in property <[string]> preset-names;
|
||||
in property <[PresetRow]> preset-rows;
|
||||
/// Whether there is an edit in hand to save, set by whichever view opened
|
||||
/// the sheet. `in-out` because that is where the answer is known.
|
||||
in-out property <bool> preset-can-save: false;
|
||||
@@ -3412,7 +3412,7 @@ in property <bool> panel-visible: true;
|
||||
if root.presets-open: PresetSheet {
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
names: root.preset-names;
|
||||
rows: root.preset-rows;
|
||||
can-save: root.preset-can-save;
|
||||
apply-count: root.preset-apply-count;
|
||||
capture-summary: root.preset-capture-summary;
|
||||
|
||||
+58
-26
@@ -16,6 +16,31 @@ export struct ScopeKind {
|
||||
on: bool,
|
||||
}
|
||||
|
||||
// TRACES: FR-DEV-6
|
||||
// Where a listed preset comes from, which decides what can be done to it.
|
||||
export enum PresetOrigin {
|
||||
// The photographer's own.
|
||||
yours,
|
||||
// Shipped with the application. Applied, never renamed or deleted.
|
||||
shipped,
|
||||
// The photographer's copy under a shipped preset's name, standing in for
|
||||
// it. Deleting it reverts to the shipped one; renaming it makes it theirs.
|
||||
changed,
|
||||
}
|
||||
|
||||
// TRACES: FR-DEV-6
|
||||
// One row of the preset sheet: a section heading, or a preset.
|
||||
//
|
||||
// One flat list with the headings in it rather than a list of sections, for
|
||||
// the reason the rows are placed by index below: a nested `for` inside a
|
||||
// `Flickable` has no layout to size it, and the arithmetic stays one line.
|
||||
export struct PresetRow {
|
||||
// Non-empty on a heading row, which names no preset.
|
||||
heading: string,
|
||||
name: string,
|
||||
origin: PresetOrigin,
|
||||
}
|
||||
|
||||
// TRACES: FR-DEV-6 | FR-UI-2
|
||||
// Which kinds of edit travel: a multi-select block of chips that wraps.
|
||||
//
|
||||
@@ -78,8 +103,9 @@ export component ScopeChips inherits Rectangle {
|
||||
// second idiom for the same gesture would be a second thing to learn for no
|
||||
// gain.
|
||||
export component PresetSheet inherits Rectangle {
|
||||
/// The saved names, in the order they are stored.
|
||||
in property <[string]> names;
|
||||
/// The photographer's presets and the shipped ones, sectioned, in the
|
||||
/// order they are listed.
|
||||
in property <[PresetRow]> rows;
|
||||
/// Whether there is an edit in hand to save. False in the library, where
|
||||
/// nothing is open, and with an image that failed to decode.
|
||||
in property <bool> can-save: false;
|
||||
@@ -198,14 +224,21 @@ export component PresetSheet inherits Rectangle {
|
||||
// A floor, so the list is not squeezed out of existence by the
|
||||
// field and the button around it on a short window.
|
||||
min-height: 120px;
|
||||
viewport-height: root.names.length * (Theme.touch-target + 2px);
|
||||
viewport-height: root.rows.length * (Theme.touch-target + 2px);
|
||||
|
||||
for entry[i] in root.names: Rectangle {
|
||||
for entry[i] in root.rows: Rectangle {
|
||||
y: i * (Theme.touch-target + 2px);
|
||||
width: parent.width;
|
||||
height: Theme.touch-target;
|
||||
|
||||
if root.renaming != entry: HorizontalLayout {
|
||||
if entry.heading != "": PanelHeading {
|
||||
text: entry.heading;
|
||||
width: parent.width;
|
||||
height: parent.height;
|
||||
vertical-alignment: bottom;
|
||||
}
|
||||
|
||||
if entry.heading == "" && root.renaming != entry.name: HorizontalLayout {
|
||||
spacing: Theme.gap-sm;
|
||||
|
||||
// The name is the apply button rather than a label
|
||||
@@ -213,33 +246,42 @@ export component PresetSheet inherits Rectangle {
|
||||
// row whose largest target does nothing is a row that
|
||||
// gets pressed by accident and then distrusted.
|
||||
Button {
|
||||
text: entry;
|
||||
text: entry.origin == PresetOrigin.changed
|
||||
? entry.name + " · changed"
|
||||
: entry.name;
|
||||
horizontal-stretch: 1;
|
||||
clicked => { root.apply(entry); }
|
||||
clicked => { root.apply(entry.name); }
|
||||
}
|
||||
|
||||
Button {
|
||||
// A shipped preset is applied and nothing else: it is
|
||||
// not the photographer's to rename or delete, and a
|
||||
// button that did either would be one that lied.
|
||||
// Saving over its name is how it is changed.
|
||||
if entry.origin != PresetOrigin.shipped: Button {
|
||||
text: "Rename";
|
||||
clicked => { root.renaming = entry; }
|
||||
clicked => { root.renaming = entry.name; }
|
||||
}
|
||||
|
||||
Button {
|
||||
text: "Delete";
|
||||
clicked => { root.remove(entry); }
|
||||
// "Revert" on a changed one, because that is what
|
||||
// deleting the copy does: the shipped preset is
|
||||
// still there underneath it.
|
||||
if entry.origin != PresetOrigin.shipped: Button {
|
||||
text: entry.origin == PresetOrigin.changed ? "Revert" : "Delete";
|
||||
clicked => { root.remove(entry.name); }
|
||||
}
|
||||
}
|
||||
|
||||
// Renaming in place rather than in a second sheet: a
|
||||
// dialogue over a dialogue is where a user loses track of
|
||||
// which one Escape closes.
|
||||
if root.renaming == entry: HorizontalLayout {
|
||||
if entry.heading == "" && root.renaming == entry.name: HorizontalLayout {
|
||||
spacing: Theme.gap-sm;
|
||||
|
||||
rename-field := Field {
|
||||
text: entry;
|
||||
text: entry.name;
|
||||
horizontal-stretch: 1;
|
||||
accepted(text) => {
|
||||
root.rename(entry, text);
|
||||
root.rename(entry.name, text);
|
||||
root.renaming = "";
|
||||
}
|
||||
}
|
||||
@@ -252,20 +294,10 @@ export component PresetSheet inherits Rectangle {
|
||||
}
|
||||
}
|
||||
|
||||
if root.names.length == 0: Text {
|
||||
text: root.can-save
|
||||
? "No presets yet. Name the edit above to make the first."
|
||||
: "No presets yet. Open a photograph and save one from the develop panel.";
|
||||
color: Theme.ink-faint;
|
||||
font-size: Theme.text-sm;
|
||||
wrap: word-wrap;
|
||||
width: parent.width;
|
||||
}
|
||||
|
||||
// Says what applying would do *before* it is done, the same way
|
||||
// the grid's "Paste to 40" does — a count in the label is worth
|
||||
// more than a confirmation asking the same question afterwards.
|
||||
if root.names.length > 0 && root.apply-count > 0: Caption {
|
||||
if root.apply-count > 0: Caption {
|
||||
text: root.apply-count == 1
|
||||
? "Applies to 1 selected photograph"
|
||||
: "Applies to " + root.apply-count + " selected photographs";
|
||||
|
||||
Reference in New Issue
Block a user