Conversation
|
I would avoid the "with fetch" option as a developer can simply fetch the relevant tiles and get the relevant elevation data. |
what I see is all except the last one apply terrain exaggeration and depend on terrain being enabled (return null otherwise) -- and only the first one is part of public API /**
* Gets the elevation at a given location, in meters above sea level.
* Returns null if terrain is not enabled.
* If terrain is enabled with some exaggeration value, the value returned here will be reflective of (multiplied by) that exaggeration value.
* This method should be used for proper positioning of custom 3d objects, as explained [here](https://maplibre.org/maplibre-gl-js/docs/examples/adding-3d-models-using-threejs-on-terrain/)
* @param lngLatLike - [x,y] or LngLat coordinates of the location
* @returns elevation in meters
*/
queryTerrainElevation(lngLatLike: LngLatLike): number | null {
if (!this.terrain) {
return null;
}
return this.terrain.getElevationForLngLat(LngLat.convert(lngLatLike), this.transform);
}whereas new approach works even in 2D, and returns elevation in meters independently of exaggeration. I could try to share a tiny bit more code (bilinear sampling?) but that would mean some refactoring. did I miss anything else ?
but then the developer has to redo the tile fetch & decode, not terrible but I think it's fairer to then say 'the functionality is limited to in-view tiles, otherwise write your own' as the fallback becomes more code than the implementation. but I'm fine with that, at least for my use, it's a rather niche case. |
|
I have the following code in my app to get elevation, it's not a lot of code so I don't mind having it on my app. I'll think about it a bit more... |
Add a DEM elevation query API on RasterDEMTileSource. unlike terrain.getElevation, works even in 2D, and returns elevation in meters independently of exaggeration. - Introduce ElevationQueryResult and queryElevations(lnglats, minzoom?, maxzoom?) on raster DEM sources. - Implement bilinear DEM sampling and highest-resolution-first tile search via calculateTileKey. - Add TileManager.findTileByKey(key) to unify lookup across in-view tiles and out-of-view cache. - Export ElevationQueryResult from public index. - Add unit tests - Add test/examples/get-elevation-values.html using mapterhorn terrarium raster-dem source and live hover sampling.
…dTileByKey to getAnyTileByID
|
clear, I overestimated how hard it is to have a clean room implem. I've removed the also simplified/refactored a bit. I guess even returning zoom is a bit overkill. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7201 +/- ##
==========================================
+ Coverage 92.68% 92.70% +0.02%
==========================================
Files 289 289
Lines 24085 24100 +15
Branches 5102 5112 +10
==========================================
+ Hits 22322 22341 +19
+ Misses 1763 1759 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @param fx - Fractional x position within the tile, in [0, 1) | ||
| * @param fy - Fractional y position within the tile, in [0, 1) | ||
| */ | ||
| function sampleDEMBilinear(dem: DEMData, fx: number, fy: number): number { |
There was a problem hiding this comment.
Isn't this code already in the demdata file? Can't we reuse it?
There was a problem hiding this comment.
I couldn't find bilinear interpolation (or any duplication) in dem_data.ts
I only found bilinear interpolation in @src/render/terrain.ts#163-185, but it works in terrain tile-coordinate space after matrix transform/normalization -- still possible to extract a common core but it needs to be put somewhere else. do you want me to do it ? and put it in dem_data ?
There was a problem hiding this comment.
Either there or a separate file just for that I guess.
Add a DEM elevation query API on RasterDEMTileSource.
I propose 2 new APIs (review commits separately), without or with fetching required tiles.
The goal is for rich map applications to be able to display elevation under pointer, or add elevation to displayed shapes, without duplicating DEM logic & download or hacking maplibre internals.
In the first API the implem is simpler but we have to rely on eg hillshade layer to download DEM tiles, so it is a bit more fragile for advanced cases (elevate shape partly off-screen).
The second is easier/more consistent to use but has more impact eg we need to decide how the tile caches interact.
I wasn't sure which option fit better the project, I hope at least one of them makes sense :)
Without fetch
Add
queryElevationsAPI on RasterDEMTileSource.With fetch
Add
queryElevationsWithFetchthat fetches missing DEM tiles and reuses queryElevations sampling.Launch Checklist
CHANGELOG.mdunder the## mainsection.