Modify

Opened 6 weeks ago

Closed 4 weeks ago

Last modified 3 weeks ago

#20253 closed defect (fixed)

Weird area shading at a corner

Reported by: richlv Owned by: GerdP
Priority: minor Milestone: 21.01
Component: Core mappaint Version: tested
Keywords: template_report regression Cc: michael2402

Description

What steps will reproduce the problem?

  1. Load the attached corner.osm
  2. Zoom in on the lower half of the buiding
  3. Observe the weird sharp angle as per corner.png
URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-11-21 14:56:29 +0100 (Sat, 21 Nov 2020)
Build-Date:2020-11-22 02:30:52
Revision:17329
Relative:URL: ^/trunk

Identification: JOSM/1.5 (17329 en_GB) Mac OS X 10.15.7
OS Build number: Mac OS X 10.15.7 (19H114)
Memory Usage: 1302 MB / 1820 MB (909 MB allocated, but free)
Java version: 1.8.0_271-b09, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.apple.laf.AquaLookAndFeel
Screen: Display 1127230987 1920x1080 (scaling 1.0x1.0), Display 69733382 1680x1050 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 16x16, 32x32 -> 32x32
VM arguments: [-Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlp.tk=awt, -Djnlpx.jvm=<java.home>/bin/java, -Djnlpx.splashport=-1, -Djnlpx.home=<java.home>/bin, -Djnlpx.remove=false, -Djnlpx.offline=false, -Djnlpx.relaunch=true, -Djnlpx.session.data=/var/folders/nl/flqxqsmj5q963r7tcnfrdt3c0000gn/T/session4261687983741410212, -Djnlpx.heapsize=-1,2147483648, -Djava.security.policy=file:<java.home>/lib/security/javaws.policy, -DtrustProxy=true, -Djnlpx.origFilenameArg=/Users/richlv/Library/Application Support/Oracle/Java/Deployment/cache/6.0/56/1ee8cfb8-72e8e992, -Dsun.awt.warmup=true, -Djava.security.manager]
Dataset consistency test: No problems found

Plugins:
+ InfoMode (35543)
+ Mapillary (1.5.27)
+ PicLayer (2a9aa7a)
+ apache-commons (35524)
+ apache-http (35589)
+ ejml (35458)
+ geotools (35458)
+ imagery_offset_db (35640)
+ javafx-osx (35458)
+ jaxb (35543)
+ jna (35458)
+ jts (35458)
+ measurement (35640)
+ opendata (35640)
+ pbf
+ photo_geotagging (35640)
+ reverter (35640)
+ utilsplugin2 (35640)

Map paint styles:
- /Users/richlv/Desktop/ChangeFontSize.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1

Last errors/warnings:
- 00007.636 W: Failed to load Mappaint styles from '/Users/richlv/Desktop/ChangeFontSize.mapcss'. Exception was: java.nio.file.NoSuchFileException: /Users/richlv/Desktop/ChangeFontSize.mapcss
- 00007.637 E: java.nio.file.NoSuchFileException: /Users/richlv/Desktop/ChangeFontSize.mapcss
- 00009.852 W: Warning - <html>Plug-in pbf requires JOSM version 17334. The current JOSM version is 17329.<br>You have to update JOSM in order to use this plug-in.</html>

Attachments (3)

corner.osm (1.6 KB) - added by richlv 6 weeks ago.
Testcase
corner.png (43.0 KB) - added by richlv 6 weeks ago.
Screenshot
20253.patch (770 bytes) - added by GerdP 4 weeks ago.

Download all attachments as: .zip

Change History (19)

Changed 6 weeks ago by richlv

Attachment: corner.osm added

Testcase

Changed 6 weeks ago by richlv

Attachment: corner.png added

Screenshot

comment:1 Changed 6 weeks ago by GerdP

It happens with the start/end node.

comment:2 Changed 4 weeks ago by GerdP

Owner: changed from team to GerdP
Status: newassigned

Changed 4 weeks ago by GerdP

Attachment: 20253.patch added

comment:3 Changed 4 weeks ago by GerdP

Milestone: 21.01

The patch fixes the problem.

comment:4 Changed 4 weeks ago by GerdP

Cc: michael2402 added

comment:5 Changed 4 weeks ago by GerdP

Keywords: regression added

Probably a regression of r11748, snapshot for r11747 is OK, r11757 is not. In r11748 the closePath() call was removed.

comment:6 Changed 4 weeks ago by michael2402

The patch might work - It does not contian much source and trac does not show the whole file - I try to look into it tomorrow, what the logic for 'first'/'connect' was.

comment:7 Changed 4 weeks ago by GerdP

Any results? I'd also be happy if we always call closePath()

comment:8 Changed 4 weeks ago by michael2402

I think you can make it much simplier:

    private void appendWay(Iterable<? extends ILatLon> nodes, boolean connect, boolean close) {
        boolean useMoveTo = !connect;
        for (ILatLon n : nodes) {
            if (useMoveTo) {
                moveTo(n);
            } else {
                lineTo(n);
            }
            useMoveTo = false;
        }
        if (close) {
            closePath();
        }
    }

But this is just the first half of the problem. We start to re-get this problem when using the ClampingPathVisitor.

In this case, handling those corners is much more difficult. We cannot use close() here, because we won't close to the last gap in the path.

comment:9 Changed 4 weeks ago by GerdP

Do you have an example that shows what goes wrong?

comment:10 Changed 4 weeks ago by michael2402

The Problem is here:
https://github.com/openstreetmap/josm/blob/e9ac48f5afba8ce80e9baab486986522e5fa4636/src/org/openstreetmap/josm/gui/draw/MapViewPath.java#L374

(1) OffsetPathVisitor
When using a line with an offset, the close() changed to a moveTo again
(2) ClampingPathVisitor
Same problem here. This is used for big paths (e.g. a very long road, big building) of which only parts are displayed in the UI.

comment:11 in reply to:  10 ; Changed 4 weeks ago by GerdP

Replying to michael2402:

The Problem is here:
https://github.com/openstreetmap/josm/blob/e9ac48f5afba8ce80e9baab486986522e5fa4636/src/org/openstreetmap/josm/gui/draw/MapViewPath.java#L374

Yes, I found this source also suspicious, but I've not yet found a case where it is executed when your patch is applied. Maybe I need a special style or setting?

comment:12 in reply to:  11 Changed 4 weeks ago by michael2402

Replying to GerdP:

Yes, I found this source also suspicious, but I've not yet found a case where it is executed when your patch is applied. Maybe I need a special style or setting?

I can't test it, since JOSM won't compile for me and I would have to debug through ANT to get it working again (and I don't have an IDE set up, so debugging this is even more difficult).

I just remember that this was the reason why I used this lineTo-Hack and not close - but the offset code has changed since then, it might not be an issue any more.

comment:13 Changed 4 weeks ago by GerdP

OK, I'll commit your version and we'll see if it produces any new issues. I didn't see any while panning or zooming.

comment:14 Changed 4 weeks ago by GerdP

Resolution: fixed
Status: assignedclosed

In 17432/josm:

fix #20253: Weird area shading at a corner

  • use closePath() instead of lineTo(first) (Patch by michael2402)

comment:15 Changed 3 weeks ago by Klumbumbus

I didn't test the new behavior yet. Just in case someone is interested, #12104 contains some special geometries which could produce weird results.

comment:16 Changed 3 weeks ago by GerdP

I tried the example file and I think it all looks good, at least with the default settings.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain GerdP.
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.