Merge pull request #480 from CesiumGS/globe-anchor-disabled-pos

Fix changing CesiumGlobeAnchor position while disabled

Author: kring

CHANGES.md

@@ -1,5 +1,11 @@
 # Change Log
 
+## ? - ?
+
+##### Fixes :wrench:
+
+- Fixed a bug that caused a `NullReferenceException` when attempting to get or set the `longitudeLatitudeHeight` property on a disabled `CesiumGlobeAnchor`.
+
 ### v1.11.0 - 2024-07-01
 
 ##### Additions :tada:

Runtime/CesiumCameraController.cs

@@ -408,6 +408,12 @@ void FixedUpdate()
 
         private bool RaycastTowardsEarthCenter(out float hitDistance)
         {
+            if (this._georeference == null)
+            {
+                hitDistance = 0.0f;
+                return false;
+            }
+
             double3 center =
                 this._georeference.TransformEarthCenteredEarthFixedPositionToUnity(new double3(0.0));
 

Runtime/CesiumGlobeAnchor.cs

@@ -185,9 +185,28 @@ public double4x4 localToGlobeFixedMatrix
         /// </remarks>
         public double3 longitudeLatitudeHeight
         {
-            get => this._georeference.ellipsoid.CenteredFixedToLongitudeLatitudeHeight(this.positionGlobeFixed);
+            get
+            {
+                this.UpdateGeoreferenceIfNecessary();
+
+                if (this._georeference == null || this._georeference.ellipsoid == null)
+                {
+                    // Cannot get property if there is no georeference, or the georeference has no ellipsoid.
+                    return new double3(0.0, 0.0, 0.0);
+                }
+
+                return this._georeference.ellipsoid.CenteredFixedToLongitudeLatitudeHeight(this.positionGlobeFixed);
+            }
             set
             {
+                this.UpdateGeoreferenceIfNecessary();
+
+                if (this._georeference == null || this._georeference.ellipsoid == null)
+                {
+                    // Cannot set property if there is no georeference, or the georeference has no ellipsoid.
+                    return;
+                }
+
                 this.positionGlobeFixed = this._georeference.ellipsoid.LongitudeLatitudeHeightToCenteredFixed(value);
             }
         }
@@ -449,10 +468,10 @@ public void Sync()
         {
             // If the ellipsoid changed since last sync, we need to update from transform since our ECEF mapping
             // is going to be invalid.
-            bool isEllipsoidChanged = _lastEllipsoidRadii.HasValue ?
-                (_lastEllipsoidRadii.Value.x != _georeference.ellipsoid.radii.x ||
-                _lastEllipsoidRadii.Value.y != _georeference.ellipsoid.radii.y ||
-                _lastEllipsoidRadii.Value.z != _georeference.ellipsoid.radii.z) : true;
+            bool isEllipsoidChanged = this._lastEllipsoidRadii.HasValue && this._georeference != null ?
+                (this._lastEllipsoidRadii.Value.x != this._georeference.ellipsoid.radii.x ||
+                this._lastEllipsoidRadii.Value.y != this._georeference.ellipsoid.radii.y ||
+                this._lastEllipsoidRadii.Value.z != this._georeference.ellipsoid.radii.z) : true;
 
             // If we don't have a local -> globe fixed matrix yet, we must update from the Transform
             bool updateFromTransform = !this._localToGlobeFixedMatrixIsValid;

Runtime/CesiumOriginShift.cs

@@ -57,6 +57,12 @@ void LateUpdate()
         {
             CesiumGeoreference georeference = this.GetComponentInParent<CesiumGeoreference>();
 
+            if (georeference == null)
+            {
+                Debug.LogWarning("CesiumOriginShift is doing nothing because it is not nested inside a game object with a CesiumGeoreference component.");
+                return;
+            }
+
             CesiumGlobeAnchor anchor = this.GetComponent<CesiumGlobeAnchor>();
 
             // The RequireComponent attribute should ensure the globe anchor exists, but it may not be active.

Tests/TestCesiumGlobeAnchor.cs

@@ -290,4 +290,27 @@ public void GivesCorrectResultsForDifferentEllipsoids()
         anchor.Sync();
         Assert.That(anchor.positionGlobeFixed, Is.EqualTo(actualPosEcef).Using(epsilon6));
     }
+
+    [Test]
+    public void CanChangePositionWhileDisabled()
+    {
+        IEqualityComparer<double3> epsilon6 = Comparers.Double3(1e-6, 1e-4);
+
+        double3 positionLlh = new double3(-20, -10, 1000.0);
+
+        GameObject goGeoreference = new GameObject("Georeference");
+        CesiumGeoreference georeference = goGeoreference.AddComponent<CesiumGeoreference>();
+        georeference.ellipsoid = CesiumEllipsoid.WGS84;
+        georeference.SetOriginLongitudeLatitudeHeight(positionLlh.x, positionLlh.y, positionLlh.z);
+
+        GameObject goAnchored = new GameObject("Anchored");
+        goAnchored.transform.parent = goGeoreference.transform;
+        goAnchored.transform.SetPositionAndRotation(new Vector3(0, 0, 0), Quaternion.identity);
+        goAnchored.SetActive(false);
+
+        CesiumGlobeAnchor anchor = goAnchored.AddComponent<CesiumGlobeAnchor>();
+        anchor.enabled = false;
+        anchor.longitudeLatitudeHeight = new double3(1, 1, 1);
+        Assert.That(anchor.longitudeLatitudeHeight, Is.EqualTo(new double3(1, 1, 1)).Using(epsilon6));
+    }
 }

native~/Runtime/src/CesiumGlobeAnchorImpl.cpp

@@ -28,12 +28,12 @@ namespace {
 
 const CesiumGeospatial::Ellipsoid&
 getAnchorEllipsoid(const ::DotNet::CesiumForUnity::CesiumGlobeAnchor& anchor) {
-
   anchor.UpdateGeoreferenceIfNecessary();
-  return anchor._georeference()
-      .ellipsoid()
-      .NativeImplementation()
-      .GetEllipsoid();
+  CesiumForUnity::CesiumGeoreference georeference = anchor._georeference();
+  if (georeference == nullptr) {
+    return CesiumGeospatial::Ellipsoid::WGS84;
+  }
+  return georeference.ellipsoid().NativeImplementation().GetEllipsoid();
 }
 
 GlobeAnchor createOrUpdateNativeGlobeAnchorFromEcef(