Modify

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#19465 closed enhancement (fixed)

Make "Overlapping ways" less aggressive

Reported by: mkoniecz Owned by: GerdP
Priority: normal Milestone: 20.12
Component: Core validator Version:
Keywords: template_report overlap way area:highway Cc:

Description

What steps will reproduce the problem?

  1. Map area
  2. Map second area partially sharing the way with the first area (two or three shared nodes)
  3. Run validator

What is the expected result?

No complaints as shared segment is short

What happens instead?

"Overlapping ways" is raised

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

Multipolygons make sense in case of long shared segments, but in case of minor overlaps creating multipolygons is not helpful - it takes plenty of time to convert into multipolygons, split ways, delete shared segments, in areas where segments were deleted add new ones...

And after all that work are becomes more complicated to edit (especially in less powerful editors) - or at best remains as editable as before.

Triggered by https://www.openstreetmap.org/way/368876696 https://www.openstreetmap.org/way/368876699

I looked at https://josm.openstreetmap.de/query?status=assigned&status=closed&status=needinfo&status=new&status=reopened&keywords=~overlap&order=priority

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-06-13 13:23:27 +0200 (Sat, 13 Jun 2020)
Revision:16613
Build-Date:2020-06-14 01:30:46
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (16613 en_GB) Linux Ubuntu 20.04 LTS
Memory Usage: 274 MB / 976 MB (37 MB allocated, but free)
Java version: 11.0.7+10-post-Ubuntu-3ubuntu1, Ubuntu, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920x1080 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 16x16, 32x32 -> 32x32
Java package: openjdk-11-jre:amd64-11.0.7+10-3ubuntu1
Java ATK Wrapper package: libatk-wrapper-java:all-0.37.1-1
libcommons-logging-java: libcommons-logging-java:all-1.2-2
fonts-noto: fonts-noto:-
Dataset consistency test: No problems found

Plugins:
+ buildings_tools (35474)
+ measurement (35405)
+ reverter (35487)
+ todo (30306)

Last errors/warnings:
- W: javax.net.ssl.SSLException: Read timed out. Cause: java.net.SocketTimeoutException: Read timed out
- W: javax.net.ssl.SSLException: Read timed out. Cause: java.net.SocketTimeoutException: Read timed out

Attachments (3)

19465.osm (27.2 KB ) - added by GerdP 3 years ago.
some test cases
19465-v2.osm (32.2 KB ) - added by GerdP 3 years ago.
more cases
19465-v0.patch (15.6 KB ) - added by GerdP 3 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 by skyper, 3 years ago

Keywords: way area:highway added

The example are two area:highway. See #20031 for this.

comment:2 by mdk, 3 years ago

And I think it should make no difference if areas are defined by a single way or by a multipoygon. Converting closed ways into MPs just for preventing validator warnings sounds not good for me.

comment:3 by GerdP, 3 years ago

+1

comment:4 by GerdP, 3 years ago

@mdk: Do you have an example where it makes a difference?

comment:5 by GerdP, 3 years ago

Ah, found one. One can suppress the informational messages. If e.g. a waterway=river and a landuse=forest share 3 segments you find
Ways share segment with area (1)
If you convert the landuse to a multipolygon and split the way so that no part shares more than 2 segments the message is suppressed.

comment:6 by GerdP, 3 years ago

I think nobody does that to suppress an informational message, but on the other hand the only way to avoid this would be to report all shared segments. Question is why we have the limit of 3 shared segments.

comment:7 by mkoniecz, 3 years ago

"I think nobody does that to suppress an informational message"

I did (or considered) it once, then decided that it stupid and disabled this complaint and created this issue.

Last edited 3 years ago by mkoniecz (previous) (diff)

comment:8 by mkoniecz, 3 years ago

The example are two area:highway. See #20031 for this.

Note that it is far more general, area:highway ended as an example just because it was first thing that I did after reset of JOSM settings. It is happening also with landuse and many other areas.

comment:9 by GerdP, 3 years ago

See #9598. It seems this was really intended to force mappers to use multipolygons. But not as I did in my example. Instead the idea is to split the river and use it for the multipolygon. I think there is no consensus about this mapping style. I'd prefer simple closed ways with shared segments.

comment:10 by GerdP, 3 years ago

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

comment:11 by Klumbumbus, 3 years ago

+1 for removing this warning. I wrongly thought it was alredy gone after #13818.

by GerdP, 3 years ago

Attachment: 19465.osm added

some test cases

comment:12 by GerdP, 3 years ago

I think it is not that easy. See attached file. For each case, the lower way is a highway. Results depend on two lists, one hardcoded, one that can be configured. See also ticket:12540#comment:4

by GerdP, 3 years ago

Attachment: 19465-v2.osm added

more cases

comment:13 by GerdP, 3 years ago

My current thinking is that we should list those types of ways which we never want to overlap, e.g. highway=*, railway=*, and river=*. The current test ignores layer=*, I think that should be changed, too.
I am not yet sure what to do with areas, esp. highway=pedestrian + area=yes.

19465-v2.osm also shows one "Way end node near other highway" which is a false positive from UnconnectedWays. Please ignore it here.

Last edited 3 years ago by GerdP (previous) (diff)

comment:14 by skyper, 3 years ago

-1 for not ignoring layer=* in general. Do not like the idea of having e.g. a river and a highway sharing segments at all and still find lots of landuse=* with negative layer=* in the data.

There are not many highway=* together with area=yes allowed, service, pedestrian and maybe footway and path. I am not a fan of it at all as it mixes line features with area features. Using area:highway all the way would help a lot. In my eyes, they should not share segments but either some common nodes when the highway crosses the area or a extra short open way connecting the two objects.

Last edited 3 years ago by skyper (previous) (diff)

comment:15 by mkoniecz, 3 years ago

" area=yes allowed, service, pedestrian and maybe footway and path"

Also highway=track makes sense in some cases (some logging operation have staging/maneuvering areas)

comment:16 by GerdP, 3 years ago

I don't care much about the different highway types, this can be checked somewhere else. The question is if we want to show a warning when such an area shares segments with another highway that is not an area.

comment:17 by skyper, 3 years ago

Forgot highway=platform and railway=platform.

In general, it depends on the mapping style, again, if and what kinds of highway are allowed to shared segments. Two open ways or closed ways without area=yes should not. Two areas are allowed, and the mixed cases is special. I think everything above residential and unclassified , both included, should not be tagged together with area=yes and therefore as a line object with a width in meters and not centimeters should not share segments with an area object but this discussion is not over, yet.

comment:18 by GerdP, 3 years ago

I think we all agree that a highway should not share any segment with another highway unless one or both are areas. For highways we can create a value list like rest_area, services, platform, + area=yes to filter allowed highway areas.
For railway, I only know railway=platform as an area tag.
I'd like to add special messages for waterway, too. waterway=riverbank is the only area tag I know.

We do allow shared segments with all kinds of those areas like highway=pedestrian,area=yes and waterway=riverbank or railway=platform or any "normal" area like landuse or natural and they should not produce any message, right?

We want to warn when a linear highway,railway, or waterway object shares segment(s) with any other linear object, right?
So also warn when e.g. riverway=stream and natural=tree_row share a segment?
Or only info? What would be the message text? Something like Waterway shares segment with way ?

Do we want messages for areas sharing segments with highway|railway|waterway? Not sure. Maybe only for certain types of areas given in another new list?
If yes, is the number of shared segments relevant? I think no.
Or the length? I think no.

comment:19 by GerdP, 3 years ago

My current plan is to

  • add waterway as special linear type
  • add evaluation of layer so that only objects on the same layer are checked
  • add more code to better distinguish linear ways (e.g. natural=tree_row) and area ways (method hasAreaTags())
  • make it more agressive when two or more linear ways of the same type share segments, e.g. highway=secondary and highway=footway will give an Error instead of a Warning.
  • more specific so that it only produces a Warning when different linear objects overlap, e.g. highway with railway
  • less agressive by adding a new switch like overlapping-ways.only-known-linear that allows to ignore all objects which are not recognized as linear, and this switch will be true by default
  • more agressive when overlapping-ways.only-known-linear is false and areas are glued to linear highway, railway, or waterway as I want to remove the counting of shared segments

This should reduce the noise for normal mappers and will allow those who really want to unglue areas from linear ways to find all cases.

I've not yet decided how members of multipolygons should be treated here. My current thinking is that it makes no difference when a linear way is also member of a multipolygon. A linear highway=footway that is used as member in a multipolygon is still a linear way. So, we can only treat it as area when it is not identified as linear.

Open question: What do we do with ways that are not identified as linear or area type? At the moment this includes all rare tags, typos, and many well known tags with well known lifecycle prefixes, e.g. abandoned:highway

in reply to:  19 comment:20 by skyper, 3 years ago

Wow, sounds like quite some work.
In general, I think, we will profit if less tags are hard-coded and instead have several list in user preferences. Though, I do not know if this is possible.

Replying to GerdP:

My current plan is to

  • add waterway as special linear type

+1

  • add evaluation of layer so that only objects on the same layer are checked

Could that be implemented as user option, please.

  • add more code to better distinguish linear ways (e.g. natural=tree_row) and area ways (method hasAreaTags())
  • make it more agressive when two or more linear ways of the same type share segments, e.g. highway=secondary and highway=footway will give an Error instead of a Warning.
  • more specific so that it only produces a Warning when different linear objects overlap, e.g. highway with railway
  • less agressive by adding a new switch like overlapping-ways.only-known-linear that allows to ignore all objects which are not recognized as linear, and this switch will be true by default
  • more agressive when overlapping-ways.only-known-linear is false and areas are glued to linear highway, railway, or waterway as I want to remove the counting of shared segments

Nice plan, thanks

This should reduce the noise for normal mappers and will allow those who really want to unglue areas from linear ways to find all cases.

+1, thanks.

I've not yet decided how members of multipolygons should be treated here. My current thinking is that it makes no difference when a linear way is also member of a multipolygon. A linear highway=footway that is used as member in a multipolygon is still a linear way. So, we can only treat it as area when it is not identified as linear.

Sorry, I start loosing track. About which warnings are we talking? You example shows "overlapping *ways" and "way shares segment with area". Are there more warnings involved?
There is another test "overlapping areas" in the MapCSS Geometry rules which seems to only check to the inner inside. Maybe #20054 is invalid and I was only surprised that the warning is only raised for areas inside areas.

I would treat MPs as what they are, areas, and their members on their own. In your example, the footway and the MP should still trigger at least a(n) ("others") warning with overlapping-ways.only-known-linear=false.

Open question: What do we do with ways that are not identified as linear or area type? At the moment this includes all rare tags, typos, and many well known tags with well known lifecycle prefixes, e.g. abandoned:highway

Could overlapping-ways.ignored-keys be used for that or is the list to long?

As we will easily get several warnings for one object if more than two objects are overlapping, I think, an update of the validator result list is needed if an involved object is modified. Therefore typos should not be a problem and a feature if no other test finds them.

Last edited 3 years ago by skyper (previous) (diff)

comment:21 by GerdP, 3 years ago

OK for the preference regarding layer.

Sorry, I start losing track.

The current implementation checks if ways are members of multipolygons and treats a linear way as an area if it is member of a multipolygon. That's wrong. I still don't know for sure if there is any good reason to look at member ships...

comment:22 by GerdP, 3 years ago

Could overlapping-ways.ignored-keys be used for that or is the list to long?

As we will easily get several warnings for one object if more than two objects are overlapping, I think, an update of the list is needed if an involved object is modified. Therefore typos should not be a problem and a feature if no other test finds them.

Now you lost me. We may add a new preference list with user defined tags that identify linear objects, but that soon starts to require mapcss capabilities.
There is also a general problem with the list in overlapping-ways.ignored-keys: If a user modifies this list and later JOSM comes with a new default the changes are not merged, the user list is used as is.

in reply to:  22 comment:23 by skyper, 3 years ago

Replying to GerdP:

Could overlapping-ways.ignored-keys be used for that or is the list to long?

As we will easily get several warnings for one object if more than two objects are overlapping, I think, an update of the validator result list is needed if an involved object is modified. Therefore typos should not be a problem and a feature if no other test finds them.

Now you lost me.

Oh, sorry I was writing about two different list without pointing to it. I meant an update of the validator result list in the second sentence.

We may add a new preference list with user defined tags that identify linear objects, but that soon starts to require mapcss capabilities.

That's what I feared. So we stay with hard-coded keys, tags and combinations.

There is also a general problem with the list in overlapping-ways.ignored-keys: If a user modifies this list and later JOSM comes with a new default the changes are not merged, the user list is used as is.

This is a general problem. How is it handled with e.g. tags.direction, tags.discardable, tags.uninteresting, tags.workinprogress, automatic-tag-conflict-resolution.choice, automatic-tag-conflict-resolution.combine or validator.TagChecker.source?

Some notification is needed if the lists have been changed manually and JOSM ships new default values. Resetting manually is not a problem but user might want to make a backup and take a closer look at the changes.

comment:24 by GerdP, 3 years ago

Oh, sorry I was writing about two different list without pointing to it. I meant an update of the validator result list in the second sentence.

That would require a lot of new code. Up to now the list is only updated when an object is deleted -> New ticket.

How is it handled

Not at all as far as I know. The program code sets a default and checks if the user has defined another value. If yes, that value is used. There is no way to ask for previous default values. Maybe a new filter in the "Advanced Preferences" dialog could show all settings with a default value that differs from the user settings -> new ticket already implemented, see #14197

Last edited 3 years ago by GerdP (previous) (diff)

by GerdP, 3 years ago

Attachment: 19465-v0.patch added

in reply to:  24 comment:25 by skyper, 3 years ago

Replying to GerdP:

OK for the preference regarding layer.

Thanks

Sorry, I start losing track.

The current implementation checks if ways are members of multipolygons and treats a linear way as an area if it is member of a multipolygon. That's wrong. I still don't know for sure if there is any good reason to look at member ships...

For overlapping ways MPs are not interesting. They only come into play with share segment with area and if I count any area and not only closed ways as overlap with overlapping-ways.only-known-linear=false but I think share segment with area could be enough.


Replying to GerdP:

Oh, sorry I was writing about two different list without pointing to it. I meant an update of the validator result list in the second sentence.

That would require a lot of new code. Up to now the list is only updated when an object is deleted -> New ticket.

Right, got that wrong. Have to think about it once more and might come up with a new ticket. I am not that convinced anymore, if validator should listen to all actions.

comment:26 by GerdP, 3 years ago

  • OK, reg. multipolygon yes/no: can be used if the way itself has no interesting tags. If such a way shares a segment with another area we want to ignore it

attachment:19465-v0.patch implements

  • new preference overlapping-ways.ignore-layer with default false
  • new preference overlapping-ways.only-known-linear with default true
  • new message texts
  • improved list in method hasAreaTags(). Probably many more candidates could be added.
  • still produces info message Ways share segment when overlapping-ways.only-known-linear is changed to false. I tend to think that this test can be removed. It's kind of a mop up message for ways which are not clearly linear or area.

comment:27 by Don-vip, 3 years ago

Milestone: 20.1120.12

Milestone renamed

comment:28 by GerdP, 3 years ago

Resolution: fixed
Status: assignedclosed

In 17350/josm:

fix #19465: Make "Overlapping ways" less aggressive

  • new preference overlapping-ways.ignore-layer with default false
  • new preference overlapping-ways.only-known-linear with default true
  • new message texts
  • improved list in method hasAreaTags(). Probably many more candidates could be added.
  • still produces info message Ways share segment when overlapping-ways.only-known-linear is changed to false. I tend to think that this test can be removed. It's kind of a mop up message for ways which are not clearly linear or area.
  • fix typo in TestError javadoc

comment:29 by mkoniecz, 3 years ago

improved list in method hasAreaTags(). Probably many more candidates could be added.

I created https://josm.openstreetmap.de/ticket/20147 for that ("add more tags to hasAreaTags (synchronize with StreetComplete)")

comment:30 by GerdP, 3 years ago

Just to make that clear: The method is rarely used so far. Typically the map style is used to decide this.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain GerdP.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.