Fixing handle collider rotation/alignment + comment consistency

Author: Finn Sinclair

Assets/MRTK/SDK/Features/UX/Scripts/BoundsControl/Visuals/PerAxisHandles.cs

@@ -9,7 +9,7 @@
 namespace Microsoft.MixedReality.Toolkit.UI.BoundsControl
 {
     /// <summary>
-    /// Rotation handles for <see cref="BoundsControl"/> that are used for rotating the
+    /// Handles for <see cref="BoundsControl"/> that are used for manipulating the
     /// Gameobject BoundsControl is attached to with near or far interaction
     /// </summary>
     public abstract class PerAxisHandles : HandlesBase
@@ -78,14 +78,14 @@ private void UpdateColliderType()
                 if (oldBoxCollider != null && config.HandlePrefabColliderType == HandlePrefabCollider.Sphere)
                 {
                     shouldCreateNewCollider = true;
-                    Object.Destroy(oldBoxCollider);
+                    Object.DestroyImmediate(oldBoxCollider);
                 }
 
                 var oldSphereCollider = handle.GetComponent<SphereCollider>();
                 if (oldSphereCollider != null && config.HandlePrefabColliderType == HandlePrefabCollider.Box)
                 {
                     shouldCreateNewCollider = true;
-                    Object.Destroy(oldSphereCollider);
+                    Object.DestroyImmediate(oldSphereCollider);
                 }
 
                 if (shouldCreateNewCollider)
@@ -185,10 +185,12 @@ private void CreateHandles(Transform parent)
                 handle.transform.position = HandlePositions[i];
                 handle.transform.parent = parent;
 
-                Bounds midpointBounds = CreateVisual(i, handle);
-                float maxDim = VisualUtils.GetMaxComponent(midpointBounds.size);
+                Bounds handleVisualBounds = CreateVisual(i, handle);
+                float maxDim = VisualUtils.GetMaxComponent(handleVisualBounds.size);
                 float invScale = maxDim == 0.0f ? 0.0f : config.HandleSize / maxDim;
-                VisualUtils.AddComponentsToAffordance(handle, new Bounds(midpointBounds.center * invScale, midpointBounds.size * invScale),
+
+                // TODO: Some subclasses of PerAxisHandles shouldn't use CursorContextInfo.CursorAction.Rotate
+                VisualUtils.AddComponentsToAffordance(handle, new Bounds(handleVisualBounds.center * invScale, handleVisualBounds.size * invScale),
                     config.HandlePrefabColliderType, CursorContextInfo.CursorAction.Rotate, config.ColliderPadding, parent, config.DrawTetherWhenManipulating);
 
                 handles.Add(handle.transform);
@@ -209,11 +211,11 @@ protected override void RecreateVisuals()
                 {
                     // get old child and remove it
                     obsoleteChild.parent = null;
-                    Object.Destroy(obsoleteChild.gameObject);
+                    Object.DestroyImmediate(obsoleteChild.gameObject);
                 }
                 else
                 {
-                    Debug.LogError("couldn't find rotation visual on recreating visuals");
+                    Debug.LogError("Couldn't find handle visual on recreating visuals");
                 }
 
                 // create new visual
@@ -247,40 +249,42 @@ protected override void UpdateColliderBounds(Transform handle, Vector3 visualSiz
 
         private Bounds CreateVisual(int handleIndex, GameObject parent)
         {
-            GameObject midpointVisual;
+            GameObject handleVisual;
             GameObject prefabType = config.HandlePrefab;
             if (prefabType != null)
             {
-                midpointVisual = Object.Instantiate(prefabType);
+                handleVisual = Object.Instantiate(prefabType);
             }
             else
             {
-                midpointVisual = GameObject.CreatePrimitive(PrimitiveType.Sphere);
-                // deactivate collider on visuals and register for deletion - actual collider
-                // of handle is attached to the handle gameobject, not the visual
-                var collider = midpointVisual.GetComponent<SphereCollider>();
+                handleVisual = GameObject.CreatePrimitive(PrimitiveType.Sphere);
+                // We only want the Primitive sphere mesh, but CreatePrimitive will
+                // give us a sphere collider too. Remove the sphere collider here
+                // so we can manually add our own properly configured collider later.
+                var collider = handleVisual.GetComponent<SphereCollider>();
                 collider.enabled = false;
-                Object.Destroy(collider);
-            }
-
-            Quaternion realignment = GetRotationRealignment(handleIndex);
-            midpointVisual.transform.localRotation = realignment * midpointVisual.transform.localRotation;
+                Object.DestroyImmediate(collider);
+            }           
 
-            Bounds midpointBounds = VisualUtils.GetMaxBounds(midpointVisual);
-            float maxDim = VisualUtils.GetMaxComponent(midpointBounds.size);
+            // handleVisualBounds are returned in handleVisual-local space.
+            Bounds handleVisualBounds = VisualUtils.GetMaxBounds(handleVisual);
+            float maxDim = VisualUtils.GetMaxComponent(handleVisualBounds.size);
             float invScale = maxDim == 0.0f ? 0.0f : config.HandleSize / maxDim;
 
-            midpointVisual.name = visualsName;
-            midpointVisual.transform.parent = parent.transform;
-            midpointVisual.transform.localScale = new Vector3(invScale, invScale, invScale);
-            midpointVisual.transform.localPosition = Vector3.zero;
+            handleVisual.name = visualsName;
+            handleVisual.transform.parent = parent.transform;
+            handleVisual.transform.localScale = new Vector3(invScale, invScale, invScale);
+            handleVisual.transform.localPosition = Vector3.zero;
+
+            Quaternion realignment = GetRotationRealignment(handleIndex);
+            parent.transform.localRotation = realignment;
 
             if (config.HandleMaterial != null)
             {
-                VisualUtils.ApplyMaterialToAllRenderers(midpointVisual, config.HandleMaterial);
+                VisualUtils.ApplyMaterialToAllRenderers(handleVisual, config.HandleMaterial);
             }
 
-            return midpointBounds;
+            return handleVisualBounds;
         }
 
         #region BoundsControlHandlerBase