Revise the Cursor class to support scaling

[arunjose696]

Previously, cursors were initialized with a single ImageData, which caused issues on systems with varying zoom levels, for example, cursors were not scaled at all on Linux, or were blurrily auto-scaled on Windows.

This commit introduces a new constructor for the Cursor class that accepts an Image object instead of ImageData. This allows the code instantiating the cursor to pass an image capable of providing appropriately scaled image data based on the current zoom level.

[github-actions]

Test Results

   539 files   -  6     539 suites   - 6   32m 7s ⏱️ + 2m 7s
 4 369 tests  - 36   4 353 ✅  - 34   15 💤  - 3  0 ❌ ±0  1 🔥 +1 
16 708 runs   - 33  16 568 ✅  - 31  139 💤  - 3  0 ❌ ±0  1 🔥 +1 

For more details on these errors, see this check.

Results for commit 20486f3. ± Comparison against base commit 51fed8e.

This pull request removes 37 and adds 1 tests. Note that renamed tests count towards both.

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
…

org.eclipse.swt.tests.junit.Test_org_eclipse_swt_graphics_Cursor ‑ test_ConstructorWithImageDataProvider

♻️ This comment has been updated with latest results.

[ptziegler]

Out of curiosity: Is there a benefit to using the Image, rather than e.g. the ImageDataProvider?
This now creates an additional handle that needs to be disposed together with the Cursor. Which is fine, of course, just not very convenient.
Latter might also play a little nicer with the ImageDescriptor, because you could simply create the cursor via new Cursor(device, imageDescriptor::getImageData, hotspotX, hotspotY).

[arunjose696]

Out of curiosity: Is there a benefit to using the Image, rather than e.g. the ImageDataProvider? This now creates an additional handle that needs to be disposed together with the Cursor. Which is fine, of course, just not very convenient. Latter might also play a little nicer with the ImageDescriptor, because you could simply create the cursor via new Cursor(device, imageDescriptor::getImageData, hotspotX, hotspotY).

If cursors are initialized with a single ImageData and later requested at a different zoom level, that image data is auto-scaled using DPUtil. This just scales up the original ImageData, which makes the cursor appear blurry at different zoom levels (e.g., when moving the cursor from one monitor to another). If Image is used this scaling can be non blurry.

[laeubi]

Out of curiosity: Is there a benefit to using the Image, rather than e.g. the ImageDataProvider? This now creates an additional handle that needs to be disposed together with the Cursor. Which is fine, of course, just not very convenient. Latter might also play a little nicer with the ImageDescriptor, because you could simply create the cursor via new Cursor(device, imageDescriptor::getImageData, hotspotX, hotspotY).

If cursors are initialized with a single ImageData and later requested at a different zoom level, that image data is auto-scaled using DPUtil. This just scales up the original ImageData, which makes the cursor appear blurry at different zoom levels (e.g., when moving the cursor from one monitor to another). If Image is used this scaling can be non blurry.

I think this does not really answers the question why not using ImageDataProvider in the first place (that might or might not construct an internal image and dispose it appropriately later on).

[akoch-yatta]

Out of curiosity: Is there a benefit to using the Image, rather than e.g. the ImageDataProvider? This now creates an additional handle that needs to be disposed together with the Cursor. Which is fine, of course, just not very convenient. Latter might also play a little nicer with the ImageDescriptor, because you could simply create the cursor via new Cursor(device, imageDescriptor::getImageData, hotspotX, hotspotY).

If cursors are initialized with a single ImageData and later requested at a different zoom level, that image data is auto-scaled using DPUtil. This just scales up the original ImageData, which makes the cursor appear blurry at different zoom levels (e.g., when moving the cursor from one monitor to another). If Image is used this scaling can be non blurry.

I think this does not really answers the question why not using ImageDataProvider in the first place (that might or might not construct an internal image and dispose it appropriately later on).

We discussed using ImageDataProvider, but thought using an Image directly would give the most freedom. Most usecases with cursors seem to be related to image files. Therefor, using an ImageDataProvider would make the implementation a bit more complex with pure SWT. Still, i see the argument with the additonal handle and the use case with ImageDescriptor would probably solve the added complexity we had in mind. So I'm actually liking the idea.

[arunjose696]

I've updated the PR to make the constructor accept an ImageDataProvider instead of an Image. This approach is preferable, as it avoids creating undisposed handles and works better with ImageDescriptor.

The only minor downside I can think is that cursors won't be scaled on GTK or macOS, since these platforms would set the cursor at 100% zoom and wouldnt update them later. However, I believe this is acceptable, as it matches the current behavior.

[arunjose696]

Have all CI checks passing.

[fedejeanne]

Failing test is unrelated #2098