[Patch] Use mapcss rules instead of CrossingWays java code to find overlapping areas

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

[Don-vip]

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.

[GerdP]

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

[skyper]

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.

[GerdP]

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.

[GerdP]

No more objections?

[GerdP]

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

[GerdP]

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.

[GerdP]

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.

[GerdP]

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.

[Hb---]

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

[GerdP]

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.

[Hb---]

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.

[GerdP]

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.

[skyper]

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}" );
}

[GerdP]

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.

[Hb---]

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.

[GerdP]

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

[GerdP]

I misunderstood the test. It's not a simple Crossing test anymore

Right, because the problem never was about crossing ways. A building overlapping a landuse area is still wrong when it shares nodes with the landuse boundary.
I don't want to remove the test CrossingWays, I just want a better test to find the problematic areas.

[stoecker]

Milestone renamed

[skyper]

@GerdP:
Please, have a look at #20378 and #20680. Crossing and Overlapping test need to watch the child nodes of the ways in order to trigger a warning on upload.

[GerdP]

Yes, many tests fail whe they are executed with a selection. It's not enough to check the ways which have modified nodes.

[GerdP]

updated patch

[GerdP]

I plan to work on this again when #23397 (and thus also #20680) are fixed.

/* #20130 Building crossing landuse (spatial test) */
area[building][building!~/no|entrance/] ⧉o area:closed:areaStyle[landuse=~ /^(commercial|farmyard|garages|industrial|retail|residential)$/][landuse] {
  throwWarning: tr("Crossing building/{0} area", "{3.tag}");
}

The message with this version would be e.g. Crossing building/landuse=residential area
This is what will be shown when you hit 6 (Zoom to problem):

[GerdP]

screenshot with highlighted areas