[PATCH] Wrongly rendered cursors on HiDPI screen

[johsin18]

What steps will reproduce the problem?

1. Start JOSM on a HiDPI screen (200% Windows scaling)
2. New Layer
3. Hover mouse over data layer and press Shift key (showing mouse cursor for "add to selection")

What is the expected result?

Mouse cursor with a dashed rectangle close to it, containing a little plus sign. Mouse arrow and rectangle have approximately the same size.

What happens instead?

Mouse cursor with a dashed rectangle quite distant to it, containing a very tiny plus sign (very hard to see). The mouse arrow is larger than the rectangle.

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

See attached screenshots for both cases (imagine the hi DPI cursor scaled by half). The behavior for other cursors with overlay is similar.
Disregard the few faulty pixel on the left in the high DPI mouse arrow. They are caused by the Windows-built-in Magnifier tool somehow.

I would like to fix this myself. However, I would need some explanations on the existing code.
Deep down in the cursor construction I get via

GuiSizesHelper.getSizeDpiAdjusted()

to

GuiSizesHelper.getScreenDPI()

which claims to return the "screen resolution in DPI".
However, unless for the unlikely case that the user sets "gui.scale" in the settings manually, it will always return 96, regardless of the actual screen resolution (and/or scaling), will it not?
Is getScreenDPI() buggy or do I misinterpret its function?

Revision:15592
Is-Local-Build:true
Build-Date:2020-02-09 18:52:27

Identification: JOSM/1.5 (15592 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1909 (18363)
Memory Usage: 290 MB / 2028 MB (127 MB allocated, but free)
Java version: 15-ea+6-123, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: \Display0 3200x1800
Maximum Screen Size: 3200x1800
VM arguments: [-Djosm.home=<josm.pref>, -Didea.launcher.port=57345, -Didea.launcher.bin.path=C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2019.3\bin, -Dfile.encoding=UTF-8]
Program arguments: [--reset-preferences]
Dataset consistency test: No problems found

Last errors/warnings:
- W: Replacing existing preference file '<josm.pref>\preferences.xml' with default preference file.
- W: extended font config - overriding 'filename.Myanmar_Text=mmrtext.ttf' with 'MMRTEXT.TTF'
- W: extended font config - overriding 'filename.Mongolian_Baiti=monbaiti.ttf' with 'MONBAITI.TTF'
- W: No configuration settings found.  Using hardcoded default values for all pools.

[johsin18]

correct lo DPI cursor

[johsin18]

faulty hi DPI cursor

[johsin18]

HiDPI cursor after fix.

[johsin18]

I finally found some time to finish a patch, which also fixes #13173.

As IMHO the changes are better reviewed commit by commit, I pushed it to a git repo:
​https://github.com/johsin18/josm/pull/1
Feel free to comment there.
I can also provide a monolithic patch if desired.

The cursor overlays are correctly rendered now, as tested on JRE 11 and JRE 15 EA on Windows.

I haven't tested on Linux or MacOS.
The problem that the cursor is too large on the low-res screen still exists, that can only be fixed by the JRE (filed to the JDK people).

[simon04]

Some remarks on patch. Haven't tested the HiDPI functionality (have to find a test environment first).

1. Please remove the public modifier of the following methods. Having multiple boolean flags as parameters is often considered as bad practice. For internal use (in ImageReource and ImageProvider) it's less an issue and we can refactor more easily (without impacting various plugins).

• org.openstreetmap.josm.tools.ImageResource#getImageIcon(java.awt.Dimension, boolean)
• org.openstreetmap.josm.tools.ImageResource#getImageIconAlreadyScaled
• org.openstreetmap.josm.tools.ImageOverlay#process(BufferedImage, boolean) (keep the original as public function, also implements ImageProcessor)

2. HiDPISupport.getResolutionVariants(overlay.getImage()).get(1) – please check that the list really has ⩾2 elements before accessing get(1)

3. How does the code behave once the mentioned Java bugs (JDK-8238734, JDK-8240568) are resolved?

[johsin18]

About testing: You don't need a real HiDPI screen. Just set the scaling to 200% and check that everything consistently is twice as large. The patch will only be effective with Java > 8, of course.

1. done, I kept the Javadoc
2. done
3. Once the mentioned bugs are fixed, the proposed code will still work, but could be simplified a bit again. Fixing JDK-8240568 will also solve the problem of the cursor having twice of half the intended size under certain circumstances (mixed scaling), which we cannot fix ourselves.

Patch adapted, update commits squashed, see
​https://github.com/johsin18/josm/pull/2

[simon04]

Replying to johsin18:

Just set the scaling to 200% and check that everything consistently is twice as large.

Easier said than done – ​https://wiki.archlinux.org/index.php/HiDPI ;-)

Running java -Dsun.java2d.uiScale=2 josm-custom.jar makes everything huge except for the mouse cursor. But it isn't broken either. :-)

---

1. The automated unit tests via Jenkins run in headless mode (which is available via -Djava.awt.headless=true). This causes two ImageProviderTest to fail:

java.awt.HeadlessException
	at org.openstreetmap.josm.tools.ImageProvider.getCursorImage(ImageProvider.java:1355)
	at org.openstreetmap.josm.tools.ImageProviderTest.testGetCursorImageWithOverlay(ImageProviderTest.java:213)
	at org.openstreetmap.josm.testutils.JOSMTestRules$TimeoutThread.run(JOSMTestRules.java:731)


java.awt.HeadlessException
	at org.openstreetmap.josm.tools.ImageProvider.getCursorImage(ImageProvider.java:1355)
	at org.openstreetmap.josm.tools.ImageProviderTest.testGetCursorImageForCrosshair(ImageProviderTest.java:203)
	at org.openstreetmap.josm.testutils.JOSMTestRules$TimeoutThread.run(JOSMTestRules.java:731)

Maybe you can mock java.awt.Toolkit similar to org.openstreetmap.josm.testutils.mockers.WindowMocker for the unit tests. Otherwise exclude them if java.awt.GraphicsEnvironment.isHeadless()

2. A tiny thing: Javadoc for org.openstreetmap.josm.tools.GuiSizesHelper#setPixelDensity is lacking @param pixelDensity

[johsin18]

complete patchset

[johsin18]

Replying to simon04:

Replying to johsin18:

Just set the scaling to 200% and check that everything consistently is twice as large.

Easier said than done – ​https://wiki.archlinux.org/index.php/HiDPI ;-)

Sorry, I was talking about Windows 10. But can't you configure it on KDE, Gnome, or XFCE level?

1. great idea to use JMockit. Done. Hope the tests work now in Jenkins as well. Can I test my branch myself on Jenkins somehow?

2. done. Could have been solved by JMockit as well, but I did not bother. At best, GuiSizesHelper, would disappear completely (#19277)

If you find other small problems in the patch, feel free to fix them yourself.

[simon04]

Can I test my branch myself on Jenkins somehow?

Sorry, this is not possible. (For a long time. we've been talking about modernizing our infrastructure, see #16871.)

Using sun.awt.HeadlessToolkit causes the following Java warnings :

    [javac] /josm/test/unit/org/openstreetmap/josm/testutils/mockers/HeadlessToolkitMocker.java:10: warning: HeadlessToolkit is internal proprietary API and may be removed in a future release
    [javac] import sun.awt.HeadlessToolkit;
    [javac]               ^
    [javac] /josm/test/unit/org/openstreetmap/josm/testutils/mockers/HeadlessToolkitMocker.java:15: warning: HeadlessToolkit is internal proprietary API and may be removed in a future release
    [javac] public class HeadlessToolkitMocker extends MockUp<HeadlessToolkit> {

$ java -version
openjdk version "11.0.6" 2020-01-14
OpenJDK Runtime Environment (build 11.0.6+10)
OpenJDK 64-Bit Server VM (build 11.0.6+10, mixed mode)

I'm not sure how to solve this. Chaning to Toolkit for HeadlessToolkitMocker extends MockUp<Toolkit> doesn't seem to work.

[simon04]

In 16486/josm:

fix #18694 - Wrongly rendered cursors on HiDPI screen (patch by johsin18)

[simon04]

I've committed your patch without the HeadlessToolkitMocker. Instead, I've added two TODO mock Toolkit.getDefaultToolkit().getBestCursorSize()

Thanks for your work on HiDPI! And welcome! :-)

[simon04]

Ticket #13173 has been marked as a duplicate of this ticket.

[simon04]

In 16983/josm:

see #18694 - Extract ImageProviderTestManual, ImageProviderTestIT

[simon04]

In 16984/josm:

see #18694 - Migrate ImageProviderTest to JUnit 5, mock bestCursorSize, compare cursors with reference images