Skip to content

Commit

Permalink
Fix seams when raster-dem-source tileSize is set to 256 (#371)
Browse files Browse the repository at this point in the history
* robust buffer check

* Add unit tests for prevPowerOfTwo

* Fix height querying overflow when tileSize != returned image size
  • Loading branch information
Arindam Bose authored Nov 30, 2020
1 parent 121142a commit 4fd5028
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/source/raster_dem_tile_source.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions src/terrain/elevation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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]);
Expand Down
9 changes: 9 additions & 0 deletions src/util/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion test/unit/util/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 4fd5028

Please sign in to comment.