mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-13 22:21:59 +00:00
Font menu: the app keeps responding when a sample finishes while a font field is drawn
A font family field looked up its wheel state with `ui.data(|d| ... ui.ctx().viewport_id() ...)`, in two places. The closure of `ui.data` runs with the egui context read-locked, and `viewport_id()` locks it for reading again. The threads that render the menu's samples call `request_repaint()` when they finish, which locks the context for writing. A write request that arrives between the two reads waits for the first read to end, and the second read waits behind the write request, so neither thread goes on. A release build stays blocked; a debug build of egui panics after ten seconds with "Failed to acquire RwLock read after 10s". `font_menu` now reads the viewport id once, before either `ui.data` call, and uses the resulting id in both. The new test draws a closed font field for 2,000 passes while four threads call `request_repaint()` on the same context. With either lookup put back inside its closure, it fails with that panic.
This commit is contained in:
@@ -367,7 +367,11 @@ pub fn font_menu(
|
||||
let state_id = ui.scope_id().with(&id).with("font-menu");
|
||||
let mut st: MenuState = ui.data(|d| d.get_temp(state_id)).unwrap_or_default();
|
||||
ui.data(|d| d.get_temp::<Filters>(filters_id())).unwrap_or_default().apply(&mut st);
|
||||
let target = ui.data(|d| d.get_temp(wheel_fields_id(ui.ctx().viewport_id()).with("target")));
|
||||
// The viewport is read outside `ui.data`. The closure of `ui.data` runs with the context
|
||||
// locked, and a second lock taken inside it blocks once a font-sample thread is waiting to
|
||||
// request a repaint.
|
||||
let wheel_fields = wheel_fields_id(ui.ctx().viewport_id());
|
||||
let target = ui.data(|d| d.get_temp(wheel_fields.with("target")));
|
||||
let target_changed = st.wheel_target != target;
|
||||
if target_changed {
|
||||
st.wheel_order = None;
|
||||
@@ -435,10 +439,8 @@ pub fn font_menu(
|
||||
out = Some(FontPick::Wheel(steps));
|
||||
}
|
||||
}
|
||||
let queued = ui.data(|d| {
|
||||
d.get_temp::<WheelFields>(wheel_fields_id(ui.ctx().viewport_id()))
|
||||
.is_some_and(|fields| fields.queued.iter().any(|event| event.field == field.id))
|
||||
});
|
||||
let queued =
|
||||
ui.data(|d| d.get_temp::<WheelFields>(wheel_fields).is_some_and(|fields| fields.queued.iter().any(|event| event.field == field.id)));
|
||||
if drawn || (!field.contains_pointer() && !queued) {
|
||||
st.wheel_order = None;
|
||||
ui.data_mut(|d| d.remove::<f32>(field.id.with("wheel")));
|
||||
|
||||
@@ -1058,6 +1058,54 @@ fn open_menu_with(id: &str, left: f32, width: f32, screen: egui::Vec2, look: Men
|
||||
drawn
|
||||
}
|
||||
|
||||
/// The field is drawn while other threads ask the context for repaints, as the threads that
|
||||
/// render the samples do when they finish. A read of the context made while the menu already
|
||||
/// holds one waits behind such a request, and a debug build of egui panics after ten seconds.
|
||||
#[test]
|
||||
fn the_field_draws_while_other_threads_request_repaints() {
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
||||
/// Stops the threads when the test ends, failed or not.
|
||||
struct Stop(Arc<AtomicBool>);
|
||||
impl Drop for Stop {
|
||||
fn drop(&mut self) {
|
||||
self.0.store(true, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
|
||||
let ctx = egui::Context::default();
|
||||
crate::theme::install_fonts(&ctx);
|
||||
let stop = Stop(Arc::default());
|
||||
let (started, running) = std::sync::mpsc::channel();
|
||||
let threads: Vec<_> = (0..4)
|
||||
.map(|_| {
|
||||
let (ctx, stop, started) = (ctx.clone(), stop.0.clone(), started.clone());
|
||||
std::thread::spawn(move || {
|
||||
let _ = started.send(());
|
||||
while !stop.load(Ordering::Relaxed) {
|
||||
ctx.request_repaint();
|
||||
}
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
for _ in &threads {
|
||||
running.recv().expect("the thread started");
|
||||
}
|
||||
// The wheel on: the field also looks up its queued wheel input.
|
||||
let look = MenuLook { wheel: true, ..Default::default() };
|
||||
for _ in 0..2000 {
|
||||
let mut out = ctx.run_ui(egui::RawInput::default(), |ui| {
|
||||
font_menu(ui, "test-repaints", "Inter", 200.0, None, look);
|
||||
});
|
||||
out.textures_delta.clear();
|
||||
}
|
||||
drop(stop);
|
||||
for thread in threads {
|
||||
thread.join().expect("the thread ended");
|
||||
}
|
||||
}
|
||||
|
||||
/// A narrow field, such as the Control bar's, opens a menu wide enough for the filters on their
|
||||
/// rows and for the families' names, each drawn whole beside its sample.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user