fix: Keep the loading card's Cancel button still; deterministic Cancel test (#686)
The loading card was centred on its height, so Cancel moved when the progress bar or the saved preview appeared, and a click could miss it. That's what made the Cancel test flaky on slow runners. The card is now anchored at its top, Cancel sits in the heading row and the progress bar is always shown. Progress::hold_at lets the test hold a load at a feature boundary until it's cancelled. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
738d532ce6
commit
e30f3c9ebe
3 files changed
+74
-25
No files matched your search
@@ -10,11 +10,19 @@ use std::sync::{
|
||||
};
|
||||
|
||||
/// Shared between the worker evaluating a design and the window showing its progress.
|
||||
#[derive(Debug, Default)]
|
||||
#[derive(Debug)]
|
||||
pub struct Progress {
|
||||
cancelled: AtomicBool,
|
||||
done: AtomicUsize,
|
||||
total: AtomicUsize,
|
||||
/// The feature the evaluation waits at until cancelled ([`Progress::hold_at`]).
|
||||
hold: AtomicUsize,
|
||||
}
|
||||
|
||||
impl Default for Progress {
|
||||
fn default() -> Self {
|
||||
Progress { cancelled: AtomicBool::new(false), done: AtomicUsize::new(0), total: AtomicUsize::new(0), hold: AtomicUsize::new(usize::MAX) }
|
||||
}
|
||||
}
|
||||
|
||||
impl Progress {
|
||||
@@ -25,6 +33,11 @@ impl Progress {
|
||||
pub fn is_cancelled(&self) -> bool {
|
||||
self.cancelled.load(Ordering::Relaxed)
|
||||
}
|
||||
/// Make the watched evaluation wait before feature `index` until it is cancelled: tests
|
||||
/// use it to act while a load is certainly still running, whatever the machine's speed.
|
||||
pub fn hold_at(&self, index: usize) {
|
||||
self.hold.store(index, Ordering::Relaxed);
|
||||
}
|
||||
/// Features evaluated so far and the timeline's length (0 before evaluation starts).
|
||||
pub fn features(&self) -> (usize, usize) {
|
||||
(self.done.load(Ordering::Relaxed), self.total.load(Ordering::Relaxed))
|
||||
@@ -85,6 +98,12 @@ impl Evaluation {
|
||||
let Some(cur) = c.as_ref() else { return false };
|
||||
if self.outer {
|
||||
cur.progress.done.store(index, Ordering::Relaxed);
|
||||
while index == cur.progress.hold.load(Ordering::Relaxed) && !cur.progress.is_cancelled() {
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
std::thread::sleep(std::time::Duration::from_millis(1));
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
std::hint::spin_loop();
|
||||
}
|
||||
}
|
||||
cur.progress.is_cancelled()
|
||||
})
|
||||
|
||||
@@ -95,6 +95,9 @@ impl Running {
|
||||
pub struct FileLoad {
|
||||
queued: VecDeque<Request>,
|
||||
running: Option<Running>,
|
||||
/// Tests: the next load waits before this feature until cancelled.
|
||||
#[cfg(test)]
|
||||
pub hold_at: Option<usize>,
|
||||
}
|
||||
|
||||
impl FileLoad {
|
||||
@@ -102,6 +105,10 @@ impl FileLoad {
|
||||
pub fn busy(&self) -> bool {
|
||||
self.running.as_ref().is_some_and(|r| !r.cancelled()) || !self.queued.is_empty()
|
||||
}
|
||||
/// The running load's progress: (features rebuilt, timeline length).
|
||||
pub fn features(&self) -> Option<(usize, usize)> {
|
||||
self.running.as_ref().map(|r| r.progress.features())
|
||||
}
|
||||
/// Nothing running or queued (a cancelled job may still be finishing in [`Self::busy`]'s eyes).
|
||||
pub fn idle(&self) -> bool {
|
||||
self.running.is_none() && self.queued.is_empty()
|
||||
@@ -210,6 +217,10 @@ fn start(app: &mut SolveApp) {
|
||||
let phase = Arc::new(AtomicU8::new(0));
|
||||
let worker_phase = phase.clone();
|
||||
let progress = Arc::new(Progress::default());
|
||||
#[cfg(test)]
|
||||
if let Some(i) = app.file_load.hold_at.take() {
|
||||
progress.hold_at(i);
|
||||
}
|
||||
let worker_progress = progress.clone();
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
let (preview_tx, preview_rx) = std::sync::mpsc::channel();
|
||||
@@ -491,22 +502,32 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) {
|
||||
ui.painter().rect_filled(cover, 0.0, egui::Color32::from_black_alpha(70));
|
||||
});
|
||||
let width = 340.0f32.min(cover.width() - 16.0).max(120.0);
|
||||
egui::Area::new(card_id).order(egui::Order::Middle).pivot(egui::Align2::CENTER_CENTER).fixed_pos(centre).show(ctx, |child| {
|
||||
// Anchored at its top with Cancel in the heading row: the card grows downwards (the saved
|
||||
// preview, the progress bar) and Cancel never moves under the pointer.
|
||||
let top = egui::pos2(centre.x, (centre.y - 120.0).max(cover.top() + 8.0));
|
||||
egui::Area::new(card_id).order(egui::Order::Middle).pivot(egui::Align2::CENTER_TOP).fixed_pos(top).show(ctx, |child| {
|
||||
egui::Frame::popup(child.style()).fill(t.panel).show(child, |ui| {
|
||||
ui.set_width(width - 16.0);
|
||||
ui.label(
|
||||
egui::RichText::new(if cancelled {
|
||||
"Waiting for file operation"
|
||||
} else {
|
||||
match r.request.kind {
|
||||
Kind::Insert => "Inserting geometry",
|
||||
Kind::Open => "Opening design",
|
||||
Kind::Recover { .. } => "Recovering design",
|
||||
}
|
||||
})
|
||||
.strong()
|
||||
.size(14.0),
|
||||
);
|
||||
ui.horizontal(|ui| {
|
||||
ui.label(
|
||||
egui::RichText::new(if cancelled {
|
||||
"Waiting for file operation"
|
||||
} else {
|
||||
match r.request.kind {
|
||||
Kind::Insert => "Inserting geometry",
|
||||
Kind::Open => "Opening design",
|
||||
Kind::Recover { .. } => "Recovering design",
|
||||
}
|
||||
})
|
||||
.strong()
|
||||
.size(14.0),
|
||||
);
|
||||
ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
|
||||
let b = ui.button("Cancel");
|
||||
crate::scenario::publish_handle("file_loading:cancel", b.rect.center());
|
||||
cancel |= b.clicked();
|
||||
});
|
||||
});
|
||||
ui.label(egui::RichText::new(format!("{name}…")).weak());
|
||||
if let Some(preview) = r.preview.as_ref().filter(|_| !cancelled) {
|
||||
let w = ui.available_width().min(320.0);
|
||||
@@ -527,15 +548,15 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) {
|
||||
}
|
||||
});
|
||||
});
|
||||
if phase == 0 && total > 0 && !cancelled {
|
||||
ui.add(egui::ProgressBar::new(done as f32 / total as f32).desired_height(6.0));
|
||||
}
|
||||
ui.add_space(4.0);
|
||||
ui.allocate_ui_with_layout(egui::vec2(ui.available_width(), 24.0), egui::Layout::right_to_left(egui::Align::Center), |ui| {
|
||||
let b = ui.button("Cancel");
|
||||
crate::scenario::publish_handle("file_loading:cancel", b.rect.center());
|
||||
cancel |= b.clicked();
|
||||
});
|
||||
// Always there, so the card keeps its height as the phases change.
|
||||
let fraction = if cancelled || total == 0 {
|
||||
0.0
|
||||
} else if phase == 0 {
|
||||
done as f32 / total as f32
|
||||
} else {
|
||||
1.0
|
||||
};
|
||||
ui.add(egui::ProgressBar::new(fraction).desired_height(6.0));
|
||||
});
|
||||
});
|
||||
if cancel && !cancelled {
|
||||
|
||||
@@ -154,8 +154,17 @@ fn cancel_stops_the_open_and_keeps_the_current_design() {
|
||||
let mut w = Win::new();
|
||||
w.app.run("solid.box", json!({"length": 10, "width": 10, "height": 10})).unwrap();
|
||||
let current = w.app.session.doc.clone();
|
||||
// The worker waits before the third feature until Cancel: the load is certainly still
|
||||
// running when Cancel is clicked, on a fast machine or a slow one.
|
||||
w.app.file_load.hold_at = Some(2);
|
||||
w.app.open_path_async(&path);
|
||||
// The overlay is laid out on its first frame and shown from the next.
|
||||
let began = std::time::Instant::now();
|
||||
while w.app.file_load.features().is_none_or(|(done, _)| done < 2) {
|
||||
assert!(began.elapsed() < std::time::Duration::from_secs(120), "the load reached the held feature");
|
||||
w.frame(vec![]);
|
||||
std::thread::sleep(std::time::Duration::from_millis(1));
|
||||
}
|
||||
// The overlay (and any saved preview the worker sent) is laid out and shown.
|
||||
for _ in 0..3 {
|
||||
w.frame(vec![]);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user