From 78ed0d1e26341755ca8779134581532cd87145df Mon Sep 17 00:00:00 2001
From: Xiaoji Chen <xiaoji@uber.com>
Date: Tue, 25 Feb 2020 21:09:54 -0800
Subject: [PATCH] improve TerrainLayer loading experience

---
 docs/layers/terrain-layer.md                  |  2 +-
 .../src/terrain-layer/terrain-layer.js        | 38 +++++++++++--------
 website/src/components/demos/terrain-demo.js  |  3 +-
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/docs/layers/terrain-layer.md b/docs/layers/terrain-layer.md
index 1ba67b8392a..76ef7d4931d 100644
--- a/docs/layers/terrain-layer.md
+++ b/docs/layers/terrain-layer.md
@@ -158,7 +158,7 @@ Set `workerUrl` to empty string to disable worker (improves error messages).
 
 ##### `color` (Color, optional)
 
-Color to use before `surfaceImage` is loaded or if `surfaceImage` is unavailable. Equivalent to setting SimpleMeshLayer `getColor` prop to `d => prop.color`.
+Color to use if `surfaceImage` is unavailable. Equivalent to setting SimpleMeshLayer `getColor` prop to `d => prop.color`.
 
 - Default: `[255, 255, 255]`
 
diff --git a/modules/geo-layers/src/terrain-layer/terrain-layer.js b/modules/geo-layers/src/terrain-layer/terrain-layer.js
index 03ba90ffbe4..1caa43df010 100644
--- a/modules/geo-layers/src/terrain-layer/terrain-layer.js
+++ b/modules/geo-layers/src/terrain-layer/terrain-layer.js
@@ -61,13 +61,15 @@ const defaultProps = {
  */
 export default class TerrainLayer extends CompositeLayer {
   updateState({props, oldProps}) {
-    if (props.terrainImage !== oldProps.terrainImage) {
+    const terrainImageChanged = props.terrainImage !== oldProps.terrainImage;
+    if (terrainImageChanged) {
       const isTiled = props.terrainImage.includes('{x}') && props.terrainImage.includes('{y}');
       this.setState({isTiled});
     }
 
     // Reloading for single terrain mesh
     const shouldReload =
+      terrainImageChanged ||
       props.meshMaxError !== oldProps.meshMaxError ||
       props.elevationDecoder !== oldProps.elevationDecoder ||
       props.bounds !== oldProps.bounds;
@@ -93,11 +95,17 @@ export default class TerrainLayer extends CompositeLayer {
   }
 
   getTiledTerrainData({bbox, x, y, z}) {
-    const {terrainImage, elevationDecoder, meshMaxError, workerUrl} = this.props;
+    const {terrainImage, surfaceImage, elevationDecoder, meshMaxError, workerUrl} = this.props;
     const url = terrainImage
       .replace('{x}', x)
       .replace('{y}', y)
       .replace('{z}', z);
+    const surfaceUrl = surfaceImage
+      ? surfaceImage
+          .replace('{x}', x)
+          .replace('{y}', y)
+          .replace('{z}', z)
+      : false;
 
     const viewport = new WebMercatorViewport({
       longitude: (bbox.west + bbox.east) / 2,
@@ -108,34 +116,32 @@ export default class TerrainLayer extends CompositeLayer {
     const topRight = viewport.projectFlat([bbox.east, bbox.north]);
     const bounds = [bottomLeft[0], bottomLeft[1], topRight[0], topRight[1]];
 
-    return this.loadTerrain({
+    const terrain = this.loadTerrain({
       terrainImage: url,
       bounds,
       elevationDecoder,
       meshMaxError,
       workerUrl
     });
+    const texture = surfaceUrl
+      ? // If surface image fails to load, the tile should still be displayed
+        load(surfaceUrl).catch(_ => null)
+      : Promise.resolve(null);
+
+    return Promise.all([terrain, texture]);
   }
 
   renderSubLayers(props) {
-    const {x, y, z} = props.tile;
-    const surfaceUrl = props.surfaceImage
-      ? props.surfaceImage
-          .replace('{x}', x)
-          .replace('{y}', y)
-          .replace('{z}', z)
-      : false;
-
     return new SimpleMeshLayer({
       id: props.id,
       wireframe: props.wireframe,
-      mesh: props.data,
+      mesh: props.data.then(([mesh]) => mesh),
       data: [1],
-      texture: surfaceUrl,
+      texture: props.data.then(([, texture]) => texture),
       getPolygonOffset: null,
       coordinateSystem: COORDINATE_SYSTEM.CARTESIAN,
       getPosition: d => [0, 0, 0],
-      getColor: d => props.color
+      getColor: props.color
     });
   }
 
@@ -155,7 +161,7 @@ export default class TerrainLayer extends CompositeLayer {
         getTileData: this.getTiledTerrainData.bind(this),
         renderSubLayers: this.renderSubLayers,
         updateTriggers: {
-          getTileData: {terrainImage, meshMaxError, elevationDecoder}
+          getTileData: {terrainImage, surfaceImage, meshMaxError, elevationDecoder}
         }
       });
     }
@@ -168,7 +174,7 @@ export default class TerrainLayer extends CompositeLayer {
         mesh: this.state.terrain,
         texture: surfaceImage,
         getPosition: d => [0, 0, 0],
-        getColor: d => color,
+        getColor: color,
         wireframe
       }
     );
diff --git a/website/src/components/demos/terrain-demo.js b/website/src/components/demos/terrain-demo.js
index 7e22549df35..be24c152f45 100644
--- a/website/src/components/demos/terrain-demo.js
+++ b/website/src/components/demos/terrain-demo.js
@@ -46,7 +46,8 @@ const LOCATIONS = {
 const SURFACE_IMAGES = {
   Sectional: 'https://wms.chartbundle.com/tms/1.0.0/sec/{z}/{x}/{y}.png?origin=nw',
   Satellite: `https://api.mapbox.com/v4/mapbox.satellite/{z}/{x}/{y}@2x.png?access_token=${MAPBOX_TOKEN}`,
-  Street: 'https://c.tile.openstreetmap.org/{z}/{x}/{y}.png'
+  Street: 'https://c.tile.openstreetmap.org/{z}/{x}/{y}.png',
+  None: null
 };
 
 export default class TerrainDemo extends Component {