[Patch] Inconsistent behavior with GeoJSON multipolygons

[Mashin]

What steps will reproduce the problem?

Load this .osm file in JOSM:

<?xml version='1.0' encoding='UTF-8'?>
<osm version='0.6' upload='false' generator='JOSM'>
  <node id='-2596285' lat='41.52291460368' lon='-73.46584481969' />
  <node id='-2596286' lat='41.52291328204' lon='-73.46546705281' />
  <node id='-2596287' lat='41.52275043969' lon='-73.46546806918' />
  <node id='-2596288' lat='41.52275176133' lon='-73.46584583606' />
  <node id='-2596289' lat='41.5228742736' lon='-73.46577875582' />
  <node id='-2596290' lat='41.52287126667' lon='-73.46556991097' />
  <node id='-2596291' lat='41.52279212833' lon='-73.46557194371' />
  <node id='-2596292' lat='41.52279513526' lon='-73.46578078856' />
  <way id='-132026' action='modify'>
    <nd ref='-2596285' />
    <nd ref='-2596286' />
    <nd ref='-2596287' />
    <nd ref='-2596288' />
    <nd ref='-2596285' />
    <tag k='name' v='part_outer' />
  </way>
  <way id='-132027' action='modify'>
    <nd ref='-2596289' />
    <nd ref='-2596290' />
    <nd ref='-2596291' />
    <nd ref='-2596292' />
    <nd ref='-2596289' />
    <tag k='name' v='part_inner' />
  </way>
  <relation id='-99881'>
    <member type='way' ref='-132026' role='outer' />
    <member type='way' ref='-132027' role='inner' />
    <tag k='name' v='part_multipolygon' />
    <tag k='type' v='multipolygon' />
  </relation>
</osm>

2. Save as geoJSON file
3. Load the geoJSON file

What is the expected result?

I would expect that when I am loading the geoJSON file, I would get the same data representation as in case of the .osm file. That means only TWO polygons ("part_outer", "part_inner") and they both would be part of a multipolygon relation.

What happens instead?

I get FOUR objects: TWO polygons ("part_outer", "part_inner") and TWO duplicate polygons that are bound in multipolygon relation.

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

I am no expert in this and GeoJSON definition is not very specific how the object should be represented. (What if I actually want to have duplicated polygons on top of each other? How would GeoJSON code differ from this case?) But I think that since OSM default is to reuse objects rather than having multiple copies at the same place, and JOSM validator flags such duplicated cases as warning, the default should be to merge duplicate features if possible.

Btw. GeoJSON specification allows in such simple cases (one outer ring and one or more holes) to use feature type "Polygon", while JOSM exports them all as "MultiPolygon". No big deal just wanted to mention that this duplication happens in both of these cases.

If this is indeed an erroneous behavior would it please be possible to make the fix into the earliest JOSM release? This is really complicating imports where one has to tediously fix these before upload.

Thank you!

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-09-06 16:54:59 +0200 (Sun, 06 Sep 2020)
Build-Date:2020-09-07 01:30:48
Revision:17013
Relative:URL: ^/trunk

Identification: JOSM/1.5 (17013 en) Mac OS X 10.15.6
OS Build number: Mac OS X 10.15.6 (19G2021)
Memory Usage: 829 MB / 1820 MB (624 MB allocated, but free)
Java version: 1.8.0_261-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.formdev.flatlaf.FlatDarkLaf
Screen: Display 69733632 1440x900 (scaling 1.0x1.0)
Maximum Screen Size: 1440x900
Best cursor sizes: 16x16 -> 16x16, 32x32 -> 32x32
VM arguments: [-Djava.security.policy=file:<java.home>/lib/security/javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>/bin, -Djava.security.manager, -Djnlpx.origFilenameArg=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/56/1ee8cfb8-2bc3e844, -Djnlpx.remove=false, -Dsun.awt.warmup=true, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=NULL,2048m, -Dmacosx.jnlpx.dock.name=JOSM, -Dmacosx.jnlpx.dock.icon=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/25/4c122699-4466e0ba.icns, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp , -Djnlpx.jvm="<java.home>/bin/java"]

Plugins:
+ FastDraw (35499)
+ MicrosoftStreetside (35248)
+ PicLayer (35405)
+ apache-commons (35524)
+ apache-http (35092)
+ buildings_tools (35500)
+ changeset-viewer (22)
+ editgpx (35248)
+ ejml (35313)
+ flatlaf (35541)
+ geotools (35169)
+ javafx-osx (35458)
+ jaxb (35092)
+ jna (35092)
+ jts (35122)
+ log4j (35092)
+ merge-overlap (35248)
+ opendata (35513)
+ poly (35248)
+ reltoolbox (35529)
+ reverter (35499)
+ terracer (35499)
+ utilsplugin2 (35487)

Tagging presets:
+ https://josm.openstreetmap.de/josmfile?page=Presets/Freemap&preset&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Golf_Course&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/hiking_routes_with_trail_marking&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/SentieriItaliani&zip=1

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/LessObtrusiveNodes&style&zip=1
- https://github.com/simon04/coloured-addresses.mapcss/raw/master/dist/coloured-addresses.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/ParkingLanes&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_features_ryg&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_features&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Cycleways&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Sidewalks&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/LitObjects&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lit&style&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Osmc&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/SlovakiaBicycleRoutes&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&style&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
- https://github.com/TelenavMapping/Community_MapRoulette/blob/master/maxspeed.mapcss
+ ${HOME}/Documents/Results/Scripts/JOSM_style/AreaHighway.mapcss

Last errors/warnings:
- 00007.047 W: Not a single layer for the name 'MapBox Satellite': []
- 00007.047 W: Not a single layer for the name 'MapBox Satellite': []
- 00007.939 W: Failed to parse Mappaint styles from 'https://github.com/TelenavMapping/Community_MapRoulette/blob/master/maxspeed.mapcss'. Error was: Encountered " "<" "< "" at line 6, column 1.
- 00007.941 E: org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.ParseException: Encountered " "<" "< "" at line 6, column 1.
- 00009.522 W: Cannot lock cache directory. Will not use disk cache
- 00013.153 W: Cannot start IPv4 remotecontrol server on port 8111: Address already in use (Bind failed)
- 00013.154 W: Cannot start IPv6 remotecontrol server on port 8111: Address already in use (Bind failed)

[GerdP]

The result of the export depends on the expert preference geojson.export.untagged-closed-is-polygon. I think you get what you expect when you set it to true.
See also #18902

[Mashin]

Oh thanks, that's useful to know.
Is there any chance this import of multipolygons duplication will be fixed? I would assume that the fix would be similar to (#19041).

[GerdP]

Looked again into this.
I was wrong in comment:1, the closed ways are tagged, so the setting has no influence.

If you try your example on ​http://tyrasd.github.io/osmtogeojson/ you'll recognize that this tool also creates 4 ways.
I don't think that it is possible to convert between OSM and geojson and back without losing information.

If I got you right you suggest to merge those pairs of ways which are flagged by validator as "ways with the same position" into one on import?

[GerdP]

quick hack to replace untagged multipolygon member by tagged duplicate

[Mashin]

Thank you so much for fixing this! Can't wait when it goes live.

I agree, there doesn't seem to be a way how to represent those objects in GeoJSON without creating separate multipolygon and linestrings/polygons.
On other hand the conversion GeoJSON -> OSM is tricky, because OSM format reuses objects through their IDs. JOSM then has to guess, which overlapping objects to merge and which are to be kept separate.

[GerdP]

Does that mean you tested the patch and it works well for you?

[Mashin]

uff.. sorry how do I test? I am not familiar with Java.

[GerdP]

OK, I'll commit the patch after the next release of milestone 20.09.
Maybe I'll add another expert option to disable this patch.
Once this was committed please try the next "latest" version.

[Mashin]

Will do! Sorry for the confusion.

[Mashin]

Has this been committed? I tried build 17092 and duplication is still there.

[GerdP]

In 17107/josm:

fix #19822: Inconsistent behavior with GeoJSON multipolygons

• replace untagged multipolygon member by tagged duplicate

[GerdP]

Replying to machyna@…:

Has this been committed? I tried build 17092 and duplication is still there.

It is now with r17107. I wanted to wait because several unit tests are failing with the recent changes since r17084.
https://josm.openstreetmap.de/jenkins/job/JOSM/lastCompletedBuild/testReport/

[Mashin]

I just tested 17112 and still the same :(
I don't know if this is helpful, but the duplicated ways always run in opposite directions (one clockwise, the other counterclockwise) and maybe this is what causes the identity check to fail?
According to GeoJSON specification all polygons should run counterclockwise and their holes clockwise. So if a hole has some tags applied in GeoJSON this becomes two objects: 1) counterclockwise polygon with tags, 2) clockwise hole without tags.

[GerdP]

Please attach a sample geojson file.

[GerdP]

BTW: Where do you get this r17112? I thought you cannot compile on your own?

[Mashin]

test files

[Mashin]

I found it under the Build tab... plus some more levels deeper https://josm.openstreetmap.de/jenkins/job/JOSM/jdk=JDK11/
(I use the MacOS build)

I think I got it. It is fixed and works when the GeoJSON object (either multipart Polygon or MultiPolygon) has also tag "type=multipolygon", but not when it is missing.

The question is then if JOSM should automatically anticipate that those are multipolygons and fill in the tag. My opinion is 'yes', because that is what the objects essentially represent.

[GerdP]

In 17115/josm:

see #19822: Inconsistent behavior with GeoJSON multipolygons
Move code line that adds tag type=multipolygon after fillTagsFromFeature(). With the old order the tag was removed by fillTagsFromFeature()
Problem existed since r15424

[GerdP]

The question is then if JOSM should automatically anticipate that those are multipolygons and fill in the tag. My opinion is 'yes', because that is what the objects essentially represent.

Thanks for the hint. It was always meant to work like that, but it seems that nobody complained so far.

[Mashin]

Tested on 17124 and all works good.
Thanks!

[GerdP]

In 17131/josm:

see #19822: Inconsistent behavior with GeoJSON multipolygons

• fix another place were fillTagsFromFeature() was called after a tag was added to the OSM primitive
• add javadoc for the rather confusing behaviour of fillTagsFromFeature()
• add unit test

[GerdP]

Replying to machyna@…:

Tested on 17124 and all works good.
Thanks!

Thanks for your help!

[Mashin]

two relations on one polygon

[Mashin]

Sorry, I just found another spacial case when the duplication happens. That is when one polygon is part of two(or more) multipolygons. e.g. serves as inner in relation #1, but as a outer for relation #2

[GerdP]

In 17185/josm:

fix #19822: Inconsistent behavior with GeoJSON multipolygons
Handle care when one polygon is part of two(or more) multipolygons. e.g. serves as inner in relation A, but as a outer for relation B

[Don-vip]

Milestone renamed