mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-01 05:57:09 -06:00
LRC-LIB-MISSING: a catalog change during a timed refresh is pending at once
A catalog change seen while a timed refresh of the Missing Photos count is running cannot start its recount until the refresh ends, and the refresh is not pending work, so settle() could return before either finished and the row the recount adds moved the folder rows afterwards (CodeRabbit on #3392). The change is now counted as pending from the frame it is seen (`owed`); the recount takes that entry over when it starts and drops it once its count is stored. settling_waits_for_a_recount_owed_during_a_timed_refresh adds a missing photo while a slow refresh runs and fails without this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
9f7936c3c6
commit
1def7587f5
@@ -184,7 +184,7 @@ pub fn section_drag_feedback(app: &mut LightcraftApp, ctx: &egui::Context) {
|
||||
/// A section's header in `r`: a disclosure chevron in the gutter (where tree rows have their
|
||||
/// chevron), then the bold title; a click folds or unfolds the section (kept in the UI state, so
|
||||
/// it survives restarts), and what the section adds to its header (its button, see `header_button`) sits at
|
||||
/// the visible edge. A folded section says how many rows it holds (`rows`, with View ▸ Show Photo
|
||||
/// the visible edge. A folded section says how many rows it holds (`rows`, with View β–Έ Show Photo
|
||||
/// Counts). `pinned`: this is the copy kept at the top of the sidebar (see `pinned_header`).
|
||||
fn section_header(app: &mut LightcraftApp, ui: &mut egui::Ui, section: SidebarSection, r: Rect, rows: usize, pinned: bool) -> HeaderOutcome {
|
||||
let t = Tokens::get(ui.ctx());
|
||||
@@ -350,12 +350,12 @@ pub(super) fn sections_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, on: Opti
|
||||
}
|
||||
}
|
||||
|
||||
/// The menu of Albums' +: Create Album, Smart Album, Folder…
|
||||
/// The menu of Albums' +: Create Album, Smart Album, Folder…
|
||||
fn new_album_menu(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
if ui.button(crate::i18n::tr("Create Album…")).clicked() {
|
||||
if ui.button(crate::i18n::tr("Create Album…")).clicked() {
|
||||
app.ui.dialog = Some(crate::state::Dialog::NewAlbum { name: String::new(), folder: false, parent: None, include_selected: true });
|
||||
}
|
||||
if ui.button(crate::i18n::tr("Create Smart Album…")).clicked() {
|
||||
if ui.button(crate::i18n::tr("Create Smart Album…")).clicked() {
|
||||
app.ui.dialog = Some(crate::state::Dialog::SmartRules {
|
||||
id: None,
|
||||
name: String::new(),
|
||||
@@ -363,16 +363,16 @@ fn new_album_menu(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
parent: None,
|
||||
});
|
||||
}
|
||||
if ui.button(crate::i18n::tr("Create Smart Album from Filter…")).clicked() {
|
||||
if ui.button(crate::i18n::tr("Create Smart Album from Filter…")).clicked() {
|
||||
app.ui.dialog = Some(crate::state::Dialog::NewSmartAlbum { name: String::new(), parent: None });
|
||||
}
|
||||
if ui.button(crate::i18n::tr("Create Folder…")).clicked() {
|
||||
if ui.button(crate::i18n::tr("Create Folder…")).clicked() {
|
||||
app.ui.dialog = Some(crate::state::Dialog::NewAlbum { name: String::new(), folder: true, parent: None, include_selected: false });
|
||||
}
|
||||
// only once the albums were put in an order by hand
|
||||
if app.session.catalog.album_children_are_ordered(None) {
|
||||
ui.separator();
|
||||
if ui.button(crate::i18n::tr("Sort Albums A–Z")).on_hover_text(crate::i18n::tr("Go back to listing them by name")).clicked() {
|
||||
if ui.button(crate::i18n::tr("Sort Albums A–Z")).on_hover_text(crate::i18n::tr("Go back to listing them by name")).clicked() {
|
||||
let _ = app.run("album.sort", json!({}));
|
||||
}
|
||||
}
|
||||
@@ -386,6 +386,8 @@ fn new_album_menu(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
/// ([`super::fs_cached_running`]): the Missing Photos row they add or take away moves every row
|
||||
/// below it. The timed refresh is not: it rarely changes the count, and on a share where counting
|
||||
/// takes seconds, a headless run's fast simulated clock would start the next one before settling.
|
||||
/// A catalog change during a timed refresh is pending from the frame it is seen, although its
|
||||
/// recount starts only when the refresh ends.
|
||||
pub(super) fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize {
|
||||
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering::AcqRel, Ordering::Relaxed};
|
||||
#[derive(Clone, Default)]
|
||||
@@ -394,19 +396,34 @@ pub(super) fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize
|
||||
running: std::sync::Arc<AtomicBool>,
|
||||
at: f64,
|
||||
rev: u64,
|
||||
/// A catalog change arrived while a count was running: its recount is pending work
|
||||
/// already, held until the recount stores its result.
|
||||
owed: bool,
|
||||
}
|
||||
let id = egui::Id::unique("missing-count");
|
||||
let now = ui.input(|i| i.time);
|
||||
let rev = app.session.catalog.revision;
|
||||
let mut job: Job = ui.data(|d| d.get_temp(id)).unwrap_or(Job { rev: u64::MAX, at: f64::MIN, ..Default::default() });
|
||||
let running = job.running.load(Relaxed);
|
||||
if running && job.rev != rev && !job.owed {
|
||||
// the recount waits for the running count (a timed refresh, not waited for), but settling
|
||||
// must wait for it from now on
|
||||
super::local::fs_running_counter(ui.ctx()).fetch_add(1, AcqRel);
|
||||
job.owed = true;
|
||||
ui.data_mut(|d| d.insert_temp(id, job.clone()));
|
||||
}
|
||||
let fresh = now - job.at < 5.0 && job.rev == rev;
|
||||
if !fresh && !job.running.load(Relaxed) {
|
||||
if !fresh && !running {
|
||||
// the same scope as the Missing Photos view: library photos only, never Local browse records
|
||||
let paths = lightcraft_engine::cmd::missing::candidates(&app.session.catalog);
|
||||
let pending = (job.rev != rev).then(|| super::local::fs_running_counter(ui.ctx()));
|
||||
if let Some(p) = &pending {
|
||||
if let Some(p) = &pending
|
||||
&& !job.owed
|
||||
{
|
||||
p.fetch_add(1, AcqRel);
|
||||
}
|
||||
// the recount takes over the entry held while it waited
|
||||
job.owed = false;
|
||||
job.running.store(true, Relaxed);
|
||||
job.at = now;
|
||||
job.rev = rev;
|
||||
|
||||
@@ -1888,6 +1888,30 @@ fn settling_waits_for_the_missing_photos_count() {
|
||||
assert!(h.settle(Duration::from_secs(30)), "the sidebar settles while a slow count refreshes");
|
||||
}
|
||||
|
||||
/// A catalog change while a slow timed refresh of the count runs: its recount starts only when
|
||||
/// the refresh ends, but settling waits for it from the frame the change is seen, so the Missing
|
||||
/// Photos row it adds is on screen once the sidebar settled.
|
||||
#[test]
|
||||
fn settling_waits_for_a_recount_owed_during_a_timed_refresh() {
|
||||
use lightcraft_catalog::{Op, Photo, Source};
|
||||
let dir = std::env::temp_dir().join(format!("lc-ui-missing-owed-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let here = dir.join("a.jpg");
|
||||
std::fs::write(&here, b"photo").unwrap();
|
||||
// settling takes the first count (2 s), in which the simulated clock passes the 5 s refresh
|
||||
// interval: a timed refresh is running when it returns
|
||||
let delay = Duration::from_secs(2);
|
||||
let services = Services { png: None, ..Default::default() };
|
||||
let mut h = folders_app_prepared(&[&here.to_string_lossy()], [1400.0, 900.0], services, |app| app.missing_count_test_delay = Some(delay));
|
||||
assert!(!has(&h, "source:missing"), "no photo is missing yet");
|
||||
let id = h.app.session.catalog.alloc_photo_id();
|
||||
let gone = Photo::new(id, Source::File { path: "/lc-missing-owed/gone.jpg".into() }, "gone.jpg", "JPEG", 60, 40, "2026-01-01T10:00:00");
|
||||
h.app.session.catalog.apply(Op::AddPhoto { photo: Box::new(gone) }).unwrap();
|
||||
assert!(h.settle(Duration::from_secs(30)), "the sidebar settles");
|
||||
assert!(has(&h, "source:missing"), "the Missing Photos row is on screen once the sidebar settled");
|
||||
let _ = std::fs::remove_dir_all(dir);
|
||||
}
|
||||
|
||||
/// Right-click a folder ▸ Set Color Label ▸ a colour: the folder's row shows the label's dot (a
|
||||
/// row without a label shows none), and None takes it off again.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user