Opened 5 years ago

Last modified 2 years ago

#20130 assigned enhancement

Use mapcss rules instead of CrossingWays java code to find overlapping areas — at Version 20

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone:
Component: Core validator Version:
Keywords: crossing Cc: Klumbumbus, simon04

Description (last modified by GerdP)

Today we use the java code in CrossingWays to find e.g. "Crossing building/residential area".
This should better be done using a rule like this

/* Building overlapping landuse residential (spatial test) */
area[building][building!~/no|entrance/]  area[landuse=residential]{
  throwWarning: tr("Overlapping buildings/residential area");
}

Reasoning:

  • it also handles multipolygons (if they are complete)
  • it marks the overlapping area, not the crossing segments in fact this is a contra argument
  • it also handles overlaps where the two areas share nodes

In r17084 CrossingWays also finds crossings between

  • building/waterway=riverbank ("Crossing building/waterway")
  • building/railway=platform ("Crossing building/railway" (false positive?)

It doesn't find other crossing like those between building/natural=water, see #20121

Maybe it is possible to create a single mapcss test to catch different types of areas which shouldn't overlap a building?

I didn't test the impact on performance, I assume it is rather small.

Change History (22)

comment:1 by GerdP, 5 years ago

Description: modified (diff)

comment:2 by Don-vip, 5 years ago

Cc: simon04 added
Component: CoreCore validator
Keywords: crossing added

I think we already discussed about it in another ticket and there was a problem, but I don't remember what. I believe CrossingWays is doing something better than the mapCSS rule, maybe the UI reporting of the crossing location, or something like that.

comment:3 by GerdP, 5 years ago

In the past, the highlighting of CrossingWays was better, now it's rather the other way around.

in reply to:  description comment:4 by skyper, 5 years ago

Replying to GerdP:

Reasoning:

  • it marks the overlapping area, not the crossing segments

Actually with long ways, I prefer the highlighting of the crossing segments. Selecting the object is already possible, so highlighting the whole ways is not an advantage.

comment:5 by GerdP, 5 years ago

so highlighting the whole ways is not an advantage.

I don't understand. It is not highlighting the whole way unless the whole area is overlapping. With building the area is rarely large. F6 zooms to the interesting (=overlapping) area.
Please try the proposed rule and compare the highlighting.

Anyway, more important is that it finds all overlaps.

comment:6 by GerdP, 5 years ago

No more objections?

comment:7 by GerdP, 5 years ago

OK, I'll commit the changes and we'll see what happens...

comment:8 by GerdP, 5 years ago

Well, now I remember why this wasn't already done: The marked overlapping area is that inside the residential area, but the problematic part is that which is outside.
So, we need a different selector for this.

by GerdP, 5 years ago

Attachment: 20130.patch added

comment:9 by GerdP, 5 years ago

Patch implements new selector ⧉o . Works like ⧉ but marks the part of the child that is outside. It also contains this rule:

area:closed:areaStyle[landuse=residential] o area[building][building!~/no|entrance/] {
  throwWarning: tr("Building partly outside of residential area");
}

I found no obvious single unicode character to express this. Please suggest improvements.

comment:10 by GerdP, 5 years ago

Milestone: 21.01
Owner: changed from team to GerdP
Status: newassigned

by GerdP, 5 years ago

Attachment: 20130.2.patch added

comment:11 by GerdP, 5 years ago

20130.2.patch

  • improves the java code so that it doesn't produce errors where the highlighted area is empty. This happened with landuse=* multipolygons where the building just shares a node. (Unfortunately the "overlapping areas" test still marks this, will attach an example)
  • removes the handling of residential areas in CrossingWays
  • finds more landuse areas with the rule
    area:closed:areaStyle[landuse=~ /^(commercial|farmyard|garages|industrial|retail|residential)$/] o area[building][building!~/no|entrance/] {
      throwWarning: tr("Building partly outside of landuse area");
    }
    

If I see no complains or suggestions for improvements I'll commit this tomorrow.

comment:12 by Hb---, 5 years ago

Why not stay with the old string "Crossing building/residential area"? All crossing test messages begin with "Crossing".

comment:13 by GerdP, 5 years ago

I tried to make clear that the building should be completely inside the landuse area.
I would not mind to use "Crossing building/residential area" but the new test also complains about other types of areas, so we would either need one test for each type or a generic "Crossing building/{0} area" unless you think that "residential" is also a generic word for the listed types. I didn't find a way to extract the value of the parent tag for the {0] placeholder in the message. Any help would be welcomed.
"Crossing building/landuse area, should be inside" might be a better alternative.

comment:14 by Hb---, 5 years ago

Similar errors should have similar messages. A bad example of the opposite is the translation of "Crossing railways" to "Bahnkreuzung" in current josm-tested.

Landuse areas may neighbour each other. So peaking of "inside" may be wrong. It is more important that the landuse boundary does not cross the building. Maybe "Crossing building/landuse boundary" would say this.

But "Crossing building/landuse area" is fine, too.

comment:15 by GerdP, 5 years ago

But "Crossing building/landuse area" is fine, too.

Would you want to get that message for all landuse areas? If yes we need a 2nd test which would use the normal operator for all landuses not listed in the first test so that the correct part of the building is highlighted. Or at least a list of those landuses which are typically not containing buildings (forest, farmland, reservoir, ...). I'd prefer to warn only about those which typically contain buildings.

in reply to:  13 comment:16 by skyper, 5 years ago

Replying to GerdP:

I didn't find a way to extract the value of the parent tag for the {0] placeholder in the message. Any help would be welcomed.

Does it work with an additional [landuse]?

area:closed:areaStyle[landuse=~ /^(commercial|farmyard|garages|industrial|retail|residential)$/][landuse] o area[building][building!~/no|entrance/] {
  throwWarning: tr("Building partly outside of {0} area", "{1.tag}" );
}

comment:17 by GerdP, 5 years ago

No, "{1.tag}" refers to the tags of the building way. I'll try to change the logic so that the landuse area is the child.

in reply to:  15 comment:18 by Hb---, 5 years ago

Replying to GerdP:

Would you want to get that message for all landuse areas?

No. My scope was limited to the message, not the broadness of the test.

Or at least a list of those landuses which are typically not containing buildings (forest, farmland, reservoir, ...). I'd prefer to warn only about those which typically contain buildings.

I misunderstood the test. It's not a simple Crossing test anymore. Seems that you implement a "Badly placed buildings" test. I would fear to much false positives. A hut in a forest or a stable on a meadow seem to be very common. But a stable crossing the boundary between a forest and a meadow should give a warning.

comment:19 by GerdP, 5 years ago

No, my primary goal is to use the mapcss code because it also handles multipolygons. I tried to implement that with the CrossingWays test but that's a lot more complex because the message creation would need a lot of changes.

OTOH comment:17 is also not a good solution. It is quite normal to have a landuse=residential glued to a landuse=commercial and I didn't think about this. If a building crosses both we'll get two different messages for the same problem, but with different highlighted areas.

Working on another solution now...

comment:20 by GerdP, 5 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.