Modify ↓

Opened 9 days ago

Last modified 4 days ago

#24983 assigned defect

[PATCH] Parallel ways action drops part of way when there is a loop

Reported by: GerdP Owned by: wangi
Priority: normal Milestone:
Component: Core Version: latest
Keywords: template_report Cc: wangi

Description

What steps will reproduce the problem?

  1. Load file
  2. Select all ways
  3. try to draw two parallel lines with a distance of 100m left and right

What is the expected result?

Two Lines near the original data

What happens instead?

The way on the left side is okay, the one on the right side is not.

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

Revision:19665
Is-Local-Build:true
Build-Date:2026-10-25 10:45:18

Identification: JOSM/1.5 (19665 SVN en) Windows 11 64-Bit
OS Build number: Windows 10 Pro 25H2 (26200)
Memory Usage: 1816 MB / 6144 MB (1059 MB allocated, but free)
Java version: 21.0.2+13-LTS, Azul Systems, Inc., OpenJDK 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920x1080x32bpp@60Hz (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: UTF-8
System property sun.jnu.encoding: Cp1252
Locale info: en_DE
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Djpackage.app-version=1.5.19039, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, --add-exports=java.base/sun.security.action=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djpackage.app-path=%UserProfile%\AppData\Local\JOSM\JOSM HWConsole.exe]
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (36493)
+ apache-commons (36483)
+ buildings_tools (36503)
+ comfort0 (36483)
+ ejml (36483)
+ geotools (36483)
+ jackson (36502)
+ jaxb (36483)
+ jts (36483)
+ o5m (36493)
+ opendata (36503)
+ pbf (36493)
+ photo_geotagging (36503)
+ poly (36483)
+ reverter (36503)
+ terracer (36503)
+ undelete (36503)
+ utilsplugin2 (36503)

Tagging presets:
+ c:\josm\core\resources\data\defaultpresets.xml

Map paint styles:
- c:\josm\core\resources\styles\standard\elemstyles.mapcss

Validator rules:
+ d:\java_tools\JOSM\mygeometry.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Rules/GermanySpecific&zip=1
+ c:\josm\core\resources\data\validator\combinations.mapcss
+ c:\josm\core\resources\data\validator\geometry.mapcss

Last errors/warnings:
- 00000.821 W: extended font config - overriding 'filename.Malgun_Gothic=malgun.ttf' with 'MALGUN.TTF'
- 00000.823 W: extended font config - overriding 'filename.Myanmar_Text=mmrtext.ttf' with 'MMRTEXT.TTF'
- 00000.824 W: extended font config - overriding 'filename.Mongolian_Baiti=monbaiti.ttf' with 'MONBAITI.TTF'
- 00001.376 E: java.security.KeyStoreException: Windows-ROOT not found. Cause: java.security.NoSuchAlgorithmException: Windows-ROOT KeyStore not available
- 00001.705 W: Missing plugin main version in plugin mapwithai
- 00003.615 W: Missing plugin main version in plugin mapwithai

Attachments (9)

24983.osm​ (34.5 KB ) - added by GerdP 9 days ago.
test data (members of a route relation)
josm-19481-parallel-ways.png​ (364.1 KB ) - added by wangi 8 days ago.
Example of parallel ways output in old version
josm-19668-parallel-ways.png​ (102.0 KB ) - added by wangi 8 days ago.
Example of parallel ways output in current version
parallel-way-24983.patch​ (41.6 KB ) - added by wangi 8 days ago.
24983-patch-parallel-ways.png​ (127.3 KB ) - added by wangi 8 days ago.
Example of parallel ways output in patch
24983-crop2.osm.xz​ (820 bytes ) - added by wangi 7 days ago.
Cropped test data
24983b.osm.xz​ (67.8 KB ) - added by wangi 7 days ago.
Test data - simplified coastline
24983-tiles-100m-30m.png​ (278.9 KB ) - added by wangi 7 days ago.
Example screenshots: 30 + 100m
24983-tiles-22km-26km.png​ (2.3 MB ) - added by wangi 7 days ago.
Example screenshots: 22 + 26km

Change History (24)

by GerdP, 9 days ago

Attachment: 24983.osm​ added

test data (members of a route relation)

comment:1 by GerdP, 9 days ago

Cc: wangi added
Version: → latest

by wangi, 8 days ago

Example of parallel ways output in old version

comment:2 by wangi, 8 days ago

Owner: changed from team to wangi
Status: new → assigned

by wangi, 8 days ago

Example of parallel ways output in current version

comment:3 by wangi, 8 days ago

Recreated by selecting a way from 24983.osm​, Shift-W to select all non-branching ways and then parallel 30m to left and again to right. On the right it's cut by the loop.

comment:4 by wangi, 8 days ago

Attached is a patch against r19668, with screenshot of the result in JOSM on the attached data (all 12 ways, 30 m to the left and to the right, left result is unselected, right is still selected).

Cause

The road crosses itself: Friesoyther Straße loops round and passes over its own approach on the bridge (way 131319249). Where a path crosses itself, or comes back within twice the offset of itself, the parallel on one side legitimately consists of several separate pieces. At 30 m to the right these are the southern approach, a ring inside the loop and the northern road. The parallel is assembled from such pieces, and only the longest was kept, so the approach and the ring were dropped. The left side lost a piece too: the arc round the outside of the loop. (The loop turns clockwise, so its outside is on the left of the road, even where it lies east of the approach.)

The patch

  • Every piece is kept, longest first, and each becomes its own way(s), split per source way and with the tags copied as before. Pieces which meet at a point share the node there, and a piece which closes on itself becomes a closed way.
  • Exception: pieces made only of retraced segments, i.e. a path which goes out and back along the same nodes (the GPX case from #24891). From such a segment both sides are "its" side, so the offset would draw a loop around the excursion on the far side of the path. Such pieces are dropped, unless nothing else is left.
  • Keeping every piece exposed three weaknesses which keeping only the longest had hidden:
    • The validity of a piece was sampled only at its ends and in its middle, so a long piece which crosses another part of the path in between was kept, and the parallel ran straight across the road. Pieces are now sampled at most r/8 apart; the source segments near a long piece are gathered once for all its samples. Short pieces, which covers every large offset, still get exactly the three samples as before.
    • An arc which survives intact is replaced by its mitre, which sticks out beyond the arc but was never checked against the rest of the path. The mitre is now only used if both of its legs keep the offset distance.
    • Joining pieces end to start compared every chain with every other one and started over after each join, roughly cubic in the number of pieces. Each chain is now extended as far as it goes before moving on.
  • A ring which is open by a small gap is now closed for an open source path too, not only for a closed one. An open path can produce a ring, e.g. around the water of a bay with a narrow mouth which is further than the offset from the shore; where such a ring passes the ends of the path, the round end caps (which trim the result but are never part of it) leave a small gap. A ring needs at least three points.
  • The preview draws every piece. New ParallelWays.getOffsetPaths() returns all of them; getOffsetPoints() and isResultClosed() keep working and refer to the longest.

Result on the attached data, 30 m

  • left: the main line (3,649 m) and the arc round the outside of the loop (298 m)
  • right: the northern road (2,375 m), the southern approach (1,041 m) and the ring inside the loop (98 m)

Very small pieces are kept as well. West of the bridge there is a speck of ground further than 30 m from three parts of the road (the approach, the westward road and its return leg); its outline gives a 5 m piece on the left and a 28 m ring on the right.

Performance

On a coastline of 40,656 nodes and 2,322 km, at 30 m, 100 m, 1 km, 3 nm and 12 nm on both sides, the timings are the same as before (within a few percent). On a random path of 20,000 nodes which crosses itself thousands of times, an offset of 50 takes 1.5 s instead of 31.7 s (the joining of pieces); at 5 it takes 2.5 s instead of 1.2 s, since such a path now gives over 3,000 pieces instead of one.

Unit tests

  • testSelfCrossingLoop: a road looping round and passing under its own approach, both sides: the exact pieces, and none of them comes closer to the road than the offset anywhere along its segments (not only at its vertices).
  • testCommitSelfCrossingLoop: the pieces become separate ways, the ring a closed way.
  • testBayPocketRing: a bay with a narrow mouth gives a closed ring although the source path is open.
  • testRetracedOnly: a path which only goes out and back still has a parallel.
  • testRetracedExcursion now also checks that the far side stays a single piece.
  • testLargeOffsetRandomCoastline now checks every piece, along its segments, and that none is degenerate.

Each change makes at least one of these fail when it is reverted. All tests in org.openstreetmap.josm.actions.mapmode pass, and checkstyle and PMD are clean.

by wangi, 8 days ago

Attachment: parallel-way-24983.patch​ added

comment:5 by wangi, 8 days ago

Summary: parallel ways action drops part of way when there is a loop → [PATCH] Parallel ways action drops part of way when there is a loop

comment:6 by wangi, 8 days ago

I've also created #24985 for feedback on a potential new create buffer action. While using the patch here left then right will result in something which is close visually to a buffer, it would need the discrete bits joined together, along with end caps...

comment:7 by GerdP, 8 days ago

Not sure if I like this more, but I must confess that I don't know what I expect for a special case like this. Having parts of the parallel way on the opposite site is confusing, so maybe this should produce a notification and maybe add a preference about it?
FYI: I'll have no time for more testing until Saturday.

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

by wangi, 8 days ago

Example of parallel ways output in patch

comment:8 by stoecker, 8 days ago

The description with these many pieces sounds troublesome and probably will cause bad data. Maybe such an action rather goes into utilsplugin and not into core?

comment:9 by wangi, 8 days ago

I forgot the screenshot attachment last night, illustrating the left/right output.

"screenshot of the result in JOSM on the attached data (all 12 ways, 30 m to the left and to the right, left result is unselected, right is still selected)."
Example of parallel ways output in patch

I suspect what you'd want is equivalent to parallel r to the left and then taking it and parallel 2r to the right.

in reply to:  8 ; comment:10 by wangi, 8 days ago

Replying to stoecker:

The description with these many pieces sounds troublesome and probably will cause bad data. Maybe such an action rather goes into utilsplugin and not into core?

I'll prepare a panel of screenshots of the past behaviour, current behaviour, the patches keep all, and another "smoothed" (really r<;2r> in the implementation) for the attached test data along with another test dataset I've been using (some OGF coastline). It will show some of the desirable behaviours the patch introduces, along with the less so. I do think leaving the behaviour as-is is a plausible resolution, especially if #24985 can implement something to address a specific use case behind this.

by wangi, 7 days ago

Attachment: 24983-crop2.osm.xz​ added

Cropped test data

by wangi, 7 days ago

Attachment: 24983b.osm.xz​ added

Test data - simplified coastline

by wangi, 7 days ago

Attachment: 24983-tiles-100m-30m.png​ added

Example screenshots: 30 + 100m

by wangi, 7 days ago

Attachment: 24983-tiles-22km-26km.png​ added

Example screenshots: 22 + 26km

in reply to:  10 comment:11 by wangi, 7 days ago

Replying to wangi:

I'll prepare a panel of screenshots of the past behaviour, current behaviour, the patches keep all,

Tested 19613 vs latest 19668 vs patch; at 100 and 30m offsets:
Example screenshots: 30 + 100m

Same versions; against the 24983b.osm.xz simplified section of OGF coastline; at 22km and 26km offsets:
Example screenshots: 22 + 26km

in reply to:  9 ; comment:12 by GerdP, 5 days ago

Replying to wangi:

I suspect what you'd want is equivalent to parallel r to the left and then taking it and parallel 2r to the right.

The old code (tested) always produced one line string, so I am used to that, but there is probably no 100% correct result for these edge cases, so the patch fixes at least the problem that major parts are completely missing as in latest now. So, I think the patch should be applied and then we'll wait for user feedback.

in reply to:  12 comment:13 by stoecker, 4 days ago

Replying to GerdP:

the patch fixes at least the problem that major parts are completely missing as in latest now. So, I think the patch should be applied and then we'll wait for user feedback.

As said I fear that patch will result in bad data. Nobody expects that creating "a" parallel way can create more than one way. So that will lead to strange artifacts.

comment:14 by GerdP, 4 days ago

So, what do you propose? Revert the change r19624?

comment:15 by stoecker, 4 days ago

No. r19624 solves a lot of issues with this action. It leaves cases which are not satisfactory, which is in the nature of that action. The proposed patch doesn't improve the situation in my eyes. Maybe there is another solution which can handle the left out part after overlaps a bit better keeping a single way as solution, but if not I think the current actions is better than the previous state.

Modify Ticket

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