From 58eef0faab6e98cadbd94ab788802beddac2181d Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Mon, 20 Apr 2026 16:11:13 +0200 Subject: [PATCH] Small improvements --- Cargo.lock | 11 +++ Cargo.toml | 1 + crates/egui_kittest/src/lib.rs | 19 +++-- crates/kittest_inspector/Cargo.toml | 3 +- crates/kittest_inspector/src/main.rs | 106 +++++++++++++++++++++++++-- 5 files changed, 128 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c80c700a2..1ae87bbc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1792,6 +1792,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs4" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8640e34b88f7652208ce9e88b1a37a2ae95227d84abec377ccd3c5cfeb141ed4" +dependencies = [ + "rustix 1.1.4", + "windows-sys 0.59.0", +] + [[package]] name = "futures-core" version = "0.3.32" @@ -2526,6 +2536,7 @@ dependencies = [ "eframe", "egui", "egui_extras", + "fs4", "serde", ] diff --git a/Cargo.toml b/Cargo.toml index f40ce82d7..2d90659de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,6 +93,7 @@ ehttp = { version = "0.7.1", default-features = false } enum-map = "2.7.3" env_logger = { version = "0.11.8", default-features = false } font-types = { version = "0.11.0", default-features = false, features = ["std"] } +fs4 = { version = "0.13.1", default-features = false, features = ["sync"] } glow = "0.17.0" glutin = { version = "0.32.3", default-features = false } glutin-winit = { version = "0.5.0", default-features = false } diff --git a/crates/egui_kittest/src/lib.rs b/crates/egui_kittest/src/lib.rs index 82a7f1f5f..9c4fad823 100644 --- a/crates/egui_kittest/src/lib.rs +++ b/crates/egui_kittest/src/lib.rs @@ -892,11 +892,12 @@ impl<'a, State> Harness<'a, State> { /// Block at the inspector until it tells us to resume, re-rendering after each batch of /// events it sends. Events drive an internal `_step_inner` (and recording capture), but /// do NOT return control to the outer test — the test advances only when the inspector - /// replies with no events *and* egui has no pending repaint (i.e. user hit Next/Play with - /// a settled UI, or Control mode is off). + /// replies with no events (i.e. user hit Next/Play/Step, or Control mode is off). /// - /// While paused in Control mode, any `request_repaint` from the app (animations, async - /// image loads, etc.) also drives another internal step so the user sees the UI tick. + /// We only loop while the inspector is *feeding events back* (Control mode) — animation + /// frames driven by `request_repaint` are handled by the outer `try_run` loop calling + /// `step()` again, so we don't need to drive them here. Doing so would send extra "no + /// event highlighted" frames between each event and confuse the Step UX. #[cfg(feature = "inspector")] fn drive_inspector(&mut self) { if self.inspector.is_none() { @@ -922,8 +923,7 @@ impl<'a, State> Harness<'a, State> { } else { return; }; - let wants_repaint = self.root_viewport_output().repaint_delay == Duration::ZERO; - if events.is_empty() && !wants_repaint { + if events.is_empty() { return; } for event in events { @@ -934,6 +934,13 @@ impl<'a, State> Harness<'a, State> { self._step_inner(false); #[cfg(feature = "recording")] self.capture_frame_if_recording(false); + + // Run one more step so effects of the just-delivered events are visible in the + // next frame we send (e.g. a clicked button's state change). Without this we'd + // show the frame *during* the click but not *after*. + self._step_inner(false); + #[cfg(feature = "recording")] + self.capture_frame_if_recording(false); } } diff --git a/crates/kittest_inspector/Cargo.toml b/crates/kittest_inspector/Cargo.toml index 6e72f3818..c3246a924 100644 --- a/crates/kittest_inspector/Cargo.toml +++ b/crates/kittest_inspector/Cargo.toml @@ -25,7 +25,7 @@ required-features = ["app"] default = ["app"] ## Build the eframe inspector binary. -app = ["dep:eframe", "dep:egui_extras"] +app = ["dep:eframe", "dep:egui_extras", "dep:fs4"] [dependencies] accesskit = { workspace = true, features = ["serde"] } @@ -36,6 +36,7 @@ serde = { workspace = true } # `app` feature dependencies: eframe = { workspace = true, features = ["default_fonts", "wgpu"], optional = true } egui_extras = { workspace = true, features = ["image"], optional = true } +fs4 = { workspace = true, optional = true } [lints] workspace = true diff --git a/crates/kittest_inspector/src/main.rs b/crates/kittest_inspector/src/main.rs index 2c1eaecf7..d68db278c 100644 --- a/crates/kittest_inspector/src/main.rs +++ b/crates/kittest_inspector/src/main.rs @@ -34,7 +34,26 @@ enum PlayState { Paused, } +/// Fast-forward state for the ⏭ Next button. +#[derive(Debug, Clone, Copy)] +enum SkipState { + Inactive, + /// Auto-release every incoming frame until `call_site_line` differs from this value. + UntilNewCallLine(Option), +} + +impl SkipState { + fn is_active(self) -> bool { + matches!(self, Self::UntilNewCallLine(_)) + } +} + fn main() -> eframe::Result<()> { + // Cross-process single-instance guard. If another inspector is already running, block + // here until that window closes. Held for the lifetime of `_lock`; the OS releases the + // flock when the file descriptor is dropped on exit. + let _lock = acquire_single_instance_lock(); + let (worker_tx, worker_rx) = mpsc::channel::(); let (release_tx, release_rx) = mpsc::channel::>(); @@ -115,6 +134,9 @@ struct InspectorApp { queued_events: Vec, /// Set when a new frame arrives; the Source section consumes it to scroll once per frame. scroll_pending: bool, + /// While `UntilNewCallLine`, auto-release every incoming frame until we see one with a + /// different `call_site_line` — i.e. until the test moves past the current runner call. + skip: SkipState, } impl InspectorApp { @@ -137,6 +159,7 @@ impl InspectorApp { control_enabled: false, queued_events: Vec::new(), scroll_pending: false, + skip: SkipState::Inactive, } } @@ -146,14 +169,32 @@ impl InspectorApp { WorkerEvent::Frame(frame) => { self.received_count += 1; self.upload_frame(ctx, &frame); + let new_call_line = frame + .source + .as_ref() + .and_then(|s| s.call_site_line); // Keep the selection sticky across frames (same NodeId may still exist). self.current_frame = Some(*frame); self.worker_waiting = true; - self.scroll_pending = true; + + // If we're fast-forwarding to the next `run()` call, stop once the + // call_site line differs from the one we started from. + let still_skipping = matches!( + self.skip, + SkipState::UntilNewCallLine(from) if new_call_line == from + ); + if still_skipping { + // Don't auto-scroll / flash for in-between frames we're about to blow + // past; the user will see the first settled frame at the new call. + } else { + self.skip = SkipState::Inactive; + self.scroll_pending = true; + } } WorkerEvent::Disconnected => { self.connected = false; self.worker_waiting = false; + self.skip = SkipState::Inactive; } } } @@ -189,10 +230,13 @@ impl eframe::App for InspectorApp { central_panel(self, ui); // End-of-frame auto-release policy: + // - Fast-forwarding to the next `run()` call: always release. // - Control mode: stay blocked, but advance one step whenever the user generates events // (each click / keypress = one harness step). - // - Otherwise, Playing mode runs freely; Paused mode waits for Next/Play. - let auto_release = if self.control_enabled { + // - Otherwise, Playing mode runs freely; Paused mode waits for Next/Play/Step. + let auto_release = if self.skip.is_active() { + true + } else if self.control_enabled { !self.queued_events.is_empty() } else { self.play_state == PlayState::Playing @@ -228,12 +272,27 @@ fn controls_panel(app: &mut InspectorApp, ui: &mut egui::Ui) { } let can_step = app.play_state == PlayState::Paused && app.worker_waiting; if ui - .add_enabled(can_step, egui::Button::new("⏭ Next")) - .on_hover_text("Advance one harness step") + .add_enabled(can_step, egui::Button::new("⏩ Step")) + .on_hover_text("Advance one harness internal step") .clicked() { app.send_release(); } + if ui + .add_enabled(can_step, egui::Button::new("⏭ Next")) + .on_hover_text( + "Fast-forward until the test reaches the next `run()` / `step()` call", + ) + .clicked() + { + let current_line = app + .current_frame + .as_ref() + .and_then(|f| f.source.as_ref()) + .and_then(|s| s.call_site_line); + app.skip = SkipState::UntilNewCallLine(current_line); + app.send_release(); + } ui.separator(); @@ -732,3 +791,40 @@ fn widget_details(ui: &mut egui::Ui, id: NodeId, node: &Node) { } }); } + +/// Try to acquire a cross-process exclusive lock on a well-known file so that only one +/// inspector window can be open on the machine at a time. Blocks here (before we open any +/// windows or touch stdio beyond this stderr line) if another inspector is already running. +fn acquire_single_instance_lock() -> Option { + use fs4::fs_std::FileExt; + + // We specifically need a stable, cross-process path here — tempfile's per-process dir + // can't serve as a system-wide mutex. + #[expect(clippy::disallowed_methods)] + let path = std::env::temp_dir().join("kittest_inspector.lock"); + + let file = match std::fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&path) + { + Ok(f) => f, + Err(err) => { + eprintln!( + "kittest_inspector: couldn't open lock file {}: {err} (running without single-instance guard)", + path.display() + ); + return None; + } + }; + + match FileExt::lock_exclusive(&file) { + Ok(()) => Some(file), + Err(err) => { + eprintln!("kittest_inspector: failed to acquire lock: {err}"); + None + } + } +}