Warn about overlapping connected buildings

[naoliv]

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

[Don-vip]

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? :)

[GerdP]

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.

[GerdP]

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?

[Klumbumbus]

Java test can highlight the affected way segments, which is a big advantage over the mapcss tests.

[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.

[GerdP]

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.

[GerdP]

In 15938/josm:

see #16707: improve highlighting of overlapping areas and "zoom to error"

• store the overlapping area with the TestError
• return also the intersection area in Geometry.polygonIntersectionResult()

[GerdP]

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"

[Klumbumbus]

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...

[GerdP]

Problem with test "Crossing building/residential area":
If we replace that with the spatial test the "good" part of the building is highlighted :(

[GerdP]

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.

[GerdP]

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.

[GerdP]

In 15961/josm:

fix #16707: Warn about overlapping connected buildings

• add rule in geometry.mapcss to check overlapping buildings
• change rule which checks overlapping identical landuses to also check landuse=residential
• disable the corresponding tests in CrossingWays

effects for the user:

• Message "Crossing buildings" is replaced by "Overlapping buildings"
• Message "Crossing residential areas" is replaced by generic "Overlapping Identical Landuses"
• different highlighting of the overlapping area

[skyper]

please, include building:part=* in building test. Thanks

[GerdP]

@Klumbumbus: Do you know how?