From 36c7428a8f010bf0b32fda7a24e6543fc3edf85c Mon Sep 17 00:00:00 2001 From: Sam Jeeves Date: Tue, 12 Jul 2022 17:38:23 +0200 Subject: [PATCH 1/2] Fix performance regression when zooming --- crates/fj-window/src/run.rs | 51 +++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index d953eab4a..998b50a68 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -6,9 +6,9 @@ use std::error; use fj_host::Watcher; -use fj_operations::shape_processor::{ProcessedShape, ShapeProcessor}; +use fj_operations::shape_processor::ShapeProcessor; use fj_viewer::{ - camera::{Camera, FocusPoint}, + camera::Camera, graphics::{self, DrawConfig, Renderer}, input, screen::{NormalizedPosition, Screen as _, Size}, @@ -36,6 +36,7 @@ pub fn run( let mut previous_cursor = None; let mut held_mouse_button = None; + let mut focus_pending = false; let mut input_handler = input::Handler::default(); let mut renderer = block_on(Renderer::new(&window))?; @@ -110,6 +111,8 @@ pub fn run( .on_event(&renderer.egui.context, window_event); } + focus_pending |= focus_event(&event); + // fj-window events match event { Event::WindowEvent { @@ -163,6 +166,16 @@ pub fn run( }; } Event::MainEventsCleared => { + if let (Some(shape), Some(camera)) = (&shape, &mut camera) { + // Only perform focus calculation once per frame (if pending) + if focus_pending { + input_handler.focus( + camera.focus_point(previous_cursor, &shape.mesh), + ); + } + focus_pending = false; + } + window.window().request_redraw(); } Event::RedrawRequested(_) => { @@ -179,15 +192,7 @@ pub fn run( _ => {} } - // fj-viewer events - if let (Some(shape), Some(camera)) = (&shape, &mut camera) { - if let Some(focus_event) = - focus(&event, previous_cursor, shape, camera) - { - input_handler.focus(focus_event); - } - } - + // fj-viewer input events let input_event = input_event( &event, &window, @@ -253,13 +258,9 @@ fn input_event( } } -fn focus( - event: &Event<()>, - previous_cursor: Option, - shape: &ProcessedShape, - camera: &Camera, -) -> Option { - let focus_point = match event { +/// Returns true if new focus point should be calculated based on this event +fn focus_event(event: &Event<()>) -> bool { + match event { Event::WindowEvent { event: WindowEvent::MouseInput { @@ -269,19 +270,15 @@ fn focus( }, .. } => match state { - ElementState::Pressed => { - camera.focus_point(previous_cursor, &shape.mesh) - } - ElementState::Released => FocusPoint::none(), + ElementState::Pressed => true, + ElementState::Released => false, }, Event::WindowEvent { event: WindowEvent::MouseWheel { .. }, .. - } => camera.focus_point(previous_cursor, &shape.mesh), - - _ => return None, - }; - Some(focus_point) + } => true, + _ => false, + } } /// Error in main loop From 74bc14ee5d36e40a6e8d1847c1d1a15a397721bf Mon Sep 17 00:00:00 2001 From: Sam Jeeves Date: Tue, 12 Jul 2022 18:55:24 +0200 Subject: [PATCH 2/2] Moving the model works without focus point --- Cargo.lock | 1 + crates/fj-viewer/Cargo.toml | 4 +++ crates/fj-viewer/src/camera.rs | 40 ++++++++++----------- crates/fj-viewer/src/input/handler.rs | 30 ++++++---------- crates/fj-viewer/src/input/movement.rs | 24 ++++++------- crates/fj-viewer/src/input/rotation.rs | 8 ++--- crates/fj-viewer/src/input/zoom.rs | 7 ++-- crates/fj-window/src/run.rs | 49 +++++++++++++++----------- 8 files changed, 78 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33c342b93..2630dbf7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -944,6 +944,7 @@ dependencies = [ "egui-winit", "fj-interop", "fj-math", + "fj-operations", "raw-window-handle", "thiserror", "tracing", diff --git a/crates/fj-viewer/Cargo.toml b/crates/fj-viewer/Cargo.toml index fd32ff0bc..f7edecd73 100644 --- a/crates/fj-viewer/Cargo.toml +++ b/crates/fj-viewer/Cargo.toml @@ -28,6 +28,10 @@ path = "../fj-interop" version = "0.7.0" path = "../fj-math" +[dependencies.fj-operations] +version = "0.7.0" +path = "../fj-operations" + [dependencies.egui] version = "0.18.1" diff --git a/crates/fj-viewer/src/camera.rs b/crates/fj-viewer/src/camera.rs index 5698aca0d..144a07662 100644 --- a/crates/fj-viewer/src/camera.rs +++ b/crates/fj-viewer/src/camera.rs @@ -1,8 +1,8 @@ //! Viewer camera module use std::f64::consts::FRAC_PI_2; -use fj_interop::mesh::Mesh; use fj_math::{Aabb, Point, Scalar, Transform, Triangle, Vector}; +use fj_operations::shape_processor::ProcessedShape; use crate::screen::NormalizedPosition; @@ -121,21 +121,25 @@ impl Camera { pub fn focus_point( &self, cursor: Option, - mesh: &Mesh>, + shape: &ProcessedShape, ) -> FocusPoint { - let cursor = match cursor { - Some(cursor) => cursor, - None => return FocusPoint::none(), - }; + self.calculate_focus_point(cursor, shape) + .unwrap_or_else(|| FocusPoint(shape.aabb.center())) + } + fn calculate_focus_point( + &self, + cursor: Option, + shape: &ProcessedShape, + ) -> Option { // Transform camera and cursor positions to model space. let origin = self.position(); - let cursor = self.cursor_to_model_space(cursor); + let cursor = self.cursor_to_model_space(cursor?); let dir = (cursor - origin).normalize(); let mut min_t = None; - for triangle in mesh.triangles() { + for triangle in shape.mesh.triangles() { let t = Triangle::from_points(triangle.points).cast_local_ray( origin, dir, @@ -150,7 +154,7 @@ impl Camera { } } - FocusPoint(min_t.map(|t| origin + dir * t)) + Some(FocusPoint(origin + dir * min_t?)) } /// Access the transform from camera to model space. @@ -211,17 +215,9 @@ impl Camera { } } -/// The point on the model that the cursor is currently pointing at. +/// The point around which camera movement happens. /// -/// Such a point might or might not exist, depending on whether the cursor is -/// pointing at the model or not. -pub struct FocusPoint(pub Option>); - -impl FocusPoint { - /// Construct the "none" instance of `FocusPoint` - /// - /// This instance represents the case that no focus point exists. - pub fn none() -> Self { - Self(None) - } -} +/// This will be the point on the model that the cursor is currently pointing at if such a point exists, +/// falling back to the center point of the model's bounding volume otherwise. +#[derive(Clone, Copy)] +pub struct FocusPoint(pub Point<3>); diff --git a/crates/fj-viewer/src/input/handler.rs b/crates/fj-viewer/src/input/handler.rs index 71f4212bd..c444b1e62 100644 --- a/crates/fj-viewer/src/input/handler.rs +++ b/crates/fj-viewer/src/input/handler.rs @@ -5,8 +5,6 @@ use crate::camera::{Camera, FocusPoint}; /// /// Takes user input and applies them to application state. pub struct Handler { - focus_point: FocusPoint, - movement: Movement, rotation: Rotation, zoom: Zoom, @@ -14,35 +12,29 @@ pub struct Handler { impl Handler { /// Handle an input event - pub fn handle_event(&mut self, event: Event, camera: &mut Camera) { + pub fn handle_event( + &mut self, + event: Event, + focus_point: FocusPoint, + camera: &mut Camera, + ) { match event { - Event::Translate { previous, current } => self.movement.apply( - previous, - current, - &self.focus_point, - camera, - ), + Event::Translate { previous, current } => { + self.movement.apply(previous, current, focus_point, camera) + } Event::Rotation { angle_x, angle_y } => { - self.rotation - .apply(angle_x, angle_y, &self.focus_point, camera) + self.rotation.apply(angle_x, angle_y, focus_point, camera) } Event::Zoom(zoom_delta) => { - self.zoom.apply(zoom_delta, &self.focus_point, camera) + self.zoom.apply(zoom_delta, focus_point, camera) } } } - - /// A new focus point was selected (or deselected) - pub fn focus(&mut self, focus_point: FocusPoint) { - self.focus_point = focus_point; - } } impl Default for Handler { fn default() -> Self { Self { - focus_point: FocusPoint::none(), - movement: Movement, rotation: Rotation, zoom: Zoom, diff --git a/crates/fj-viewer/src/input/movement.rs b/crates/fj-viewer/src/input/movement.rs index 058093207..ed430474c 100644 --- a/crates/fj-viewer/src/input/movement.rs +++ b/crates/fj-viewer/src/input/movement.rs @@ -12,25 +12,23 @@ impl Movement { &mut self, previous: NormalizedPosition, current: NormalizedPosition, - focus_point: &FocusPoint, + focus_point: FocusPoint, camera: &mut Camera, ) { let previous = camera.cursor_to_model_space(previous); let cursor = camera.cursor_to_model_space(current); - if let Some(focus_point) = focus_point.0 { - let d1 = Point::distance(&camera.position(), &cursor); - let d2 = Point::distance(&camera.position(), &focus_point); + let d1 = Point::distance(&camera.position(), &cursor); + let d2 = Point::distance(&camera.position(), &focus_point.0); - let diff = (cursor - previous) * d2 / d1; - let offset = camera.camera_to_model().transform_vector(&diff); + let diff = (cursor - previous) * d2 / d1; + let offset = camera.camera_to_model().transform_vector(&diff); - camera.translation = camera.translation - * Transform::translation(Vector::from([ - offset.x, - offset.y, - Scalar::ZERO, - ])); - } + camera.translation = camera.translation + * Transform::translation(Vector::from([ + offset.x, + offset.y, + Scalar::ZERO, + ])); } } diff --git a/crates/fj-viewer/src/input/rotation.rs b/crates/fj-viewer/src/input/rotation.rs index ca10260c3..1076036da 100644 --- a/crates/fj-viewer/src/input/rotation.rs +++ b/crates/fj-viewer/src/input/rotation.rs @@ -1,4 +1,4 @@ -use fj_math::{Point, Transform, Vector}; +use fj_math::{Transform, Vector}; use crate::camera::{Camera, FocusPoint}; @@ -9,12 +9,10 @@ impl Rotation { &self, angle_x: f64, angle_y: f64, - focus_point: &FocusPoint, + focus_point: FocusPoint, camera: &mut Camera, ) { - let rotate_around: Vector<3> = - focus_point.0.unwrap_or_else(Point::origin).coords; - let rotate_around = Transform::translation(rotate_around); + let rotate_around = Transform::translation(focus_point.0.coords); // the model rotates not the camera, so invert the transform let camera_rotation = camera.rotation.inverse(); diff --git a/crates/fj-viewer/src/input/zoom.rs b/crates/fj-viewer/src/input/zoom.rs index c6ddcb212..f4f7a0eb1 100644 --- a/crates/fj-viewer/src/input/zoom.rs +++ b/crates/fj-viewer/src/input/zoom.rs @@ -8,13 +8,10 @@ impl Zoom { pub fn apply( &mut self, zoom_delta: f64, - focus_point: &FocusPoint, + focus_point: FocusPoint, camera: &mut Camera, ) { - let distance = match focus_point.0 { - Some(fp) => (fp - camera.position()).magnitude(), - None => camera.position().coords.magnitude(), - }; + let distance = (focus_point.0 - camera.position()).magnitude(); let displacement = zoom_delta * distance.into_f64(); camera.translation = camera.translation * Transform::translation(Vector::from([0.0, 0.0, -displacement])); diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 998b50a68..67071390a 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -36,7 +36,7 @@ pub fn run( let mut previous_cursor = None; let mut held_mouse_button = None; - let mut focus_pending = false; + let mut focus_point = None; let mut input_handler = input::Handler::default(); let mut renderer = block_on(Renderer::new(&window))?; @@ -111,8 +111,6 @@ pub fn run( .on_event(&renderer.egui.context, window_event); } - focus_pending |= focus_event(&event); - // fj-window events match event { Event::WindowEvent { @@ -166,16 +164,6 @@ pub fn run( }; } Event::MainEventsCleared => { - if let (Some(shape), Some(camera)) = (&shape, &mut camera) { - // Only perform focus calculation once per frame (if pending) - if focus_pending { - input_handler.focus( - camera.focus_point(previous_cursor, &shape.mesh), - ); - } - focus_pending = false; - } - window.window().request_redraw(); } Event::RedrawRequested(_) => { @@ -193,14 +181,32 @@ pub fn run( } // fj-viewer input events + // These can fire multiple times per frame + + if let (Some(shape), Some(camera), Some(should_focus)) = + (&shape, &camera, focus_event(&event)) + { + if should_focus { + // Don't unnecessarily recalculate focus point + if focus_point.is_none() { + focus_point = + Some(camera.focus_point(previous_cursor, shape)); + } + } else { + focus_point = None; + } + } + let input_event = input_event( &event, &window, &held_mouse_button, &mut previous_cursor, ); - if let (Some(input_event), Some(camera)) = (input_event, &mut camera) { - input_handler.handle_event(input_event, camera); + if let (Some(input_event), Some(fp), Some(camera)) = + (input_event, focus_point, &mut camera) + { + input_handler.handle_event(input_event, fp, camera); } }); } @@ -258,8 +264,9 @@ fn input_event( } } -/// Returns true if new focus point should be calculated based on this event -fn focus_event(event: &Event<()>) -> bool { +/// Returns true/false if focus point point should be created/removed +/// None means no change to focus point is needed +fn focus_event(event: &Event<()>) -> Option { match event { Event::WindowEvent { event: @@ -270,14 +277,14 @@ fn focus_event(event: &Event<()>) -> bool { }, .. } => match state { - ElementState::Pressed => true, - ElementState::Released => false, + ElementState::Pressed => Some(true), + ElementState::Released => Some(false), }, Event::WindowEvent { event: WindowEvent::MouseWheel { .. }, .. - } => true, - _ => false, + } => Some(true), + _ => None, } }