search

mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.26k stars 2.23k forks source link

Inconsistent display of fill-extrusion features on flat, custom terrain #10950

Open danvk opened 3 years ago

danvk commented 3 years ago

mapbox-gl-js version: 2.4.0

browser: Chrome

Steps to Trigger Behavior

  1. Set up custom terrain tiles with some flat, elevated area. Create a fill-extrusion layer with a stack of features to produce a building.
  2. Zoom in and out
  3. Observe that the floor heights glitch in and out.

If you look at my custom topo tiles, I've generated them for zoom levels 12, 13 and 14 and the area under this building should be perfectly flat (though elevated) in all of them:

Link to Demonstration

Here's a codepen: https://codepen.io/danvk/pen/vYmomJz

Expected Behavior

The building stays the same shape as you zoom in and out.

Actual Behavior

As I zoom in and out, the floors change height:

inconsistent-height

I can also sometimes trigger the same glitch by pitching the map, rather than zooming in and out:

glitch-pitch

Possibly related to #10536 and #10355.

I understand why this might happen if the building is placed on a hillside, but in this case the terrain is flat. So I'm not sure why Mapbox would be making different decisions about elevations or roofs depending on the zoom level.

danvk commented 3 years ago

Thinking more about this, another way to solve this problem would be have a way for a fill-extrusion to specify its height as meters above sea level, rather than as meters above terrain. This would remain stable as Mapbox makes different decisions about which terrain tiles to use.

arindam1993 commented 3 years ago

Thinking more about this, another way to solve this problem would be have a way for a fill-extrusion to specify its height as meters above sea level, rather than as meters above terrain. This would remain stable as Mapbox makes different decisions about which terrain tiles to use.

You're completely on-point about this @danvk

danvk commented 3 years ago

I appreciate that @arindam1993! But I do hope the feature request aspect of this issue doesn't distract from the immediate problem I'm reporting: it is very much a 🐞 bug 🐞 in the current version of Mapbox GL JS that you can't use fill-extrusion and terrain without glitchy buildings.

astojilj commented 3 years ago

@danvk Thanks for example. Can we use several DEM tiles from your custom dataset to make unit or render test?

Let me explain what's happening. We calculate flat roof elevation on GPU, by sampling points around building center. The larger the building footprint is, further away are samples. With limited number of samples and higher resolution DEM, sampling might produce inconsistent results. E.g. Mapbox DEM is smoother (lower resolution) so the issue is less visible:

custom DEM:

https://user-images.githubusercontent.com/549216/131835014-0025ad5c-a55e-4be1-af70-9a5ccef67bbc.mov

Mapbox DEM:

https://user-images.githubusercontent.com/549216/131836479-37197a63-5650-401d-b8d3-25ae105d1272.mov

We need to investigate different options to calculate roof height, especially when multiple features overlap to a single building.

As a workaround, is it feasible for you to render custom layer building for this?

danvk commented 3 years ago

Thanks for the explanation @astojilj. Is there any way for me to turn on the terrain wireframe as a Mapbox user? It would have made it much easier to debug my custom DEM!

Yes, please feel free to turn my data and tiles into a test.

I can try using a custom layer for now but that does come with some other downsides around transparency and performance.

If the inconsistencies are coming from the flat roofs behavior, would adding an option to disable this fix the issue? (#10355) Or would that make the problem manifest in another way, say by having sloped roofs at some zoom levels.

Ultimately I do think that having a way to specify elevation w/r/t/ sea level instead of the terrain is the most robust solution.

astojilj commented 3 years ago

way for me to turn on the terrain wireframe as a Mapbox user?

This should work: https://github.com/mapbox/mapbox-gl-js/blob/10961e05522d38bae952d7f01947fe6bfd6f2aa1/debug/terrain-debug.html#L270

If the inconsistencies are coming from the flat roofs behavior, would adding an option to disable this fix the issue? (#10355) Or would that make the problem manifest in another way, say by having sloped roofs at some zoom levels.

It would elevate every vertex exactly as high meters as specified in the feature above the ground around it. Roof would be sloped but, given that your terrain data is specifying this area as flat, it would look better.

Ultimately I do think that having a way to specify elevation w/r/t/ sea level instead of the terrain is the most robust solution.

Yes, I guess that final value, multiplied by exaggeration would work. Let us discuss this internally and check how we can fix existing solution, too.

Thanks a lot for the data.