Fill extrusion querying with terrain #10293
Conversation
| } | ||
|
|
||
| function elevationFromUint16(n: number): number { | ||
| return n / 7.3; |
There was a problem hiding this comment.
Needs a comment and/or constant for where 7.3 comes from.
| const heightOffset = getTerrainHeightOffset(x, y, zBase, zTop, demSampler, centroid, exaggeration, lat); | ||
|
|
||
| const base = toPoint(vec3.transformMat4([], [x, y, heightOffset.base], m)); | ||
| const top = toPoint(vec3.transformMat4([], [x, y, heightOffset.top], m)); |
There was a problem hiding this comment.
We can get rid of two array allocations per point here by doing in-place matrix transforms (vec3.transformMat4(base, base, m)).
| const slope = [ | ||
| Math.min(0.25, meterToDEM * 0.5 * diffSum[0] / offset[0]), | ||
| Math.min(0.25, meterToDEM * 0.5 * diffSum[1] / offset[1]) | ||
| ]; |
There was a problem hiding this comment.
Nit: a lot of temporary array allocations here that get immediately discarded. Should we inline some of these and keep more separate x/y variables?
There was a problem hiding this comment.
yea thats a good call it will make it more readable too
|
|
||
| function flatElevation(demSampler: DEMSampler, centroid: vec2, lat: number): number { | ||
| const pos = [Math.floor(centroid[0] / 8), Math.floor(centroid[1] / 8)]; | ||
| const span = [10 * (centroid[0] - pos[0] * 8), 10 * (centroid[1] - pos[1] * 8)]; |
There was a problem hiding this comment.
Where is this 10 coming from?
And why does pos need to be rounded to the nearest 8?
build/generate-struct-arrays.js
Outdated
| @@ -185,7 +185,9 @@ createStructArrayType('feature_index', createLayout([ | |||
| // the source layer the feature appears in | |||
| { type: 'Uint16', name: 'sourceLayerIndex' }, | |||
| // the bucket the feature appears in | |||
| { type: 'Uint16', name: 'bucketIndex' } | |||
| { type: 'Uint16', name: 'bucketIndex' }, | |||
| // Offsetinto bucket.layoutVertexArray the vertcies for this feature appear in | |||
| function getTerrainHeightOffset(x: number, y: number, zBase: number, zTop: number, demSampler: DEMSampler, centroid: vec2, exaggeration: number, lat: number): { base: number, top: number} { | ||
| const ele = exaggeration * demSampler.getElevationAt(x, y, true, true); | ||
| const flatRoof = centroid[0] !== 0; | ||
| const centroidElevation = flatRoof ? centroid[1] === 0 ? exaggeration * elevationFromUint16(centroid[0]) : exaggeration * flatElevation(demSampler, centroid, lat) : ele; |
There was a problem hiding this comment.
no need to sample elevation when on border (centroid[1] === 0).
Edit: ignore this please, flat roof is not sampled in this case.
| const pixels = new Uint8Array(this.data.buffer); | ||
| if (clampToEdge) { |
There was a problem hiding this comment.
When does it happen? Buildings that cross tile border have height encoded in centroid and there's no need to sample.
There was a problem hiding this comment.
For slope calculation, samples could be outside.
Lower clamp bound might need to be -1 (padded [0..511] range mapped to [-1..512]).
There was a problem hiding this comment.
Yes it happens for the slop calculation, as well as when elevation is sampled for a given vertex in getTerrainHeightOffset and when the vertex is outside of tile boundaries.
I updated the clamp ranges to account for padding.
Fixes #10252
Fixes #10286
For now I'm abandoning the approach in #10266 and instead porting the existing centroid encoding technique used in the fill extrusion vertex shader to CPU, with some minor differences. The main one being I perform all DEM lookup calculations in image-pixel space instead of uv space.
In order to retrive the centroid for a feature, I lookup the centroid directly from the attribute buffer
centroidVertexArray. In order to do this I added an additional bit of metadata to featureIndex, which now also tracks the offset into the layoutVertexArray for the given feature.Demo:
Screen.Recording.2021-01-14.at.10.54.30.AM.mov
Known Bugs:
-5for height of the base, when its0and terrain is enabled lets picking work below terrain.Launch Checklist
mapbox-gl-jschangelog:<changelog>Fix querying of fill-extrusions when terrain is enabled.</changelog>