fix(TransformControlsWidget): address reviews

Author: floryst

Sources/Filters/Sources/TorusSource/index.js

@@ -14,10 +14,6 @@ function vtkTorusSource(publicAPI, model) {
   model.classHierarchy.push('vtkTorusSource');
 
   function requestData(inData, outData) {
-    if (model.deleted) {
-      return;
-    }
-
     let dataset = outData[0];
 
     // Points
@@ -29,13 +25,15 @@ function vtkTorusSource(publicAPI, model) {
 
     for (let ti = 0; ti <= model.tubeResolution; ti++) {
       const v = (ti / model.tubeResolution) * TAU;
+      const cosV = Math.cos(v);
+      const sinV = Math.sin(v);
       for (let ri = 0; ri <= model.resolution; ri++) {
         const u = (ri / model.resolution) * model.arcLength;
         points[pointIdx++] =
-          (model.radius + model.tubeRadius * Math.cos(v)) * Math.cos(u);
+          (model.radius + model.tubeRadius * cosV) * Math.cos(u);
         points[pointIdx++] =
-          (model.radius + model.tubeRadius * Math.cos(v)) * Math.sin(u);
-        points[pointIdx++] = model.tubeRadius * Math.sin(v);
+          (model.radius + model.tubeRadius * cosV) * Math.sin(u);
+        points[pointIdx++] = model.tubeRadius * sinV;
       }
     }
 

Sources/Rendering/Core/Prop3D/index.js

@@ -117,8 +117,14 @@ function vtkProp3D(publicAPI, model) {
   };
 
   publicAPI.setOrientationFromQuaternion = (q) => {
-    mat4.fromQuat(model.rotation, q);
-    publicAPI.modified();
+    const rotation = mat4.create();
+    mat4.fromQuat(rotation, q);
+    if (!vtkMath.areMatricesEqual(rotation, model.rotation)) {
+      model.rotation = rotation;
+      publicAPI.modified();
+      return true;
+    }
+    return false;
   };
 
   publicAPI.setUserMatrix = (matrix) => {

Sources/Widgets/Representations/TranslateTransformHandleRepresentation/TransformHandleSource.js

@@ -11,8 +11,10 @@ function rotatePolyData(pd, direction) {
     .rotateFromDirections([0, 1, 0], direction)
     .apply(points);
 
+  pd.getPoints().modified();
   pd.modified();
 }
+
 function translatePolyData(pd, translation) {
   const points = pd.getPoints().getData();
 
@@ -29,10 +31,6 @@ function vtkTransformHandleSource(publicAPI, model) {
   model.classHierarchy.push('vtkTransformHandleSource');
 
   function requestData(inData, outData) {
-    if (model.deleted) {
-      return;
-    }
-
     const cylinderSource = vtkCylinderSource.newInstance({
       height: model.height,
       initAngle: model.initAngle,
@@ -90,7 +88,7 @@ export function extend(publicAPI, model, initialValues = {}) {
   Object.assign(model, DEFAULT_VALUES, initialValues);
 
   vtkCylinderSource.extend(publicAPI, model, initialValues);
-  macro.algo(publicAPI, model, 1, 1);
+  macro.algo(publicAPI, model, 2, 1);
 
   vtkTransformHandleSource(publicAPI, model);
 }

Sources/Widgets/Widgets3D/TransformControlsWidget/behavior.js

@@ -23,7 +23,7 @@ export default function widgetBehavior(publicAPI, model) {
     dragStartCoord: [0, 0, 0],
   };
 
-  publicAPI.getBounds = () => [vtkBoundingBox.INIT_BOUNDS];
+  publicAPI.getBounds = () => [...vtkBoundingBox.INIT_BOUNDS];
 
   publicAPI.setDisplayCallback = (callback) =>
     model.representations[0].setDisplayCallback(callback);
@@ -83,10 +83,8 @@ export default function widgetBehavior(publicAPI, model) {
       scaleState.startScale = model.widgetState.getTransform().getScale()[
         axisIndex
       ];
-      scaleState.startDistFromOrigin = vec3.dist(
-        worldCoords,
-        model.activeState.getOrigin()
-      );
+      scaleState.startDistFromOrigin =
+        vec3.dist(worldCoords, model.activeState.getOrigin()) || 0.0001;
     }
   };
 

Sources/Widgets/Widgets3D/TransformControlsWidget/constants.js

@@ -3,3 +3,13 @@ export const TRANSLATE_HANDLE_RADIUS = 3;
 export const SCALE_HANDLE_RADIUS = 3;
 export const SCALE_HANDLE_CUBE_SIDE_LENGTH = 20;
 export const SCALE_HANDLE_PIXEL_SCALE = 320;
+
+export const TransformMode = {
+  TRANSLATE: 'translate',
+  SCALE: 'scale',
+  ROTATE: 'rotate',
+};
+
+export default {
+  TransformMode,
+};

Sources/Widgets/Widgets3D/TransformControlsWidget/example/index.js

@@ -10,6 +10,7 @@ import vtkMapper from '@kitware/vtk.js/Rendering/Core/Mapper';
 import vtkWidgetManager from '@kitware/vtk.js/Widgets/Core/WidgetManager';
 
 import vtkTransformControlsWidget from '@kitware/vtk.js/Widgets/Widgets3D/TransformControlsWidget';
+const { TransformMode } = vtkTransformControlsWidget;
 
 // ----------------------------------------------------------------------------
 // Standard rendering code setup
@@ -62,8 +63,6 @@ viewWidget.setActiveColor([255, 255, 0]);
 viewWidget.getRepresentations().forEach((rep) =>
   rep.getActors().forEach((actor) => {
     actor.getProperty().setAmbient(1);
-    actor.getProperty().setSpecular(0);
-    actor.getProperty().setDiffuse(1);
   })
 );
 
@@ -86,15 +85,15 @@ global.vw = viewWidget;
 window.onkeydown = (ev) => {
   switch (ev.key) {
     case 'q':
-      widget.setMode('rotate');
+      widget.setMode(TransformMode.ROTATE);
       renderWindow.render();
       break;
     case 't':
-      widget.setMode('translate');
+      widget.setMode(TransformMode.TRANSLATE);
       renderWindow.render();
       break;
     case 'x':
-      widget.setMode('scale');
+      widget.setMode(TransformMode.SCALE);
       renderWindow.render();
       break;
   }

Sources/Widgets/Widgets3D/TransformControlsWidget/index.js

@@ -11,6 +11,7 @@ import {
   SCALE_HANDLE_RADIUS,
   SCALE_HANDLE_CUBE_SIDE_LENGTH,
   SCALE_HANDLE_PIXEL_SCALE,
+  TransformMode,
 } from './constants';
 
 import widgetBehavior from './behavior';
@@ -127,11 +128,8 @@ function vtkTransformControlsWidget(publicAPI, model) {
     });
   };
 
-  const parentSetMode = publicAPI.setMode;
-  publicAPI.setMode = (mode) => {
-    if (parentSetMode(mode)) {
-      publicAPI.updateHandleVisibility();
-    }
+  model._onModeChanged = () => {
+    publicAPI.updateHandleVisibility();
   };
 
   // --- Widget Requirement ---------------------------------------------------
@@ -154,7 +152,7 @@ function vtkTransformControlsWidget(publicAPI, model) {
 // ----------------------------------------------------------------------------
 
 const DEFAULT_VALUES = {
-  mode: 'translate',
+  mode: TransformMode.TRANSLATE,
 };
 
 // ----------------------------------------------------------------------------
@@ -178,4 +176,4 @@ export const newInstance = macro.newInstance(
 
 // ----------------------------------------------------------------------------
 
-export default { newInstance, extend };
+export default { newInstance, extend, TransformMode };