fix: AxialRotationHandle now correctly handles rotating objects (#187)

* fix: AxialRotationHandle now correctly handles rotating objects around arbitrary axes, when not connected to the selection manager.

 - simplified the Handles overall.

Author: phtaylor

src/Handles/ArcSlider.ts

@@ -10,16 +10,15 @@ import {
   Circle,
   Sphere,
   Registry,
-  Parameter,
   ZeaPointerEvent,
   ZeaMouseEvent,
   ZeaTouchEvent,
-  XRControllerEvent,
 } from '@zeainc/zea-engine'
-import { BaseAxialRotationHandle } from './BaseAxialRotationHandle'
+import { Handle } from './Handle'
 import ParameterValueChange from '../UndoRedo/Changes/ParameterValueChange'
 import './Shaders/HandleShader'
 import UndoRedoManager from '../UndoRedo/UndoRedoManager'
+import { Change } from '../UndoRedo/Change'
 
 /**
  * Class representing a slider scene widget with an arc shape. There are two parts in this widget, the slider and the handle.<br>
@@ -35,17 +34,22 @@ import UndoRedoManager from '../UndoRedo/UndoRedoManager'
  * * **dragStart:** Triggered when the pointer is down.
  * * **dragEnd:** Triggered when the pointer is released.
  *
- * @extends BaseAxialRotationHandle
+ * @extends Handle
  */
-class ArcSlider extends BaseAxialRotationHandle {
+class ArcSlider extends Handle {
+  param: XfoParameter | NumberParameter
   arcRadiusParam: NumberParameter
   arcAngleParam: NumberParameter
   handleRadiusParam: NumberParameter
-  handleMat: Material
-  handle: GeomItem
-  arc: GeomItem
-  handleXfo = new Xfo()
-  handleGeomOffsetXfo = new Xfo()
+  range: Array<number>
+  private handleMat: Material
+  private handle: GeomItem
+  private arc: GeomItem
+  private baseXfo: Xfo
+  private handleXfo = new Xfo()
+  private vec0: Vec3
+  private change: Change
+  private handleGeomOffsetXfo = new Xfo()
 
   /**
    * Creates an instance of ArcSlider.
@@ -81,27 +85,27 @@ class ArcSlider extends BaseAxialRotationHandle {
     this.handle.geomOffsetXfoParam.value = this.handleGeomOffsetXfo
 
     // this.barRadiusParam.on('valueChanged', () => {
-    //   arcGeom.radiusParam.value = this.barRadiusParam.getValue();
+    //   arcGeom.radiusParam.value = this.barRadiusParam.value;
     // });
 
     this.range = [0, arcAngle]
     this.arcAngleParam.on('valueChanged', () => {
-      const arcAngle = this.arcAngleParam.getValue()
+      const arcAngle = this.arcAngleParam.value
       arcGeom.angleParam.value = arcAngle
       this.range = [0, arcAngle]
     })
     this.arcRadiusParam.on('valueChanged', () => {
-      const arcRadius = this.arcRadiusParam.getValue()
+      const arcRadius = this.arcRadiusParam.value
       arcGeom.radiusParam.value = arcRadius
       this.handleGeomOffsetXfo.tr.x = arcRadius
       this.handle.geomOffsetXfoParam.value = this.handleGeomOffsetXfo
     })
     this.handleRadiusParam.on('valueChanged', () => {
-      handleGeom.radiusParam.value = this.handleRadiusParam.getValue()
+      handleGeom.radiusParam.value = this.handleRadiusParam.value
     })
 
     this.colorParam.on('valueChanged', () => {
-      this.handleMat.getParameter('BaseColor').value = this.colorParam.getValue()
+      this.handleMat.getParameter('BaseColor').value = this.colorParam.value
     })
 
     this.addChild(this.handle)
@@ -149,34 +153,21 @@ class ArcSlider extends BaseAxialRotationHandle {
    */
   highlight(): void {
     super.highlight()
-    this.handleMat.getParameter('BaseColor').value = this.highlightColorParam.getValue()
+    this.handleMat.getParameter('BaseColor').value = this.highlightColorParam.value
   }
 
   /**
    * Removes the shining shader from the handle.
    */
   unhighlight(): void {
     super.unhighlight()
-    this.handleMat.getParameter('BaseColor').value = this.colorParam.getValue()
+    this.handleMat.getParameter('BaseColor').value = this.colorParam.value
   }
 
-  // /**
-  //  * The setTargetParam method.
-  //  * @param param - The param param.
-  //  */
-  // setTargetParam(param) {
-  //   this.param = param;
-  //   const __updateSlider = () => {
-  //     this.__updateSlider(param.getValue());
-  //   };
-  //   __updateSlider();
-  //   param.on('valueChanged', __updateSlider);
-  // }
-
   /**
    * Sets global xfo target parameter
    *
-   * @param param - The param param.
+   * @param param - The parameter that will be modified during manipulation
    * @param track - The track param.
    */
   setTargetParam(param: XfoParameter | NumberParameter, track = true): void {
@@ -190,7 +181,7 @@ class ArcSlider extends BaseAxialRotationHandle {
         param.on('valueChanged', __updateGizmo)
       } else if (this.param instanceof NumberParameter) {
         const __updateGizmo = () => {
-          this.handleXfo.ori.setFromAxisAndAngle(new Vec3(0, 0, 1), <number>param.getValue())
+          this.handleXfo.ori.setFromAxisAndAngle(new Vec3(0, 0, 1), <number>param.value)
           this.handle.globalXfoParam.value = this.handleXfo
         }
         __updateGizmo()
@@ -205,7 +196,7 @@ class ArcSlider extends BaseAxialRotationHandle {
   //   const range =
   //     this.param && this.param.getRange() ? this.param.getRange() : [0, 1];
   //   const v = Math.remap(value, range[0], range[1], 0, 1);
-  //   const length = this.arcAngleParam.getValue();
+  //   const length = this.arcAngleParam.value;
   //   this.handleXfo.ori.setFromAxisAndAngle(this.axis, ) = v * length;
   //   this.handle.localXfoParam.value = this.handleXfo;
   // }
@@ -230,10 +221,10 @@ class ArcSlider extends BaseAxialRotationHandle {
   onDragStart(event: ZeaPointerEvent): void {
     this.baseXfo = this.globalXfoParam.value.clone()
     this.baseXfo.sc.set(1, 1, 1)
-    // this.offsetXfo = this.baseXfo.inverse().multiply(this.param.getValue());
+    // this.offsetXfo = this.baseXfo.inverse().multiply(this.param.value);
 
     this.vec0 = this.globalXfoParam.value.ori.getXaxis()
-    // this.grabCircleRadius = this.arcRadiusParam.getValue();
+    // this.grabCircleRadius = this.arcRadiusParam.value;
     this.vec0.normalizeInPlace()
 
     this.change = new ParameterValueChange(this.param)

src/Handles/AxialRotationHandle.ts

@@ -20,8 +20,7 @@ import './Shaders/HandleShader'
  */
 class AxialRotationHandle extends BaseAxialRotationHandle {
   radiusParam: NumberParameter
-  handleMat: Material
-  handleXfo: Xfo
+  private handleMat: Material
   handle: GeomItem
   /**
    * Create an axial rotation scene widget.
@@ -46,7 +45,6 @@ class AxialRotationHandle extends BaseAxialRotationHandle {
     // const handleGeom = new Cylinder(radius, thickness * 2, 64, 2, false);
     const handleGeom = new Torus(thickness, radius, 64, Math.PI * 0.5)
     this.handle = new GeomItem('handle', handleGeom, this.handleMat)
-    this.handleXfo = new Xfo()
 
     this.radiusParam.on('valueChanged', () => {
       radius = this.radiusParam.getValue()

src/Handles/BaseAxialRotationHandle.ts

@@ -1,6 +1,5 @@
 import {
   MathFunctions,
-  Parameter,
   Vec3,
   Xfo,
   XfoParameter,
@@ -14,22 +13,22 @@ import ParameterValueChange from '../UndoRedo/Changes/ParameterValueChange'
 import UndoRedoManager from '../UndoRedo/UndoRedoManager'
 import SelectionXfoChange from '../UndoRedo/Changes/SelectionXfoChange'
 import SelectionGroup from '../SelectionGroup'
-import { Change } from '..'
+import { Change } from '../UndoRedo/Change'
 
 /**
  * Class representing an axial rotation scene widget.
  *
  * @extends Handle
  */
 class BaseAxialRotationHandle extends Handle {
-  param: NumberParameter | XfoParameter
-  baseXfo: Xfo
-  grabCircleRadius: number
-  vec0: Vec3
-  change: Change
-  range: Array<number>
-
+  param: XfoParameter
+  private baseXfo: Xfo
+  private handleXfo: Xfo
+  private handleToTargetXfo: Xfo
+  private vec0: Vec3
+  private change: Change
   selectionGroup: SelectionGroup
+
   /**
    * Create an axial rotation scene widget.
    *
@@ -49,28 +48,21 @@ class BaseAxialRotationHandle extends Handle {
   }
 
   /**
-   * Sets global xfo target parameter
+   * Sets the target parameter for this manipulator.
+   * This parameter will be modified by interactions on the manipulator.
    *
-   * @param param - The param param.
-   * @param track - The track param.
+   * @param param - The parameter that will be modified during manipulation
    */
-  setTargetParam(param: XfoParameter, track = true): void {
+  setTargetParam(param: XfoParameter): void {
     this.param = param
-    if (track) {
-      const __updateGizmo = () => {
-        this.globalXfoParam.value = param.getValue()
-      }
-      __updateGizmo()
-      param.on('valueChanged', __updateGizmo)
-    }
   }
 
   /**
    * Returns target's global xfo parameter.
    *
-   * @return {Parameter} - returns handle's target global Xfo.
+   * @return {Parameter} - returns parameter
    */
-  getTargetParam(): NumberParameter | XfoParameter {
+  getTargetParam(): XfoParameter {
     return this.param ? this.param : this.globalXfoParam
   }
 
@@ -83,7 +75,6 @@ class BaseAxialRotationHandle extends Handle {
     this.baseXfo = this.globalXfoParam.value.clone()
 
     this.vec0 = this.grabPos.subtract(this.baseXfo.tr)
-    this.grabCircleRadius = this.vec0.length()
     this.vec0.normalizeInPlace()
 
     // this.offsetXfo = this.localXfoParam.value.inverse()
@@ -92,7 +83,9 @@ class BaseAxialRotationHandle extends Handle {
       this.change = new SelectionXfoChange(Array.from(items), this.baseXfo)
       UndoRedoManager.getInstance().addChange(this.change)
     } else {
-      const param = this.getTargetParam()
+      const invBaseXfo = this.baseXfo.inverse()
+      const param = this.getTargetParam() as XfoParameter
+      this.handleToTargetXfo = invBaseXfo.multiply(param.value)
       this.change = new ParameterValueChange(param)
       UndoRedoManager.getInstance().addChange(this.change)
     }
@@ -105,29 +98,13 @@ class BaseAxialRotationHandle extends Handle {
    */
   onDrag(event: ZeaPointerEvent): void {
     const vec1 = this.holdPos.subtract(this.baseXfo.tr)
-    // const dragCircleRadius = vec1.length()
     vec1.normalizeInPlace()
-
-    // modulate the angle by the radius the mouse moves
-    // away from the center of the handle.
-    // This makes it possible to rotate the object more than
-    // 180 degrees in a single movement.
-    // Note: this modulator made rotations quite unpredictable
-    // especially when the angle between the ray and the plane is acute.
-    // disabling for now.
-    const modulator = 1.0 //dragCircleRadius / this.grabCircleRadius
-    let angle = this.vec0.angleTo(vec1) * modulator
+    let angle = this.vec0.angleTo(vec1)
     if (this.vec0.cross(vec1).dot(this.baseXfo.ori.getZaxis()) < 0) angle = -angle
 
-    if (this.range) {
-      angle = MathFunctions.clamp(angle, this.range[0], this.range[1])
-    }
-
     if ((event instanceof ZeaMouseEvent || event instanceof ZeaTouchEvent) && event.shiftKey) {
-      // modulat the angle to X degree increments.
-      const degree: number = 22.5
-      const rad: number = degree * (Math.PI / 180)
-      const increment = rad //Math.degToRad(22.5)
+      // modulate the angle to X degree increments.
+      const increment = MathFunctions.degToRad(22.5)
       angle = Math.floor(angle / increment) * increment
     }
 
@@ -138,8 +115,10 @@ class BaseAxialRotationHandle extends Handle {
       const selectionXfoChange = <SelectionXfoChange>this.change
       selectionXfoChange.setDeltaXfo(deltaXfo)
     } else {
-      const value = this.baseXfo.clone()
-      value.ori = deltaXfo.ori.multiply(value.ori)
+      // Add the values in global space.
+      const newBase = this.baseXfo.clone()
+      newBase.ori = deltaXfo.ori.multiply(newBase.ori)
+      const value = newBase.multiply(this.handleToTargetXfo)
 
       this.change.update({
         value,

src/Handles/Handle.ts

@@ -282,8 +282,12 @@ class Handle extends TreeItem {
     console.warn('@Handle#onDragEnd - Implement me!', event)
   }
 
-  // TODO:(check) added this method since we check for type Handle and call this method in XfoHandle.ts
-  setTargetParam(param: Parameter<unknown>, track: boolean): void {
+  /**
+   * Sets the target parameter for manipulation
+   *
+   * @param param - The parameter that will be modified during manipulation
+   */
+  setTargetParam(param: Parameter<unknown>): void {
     console.warn('setTargetParam not implemented')
   }
 }