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?
- Load file
- Select all ways
- 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)
Change History (24)
by , 9 days ago
comment:1 by , 9 days ago
| Cc: | added |
|---|---|
| Version: | → latest |
by , 8 days ago
| Attachment: | josm-19481-parallel-ways.png added |
|---|
Example of parallel ways output in old version
comment:2 by , 8 days ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
by , 8 days ago
| Attachment: | josm-19668-parallel-ways.png added |
|---|
Example of parallel ways output in current version
comment:3 by , 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 , 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()andisResultClosed()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.testRetracedExcursionnow also checks that the far side stays a single piece.testLargeOffsetRandomCoastlinenow 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 , 8 days ago
| Attachment: | parallel-way-24983.patch added |
|---|
comment:5 by , 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 , 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 , 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.
by , 8 days ago
| Attachment: | 24983-patch-parallel-ways.png added |
|---|
Example of parallel ways output in patch
follow-up: 10 comment:8 by , 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?
follow-up: 12 comment:9 by , 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)."

I suspect what you'd want is equivalent to parallel r to the left and then taking it and parallel 2r to the right.
follow-up: 11 comment:10 by , 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.
comment:11 by , 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:

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

follow-up: 13 comment:12 by , 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.
comment:13 by , 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:15 by , 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.



test data (members of a route relation)