Modify

Opened 9 days ago

Last modified 5 days ago

#20121 assigned defect

Confusing handling of water objects in CrossingWays

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

Description

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

Attachments (2)

20121-data.osm (4.5 KB) - added by GerdP 9 days ago.
20121.patch (5.4 KB) - added by GerdP 8 days ago.

Download all attachments as: .zip

Change History (16)

Changed 9 days ago by GerdP

Attachment: 20121-data.osm added

comment:1 Changed 8 days ago by GerdP

Owner: changed from team to GerdP
Status: newassigned

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"

Last edited 8 days ago by GerdP (previous) (diff)

comment:2 Changed 8 days ago by 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()

Changed 8 days ago by GerdP

Attachment: 20121.patch added

comment:3 Changed 8 days ago by GerdP

Cc: Klumbumbus skyper added

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.

comment:4 Changed 8 days ago by 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.

comment:5 Changed 8 days ago by 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?

comment:6 Changed 8 days ago by 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.

comment:7 Changed 8 days ago by 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=*.

comment:8 Changed 8 days ago by 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".

Last edited 5 days ago by skyper (previous) (diff)

comment:9 in reply to:  3 Changed 7 days ago by 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.

comment:10 Changed 7 days ago by GerdP

Yes, agreed.

comment:11 Changed 7 days ago by GerdP

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

comment:12 Changed 7 days ago by 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.

comment:13 in reply to:  11 ; Changed 5 days ago by 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.

Last edited 5 days ago by skyper (previous) (diff)

comment:14 in reply to:  13 Changed 5 days ago by 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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as assigned The owner will remain GerdP.
as The resolution will be set.
to The owner will be changed from GerdP to the specified user.
The owner will change to GerdP
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket

Add Comment


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

 
Note: See TracTickets for help on using tickets.