From 4fd5028a8004ef01bb2e0e9f8073139561a462c2 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Mon, 30 Nov 2020 09:35:17 -0800 Subject: [PATCH] Fix seams when raster-dem-source tileSize is set to 256 (#371) * robust buffer check * Add unit tests for prevPowerOfTwo * Fix height querying overflow when tileSize != returned image size --- src/source/raster_dem_tile_source.js | 4 ++-- src/terrain/elevation.js | 8 ++++---- src/util/util.js | 9 +++++++++ test/unit/util/util.test.js | 13 ++++++++++++- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/source/raster_dem_tile_source.js b/src/source/raster_dem_tile_source.js index 2029e3c78df..2420f623295 100644 --- a/src/source/raster_dem_tile_source.js +++ b/src/source/raster_dem_tile_source.js @@ -1,7 +1,7 @@ // @flow import {getImage, ResourceType} from '../util/ajax'; -import {extend} from '../util/util'; +import {extend, prevPowerOfTwo} from '../util/util'; import {Evented} from '../util/evented'; import browser from '../util/browser'; import window from '../util/window'; @@ -58,7 +58,7 @@ class RasterDEMTileSource extends RasterTileSource implements Source { const transfer = window.ImageBitmap && img instanceof window.ImageBitmap && offscreenCanvasSupported(); // DEMData uses 1px padding. Handle cases with image buffer of 1 and 2 pxs, the rest assume default buffer 0 // in order to keep the previous implementation working (no validation against tileSize). - const buffer = (img.width - this.tileSize) <= 4 && (img.width & 1) === 0 ? Math.max(0, img.width - this.tileSize) / 2 : 0; + const buffer = (img.width - prevPowerOfTwo(img.width)) / 2; // padding is used in getImageData. As DEMData has 1px padding, if DEM tile buffer is 2px, discard outermost pixels. const padding = 1 - buffer; const borderReady = padding < 1; diff --git a/src/terrain/elevation.js b/src/terrain/elevation.js index 3f180baf4b7..6cb6ad32c96 100644 --- a/src/terrain/elevation.js +++ b/src/terrain/elevation.js @@ -41,8 +41,8 @@ export class Elevation { if (!(demTile && demTile.dem)) { return defaultIfNotLoaded; } const dem: DEMData = demTile.dem; const tilesAtTileZoom = 1 << demTile.tileID.canonical.z; - const x = (px * tilesAtTileZoom - demTile.tileID.canonical.x) * demTile.tileSize; - const y = (point.y * tilesAtTileZoom - demTile.tileID.canonical.y) * demTile.tileSize; + const x = (px * tilesAtTileZoom - demTile.tileID.canonical.x) * dem.dim; + const y = (point.y * tilesAtTileZoom - demTile.tileID.canonical.y) * dem.dim; const i = Math.floor(x); const j = Math.floor(y); @@ -180,8 +180,8 @@ export class DEMSampler { const dem: DEMData = demTile.dem; const demTileID = demTile.tileID; const scale = 1 << tileID.canonical.z - demTileID.canonical.z; - const xOffset = (tileID.canonical.x / scale - demTileID.canonical.x) * demTile.tileSize; - const yOffset = (tileID.canonical.y / scale - demTileID.canonical.y) * demTile.tileSize; + const xOffset = (tileID.canonical.x / scale - demTileID.canonical.x) * dem.dim; + const yOffset = (tileID.canonical.y / scale - demTileID.canonical.y) * dem.dim; const k = demTile.tileSize / EXTENT / scale; return new DEMSampler(dem, k, [xOffset, yOffset]); diff --git a/src/util/util.js b/src/util/util.js index 809f2c777ec..493e05f26e6 100644 --- a/src/util/util.js +++ b/src/util/util.js @@ -329,6 +329,15 @@ export function nextPowerOfTwo(value: number): number { return Math.pow(2, Math.ceil(Math.log(value) / Math.LN2)); } +/** + * Return the previous power of two, or the input value if already a power of two + * @private + */ +export function prevPowerOfTwo(value: number): number { + if (value <= 1) return 1; + return Math.pow(2, Math.floor(Math.log(value) / Math.LN2)); +} + /** * Validate a string to match UUID(v4) of the * form: xxxxxxxx-xxxx-4xxx-[89ab]xxx-xxxxxxxxxxxx diff --git a/test/unit/util/util.test.js b/test/unit/util/util.test.js index cea83cc8060..4afda3d922e 100644 --- a/test/unit/util/util.test.js +++ b/test/unit/util/util.test.js @@ -2,7 +2,7 @@ import {test} from '../../util/test'; -import {degToRad, radToDeg, easeCubicInOut, keysDifference, extend, pick, uniqueId, bindAll, asyncAll, clamp, wrap, bezier, endsWith, mapObject, filterObject, deepEqual, clone, arraysIntersect, isCounterClockwise, isClosedPolygon, parseCacheControl, uuid, validateUuid, nextPowerOfTwo, isPowerOfTwo, bufferConvexPolygon} from '../../../src/util/util'; +import {degToRad, radToDeg, easeCubicInOut, keysDifference, extend, pick, uniqueId, bindAll, asyncAll, clamp, wrap, bezier, endsWith, mapObject, filterObject, deepEqual, clone, arraysIntersect, isCounterClockwise, isClosedPolygon, parseCacheControl, uuid, validateUuid, nextPowerOfTwo, isPowerOfTwo, bufferConvexPolygon, prevPowerOfTwo} from '../../../src/util/util'; import Point from '@mapbox/point-geometry'; const EPSILON = 1e-8; @@ -113,6 +113,17 @@ test('util', (t) => { t.end(); }); + t.test('prevPowerOfTwo', (t) => { + t.equal(prevPowerOfTwo(1), 1); + t.equal(prevPowerOfTwo(2), 2); + t.equal(prevPowerOfTwo(256), 256); + t.equal(prevPowerOfTwo(258), 256); + t.equal(prevPowerOfTwo(514), 512); + t.equal(prevPowerOfTwo(512), 512); + t.equal(prevPowerOfTwo(98), 64); + t.end(); + }); + t.test('nextPowerOfTwo', (t) => { t.equal(isPowerOfTwo(nextPowerOfTwo(1)), true); t.equal(isPowerOfTwo(nextPowerOfTwo(2)), true);