v5 coveringTiles terrain

[dennemark]

Hi,
I am currently migrating maplibre from v4 to v5. One issue that stalled this transition was the coveringTiles function. I tried to revisit this now. It seems like the coveringTiles algorithm does not respect the terrain anymore, therefore in some occasions some tiles will be invisible even though they would be within the viewport.

I guess this functionality was deprecated in favor of globe feature?
Kind regards

[HarelM]

I'm not sure this was intentional.
If there's a bug in the new algorithm please open an issue with minimal steps to reproduce.
Converting tiles method can be overwritten in latests versions so you might be able to solve this without a new release or submit a PR to fix the issue.
There was a lot of effort invested in this area in recent versions...
Cc @NathanMOlson, @kubapelc, @wayofthefuture

[wayofthefuture]

@dennemark Please start with 5.7.1 and see if you can isolate the version where it starts. A jsbin or codepen would also be great to reproduce.

https://github.com/maplibre/maplibre-gl-js/tags

[dennemark]

I just noticed that the coveringTiles for globe seems to consider terrain. This is also part of testing. CoveringTiles without globe does not seem to consider it.
After turning on globe I also have the feeling that navigation is smoother. I was confused in the beginning that rotation works different than in v4.
I will stick to globe projection, since it seems to be much more compatible with terrain. I hope it will have feature compatibility with v4.

If it is necessary to implement terrain for the classic projection, it is probably useful to look at the globe_covering_tiles_details_provider.ts. covering_tiles_details_provider.ts seems to be missing the terrain implementation.

maplibre-gl-js/src/geo/projection/globe_covering_tiles_details_provider.ts

Lines 114 to 118 in c53e5c3

	if (options?.terrain) {
	const overscaledTileID = new OverscaledTileID(tileID.z, wrap, tileID.z, tileID.x, tileID.y);
	const minMax = options.terrain.getMinMaxElevation(overscaledTileID);
	minElevation = minMax.minElevation ?? Math.min(0, elevation);
	maxElevation = minMax.maxElevation ?? Math.max(0, elevation);

[NathanMOlson]

I just noticed that the coveringTiles for globe seems to consider terrain. This is also part of testing. CoveringTiles without globe does not seem to consider it.

Without globe, the implementation in mercator_covering_tiles_details_provider.ts is used. This implementation also considers terrrain:

    /**
     * Returns the AABB of the specified tile.
     * @param tileID - Tile x, y and z for zoom.
     */
    getTileBoundingVolume(tileID: {x: number; y: number; z: number}, wrap: number, elevation: number, options: CoveringTilesOptionsInternal): Aabb {
        let minElevation = 0;
        let maxElevation = 0;
        if (options?.terrain) {
            const overscaledTileID = new OverscaledTileID(tileID.z, wrap, tileID.z, tileID.x, tileID.y);
            const minMax = options.terrain.getMinMaxElevation(overscaledTileID);
            minElevation = minMax.minElevation ?? Math.min(0, elevation);
            maxElevation = minMax.maxElevation ?? Math.max(0, elevation);
        }
        const numTiles = 1 << tileID.z;
        return new Aabb([wrap + tileID.x / numTiles, tileID.y / numTiles, minElevation],
            [wrap + (tileID.x + 1) / numTiles, (tileID.y + 1) / numTiles, maxElevation]);
    }
    ```

[wayofthefuture]

@dennemark I think it's still worth looking into as it should work for both... anything to help replicate or what version it started would help otherwise we are in the dark

[dennemark]

Sorry for leaving this here. My schedule is a bit tight. Hope to revisit this.
The globe approach works so far though for this issue. The globe change is anyways impressive work!