correct coveringTiles when terrain is enabled (#1300)

prozessor13 · flother · HarelM · web-flow · commit 6fa592cec0f6 · 2022-06-24T20:22:48.000+03:00
* Fixes #1241 - correct coveringTiles when terrain is enabled * fix lint * Add unit test for calculating min/max elevation (#1316) * Add unit test for calculating min/max elevation * Use correct method name * Move min/max elevation value calc to Terrain class * Alter Terrain.getMinMaxElevation to take tileID arg * Fix unit tests after merge from main * Fix lint * Add render test for 3D terrain (#1320) * Add terrain style spec integration test Tests that the root level terrain object must be in the following form: "terrain": { "source": string, "exaggeration": number, "elevationOffset": number } * Add work-in-progress render test for 3D terrain * Fix terrain render test so it passes The original test was failing because there were no terrain tiles available at the zoom being requested. Now it's testing at zoom 13, which means requesting terrain tiles at zoom 12, which are available to the tests. * Update terrain render test to test fix for #1241 The fix for #1241, commit 352bc03, ensures that elevationOffset is taken into account when gauging which tiles are required to render 3D terrain. If we add an extreme offset then we have a test that fails on the current main branch but passes after the fix. * Add changelog comment Co-authored-by: Matt Riggott <flother@users.noreply.github.com> Co-authored-by: HarelM <harel.mazor@gmail.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 - Handle maxBounds which cross the meridian at longitude ±180° (#1298, #1299)
 - Hide arrow displayed in default `summary` styles on the attribution control (#1258)
 - Fix memory usage in terrain 3D (#1291, #1302)
+- Fix disappearence of closest tiles when 3D terrain is enabled (#1241, #1300)
 
 ## 2.2.0-pre.2
 
diff --git a/src/geo/transform.ts b/src/geo/transform.ts
@@ -438,12 +438,9 @@ class Transform {
                 let quadrant = it.aabb.quadrant(i);
                 if (options.terrain) {
                     const tileID = new OverscaledTileID(childZ, it.wrap, childZ, childX, childY);
-                    const tile = options.terrain.getTerrainData(tileID).tile;
-                    let minElevation = this.elevation, maxElevation = this.elevation;
-                    if (tile && tile.dem) {
-                        minElevation = tile.dem.min * options.terrain.exaggeration;
-                        maxElevation = tile.dem.max * options.terrain.exaggeration;
-                    }
+                    const minMax = options.terrain.getMinMaxElevation(tileID);
+                    const minElevation = minMax.minElevation ?? this.elevation;
+                    const maxElevation = minMax.maxElevation ?? this.elevation;
                     quadrant = new Aabb(
                         [quadrant.min[0], quadrant.min[1], minElevation] as vec3,
                         [quadrant.max[0], quadrant.max[1], maxElevation] as vec3
diff --git a/src/render/terrain.test.ts b/src/render/terrain.test.ts
@@ -6,7 +6,9 @@ import {RGBAImage} from '../util/image';
 import Texture from './texture';
 import type Style from '../style/style';
 import type SourceCache from '../source/source_cache';
+import {OverscaledTileID} from '../source/tile_id';
 import type {TerrainSpecification} from '../style-spec/types.g';
+import type DEMData from '../data/dem_data';
 import TileCache from '../source/tile_cache';
 import Tile from '../source/tile';
 
@@ -26,7 +28,7 @@ describe('Terrain', () => {
         } as SourceCache;
         const getTileByID = (tileID) : Tile => {
             if (tileID !== 'abcd') {
-                return null;
+                return null as any as Tile;
             }
             return {
                 tileID: {
@@ -36,7 +38,7 @@ describe('Terrain', () => {
                         z: 0
                     }
                 }
-            };
+            } as any as Tile;
         };
         const terrain = new Terrain(style, sourceCache, {} as any as TerrainSpecification);
         terrain.sourceCache.getTileByID = getTileByID;
@@ -52,4 +54,102 @@ describe('Terrain', () => {
 
         expect(coordinate).not.toBeNull();
     });
+
+    test('Calculate tile minimum and maximum elevation', () => {
+        const tileID = new OverscaledTileID(5, 0, 5, 17, 11);
+        const tile = new Tile(tileID, 256);
+        tile.dem = {
+            min: 0,
+            max: 100,
+            getPixels: () => new RGBAImage({width: 1, height: 1}, new Uint8Array(1 * 4)),
+            getUnpackVector: () => [6553.6, 25.6, 0.1, 10000.0],
+        } as any as DEMData;
+        const style = {
+            map: {
+                painter: {
+                    context: new Context(gl(1, 1)),
+                    width: 1,
+                    height: 1,
+                    getTileTexture: () => null
+                }
+            }
+        } as any as Style;
+        const sourceCache = {
+            _source: {maxzoom: 12},
+            _cache: {max: 10},
+            getTileByID: () => {
+                return tile;
+            },
+        } as any as SourceCache;
+        const terrain = new Terrain(
+            style,
+            sourceCache,
+            {exaggeration: 2, elevationOffset: 50} as any as TerrainSpecification,
+        );
+
+        terrain.sourceCache._tiles[tileID.key] = tile;
+        const {minElevation, maxElevation} = terrain.getMinMaxElevation(tileID);
+
+        expect(minElevation).toBe(100);
+        expect(maxElevation).toBe(300);
+    });
+
+    test('Return null elevation values when no tile', () => {
+        const tileID = new OverscaledTileID(5, 0, 5, 17, 11);
+        const style = {
+            map: {
+                painter: {
+                    context: new Context(gl(1, 1)),
+                    width: 1,
+                    height: 1,
+                }
+            }
+        } as any as Style;
+        const sourceCache = {
+            _source: {maxzoom: 12},
+            _cache: {max: 10},
+            getTileByID: () => null,
+        } as any as SourceCache;
+        const terrain = new Terrain(
+            style,
+            sourceCache,
+            {exaggeration: 2, elevationOffset: 50} as any as TerrainSpecification,
+        );
+
+        const minMaxNoTile = terrain.getMinMaxElevation(tileID);
+
+        expect(minMaxNoTile.minElevation).toBeNull();
+        expect(minMaxNoTile.maxElevation).toBeNull();
+    });
+
+    test('Return null elevation values when no DEM', () => {
+        const tileID = new OverscaledTileID(5, 0, 5, 17, 11);
+        const tile = new Tile(tileID, 256);
+        tile.dem = null as any as DEMData;
+        const style = {
+            map: {
+                painter: {
+                    context: new Context(gl(1, 1)),
+                    width: 1,
+                    height: 1,
+                }
+            }
+        } as any as Style;
+        const sourceCache = {
+            _source: {maxzoom: 12},
+            _cache: {max: 10},
+            getTileByID: () => {
+                return tile;
+            },
+        } as any as SourceCache;
+        const terrain = new Terrain(
+            style,
+            sourceCache,
+            {exaggeration: 2, elevationOffset: 50} as any as TerrainSpecification,
+        );
+        const minMaxNoDEM = terrain.getMinMaxElevation(tileID);
+
+        expect(minMaxNoDEM.minElevation).toBeNull();
+        expect(minMaxNoDEM.maxElevation).toBeNull();
+    });
 });
diff --git a/src/render/terrain.ts b/src/render/terrain.ts
@@ -366,4 +366,22 @@ export default class Terrain {
         return this._mesh;
     }
 
+    /**
+     * Get the minimum and maximum elevation contained in a tile. This includes any elevation offset
+     * and exaggeration included in the terrain.
+     *
+     * @param tileID Id of the tile to be used as a source for the min/max elevation
+     * @returns {Object} Minimum and maximum elevation found in the tile, including the terrain's
+     * elevation offset and exaggeration
+     */
+    getMinMaxElevation(tileID: OverscaledTileID): {minElevation: number | null; maxElevation: number | null} {
+        const tile = this.getTerrainData(tileID).tile;
+        const minMax = {minElevation: null, maxElevation: null};
+        if (tile && tile.dem) {
+            minMax.minElevation = (tile.dem.min + this.elevationOffset) * this.exaggeration;
+            minMax.maxElevation = (tile.dem.max + this.elevationOffset) * this.exaggeration;
+        }
+        return minMax;
+    }
+
 }
diff --git a/src/source/terrain_source_cache.test.ts b/src/source/terrain_source_cache.test.ts
@@ -9,6 +9,9 @@ import Painter from '../render/painter';
 import Context from '../gl/context';
 import gl from 'gl';
 import RasterDEMTileSource from './raster_dem_tile_source';
+import {OverscaledTileID} from './tile_id';
+import Tile from './tile';
+import DEMData from '../data/dem_data';
 
 const context = new Context(gl(10, 10));
 const transform = new Transform();
@@ -54,8 +57,8 @@ describe('TerrainSourceCache', () => {
         global.fetch = null;
         server = fakeServer.create();
         server.respondWith('/source.json', JSON.stringify({
-            minzoom: 0,
-            maxzoom: 22,
+            minzoom: 5,
+            maxzoom: 12,
             attribution: 'MapLibre',
             tiles: ['http://example.com/{z}/{x}/{y}.pngraw'],
             bounds: [-47, -7, -45, -5]
@@ -86,4 +89,16 @@ describe('TerrainSourceCache', () => {
         expect(tsc.sourceCache.tileSize).toBe(tsc.tileSize * 2 ** tsc.deltaZoom);
     });
 
+    test('#getSourceTile', () => {
+        const tileID = new OverscaledTileID(5, 0, 5, 17, 11);
+        const tile = new Tile(tileID, 256);
+        tile.dem = {} as DEMData;
+        tsc.sourceCache._tiles[tileID.key] = tile;
+        expect(tsc.deltaZoom).toBe(1);
+        expect(tsc.getSourceTile(tileID)).toBeFalsy();
+        expect(tsc.getSourceTile(tileID.children(12)[0])).toBeTruthy();
+        expect(tsc.getSourceTile(tileID.children(12)[0].children(12)[0])).toBeFalsy();
+        expect(tsc.getSourceTile(tileID.children(12)[0].children(12)[0], true)).toBeTruthy();
+    });
+
 });
diff --git a/src/source/terrain_source_cache.ts b/src/source/terrain_source_cache.ts
@@ -189,7 +189,7 @@ export default class TerrainSourceCache extends Evented {
         let tile = this.sourceCache.getTileByID(this._sourceTileCache[tileID.key]);
         // during tile-loading phase look if parent tiles (with loaded dem) are available.
         if (!(tile && tile.dem) && searchForDEM)
-            while (z > source.minzoom && !(tile && tile.dem))
+            while (z >= source.minzoom && !(tile && tile.dem))
                 tile = this.sourceCache.getTileByID(tileID.scaledTo(z--).key);
         return tile;
     }
diff --git a/test/integration/render/tests/terrain/default/expected.png b/test/integration/render/tests/terrain/default/expected.png
diff --git a/test/integration/render/tests/terrain/default/style.json b/test/integration/render/tests/terrain/default/style.json
@@ -0,0 +1,48 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "height": 256,
+      "width": 256
+    }
+  },
+  "center": [-113.33496, 35.96022],
+  "zoom": 13,
+  "pitch": 60,
+  "sources": {
+    "terrain": {
+      "type": "raster-dem",
+      "tiles": ["local://tiles/{z}-{x}-{y}.terrain.png"],
+      "maxzoom": 15,
+      "tileSize": 256
+    },
+    "satellite": {
+      "type": "raster",
+      "tiles": ["local://tiles/{z}-{x}-{y}.satellite.png"],
+      "maxzoom": 17,
+      "tileSize": 256
+    }
+  },
+  "layers": [
+    {
+      "id": "background",
+      "type": "background",
+      "paint": {
+        "background-color": "white"
+      }
+    },
+    {
+      "id": "raster",
+      "type": "raster",
+      "source": "satellite",
+      "paint": {
+        "raster-opacity": 1.0
+      }
+    }
+  ],
+  "terrain": {
+    "source": "terrain",
+    "exaggeration": 2,
+    "elevationOffset": 2000
+  }
+}
diff --git a/test/integration/style-spec/tests/terrain.input.json b/test/integration/style-spec/tests/terrain.input.json
@@ -0,0 +1,17 @@
+{
+  "version": 8,
+  "sources": {
+    "terrain": {
+      "type": "raster-dem",
+      "tiles": ["local://tiles/{z}-{x}-{y}.terrain.png"],
+      "maxzoom": 15,
+      "tileSize": 256
+    }
+  },
+  "layers": [],
+  "terrain": {
+    "source": 123,
+    "exaggeration": "2",
+    "elevationOffset": "50"
+  }
+}
diff --git a/test/integration/style-spec/tests/terrain.output.json b/test/integration/style-spec/tests/terrain.output.json
@@ -0,0 +1,14 @@
+[
+  {
+    "message": "source: string expected, number found",
+    "line": 13
+  },
+  {
+    "message": "exaggeration: number expected, string found",
+    "line": 14
+  },
+  {
+    "message": "elevationOffset: number expected, string found",
+    "line": 15
+  }
+]

Original file line number	Diff line number	Diff line change
`@@ -189,7 +189,7 @@ export default class TerrainSourceCache extends Evented {`
`189`	`189`	`let tile = this.sourceCache.getTileByID(this._sourceTileCache[tileID.key]);`
`190`	`190`	`// during tile-loading phase look if parent tiles (with loaded dem) are available.`
`191`	`191`	`if (!(tile && tile.dem) && searchForDEM)`
`192`		`- while (z > source.minzoom && !(tile && tile.dem))`
	`192`	`+ while (z >= source.minzoom && !(tile && tile.dem))`
`193`	`193`	`tile = this.sourceCache.getTileByID(tileID.scaledTo(z--).key);`
`194`	`194`	`return tile;`
`195`	`195`	`}`