Modify

Opened 5 years ago

Last modified 4 years ago

#18019 assigned enhancement

[Patch] Creating a multipolygon is wrongly moving highway=* to the relation

Reported by: naoliv Owned by: GerdP
Priority: normal Milestone:
Component: Core Version:
Keywords: multipolygon create move tags Cc:

Description

If I am not wrong, I remember that this didn't happen before.

Open the attached file, select all the ways, Ctrl+B to create a multipolygon and see how the highway=residential tag was moved from the ways to the relation.

JOSM:

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2019-08-07 02:39:36 +0200 (Wed, 07 Aug 2019)
Revision:15288
Build-Date:2019-08-07 01:30:53
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (15288 pt_BR) Linux Debian GNU/Linux bullseye/sid
Memory Usage: 1139 MB / 2048 MB (268 MB allocated, but free)
Java version: 13-ea+30-Debian-1, Debian, OpenJDK 64-Bit Server VM
Screen: :0.0 1920x1080
Maximum Screen Size: 1920x1080
Java ATK Wrapper package: libatk-wrapper-java:all-0.35.0-2
libcommons-compress-java: libcommons-compress-java:all-1.18-2
libcommons-logging-java: libcommons-logging-java:all-1.2-2
VM arguments: [-Dawt.useSystemAAFontSettings=gasp]
Dataset consistency test: No problems found

Plugins:
+ EasyPresets (1537621333)
+ FastDraw (34977)
+ OpeningHoursEditor (34977)
+ SimplifyArea (34977)
+ apache-commons (34908)
+ buildings_tools (34982)
+ ejml (35049)
+ geojson (124)
+ geotools (34908)
+ http2 (35062)
+ jaxb (35014)
+ jogl (1.2.3)
+ jts (35064)
+ log4j (34908)
+ measurement (35051)
+ opendata (34997)
+ reverter (34999)
+ tageditor (34977)
+ tagging-preset-tester (34908)
+ todo (30306)
+ turnlanes-tagging (281)
+ turnrestrictions (34977)
+ undelete (34977)
+ utilsplugin2 (34977)
+ wikipedia (v1.1.1)

Attachments (2)

example.osm (1.1 KB ) - added by naoliv 5 years ago.
18019.patch (1.8 KB ) - added by GerdP 5 years ago.
Possible solution

Download all attachments as: .zip

Change History (12)

by naoliv, 5 years ago

Attachment: example.osm added

comment:1 by Don-vip, 5 years ago

Cc: GerdP added
Keywords: multipolygon added

comment:2 by GerdP, 5 years ago

Yes, that looks wrong. Do you have an idea about a version that worked differently? I just tried my oldest binary (r9086) and it shows the same problem.

comment:3 by GerdP, 5 years ago

I just found out that the "Create Multipolygon" button of plugin reltoolbox doesn't show this problem, so maybe you confused them, as I cannot find any hint in the source history of core method CreateMultipolygonAction that it ever worked well in this case unless you add "highway" to this list in advanced preference:

multipoly.lineartagstokeep	[barrier, fence_type, source]

Does that ring a bell?

comment:4 by naoliv, 5 years ago

Type: defectenhancement

Or I am remembering too old stuff (#9623; testing with r7000 it still didn't move highway to the relation) or, coincidentally, this is the first time that I am using all ways with the same highway value since that time.

Maybe there isn't an easy and fast solution that doesn't involve working with the code.

For example, I could include highway in multipoly.lineartagstokeep but when creating a multipolygon from a highway=services|highway=rest_area area, I guess that it would wrongly keep the value in the ways instead moving it to the relation.

comment:5 by GerdP, 5 years ago

Yes, that was also my thought. I'll have a closer look at the code in reltoolbox. Maybe it is time again to merge it into core.

by GerdP, 5 years ago

Attachment: 18019.patch added

Possible solution

comment:6 by GerdP, 5 years ago

Seems we were both mistaken. The reltoolbox plugin behaves like the core method. I was confused because it selects the new MP while the core method keeps the selection. I also don't find a difference in r7000 (tested with r6999) from the download page. Maybe you tried with added "highway" in multipoly.lineartagstokeep?
So, my only idea so far is to use method ElemStyles.getAreaElemStyle() to test if the copied tags are rendered as an area.
The attached patch implements this and also creates a warning popup.
Problem: The result depends on the active Mappaint style(s).

comment:7 by skyper, 4 years ago

Keywords: create added
Summary: Creating a multipolygon is wrongly moving highway=* to the relation[Patch] Creating a multipolygon is wrongly moving highway=* to the relation

The patch was never applied and JOSM still does not find the right tags to move. With some highway=* it moved all tags without conflicting value. Additionally, if I cancel the upcoming relation editor the relation is already created where I expected, at least, a dialog to undo the creation or even a silent undo/cancel.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-07-26 09:33:11 +0200 (Sun, 26 Jul 2020)
Revision:16809
Build-Date:2020-07-27 01:30:49
URL:https://josm.openstreetmap.de/svn/trunk

Not sure how to handle this in general but for highway=* we have strict rules for values allowed on areas. Why not use them?

comment:8 by skyper, 4 years ago

Keywords: move tags added

comment:9 by skyper, 4 years ago

Oh, and moving only some tags is not acceptable as they all belong together, see #19594, and in my test lanes=* were left as only tag on the ways caused by conflicting values.

#19594 is probably a duplicate but, in my eyes, this is a defect and not a enhancement.

comment:10 by GerdP, 4 years ago

Cc: GerdP removed
Owner: changed from team to GerdP
Status: newassigned

Modify Ticket

Change Properties
Set your email in Preferences
Action
as assigned The owner will remain GerdP.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from GerdP to the specified user. Next status will be 'new'.
Next status will be 'needinfo'. The owner will be changed from GerdP to naoliv.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. 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.