Merge pull request #6831 from TerriaJS/fix-time-filter-bug

Fix timefilter bug.

CHANGES.md

@@ -2,6 +2,7 @@
 
 #### next release (8.3.2)
 
+- Fixed a bug when restoring timefilter from a share link having more than one imagery item with the same base URL (but different layer names).
 - [The next improvement]
 
 #### 8.3.1 - 2023-06-29

lib/ModelMixins/TimeFilterMixin.ts

@@ -6,10 +6,13 @@ import {
   makeObservable,
   override
 } from "mobx";
+import Cartographic from "terriajs-cesium/Source/Core/Cartographic";
 import Ellipsoid from "terriajs-cesium/Source/Core/Ellipsoid";
 import JulianDate from "terriajs-cesium/Source/Core/JulianDate";
 import CesiumMath from "terriajs-cesium/Source/Core/Math";
 import Entity from "terriajs-cesium/Source/DataSources/Entity";
+import ImageryLayerFeatureInfo from "terriajs-cesium/Source/Scene/ImageryLayerFeatureInfo";
+import ImageryProvider from "terriajs-cesium/Source/Scene/ImageryProvider";
 import AbstractConstructor from "../Core/AbstractConstructor";
 import filterOutUndefined from "../Core/filterOutUndefined";
 import LatLonHeight from "../Core/LatLonHeight";
@@ -21,6 +24,7 @@ import {
 import CommonStrata from "../Models/Definition/CommonStrata";
 import createStratumInstance from "../Models/Definition/createStratumInstance";
 import Model from "../Models/Definition/Model";
+import TerriaFeature from "../Models/Feature/Feature";
 import TimeFilterTraits, {
   TimeFilterCoordinates
 } from "../Traits/TraitsClasses/TimeFilterTraits";
@@ -70,28 +74,78 @@ function TimeFilterMixin<T extends AbstractConstructor<BaseType>>(Base: T) {
       });
     }
 
+    /**
+     * Loads the time filter by querying the imagery layers at the given
+     * co-ordinates.
+     *
+     * @param coordinates.position The lat, lon, height of the location to pick in degrees
+     * @param coordinates.tileCoords the x, y, level co-ordinates of the tile to pick
+     * @returns Promise that fulfills when the picking is complete.
+     *   The promise resolves to `false` if the filter could not be set for some reason.
+     */
     @action
     async setTimeFilterFromLocation(coordinates: {
       position: LatLonHeight;
       tileCoords: ProviderCoords;
     }): Promise<boolean> {
-      const propertyName = this.timeFilterPropertyName;
-      if (propertyName === undefined || !MappableMixin.isMixedInto(this)) {
+      const timeProperty = this.timeFilterPropertyName;
+      if (
+        this.terria.allowFeatureInfoRequests === false ||
+        timeProperty === undefined ||
+        !MappableMixin.isMixedInto(this)
+      ) {
         return false;
       }
 
-      const resolved = await resolveFeature(
-        this,
-        propertyName,
-        coordinates.position,
-        coordinates.tileCoords
+      const pickTileCoordinates = coordinates.tileCoords;
+      const pickCartographicPosition = Cartographic.fromDegrees(
+        coordinates.position.longitude,
+        coordinates.position.latitude,
+        coordinates.position.height
+      );
+
+      // Pick all imagery provider for this item
+      const imageryProviders = filterOutUndefined(
+        this.mapItems.map((mapItem) =>
+          ImageryParts.is(mapItem) ? mapItem.imageryProvider : undefined
+        )
+      );
+      const picks = await pickImageryProviders(
+        imageryProviders,
+        pickTileCoordinates,
+        pickCartographicPosition
       );
 
-      if (resolved) {
-        this.setTimeFilterFeature(resolved.feature, resolved.providers);
-        return true;
+      // Find the first imageryProvider and feature with a matching time property
+      let timeFeature: TerriaFeature | undefined;
+      let imageryUrl: string | undefined;
+      for (let i = 0; i < picks.length; i++) {
+        const pick = picks[i];
+        const imageryFeature = pick.imageryFeatures.find(
+          (f) => f.properties[timeProperty] !== undefined
+        );
+        imageryUrl = (pick.imageryProvider as any).url;
+        if (imageryFeature && imageryUrl) {
+          imageryFeature.position =
+            imageryFeature.position ?? pickCartographicPosition;
+          timeFeature =
+            TerriaFeature.fromImageryLayerFeatureInfo(imageryFeature);
+          break;
+        }
+      }
+
+      if (!timeFeature || !imageryUrl) {
+        return false;
       }
-      return false;
+
+      // Update time filter
+      this.setTimeFilterFeature(timeFeature, {
+        [imageryUrl]: {
+          ...coordinates.position,
+          ...coordinates.tileCoords
+        }
+      });
+      return true;
     }
 
     get hasTimeFilterMixin() {
@@ -213,43 +267,35 @@ namespace TimeFilterMixin {
 }
 
 /**
- * Return the feature at position containing the time filter property.
+ * Picks all the imagery providers at the given coordinates and return a
+ * promise that resolves to the (imageryProvider, imageryFeatures) pairs.
  */
-const resolveFeature = action(async function (
-  model: MappableMixin.Instance & TimeVarying,
-  propertyName: string,
-  position: LatLonHeight,
-  tileCoords: ProviderCoords
-) {
-  const { latitude, longitude, height } = position;
-  const { x, y, level } = tileCoords;
-  const providers: ProviderCoordsMap = {};
-  model.mapItems.forEach((mapItem) => {
-    if (ImageryParts.is(mapItem)) {
-      // @ts-ignore
-      providers[mapItem.imageryProvider.url] = { x, y, level };
-    }
-  });
-  const viewer = model.terria.mainViewer.currentViewer;
-  const features = await viewer.getFeaturesAtLocation(
-    { latitude, longitude, height },
-    providers
+async function pickImageryProviders(
+  imageryProviders: ImageryProvider[],
+  pickCoordinates: ProviderCoords,
+  pickPosition: Cartographic
+): Promise<
+  {
+    imageryProvider: ImageryProvider;
+    imageryFeatures: ImageryLayerFeatureInfo[];
+  }[]
+> {
+  return filterOutUndefined(
+    await Promise.all(
+      imageryProviders.map((imageryProvider) =>
+        imageryProvider
+          .pickFeatures(
+            pickCoordinates.x,
+            pickCoordinates.y,
+            pickCoordinates.level,
+            pickPosition.longitude,
+            pickPosition.latitude
+          )
+          ?.then((imageryFeatures) => ({ imageryProvider, imageryFeatures }))
+      )
+    )
   );
-
-  const feature = (features || []).find((feature) => {
-    if (!feature.properties) {
-      return false;
-    }
-
-    const prop = feature.properties[propertyName];
-    const times = prop?.getValue(model.currentTimeAsJulianDate);
-    return Array.isArray(times) && times.length > 0;
-  });
-
-  if (feature) {
-    return { feature, providers };
-  }
-});
+}
 
 function coordinatesFromTraits(traits: Model<TimeFilterCoordinates>) {
   const {

lib/Models/Cesium.ts

@@ -1358,44 +1358,6 @@ export default class Cesium extends GlobeOrMap {
     }
   }
 
-  /**
-   * Return features at a latitude, longitude and (optionally) height for the given imagery layers.
-   * @param latLngHeight The position on the earth to pick
-   * @param tileCoords A map of imagery provider urls to the tile coords used to get features for those imagery
-   * @returns A flat array of all the features for the given tiles that are currently on the map
-   */
-  async getFeaturesAtLocation(
-    latLngHeight: LatLonHeight,
-    providerCoords: ProviderCoordsMap,
-    existingFeatures: TerriaFeature[] = []
-  ) {
-    const pickPosition = this.scene.globe.ellipsoid.cartographicToCartesian(
-      Cartographic.fromDegrees(
-        latLngHeight.longitude,
-        latLngHeight.latitude,
-        latLngHeight.height
-      )
-    );
-    const pickPositionCartographic =
-      Ellipsoid.WGS84.cartesianToCartographic(pickPosition);
-
-    const promises = this.terria.allowFeatureInfoRequests
-      ? this.pickImageryLayerFeatures(pickPositionCartographic, providerCoords)
-      : [];
-
-    const pickedFeatures = this._buildPickedFeatures(
-      providerCoords,
-      pickPosition,
-      existingFeatures,
-      filterOutUndefined(promises),
-      pickPositionCartographic.height,
-      false
-    );
-
-    await pickedFeatures.allFeaturesAvailablePromise;
-    return pickedFeatures.features;
-  }
-
   private pickImageryLayerFeatures(
     pickPosition: Cartographic,
     providerCoords: ProviderCoordsMap

lib/Models/Feature/Feature.ts

@@ -1,8 +1,11 @@
-import { observable, makeObservable } from "mobx";
+import { makeObservable, observable } from "mobx";
+import Ellipsoid from "terriajs-cesium/Source/Core/Ellipsoid";
+import ConstantPositionProperty from "terriajs-cesium/Source/DataSources/ConstantPositionProperty";
 import Entity from "terriajs-cesium/Source/DataSources/Entity";
 import Cesium3DTileFeature from "terriajs-cesium/Source/Scene/Cesium3DTileFeature";
 import Cesium3DTilePointFeature from "terriajs-cesium/Source/Scene/Cesium3DTilePointFeature";
 import ImageryLayer from "terriajs-cesium/Source/Scene/ImageryLayer";
+import ImageryLayerFeatureInfo from "terriajs-cesium/Source/Scene/ImageryLayerFeatureInfo";
 import { JsonObject } from "../../Core/Json";
 import { BaseModel } from "../Definition/Model";
 import { TerriaFeatureData } from "./FeatureData";
@@ -73,6 +76,23 @@ export default class TerriaFeature extends Entity {
     }
     return feature;
   }
+
+  static fromImageryLayerFeatureInfo(imageryFeature: ImageryLayerFeatureInfo) {
+    const feature = new TerriaFeature({
+      id: imageryFeature.name,
+      name: imageryFeature.name,
+      description: imageryFeature.description,
+      properties: imageryFeature.properties
+    });
+    feature.data = imageryFeature.data;
+    feature.imageryLayer = imageryFeature.imageryLayer;
+    if (imageryFeature.position) {
+      feature.position = new ConstantPositionProperty(
+        Ellipsoid.WGS84.cartographicToCartesian(imageryFeature.position)
+      );
+    }
+    return feature;
+  }
 }
 
 // Features have 'entityCollection', 'properties' and 'data' properties, which we must add to the entity's property list,

lib/Models/GlobeOrMap.ts

@@ -1,10 +1,4 @@
-import {
-  action,
-  computed,
-  observable,
-  runInAction,
-  makeObservable
-} from "mobx";
+import { action, makeObservable, observable, runInAction } from "mobx";
 import Cartesian2 from "terriajs-cesium/Source/Core/Cartesian2";
 import Cartesian3 from "terriajs-cesium/Source/Core/Cartesian3";
 import Color from "terriajs-cesium/Source/Core/Color";
@@ -15,9 +9,7 @@ import Rectangle from "terriajs-cesium/Source/Core/Rectangle";
 import ColorMaterialProperty from "terriajs-cesium/Source/DataSources/ColorMaterialProperty";
 import ConstantPositionProperty from "terriajs-cesium/Source/DataSources/ConstantPositionProperty";
 import ConstantProperty from "terriajs-cesium/Source/DataSources/ConstantProperty";
-import Entity from "terriajs-cesium/Source/DataSources/Entity";
 import ImageryLayerFeatureInfo from "terriajs-cesium/Source/Scene/ImageryLayerFeatureInfo";
-import ImageryProvider from "terriajs-cesium/Source/Scene/ImageryProvider";
 import SplitDirection from "terriajs-cesium/Source/Scene/SplitDirection";
 import isDefined from "../Core/isDefined";
 import { isJsonObject } from "../Core/Json";
@@ -141,17 +133,6 @@ export default abstract class GlobeOrMap {
     existingFeatures: TerriaFeature[]
   ): void;
 
-  /**
-   * Return features at a latitude, longitude and (optionally) height for the given imagery layers.
-   * @param latLngHeight The position on the earth to pick
-   * @param providerCoords A map of imagery provider urls to the tile coords used to get features for those imagery
-   * @returns A flat array of all the features for the given tiles that are currently on the map
-   */
-  abstract getFeaturesAtLocation(
-    latLngHeight: LatLonHeight,
-    providerCoords: ProviderCoordsMap
-  ): Promise<Entity[] | undefined> | void;
-
   /**
    * Creates a {@see Feature} (based on an {@see Entity}) from a {@see ImageryLayerFeatureInfo}.
    * @param imageryFeature The imagery layer feature for which to create an entity-based feature.

lib/Models/Leaflet.ts

@@ -1,13 +1,12 @@
-import i18next from "i18next";
 import { GridLayer } from "leaflet";
 import {
   action,
   autorun,
   computed,
+  makeObservable,
   observable,
   reaction,
-  runInAction,
-  makeObservable
+  runInAction
 } from "mobx";
 import { computedFn } from "mobx-utils";
 import cesiumCancelAnimationFrame from "terriajs-cesium/Source/Core/cancelAnimationFrame";
@@ -61,7 +60,6 @@ import hasTraits from "./Definition/hasTraits";
 import TerriaFeature from "./Feature/Feature";
 import GlobeOrMap from "./GlobeOrMap";
 import { LeafletAttribution } from "./LeafletAttribution";
-import MapInteractionMode from "./MapInteractionMode";
 import Terria from "./Terria";
 
 // We want TS to look at the type declared in lib/ThirdParty/terriajs-cesium-extra/index.d.ts
@@ -583,46 +581,6 @@ export default class Leaflet extends GlobeOrMap {
     );
   }
 
-  /**
-   * Return features at a latitude, longitude and (optionally) height for the given imageryLayer.
-   * @param latLngHeight The position on the earth to pick
-   * @param providerCoords A map of imagery provider urls to the tile coords used to get features for those imagery
-   * @returns A flat array of all the features for the given tiles that are currently on the map
-   */
-  @action
-  async getFeaturesAtLocation(
-    latLngHeight: LatLonHeight,
-    providerCoords: ProviderCoordsMap
-  ) {
-    const pickMode = new MapInteractionMode({
-      message: i18next.t("models.imageryLayer.resolvingAvailability"),
-      onCancel: () => {
-        this.terria.mapInteractionModeStack.pop();
-      }
-    });
-    this.terria.mapInteractionModeStack.push(pickMode);
-    this._pickFeatures(
-      L.latLng({
-        lat: latLngHeight.latitude,
-        lng: latLngHeight.longitude,
-        alt: latLngHeight.height
-      }),
-      providerCoords,
-      [],
-      true
-    );
-
-    if (pickMode.pickedFeatures) {
-      const pickedFeatures = pickMode.pickedFeatures;
-      await pickedFeatures.allFeaturesAvailablePromise;
-    }
-
-    return runInAction(() => {
-      this.terria.mapInteractionModeStack.pop();
-      return pickMode.pickedFeatures?.features;
-    });
-  }
-
   /*
    * There are two "listeners" for clicks which are set up in our constructor.
    * - One fires for any click: `map.on('click', ...`.  It calls `pickFeatures`.