From da85b499e97436d33768ff132d6509b5bdb806e2 Mon Sep 17 00:00:00 2001 From: Zach Thompson Date: Fri, 30 Dec 2022 11:05:12 -0500 Subject: [PATCH 1/5] Add `ShapeProcessor` to `Evaluator` --- Cargo.lock | 1 + crates/fj-host/Cargo.toml | 1 + crates/fj-host/src/evaluator.rs | 5 ++++- crates/fj-host/src/host.rs | 5 +++-- crates/fj-operations/src/shape_processor.rs | 1 + crates/fj-window/src/event_loop_handler.rs | 3 ++- crates/fj-window/src/run.rs | 4 +++- 7 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1121f4f96..ae22adcf8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1175,6 +1175,7 @@ dependencies = [ "cargo_metadata", "crossbeam-channel", "fj", + "fj-operations", "libloading", "notify", "thiserror", diff --git a/crates/fj-host/Cargo.toml b/crates/fj-host/Cargo.toml index 02a37f5f2..b244a3606 100644 --- a/crates/fj-host/Cargo.toml +++ b/crates/fj-host/Cargo.toml @@ -15,6 +15,7 @@ categories.workspace = true cargo_metadata = "0.15.2" crossbeam-channel = "0.5.6" fj.workspace = true +fj-operations.workspace = true libloading = "0.7.4" notify = "5.0.0" thiserror = "1.0.35" diff --git a/crates/fj-host/src/evaluator.rs b/crates/fj-host/src/evaluator.rs index 307fa95f3..152c3ec0c 100644 --- a/crates/fj-host/src/evaluator.rs +++ b/crates/fj-host/src/evaluator.rs @@ -1,18 +1,20 @@ use std::thread; use crossbeam_channel::{Receiver, SendError, Sender}; +use fj_operations::shape_processor::ShapeProcessor; use crate::{Error, Evaluation, Model}; /// Evaluates a model in a background thread pub struct Evaluator { + shape_processor: ShapeProcessor, trigger_tx: Sender, event_rx: Receiver, } impl Evaluator { /// Create an `Evaluator` from a model - pub fn from_model(model: Model) -> Self { + pub fn new(model: Model, shape_processor: ShapeProcessor) -> Self { let (event_tx, event_rx) = crossbeam_channel::bounded(0); let (trigger_tx, trigger_rx) = crossbeam_channel::bounded(0); @@ -49,6 +51,7 @@ impl Evaluator { }); Self { + shape_processor, trigger_tx, event_rx, } diff --git a/crates/fj-host/src/host.rs b/crates/fj-host/src/host.rs index d681347a0..c9d278729 100644 --- a/crates/fj-host/src/host.rs +++ b/crates/fj-host/src/host.rs @@ -1,4 +1,5 @@ use crossbeam_channel::Receiver; +use fj_operations::shape_processor::ShapeProcessor; use crate::{Error, Evaluator, Model, ModelEvent, Watcher}; @@ -13,9 +14,9 @@ impl Host { /// /// This is only useful, if you want to continuously watch the model for /// changes. If you don't, just keep using `Model`. - pub fn from_model(model: Model) -> Result { + pub fn new(model: Model, shape_processor: ShapeProcessor) -> Result { let watch_path = model.watch_path(); - let evaluator = Evaluator::from_model(model); + let evaluator = Evaluator::new(model, shape_processor); let watcher = Watcher::watch_model(watch_path, &evaluator)?; Ok(Self { diff --git a/crates/fj-operations/src/shape_processor.rs b/crates/fj-operations/src/shape_processor.rs index 2a6720608..aaeede5cc 100644 --- a/crates/fj-operations/src/shape_processor.rs +++ b/crates/fj-operations/src/shape_processor.rs @@ -14,6 +14,7 @@ use fj_math::Scalar; use crate::Shape as _; /// Processes an [`fj::Shape`] into a [`ProcessedShape`] +#[derive(Clone)] pub struct ShapeProcessor { /// The tolerance value used for creating the triangle mesh pub tolerance: Option, diff --git a/crates/fj-window/src/event_loop_handler.rs b/crates/fj-window/src/event_loop_handler.rs index 996676d2b..747fd83e9 100644 --- a/crates/fj-window/src/event_loop_handler.rs +++ b/crates/fj-window/src/event_loop_handler.rs @@ -193,7 +193,8 @@ impl EventLoopHandler { if let Some(model_path) = new_model_path { let model = Model::new(model_path, Parameters::empty()) .unwrap(); - let new_host = Host::from_model(model)?; + let new_host = + Host::new(model, self.shape_processor.clone())?; self.host = Some(new_host); } } diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 35f57c194..9750fb698 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -32,7 +32,9 @@ pub fn run( let egui_winit_state = egui_winit::State::new(&event_loop); - let host = model.map(Host::from_model).transpose()?; + let host = model + .map(|model| Host::new(model, shape_processor.clone())) + .transpose()?; let mut handler = EventLoopHandler { invert_zoom, From ff5bb47f00840a40295b368e0579bc8937aada36 Mon Sep 17 00:00:00 2001 From: Zach Thompson Date: Fri, 30 Dec 2022 16:32:13 -0500 Subject: [PATCH 2/5] Send `EventLoopProxy` to `Host` --- Cargo.lock | 1 + crates/fj-host/Cargo.toml | 1 + crates/fj-host/src/evaluator.rs | 10 +++++++++- crates/fj-host/src/host.rs | 9 +++++++-- crates/fj-host/src/model.rs | 1 + crates/fj-window/src/event_loop_handler.rs | 12 ++++++++---- crates/fj-window/src/run.rs | 12 ++++++++---- 7 files changed, 35 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ae22adcf8..935556179 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1180,6 +1180,7 @@ dependencies = [ "notify", "thiserror", "tracing", + "winit", ] [[package]] diff --git a/crates/fj-host/Cargo.toml b/crates/fj-host/Cargo.toml index b244a3606..86c555170 100644 --- a/crates/fj-host/Cargo.toml +++ b/crates/fj-host/Cargo.toml @@ -20,3 +20,4 @@ libloading = "0.7.4" notify = "5.0.0" thiserror = "1.0.35" tracing = "0.1.37" +winit = "0.27.5" diff --git a/crates/fj-host/src/evaluator.rs b/crates/fj-host/src/evaluator.rs index 152c3ec0c..9e0633d83 100644 --- a/crates/fj-host/src/evaluator.rs +++ b/crates/fj-host/src/evaluator.rs @@ -2,19 +2,25 @@ use std::thread; use crossbeam_channel::{Receiver, SendError, Sender}; use fj_operations::shape_processor::ShapeProcessor; +use winit::event_loop::EventLoopProxy; use crate::{Error, Evaluation, Model}; /// Evaluates a model in a background thread pub struct Evaluator { shape_processor: ShapeProcessor, + event_loop_proxy: EventLoopProxy, trigger_tx: Sender, event_rx: Receiver, } impl Evaluator { /// Create an `Evaluator` from a model - pub fn new(model: Model, shape_processor: ShapeProcessor) -> Self { + pub fn new( + model: Model, + shape_processor: ShapeProcessor, + event_loop_proxy: EventLoopProxy, + ) -> Self { let (event_tx, event_rx) = crossbeam_channel::bounded(0); let (trigger_tx, trigger_rx) = crossbeam_channel::bounded(0); @@ -52,6 +58,7 @@ impl Evaluator { Self { shape_processor, + event_loop_proxy, trigger_tx, event_rx, } @@ -72,6 +79,7 @@ impl Evaluator { pub struct TriggerEvaluation; /// An event emitted by [`Evaluator`] +#[derive(Debug)] pub enum ModelEvent { /// A change in the model has been detected ChangeDetected, diff --git a/crates/fj-host/src/host.rs b/crates/fj-host/src/host.rs index c9d278729..101103f34 100644 --- a/crates/fj-host/src/host.rs +++ b/crates/fj-host/src/host.rs @@ -1,5 +1,6 @@ use crossbeam_channel::Receiver; use fj_operations::shape_processor::ShapeProcessor; +use winit::event_loop::EventLoopProxy; use crate::{Error, Evaluator, Model, ModelEvent, Watcher}; @@ -14,9 +15,13 @@ impl Host { /// /// This is only useful, if you want to continuously watch the model for /// changes. If you don't, just keep using `Model`. - pub fn new(model: Model, shape_processor: ShapeProcessor) -> Result { + pub fn new( + model: Model, + shape_processor: ShapeProcessor, + event_loop_proxy: EventLoopProxy, + ) -> Result { let watch_path = model.watch_path(); - let evaluator = Evaluator::new(model, shape_processor); + let evaluator = Evaluator::new(model, shape_processor, event_loop_proxy); let watcher = Watcher::watch_model(watch_path, &evaluator)?; Ok(Self { diff --git a/crates/fj-host/src/model.rs b/crates/fj-host/src/model.rs index 0e752215e..80cad13c9 100644 --- a/crates/fj-host/src/model.rs +++ b/crates/fj-host/src/model.rs @@ -170,6 +170,7 @@ impl Model { /// The result of evaluating a model /// /// See [`Model::evaluate`]. +#[derive(Debug)] pub struct Evaluation { /// The shape pub shape: fj::Shape, diff --git a/crates/fj-window/src/event_loop_handler.rs b/crates/fj-window/src/event_loop_handler.rs index 747fd83e9..c3d2ed1ed 100644 --- a/crates/fj-window/src/event_loop_handler.rs +++ b/crates/fj-window/src/event_loop_handler.rs @@ -10,7 +10,7 @@ use winit::{ ElementState, Event, KeyboardInput, MouseButton, MouseScrollDelta, VirtualKeyCode, WindowEvent, }, - event_loop::ControlFlow, + event_loop::{ControlFlow, EventLoopProxy}, }; use crate::window::Window; @@ -18,6 +18,7 @@ use crate::window::Window; pub struct EventLoopHandler { pub invert_zoom: bool, pub shape_processor: ShapeProcessor, + pub event_loop_proxy: EventLoopProxy, pub window: Window, pub viewer: Viewer, pub egui_winit_state: egui_winit::State, @@ -37,7 +38,7 @@ impl EventLoopHandler { #[allow(clippy::result_large_err)] pub fn handle_event( &mut self, - event: Event<()>, + event: Event, control_flow: &mut ControlFlow, ) -> Result<(), Error> { if let Some(host) = &self.host { @@ -193,8 +194,11 @@ impl EventLoopHandler { if let Some(model_path) = new_model_path { let model = Model::new(model_path, Parameters::empty()) .unwrap(); - let new_host = - Host::new(model, self.shape_processor.clone())?; + let new_host = Host::new( + model, + self.shape_processor.clone(), + self.event_loop_proxy.clone(), + )?; self.host = Some(new_host); } } diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 9750fb698..787fd4f0f 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -8,12 +8,12 @@ use std::{ fmt::{self, Write}, }; -use fj_host::{Host, Model}; +use fj_host::{Host, Model, ModelEvent}; use fj_operations::shape_processor::ShapeProcessor; use fj_viewer::{RendererInitError, StatusReport, Viewer}; use futures::executor::block_on; use tracing::trace; -use winit::event_loop::EventLoop; +use winit::event_loop::EventLoopBuilder; use crate::{ event_loop_handler::{self, EventLoopHandler}, @@ -26,19 +26,23 @@ pub fn run( shape_processor: ShapeProcessor, invert_zoom: bool, ) -> Result<(), Error> { - let event_loop = EventLoop::new(); + let event_loop = EventLoopBuilder::::with_user_event().build(); let window = Window::new(&event_loop)?; let viewer = block_on(Viewer::new(&window))?; let egui_winit_state = egui_winit::State::new(&event_loop); + let event_loop_proxy = event_loop.create_proxy(); let host = model - .map(|model| Host::new(model, shape_processor.clone())) + .map(|model| { + Host::new(model, shape_processor.clone(), event_loop_proxy.clone()) + }) .transpose()?; let mut handler = EventLoopHandler { invert_zoom, shape_processor, + event_loop_proxy, window, viewer, egui_winit_state, From fa7abfcae16bfc135d74d0c002ddc6ff4b369752 Mon Sep 17 00:00:00 2001 From: Zach Thompson Date: Fri, 30 Dec 2022 21:32:38 -0500 Subject: [PATCH 3/5] Remove model processing from event loop The event loop is no longer blocked by model processing. Status updates are initiated with `EventLoopProxy` from a background thread. Separate status updates occur for 'initial load' and 'change detected' of a model. --- Cargo.lock | 1 + crates/fj-host/Cargo.toml | 1 + crates/fj-host/src/evaluator.rs | 83 ++++++++++++--------- crates/fj-host/src/host.rs | 13 +--- crates/fj-host/src/watcher.rs | 18 +---- crates/fj-window/src/event_loop_handler.rs | 87 +++++++++------------- 6 files changed, 90 insertions(+), 113 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 935556179..0cbff467b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1175,6 +1175,7 @@ dependencies = [ "cargo_metadata", "crossbeam-channel", "fj", + "fj-interop", "fj-operations", "libloading", "notify", diff --git a/crates/fj-host/Cargo.toml b/crates/fj-host/Cargo.toml index 86c555170..f47e374cf 100644 --- a/crates/fj-host/Cargo.toml +++ b/crates/fj-host/Cargo.toml @@ -15,6 +15,7 @@ categories.workspace = true cargo_metadata = "0.15.2" crossbeam-channel = "0.5.6" fj.workspace = true +fj-interop.workspace = true fj-operations.workspace = true libloading = "0.7.4" notify = "5.0.0" diff --git a/crates/fj-host/src/evaluator.rs b/crates/fj-host/src/evaluator.rs index 9e0633d83..686fa7628 100644 --- a/crates/fj-host/src/evaluator.rs +++ b/crates/fj-host/src/evaluator.rs @@ -1,17 +1,15 @@ use std::thread; -use crossbeam_channel::{Receiver, SendError, Sender}; +use crossbeam_channel::Sender; +use fj_interop::processed_shape::ProcessedShape; use fj_operations::shape_processor::ShapeProcessor; -use winit::event_loop::EventLoopProxy; +use winit::event_loop::{EventLoopClosed, EventLoopProxy}; -use crate::{Error, Evaluation, Model}; +use crate::{Error, Model}; /// Evaluates a model in a background thread pub struct Evaluator { - shape_processor: ShapeProcessor, - event_loop_proxy: EventLoopProxy, trigger_tx: Sender, - event_rx: Receiver, } impl Evaluator { @@ -21,34 +19,24 @@ impl Evaluator { shape_processor: ShapeProcessor, event_loop_proxy: EventLoopProxy, ) -> Self { - let (event_tx, event_rx) = crossbeam_channel::bounded(0); let (trigger_tx, trigger_rx) = crossbeam_channel::bounded(0); thread::spawn(move || { + if let Err(EventLoopClosed(..)) = + event_loop_proxy.send_event(ModelEvent::StartWatching) + { + return; + } + evaluate_model(&model, &shape_processor, &event_loop_proxy); + while matches!(trigger_rx.recv(), Ok(TriggerEvaluation)) { - if let Err(SendError(_)) = - event_tx.send(ModelEvent::ChangeDetected) + if let Err(EventLoopClosed(..)) = + event_loop_proxy.send_event(ModelEvent::ChangeDetected) { - break; + return; } - let evaluation = match model.evaluate() { - Ok(evaluation) => evaluation, - Err(err) => { - if let Err(SendError(_)) = - event_tx.send(ModelEvent::Error(err)) - { - break; - } - continue; - } - }; - - if let Err(SendError(_)) = - event_tx.send(ModelEvent::Evaluation(evaluation)) - { - break; - }; + evaluate_model(&model, &shape_processor, &event_loop_proxy); } // The channel is disconnected, which means this instance of @@ -57,10 +45,7 @@ impl Evaluator { }); Self { - shape_processor, - event_loop_proxy, trigger_tx, - event_rx, } } @@ -68,10 +53,34 @@ impl Evaluator { pub fn trigger(&self) -> Sender { self.trigger_tx.clone() } +} + +pub fn evaluate_model( + model: &Model, + shape_processor: &ShapeProcessor, + event_loop_proxy: &EventLoopProxy, +) { + let evaluation = match model.evaluate() { + Ok(evaluation) => evaluation, + + Err(err) => { + if let Err(EventLoopClosed(..)) = + event_loop_proxy.send_event(ModelEvent::Error(err)) + { + panic!(); + } + return; + } + }; + + event_loop_proxy.send_event(ModelEvent::Evaluated).unwrap(); - /// Access a channel for receiving status updates - pub fn events(&self) -> Receiver { - self.event_rx.clone() + let shape = shape_processor.process(&evaluation.shape).unwrap(); + + if let Err(EventLoopClosed(..)) = + event_loop_proxy.send_event(ModelEvent::ProcessedShape(shape)) + { + panic!(); } } @@ -81,11 +90,17 @@ pub struct TriggerEvaluation; /// An event emitted by [`Evaluator`] #[derive(Debug)] pub enum ModelEvent { + /// A new model is being watched + StartWatching, + /// A change in the model has been detected ChangeDetected, /// The model has been evaluated - Evaluation(Evaluation), + Evaluated, + + /// The model has been processed into a `Shape` + ProcessedShape(ProcessedShape), /// An error Error(Error), diff --git a/crates/fj-host/src/host.rs b/crates/fj-host/src/host.rs index 101103f34..e6e98ed9c 100644 --- a/crates/fj-host/src/host.rs +++ b/crates/fj-host/src/host.rs @@ -1,4 +1,3 @@ -use crossbeam_channel::Receiver; use fj_operations::shape_processor::ShapeProcessor; use winit::event_loop::EventLoopProxy; @@ -6,7 +5,7 @@ use crate::{Error, Evaluator, Model, ModelEvent, Watcher}; /// A Fornjot model host pub struct Host { - evaluator: Evaluator, + _evaluator: Evaluator, _watcher: Watcher, } @@ -21,17 +20,13 @@ impl Host { event_loop_proxy: EventLoopProxy, ) -> Result { let watch_path = model.watch_path(); - let evaluator = Evaluator::new(model, shape_processor, event_loop_proxy); + let evaluator = + Evaluator::new(model, shape_processor, event_loop_proxy); let watcher = Watcher::watch_model(watch_path, &evaluator)?; Ok(Self { - evaluator, + _evaluator: evaluator, _watcher: watcher, }) } - - /// Access a channel with evaluation events - pub fn events(&self) -> Receiver { - self.evaluator.events() - } } diff --git a/crates/fj-host/src/watcher.rs b/crates/fj-host/src/watcher.rs index 4f56ff4bb..a49dfc10f 100644 --- a/crates/fj-host/src/watcher.rs +++ b/crates/fj-host/src/watcher.rs @@ -1,4 +1,4 @@ -use std::{collections::HashSet, ffi::OsStr, path::Path, thread}; +use std::{collections::HashSet, ffi::OsStr, path::Path}; use notify::Watcher as _; @@ -18,7 +18,6 @@ impl Watcher { let watch_path = watch_path.as_ref(); let watch_tx = evaluator.trigger(); - let watch_tx_2 = evaluator.trigger(); let mut watcher = notify::recommended_watcher( move |event: notify::Result| { @@ -68,21 +67,6 @@ impl Watcher { watcher.watch(watch_path, notify::RecursiveMode::Recursive)?; - // To prevent a race condition between the initial load and the start of - // watching, we'll trigger the initial load here, after having started - // watching. - // - // This happens in a separate thread, because the channel is bounded and - // has no buffer. - // - // Will panic, if the receiving end has panicked. Not much we can do - // about that, if it happened. - thread::spawn(move || { - watch_tx_2 - .send(TriggerEvaluation) - .expect("Channel is disconnected"); - }); - Ok(Self { _watcher: Box::new(watcher), }) diff --git a/crates/fj-window/src/event_loop_handler.rs b/crates/fj-window/src/event_loop_handler.rs index c3d2ed1ed..1855d4a66 100644 --- a/crates/fj-window/src/event_loop_handler.rs +++ b/crates/fj-window/src/event_loop_handler.rs @@ -41,48 +41,6 @@ impl EventLoopHandler { event: Event, control_flow: &mut ControlFlow, ) -> Result<(), Error> { - if let Some(host) = &self.host { - loop { - let events = host.events(); - let event = events - .try_recv() - .map_err(|err| { - assert!( - !err.is_disconnected(), - "Expected channel to never disconnect" - ); - }) - .ok(); - - let Some(event) = event else { - break - }; - - match event { - ModelEvent::ChangeDetected => { - self.status.update_status( - "Change in model detected. Evaluating model...", - ); - } - ModelEvent::Evaluation(evaluation) => { - self.status.update_status( - "Model evaluated. Processing model...", - ); - - let shape = - self.shape_processor.process(&evaluation.shape)?; - self.viewer.handle_shape_update(shape); - - self.status.update_status("Model processed."); - } - - ModelEvent::Error(err) => { - return Err(err.into()); - } - } - } - } - if let Event::WindowEvent { event, .. } = &event { let egui_winit::EventResponse { consumed, @@ -100,8 +58,42 @@ impl EventLoopHandler { } } + let input_event = input_event( + &event, + &self.window, + &self.held_mouse_button, + &mut self.viewer.cursor, + self.invert_zoom, + ); + if let Some(input_event) = input_event { + self.viewer.handle_input_event(input_event); + } + // fj-window events match event { + Event::UserEvent(event) => match event { + ModelEvent::StartWatching => { + self.status + .update_status("New model loaded. Evaluating model..."); + } + ModelEvent::ChangeDetected => { + self.status.update_status( + "Change in model detected. Evaluating model...", + ); + } + ModelEvent::Evaluated => { + self.status + .update_status("Model evaluated. Processing model..."); + } + ModelEvent::ProcessedShape(shape) => { + self.viewer.handle_shape_update(shape); + self.status.update_status("Model processed."); + } + + ModelEvent::Error(err) => { + return Err(err.into()); + } + }, Event::WindowEvent { event: WindowEvent::CloseRequested, .. @@ -206,17 +198,6 @@ impl EventLoopHandler { _ => {} } - let input_event = input_event( - &event, - &self.window, - &self.held_mouse_button, - &mut self.viewer.cursor, - self.invert_zoom, - ); - if let Some(input_event) = input_event { - self.viewer.handle_input_event(input_event); - } - Ok(()) } } From e2a1a92ad64380cfbf30061d8aa8ef678968370d Mon Sep 17 00:00:00 2001 From: Zach Thompson Date: Sun, 1 Jan 2023 13:08:35 -0500 Subject: [PATCH 4/5] Propagate panics from evaluator thread This commit also removes `Evaluator` and replaces it with free functions. --- crates/fj-host/src/evaluator.rs | 69 ++++++++-------------- crates/fj-host/src/host.rs | 44 ++++++++++++-- crates/fj-host/src/lib.rs | 2 +- crates/fj-host/src/watcher.rs | 9 ++- crates/fj-window/src/event_loop_handler.rs | 4 ++ 5 files changed, 72 insertions(+), 56 deletions(-) diff --git a/crates/fj-host/src/evaluator.rs b/crates/fj-host/src/evaluator.rs index 686fa7628..5bf4a6bc7 100644 --- a/crates/fj-host/src/evaluator.rs +++ b/crates/fj-host/src/evaluator.rs @@ -1,4 +1,4 @@ -use std::thread; +use std::thread::{self, JoinHandle}; use crossbeam_channel::Sender; use fj_interop::processed_shape::ProcessedShape; @@ -7,21 +7,17 @@ use winit::event_loop::{EventLoopClosed, EventLoopProxy}; use crate::{Error, Model}; -/// Evaluates a model in a background thread -pub struct Evaluator { - trigger_tx: Sender, -} - -impl Evaluator { - /// Create an `Evaluator` from a model - pub fn new( - model: Model, - shape_processor: ShapeProcessor, - event_loop_proxy: EventLoopProxy, - ) -> Self { - let (trigger_tx, trigger_rx) = crossbeam_channel::bounded(0); - - thread::spawn(move || { +/// Start a background thread for evaluating a model +pub fn spawn_evaluator( + model: Model, + shape_processor: ShapeProcessor, + event_loop_proxy: EventLoopProxy, +) -> (JoinHandle<()>, Sender) { + let (trigger_tx, trigger_rx) = crossbeam_channel::bounded(0); + + let join_handle = thread::Builder::new() + .name("evaluator".to_string()) + .spawn(move || { if let Err(EventLoopClosed(..)) = event_loop_proxy.send_event(ModelEvent::StartWatching) { @@ -35,53 +31,38 @@ impl Evaluator { { return; } - evaluate_model(&model, &shape_processor, &event_loop_proxy); } + }) + .expect("Cannot create thread in evaluator"); - // The channel is disconnected, which means this instance of - // `Evaluator`, as well as all `Sender`s created from it, have been - // dropped. We're done. - }); - - Self { - trigger_tx, - } - } - - /// Access a channel for triggering evaluations - pub fn trigger(&self) -> Sender { - self.trigger_tx.clone() - } + (join_handle, trigger_tx) } -pub fn evaluate_model( +fn evaluate_model( model: &Model, shape_processor: &ShapeProcessor, event_loop_proxy: &EventLoopProxy, ) { let evaluation = match model.evaluate() { Ok(evaluation) => evaluation, - Err(err) => { - if let Err(EventLoopClosed(..)) = - event_loop_proxy.send_event(ModelEvent::Error(err)) - { - panic!(); - } + event_loop_proxy + .send_event(ModelEvent::Error(err)) + .expect("Event loop proxy closed"); return; } }; - event_loop_proxy.send_event(ModelEvent::Evaluated).unwrap(); + event_loop_proxy + .send_event(ModelEvent::Evaluated) + .expect("Event loop proxy closed"); let shape = shape_processor.process(&evaluation.shape).unwrap(); - if let Err(EventLoopClosed(..)) = - event_loop_proxy.send_event(ModelEvent::ProcessedShape(shape)) - { - panic!(); - } + event_loop_proxy + .send_event(ModelEvent::ProcessedShape(shape)) + .expect("Event loop proxy closed"); } /// Command received by [`Evaluator`] through its channel diff --git a/crates/fj-host/src/host.rs b/crates/fj-host/src/host.rs index e6e98ed9c..775513a34 100644 --- a/crates/fj-host/src/host.rs +++ b/crates/fj-host/src/host.rs @@ -1,11 +1,13 @@ +use std::thread::JoinHandle; + use fj_operations::shape_processor::ShapeProcessor; use winit::event_loop::EventLoopProxy; -use crate::{Error, Evaluator, Model, ModelEvent, Watcher}; +use crate::{spawn_evaluator, Error, Model, ModelEvent, Watcher}; /// A Fornjot model host pub struct Host { - _evaluator: Evaluator, + evaluator_thread: Option>, _watcher: Watcher, } @@ -20,13 +22,43 @@ impl Host { event_loop_proxy: EventLoopProxy, ) -> Result { let watch_path = model.watch_path(); - let evaluator = - Evaluator::new(model, shape_processor, event_loop_proxy); - let watcher = Watcher::watch_model(watch_path, &evaluator)?; + let (evaluator_thread, trigger_tx) = + spawn_evaluator(model, shape_processor, event_loop_proxy); + let watcher = Watcher::watch_model(watch_path, trigger_tx)?; Ok(Self { - _evaluator: evaluator, + evaluator_thread: Some(evaluator_thread), _watcher: watcher, }) } + + /// Check if the evaluator thread has exited with a panic. + /// + /// # Panics + /// + /// Panics if the evaluator thread has panicked. + pub fn propagate_panic(&mut self) { + if self.evaluator_thread.is_none() { + unreachable!("Constructor requires host thread") + } + if let Some(evaluator_thread) = &self.evaluator_thread { + // The host thread should not finish while this handle holds the + // `command_tx` channel open, so an exit means the thread panicked. + if evaluator_thread.is_finished() { + let evaluator_thread = self.evaluator_thread.take().unwrap(); + match evaluator_thread.join() { + Ok(()) => { + unreachable!( + "Evaluator thread cannot exit until host handle disconnects" + ) + } + // The error value has already been reported by the panic + // in the host thread, so just ignore it here. + Err(_) => { + panic!("Evaluator thread panicked") + } + } + } + } + } } diff --git a/crates/fj-host/src/lib.rs b/crates/fj-host/src/lib.rs index 1e6526830..f0fb318ec 100644 --- a/crates/fj-host/src/lib.rs +++ b/crates/fj-host/src/lib.rs @@ -23,7 +23,7 @@ mod platform; mod watcher; pub use self::{ - evaluator::{Evaluator, ModelEvent}, + evaluator::{spawn_evaluator, ModelEvent}, host::Host, model::{Error, Evaluation, Model}, parameters::Parameters, diff --git a/crates/fj-host/src/watcher.rs b/crates/fj-host/src/watcher.rs index a49dfc10f..56c5f4d95 100644 --- a/crates/fj-host/src/watcher.rs +++ b/crates/fj-host/src/watcher.rs @@ -1,8 +1,9 @@ use std::{collections::HashSet, ffi::OsStr, path::Path}; +use crossbeam_channel::Sender; use notify::Watcher as _; -use crate::{evaluator::TriggerEvaluation, Error, Evaluator}; +use crate::{evaluator::TriggerEvaluation, Error}; /// Watches a model for changes, reloading it continually pub struct Watcher { @@ -13,12 +14,10 @@ impl Watcher { /// Watch the provided model for changes pub fn watch_model( watch_path: impl AsRef, - evaluator: &Evaluator, + trigger_tx: Sender, ) -> Result { let watch_path = watch_path.as_ref(); - let watch_tx = evaluator.trigger(); - let mut watcher = notify::recommended_watcher( move |event: notify::Result| { // Unfortunately the `notify` documentation doesn't say when @@ -58,7 +57,7 @@ impl Watcher { // application is being shut down. // // Either way, not much we can do about it here. - watch_tx + trigger_tx .send(TriggerEvaluation) .expect("Channel is disconnected"); } diff --git a/crates/fj-window/src/event_loop_handler.rs b/crates/fj-window/src/event_loop_handler.rs index 1855d4a66..47dd6adc1 100644 --- a/crates/fj-window/src/event_loop_handler.rs +++ b/crates/fj-window/src/event_loop_handler.rs @@ -41,6 +41,10 @@ impl EventLoopHandler { event: Event, control_flow: &mut ControlFlow, ) -> Result<(), Error> { + if let Some(host) = &mut self.host { + host.propagate_panic(); + } + if let Event::WindowEvent { event, .. } = &event { let egui_winit::EventResponse { consumed, From db26e636f75535a1d968d6c439c7f7e05104ca8d Mon Sep 17 00:00:00 2001 From: Zach Thompson Date: Tue, 3 Jan 2023 10:08:50 -0500 Subject: [PATCH 5/5] Fix documentation --- crates/fj-host/src/evaluator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-host/src/evaluator.rs b/crates/fj-host/src/evaluator.rs index 5bf4a6bc7..f5811c1ff 100644 --- a/crates/fj-host/src/evaluator.rs +++ b/crates/fj-host/src/evaluator.rs @@ -65,10 +65,10 @@ fn evaluate_model( .expect("Event loop proxy closed"); } -/// Command received by [`Evaluator`] through its channel +/// Command received by an evaluator thread through its channel pub struct TriggerEvaluation; -/// An event emitted by [`Evaluator`] +/// An event emitted by an evaluator thread #[derive(Debug)] pub enum ModelEvent { /// A new model is being watched