Merge pull request #11844 from malaretv/camera-changed-roll

updating Camera.changed to account for changes in roll
CesiumGS · Feb 23, 2024 · ba6f5a3 · ba6f5a3
2 parents 1d0bf7b + ca88d4e
commit ba6f5a3
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 5 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -15,6 +15,7 @@
 - Fixed a bug affecting voxel shader compilation in WebGL1 contexts. [#11798](https://github.com/CesiumGS/cesium/pull/11798)
 - Fixed a bug where legacy B3DM files that contained glTF 1.0 data that used a `CONSTANT` technique in the `KHR_material_common` extension and only defined ambient- or emissive textures (but no diffuse textures) showed up without any texture [#11825](https://github.com/CesiumGS/cesium/pull/11825)
 - Fixed an error when the `screenSpaceEventHandler` was destroyed before `Viewer` [#10576](https://github.com/CesiumGS/cesium/issues/10576)
+- Fixes how `Camera.changed` handles changes in `roll`. [#11844](https://github.com/CesiumGS/cesium/pull/11844)
 
 ##### Additions :tada:
 

diff --git a/packages/engine/Source/Scene/Camera.js b/packages/engine/Source/Scene/Camera.js
@@ -217,6 +217,7 @@ function Camera(scene) {
   this._changedDirection = undefined;
   this._changedFrustum = undefined;
   this._changedHeading = undefined;
+  this._changedRoll = undefined;
 
   /**
    * The amount the camera has to change before the <code>changed</code> event is raised. The value is a percentage in the [0, 1] range.
@@ -380,25 +381,55 @@ Camera.prototype._updateCameraChanged = function () {
 
   const percentageChanged = camera.percentageChanged;
 
+  // check heading
   const currentHeading = camera.heading;
 
   if (!defined(camera._changedHeading)) {
     camera._changedHeading = currentHeading;
   }
 
-  let delta =
+  let headingDelta =
     Math.abs(camera._changedHeading - currentHeading) % CesiumMath.TWO_PI;
-  delta = delta > CesiumMath.PI ? CesiumMath.TWO_PI - delta : delta;
+  headingDelta =
+    headingDelta > CesiumMath.PI
+      ? CesiumMath.TWO_PI - headingDelta
+      : headingDelta;
 
   // Since delta is computed as the shortest distance between two angles
   // the percentage is relative to the half circle.
-  const headingChangedPercentage = delta / Math.PI;
+  const headingChangedPercentage = headingDelta / Math.PI;
 
   if (headingChangedPercentage > percentageChanged) {
-    camera._changed.raiseEvent(headingChangedPercentage);
     camera._changedHeading = currentHeading;
   }
 
+  // check roll
+  const currentRoll = camera.roll;
+
+  if (!defined(camera._changedRoll)) {
+    camera._changedRoll = currentRoll;
+  }
+
+  let rollDelta =
+    Math.abs(camera._changedRoll - currentRoll) % CesiumMath.TWO_PI;
+  rollDelta =
+    rollDelta > CesiumMath.PI ? CesiumMath.TWO_PI - rollDelta : rollDelta;
+
+  // Since delta is computed as the shortest distance between two angles
+  // the percentage is relative to the half circle.
+  const rollChangedPercentage = rollDelta / Math.PI;
+
+  if (rollChangedPercentage > percentageChanged) {
+    camera._changedRoll = currentRoll;
+  }
+  if (
+    rollChangedPercentage > percentageChanged ||
+    headingChangedPercentage > percentageChanged
+  ) {
+    camera._changed.raiseEvent(
+      Math.max(rollChangedPercentage, headingChangedPercentage)
+    );
+  }
   if (camera._mode === SceneMode.SCENE2D) {
     if (!defined(camera._changedFrustum)) {
       camera._changedPosition = Cartesian3.clone(

diff --git a/packages/engine/Specs/Scene/SceneSpec.js b/packages/engine/Specs/Scene/SceneSpec.js
@@ -1263,8 +1263,10 @@ describe(
       expect(args[0][0]).toBeGreaterThan(scene.camera.percentageChanged);
     });
 
-    it("raises the camera changed event on heading changed", function () {
+    it("raises the camera changed event on heading changed when looking up", function () {
       const spyListener = jasmine.createSpy("listener");
+
+      scene.camera.lookUp(0.785398);
       scene.camera.changed.addEventListener(spyListener);
 
       scene.initializeFrame();
@@ -1284,7 +1286,27 @@ describe(
       expect(args[0].length).toEqual(1);
       expect(args[0][0]).toBeGreaterThan(scene.camera.percentageChanged);
     });
+    it("raises the camera changed event on roll changed", function () {
+      const spyListener = jasmine.createSpy("listener");
+      scene.camera.changed.addEventListener(spyListener);
+
+      scene.initializeFrame();
+      scene.render();
+
+      scene.camera.twistLeft(
+        CesiumMath.PI * (scene.camera.percentageChanged + 0.1)
+      );
 
+      scene.initializeFrame();
+      scene.render();
+
+      expect(spyListener.calls.count()).toBe(1);
+
+      const args = spyListener.calls.allArgs();
+      expect(args.length).toEqual(1);
+      expect(args[0].length).toEqual(1);
+      expect(args[0][0]).toBeGreaterThan(scene.camera.percentageChanged);
+    });
     it("raises the camera changed event on position changed", function () {
       const spyListener = jasmine.createSpy("listener");
       scene.camera.changed.addEventListener(spyListener);