From 50c9b1d57738baf8125d4a9f711658d5b6fbfd0b Mon Sep 17 00:00:00 2001
From: Huon Imberger <Plonq@users.noreply.github.com>
Date: Mon, 12 Feb 2024 18:37:42 +1100
Subject: [PATCH] Remove built-in roll controls (#51)

---
 examples/advanced.rs      |  7 ----
 examples/roll_axis.rs     | 81 +++++++++++++++++++++++++++++++++++++++
 examples/touch/Cargo.lock |  2 +-
 examples/touch/src/lib.rs |  8 +---
 src/input.rs              | 57 ++-------------------------
 src/lib.rs                | 71 ++++------------------------------
 6 files changed, 95 insertions(+), 131 deletions(-)
 create mode 100644 examples/roll_axis.rs

diff --git a/examples/advanced.rs b/examples/advanced.rs
index 34a26d8..a0263ca 100644
--- a/examples/advanced.rs
+++ b/examples/advanced.rs
@@ -5,7 +5,6 @@
 //!   Orbit: Middle click
 //!   Pan: Shift + Middle click
 //!   Zoom: Mousewheel
-//!   Roll: A (roll left) and D (roll right)
 
 use bevy::prelude::*;
 use bevy_panorbit_camera::{PanOrbitCamera, PanOrbitCameraPlugin};
@@ -79,12 +78,6 @@ fn setup(
             modifier_pan: Some(KeyCode::ShiftLeft),
             // Reverse the zoom direction
             reversed_zoom: true,
-            // Enable roll in addition to orbit.
-            // Note: when enabling roll you probably also want to set `allow_upside_down` to `true`
-            // because upside down loses its meaning when you can roll freely.
-            key_roll_left: Some(KeyCode::A),
-            key_roll_right: Some(KeyCode::D),
-            roll_sensitivity: 0.5,
             ..default()
         },
     ));
diff --git a/examples/roll_axis.rs b/examples/roll_axis.rs
new file mode 100644
index 0000000..2a79327
--- /dev/null
+++ b/examples/roll_axis.rs
@@ -0,0 +1,81 @@
+//! Demonstrates how to 'roll' the camera, thus control the 3rd axis of rotation
+
+use bevy::prelude::*;
+use bevy_panorbit_camera::{PanOrbitCamera, PanOrbitCameraPlugin};
+use std::f32::consts::TAU;
+
+fn main() {
+    App::new()
+        .add_plugins(DefaultPlugins)
+        .add_plugins(PanOrbitCameraPlugin)
+        .add_systems(Startup, setup)
+        .add_systems(Update, roll_controls)
+        .run();
+}
+
+fn setup(
+    mut commands: Commands,
+    mut meshes: ResMut<Assets<Mesh>>,
+    mut materials: ResMut<Assets<StandardMaterial>>,
+) {
+    // Ground
+    commands.spawn(PbrBundle {
+        mesh: meshes.add(shape::Plane::from_size(5.0).into()),
+        material: materials.add(Color::rgb(0.3, 0.5, 0.3).into()),
+        ..default()
+    });
+    // Cube
+    commands.spawn(PbrBundle {
+        mesh: meshes.add(Mesh::from(shape::Cube { size: 1.0 })),
+        material: materials.add(Color::rgb(0.8, 0.7, 0.6).into()),
+        transform: Transform::from_xyz(0.0, 0.5, 0.0),
+        ..default()
+    });
+    // Light
+    commands.spawn(PointLightBundle {
+        point_light: PointLight {
+            intensity: 1500.0,
+            shadows_enabled: true,
+            ..default()
+        },
+        transform: Transform::from_xyz(4.0, 8.0, 4.0),
+        ..default()
+    });
+    // Camera
+    commands.spawn((
+        Camera3dBundle {
+            transform: Transform::from_translation(Vec3::new(0.0, 1.5, 5.0)),
+            ..default()
+        },
+        PanOrbitCamera {
+            // Changing the up vector (which rolling does) changes what 'up' means, so you likely
+            // want to allow upside down when rolling.
+            allow_upside_down: true,
+            ..default()
+        },
+    ));
+}
+
+/// Use left/right arrow keys to roll the camera
+fn roll_controls(
+    mut pan_orbit_q: Query<(&mut PanOrbitCamera, &Transform)>,
+    time: Res<Time>,
+    key_input: Res<Input<KeyCode>>,
+) {
+    if let Ok((mut pan_orbit, transform)) = pan_orbit_q.get_single_mut() {
+        let mut roll_angle = 0.0;
+        let roll_amount = TAU / 3.0 * time.delta_seconds();
+        if key_input.pressed(KeyCode::Left) {
+            roll_angle -= roll_amount;
+        }
+        if key_input.pressed(KeyCode::Right) {
+            roll_angle += roll_amount;
+        }
+        // Rotate the base transform by the roll amount around its current 'look' axis
+        pan_orbit
+            .base_transform
+            .rotate_axis(transform.local_z(), roll_angle);
+        // Whenever controlling the camera manually you must make it force update every frame
+        pan_orbit.force_update = true;
+    }
+}
diff --git a/examples/touch/Cargo.lock b/examples/touch/Cargo.lock
index 60279e0..7f69656 100644
--- a/examples/touch/Cargo.lock
+++ b/examples/touch/Cargo.lock
@@ -756,7 +756,7 @@ dependencies = [
 
 [[package]]
 name = "bevy_panorbit_camera"
-version = "0.11.1"
+version = "0.12.1"
 dependencies = [
  "bevy",
  "bevy_easings",
diff --git a/examples/touch/src/lib.rs b/examples/touch/src/lib.rs
index b71ec55..cbde955 100644
--- a/examples/touch/src/lib.rs
+++ b/examples/touch/src/lib.rs
@@ -80,13 +80,7 @@ fn setup_scene(
             transform: Transform::from_translation(Vec3::new(0.0, 1.5, 5.0)),
             ..default()
         },
-        PanOrbitCamera {
-            // Enable roll for demonstration purposes. Typically you wouldn't want to enable
-            // this. See documentation for `key_roll_left` / `key_roll_right` and `base_transform`
-            // for more information.
-            touch_roll_enabled: true,
-            ..default()
-        },
+        PanOrbitCamera::default(),
     ));
 }
 
diff --git a/src/input.rs b/src/input.rs
index 2308587..d343d5a 100644
--- a/src/input.rs
+++ b/src/input.rs
@@ -1,10 +1,9 @@
-use crate::{ActiveCameraData, PanOrbitCamera};
 use bevy::input::mouse::{MouseMotion, MouseScrollUnit, MouseWheel};
 use bevy::input::touch::Touch;
 use bevy::prelude::*;
-use std::f32::consts::TAU;
 
 use crate::traits::Midpoint;
+use crate::{ActiveCameraData, PanOrbitCamera};
 
 #[derive(Resource, Default, Debug)]
 pub struct MouseKeyTracker {
@@ -12,11 +11,10 @@ pub struct MouseKeyTracker {
     pub pan: Vec2,
     pub scroll_line: f32,
     pub scroll_pixel: f32,
-    pub roll_angle: f32,
+    pub rotation_angle: f32,
     pub orbit_button_changed: bool,
 }
 
-#[allow(clippy::too_many_arguments)]
 pub fn mouse_key_tracker(
     mut camera_movement: ResMut<MouseKeyTracker>,
     mouse_input: Res<Input<MouseButton>>,
@@ -25,7 +23,6 @@ pub fn mouse_key_tracker(
     mut scroll_events: EventReader<MouseWheel>,
     active_cam: Res<ActiveCameraData>,
     orbit_cameras: Query<&PanOrbitCamera>,
-    time: Res<Time>,
 ) {
     // Always consume event reader events to prevent them building up and causing spikes
     let mouse_delta = mouse_motion.read().map(|event| event.delta).sum::<Vec2>();
@@ -43,7 +40,6 @@ pub fn mouse_key_tracker(
             let mut pan = Vec2::ZERO;
             let mut scroll_line = 0.0;
             let mut scroll_pixel = 0.0;
-            let mut roll_angle = 0.0;
             let mut orbit_button_changed = false;
 
             // Orbit and pan
@@ -58,19 +54,6 @@ pub fn mouse_key_tracker(
             scroll_line += scroll_line_delta;
             scroll_pixel += scroll_pixel_delta;
 
-            // Roll
-            let roll_amount = TAU * 0.3 * time.delta_seconds();
-            if let Some(roll_left_key) = pan_orbit.key_roll_left {
-                if key_input.pressed(roll_left_key) {
-                    roll_angle -= roll_amount;
-                }
-            }
-            if let Some(roll_right_key) = pan_orbit.key_roll_right {
-                if key_input.pressed(roll_right_key) {
-                    roll_angle += roll_amount;
-                }
-            }
-
             // Other
             if orbit_just_pressed(pan_orbit, &mouse_input, &key_input)
                 || orbit_just_released(pan_orbit, &mouse_input, &key_input)
@@ -82,7 +65,6 @@ pub fn mouse_key_tracker(
             camera_movement.pan = pan;
             camera_movement.scroll_line = scroll_line;
             camera_movement.scroll_pixel = scroll_pixel;
-            camera_movement.roll_angle = roll_angle;
             camera_movement.orbit_button_changed = orbit_button_changed;
         }
     }
@@ -97,10 +79,9 @@ pub struct TouchTracker {
 
 impl TouchTracker {
     /// Return orbit, pan, and zoom values based on touch data
-    pub fn calculate_movement(&self) -> (Vec2, Vec2, f32, f32) {
+    pub fn calculate_movement(&self) -> (Vec2, Vec2, f32) {
         let mut orbit = Vec2::ZERO;
         let mut pan = Vec2::ZERO;
-        let mut roll_angle = 0.0;
         let mut zoom_pixel = 0.0;
 
         // Only match when curr and prev have same number of touches, for simplicity.
@@ -128,32 +109,11 @@ impl TouchTracker {
                 let curr_dist = curr1_pos.distance(curr2_pos);
                 let prev_dist = prev1_pos.distance(prev2_pos);
                 zoom_pixel += (curr_dist - prev_dist) * 0.015;
-
-                // Roll
-                let prev_vec = prev2_pos - prev1_pos;
-                let curr_vec = curr2_pos - curr1_pos;
-                let prev_angle_negy = prev_vec.angle_between(Vec2::NEG_Y);
-                let curr_angle_negy = curr_vec.angle_between(Vec2::NEG_Y);
-                let prev_angle_posy = prev_vec.angle_between(Vec2::Y);
-                let curr_angle_posy = curr_vec.angle_between(Vec2::Y);
-                let roll_angle_negy = prev_angle_negy - curr_angle_negy;
-                let roll_angle_posy = prev_angle_posy - curr_angle_posy;
-                // The angle between -1deg and +1deg is 358deg according to Vec2::angle_between,
-                // but we want the answer to be +2deg (or -2deg if swapped). Therefore, we calculate
-                // two angles - one from UP and one from DOWN, and use the smallest absolute value
-                // of the two. This is necessary to get a predictable result when the two touches
-                // swap sides (change from one being on the left and one being on the right to the
-                // other way round).
-                if roll_angle_negy.abs() < roll_angle_posy.abs() {
-                    roll_angle = roll_angle_negy;
-                } else {
-                    roll_angle = roll_angle_posy;
-                }
             }
             _ => {}
         }
 
-        (orbit, pan, roll_angle, zoom_pixel)
+        (orbit, pan, zoom_pixel)
     }
 }
 
@@ -260,12 +220,3 @@ pub fn pan_just_pressed(
             .modifier_orbit
             .map_or(true, |modifier| !key_input.pressed(modifier))
 }
-
-pub fn roll_just_pressed(pan_orbit: &PanOrbitCamera, key_input: &Res<Input<KeyCode>>) -> bool {
-    pan_orbit
-        .key_roll_left
-        .map_or(false, |key| key_input.just_pressed(key))
-        || pan_orbit
-            .key_roll_right
-            .map_or(false, |key| key_input.just_pressed(key))
-}
diff --git a/src/lib.rs b/src/lib.rs
index aac3cd1..13a9a36 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -228,9 +228,6 @@ pub struct PanOrbitCamera {
     /// The sensitivity of moving the camera closer or further way using the scroll wheel.
     /// Defaults to `1.0`.
     pub zoom_sensitivity: f32,
-    /// The sensitivity of rolling the camera (rotating around local Z-axis).
-    /// Defaults to `1.0`.
-    pub roll_sensitivity: f32,
     /// How much smoothing is applied to the zoom motion. A value of `0.0` disables smoothing,
     /// so there's a 1:1 mapping of input to camera position. A value of `1.0` is infinite
     /// smoothing.
@@ -250,41 +247,9 @@ pub struct PanOrbitCamera {
     /// Key that must be pressed for `button_pan` to work.
     /// Defaults to `None` (no modifier).
     pub modifier_pan: Option<KeyCode>,
-    /// Key to use for rolling right (clockwise) around the local -Z-axis (i.e. the direction the
-    /// camera is currently facing).
-    /// Enabling roll by setting this and/or `key_roll_left` to a `Some` value introduces the 3rd
-    /// axis of rotation. This can be useful in some scenarios, but has some drawbacks. Notably
-    /// it effectively allows changing the 'up' vector, and once changed it's practically impossible
-    /// to reset it back to the world 'up' vector with the mouse controls alone (you must do it
-    /// programmatically). Hence, by default rolling is disabled.
-    /// If you do want to reset the up vector programmatically, set `base_transform` to
-    /// `Transform::IDENTITY`.
-    /// If you enable roll, I highly recommend also setting `allow_upside_down` to `true`, because
-    /// being 'upside down' doesn't really mean anything when you can roll freely.
-    /// Defaults to `None`.
-    pub key_roll_right: Option<KeyCode>,
-    /// Key to use for rolling left (anti-clockwise) around the local -Z-axis (i.e. the direction
-    /// the camera is currently facing).
-    /// Enabling roll by setting this and/or `key_roll_right` to a `Some` value introduces the 3rd
-    /// axis of rotation. This can be useful in some scenarios, but has some drawbacks. Notably
-    /// it effectively allows changing the 'up' vector, and once changed it's practically impossible
-    /// to reset it back to the world 'up' vector with the mouse controls alone (you must do it
-    /// programmatically). Hence, by default rolling is disabled.
-    /// If you do want to reset the up vector programmatically, set `base_transform` to
-    /// `Transform::IDENTITY`.
-    /// If you enable roll, I highly recommend also setting `allow_upside_down` to `true`, because
-    /// being 'upside down' doesn't really mean anything when you can roll freely.
-    /// Defaults to `None`.
-    pub key_roll_left: Option<KeyCode>,
     /// Whether touch controls are enabled.
     /// Defaults to `true`.
     pub touch_enabled: bool,
-    /// Whether 'roll' touch control is enabled. Roll is achieved by rotating two fingers in a
-    /// circle. It is disabled by default because rolling changes the 'up' vector which is not
-    /// desirable in most circumstances. See documentation for `key_roll_left` / `key_roll_right`
-    /// and `base_transform` for more information.
-    /// Defaults to `false`.
-    pub touch_roll_enabled: bool,
     /// Whether to reverse the zoom direction.
     /// Defaults to `false`.
     pub reversed_zoom: bool,
@@ -295,8 +260,6 @@ pub struct PanOrbitCamera {
     /// Defaults to `false` (but will be updated immediately).
     pub is_upside_down: bool,
     /// Whether to allow the camera to go upside down.
-    /// You probably want to set this to `true` if you change `base_transform` or `key_roll_left` /
-    /// `key_roll_right`. See the documentation for those properties for more information.
     /// Defaults to `false`.
     pub allow_upside_down: bool,
     /// If `false`, disable control of the camera. Defaults to `true`.
@@ -311,12 +274,12 @@ pub struct PanOrbitCamera {
     /// Defaults to `false`.
     pub force_update: bool,
     /// The transform that all rotations and transforms are based upon.
-    /// Typically this will always be equal to `Transform::IDENTITY`. It is used for the 'roll' axis
-    /// (see `key_roll_left` and `key_roll_right`), but it can also be used to manually set the
-    /// vector the camera treats as 'up'. If you change this (or enable roll controls), you
+    /// Typically this will always be equal to `Transform::IDENTITY`. It can be used to implement
+    /// 'roll' controls (see `roll_axis` example), or it can be used to set a custom 'up' vector
+    /// (see `alternate_up_vector` example). If you do change this (or enable roll controls), you
     /// probably also want to set `allow_upside_down` to `true`. Also note that alpha/beta limits
     /// will be applied after `base_transform`, and thus may not act like you expect.
-    /// Note that changing the translation or scale of this transform is currently not supported.
+    /// Changing the translation or scale of this transform is currently not supported.
     /// Defaults to `Transform::IDENTITY`.
     pub base_transform: Transform,
 }
@@ -335,15 +298,11 @@ impl Default for PanOrbitCamera {
             pan_smoothness: 0.6,
             zoom_sensitivity: 1.0,
             zoom_smoothness: 0.8,
-            roll_sensitivity: 1.0,
             button_orbit: MouseButton::Left,
             button_pan: MouseButton::Right,
             modifier_orbit: None,
             modifier_pan: None,
-            key_roll_right: None,
-            key_roll_left: None,
             touch_enabled: true,
-            touch_roll_enabled: false,
             reversed_zoom: false,
             enabled: true,
             alpha: None,
@@ -411,7 +370,6 @@ fn active_viewport_data(
     for (entity, camera, pan_orbit) in orbit_cameras.iter() {
         let input_just_activated = input::orbit_just_pressed(pan_orbit, &mouse_input, &key_input)
             || input::pan_just_pressed(pan_orbit, &mouse_input, &key_input)
-            || input::roll_just_pressed(pan_orbit, &key_input)
             || !scroll_events.is_empty()
             || (touches.iter_just_pressed().count() > 0
                 && touches.iter_just_pressed().count() == touches.iter().count());
@@ -549,15 +507,13 @@ fn pan_orbit_camera(
         let mut pan = Vec2::ZERO;
         let mut scroll_line = 0.0;
         let mut scroll_pixel = 0.0;
-        let mut roll_angle = 0.0;
         let mut orbit_button_changed = false;
 
         // The reason we only skip getting input if the camera is inactive/disabled is because
         // it might still be moving (lerping towards target values) when the user is not
         // actively controlling it.
         if pan_orbit.enabled && active_cam.entity == Some(entity) {
-            let (touch_orbit, touch_pan, touch_roll_angle, touch_zoom_pixel) =
-                touch_tracker.calculate_movement();
+            let (touch_orbit, touch_pan, touch_zoom_pixel) = touch_tracker.calculate_movement();
             let zoom_direction = match pan_orbit.reversed_zoom {
                 true => -1.0,
                 false => 1.0,
@@ -569,25 +525,16 @@ fn pan_orbit_camera(
                 mouse_key_tracker.scroll_line * zoom_direction * pan_orbit.zoom_sensitivity;
             scroll_pixel =
                 mouse_key_tracker.scroll_pixel * zoom_direction * pan_orbit.zoom_sensitivity;
-            roll_angle = mouse_key_tracker.roll_angle * pan_orbit.roll_sensitivity;
             orbit_button_changed = mouse_key_tracker.orbit_button_changed;
 
             if pan_orbit.touch_enabled {
                 orbit += touch_orbit * pan_orbit.orbit_sensitivity;
                 pan += touch_pan * pan_orbit.pan_sensitivity;
                 scroll_pixel += touch_zoom_pixel * zoom_direction * pan_orbit.zoom_sensitivity;
-                if pan_orbit.touch_roll_enabled {
-                    roll_angle += touch_roll_angle * pan_orbit.roll_sensitivity;
-                }
             }
         }
 
-        // 2 - Adjust base transform based on roll
-        pan_orbit
-            .base_transform
-            .rotate_axis(transform.local_z(), roll_angle);
-
-        // 3 - Process input into target alpha/beta, or focus, radius
+        // 2 - Process input into target alpha/beta, or focus, radius
 
         if orbit_button_changed {
             // Only check for upside down when orbiting started or ended this frame,
@@ -662,7 +609,7 @@ fn pan_orbit_camera(
             has_moved = true;
         }
 
-        // 4 - Apply constraints
+        // 3 - Apply constraints
 
         pan_orbit.target_alpha = apply_alpha_limits(pan_orbit.target_alpha);
         pan_orbit.target_beta = apply_beta_limits(pan_orbit.target_beta);
@@ -673,7 +620,7 @@ fn pan_orbit_camera(
             pan_orbit.target_beta = pan_orbit.target_beta.clamp(-PI / 2.0, PI / 2.0);
         }
 
-        // 5 - Update the camera's transform based on current values
+        // 4 - Update the camera's transform based on current values
 
         if let (Some(alpha), Some(beta), Some(radius)) =
             (pan_orbit.alpha, pan_orbit.beta, pan_orbit.radius)
@@ -687,8 +634,6 @@ fn pan_orbit_camera(
                 || pan_orbit.target_beta != beta
                 || pan_orbit.target_radius != radius
                 || pan_orbit.target_focus != pan_orbit.focus
-                // Rolling is not smoothed, so just checking whether it's non-zero is fine
-                || roll_angle != 0.0
                 // Scale will always be None for non-orthographic cameras, so we can't include it in
                 // the 'if let' above
                 || Some(pan_orbit.target_scale) != pan_orbit.scale