Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutipolygon buildings do not work well with 3d terrain, show floating or underground. #3313

Closed
acalcutt opened this issue Nov 4, 2023 · 9 comments
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed Terrain 3D

Comments

@acalcutt
Copy link
Contributor

acalcutt commented Nov 4, 2023

When using a planetiler 0.7.0 generated pmtiles file in maplibre-js with 3d terrain enabled, I observed floating buildings and buildings going under the ground. With a file generated with planetiler 0.6.0, this issue did not occur. After discussion on slack in https://osmus.slack.com/archives/C03TFH5NE83/p1698949699781319 , I think this relates to a change in planetiler 0.7.0 which merges buildings with the same tags into a single multipolygon to save space. In 0.6.0 each building was it's own polygon and each was tagged individually.

0.6.0
image

0.7.0
image

While this could be considered a planetiler issue, I am also not sure that maplibre-js is handling the multipolygon properly with 3d terrain enabled.

For example, I pulled this building geojson out of a planetiler 0.7.0 pbf file with vt2geojson
vt2geojson --layer building https://tiles.wifidb.net/data/planetiler-0-7-0_pmtiles/14/3094/6569.pbf > 14-3094-6569.geojson
this makes a multipolygon like
multipolygon
14-3094-6569.geojson.txt

If I put that into the maplibre building example I get this stackblitz example page
https://stackblitz.com/edit/web-platform-zaxezv?file=index.html
In this example you can see these buildings floating in the zoom 14 range
floating_buildings
it looks like they get placed at a height in the center of the multipolygon above, which is much higher than the terrain where the building sits, causing them to float.

I also made this example using a live planetiler 0.7.0 pmtiles file, you can see it looks much worse than the geojson example, with this issue showing past zoom 14
https://stackblitz.com/edit/web-platform-qmqmrs?file=index.html
not only do you see building floating, but you also see large groups of buildings completely or partially underground
image

maplibre-gl-js version: 3.5.2
browser: chrome

@HarelM
Copy link
Collaborator

HarelM commented Nov 5, 2023

If I needed to guess it has to do with the fact that it probably treats the entire multiploygon with the same elevation.
Having said that, it does act weird in the transition between zoom 13 and zoom 14, and between zoon 14 and zoom 15.
If I disable all the styling related to height and min base, I still see the buildings jump up and down when changing the zoom levels, not sure why...

@HarelM HarelM added bug Something isn't working Terrain 3D PR is more than welcomed Extra attention is needed labels Nov 5, 2023
@acalcutt
Copy link
Contributor Author

acalcutt commented Nov 5, 2023

I did notice that with the geojson source. in the second example with full planet file it seems to be floating at all zoom levels and they don't jump around like that. I'm not quite sure the difference though since that geojson was pulled from the tile...maybe something to do with being stiched with other tiles in the full planet example.

@prozessor13
Copy link
Collaborator

Hi, yes, elevation is pulled per feature only once. I dont know if this is a bug or not? More a design decision? Why there is a difference between geojson and normal vectortile, i also dont know.

@HarelM
Copy link
Collaborator

HarelM commented Nov 9, 2023

@prozessor13 how complicated it would be to split the elevation calculation to be per polygon?

@prozessor13
Copy link
Collaborator

Hi Harel,

it should be simple: https://github.com/maplibre/maplibre-gl-js/blob/main/src/data/bucket/fill_extrusion_bucket.ts#L163

but i am not sure if thats the way to go. Think about multipolygon buildings, like churches or other complex buildings in heavy terrain.

@HarelM
Copy link
Collaborator

HarelM commented Nov 9, 2023

I see, thanks for the input!
cc: @msbarry - see comment above related to planetiler polygon compression...

@msbarry
Copy link
Contributor

msbarry commented Nov 9, 2023

Thanks! It might be useful to find one counterexample multipolygon building here that you want to retain this behavior. To me it seems like we've got these cases:

  • If the building has multiple parts at different heights to define a complex roofline then those will be separate polygons anyway since they have different height tags.
  • If a multipolygon building has multiple parts with the same height tag that spans a large area over heavy terrain then what does that even mean? To me it seems like the height would apply to the elevation above the ground for each part

Maybe what we're getting at here is the need to explicitly group several building:part polygons into one "height group" that share the same base elevation. A multipolygon without this would get a new base for each part. A single multipolygon with this would get the same base for each part. Many polygons/multipolygons with the same value would all get the same base height.

@prozessor13
Copy link
Collaborator

  • If the building has multiple parts at different heights to define a complex roofline then those will be separate polygons anyway since they have different height tags.

Ah, did not thought about this, you are right. Changing grabbing elevation for each polygon is definitely worth to try.

@acalcutt
Copy link
Contributor Author

acalcutt commented Aug 9, 2024

Fixed by #3841

@acalcutt acalcutt closed this as completed Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed Terrain 3D
Projects
None yet
Development

No branches or pull requests

4 participants