From 93b8d0db17058b63478b710e8613c63c6c86e829 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 13:17:54 +0100 Subject: [PATCH 01/17] Add `Model::watch` --- fj-app/src/main.rs | 64 +------------------------------- fj-app/src/model.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 65 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index e4385dbcd..d8c8b0b6c 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -7,8 +7,6 @@ mod mesh; mod model; mod window; -use std::collections::HashSet; -use std::ffi::OsStr; use std::path::PathBuf; use std::{collections::HashMap, sync::mpsc, time::Instant}; @@ -16,7 +14,6 @@ use fj_debug::DebugInfo; use fj_math::{Aabb, Scalar, Triangle}; use fj_operations::ToShape as _; use futures::executor::block_on; -use notify::Watcher as _; use tracing::trace; use tracing_subscriber::fmt::format; use tracing_subscriber::EnvFilter; @@ -132,66 +129,7 @@ fn main() -> anyhow::Result<()> { } let (watcher_tx, watcher_rx) = mpsc::sync_channel(0); - - let watch_path = model.src_path(); - let mut watcher = notify::recommended_watcher( - move |event: notify::Result| { - // Unfortunately the `notify` documentation doesn't say when this - // might happen, so no idea if it needs to be handled. - let event = event.expect("Error handling watch event"); - - //Various acceptable ModifyKind kinds. Varies across platforms (e.g. MacOs vs. Windows10) - if let notify::EventKind::Modify(notify::event::ModifyKind::Any) - | notify::EventKind::Modify(notify::event::ModifyKind::Data( - notify::event::DataChange::Any, - )) - | notify::EventKind::Modify(notify::event::ModifyKind::Data( - notify::event::DataChange::Content, - )) = event.kind - { - let file_ext = event - .paths - .get(0) - .expect("File path missing in watch event") - .extension(); - - let black_list = HashSet::from([ - OsStr::new("swp"), - OsStr::new("tmp"), - OsStr::new("swx"), - ]); - - if let Some(ext) = file_ext { - if black_list.contains(ext) { - return; - } - } - - let shape = match model.load(¶meters) { - Ok(shape) => shape, - Err(model::Error::Compile) => { - // It would be better to display an error in the UI, - // where the user can actually see it. Issue: - // https://github.com/hannobraun/fornjot/issues/30 - println!("Error compiling model"); - return; - } - Err(err) => { - panic!("Error reloading model: {:?}", err); - } - }; - - // This will panic, if the other end is disconnected, which is - // probably the result of a panic on that thread, or the - // application is being shut down. - // - // Either way, not much we can do about it here, except maybe to - // provide a better error message in the future. - watcher_tx.send(shape).unwrap(); - } - }, - )?; - watcher.watch(&watch_path, notify::RecursiveMode::Recursive)?; + let _watcher = model.watch(watcher_tx, parameters)?; let event_loop = EventLoop::new(); let window = Window::new(&event_loop); diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index 74000bc00..f6d6de39f 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -1,5 +1,13 @@ -use std::{collections::HashMap, io, path::PathBuf, process::Command}; - +use std::{ + collections::{HashMap, HashSet}, + ffi::OsStr, + io, + path::PathBuf, + process::Command, + sync::mpsc, +}; + +use notify::Watcher as _; use thiserror::Error; pub struct Model { @@ -90,6 +98,83 @@ impl Model { Ok(shape) } + + pub fn watch( + self, + tx: mpsc::SyncSender, + parameters: HashMap, + ) -> notify::Result { + let watch_path = self.src_path(); + + let mut watcher = notify::recommended_watcher( + move |event: notify::Result| { + // Unfortunately the `notify` documentation doesn't say when + // this might happen, so no idea if it needs to be handled. + let event = event.expect("Error handling watch event"); + + // Various acceptable ModifyKind kinds. Varies across platforms + // (e.g. MacOs vs. Windows10) + if let notify::EventKind::Modify( + notify::event::ModifyKind::Any, + ) + | notify::EventKind::Modify( + notify::event::ModifyKind::Data( + notify::event::DataChange::Any, + ), + ) + | notify::EventKind::Modify( + notify::event::ModifyKind::Data( + notify::event::DataChange::Content, + ), + ) = event.kind + { + let file_ext = event + .paths + .get(0) + .expect("File path missing in watch event") + .extension(); + + let black_list = HashSet::from([ + OsStr::new("swp"), + OsStr::new("tmp"), + OsStr::new("swx"), + ]); + + if let Some(ext) = file_ext { + if black_list.contains(ext) { + return; + } + } + + let shape = match self.load(¶meters) { + Ok(shape) => shape, + Err(Error::Compile) => { + // It would be better to display an error in the UI, + // where the user can actually see it. Issue: + // https://github.com/hannobraun/fornjot/issues/30 + println!("Error compiling model"); + return; + } + Err(err) => { + panic!("Error reloading model: {:?}", err); + } + }; + + // This will panic, if the other end is disconnected, which + // is probably the result of a panic on that thread, or the + // application is being shut down. + // + // Either way, not much we can do about it here, except + // maybe to provide a better error message in the future. + tx.send(shape).unwrap(); + } + }, + )?; + + watcher.watch(&watch_path, notify::RecursiveMode::Recursive)?; + + Ok(watcher) + } } #[derive(Debug, Error)] From 3b0bbf969e3b42ca9e4e40ebd604519caf3fb8b7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 13:19:47 +0100 Subject: [PATCH 02/17] Inline method call --- fj-app/src/model.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index f6d6de39f..67263215e 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -55,10 +55,6 @@ impl Model { }) } - pub fn src_path(&self) -> PathBuf { - self.src_path.clone() - } - pub fn load( &self, arguments: &HashMap, @@ -104,7 +100,7 @@ impl Model { tx: mpsc::SyncSender, parameters: HashMap, ) -> notify::Result { - let watch_path = self.src_path(); + let watch_path = self.src_path.clone(); let mut watcher = notify::recommended_watcher( move |event: notify::Result| { From da3c599cbbee705e5d76adca9cc144b8e35b7201 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 13:24:34 +0100 Subject: [PATCH 03/17] Make error returned by `Model::watch` more general --- fj-app/src/model.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index 67263215e..c426d59d7 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -99,7 +99,7 @@ impl Model { self, tx: mpsc::SyncSender, parameters: HashMap, - ) -> notify::Result { + ) -> Result { let watch_path = self.src_path.clone(); let mut watcher = notify::recommended_watcher( @@ -183,6 +183,9 @@ pub enum Error { #[error("Error loading model from dynamic library")] LibLoading(#[from] libloading::Error), + + #[error("Error watching model for changes")] + Notify(#[from] notify::Error), } type ModelFn = From 22944676ba17e2298c7657faa4531bccfca8012a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 14:16:16 +0100 Subject: [PATCH 04/17] Add separate shape loading code path for export This enables some fixes and cleanups, as both code paths can now be changed independently. --- fj-app/src/main.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index d8c8b0b6c..ba1938006 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -92,9 +92,11 @@ fn main() -> anyhow::Result<()> { let shape = model.load(¶meters)?; let shape_processor = ShapeProcessor::new(args.tolerance)?; - let mut processed_shape = shape_processor.process(&shape); if let Some(path) = args.export { + let shape = model.load(¶meters)?; + let processed_shape = shape_processor.process(&shape); + let mut mesh_maker = MeshMaker::new(); for triangle in processed_shape.triangles { @@ -139,6 +141,7 @@ fn main() -> anyhow::Result<()> { let mut input_handler = input::Handler::new(previous_time); let mut renderer = block_on(Renderer::new(&window))?; + let mut processed_shape = shape_processor.process(&shape); processed_shape.update_geometry(&mut renderer); let mut draw_config = DrawConfig::default(); From 4e46824e779b90605ac53f40d5157bbe61d9bb9f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 14:18:20 +0100 Subject: [PATCH 05/17] Simplify variable name --- fj-app/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index ba1938006..95afd60ad 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -95,11 +95,11 @@ fn main() -> anyhow::Result<()> { if let Some(path) = args.export { let shape = model.load(¶meters)?; - let processed_shape = shape_processor.process(&shape); + let shape = shape_processor.process(&shape); let mut mesh_maker = MeshMaker::new(); - for triangle in processed_shape.triangles { + for triangle in shape.triangles { for vertex in triangle.points() { mesh_maker.push(vertex); } From c2dbbf36c078533705458c28dd5c2f138a830474 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:03:26 +0100 Subject: [PATCH 06/17] Add `model::Watcher` --- fj-app/src/main.rs | 22 +++++----------------- fj-app/src/model.rs | 32 +++++++++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index 95afd60ad..b3cbbeda0 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -8,7 +8,7 @@ mod model; mod window; use std::path::PathBuf; -use std::{collections::HashMap, sync::mpsc, time::Instant}; +use std::{collections::HashMap, time::Instant}; use fj_debug::DebugInfo; use fj_math::{Aabb, Scalar, Triangle}; @@ -130,8 +130,7 @@ fn main() -> anyhow::Result<()> { return Ok(()); } - let (watcher_tx, watcher_rx) = mpsc::sync_channel(0); - let _watcher = model.watch(watcher_tx, parameters)?; + let watcher = model.watch(parameters)?; let event_loop = EventLoop::new(); let window = Window::new(&event_loop); @@ -154,20 +153,9 @@ fn main() -> anyhow::Result<()> { let now = Instant::now(); - match watcher_rx.try_recv() { - Ok(shape) => { - processed_shape = shape_processor.process(&shape); - processed_shape.update_geometry(&mut renderer); - } - Err(mpsc::TryRecvError::Empty) => { - // Nothing to receive from the channel. We don't care. - } - Err(mpsc::TryRecvError::Disconnected) => { - // The other end has disconnected. This is probably the result - // of a panic on the other thread, or a program shutdown in - // progress. In any case, not much we can do here. - panic!(); - } + if let Some(shape) = watcher.receive() { + processed_shape = shape_processor.process(&shape); + processed_shape.update_geometry(&mut renderer); } match event { diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index c426d59d7..89b5dcd68 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -97,9 +97,9 @@ impl Model { pub fn watch( self, - tx: mpsc::SyncSender, parameters: HashMap, - ) -> Result { + ) -> Result { + let (tx, rx) = mpsc::sync_channel(0); let watch_path = self.src_path.clone(); let mut watcher = notify::recommended_watcher( @@ -169,7 +169,33 @@ impl Model { watcher.watch(&watch_path, notify::RecursiveMode::Recursive)?; - Ok(watcher) + Ok(Watcher { + _watcher: Box::new(watcher), + channel: rx, + }) + } +} + +pub struct Watcher { + _watcher: Box, + channel: mpsc::Receiver, +} + +impl Watcher { + pub fn receive(&self) -> Option { + match self.channel.try_recv() { + Ok(shape) => Some(shape), + Err(mpsc::TryRecvError::Empty) => { + // Nothing to receive from the channel. + None + } + Err(mpsc::TryRecvError::Disconnected) => { + // The other end has disconnected. This is probably the result + // of a panic on the other thread, or a program shutdown in + // progress. In any case, not much we can do here. + panic!(); + } + } } } From 3a35b4b803b5ce4a32ac71d06398faf613c58a96 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:08:00 +0100 Subject: [PATCH 07/17] Move model loading into `Watcher` This is slightly more complex, but makes it easier to trigger a model load from other sources than the watcher thread. --- fj-app/src/model.rs | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index 89b5dcd68..6020ffb3f 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -142,27 +142,13 @@ impl Model { } } - let shape = match self.load(¶meters) { - Ok(shape) => shape, - Err(Error::Compile) => { - // It would be better to display an error in the UI, - // where the user can actually see it. Issue: - // https://github.com/hannobraun/fornjot/issues/30 - println!("Error compiling model"); - return; - } - Err(err) => { - panic!("Error reloading model: {:?}", err); - } - }; - // This will panic, if the other end is disconnected, which // is probably the result of a panic on that thread, or the // application is being shut down. // // Either way, not much we can do about it here, except // maybe to provide a better error message in the future. - tx.send(shape).unwrap(); + tx.send(()).unwrap(); } }, )?; @@ -172,19 +158,39 @@ impl Model { Ok(Watcher { _watcher: Box::new(watcher), channel: rx, + model: self, + parameters, }) } } pub struct Watcher { _watcher: Box, - channel: mpsc::Receiver, + channel: mpsc::Receiver<()>, + model: Model, + parameters: HashMap, } impl Watcher { pub fn receive(&self) -> Option { match self.channel.try_recv() { - Ok(shape) => Some(shape), + Ok(()) => { + let shape = match self.model.load(&self.parameters) { + Ok(shape) => shape, + Err(Error::Compile) => { + // It would be better to display an error in the UI, + // where the user can actually see it. Issue: + // https://github.com/hannobraun/fornjot/issues/30 + println!("Error compiling model"); + return None; + } + Err(err) => { + panic!("Error reloading model: {:?}", err); + } + }; + + Some(shape) + } Err(mpsc::TryRecvError::Empty) => { // Nothing to receive from the channel. None From 1227f69d0620febda75284e17215d0e1d80b0192 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:13:29 +0100 Subject: [PATCH 08/17] Prepare main loop for not having camera available This will help with a fix I'm working on. --- fj-app/src/main.rs | 61 ++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index b3cbbeda0..ccb063e5b 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -144,7 +144,7 @@ fn main() -> anyhow::Result<()> { processed_shape.update_geometry(&mut renderer); let mut draw_config = DrawConfig::default(); - let mut camera = Camera::new(&processed_shape.aabb); + let mut camera = Some(Camera::new(&processed_shape.aabb)); event_loop.run(move |event, _, control_flow| { trace!("Handling event: {:?}", event); @@ -181,23 +181,28 @@ fn main() -> anyhow::Result<()> { event: WindowEvent::CursorMoved { position, .. }, .. } => { - input_handler.handle_cursor_moved( - position, - &mut camera, - &window, - ); + if let Some(camera) = &mut camera { + input_handler + .handle_cursor_moved(position, camera, &window); + } } Event::WindowEvent { event: WindowEvent::MouseInput { state, button, .. }, .. } => { - let focus_point = camera.focus_point( - &window, - input_handler.cursor(), - &processed_shape.triangles, - ); - - input_handler.handle_mouse_input(button, state, focus_point); + if let Some(camera) = &camera { + let focus_point = camera.focus_point( + &window, + input_handler.cursor(), + &processed_shape.triangles, + ); + + input_handler.handle_mouse_input( + button, + state, + focus_point, + ); + } } Event::WindowEvent { event: WindowEvent::MouseWheel { delta, .. }, @@ -209,23 +214,27 @@ fn main() -> anyhow::Result<()> { let delta_t = now.duration_since(previous_time); previous_time = now; - input_handler.update( - delta_t.as_secs_f64(), - now, - &mut camera, - &window, - &processed_shape.triangles, - ); + if let Some(camera) = &mut camera { + input_handler.update( + delta_t.as_secs_f64(), + now, + camera, + &window, + &processed_shape.triangles, + ); + } window.inner().request_redraw(); } Event::RedrawRequested(_) => { - camera.update_planes(&processed_shape.aabb); - - match renderer.draw(&camera, &draw_config) { - Ok(()) => {} - Err(err) => { - panic!("Draw error: {}", err); + if let Some(camera) = &mut camera { + camera.update_planes(&processed_shape.aabb); + + match renderer.draw(camera, &draw_config) { + Ok(()) => {} + Err(err) => { + panic!("Draw error: {}", err); + } } } } From 6568d0c8c3559a2560606d6eee8e6528555bb82b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:27:01 +0100 Subject: [PATCH 09/17] Prevent race condition when watching model This temporarily leads to two initial loads of the model. I'm going to fix that in subsequent commits. --- fj-app/src/main.rs | 9 --------- fj-app/src/model.rs | 11 +++++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index ccb063e5b..9e68ab570 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -80,15 +80,6 @@ fn main() -> anyhow::Result<()> { parameters.insert(key, value); } - // Since we're loading the model before setting up the watcher below, - // there's a race condition, and a modification could be missed between - // those two events. - // - // This can't be addressed with the current structure, since the watcher - // closure takes ownership of the model. - // - // This is being tracked in the following issue: - // https://github.com/hannobraun/fornjot/issues/32 let shape = model.load(¶meters)?; let shape_processor = ShapeProcessor::new(args.tolerance)?; diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index 6020ffb3f..2febf8981 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -5,6 +5,7 @@ use std::{ path::PathBuf, process::Command, sync::mpsc, + thread, }; use notify::Watcher as _; @@ -100,6 +101,8 @@ impl Model { parameters: HashMap, ) -> Result { let (tx, rx) = mpsc::sync_channel(0); + let tx2 = tx.clone(); + let watch_path = self.src_path.clone(); let mut watcher = notify::recommended_watcher( @@ -155,6 +158,14 @@ impl Model { 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. + // + // Will panic, if the receiving end has panicked. Not much we can do + // about that, if it happened. + thread::spawn(move || tx2.send(()).unwrap()); + Ok(Watcher { _watcher: Box::new(watcher), channel: rx, From a34e872db4abf9fdbeab72f0b133a1b949f565e5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:28:07 +0100 Subject: [PATCH 10/17] Initialize camera on initial model load --- fj-app/src/main.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index 9e68ab570..d49281635 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -135,7 +135,7 @@ fn main() -> anyhow::Result<()> { processed_shape.update_geometry(&mut renderer); let mut draw_config = DrawConfig::default(); - let mut camera = Some(Camera::new(&processed_shape.aabb)); + let mut camera = None; event_loop.run(move |event, _, control_flow| { trace!("Handling event: {:?}", event); @@ -147,6 +147,10 @@ fn main() -> anyhow::Result<()> { if let Some(shape) = watcher.receive() { processed_shape = shape_processor.process(&shape); processed_shape.update_geometry(&mut renderer); + + if camera.is_none() { + camera = Some(Camera::new(&processed_shape.aabb)); + } } match event { From 920d4708703aa0effa5aa1630cf7e25f4828b6e9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:33:58 +0100 Subject: [PATCH 11/17] Fix double-load of model on initialization --- fj-app/src/main.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index d49281635..f47d01dc7 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -80,8 +80,6 @@ fn main() -> anyhow::Result<()> { parameters.insert(key, value); } - let shape = model.load(¶meters)?; - let shape_processor = ShapeProcessor::new(args.tolerance)?; if let Some(path) = args.export { @@ -131,8 +129,7 @@ fn main() -> anyhow::Result<()> { let mut input_handler = input::Handler::new(previous_time); let mut renderer = block_on(Renderer::new(&window))?; - let mut processed_shape = shape_processor.process(&shape); - processed_shape.update_geometry(&mut renderer); + let mut processed_shape = None; let mut draw_config = DrawConfig::default(); let mut camera = None; @@ -145,12 +142,14 @@ fn main() -> anyhow::Result<()> { let now = Instant::now(); if let Some(shape) = watcher.receive() { - processed_shape = shape_processor.process(&shape); - processed_shape.update_geometry(&mut renderer); + let shape = shape_processor.process(&shape); + shape.update_geometry(&mut renderer); if camera.is_none() { - camera = Some(Camera::new(&processed_shape.aabb)); + camera = Some(Camera::new(&shape.aabb)); } + + processed_shape = Some(shape); } match event { @@ -185,11 +184,12 @@ fn main() -> anyhow::Result<()> { event: WindowEvent::MouseInput { state, button, .. }, .. } => { - if let Some(camera) = &camera { + if let (Some(shape), Some(camera)) = (&processed_shape, &camera) + { let focus_point = camera.focus_point( &window, input_handler.cursor(), - &processed_shape.triangles, + &shape.triangles, ); input_handler.handle_mouse_input( @@ -209,21 +209,25 @@ fn main() -> anyhow::Result<()> { let delta_t = now.duration_since(previous_time); previous_time = now; - if let Some(camera) = &mut camera { + if let (Some(shape), Some(camera)) = + (&processed_shape, &mut camera) + { input_handler.update( delta_t.as_secs_f64(), now, camera, &window, - &processed_shape.triangles, + &shape.triangles, ); } window.inner().request_redraw(); } Event::RedrawRequested(_) => { - if let Some(camera) = &mut camera { - camera.update_planes(&processed_shape.aabb); + if let (Some(shape), Some(camera)) = + (&processed_shape, &mut camera) + { + camera.update_planes(&shape.aabb); match renderer.draw(camera, &draw_config) { Ok(()) => {} From e5cd42a4ad9c1ff6aa5bc30e25e757e7b5bf06f3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:35:10 +0100 Subject: [PATCH 12/17] Improve grouping of variables --- fj-app/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index f47d01dc7..522384f2f 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -129,9 +129,9 @@ fn main() -> anyhow::Result<()> { let mut input_handler = input::Handler::new(previous_time); let mut renderer = block_on(Renderer::new(&window))?; - let mut processed_shape = None; - let mut draw_config = DrawConfig::default(); + + let mut processed_shape = None; let mut camera = None; event_loop.run(move |event, _, control_flow| { From baf7dde08e4737fd41b77581ad5dc5b430001532 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:36:11 +0100 Subject: [PATCH 13/17] Rename variable --- fj-app/src/main.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index 522384f2f..ad050bf54 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -142,14 +142,14 @@ fn main() -> anyhow::Result<()> { let now = Instant::now(); if let Some(shape) = watcher.receive() { - let shape = shape_processor.process(&shape); - shape.update_geometry(&mut renderer); + let new_shape = shape_processor.process(&shape); + new_shape.update_geometry(&mut renderer); if camera.is_none() { - camera = Some(Camera::new(&shape.aabb)); + camera = Some(Camera::new(&new_shape.aabb)); } - processed_shape = Some(shape); + processed_shape = Some(new_shape); } match event { From 3a5441fff8018da39c56e93d132771c366c6ef8d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:36:55 +0100 Subject: [PATCH 14/17] Rename variable --- fj-app/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index ad050bf54..db3a2b246 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -141,8 +141,8 @@ fn main() -> anyhow::Result<()> { let now = Instant::now(); - if let Some(shape) = watcher.receive() { - let new_shape = shape_processor.process(&shape); + if let Some(new_shape) = watcher.receive() { + let new_shape = shape_processor.process(&new_shape); new_shape.update_geometry(&mut renderer); if camera.is_none() { From 3574ed401edc74d7e8f09384990e860d7c6e22ca Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:37:34 +0100 Subject: [PATCH 15/17] Simplify variable name --- fj-app/src/main.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index db3a2b246..6dc44fce0 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -131,7 +131,7 @@ fn main() -> anyhow::Result<()> { let mut draw_config = DrawConfig::default(); - let mut processed_shape = None; + let mut shape = None; let mut camera = None; event_loop.run(move |event, _, control_flow| { @@ -149,7 +149,7 @@ fn main() -> anyhow::Result<()> { camera = Some(Camera::new(&new_shape.aabb)); } - processed_shape = Some(new_shape); + shape = Some(new_shape); } match event { @@ -184,8 +184,7 @@ fn main() -> anyhow::Result<()> { event: WindowEvent::MouseInput { state, button, .. }, .. } => { - if let (Some(shape), Some(camera)) = (&processed_shape, &camera) - { + if let (Some(shape), Some(camera)) = (&shape, &camera) { let focus_point = camera.focus_point( &window, input_handler.cursor(), @@ -209,9 +208,7 @@ fn main() -> anyhow::Result<()> { let delta_t = now.duration_since(previous_time); previous_time = now; - if let (Some(shape), Some(camera)) = - (&processed_shape, &mut camera) - { + if let (Some(shape), Some(camera)) = (&shape, &mut camera) { input_handler.update( delta_t.as_secs_f64(), now, @@ -224,9 +221,7 @@ fn main() -> anyhow::Result<()> { window.inner().request_redraw(); } Event::RedrawRequested(_) => { - if let (Some(shape), Some(camera)) = - (&processed_shape, &mut camera) - { + if let (Some(shape), Some(camera)) = (&shape, &mut camera) { camera.update_planes(&shape.aabb); match renderer.draw(camera, &draw_config) { From b0d26c0dc9af7218f939487e78955090f69243bf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:38:23 +0100 Subject: [PATCH 16/17] Make method name more explicit --- fj-app/src/main.rs | 2 +- fj-app/src/model.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index 6dc44fce0..e8b3afe48 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -119,7 +119,7 @@ fn main() -> anyhow::Result<()> { return Ok(()); } - let watcher = model.watch(parameters)?; + let watcher = model.load_and_watch(parameters)?; let event_loop = EventLoop::new(); let window = Window::new(&event_loop); diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index 2febf8981..53b6c40a8 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -96,7 +96,7 @@ impl Model { Ok(shape) } - pub fn watch( + pub fn load_and_watch( self, parameters: HashMap, ) -> Result { From 0b97a1a98a0fe7423dec0dbc55de524abcdf5bad Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 18 Mar 2022 15:38:46 +0100 Subject: [PATCH 17/17] Make method name more explicit --- fj-app/src/main.rs | 2 +- fj-app/src/model.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fj-app/src/main.rs b/fj-app/src/main.rs index e8b3afe48..4ccf11734 100644 --- a/fj-app/src/main.rs +++ b/fj-app/src/main.rs @@ -83,7 +83,7 @@ fn main() -> anyhow::Result<()> { let shape_processor = ShapeProcessor::new(args.tolerance)?; if let Some(path) = args.export { - let shape = model.load(¶meters)?; + let shape = model.load_once(¶meters)?; let shape = shape_processor.process(&shape); let mut mesh_maker = MeshMaker::new(); diff --git a/fj-app/src/model.rs b/fj-app/src/model.rs index 53b6c40a8..acba8d608 100644 --- a/fj-app/src/model.rs +++ b/fj-app/src/model.rs @@ -56,7 +56,7 @@ impl Model { }) } - pub fn load( + pub fn load_once( &self, arguments: &HashMap, ) -> Result { @@ -186,7 +186,7 @@ impl Watcher { pub fn receive(&self) -> Option { match self.channel.try_recv() { Ok(()) => { - let shape = match self.model.load(&self.parameters) { + let shape = match self.model.load_once(&self.parameters) { Ok(shape) => shape, Err(Error::Compile) => { // It would be better to display an error in the UI,