From a324a54d6412e13a4fa6244155a8e55f39fec416 Mon Sep 17 00:00:00 2001 From: Sam Jeeves Date: Sat, 2 Jul 2022 09:03:54 +0200 Subject: [PATCH 1/5] Use current camera position to scale zoom --- crates/fj-viewer/src/input/handler.rs | 27 +--- crates/fj-viewer/src/input/zoom.rs | 213 ++------------------------ 2 files changed, 23 insertions(+), 217 deletions(-) diff --git a/crates/fj-viewer/src/input/handler.rs b/crates/fj-viewer/src/input/handler.rs index a7918dbeb..50ad3a9cb 100644 --- a/crates/fj-viewer/src/input/handler.rs +++ b/crates/fj-viewer/src/input/handler.rs @@ -1,17 +1,16 @@ use std::time::Instant; use fj_interop::mesh::Mesh; -use fj_math::{Point, Transform, Vector}; - -use crate::{ - camera::Camera, - screen::{Position, Size}, -}; +use fj_math::Point; use super::{ event::KeyState, movement::Movement, rotation::Rotation, zoom::Zoom, Event, Key, }; +use crate::{ + camera::Camera, + screen::{Position, Size}, +}; /// Input handling abstraction /// @@ -39,7 +38,7 @@ impl Handler { movement: Movement::new(), rotation: Rotation::new(), - zoom: Zoom::new(now), + zoom: Zoom::new(), } } @@ -102,7 +101,7 @@ impl Handler { } Event::Scroll(delta) => { - self.zoom.push_input_delta(delta, now); + self.zoom.push(delta); } _ => {} @@ -118,17 +117,7 @@ impl Handler { size: Size, mesh: &Mesh>, ) { - let focus_point = camera.focus_point(size, self.cursor, mesh); - - self.zoom.discard_old_events(now); - self.zoom.update_speed(now, delta_t, focus_point, camera); - - camera.translation = camera.translation - * Transform::translation(Vector::from([ - 0.0, - 0.0, - -self.zoom.speed(), - ])); + self.zoom.apply_to_camera(delta_t, camera); } } diff --git a/crates/fj-viewer/src/input/zoom.rs b/crates/fj-viewer/src/input/zoom.rs index c6e13f4a5..cdd05ad0c 100644 --- a/crates/fj-viewer/src/input/zoom.rs +++ b/crates/fj-viewer/src/input/zoom.rs @@ -1,216 +1,33 @@ -use std::{ - collections::VecDeque, - time::{Duration, Instant}, -}; +use std::time::Instant; -use fj_math::Point; +use fj_math::{Transform, Vector}; -use crate::camera::{Camera, FocusPoint}; +use crate::camera::Camera; pub struct Zoom { - events: VecDeque<(Instant, f64)>, - - target_speed: f64, current_speed: f64, - - last_direction: Direction, - idle_since: Option, } impl Zoom { - pub fn new(now: Instant) -> Self { - Self { - events: VecDeque::new(), - - target_speed: 0.0, - current_speed: 0.0, - - last_direction: Direction::None, - idle_since: Some(now), - } + pub fn new() -> Self { + Self { current_speed: 0.0 } } - /// Push an input delta from the mouse wheel or track pad - /// - /// Expects the delta to be normalized, so using the mouse wheel and track - /// pad lead to the same zoom feel. - pub fn push_input_delta(&mut self, delta: f64, now: Instant) { - let new_event = delta * 0.01; - - // If this input is opposite to previous inputs, discard previous inputs - // to stop ongoing zoom. - let can_break = match self.idle_since { - Some(idle_since) => idle_since.elapsed() < BREAK_WINDOW, - None => true, // ongoing movement; can always break that - }; - if self.last_direction.is_opposite(&Direction::from(new_event)) - && can_break - { - self.events.clear(); - - // Make sure that this breaks the zoom instantly. - self.current_speed = 0.0; - - return; - } - - self.events.push_back((now, new_event)); + pub fn push(&mut self, delta: f64) { + // Accumulate all zoom inputs + self.current_speed += delta * ACCELERATION; } - /// Discard zoom events that fall out of the zoom input time window - /// - /// See [`INPUT_WINDOW`]. - pub fn discard_old_events(&mut self, now: Instant) { - while let Some((time, _)) = self.events.front() { - if now.duration_since(*time) > INPUT_WINDOW { - self.events.pop_front(); - continue; - } + pub fn apply_to_camera(&mut self, delta_t: f64, camera: &mut Camera) { + let distance: f64 = camera.position().coords.magnitude().into(); + let displacement = self.current_speed * delta_t * distance; + camera.translation = camera.translation + * Transform::translation(Vector::from([0.0, 0.0, -displacement])); - break; - } + self.current_speed = 0.; } - - /// Update the zoom speed based on active zoom events - pub fn update_speed( - &mut self, - now: Instant, - delta_t: f64, - focus_point: FocusPoint, - camera: &Camera, - ) { - self.target_speed = self.events.iter().map(|(_, event)| event).sum(); - - // Compute current speed from target speed. Gradually converge towards - // target speed, but snap to target speed once the difference becomes - // minuscule. - let speed_delta = self.target_speed - self.current_speed; - if speed_delta.abs() >= MIN_SPEED_DELTA { - let accel = speed_delta.signum() * ACCELERATION; - self.current_speed += accel * delta_t; - - // By just applying the acceleration, we can overshoot the target, - // which can cause erratic movement, as we overshoot again every - // frame. - // - // If you need to test this code, you can trigger the problem quite - // easily by setting acceleration to a really high value. - if (self.target_speed - self.current_speed).signum() - != speed_delta.signum() - { - // We overshot. Snap to target speed. - self.current_speed = self.target_speed; - } - } else { - self.current_speed = self.target_speed; - } - - // Track last zoom direction. - self.last_direction = Direction::from(self.current_speed); - - // Limit current speed, if close to focus point and zooming in. - if let Some(focus_point) = focus_point.0 { - if self.last_direction == Direction::In { - let d = Point::distance(&focus_point, &camera.position()); - self.current_speed = - -f64::min(-self.current_speed, d.into_f64() / 8.); - } - } - - // Track idle time - if self.current_speed == 0.0 { - if self.idle_since.is_none() { - self.idle_since = Some(now); - } - } else { - self.idle_since = None - } - } - - /// Access the current zoom speed - pub fn speed(&self) -> f64 { - self.current_speed - } -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum Direction { - Out, - In, - None, } -impl Direction { - fn is_opposite(&self, other: &Self) -> bool { - #[allow(clippy::match_like_matches_macro)] - match (self, other) { - (Self::Out, Self::In) => true, - (Self::In, Self::Out) => true, - _ => false, - } - } -} - -impl From for Direction { - fn from(speed: f64) -> Self { - if speed > 0.0 { - return Self::Out; - } - if speed < 0.0 { - return Self::In; - } - - Self::None - } -} - -/// Time window for active zoom events -/// -/// This is the time window during which a zoom input event still has an effect -/// on target zoom speed. -/// -/// Tuning notes: -/// - If this value is too low, the user can't accumulate many active zooming -/// events, meaning zoom speed can't get very high. -/// - If this value is too high, a single zoom event will have too long of an -/// effect, leading to spongy control behavior. -/// -/// This value should be as low as possible, giving the user precise control, -/// while still accommodating high enough zoom speeds. -const INPUT_WINDOW: Duration = Duration::from_millis(500); - -/// Time window in which opposite movement is interpreted as breaking -/// -/// Defines the time window after a movement ends, during which an input -/// opposite to the movement is interpreted as breaking, not as a new movement. -/// -/// Tuning notes: -/// - If this value is too low, zoom input intended to stop the previous -/// movement will instead start a new, opposite movement, leading to jumpy -/// zooming behavior. -/// - If this value is too high, input meant to start a new zoom movement will -/// not be detected, making zoom less controllable. -/// -/// This value should be as low as possible, while still preventing jumpy -/// zooming behavior. -const BREAK_WINDOW: Duration = Duration::from_millis(50); - -/// The minimum delta between current and target zoom speed -/// -/// If the speed delta is below this value, the current zoom speed is snapped to -/// the target zoom speed. -/// -/// Tuning notes: -/// - If this value is too low, zoom speed will technically be non-zero, even -/// though no movement is perceivable. This makes detection of last zoom speed -/// and idle time inaccurate, leading to problems. -/// - If this value is too high, zoom acceleration will jump to infinite in that -/// last moment before reaching the target speed, which can seem jarring. -/// -/// This value should be as high as possible, allowing for precise detection of -/// last zoom speed an idle time, while not causing jarring accelerations. -const MIN_SPEED_DELTA: f64 = 0.01; - /// Acceleration value for the zoom movement /// /// Tuning notes: @@ -221,4 +38,4 @@ const MIN_SPEED_DELTA: f64 = 0.01; /// /// This value should be as high as possible, while not causing jarring /// accelerations. -const ACCELERATION: f64 = 0.5; +const ACCELERATION: f64 = 1.; From 84910170eb1cc1f9eb3cf86d4cca44a03122b05a Mon Sep 17 00:00:00 2001 From: Sam Jeeves Date: Sun, 3 Jul 2022 17:56:43 +0200 Subject: [PATCH 2/5] Zoom relative to the distance from the origin --- crates/fj-viewer/src/input/zoom.rs | 32 +++++++++++++----------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/crates/fj-viewer/src/input/zoom.rs b/crates/fj-viewer/src/input/zoom.rs index cdd05ad0c..e0936208d 100644 --- a/crates/fj-viewer/src/input/zoom.rs +++ b/crates/fj-viewer/src/input/zoom.rs @@ -1,41 +1,37 @@ -use std::time::Instant; - use fj_math::{Transform, Vector}; use crate::camera::Camera; pub struct Zoom { - current_speed: f64, + accumulated_delta: f64, } impl Zoom { pub fn new() -> Self { - Self { current_speed: 0.0 } + Self { + accumulated_delta: 0.0, + } } pub fn push(&mut self, delta: f64) { // Accumulate all zoom inputs - self.current_speed += delta * ACCELERATION; + self.accumulated_delta += delta; } pub fn apply_to_camera(&mut self, delta_t: f64, camera: &mut Camera) { - let distance: f64 = camera.position().coords.magnitude().into(); - let displacement = self.current_speed * delta_t * distance; + let displacement = self.accumulated_delta + * delta_t + * ZOOM_FACTOR + * camera.position().coords.magnitude().into_f64(); camera.translation = camera.translation * Transform::translation(Vector::from([0.0, 0.0, -displacement])); - self.current_speed = 0.; + self.accumulated_delta = 0.; } } -/// Acceleration value for the zoom movement -/// -/// Tuning notes: -/// - If this value is too low, target zoom speed will be reached slowly, -/// leading to less precise control. -/// - If this value is too high, zoom movement seems unnatural, which can cause -/// a jarring experience. +/// Affects the speed of zoom movement. /// -/// This value should be as high as possible, while not causing jarring -/// accelerations. -const ACCELERATION: f64 = 1.; +/// Smaller values will move the camera less with the same input. +/// Larger values will move the camera more with the same input. +const ZOOM_FACTOR: f64 = 0.05; From be97b445d524c32a6dc8c3a566e818e6bd90722e Mon Sep 17 00:00:00 2001 From: Sam Jeeves Date: Sun, 3 Jul 2022 18:33:55 +0200 Subject: [PATCH 3/5] Remove unused function inputs --- crates/fj-viewer/src/input/handler.rs | 39 +++++++++++---------------- crates/fj-viewer/src/input/zoom.rs | 15 ++++++++--- crates/fj-window/src/run.rs | 4 +-- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/crates/fj-viewer/src/input/handler.rs b/crates/fj-viewer/src/input/handler.rs index 50ad3a9cb..7bc24446c 100644 --- a/crates/fj-viewer/src/input/handler.rs +++ b/crates/fj-viewer/src/input/handler.rs @@ -1,5 +1,3 @@ -use std::time::Instant; - use fj_interop::mesh::Mesh; use fj_math::Point; @@ -24,24 +22,6 @@ pub struct Handler { } impl Handler { - /// Returns a new Handler. - /// - /// # Examples - /// ```rust no_run - /// // Store initialization time for camera zoom calculations - /// let instant = std::time::Instant::now(); - /// let input_handler = fj_viewer::input::Handler::new(instant); - /// ``` - pub fn new(now: Instant) -> Self { - Self { - cursor: None, - - movement: Movement::new(), - rotation: Rotation::new(), - zoom: Zoom::new(), - } - } - /// Returns the state of the cursor position. pub fn cursor(&self) -> Option { self.cursor @@ -52,7 +32,6 @@ impl Handler { &mut self, event: Event, screen_size: Size, - now: Instant, mesh: &Mesh>, camera: &mut Camera, actions: &mut Actions, @@ -112,12 +91,24 @@ impl Handler { pub fn update( &mut self, delta_t: f64, - now: Instant, camera: &mut Camera, - size: Size, + screen_size: Size, mesh: &Mesh>, ) { - self.zoom.apply_to_camera(delta_t, camera); + let focus_point = camera.focus_point(screen_size, self.cursor(), mesh); + self.zoom.apply_to_camera(delta_t, focus_point, camera); + } +} + +impl Default for Handler { + fn default() -> Self { + Self { + cursor: None, + + movement: Movement::new(), + rotation: Rotation::new(), + zoom: Zoom::new(), + } } } diff --git a/crates/fj-viewer/src/input/zoom.rs b/crates/fj-viewer/src/input/zoom.rs index e0936208d..9827933e7 100644 --- a/crates/fj-viewer/src/input/zoom.rs +++ b/crates/fj-viewer/src/input/zoom.rs @@ -1,6 +1,6 @@ use fj_math::{Transform, Vector}; -use crate::camera::Camera; +use crate::camera::{Camera, FocusPoint}; pub struct Zoom { accumulated_delta: f64, @@ -18,11 +18,20 @@ impl Zoom { self.accumulated_delta += delta; } - pub fn apply_to_camera(&mut self, delta_t: f64, camera: &mut Camera) { + pub fn apply_to_camera( + &mut self, + delta_t: f64, + focus_point: FocusPoint, + camera: &mut Camera, + ) { + let distance = match focus_point.0 { + Some(fp) => (fp - camera.position()).magnitude(), + None => camera.position().coords.magnitude(), + }; let displacement = self.accumulated_delta * delta_t * ZOOM_FACTOR - * camera.position().coords.magnitude().into_f64(); + * 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 9764523df..85015fdd5 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_time = Instant::now(); - let mut input_handler = input::Handler::new(previous_time); + let mut input_handler = input::Handler::default(); let mut renderer = block_on(Renderer::new(&window))?; let mut draw_config = DrawConfig::default(); @@ -182,7 +182,6 @@ pub fn run( if let (Some(shape), Some(camera)) = (&shape, &mut camera) { input_handler.update( delta_t.as_secs_f64(), - now, camera, window.size(), &shape.mesh, @@ -213,7 +212,6 @@ pub fn run( input_handler.handle_event( event, window.size(), - now, &shape.mesh, camera, &mut actions, From a67c088abc4517d9822279f20e0bf9820114bf0e Mon Sep 17 00:00:00 2001 From: Sam Jeeves Date: Mon, 4 Jul 2022 14:05:19 +0200 Subject: [PATCH 4/5] Adjust zoom factor to a value more appropriate for scroll wheels --- crates/fj-viewer/src/input/zoom.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-viewer/src/input/zoom.rs b/crates/fj-viewer/src/input/zoom.rs index 9827933e7..1d29a0243 100644 --- a/crates/fj-viewer/src/input/zoom.rs +++ b/crates/fj-viewer/src/input/zoom.rs @@ -43,4 +43,4 @@ impl Zoom { /// /// Smaller values will move the camera less with the same input. /// Larger values will move the camera more with the same input. -const ZOOM_FACTOR: f64 = 0.05; +const ZOOM_FACTOR: f64 = 0.1; From fe8a66097e760f61839b150107b63ad3b18c7125 Mon Sep 17 00:00:00 2001 From: Sam Jeeves Date: Wed, 6 Jul 2022 13:22:16 +0200 Subject: [PATCH 5/5] Separate line- and pixel-based scroll input --- crates/fj-viewer/src/input/event.rs | 12 +++++++++++- crates/fj-viewer/src/input/mod.rs | 2 +- crates/fj-viewer/src/input/zoom.rs | 25 +++++++++++++++++-------- crates/fj-window/src/run.rs | 18 ++++++++---------- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/crates/fj-viewer/src/input/event.rs b/crates/fj-viewer/src/input/event.rs index 48542fc4d..41f68bd91 100644 --- a/crates/fj-viewer/src/input/event.rs +++ b/crates/fj-viewer/src/input/event.rs @@ -9,7 +9,17 @@ pub enum Event { Key(Key, KeyState), /// The user scrolled - Scroll(f64), + Scroll(MouseScrollDelta), +} + +/// Describes a difference in the mouse scroll wheel state. +pub enum MouseScrollDelta { + /// Amount in lines to scroll in the horizontal direction. + /// + /// Positive values indicate movement forward (away from the user). + Line(f64), + /// Amount in pixels to scroll in the vertical direction. + Pixel(f64), } /// A keyboard or mouse key diff --git a/crates/fj-viewer/src/input/mod.rs b/crates/fj-viewer/src/input/mod.rs index 3de606fe4..f206600f1 100644 --- a/crates/fj-viewer/src/input/mod.rs +++ b/crates/fj-viewer/src/input/mod.rs @@ -7,6 +7,6 @@ mod rotation; mod zoom; pub use self::{ - event::{Event, Key, KeyState}, + event::{Event, Key, KeyState, MouseScrollDelta}, handler::{Actions, Handler}, }; diff --git a/crates/fj-viewer/src/input/zoom.rs b/crates/fj-viewer/src/input/zoom.rs index 1d29a0243..b95765281 100644 --- a/crates/fj-viewer/src/input/zoom.rs +++ b/crates/fj-viewer/src/input/zoom.rs @@ -2,6 +2,8 @@ use fj_math::{Transform, Vector}; use crate::camera::{Camera, FocusPoint}; +use super::event::MouseScrollDelta; + pub struct Zoom { accumulated_delta: f64, } @@ -13,9 +15,12 @@ impl Zoom { } } - pub fn push(&mut self, delta: f64) { + pub fn push(&mut self, delta: MouseScrollDelta) { // Accumulate all zoom inputs - self.accumulated_delta += delta; + self.accumulated_delta += match delta { + MouseScrollDelta::Line(delta) => ZOOM_FACTOR_LINE * delta, + MouseScrollDelta::Pixel(delta) => ZOOM_FACTOR_PIXEL * delta, + }; } pub fn apply_to_camera( @@ -28,10 +33,8 @@ impl Zoom { Some(fp) => (fp - camera.position()).magnitude(), None => camera.position().coords.magnitude(), }; - let displacement = self.accumulated_delta - * delta_t - * ZOOM_FACTOR - * distance.into_f64(); + let displacement = + self.accumulated_delta * delta_t * distance.into_f64(); camera.translation = camera.translation * Transform::translation(Vector::from([0.0, 0.0, -displacement])); @@ -39,8 +42,14 @@ impl Zoom { } } -/// Affects the speed of zoom movement. +/// Affects the speed of zoom movement given a scroll wheel input in lines. +/// +/// Smaller values will move the camera less with the same input. +/// Larger values will move the camera more with the same input. +const ZOOM_FACTOR_LINE: f64 = 15.0; + +/// Affects the speed of zoom movement given a scroll wheel input in pixels. /// /// Smaller values will move the camera less with the same input. /// Larger values will move the camera more with the same input. -const ZOOM_FACTOR: f64 = 0.1; +const ZOOM_FACTOR_PIXEL: f64 = 1.0; diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 85015fdd5..d48949754 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -165,16 +165,14 @@ pub fn run( Event::WindowEvent { event: WindowEvent::MouseWheel { delta, .. }, .. - } => { - let delta = match delta { - MouseScrollDelta::LineDelta(_, y) => y as f64 * 10.0, - MouseScrollDelta::PixelDelta(PhysicalPosition { - y, - .. - }) => y, - }; - Some(input::Event::Scroll(delta)) - } + } => Some(input::Event::Scroll(match delta { + MouseScrollDelta::LineDelta(_, y) => { + input::MouseScrollDelta::Line(y as f64) + } + MouseScrollDelta::PixelDelta(PhysicalPosition { + y, .. + }) => input::MouseScrollDelta::Pixel(y), + })), Event::MainEventsCleared => { let delta_t = now.duration_since(previous_time); previous_time = now;