#16707 closed enhancement (fixed)
Warn about overlapping connected buildings
Reported by: | naoliv | Owned by: | GerdP |
---|---|---|---|
Priority: | normal | Milestone: | 20.03 |
Component: | Core validator | Version: | |
Keywords: | building overlap | Cc: | GerdP, michael2402, Klumbumbus |
Description
It seems that for cases like this (also attached the example) JOSM should warn about the overlapping buildings.
We can only see an informational level message about "Overlapping Areas", but not any kind of warning about the overlapping buildings.
JOSM:
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2018-09-02 22:28:43 +0200 (Sun, 02 Sep 2018) Revision:14220 Build-Date:2018-09-03 01:32:21 URL:https://josm.openstreetmap.de/svn/trunk Identification: JOSM/1.5 (14220 en) Linux Debian GNU/Linux testing (buster) Memory Usage: 819 MB / 7168 MB (329 MB allocated, but free) Java version: 10.0.2+13-Debian-1, Oracle Corporation, OpenJDK 64-Bit Server VM Screen: :0.0 1600x900, :0.1 1280x1024 Maximum Screen Size: 1600x1024 Java package: openjdk-10-jre:amd64-10.0.2+13-1 Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-21 VM arguments: [--add-modules=java.activation,java.se.ee, -Dawt.useSystemAAFontSettings=gasp] Program arguments: [--language=en] Dataset consistency test: No problems found
Attachments (3)
Change History (26)
Changed 2 years ago by
Attachment: | example.osm added |
---|
comment:1 Changed 2 years ago by
Keywords: | building overlap added |
---|---|
Milestone: | → 18.09 |
Type: | enhancement → defect |
comment:2 Changed 2 years ago by
Cc: | GerdP michael2402 added |
---|---|
Milestone: | 18.09 |
Type: | defect → enhancement |
comment:3 Changed 11 months ago by
We can use the routines from MultipolygonTest for this. Something like
collect all building polygons (not multipolygons) in collection buildings and call
MultipolygonTest mptest = new MultipolygonTest(); mpTest.makeFromWays(buildings); for (TestError err mpTest.getErrors()) { ... }
Of course we have to filter warnings about touching rings.
We could also examine the resulting relation to check if any inners were created.
comment:4 Changed 11 months ago by
Cc: | Klumbumbus added |
---|
We have a test in geometry.mapcss
area:closed:areaStyle ⧉ area:closed:areaStyle { throwOther: tr("Overlapping Areas"); }
which finds this kind of problem.
We could add rules like this to produce a warning for some of the objects which are tested in CrossingWays
and remove them from CrossingWays
or I might add a new java test OverlappingAreas
which would perform those tests.
What would be the preferred solution?
comment:5 Changed 11 months ago by
Java test can highlight the affected way segments, which is a big advantage over the mapcss tests.
comment:6 Changed 11 months ago by
Milestone: | → 20.03 |
---|---|
Owner: | changed from team to GerdP |
With buildings the highlighting is probably not so important, but with large natural polygons it can help. I've started to code today and first tests look promising. Some fine tuning is needed reg. layer tag, but performance should be no problem.
comment:7 Changed 11 months ago by
Seems I forgot what I wrote in ticket:17011#comment:21 and later :(
I think I failed because I didn't like the changes needed to implement better highlighting.
Changed 11 months ago by
Attachment: | 16707.patch added |
---|
comment:9 Changed 11 months ago by
The attached patch adds a test in geometry.mapcss and disables the corresponding test in the java code.
Please review the mapcss rule. I don't think we need the entrance part here but it also exists in the above rule for "building inside building", so I copied it.
@Klumbumbus: I think we should also replace these spatial tests in CrossingWays:
"Crossing building/residential area"
"Crossing residential areas"
"Crossing residential area/way"
comment:10 Changed 11 months ago by
The entrance part seems to be obsolete meanwhile. When it was added in 2014 (r6611) building=entrance had still ~135.000 uses. However mapped as area was always strange I guess (~500 at this time).
On the other hand the entrance part makes sense as building=entrance is deprecated in general. So it avoids throwing the "building inside building" warning for a case where we have the "is deprecated" warning anyway. Then again, we have more deprecated building values in deprecated.mapcss which could be added too...
comment:11 Changed 11 months ago by
Problem with test "Crossing building/residential area":
If we replace that with the spatial test the "good" part of the building is highlighted :(
Changed 11 months ago by
Attachment: | 16707.2.patch added |
---|
comment:12 Changed 11 months ago by
Please review 16707.2.patch:
- use spatial test in geometry.mapcss instead of CrossingWays.java to find overlapping buildings or overlapping residential areas.
- "Crossing buildings" is replaced by "Overlapping buildings"
- "Crossing residential areas" is replaced by "Overlapping identical landuses"
If I got that right the message "Crossing residential area/way" is never actually produced because closed areaStyle ways with e.g. amenity=parking are ignored in CrossingWays.
comment:13 Changed 11 months ago by
I forgot about the tag level=*
In CrossingWays
we ignore all objects with different level tag. Should we do the same in the CrossingFinder
?
Up to now CrossingFinder
evaluates the layer tag only. I think this is OK, see #18293.
comment:18 Changed 6 months ago by
Thanks to someone trying to add spam to this ticket I saw this forgotten rule :D
comment:19 Changed 6 months ago by
Hmm. Thinking about it again r16779 likely produces too much false positives. building:part usually is not used with layer=* so overlaps exist. More opinions?
comment:20 follow-up: 21 Changed 6 months ago by
That's why I wrote comment:16
I was also unsure if building:part=* is allowed to overlap building=*
comment:21 follow-up: 22 Changed 6 months ago by
Replying to GerdP:
if building:part=* is allowed to overlap building=*
I would say no. This line should be fine:
area[building][building!~/no|entrance/] ⧉ area[building:part][building:part!~/no|entrance/],
comment:22 Changed 6 months ago by
So we need to look at building:levels
and building:min_level
to find overlapping building:parts
. Still would add a sharing crossing node if the parts are attached, e.g. on top of each other without space in between.
Replying to Klumbumbus:
Replying to GerdP:
if building:part=* is allowed to overlap building=*
I would say no. This line should be fine:
area[building][building!~/no|entrance/] ⧉ area[building:part][building:part!~/no|entrance/],
+1, but I have not looked at real data, yet, and where is a complex example on the wiki?
Not so easy as I thought. CrossingWays should probably be reworked to detect this, but it could quickly result in a major performance loss. Do we have a volunteer for the task? :)