Mouse wheel for FreeCamera: Responses to review comments.

src/Cameras/Inputs/BaseCameraMouseWheelInput.ts

@@ -1,6 +1,6 @@
 import { Nullable } from "../../types";
 import { serialize } from "../../Misc/decorators";
-import { EventState, Observable, Observer } from "../../Misc/observable";
+import { Observable, Observer } from "../../Misc/observable";
 import { Camera } from "../../Cameras/camera";
 import { ICameraInput } from "../../Cameras/cameraInputsManager";
 import { PointerInfo, PointerEventTypes } from "../../Events/pointerEvents";
@@ -43,7 +43,7 @@ export abstract class BaseCameraMouseWheelInput implements ICameraInput<Camera>
     public onChangedObservable = new Observable<
         {wheelDeltaX: number, wheelDeltaY: number, wheelDeltaZ: number}>();
 
-    private _wheel: Nullable<(pointer: PointerInfo, _: EventState) => void>;
+    private _wheel: Nullable<(pointer: PointerInfo) => void>;
     private _observer: Nullable<Observer<PointerInfo>>;
 
     /**
@@ -54,7 +54,7 @@ export abstract class BaseCameraMouseWheelInput implements ICameraInput<Camera>
      *   (https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault)
      */
     public attachControl(element: HTMLElement, noPreventDefault?: boolean): void {
-        this._wheel = (pointer, _) => {
+        this._wheel = (pointer) => {
             // sanity check - this should be a PointerWheel event.
             if (pointer.type !== PointerEventTypes.POINTERWHEEL) { return; }
 
@@ -92,19 +92,6 @@ export abstract class BaseCameraMouseWheelInput implements ICameraInput<Camera>
                 // Maybe others?
                 this._wheelDeltaY -=
                     this.wheelPrecisionY * (<any>event).wheelDelta / this._normalize;
-            } else if (event.detail) {
-                // Firefox < v17  (Has WebGL >= v4)
-                // TODO How should we scale this?
-                // Since it's Firefox, it's probably the same as
-                // WheelEvent.DOM_DELTA_LINE.
-                // ie: we can presume it needs scaled to match per-pixel.
-                this._wheelDeltaY +=
-                    this.wheelPrecisionY * this._ffMultiplier * event.detail;
-                if ("axis" in event &&
-                      (<any>event).axis === (<any>event).HORIZONTAL_AXIS) {
-                    this._wheelDeltaX = this._wheelDeltaY;
-                    this._wheelDeltaY = 0;
-                }
             }
 
             if (event.preventDefault) {
@@ -124,7 +111,7 @@ export abstract class BaseCameraMouseWheelInput implements ICameraInput<Camera>
      * @param element Defines the element to stop listening the inputs from
      */
     public detachControl(element: Nullable<HTMLElement>): void {
-        if (this._observer && element) {
+        if (this._observer) {
             this.camera.getScene().onPointerObservable.remove(this._observer);
             this._observer = null;
             this._wheel = null;

src/Cameras/Inputs/freeCameraMouseWheelInput.ts

@@ -4,24 +4,7 @@ import { FreeCamera } from "../../Cameras/freeCamera";
 import { CameraInputTypes } from "../../Cameras/cameraInputsManager";
 import { BaseCameraMouseWheelInput } from "../../Cameras/Inputs/BaseCameraMouseWheelInput";
 import { Matrix, Vector3 } from "../../Maths/math.vector";
-
-/**
- * Defines the potential axis to be altered in a transform operation.
- */
-export enum AXIS {
-    /**
-     * X axis.
-     */
-    X,
-    /**
-     * Y axis.
-     */
-    Y,
-    /**
-     * Z axis.
-     */
-    Z
-}
+import { Coordinate } from "../../Maths/math.axis";
 
 /**
  * Manage the mouse wheel inputs to control a free camera.
@@ -48,13 +31,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelXMoveRelative(axis: Nullable<AXIS>) {
+    public set wheelXMoveRelative(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelXAction !== this._moveRelative) {
             // Attempting to clear different _wheelXAction.
             return;
         }
         this._wheelXAction = this._moveRelative;
-        this._wheelXActionProperty = axis;
+        this._wheelXActionCoordinate = axis;
     }
 
     /**
@@ -62,11 +45,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * mouse wheel's X axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelXMoveRelative(): Nullable<AXIS> {
+    public get wheelXMoveRelative(): Nullable<Coordinate> {
         if (this._wheelXAction !== this._moveRelative) {
             return null;
         }
-        return this._wheelXActionProperty;
+        return this._wheelXActionCoordinate;
     }
 
     /**
@@ -75,13 +58,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelYMoveRelative(axis: Nullable<AXIS>) {
+    public set wheelYMoveRelative(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelYAction !== this._moveRelative) {
             // Attempting to clear different _wheelYAction.
             return;
         }
         this._wheelYAction = this._moveRelative;
-        this._wheelYActionProperty = axis;
+        this._wheelYActionCoordinate = axis;
     }
 
     /**
@@ -89,11 +72,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * mouse wheel's Y axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelYMoveRelative(): Nullable<AXIS> {
+    public get wheelYMoveRelative(): Nullable<Coordinate> {
         if (this._wheelYAction !== this._moveRelative) {
             return null;
         }
-        return this._wheelYActionProperty;
+        return this._wheelYActionCoordinate;
     }
 
     /**
@@ -102,13 +85,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelZMoveRelative(axis: Nullable<AXIS>) {
+    public set wheelZMoveRelative(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelZAction !== this._moveRelative) {
             // Attempting to clear different _wheelZAction.
             return;
         }
         this._wheelZAction = this._moveRelative;
-        this._wheelZActionProperty = axis;
+        this._wheelZActionCoordinate = axis;
     }
 
     /**
@@ -116,11 +99,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * mouse wheel's Z axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelZMoveRelative(): Nullable<AXIS> {
+    public get wheelZMoveRelative(): Nullable<Coordinate> {
         if (this._wheelZAction !== this._moveRelative) {
             return null;
         }
-        return this._wheelZActionProperty;
+        return this._wheelZActionCoordinate;
     }
 
     /**
@@ -129,13 +112,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelXRotateRelative(axis: Nullable<AXIS>) {
+    public set wheelXRotateRelative(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelXAction !== this._rotateRelative) {
             // Attempting to clear different _wheelXAction.
             return;
         }
         this._wheelXAction = this._rotateRelative;
-        this._wheelXActionProperty = axis;
+        this._wheelXActionCoordinate = axis;
     }
 
     /**
@@ -143,11 +126,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * mouse wheel's X axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelXRotateRelative(): Nullable<AXIS> {
+    public get wheelXRotateRelative(): Nullable<Coordinate> {
         if (this._wheelXAction !== this._rotateRelative) {
             return null;
         }
-        return this._wheelXActionProperty;
+        return this._wheelXActionCoordinate;
     }
 
     /**
@@ -156,13 +139,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelYRotateRelative(axis: Nullable<AXIS>) {
+    public set wheelYRotateRelative(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelYAction !== this._rotateRelative) {
             // Attempting to clear different _wheelYAction.
             return;
         }
         this._wheelYAction = this._rotateRelative;
-        this._wheelYActionProperty = axis;
+        this._wheelYActionCoordinate = axis;
     }
 
     /**
@@ -170,11 +153,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * mouse wheel's Y axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelYRotateRelative(): Nullable<AXIS> {
+    public get wheelYRotateRelative(): Nullable<Coordinate> {
         if (this._wheelYAction !== this._rotateRelative) {
             return null;
         }
-        return this._wheelYActionProperty;
+        return this._wheelYActionCoordinate;
     }
 
     /**
@@ -183,13 +166,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelZRotateRelative(axis: Nullable<AXIS>) {
+    public set wheelZRotateRelative(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelZAction !== this._rotateRelative) {
             // Attempting to clear different _wheelZAction.
             return;
         }
         this._wheelZAction = this._rotateRelative;
-        this._wheelZActionProperty = axis;
+        this._wheelZActionCoordinate = axis;
     }
 
     /**
@@ -197,11 +180,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * mouse wheel's Z axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelZRotateRelative(): Nullable<AXIS> {
+    public get wheelZRotateRelative(): Nullable<Coordinate> {
         if (this._wheelZAction !== this._rotateRelative) {
             return null;
         }
-        return this._wheelZActionProperty;
+        return this._wheelZActionCoordinate;
     }
 
     /**
@@ -210,13 +193,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelXMoveScene(axis: Nullable<AXIS>) {
+    public set wheelXMoveScene(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelXAction !== this._rotateRelative) {
             // Attempting to clear different _wheelXAction.
             return;
         }
         this._wheelXAction = this._moveScene;
-        this._wheelXActionProperty = axis;
+        this._wheelXActionCoordinate = axis;
     }
 
     /**
@@ -224,11 +207,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * X axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelXMoveScene(): Nullable<AXIS> {
+    public get wheelXMoveScene(): Nullable<Coordinate> {
         if (this._wheelXAction !== this._moveScene) {
             return null;
         }
-        return this._wheelXActionProperty;
+        return this._wheelXActionCoordinate;
     }
 
     /**
@@ -237,13 +220,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelYMoveScene(axis: Nullable<AXIS>) {
+    public set wheelYMoveScene(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelYAction !== this._rotateRelative) {
             // Attempting to clear different _wheelYAction.
             return;
         }
         this._wheelYAction = this._moveScene;
-        this._wheelYActionProperty = axis;
+        this._wheelYActionCoordinate = axis;
     }
 
     /**
@@ -251,11 +234,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * Y axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelYMoveScene(): Nullable<AXIS> {
+    public get wheelYMoveScene(): Nullable<Coordinate> {
         if (this._wheelYAction !== this._moveScene) {
             return null;
         }
-        return this._wheelYActionProperty;
+        return this._wheelYActionCoordinate;
     }
 
     /**
@@ -264,13 +247,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * @param axis The axis to be moved. Set null to clear.
      */
     @serialize()
-    public set wheelZMoveScene(axis: Nullable<AXIS>) {
+    public set wheelZMoveScene(axis: Nullable<Coordinate>) {
         if (axis === null && this._wheelZAction !== this._rotateRelative) {
             // Attempting to clear different _wheelZAction.
             return;
         }
         this._wheelZAction = this._moveScene;
-        this._wheelZActionProperty = axis;
+        this._wheelZActionCoordinate = axis;
     }
 
     /**
@@ -278,11 +261,11 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * Z axis controls.
      * @returns The configured axis or null if none.
      */
-    public get wheelZMoveScene(): Nullable<AXIS> {
+    public get wheelZMoveScene(): Nullable<Coordinate> {
         if (this._wheelZAction !== this._moveScene) {
             return null;
         }
-        return this._wheelZActionProperty;
+        return this._wheelZActionCoordinate;
     }
 
     /**
@@ -295,13 +278,13 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
             return;
         }
 
+        // Clear the camera properties that we might be updating.
         this._moveRelative.setAll(0);
         this._rotateRelative.setAll(0);
         this._moveScene.setAll(0);
 
-        this._updateCamera(this._wheelDeltaX, this._wheelXAction, this._wheelXActionProperty);
-        this._updateCamera(this._wheelDeltaY, this._wheelYAction, this._wheelYActionProperty);
-        this._updateCamera(this._wheelDeltaZ, this._wheelZAction, this._wheelZActionProperty);
+        // Set the camera properties that are to be updated.
+        this._updateCamera();
 
         if (this.camera.getScene().useRightHandedSystem) {
             // TODO: Does this need done for worldUpdate too?
@@ -334,38 +317,48 @@ export class FreeCameraMouseWheelInput extends BaseCameraMouseWheelInput {
      * These are set to the desired default behaviour.
      */
     private _wheelXAction: Nullable<Vector3> = this._moveRelative;
-    private _wheelXActionProperty: Nullable<AXIS> = AXIS.X;
+    private _wheelXActionCoordinate: Nullable<Coordinate> = Coordinate.X;
     private _wheelYAction: Nullable<Vector3> = this._moveRelative;
-    private _wheelYActionProperty: Nullable<AXIS> = AXIS.Z;
+    private _wheelYActionCoordinate: Nullable<Coordinate> = Coordinate.Z;
     private _wheelZAction: Nullable<Vector3> = null;
-    private _wheelZActionProperty: Nullable<AXIS> = null;
+    private _wheelZActionCoordinate: Nullable<Coordinate> = null;
 
     /**
-     * Called once per mouse wheel axis. Will update the camera according to any
-     * configured properties for that axis.
+     * Update the camera according to any configured properties for the 3
+     * mouse-wheel axis.
      */
-    private _updateCamera(
-        value: number, action: Nullable<Vector3>, property: Nullable<AXIS>): void {
-            if (value === 0) {
-                // Mouse wheel has not moved.
-                return;
-            }
-            if (action === null || property === null) {
-                // Mouse wheel axis not configured.
-                return;
-            }
-
-            switch (property) {
-                case AXIS.X:
-                    action.set(value, 0, 0);
-                    break;
-                case AXIS.Y:
-                    action.set(0, value, 0);
-                    break;
-                case AXIS.Z:
-                    action.set(0, 0, value);
-                    break;
+    private _updateCamera(): void {
+        let updateCameraProperty = function(value: number,
+                                            action: Nullable<Vector3>,
+                                            coordinate: Nullable<Coordinate>): void {
+                if (value === 0) {
+                    // Mouse wheel has not moved.
+                    return;
+                }
+                if (action === null || coordinate === null) {
+                    // Mouse wheel axis not configured.
+                    return;
+                }
+
+                switch (coordinate) {
+                    case Coordinate.X:
+                        action.set(value, 0, 0);
+                        break;
+                    case Coordinate.Y:
+                        action.set(0, value, 0);
+                        break;
+                    case Coordinate.Z:
+                        action.set(0, 0, value);
+                        break;
+                }
             }
+        // Do the camera updates for each of the 3 touch-wheel axis.
+        updateCameraProperty(
+            this._wheelDeltaX, this._wheelXAction, this._wheelXActionCoordinate);
+        updateCameraProperty(
+            this._wheelDeltaY, this._wheelYAction, this._wheelYActionCoordinate);
+        updateCameraProperty(
+            this._wheelDeltaZ, this._wheelZAction, this._wheelZActionCoordinate);
     }
 
 }

src/Maths/math.axis.ts

@@ -18,4 +18,17 @@ export class Axis {
     public static Y: Vector3 = new Vector3(0.0, 1.0, 0.0);
     /** Z axis */
     public static Z: Vector3 = new Vector3(0.0, 0.0, 1.0);
-}
+}
+
+/**
+ * Defines cartesian components.
+ */
+export enum Coordinate {
+    /** X axis */
+    X,
+    /** Y axis */
+    Y,
+    /** Z axis */
+    Z
+}
+