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

Building outline overlaps building:part= features #1062

Closed
mxxcon opened this issue Aug 29, 2023 · 5 comments
Closed

Building outline overlaps building:part= features #1062

mxxcon opened this issue Aug 29, 2023 · 5 comments
Labels
feature-3d Issue or request related to the 3d viewer
Milestone

Comments

@mxxcon
Copy link

mxxcon commented Aug 29, 2023

Description

I think 3d building preview has an oversight or a bug. The perimeter building= way is supposed to have values of the maximum height= and the highest floor(building:levels=).
It looks like the building= way overrides any building:part=yes ways. But it's supposed to be other way around.
image

Here you can see that I selected a section of a building that's smaller than its highest point and it's swallowed by the overall building volume.

from https://wiki.openstreetmap.org/wiki/Key:building:part

Where a building has been split into parts, a building outline has still to be drawn of the whole building way and should have the tags building:levels=* and height=*. These values should be the maximum of all the building parts. A building that has a three-story part and a six-story part is still a six-story building.

Version

2.1

What browser are you seeing the problem on? What version are you running?

Firefox 117, Chrome 117

The OS you're using

Windows 11

Steps to reproduce

Look at building 1052877937

The browser URL at the time you encountered the bug

https://rapideditor.org/rapid#background=Bing&id=w1052877937&map=20.17/40.58633/-73.96477

The auto-detected useragent string for your browser (leave blank if you're manually filling this form out)

No response

@bhousel bhousel added the feature-3d Issue or request related to the 3d viewer label Aug 29, 2023
@Bonkles
Copy link
Contributor

Bonkles commented Aug 29, 2023

Thanks for the submission @mxxcon! There seems to exist a bit of a discrepancy between the way the wiki prescribes the way heights should be mapped, and the way things are often mapped in practice. Additionally, I'm not sure how to actually make the renderer handle this case. I'll expand a bit:

A lot of multi-part buildings that are mapped with min/max heights in OSM do not actually seem to honor the 'Rules-as-Written' that you reference here in the wiki- that is to say, they do not have a single all-encompassing building footprint with height/level values that fully enclose all the parts.

Ex: the Lincoln memorial

image

Writing code to hide the boundary polygon isn't trivial and may not catch every instance. It would also slow down the renderer, as we'd have to start recursively checking the scene for encompassing polygon boundaries, then evaluating their heights with respect to each building part within it, etc.

I'm going to have to think on it a bit more!

@mxxcon
Copy link
Author

mxxcon commented Aug 29, 2023

A lot of multi-part buildings that are mapped with min/max heights in OSM do not actually seem to honor the 'Rules-as-Written' that you reference here in the wiki- that is to say, they do not have a single all-encompassing building footprint with height/level values that fully enclose all the parts.

That sounds more like tagging for renderer behavior

And with rapid's current behavior it might also encourage people to incorrectly tag buildings just so that they look good in that preview window.

Perhaps the simplest solution would be in there's even 1 building:part anywhere in the perimeter of building=, don't render perimeter shape. That might actually encourage people to fill in missing sections because it'll look obvious incomplete

@bhousel
Copy link
Contributor

bhousel commented Aug 29, 2023

I don't have an opinion on the correct approach and haven't really looked into 3D tagging but ...
Just wanted to add that now that we are able to render 3D shapes, I would be ok with adding validation rules to catch cases where the buildings are tagged incorrectly (whatever we decide that means).

@Bonkles
Copy link
Contributor

Bonkles commented Aug 31, 2023

Wrote some code to try this approach out. The algorithm is:

  1. For each building perimeter in the scene that isn't a 'part'
  2. For each of that building's nodes
  3. Get all the node's parent ways
  4. If any of those parent ways are also contain the building:part tag, AND the way is wholly contained by the building, ignore this building for rendering purposes
fix_for_1062.mov

Not sure if this is sufficient, but it seems to be a start.

@Bonkles
Copy link
Contributor

Bonkles commented Aug 31, 2023

Update- this seems to be sufficient to hide the polygons but absolutely blows up performance once you zoom out a lot.

(The algorithm is complicated and gets incredibly unwieldy very fast).

So I will ship a compromise- anytime there are more than 250 buildings in the scene, we don't run the expensive algorithm (ie. we just show everything). As soon as there are fewer than 250 buildings in the scene, we will do the extra work to hide the encompassing building shapes.

The overall effect is something akin to 'refining' the shape of the multi-part building on zoom-in.

multi-part-building-zoom-in.mov

@Bonkles Bonkles added this to the v2.1 milestone Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-3d Issue or request related to the 3d viewer
Projects
None yet
Development

No branches or pull requests

3 participants