Modify

Opened 7 years ago

Closed 7 years ago

#9361 closed defect (fixed)

[Patch] building inside building should not be triggered for hollow buildings

Reported by: mkoniecz Owned by: team
Priority: normal Milestone: 14.05
Component: Core validator Version:
Keywords: Cc: Balaitous

Description (last modified by mkoniecz)

Example: osmwww:browse/way/151913827 inside osmwww:browse/relation/2052964

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2013-11-20 02:34:39
Last Changed Author: Don-vip
Revision: 6394
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2013-11-19 02:29:10 +0100 (Tue, 19 Nov 2013)
Last Changed Rev: 6394

Identification: JOSM/1.5 (6394 en_GB) Windows 7 32-Bit
Memory Usage: 84 MB / 247 MB (26 MB allocated, but free)
Java version: 1.7.0_45, Oracle Corporation, Java HotSpot(TM) Client VM
Dataset consistency test: No problems found

Plugin: OpeningHoursEditor (29854)
Plugin: notes (v0.6)

Attachments (3)

OverlappingIdenticalLanduse.osm (44.9 KB) - added by mdk 7 years ago.
showBothOverlapping.patch (2.9 KB) - added by akks 7 years ago.
9361.patch (4.2 KB) - added by simon04 7 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 7 years ago by mkoniecz

Description: modified (diff)

comment:2 Changed 7 years ago by Don-vip

Milestone: 14.01

comment:3 Changed 7 years ago by simon04

This is due to building=yes being set on the outer way. This causes the code to test containment using the outer ring as it is without the multipolygon relation.

comment:4 in reply to:  3 ; Changed 7 years ago by skyper

Replying to simon04:

This is due to building=yes being set on the outer way. This causes the code to test containment using the outer ring as it is without the multipolygon relation.

IMO this is an outdated way of tagging. What we probably need is an additional warning about an multipolygon without an primary tag like *way,building,landuse,natural etc.. Adding building=yes to the relation will/should raise a warning about same tags on outer way and relation.

comment:5 Changed 7 years ago by simon04

Milestone: 14.0114.02

This is not too easy to fix …

comment:6 Changed 7 years ago by mdk

This looks very similar to #7034. It looks like a new variation not handled yet.

The problem is definitely the building=yes on the outer way. The missing of any tags in the relation is not responsible for this problem, but for me this looks like a mapping error. Redundant tags on outer way and MP are unnecessary, but no error.

Changed 7 years ago by mdk

comment:7 Changed 7 years ago by mdk

I realized, that the problem is more general. Ignoring multipolygon relations creates some false positives like
outer: natural=wetland (http://www.openstreetmap.org/way/24543664)
inner: natural=water (http://www.openstreetmap.org/way/44866110)
relation: type=multipolygon natural=wetland (http://www.openstreetmap.org/relation/1718903)
warning: Overlapping water areas

Even if I remove all tags from the outer way, I get the same warning!

Same problem with "Overlapping identical landuses". Can somebody explain me, where the problem is in the attached OverlappingIdenticalLanduse.osm file?

Is it possible to list BOTH objects if the validator found a problem between two? In both above cases only the relation is listed. I'm an experienced mapper, but it is hard or even impossible for me to find out, what the problems are.

comment:8 Changed 7 years ago by akks

Looking...

comment:9 Changed 7 years ago by akks

In OverlappingIdenticalLanduse.osm​ there is a real problem, validator is right: there exist both closed way AND multipoligon on the same area (try Alt-click)

However, I agree that validator should show both (or more overlapped objects).

comment:10 Changed 7 years ago by akks

By the way, there are special charachters ⧉ in geometry.mapcss
http://josm.openstreetmap.de/browser/josm/trunk/data/validator/geometry.mapcss
The look nice in Trac and maybe in some font-rich-Linux distributions, but not in Netbeans or Far Manager :)

Last edited 7 years ago by akks (previous) (diff)

comment:11 Changed 7 years ago by Don-vip

No problem with Eclipse :) You could open a ticket to Netbeans :)

comment:12 Changed 7 years ago by akks

What about special rule or modificator for marking both intersecting objects?
Is it possible with mapcss syntax?
Or maybe we should leave old overlapping tests written in classic Java and use mapcss tests only for markinig single incorrectly placed/tagged objects, not pairs?

Last edited 7 years ago by akks (previous) (diff)

comment:13 Changed 7 years ago by akks

Maybe offtopic (where to place it?), but

Here is an "intuitive" patch to show both opverlapping or intersecting objects detected with mapcss geometry vaildator.
It make detection of errors like in 6 much easier.

Since I do not fully understand mapcss processing chain (Selectors, Finder, Environment, etc. ), I ask the authors to check and include it in some form :)

Changed 7 years ago by akks

Attachment: showBothOverlapping.patch added

comment:14 Changed 7 years ago by simon04

In 6773/josm:

see ticket:9361#comment:13 - MapCSS validator: highlight both primitives for spatial tests - patch by akks

comment:15 in reply to:  13 Changed 7 years ago by simon04

Replying to akks:

Since I do not fully understand mapcss processing chain (Selectors, Finder, Environment, etc. ), I ask the authors to check and include it in some form :)

Nice enhancement and patch. :-) I slightly adapted your patch, mainly to fix the test case compile error.

comment:16 in reply to:  4 Changed 7 years ago by aceman

Replying to skyper:

Replying to simon04:
What we probably need is an additional warning about an multipolygon without an primary tag like *way,building,landuse,natural etc...

+1, can we get it?

comment:17 Changed 7 years ago by Don-vip

Milestone: 14.0214.03

comment:18 Changed 7 years ago by Don-vip

Milestone: 14.0314.04

comment:19 Changed 7 years ago by Don-vip

Milestone: 14.0414.05

comment:20 Changed 7 years ago by Don-vip

Cc: Balaitous added

I think this is the same problem as reported in #10037 comments.

comment:21 Changed 7 years ago by simon04

Summary: building inside building should not be triggered for hollow buildings[Patch] building inside building should not be triggered for hollow buildings

The patch is a bit of a hack, but it seems to work for the example in the ticket description.

Changed 7 years ago by simon04

Attachment: 9361.patch added

comment:22 Changed 7 years ago by bastiK

The different styles of area tagging (closed way, multipolygon with tags on relation or tags on outer way) make this very messy. As long as we do not have a more abstract concept of an area, this may be the best we can do.

It should probably be

containsFinder = new ContainsFinder(e.withPrimitive(multipolygon)) { 

instead of

e.osm = multipolygon;

Also note that there is now a class OptimizedGeneralSelector which is a base class of GeneralSelector, so should be used instead for instanceof and casting.

comment:23 Changed 7 years ago by simon04

Paul, thanks for the feedback. Let's go with this patch for now (with your suggested modification), since it fixes this bug …

comment:24 Changed 7 years ago by simon04

Resolution: fixed
Status: newclosed

In 7169/josm:

fix #9361 - MapCSS: consider multipolygon when matching outer ring of a multipolygon for the selector

Modify Ticket

Change Properties
Set your email in Preferences
Action
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.