Modify

Opened 5 weeks ago

Closed 2 weeks ago

Last modified 8 days ago

#19822 closed enhancement (fixed)

[Patch] Inconsistent behavior with GeoJSON multipolygons

Reported by: machyna@… Owned by: team
Priority: normal Milestone: 20.10
Component: Core geojson Version: latest
Keywords: GeoJSON, multipolygons Cc:

Description (last modified by Don-vip)

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>
  1. Save as geoJSON file
  2. 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)

Attachments (3)

19822.patch (2.8 KB) - added by GerdP 3 weeks ago.
quick hack to replace untagged multipolygon member by tagged duplicate
GeoJSON_multipolygons.zip (7.4 KB) - added by machyna@… 13 days ago.
test files
problem3.geojson (7.0 KB) - added by machyna@… 9 days ago.
two relations on one polygon

Download all attachments as: .zip

Change History (27)

comment:1 Changed 4 weeks ago by 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

comment:2 Changed 4 weeks ago by machyna@…

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

comment:3 Changed 3 weeks ago by Don-vip

Description: modified (diff)

comment:4 Changed 3 weeks ago by 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?

Changed 3 weeks ago by GerdP

Attachment: 19822.patch added

quick hack to replace untagged multipolygon member by tagged duplicate

comment:5 Changed 3 weeks ago by machyna@…

Resolution: fixed
Status: newclosed

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.

comment:6 Changed 3 weeks ago by GerdP

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

comment:7 Changed 3 weeks ago by machyna@…

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

comment:8 Changed 3 weeks ago by GerdP

Milestone: 20.10
Resolution: fixed
Status: closedreopened
Summary: Inconsistent behavior with GeoJSON multipolygons[Patch] Inconsistent behavior with GeoJSON multipolygons

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.

comment:9 Changed 3 weeks ago by machyna@…

Will do! Sorry for the confusion.

comment:10 Changed 2 weeks ago by GerdP

Priority: majornormal
Type: defectenhancement

comment:11 Changed 2 weeks ago by machyna@…

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

comment:12 Changed 2 weeks ago by GerdP

Resolution: fixed
Status: reopenedclosed

In 17107/josm:

fix #19822: Inconsistent behavior with GeoJSON multipolygons

  • replace untagged multipolygon member by tagged duplicate

comment:13 in reply to:  11 Changed 2 weeks ago by 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/

comment:14 Changed 13 days ago by machyna@…

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.

comment:15 Changed 13 days ago by GerdP

Please attach a sample geojson file.

comment:16 Changed 13 days ago by GerdP

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

Changed 13 days ago by machyna@…

Attachment: GeoJSON_multipolygons.zip added

test files

comment:17 Changed 13 days ago by machyna@…

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.

comment:18 Changed 13 days ago by 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

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

comment:20 Changed 13 days ago by machyna@…

Tested on 17124 and all works good.
Thanks!

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

comment:22 in reply to:  20 Changed 12 days ago by GerdP

Replying to machyna@…:

Tested on 17124 and all works good.
Thanks!

Thanks for your help!

Changed 9 days ago by machyna@…

Attachment: problem3.geojson added

two relations on one polygon

comment:23 Changed 9 days ago by machyna@…

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

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.