Merged PR 30291: #129251 - Clean up background map API

#129251 - Clean up background map API
BackgroundMapProps is awkward to use - everywhere it's used there's error-prone duplicated code to try to validate provider name, apply default values, etc. Use it only as wire format - use BackgroundMapSettings for actual in-memory representation.
Allow a subset of map settings to be changed.
Add Viewport API to ensure view updates in response to changes.

Related work items: #129251

common/api/imodeljs-common.api.md

@@ -283,15 +283,31 @@ export enum BackgroundFill {
 
 // @public
 export interface BackgroundMapProps {
-    // (undocumented)
     groundBias?: number;
-    // (undocumented)
     providerData?: {
         mapType?: BackgroundMapType;
     };
     providerName?: string;
 }
 
+// @beta
+export type BackgroundMapProviderName = "BingProvider" | "MapBoxProvider";
+
+// @beta
+export class BackgroundMapSettings {
+    constructor(providerName?: BackgroundMapProviderName, mapType?: BackgroundMapType, groundBias?: number);
+    clone(changedProps?: BackgroundMapProps): BackgroundMapSettings;
+    // (undocumented)
+    equals(other: BackgroundMapSettings): boolean;
+    equalsJSON(json?: BackgroundMapProps): boolean;
+    static fromJSON(json?: BackgroundMapProps): BackgroundMapSettings;
+    readonly groundBias: number;
+    readonly mapType: BackgroundMapType;
+    readonly providerName: BackgroundMapProviderName;
+    // (undocumented)
+    toJSON(): BackgroundMapProps;
+}
+
 // @public
 export enum BackgroundMapType {
     // (undocumented)
@@ -1213,7 +1229,7 @@ export class DisplayStyleSettings {
     addExcludedElements(id: Id64String): void;
     backgroundColor: ColorDef;
     // @alpha (undocumented)
-    backgroundMap: BackgroundMapProps | undefined;
+    backgroundMap: BackgroundMapSettings;
     dropExcludedElement(id: Id64String): void;
     dropSubCategoryOverride(id: Id64String): void;
     // @internal (undocumented)

common/api/imodeljs-frontend.api.md

@@ -16,6 +16,8 @@ import { AuxCoordSystem3dProps } from '@bentley/imodeljs-common';
 import { AuxCoordSystemProps } from '@bentley/imodeljs-common';
 import { AxisAlignedBox3d } from '@bentley/imodeljs-common';
 import { BackgroundMapProps } from '@bentley/imodeljs-common';
+import { BackgroundMapProviderName } from '@bentley/imodeljs-common';
+import { BackgroundMapSettings } from '@bentley/imodeljs-common';
 import { BackgroundMapType } from '@bentley/imodeljs-common';
 import { BatchType } from '@bentley/imodeljs-common';
 import { BeDuration } from '@bentley/bentleyjs-core';
@@ -1127,15 +1129,17 @@ export abstract class AuxCoordSystemState extends ElementState implements AuxCoo
 
 // @internal
 export class BackgroundMapProvider extends BaseTiledMapProvider implements TiledGraphicsProvider.Provider {
-    constructor(json: BackgroundMapProps, iModel: IModelConnection);
-    // (undocumented)
-    equalsProps(props: BackgroundMapProps): boolean;
+    constructor(settings: BackgroundMapSettings, iModel: IModelConnection);
     // (undocumented)
     getTileTree(viewport: Viewport): TiledGraphicsProvider.Tree | undefined;
     // (undocumented)
-    mapType: BackgroundMapType;
+    readonly groundBias: number;
+    // (undocumented)
+    readonly mapType: BackgroundMapType;
+    // (undocumented)
+    readonly providerName: BackgroundMapProviderName;
     // (undocumented)
-    providerName: string;
+    readonly settings: BackgroundMapSettings;
 }
 
 // @alpha
@@ -1151,16 +1155,18 @@ export interface BasePropertyValue {
 }
 
 // @internal
-export class BaseTiledMapProvider {
-    constructor(iModel: IModelConnection, groundBias: number);
+export abstract class BaseTiledMapProvider implements IDisposable {
+    constructor(iModel: IModelConnection);
     // (undocumented)
     decorate(context: DecorateContext): void;
     // (undocumented)
+    dispose(): void;
+    // (undocumented)
     getPlane(): Plane3dByOriginAndUnitNormal;
     // (undocumented)
     getTilesForView(viewport: ScreenViewport): Tile[];
     // (undocumented)
-    protected _groundBias: number;
+    abstract readonly groundBias: number;
     // (undocumented)
     protected _imageryProvider?: ImageryProvider;
     // (undocumented)
@@ -1866,6 +1872,10 @@ export abstract class DisplayStyleState extends ElementState implements DisplayS
     readonly backgroundMap: BackgroundMapProvider;
     // @internal (undocumented)
     readonly backgroundMapPlane: Plane3dByOriginAndUnitNormal | undefined;
+    // @beta (undocumented)
+    backgroundMapSettings: BackgroundMapSettings;
+    // @beta
+    changeBackgroundMapProps(props: BackgroundMapProps): void;
     // @internal (undocumented)
     static readonly className: string;
     // @internal (undocumented)
@@ -1890,7 +1900,7 @@ export abstract class DisplayStyleState extends ElementState implements DisplayS
     removeContextRealityModel(index: number): void;
     // @internal (undocumented)
     readonly scheduleScript: RenderScheduleState.Script | undefined;
-    // @alpha
+    // @deprecated
     setBackgroundMap(mapProps: BackgroundMapProps): void;
     abstract readonly settings: DisplayStyleSettings;
     viewFlags: ViewFlags;
@@ -4126,7 +4136,9 @@ export class OverlayMapProvider extends BaseTiledMapProvider implements TiledGra
     constructor(imageryProvider: ImageryProvider, groundBias: number, iModel: IModelConnection);
     // (undocumented)
     getTileTree(_viewport: Viewport): TiledGraphicsProvider.Tree | undefined;
-}
+    // (undocumented)
+    readonly groundBias: number;
+    }
 
 // @internal (undocumented)
 export interface PackedFeature {
@@ -7859,6 +7871,10 @@ export abstract class Viewport implements IDisposable {
     readonly auxCoordSystem: AuxCoordSystemState;
     // @internal (undocumented)
     readonly backgroundMapPlane: Plane3dByOriginAndUnitNormal | undefined;
+    // @beta
+    backgroundMapSettings: BackgroundMapSettings;
+    // @beta
+    changeBackgroundMapProps(props: BackgroundMapProps): void;
     changeCategoryDisplay(categories: Id64Arg, display: boolean, enableAllSubCategories?: boolean): void;
     // @internal (undocumented)
     changeDynamics(dynamics: GraphicList | undefined): void;

common/api/summary/imodeljs-common.exports.csv

@@ -14,6 +14,8 @@ public;AxisAlignedBox3dProps = Range3dProps
 public;BackendError 
 public;BackgroundFill
 public;BackgroundMapProps
+beta;BackgroundMapProviderName = "BingProvider" | "MapBoxProvider"
+beta;BackgroundMapSettings
 public;BackgroundMapType
 beta;BatchType
 public;class BentleyCloudRpcConfiguration 

common/api/summary/imodeljs-frontend.exports.csv

@@ -26,7 +26,7 @@ public;class AuxCoordSystemState
 internal;BackgroundMapProvider 
 alpha;BasePropertyEditorParams
 beta;BasePropertyValue
-internal;BaseTiledMapProvider
+internal;class BaseTiledMapProvider 
 public;BeButton
 public;BeButtonEvent 
 public;BeButtonEventProps

common/changes/@bentley/imodeljs-backend/background-map-api_2019-06-05-19-40.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "",
+      "packageName": "@bentley/imodeljs-backend",
+      "type": "none"
+    }
+  ],
+  "packageName": "@bentley/imodeljs-backend",
+  "email": "[email protected]"
+}

common/changes/@bentley/imodeljs-common/background-map-api_2019-06-05-13-19.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "",
+      "packageName": "@bentley/imodeljs-common",
+      "type": "none"
+    }
+  ],
+  "packageName": "@bentley/imodeljs-common",
+  "email": "[email protected]"
+}

common/changes/@bentley/imodeljs-frontend/background-map-api_2019-06-04-18-40.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "Cleaned up background map API",
+      "packageName": "@bentley/imodeljs-frontend",
+      "type": "none"
+    }
+  ],
+  "packageName": "@bentley/imodeljs-frontend",
+  "email": "[email protected]"
+}

core/backend/src/IModelDb.ts

@@ -555,6 +555,12 @@ export class IModelDb extends IModel {
         resolve({ status: QueryResponseStatus.PostError, rows: [] });
 
       const poll = () => {
+        // We are in an async timer callback. We must verify the briefcase was not closed before we were called back.
+        if (!this.briefcase || !this.briefcase.nativeDb || !this.briefcase.nativeDb.isOpen()) {
+          resolve({ status: QueryResponseStatus.Done, rows: [] });
+          return;
+        }
+
         const pollrc = this.nativeDb.pollConcurrentQuery(postrc.taskId);
         if (pollrc.status === PollStatus.Done)
           resolve({ status: QueryResponseStatus.Done, rows: JSON.parse(pollrc.result, reviver) });

core/common/src/ViewProps.ts

@@ -146,14 +146,96 @@ export enum BackgroundMapType {
  * @public
  */
 export interface BackgroundMapProps {
+  /** The elevation of the map in meters relative to sea level. Default value: 0. */
   groundBias?: number;
-  /** The Id of a map tile provider. Currently the type should technically be `"BingProvider" | "MapBoxProvider"`, but support for other providers may be added in the future. */
+  /** Identifies the source of the map tiles. Currently supported providers are "BingProvider" and "MapBoxProvider". Support for additional providers may be added in the future.
+   *
+   * Default value: "BingProvider"
+   */
   providerName?: string;
+  /** Options for customizing the tiles supplied by the provider. If undefined, default values of all members are used. */
   providerData?: {
+    /** The type of map graphics to request. Default value: BackgroundMapType.Hybrid. */
     mapType?: BackgroundMapType;
   };
 }
 
+/** The current set of supported background map providers.
+ * @beta
+ */
+export type BackgroundMapProviderName = "BingProvider" | "MapBoxProvider";
+
+/** Normalized representation of a [[BackgroundMapProps]] for which type and provider have been validated and default values have been applied where explicit values not defined.
+ * @beta
+ */
+export class BackgroundMapSettings {
+  /** Elevation in meters, relative to sea level. */
+  public readonly groundBias: number;
+  /** Identifies the provider from which map graphics will be obtained. */
+  public readonly providerName: BackgroundMapProviderName;
+  /** The type of map graphics to be drawn. */
+  public readonly mapType: BackgroundMapType;
+
+  public constructor(providerName: BackgroundMapProviderName = "BingProvider", mapType: BackgroundMapType = BackgroundMapType.Hybrid, groundBias: number = 0) {
+    this.groundBias = groundBias;
+    this.providerName = providerName;
+    switch (mapType) {
+      case BackgroundMapType.Street:
+      case BackgroundMapType.Aerial:
+        this.mapType = mapType;
+        break;
+      default:
+        this.mapType = BackgroundMapType.Hybrid;
+    }
+  }
+
+  /** Construct from JSON, performing validation and applying default values for undefined fields. */
+  public static fromJSON(json?: BackgroundMapProps): BackgroundMapSettings {
+    if (undefined === json)
+      return new BackgroundMapSettings();
+
+    const providerName = ("MapBoxProvider" === json.providerName) ? "MapBoxProvider" : "BingProvider";
+    const mapType = (undefined !== json.providerData) ? json.providerData.mapType : BackgroundMapType.Hybrid;
+    return new BackgroundMapSettings(providerName, mapType, json.groundBias);
+  }
+
+  public toJSON(): BackgroundMapProps {
+    return {
+      groundBias: this.groundBias,
+      providerName: this.providerName,
+      providerData: { mapType: this.mapType },
+    };
+  }
+
+  /** Returns true if these settings are equivalent to the supplied JSON settings. */
+  public equalsJSON(json?: BackgroundMapProps): boolean {
+    return this.equals(BackgroundMapSettings.fromJSON(json));
+  }
+
+  public equals(other: BackgroundMapSettings): boolean {
+    return this.groundBias === other.groundBias && this.providerName === other.providerName && this.mapType === other.mapType;
+  }
+
+  /** Create a copy of this BackgroundMapSettings, optionally modifying some of its properties.
+   * @param changedProps JSON representation of the properties to change.
+   * @returns A BackgroundMapSettings with all of its properties set to match those of `this`, except those explicitly defined in `changedProps`.
+   */
+  public clone(changedProps?: BackgroundMapProps): BackgroundMapSettings {
+    if (undefined === changedProps)
+      return this;
+
+    const props = {
+      providerName: undefined !== changedProps.providerName ? changedProps.providerName : this.providerName,
+      groundBias: undefined !== changedProps.groundBias ? changedProps.groundBias : this.groundBias,
+      providerData: {
+        mapType: undefined !== changedProps.providerData && undefined !== changedProps.providerData.mapType ? changedProps.providerData.mapType : this.mapType,
+      },
+    };
+
+    return BackgroundMapSettings.fromJSON(props);
+  }
+}
+
 /** JSON representation of a [[GroundPlane]].
  * @public
  */
@@ -418,6 +500,7 @@ export class DisplayStyleSettings {
   private readonly _monochrome: ColorDef;
   private readonly _subCategoryOverrides: Map<string, SubCategoryOverride> = new Map<string, SubCategoryOverride>();
   private readonly _excludedElements: Set<Id64String> = new Set<Id64String>();
+  private _backgroundMap: BackgroundMapSettings;
 
   /** Construct a new DisplayStyleSettings from an [[ElementProps.jsonProperties]].
    * @param jsonProperties An object with an optional `styles` property containing a display style's settings.
@@ -428,10 +511,12 @@ export class DisplayStyleSettings {
   public constructor(jsonProperties: { styles?: DisplayStyleSettingsProps }) {
     if (undefined === jsonProperties.styles)
       jsonProperties.styles = {};
+
     this._json = jsonProperties.styles;
     this._viewFlags = ViewFlags.fromJSON(this._json.viewflags);
     this._background = ColorDef.fromJSON(this._json.backgroundColor);
     this._monochrome = undefined !== this._json.monochromeColor ? ColorDef.fromJSON(this._json.monochromeColor) : ColorDef.white.clone();
+    this._backgroundMap = BackgroundMapSettings.fromJSON(this._json.backgroundMap);
 
     const ovrsArray = JsonUtils.asArray(this._json.subCategoryOvr);
     if (undefined !== ovrsArray) {
@@ -487,12 +572,15 @@ export class DisplayStyleSettings {
   }
 
   /** @alpha */
-  public get backgroundMap(): BackgroundMapProps | undefined {
-    const props = this._json.backgroundMap;
-    return undefined !== props ? props : {};
-  }
+  public get backgroundMap(): BackgroundMapSettings { return this._backgroundMap; }
+
   /** @alpha */
-  public set backgroundMap(map: BackgroundMapProps | undefined) { this._json.backgroundMap = map; }
+  public set backgroundMap(map: BackgroundMapSettings) {
+    if (!this.backgroundMap.equals(map)) {
+      this._backgroundMap = map; // it's an immutable type.
+      this._json.backgroundMap = map.toJSON();
+    }
+  }
 
   /** Customize the way geometry belonging to a [[SubCategory]] is drawn by this display style.
    * @param id The ID of the SubCategory whose appearance is to be overridden.