Refactor Cursor to lazy load Cursor handle

[ShahzaibIbrahim]

Problem

Currently, we have a logic to always initialize the first handle in a handle field. Instead of using it, we could switch to lazy loading via using zoomLevelToHandle map. The handle will only created when win32_getHandle is called.

---

Proposed Solution

To fix this, the Cursor class should be refactored to manage zoom-level-specific handles more robustly and safely. The following changes are needed:

1. Remove handle Field
   Eliminate the dedicated handle field from the class. All handle accesses should go through the zoomLevelToHandle map, which now acts as the single source of truth for all zoom levels.

2. Update All Related Methods
   Refactor all methods that previously relied on the handle field to instead use zoomLevelToHandle. This includes:

   • hashCode()
   • equals(Object obj)
   • isDisposed()
   • destroy()

---

Benefits

• Enables safe, lazy creation of per-zoom handles when needed.
• Simplifies and centralizes handle management logic for better maintainability.

Example

import org.eclipse.swt.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.widgets.*;

public class CursorZoomTest {
    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);

        // 1. Constructor: Cursor(Device, int style)
        Cursor cursorFromStyle = new Cursor(display, SWT.CURSOR_HAND);

        // 2. Constructor: Cursor(Device, ImageData source, ImageData mask, int hotspotX, int hotspotY)
        ImageData source = new ImageData(16, 16, 1, new PaletteData(new RGB[]{new RGB(0, 0, 0)}));
        ImageData mask = new ImageData(16, 16, 1, new PaletteData(new RGB[]{new RGB(0, 0, 0)}));
        Cursor cursorFromImageAndMask = new Cursor(display, source, mask, 0, 0);

        // 3. Constructor: Cursor(Device, ImageData source, int hotspotX, int hotspotY)
        Cursor cursorFromImageOnly = new Cursor(display, source, 0, 0);

        // 4. Constructor: Cursor(Device, ImageDataProvider imageDataProvider, int hotspotX, int hotspotY)
        ImageDataProvider provider = zoom -> source;
        Cursor cursorFromProvider = new Cursor(display, provider, 0, 0);

        Cursor[] cursors = {
            cursorFromStyle,
            cursorFromImageAndMask,
            cursorFromImageOnly,
            cursorFromProvider
        };

        int[] zoomLevels = {100, 125, 150, 175, 200, 250};

        for (int i = 0; i < cursors.length; i++) {
            System.out.println("Cursor #" + (i + 1));
            for (int zoom : zoomLevels) {
                long handle = Cursor.win32_getHandle(cursors[i], zoom);
                System.out.println("  Zoom " + zoom + ": handle = " + handle);
            }
        }

        // Dispose resources
        for (Cursor cursor : cursors) {
            cursor.dispose();
        }

        shell.dispose();
        display.dispose();
    }
}

Output

Cursor #1
  Zoom 100: handle = 65567
  Zoom 125: handle = 65567
  Zoom 150: handle = 65567
  Zoom 175: handle = 65567
  Zoom 200: handle = 65567
  Zoom 250: handle = 65567
Cursor #2
  Zoom 100: handle = 75434631
  Zoom 125: handle = 53612289
  Zoom 150: handle = 43651639
  Zoom 175: handle = 61083667
  Zoom 200: handle = 30212277
  Zoom 250: handle = 141363963
Cursor #3
  Zoom 100: handle = 33559507
  Zoom 125: handle = 263197751
  Zoom 150: handle = 84413253
  Zoom 175: handle = 21237883
  Zoom 200: handle = 56361347
  Zoom 250: handle = 46928295
Cursor #4
  Zoom 100: handle = 20124285
  Zoom 125: handle = 63705063
  Zoom 150: handle = 46729749
  Zoom 175: handle = 68358127
  Zoom 200: handle = 79891305
  Zoom 250: handle = 14290407

Note that for the Cursor 1, we always get the same handle from OS but it's scaled correctly for all zoom levels.

Fixes: #2355

[github-actions]

Test Results

   539 files   -  7     539 suites   - 7   31m 18s ⏱️ + 1m 45s
 4 371 tests  - 54   4 356 ✅  - 52   14 💤  - 3  0 ❌ ±0  1 🔥 +1 
16 692 runs   - 54  16 567 ✅  - 52  124 💤  - 3  0 ❌ ±0  1 🔥 +1 

For more details on these errors, see this check.

Results for commit 09bfb02. ± Comparison against base commit 20d18aa.

This pull request removes 54 tests.

AllWin32Tests ImageWin32Tests ‑ testDisposeDrawnImageBeforeRequestingTargetForOtherZoom
AllWin32Tests ImageWin32Tests ‑ testDrawImageAtDifferentZooms(boolean)[1] true
AllWin32Tests ImageWin32Tests ‑ testDrawImageAtDifferentZooms(boolean)[2] false
AllWin32Tests ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
AllWin32Tests ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
AllWin32Tests ImageWin32Tests ‑ testRetrieveImageDataAtDifferentZooms(boolean)[1] true
AllWin32Tests ImageWin32Tests ‑ testRetrieveImageDataAtDifferentZooms(boolean)[2] false
AllWin32Tests TestTreeColumn ‑ test_ColumnOrder
AllWin32Tests Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
AllWin32Tests Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
…

♻️ This comment has been updated with latest results.

[arunjose696]

CI fails as Added tests uses windows only API, they should be probably moved to org.eclipse.swt.tests.win32

[HeikoKlare]

Can we please split to up into multiple commits or even PRs to make it easier to review and (hopefully not necessary) to find regressions? I see three changes combined here that we could split up:

• Fixing the hotspot coordinate calculation
• Refactoring the constructors (moving initialization logic to other methods)
• Managing multiple handles for different zooms

[ShahzaibIbrahim]

Can we please split to up into multiple commits or even PRs to make it easier to review and (hopefully not necessary) to find regressions? I see three changes combined here that we could split up:

• Fixing the hotspot coordinate calculation
• Refactoring the constructors (moving initialization logic to other methods)
• Managing multiple handles for different zooms

As you proposed, I have 3 PRs for the following

• Coordinate Calculation: [Win32] Fix Cursor hotspot coordinate calculation #2366
• Move Initialization: [Win32] Refactor the Cursor constructors  #2367
• Cursor handle management: this

[HeikoKlare]

Thank you for splitting up this PR! Now with the reduced complexity, it sstill seems to be a combination of multiple changes:

• Proper initialization of the handles for other zooms by not creating a new Cursor instance
• Bug that caused the ImageDataProvider not to be used when requesting a handle for another zoom (as cursor.source is not null then, thus this will be used)
• Proper handling of multiple zoom handles instead of having one default handle (probably including full on-demand handle creation, see below)

So it would again be good to split this up accordingly.
What I do not understand with the current change is why we need to create handles for default zoom upon cursor instantiation. All other resources create their handles only on demand and since consumers need to use win32_getHandle anyway, we could create the handle when first calling that method. That would even remove the duplication of calling initialization logic inside constructors and in win32_getHandle.

[ShahzaibIbrahim]

• Bug that caused the ImageDataProvider not to be used when requesting a handle for another zoom (as cursor.source is not null then, thus this will be used)

I am not sure what bug is it you're talking about. cursor.source is always going to be null when created with style attribute. From what I understand you mean the source will not be null when requesting handle for other zoom like this snippet?

ImageData source = new ImageData(16, 16, 1, new PaletteData(new RGB[]{new RGB(0, 0, 0)}));
		ImageDataProvider provider = zoom -> source;
Cursor cursorFromProvider = new Cursor(Display.getDefault(), provider, 0, 0);
Cursor.win32_getHandle(cursorFromProvider, 200);
Cursor.win32_getHandle(cursorFromProvider, 100);

@HeikoKlare