Use the native zoom when setting the cursor in Control #2057

[fedejeanne]

Fixes #2057

How to test

• Use 2 monitors with different zoom levels (none of them should match the zoom level passed to -Dswt.autoScale in the next step)
• Run Snippet119 with the following JVM parameters:
  • -Dswt.autoScale=200 (or use another zoom level that doesn't match any of the zooms in your monitors)
  • -Dswt.autoScale.updateOnRuntime=true

Expected: the size of the cursor matches the size of the current monitor and not the one you provided for -Dswt.autoScale

• Move the window of the snippet to the monitor

Expected: the size of the cursor still matches the size of the current monitor and not the one you provided for -Dswt.autoScale

[github-actions]

Test Results

   539 files   -  6     539 suites   - 6   25m 16s ⏱️ - 1m 52s
 4 337 tests  - 37   4 321 ✅  - 35   15 💤  - 3  1 ❌ +1 
16 601 runs   - 37  16 463 ✅  - 35  137 💤  - 3  1 ❌ +1 

For more details on these failures, see this check.

Results for commit b750b51. ± Comparison against base commit f92ce76.

This pull request removes 37 tests.

AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testHtmlTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromCopiedImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageData
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageDataFromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testRtfTransfer
…

♻️ This comment has been updated with latest results.

[HeikoKlare]

Note that #2057 is without -Dswt.autoScale.updateOnRuntime=true. Does this work for that as well? I would expect getNativeZoom() to return the value of swt.autoScale in that case.

[fedejeanne]

Note that #2057 is without -Dswt.autoScale.updateOnRuntime=true. Does this work for that as well? I would expect getNativeZoom() to return the value of swt.autoScale in that case.

The current proposal (using the native zoom) means that the size of the cursor will not be affected by the parameter -Dswt.autoscale but will react to the zoom of the current monitor instead. Additionally, if one does not use the monitor-specific UI scaling (e.g. if -Dswt.autoScale.updateOnRuntime=false) then the size of the cursor will be the same in all monitors and it will be given by the zoom of the primary monitor.

[fedejeanne]

I see the following error in the check:

Error:  Cannot resolve dependencies of project org.eclipse.platform:org.eclipse.swt.svg:eclipse-plugin:3.130.0-SNAPSHOT
Error:   with context {osgi.os=linux, org.eclipse.update.install.features=true, org.eclipse.swt.buildtime=true, osgi.arch=x86_64, org.eclipse.update.install.sources=true, osgi.ws=gtk, org.eclipse.jdt.buildtime=true}
Error:    Software being installed: org.eclipse.swt.svg 3.130.0.qualifier
Error:    Missing requirement: org.eclipse.swt.svg 3.130.0.qualifier requires 'org.eclipse.equinox.p2.iu; org.eclipse.swt.win32.win32.x86_64 0.0.0' but it could not be found: See log for details

And it seems unrelated to these changes.

@HeikoKlare all good from your side ✔️ ?

[HeikoKlare]

What about all the other places where getZoom() has been introduced to the handle retrieval in OS.SetCursor? I only see one such place fixed, so I wonder why the others do not need to be changed.
See the according commit: https://github.com/eclipse-platform/eclipse.platform.swt/pull/1603/files

The version increment check is failing, but I guess version increment is not necessary here?

[fedejeanne]

What about all the other places where getZoom() has been introduced to the handle retrieval in OS.SetCursor? I only see one such place fixed, so I wonder why the others do not need to be changed.
See the according commit: https://github.com/eclipse-platform/eclipse.platform.swt/pull/1603/files

I see the places and it could also make sense to apply the same change in all of them, but I'm having trouble testing those changes since I can't find any snippet or execution path in the workbench that runs through them. I was only able to run through 1 other place, namely the one in org.eclipse.swt.widgets.Tree.setCursor(), but I haven't seen any odd behavior in there, the cursor (which is a "busy cursor") has the correct size. Do you (@HeikoKlare / @ShahzaibIbrahim ) know how to reach the other places affected by #1603 where the cursor is set (the ones doing Cursor.win32_getHandle(someCursor, getZoom()) ?

The version increment check is failing, but I guess version increment is not necessary here?

Correct.

[fedejeanne]

The version increment check is failing, but I guess version increment is not necessary here?

Correct.

FTR this is the error ☝️

I see the following error in the check:

Error:  Cannot resolve dependencies of project org.eclipse.platform:org.eclipse.swt.svg:eclipse-plugin:3.130.0-SNAPSHOT
Error:   with context {osgi.os=linux, org.eclipse.update.install.features=true, org.eclipse.swt.buildtime=true, osgi.arch=x86_64, org.eclipse.update.install.sources=true, osgi.ws=gtk, org.eclipse.jdt.buildtime=true}
Error:    Software being installed: org.eclipse.swt.svg 3.130.0.qualifier
Error:    Missing requirement: org.eclipse.swt.svg 3.130.0.qualifier requires 'org.eclipse.equinox.p2.iu; org.eclipse.swt.win32.win32.x86_64 0.0.0' but it could not be found: See log for details

[fedejeanne]

I ran all the UI tests in Platform and also all the tests in SWT and the other places are not covered, so I assume they are very rare use-cases. In any case, I adapted all invocations of org.eclipse.swt.graphics.Cursor.win32_getHandle(Cursor) in b750b51 so they don't pass the zoom as parameter and use the zoom of the current monitor instead since the mouse should always match the zoom of the monitor and nothing else.

[fedejeanne]

Test failure is unrelated (#1843) and the failed version check is due to the infrastructure issues.