Merge pull request #11412 from CesiumGS/depth-test

Fixed `depthTestAgainstTerrain` value being overridden when using the base layer picker widget
CesiumGS · Jul 12, 2023 · e658f01 · e658f01
2 parents 08f28d4 + 971e5b7
commit e658f01
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 25 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -13,7 +13,8 @@
 
 ##### Fixes :wrench:
 
-- Fixed the error report "rectanglePromise.then is not a function" that occurred when using `viewer.flyTo` to navigate to an ImageryLayer. [#11392](https://github.com/CesiumGS/cesium/pull/11392)
+- Fixed promise return value when using `viewer.flyTo` to navigate to an ImageryLayer. [#11392](https://github.com/CesiumGS/cesium/pull/11392)
+- Fixed `depthTestAgainstTerrain` value being overridden when using the base layer picker widget. [#11393](https://github.com/CesiumGS/cesium/issues/11393)
 
 ### 1.107 - 2023-07-03
 

diff --git a/packages/widgets/Source/BaseLayerPicker/BaseLayerPickerViewModel.js b/packages/widgets/Source/BaseLayerPicker/BaseLayerPickerViewModel.js
@@ -260,28 +260,38 @@ function BaseLayerPickerViewModel(options) {
         newProvider = value.creationCommand();
       }
 
-      let cancelUpdate = false;
-      const removeCancelListener = this._globe.terrainProviderChanged.addEventListener(
-        () => {
-          cancelUpdate = true;
-          removeCancelListener();
-        }
-      );
-      const terrain = new Terrain(Promise.resolve(newProvider));
-      const removeEventListener = terrain.readyEvent.addEventListener(
-        (terrainProvider) => {
-          if (cancelUpdate) {
-            // Early return in case something has outside of the picker.
-            return;
+      // If this is not a promise, we must set this synchronously to avoid overriding depthTestAgainstTerrain
+      // See https://github.com/CesiumGS/cesium/issues/6991
+      if (defined(newProvider) && !defined(newProvider.then)) {
+        this._globe.depthTestAgainstTerrain = !(
+          newProvider instanceof EllipsoidTerrainProvider
+        );
+        this._globe.terrainProvider = newProvider;
+      } else if (defined(newProvider)) {
+        let cancelUpdate = false;
+        const removeCancelListener = this._globe.terrainProviderChanged.addEventListener(
+          () => {
+            cancelUpdate = true;
+            removeCancelListener();
           }
+        );
 
-          this._globe.depthTestAgainstTerrain = !(
-            terrainProvider instanceof EllipsoidTerrainProvider
-          );
-          this._globe.terrainProvider = terrainProvider;
-          removeEventListener();
-        }
-      );
+        const terrain = new Terrain(newProvider);
+        const removeEventListener = terrain.readyEvent.addEventListener(
+          (terrainProvider) => {
+            if (cancelUpdate) {
+              // Early return in case something has changed outside of the picker.
+              return;
+            }
+
+            this._globe.depthTestAgainstTerrain = !(
+              terrainProvider instanceof EllipsoidTerrainProvider
+            );
+            this._globe.terrainProvider = terrainProvider;
+            removeEventListener();
+          }
+        );
+      }
 
       selectedTerrainViewModel(value);
       this.dropDownVisible = false;

diff --git a/packages/widgets/Specs/BaseLayerPicker/BaseLayerPickerViewModelSpec.js b/packages/widgets/Specs/BaseLayerPicker/BaseLayerPickerViewModelSpec.js
@@ -13,6 +13,7 @@ describe("Widgets/BaseLayerPicker/BaseLayerPickerViewModel", function () {
     this.imageryLayers = new ImageryLayerCollection();
     this.terrainProvider = new EllipsoidTerrainProvider();
     this.terrainProviderChanged = new Event();
+    this.depthTestAgainstTerrain = false;
   }
   MockGlobe.prototype.isDestroyed = () => false;
 
@@ -26,6 +27,15 @@ describe("Widgets/BaseLayerPicker/BaseLayerPickerViewModel", function () {
     tilingScheme: new GeographicTilingScheme(),
   };
 
+  const testEllipsoidProviderViewModel = new ProviderViewModel({
+    name: "name",
+    tooltip: "tooltip",
+    iconUrl: "url",
+    creationFunction: function () {
+      return new EllipsoidTerrainProvider();
+    },
+  });
+
   const testProviderViewModel = new ProviderViewModel({
     name: "name",
     tooltip: "tooltip",
@@ -268,13 +278,41 @@ describe("Widgets/BaseLayerPicker/BaseLayerPickerViewModel", function () {
     viewModel.selectedTerrain = testProviderViewModelAsync;
     await testProviderViewModelAsync.creationCommand();
     expect(globe.terrainProvider).toBe(testProvider);
+    expect(globe.depthTestAgainstTerrain).toBeTrue();
+  });
+
+  it("selectedTerrain sets ellipsoid terrain provider", async function () {
+    const terrainProviderViewModels = [testEllipsoidProviderViewModel];
+    const globe = new MockGlobe();
+    const viewModel = new BaseLayerPickerViewModel({
+      globe: globe,
+      terrainProviderViewModels: terrainProviderViewModels,
+    });
+
+    viewModel.selectedTerrain = testEllipsoidProviderViewModel;
+    await testProviderViewModelAsync.creationCommand();
+    expect(globe.terrainProvider).toBeInstanceOf(EllipsoidTerrainProvider);
+    expect(globe.depthTestAgainstTerrain).toBeFalse();
+  });
+
+  it("default does not override default value of depthTestAgainstTerrain", async function () {
+    const terrainProviderViewModels = [testEllipsoidProviderViewModel];
+    const globe = new MockGlobe();
+    // eslint-disable-next-line no-unused-vars
+    const viewModel = new BaseLayerPickerViewModel({
+      globe: globe,
+      terrainProviderViewModels: terrainProviderViewModels,
+    });
+
+    globe.depthTestAgainstTerrain = true;
+
+    await testEllipsoidProviderViewModel.creationCommand();
+    expect(globe.terrainProvider).toBeInstanceOf(EllipsoidTerrainProvider);
+    expect(globe.depthTestAgainstTerrain).toBeTrue();
   });
 
   it("selectedTerrain cancels update if terrainProvider is set externally", async function () {
-    const terrainProviderViewModels = [
-      testProviderViewModel,
-      testProviderViewModelAsync,
-    ];
+    const terrainProviderViewModels = [testProviderViewModel3];
     const globe = new MockGlobe();
     const viewModel = new BaseLayerPickerViewModel({
       globe: globe,