Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#17695 closed defect (fixed)

Building inside building doesn't find all cases

Reported by: GerdP Owned by: team
Priority: normal Milestone: 19.05
Component: Core validator Version:
Keywords: template_report Cc:


What steps will reproduce the problem?

  1. Have a node inside a closed way, both tagged building=yes
  2. Make sure nothing is selected, run Validator
  3. Select the way, run Validator
  4. Select the node, run Validator

What is the expected result?

Steps 2, 3 and 4 should produce the same warning "Building inside building"

What happens instead?

Only steps 2 and 3 produce the warning

Please provide any additional information below. Attach a screenshot if possible.

The same problem occurs when you add a building=yes node inside a building and upload the changeset: No warning

Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2019-04-28 04:36:41 +0200 (Sun, 28 Apr 2019)
Build-Date:2019-04-28 02:37:58
Relative:URL: ^/trunk

Identification: JOSM/1.5 (15031 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1803 (17134)
Memory Usage: 591 MB / 1820 MB (356 MB allocated, but free)
Java version: 1.8.0_201-b09, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
VM arguments: [-XX:StartFlightRecording=name=MyRecording2,settings=d:\dbg\gerd.jfc, -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true,dumponexitpath=e:\ld\perf_20190508_115344.jfr]

+ OpeningHoursEditor (34977)
+ apache-commons (34908)
+ buildings_tools (34982)
+ continuosDownload (82)
+ ejml (34908)
+ geotools (34908)
+ jaxb (34908)
+ jts (34908)
+ o5m (34908)
+ opendata (34997)
+ pbf (34908)
+ poly (34991)
+ reverter (34977)
+ undelete (34977)
+ utilsplugin2 (34977)

Validator rules:
+ c:\josm\core\data\validator\geometry.mapcss
+ c:\josm\core\data\validator\wikipedia.mapcss

Last errors/warnings:
- W: No configuration settings found.  Using hardcoded default values for all pools.

Attachments (3)

17695.patch (2.5 KB) - added by GerdP 17 months ago.
Possible solution
17695-v2.patch (2.1 KB) - added by GerdP 17 months ago.
bib2.osm (11.8 KB) - added by GerdP 17 months ago.

Download all attachments as: .zip

Change History (11)

Changed 17 months ago by GerdP

Attachment: 17695.patch added

Possible solution

comment:1 Changed 17 months ago by GerdP

After uploading the patch I've just recognized that the same problem occurs when a way is inside a way. Will post a 2nd patch.

Changed 17 months ago by GerdP

Attachment: 17695-v2.patch added

comment:2 Changed 17 months ago by GerdP

v2 works also with ways, but doesn't solve the problem #14287. So, if you add two or more nodes with building=yes in the same enclosing building only one will be found, in fact the problem gets greater. If you select the two nodes JOSM will produce two warnings, but both for the same node :(

comment:3 Changed 17 months ago by GerdP

Resolution: fixed
Status: newclosed

In 15064/josm:

fix #12627,#14287,#14289,#17695

  • let CrossingFinder and ContainsFinder find all objects instead of stopping at first match
  • if objects are selected, make sure that ContainsFinder is called for enclosing objects which are not in the selection
  • enable corresponding unit tests

Draw back: MapCSSTagChecker is a bit slower.

Changed 17 months ago by GerdP

Attachment: bib2.osm added

comment:4 Changed 17 months ago by GerdP

Resolution: fixed
Status: closedreopened

Found a few more cases which still don't work. In attached file bib2.osm all 6 groups should produce a warning or error.
Both r15031 and r15067 don't find the errors in the bottom row.
The sample relation R3 doesn't work because we do not yet have code to find a multipolygon inside a polygon or multipolygon.

We have code which seems to support erreneous multipolygon relations where the outer way has the style attribute instead of the relation. This code doesn't work and is thus the reason for the problems in the lower row.

comment:5 Changed 17 months ago by GerdP

In 15068/josm:

see #17695: Fix ContainsFinder by removing wrong code

  • did not work because the results in the newly created environment were not transferred into the original environment
  • if that transfer is implemented false positives are produced for the examples in the lower row once you add another building inside the mp but outside of the inner way.

TODO: add code to support mp and improve performance of Geometry tests

comment:6 Changed 17 months ago by GerdP

Resolution: fixed
Status: reopenedclosed

In 15069/josm:

fix #17695

  • add support to find multipolygon inside polygon or multipolygon with ContainsFinder
  • add some unit tests
  • performance_1: improve isPolygonInsideMultiPolygon() by using the areas calculated

in MultipolygonBuilder.joinWays() instead of calling getArea() again.

  • performance_2: improve isPolygonInsideMultiPolygon() by first checking bounding boxes (helps with complex MP containing many inners as it avoids the Area.intersect() method)
  • performance_3: implement new methods to reuse result of complex method MultipolygonBuilder.joinWays() in ContainsFinder

comment:7 Changed 17 months ago by Don-vip

In 15083/josm:

fix #17729, see #17695 - don't call geometry functions for non-multipolygon relations

comment:8 in reply to:  3 Changed 17 months ago by GerdP

Last edited 17 months ago by GerdP (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment

E-mail address and name can be saved in the Preferences.

Note: See TracTickets for help on using tickets.