Confusing handling of water objects in CrossingWays

[GerdP]

What steps will reproduce the problem?

1. Load attached file
2. Run Validator

What is the expected result?

• Warnings for crossing ways for the two footways and the barrier
• better no warning "Crossing ways" for the two landuse=reservoir ways and the two natural=water was as they are duplicating the results of dedicated mapcss tests which produce "Overlapping Water Areas"

What happens instead?

• no warnings about footways or barriers
• obsolete (duplicate) warnings for water areas

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

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-10-03 13:42:38 +0200 (Sat, 03 Oct 2020)
Build-Date:2020-10-04 01:30:47
Revision:17084
Relative:URL: ^/trunk

Identification: JOSM/1.5 (17084 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 2004 (19041)
Memory Usage: 1037 MB / 3641 MB (888 MB allocated, but free)
Java version: 1.8.0_221-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920x1080 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 32x32, 32x32 -> 32x32
VM arguments: [-XX:StartFlightRecording=name=MyRecording2,settings=d:\dbg\gerd.jfc, -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true,dumponexitpath=e:\ld\perf_20201120_170253.jfr]
Dataset consistency test: No problems found

Plugins:
+ buildings_tools (35640)
+ o5m (35640)
+ pbf (35640)
+ poly (35640)
+ reltoolbox (35640)
+ reverter (35640)
+ undelete (35640)
+ utilsplugin2 (35640)

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

[GerdP]

I am also no longer happy with the results for self crossing ways. Since r11136 (my patch) all ways are tested for self crossing, but only some are reported as "Self crossing ways". A self crossing barrier is reported as "Crossing barriers"
A boundary=administrative is reported as "Crossing boundaries", but a boundary=protected_area is reported as "Self crossing ways"
I think all self crossing ways should be reported as "Self crossing ways"

[GerdP]

work-in-progress patch. I'll add unit tests:

• report all self crossing ways with the same message
• ignore corssing water areas because they are reported by mapcss geometry tests
• don't ignore crossing between water areas and highway, railway, barrier, or building
• remove redundant and missleading checks in CrossingWays.Ways.ignoreWaySegmentCombination()

[GerdP]

We have special code to ignore crossings of waterway=riverbank with linear waterway objects. The combination of natural=water+water=river is suggested as an alternative to waterway=riverbank.
What about waterway=river crossing this alternative?
My patch produces new warnings like "Crossing waterway/way" for them, see eg.area around n2075246907.

[GerdP]

The wiki also mentions natural=water with other water objects like canal, stream, ditch, or drain.
So, I guess we should not produce any message for linear water objects crossing water areas since this is an often used mapping style.
One disadvantage of this exclusion is that we don't report cases where a river flows outside of its own riverbed.

[skyper]

natural=water usage does depend on the tagging style. In some areas it is used for all water areas but, as this "new" system does produce duplicates with other tags or might change the meaning of established tags like some landuse=*, it is still debatable.

Does a common node between the area and the linear way harm?
Would a crossing between an area and a linear way be worse an info warning?

[GerdP]

Does a common node between the area and the linear way harm?

No idea. See #6911 and #13069.

Would a crossing between an area and a linear way be worse an info warning?

I think JOSM should only produce messages when something is at least dubious. A power=line crossing landuse areas is normal and thus should not be reported. It's probably better to analyse only those linear ways which are problematic.

I am working on code to also find crossing ways when e.g. a highway crosses a building multipolygon.

[skyper]

Well in my eyes, we cannot tell if a linear waterway crossing a water area is a problem or not. The example with a river crossing its riverbank might be an error but does not have to. Similar is true for a lake/pond and a linear waterway. The waterway might run through the area or not. Without common nodes one of the objects might just have been placed wrong.

Similar is true for highways.

The problem is with different mapping styles and different interest in details, we have a huge gray list and only small black and white lists.

We can work with more, detailed warnings and the three levels of warning types to get more explicit errors/warnings/others which can be added to the ignore list in order to get rid of warnings you aren't interested in.

Personally, I'd like to get a separate warning about each, linear crossing area, for highway, railway, waterway. I miss at info warning about *way crossing certain areas like highway + amenity=parking or railway + amenity=*.

[GerdP]

Well in my eyes, we cannot tell if a linear waterway crossing a water area is a problem or not.

I came to the same conclusion. A common node doesn't change this, so it is OK to ignore these crossings to avoid many false positives.

Similar is true for highways.

Do you mean highway with water areas or with area:highway?

Personally, I'd like to get a separate warning about each, linear crossing area, for highway, railway, waterway. I miss an info warning about *way crossing certain areas like highway + amenity=parking or railway + amenity=*.

#19600 is probably related.

Maybe one problem with this test is that it reports different types of problems. While crossing highways often just need a common node the problem with a highway crossing the outline of a building or a natural=water is completely different. Adding a node doesn't fix the problem but will remove the warning. Mappers might say "JOSM told me to add a common node".

[Klumbumbus]

Replying to GerdP:

My patch produces new warnings like "Crossing waterway/way" for them, see eg.area around n2075246907.

The mapping around this node is fine and shouldn't produce a warning.

[GerdP]

Yes, agreed.

[GerdP]

I think self crossing ways should always be flagged as an error instead of a warning. Right?

[GerdP]

Would a crossing between an area and a linear way be worse an info warning?

The current (unpatched) code roughly works like this:

1. Collect all ways which match certain criteria. (list of tag key/value combinations in isPrimitiveUsable()
2. Find crossing segments between all those ways
3. For each pair of crossing ways decide if a message should be produced (method ignoreWaySegmentCombination())
4. Use the tags of the crossing ways to decide what message text we produce

We have a very similar approach in OverlappingWays, but is also consideres some preference keys.
I don't know if new mapcss selectors to find crossing/shared segments or would help here?
WayConnectedToArea looks like another candidate, but I am not yet familiar with that. It produces the Way terminates on Area messages.

[skyper]

Replying to GerdP:

Well in my eyes, we cannot tell if a linear waterway crossing a water area is a problem or not.

I came to the same conclusion. A common node doesn't change this, so it is OK to ignore these crossings to avoid many false positives.

A common node is an indicator. There will be no common node if the river leaves its area, accicdently. I have no problem with false positives on info level, though, these test were and some still are too general, which lead/led to many false positives without proper message.

Similar is true for highways.

Do you mean highway with water areas or with area:highway?

I meant highway=* crossing highway=* with area=*. area:highway is special and I would add common nodes but I am not sure if that is expected.

Personally, I'd like to get a separate warning about each, linear crossing area, for highway, railway, waterway. I miss an info warning about *way crossing certain areas like highway + amenity=parking or railway + amenity=*.

#19600 is probably related.

Maybe one problem with this test is that it reports different types of problems. While crossing highways often just need a common node the problem with a highway crossing the outline of a building or a natural=water is completely different. Adding a node doesn't fix the problem but will remove the warning. Mappers might say "JOSM told me to add a common node".

+1, we need separate warnings and tests depending on the tags of the crossing ways. The situation might also depend on the tags of the common node and if a linear way of the same kind is sharing the common node. With a highway crossing a building, I expect the highway to have a different layer=* and bridge=* or tunnel=*. With common nodes, I expect to have the way inside the building to have tunnel=building_passage and be connected to other highway=* at the common nodes. If the linear highway=* ends at the common node with the building without another linear highway to the inside connected, I expect a tag on the common node like entrance=* or barrier=*.

Replying to GerdP:

I think self crossing ways should always be flagged as an error instead of a warning. Right?

Are you talking about self-crossing ways with or without intersection node. With an intersection node the object is valid, though, I do not like it, but I am not sure if an error warning is correct.

[GerdP]

Replying to skyper:

Replying to GerdP:

I think self crossing ways should always be flagged as an error instead of a warning. Right?

Are you talking about self-crossing ways with or without intersection node. With an intersection node the object is valid, though, I do not like it, but I am not sure if an error warning is correct.

We have a separate test for self-intersecting ways (with node). We allow p-shaped ways there because they are very common.

[GerdP]

In 17400/josm:

fix #20121: Confusing handling of water objects in CrossingWays

• report all self crossing ways with the same message
• ignore corssing water areas because they are reported by mapcss geometry tests
• don't ignore crossing between water areas and highway, railway, barrier, or building
• remove redundant and missleading checks in CrossingWays.Ways.ignoreWaySegmentCombination()
• add unit test for coverage and test of these changes
• fix @since

[GerdP]

I guess we'll get complains about the case when a waterway=river crosses the waterway=riverbank without sharing a node.

[skyper]

Replying to GerdP:

I guess we'll get complains about the case when a waterway=river crosses the waterway=riverbank without sharing a node.

See #20352.