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

Implement multipolygon support for table buildings #2476

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

Famlam
Copy link
Collaborator

@Famlam Famlam commented Mar 15, 2025

Implement multipolygon support for table buildings and convert the analysers accordingly.

Notes:

  • For the two cases of b1.id > b2.id in analyser_osmosis_building_overlaps I assume this was just such that comparing [A B C] with [A B C] would go like [BA, CA, CB] (n!-n comparisons) rather than [BA, CA, AB, CB, AC, BC] (n^2-n comparisons, if b1!=b2), e.g. it doesn't have to know the numerical value. This makes it much easier to keep it as it is as it may have to compare relations with ways.
  • I dropped the buildings.relation property. Based on the commit history this was added a decade ago in case a way with tag building=* was part of a multipolygon relation. However, nowadays the building=* tag should be on the multipolygon relation itself, not on the (outer) way. If it's on the inner way, that's a valid building. (Additionally, this inadvertently also blocked detections involving buildings in e.g. associatedStreet relations)
  • In analyser_osmosis_building_in_polygon I switched the other multipolygon implementation to table polygon to simplify it, but forgot to make this a separate commit... apologies
  • [offtopic] I'm not sure what analyser osmosis_building_3nodes is supposed to detect, but it seems like an "overlapping buildings" detection dedicated to 3-node buildings and hasn't detected anything since 2018. Probably it's redundant due to the other overlapping buildings analyser.

@Famlam Famlam marked this pull request as ready for review March 16, 2025 13:28
@Famlam Famlam force-pushed the famlam-multipoly-buildings branch from 4a9b87c to fc84601 Compare March 16, 2025 13:34
@frodrigo
Copy link
Member

[offtopic] I'm not sure what analyser osmosis_building_3nodes is supposed to detect, but it seems like an "overlapping buildings" detection dedicated to 3-node buildings and hasn't detected anything since 2018. Probably it's redundant due to the other overlapping buildings analyser.

I guess it was to detected small spited building part imported from French cadastre.

If now it is broken or useless we can remove it, rather than run it every days!

@didier2020 It looks like you made this analyser.

@Famlam
Copy link
Collaborator Author

Famlam commented Mar 17, 2025

I guess it was to detected small spited building part imported from French cadastre.

I suspect these cases are covered nowadays by the building_overlaps analyser (class 1 + 2 for overlaps and 6 for "triangles" that only touch). I'll remove it if no-one objects (in a separate PR, after this one)

@didier2020
Copy link
Contributor

didier2020 commented Mar 18, 2025

[offtopic] I'm not sure what analyser osmosis_building_3nodes is supposed to detect, but it seems like an "overlapping buildings" detection dedicated to 3-node buildings and hasn't detected anything since 2018. Probably it's redundant due to the other overlapping buildings analyser.

I guess it was to detected small spited building part imported from French cadastre.

If now it is broken or useless we can remove it, rather than run it every days!

@didier2020 It looks like you made this analyser.
hello, it's old ...

dans le cadaste, les traits noir des delimitations de parcelles "coupaient" les batiments en orange ce qui générait des batiments morcelés. les batiments a 3 points (en triangle) cela n'existe pas trop dans la realité, c'était donc un moyen de detecter ces zones de batiments pour les corriger. Depuis, les exports du cadastre ont été totalement revus (cf http://cadastre.openstreetmap.fr/ http://cadastre.openstreetmap.fr/data/076/T4255-EU-houses-simplifie.osm), cette analyse n'a plus de sens

In the land registry, the black lines of the plot boundaries "cut" the orange buildings, resulting in fragmented buildings.
Buildings with 3 points (in a triangle) don't really exist in reality, so this was a way to detect these building zones and correct them.
Since then, the land registry exports have been completely revised; this analysis no longer makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants