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

Split identity_ui::wire into one function per feature

Browse Source 
The 837-line wire() had almost no section comments, unlike its siblings, so
the seams had to be found by reading it rather than following markers. Lift
each into its own fn: wire_dials (the two grouping sliders), wire_navigation
(open/close/switch person — needs models() too, for the missing-model
banner), wire_rename_and_merge (a rename and the namesake offer it can
raise), wire_face_actions (pick/confirm/reject/split, the grid's own
actions), wire_grouping_preview, wire_recluster, wire_indexing (the shared
launcher behind Index/Re-index plus Stop), and wire_coverage_and_ignore.
wire() keeps the generic-to-trait-object coercions and the eyes_available
closure, since most of the above need it, and calls each function in the
original order. The reload! macro moved from inside wire() to module scope,
dedented, since macro_rules is scoped textually and every extracted function
uses it.
This commit is contained in:
dtourolle
2026-09-20 18:49:43 +02:00
parent 8540518022
commit 8d08ffd7b7
2 changed files with 158 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/identity_ui.rs
+156 -40
View File
@@ -582,6 +582,20 @@ fn push_grouping(window: &AppWindow, settings: &crate::settings_ui::SettingsCont
window.set_identity_min_group_size(s.faces.min_group_size as i32);
}
macro_rules! reload {
($w:expr, $ctl:expr, $catalog:expr, $store:expr, $settings:expr, $eyes:expr, $changed:ident) => {
refresh(
&$w,
&$ctl,
&$catalog,
$store(),
&model_id(&$settings),
$eyes(),
Changed::$changed,
)
};
}
/// Attach every Identity callback.
///
/// Eight arguments because the screen has eight distinct dependencies and no
@@ -616,10 +630,69 @@ pub fn wire<S, M, P>(
let models: Rc<dyn Fn() -> Option<ModelPaths>> = Rc::new(models);
let paths: Rc<dyn Fn() -> Option<SweepPaths>> = Rc::new(paths);
// The dials start where the settings file left them, once, rather than on
// every open: the screen writes them back through the two callbacks below,
// and re-pushing them mid-drag would fight the slider's own live value.
push_grouping(window, &settings);
// Re-read everything and redraw. Every mutating callback ends in this
// rather than patching the model in place: the operations here have
// second-order effects — a merge empties a person, a split creates one,
// a rejection changes two counts — and a model patched by hand would
// drift from the catalog in exactly the cases that matter.
// Whether the eye models are on this machine, asked each time rather
// than once: it is two `is_file` checks, and a user who drops the files
// in while the app is open should see the faces become work to measure.
let eyes_available: Rc<dyn Fn() -> bool> = {
let models = models.clone();
Rc::new(move || models().is_some_and(|m| m.eyes.is_some()))
};
// The availability closure is an argument rather than named in the
// body: macro hygiene would bind the name to this scope's `Rc`, which the
// first `move` closure would then take with it.
// The last argument is a `Changed` variant, named bare so the call
// stays on one line.
wire_dials(window, &settings);
wire_navigation(
window,
&ctl,
&catalog,
&store,
&models,
&settings,
&eyes_available,
);
wire_rename_and_merge(window, &ctl, &catalog, &store, &settings, &eyes_available);
wire_face_actions(window, &ctl, &catalog, &store, &settings, &eyes_available);
wire_grouping_preview(window, &ctl, &paths, &settings);
wire_recluster(
window,
&ctl,
&catalog,
&store,
&paths,
&settings,
&eyes_available,
);
wire_indexing(
window,
&ctl,
&catalog,
&activity,
&store,
&models,
&paths,
&gpu,
&settings,
&eyes_available,
);
wire_coverage_and_ignore(window, &ctl, &catalog, &store, &settings, &eyes_available);
}
/// The dials.
///
/// The dials start where the settings file left them, once, rather than on
/// every open: the screen writes them back through the two callbacks below,
/// and re-pushing them mid-drag would fight the slider's own live value.
fn wire_dials(window: &AppWindow, settings: &Rc<crate::settings_ui::SettingsController>) {
push_grouping(window, settings);
{
let weak = window.as_weak();
@@ -645,38 +718,18 @@ pub fn wire<S, M, P>(
w.set_identity_grouping_preview(Default::default());
});
}
}
// Re-read everything and redraw. Every mutating callback ends in this
// rather than patching the model in place: the operations here have
// second-order effects — a merge empties a person, a split creates one,
// a rejection changes two counts — and a model patched by hand would
// drift from the catalog in exactly the cases that matter.
// Whether the eye models are on this machine, asked each time rather
// than once: it is two `is_file` checks, and a user who drops the files
// in while the app is open should see the faces become work to measure.
let eyes_available: Rc<dyn Fn() -> bool> = {
let models = models.clone();
Rc::new(move || models().is_some_and(|m| m.eyes.is_some()))
};
// The availability closure is an argument rather than named in the
// body: macro hygiene would bind the name to this scope's `Rc`, which the
// first `move` closure would then take with it.
// The last argument is a `Changed` variant, named bare so the call
// stays on one line.
macro_rules! reload {
($w:expr, $ctl:expr, $catalog:expr, $store:expr, $settings:expr, $eyes:expr, $changed:ident) => {
refresh(
&$w,
&$ctl,
&$catalog,
$store(),
&model_id(&$settings),
$eyes(),
Changed::$changed,
)
};
}
/// Opening and closing the screen, and switching which person is shown.
fn wire_navigation(
window: &AppWindow,
ctl: &Rc<IdentityController>,
catalog: &Rc<RefCell<Option<Catalog>>>,
store: &Rc<dyn Fn() -> Option<Rc<ThumbStore>>>,
models: &Rc<dyn Fn() -> Option<ModelPaths>>,
settings: &Rc<crate::settings_ui::SettingsController>,
eyes_available: &Rc<dyn Fn() -> bool>,
) {
{
let weak = window.as_weak();
let eyes_available = eyes_available.clone();
@@ -742,7 +795,17 @@ pub fn wire<S, M, P>(
reset_name_field(&w);
});
}
}
/// Renaming a person, and the namesake merge offer a rename can raise.
fn wire_rename_and_merge(
window: &AppWindow,
ctl: &Rc<IdentityController>,
catalog: &Rc<RefCell<Option<Catalog>>>,
store: &Rc<dyn Fn() -> Option<Rc<ThumbStore>>>,
settings: &Rc<crate::settings_ui::SettingsController>,
eyes_available: &Rc<dyn Fn() -> bool>,
) {
{
let weak = window.as_weak();
let eyes_available = eyes_available.clone();
@@ -850,7 +913,17 @@ pub fn wire<S, M, P>(
clear_merge_offer(&w, &ctl);
});
}
}
/// The grid: picking, confirming, rejecting and splitting off faces.
fn wire_face_actions(
window: &AppWindow,
ctl: &Rc<IdentityController>,
catalog: &Rc<RefCell<Option<Catalog>>>,
store: &Rc<dyn Fn() -> Option<Rc<ThumbStore>>>,
settings: &Rc<crate::settings_ui::SettingsController>,
eyes_available: &Rc<dyn Fn() -> bool>,
) {
{
let weak = window.as_weak();
let eyes_available = eyes_available.clone();
@@ -970,7 +1043,15 @@ pub fn wire<S, M, P>(
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
});
}
}
/// A dry run of the grouping dials, without writing anything.
fn wire_grouping_preview(
window: &AppWindow,
ctl: &Rc<IdentityController>,
paths: &Rc<dyn Fn() -> Option<SweepPaths>>,
settings: &Rc<crate::settings_ui::SettingsController>,
) {
{
let weak = window.as_weak();
let ctl = ctl.clone();
@@ -1033,7 +1114,18 @@ pub fn wire<S, M, P>(
park_preview_timer(timer);
});
}
}
/// Regroup: rerun clustering over faces already indexed.
fn wire_recluster(
window: &AppWindow,
ctl: &Rc<IdentityController>,
catalog: &Rc<RefCell<Option<Catalog>>>,
store: &Rc<dyn Fn() -> Option<Rc<ThumbStore>>>,
paths: &Rc<dyn Fn() -> Option<SweepPaths>>,
settings: &Rc<crate::settings_ui::SettingsController>,
eyes_available: &Rc<dyn Fn() -> bool>,
) {
{
let weak = window.as_weak();
let ctl = ctl.clone();
@@ -1135,12 +1227,26 @@ pub fn wire<S, M, P>(
park_timer(timer);
});
}
}
// One launcher behind two buttons. "Index faces" and "Re-index every
// face" differ only in which images the pass visits (`FaceSweepScope`);
// the models, the progress, the activity row and the Stop button are the
// same, and a second copy of this closure would be a second place for
// them to disagree.
#[allow(clippy::too_many_arguments)]
/// One launcher behind two buttons. "Index faces" and "Re-index every
/// face" differ only in which images the pass visits (`FaceSweepScope`);
/// the models, the progress, the activity row and the Stop button are the
/// same, and a second copy of this closure would be a second place for
/// them to disagree.
fn wire_indexing(
window: &AppWindow,
ctl: &Rc<IdentityController>,
catalog: &Rc<RefCell<Option<Catalog>>>,
activity: &Rc<crate::activity::ActivityLog>,
store: &Rc<dyn Fn() -> Option<Rc<ThumbStore>>>,
models: &Rc<dyn Fn() -> Option<ModelPaths>>,
paths: &Rc<dyn Fn() -> Option<SweepPaths>>,
gpu: &Rc<Option<dr_gpu::GpuContext>>,
settings: &Rc<crate::settings_ui::SettingsController>,
eyes_available: &Rc<dyn Fn() -> bool>,
) {
let launch: Rc<dyn Fn(crate::repairs::Scope)> = {
let weak = window.as_weak();
let ctl = ctl.clone();
@@ -1347,7 +1453,17 @@ pub fn wire<S, M, P>(
reload!(w, ctl, catalog, store, settings, eyes_available, Library);
});
}
}
/// Coverage, setting a person aside, and clearing every person and face.
fn wire_coverage_and_ignore(
window: &AppWindow,
ctl: &Rc<IdentityController>,
catalog: &Rc<RefCell<Option<Catalog>>>,
store: &Rc<dyn Fn() -> Option<Rc<ThumbStore>>>,
settings: &Rc<crate::settings_ui::SettingsController>,
eyes_available: &Rc<dyn Fn() -> bool>,
) {
{
let weak = window.as_weak();
let catalog = catalog.clone();