Skip to content

Commit

Permalink
Fixing labels not appearing when enabling terrain at high zoom (#3545)
Browse files Browse the repository at this point in the history
* Fixing bug by reloading terrain source on render following terrain change, fixing typos

* Making fix more robust

* Removing reference to Mapbox in test

* Adding tests

* Updating changelog

* Reloading sourceCache immediately on terrain add

* Removing timed waits from render test
  • Loading branch information
SnailBones authored Jan 9, 2024
1 parent c6f3a11 commit 2db2f4e
Show file tree
Hide file tree
Showing 13 changed files with 244 additions and 30 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## main

### ✨ Features and improvements

- ⚠️ Add the ability to import a script in the worker thread and call `addProtocol` and `removeProtocol` there ([#3459](https://github.com/maplibre/maplibre-gl-js/pull/3459)) - this also changed how `addSourceType` works since now you'll need to load the script with `maplibregl.importScriptInWorkers`.
- Upgraded to use Node JS 20 and removed the dependency of `gl` package from the tests to allow easier develpment setup.
- Improved precision and added a subtle fade transition to marker opacity changes ([#3431](https://github.com/maplibre/maplibre-gl-js/pull/3431))
Expand All @@ -10,6 +11,7 @@

- Fix the shifted mouse events after a css transform scale on the map container ([#3437](https://github.com/maplibre/maplibre-gl-js/pull/3437))
- Fix markers remaining transparent when disabling terrain ([#3431](https://github.com/maplibre/maplibre-gl-js/pull/3431))
- Fix labels disappearing when enabling terrain at high zoom ([#3545](https://github.com/maplibre/maplibre-gl-js/pull/3545))
- _...Add new stuff here..._

## 4.0.0-pre.2
Expand Down
2 changes: 1 addition & 1 deletion src/data/bucket/symbol_bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ register('CollisionBuffers', CollisionBuffers);
* `this.textCollisionBox`: Debug SymbolBuffers for text collision boxes
* The results are sent to the foreground for rendering
*
* 4. performSymbolPlacement(bucket, collisionIndex) is run on the foreground,
* 4. placement.ts is run on the foreground,
* and uses the CollisionIndex along with current camera settings to determine
* which symbols can actually show on the map. Collided symbols are hidden
* using a dynamic "OpacityVertexArray".
Expand Down
8 changes: 4 additions & 4 deletions src/geo/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export class Transform {
pixelMatrixInverse: mat4;
glCoordMatrix: mat4;
labelPlaneMatrix: mat4;
minElevationForCurrentTile: number;
_fov: number;
_pitch: number;
_zoom: number;
Expand All @@ -56,7 +57,6 @@ export class Transform {
_constraining: boolean;
_posMatrixCache: {[_: string]: mat4};
_alignedPosMatrixCache: {[_: string]: mat4};
_minEleveationForCurrentTile: number;

constructor(minZoom?: number, maxZoom?: number, minPitch?: number, maxPitch?: number, renderWorldCopies?: boolean) {
this.tileSize = 512; // constant
Expand All @@ -83,7 +83,7 @@ export class Transform {
this._edgeInsets = new EdgeInsets();
this._posMatrixCache = {};
this._alignedPosMatrixCache = {};
this._minEleveationForCurrentTile = 0;
this.minElevationForCurrentTile = 0;
}

clone(): Transform {
Expand All @@ -99,7 +99,7 @@ export class Transform {
this.height = that.height;
this._center = that._center;
this._elevation = that._elevation;
this._minEleveationForCurrentTile = that._minEleveationForCurrentTile;
this.minElevationForCurrentTile = that.minElevationForCurrentTile;
this.zoom = that.zoom;
this.angle = that.angle;
this._fov = that._fov;
Expand Down Expand Up @@ -816,7 +816,7 @@ export class Transform {
// Calculate the camera to sea-level distance in pixel in respect of terrain
const cameraToSeaLevelDistance = this.cameraToCenterDistance + this._elevation * this._pixelPerMeter / Math.cos(this._pitch);
// In case of negative minimum elevation (e.g. the dead see, under the sea maps) use a lower plane for calculation
const minElevation = Math.min(this.elevation, this._minEleveationForCurrentTile);
const minElevation = Math.min(this.elevation, this.minElevationForCurrentTile);
const cameraToLowestPointDistance = cameraToSeaLevelDistance - minElevation * this._pixelPerMeter / Math.cos(this._pitch);
const lowestPlane = minElevation < 0 ? cameraToLowestPointDistance : cameraToSeaLevelDistance;

Expand Down
2 changes: 1 addition & 1 deletion src/render/terrain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export type TerrainMesh = {
*/
export class Terrain {
/**
* The style this terrain crresponds to
* The style this terrain corresponds to
*/
painter: Painter;
/**
Expand Down
2 changes: 1 addition & 1 deletion src/source/terrain_source_cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export class TerrainSourceCache extends Evented {
}

/**
* get a list of tiles, loaded after a spezific time. This is used to update depth & coords framebuffers.
* get a list of tiles, loaded after a specific time. This is used to update depth & coords framebuffers.
* @param time - the time
* @returns the relevant tiles
*/
Expand Down
2 changes: 1 addition & 1 deletion src/ui/camera.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,7 @@ export abstract class Camera extends Evented {
}

_updateElevation(k: number) {
this.transform._minEleveationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this._elevationCenter, this.transform.tileZoom);
this.transform.minElevationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this._elevationCenter, this.transform.tileZoom);
const elevation = this.terrain.getElevationForLngLatZoom(this._elevationCenter, this.transform.tileZoom);
// target terrain updated during flight, slowly move camera to new height
if (k < 1 && elevation !== this._elevationTarget) {
Expand Down
11 changes: 6 additions & 5 deletions src/ui/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1922,12 +1922,13 @@ export class Map extends Camera {
this.terrain = null;
if (this.painter.renderToTexture) this.painter.renderToTexture.destruct();
this.painter.renderToTexture = null;
this.transform._minEleveationForCurrentTile = 0;
this.transform.minElevationForCurrentTile = 0;
this.transform.elevation = 0;
} else {
// add terrain
const sourceCache = this.style.sourceCaches[options.source];
if (!sourceCache) throw new Error(`cannot load terrain, because there exists no source with ID: ${options.source}`);
sourceCache.reload();
// Warn once if user is using the same source for hillshade and terrain
for (const index in this.style._layers) {
const thisLayer = this.style._layers[index];
Expand All @@ -1937,14 +1938,14 @@ export class Map extends Camera {
}
this.terrain = new Terrain(this.painter, sourceCache, options);
this.painter.renderToTexture = new RenderToTexture(this.painter, this.terrain);
this.transform._minEleveationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
this.transform.minElevationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
this.transform.elevation = this.terrain.getElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
this._terrainDataCallback = e => {
if (e.dataType === 'style') {
this.terrain.sourceCache.freeRtt();
} else if (e.dataType === 'source' && e.tile) {
if (e.sourceId === options.source && !this._elevationFreeze) {
this.transform._minEleveationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
this.transform.minElevationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
this.transform.elevation = this.terrain.getElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
}
this.terrain.sourceCache.freeRtt(e.tile.tileID);
Expand Down Expand Up @@ -3070,12 +3071,12 @@ export class Map extends Camera {
// update terrain stuff
if (this.terrain) {
this.terrain.sourceCache.update(this.transform, this.terrain);
this.transform._minEleveationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
this.transform.minElevationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
if (!this._elevationFreeze) {
this.transform.elevation = this.terrain.getElevationForLngLatZoom(this.transform.center, this.transform.tileZoom);
}
} else {
this.transform._minEleveationForCurrentTile = 0;
this.transform.minElevationForCurrentTile = 0;
this.transform.elevation = 0;
}

Expand Down
Binary file modified test/integration/render/tests/symbol-geometry/point/expected.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
45 changes: 28 additions & 17 deletions test/integration/render/tests/symbol-geometry/point/style.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,29 @@
"height": 256
}
},
"center": [0, 0],
"center": [
0,
0
],
"zoom": 0,
"sources": {
"geometry": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [{
"type": "Feature",
"properties": {},
"geometry": {
"type": "Point",
"coordinates": [0, 0]
"features": [
{
"type": "Feature",
"properties": {},
"geometry": {
"type": "Point",
"coordinates": [
0,
0
]
}
}
}]
]
}
}
},
Expand All @@ -38,15 +46,18 @@
"type": "symbol",
"source": "geometry",
"layout": {
"icon-image": "rocket-12",
"text-field": "Mapbox",
"text-font": [
"Open Sans Semibold",
"Arial Unicode MS Bold"
],
"text-allow-overlap": true,
"text-ignore-placement": true,
"text-offset": [0, 1]
"icon-image": "rocket-12",
"text-field": "MapLibre",
"text-font": [
"Open Sans Semibold",
"Arial Unicode MS Bold"
],
"text-allow-overlap": true,
"text-ignore-placement": true,
"text-offset": [
0,
1
]
}
}
]
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
115 changes: 115 additions & 0 deletions test/integration/render/tests/terrain/symbol-add-terrain/style.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
{
"version": 8,
"sprite": "local://sprites/sprite",
"glyphs": "local://glyphs/{fontstack}/{range}.pbf",
"timeout": 60000,
"metadata": {
"test": {
"height": 512,
"width": 512,
"operations": [
[
"wait"
],
[
"setTerrain"
],
[
"wait"
],
[
"setTerrain",
{
"source": "terrain",
"exaggeration": 1
}
],
[
"wait"
]
]
}
},
"center": [
35.38,
31.55
],
"zoom": 16.25,
"pitch": 53,
"sources": {
"hillshadeSource": {
"type": "raster-dem",
"tiles": [
"local://tiles/terrain/{z}-{x}-{y}.terrain.png"
],
"minzoom": 0,
"maxzoom": 12
},
"terrain": {
"type": "raster-dem",
"tiles": [
"local://tiles/terrain/{z}-{x}-{y}.terrain.png"
],
"minzoom": 7,
"maxzoom": 12,
"tileSize": 256
},
"geometry": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [
{
"type": "Feature",
"properties": {},
"geometry": {
"type": "Point",
"coordinates": [
35.38,
31.55
]
}
}
]
}
}
},
"layers": [
{
"id": "hills",
"type": "hillshade",
"source": "hillshadeSource",
"layout": {
"visibility": "visible"
},
"paint": {
"hillshade-shadow-color": "#473B24",
"hillshade-illumination-anchor": "map",
"hillshade-illumination-direction": 150
}
},
{
"id": "geometry",
"type": "symbol",
"source": "geometry",
"layout": {
"icon-image": "rocket-12",
"text-field": "MapLibre",
"text-font": [
"Open Sans Semibold",
"Arial Unicode MS Bold"
],
"text-allow-overlap": true,
"text-ignore-placement": true,
"text-offset": [
0,
1
]
}
}
],
"terrain": {
"source": "terrain",
"exaggeration": 1
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 2db2f4e

Please sign in to comment.