Refactor Cursor handle to support per-zoom handles

- Removed the dedicated `handle` field from Cursor; all native handles
are now stored in `zoomLevelToHandle`.
- Updated hashCode, equals, isDisposed, and destroy to work with
zoom-level handles.

Author: ShahzaibIbrahim

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Cursor.java

@@ -49,18 +49,6 @@
  */
 public final class Cursor extends Resource {
 
-	/**
-	 * the handle to the OS cursor resource
-	 * (Warning: This field is platform dependent)
-	 * <p>
-	 * <b>IMPORTANT:</b> This field is <em>not</em> part of the SWT
-	 * public API. It is marked public only so that it can be shared
-	 * within the packages provided by SWT. It is not available on all
-	 * platforms and should never be accessed from application code.
-	 * </p>
-	 *
-	 */
-	private long handle;
 	/**
 	 * Attribute to cache current native zoom level
 	 */
@@ -70,6 +58,8 @@ public final class Cursor extends Resource {
 
 	private final CursorHandleProvider cursorHandleProvider;
 
+	private boolean isDestroyed;
+
 /**
  * Constructs a new cursor given a device and a style
  * constant describing the desired cursor appearance.
@@ -119,7 +109,6 @@ public final class Cursor extends Resource {
 public Cursor(Device device, int style) {
 	super(device);
 	this.cursorHandleProvider = new StyleCursorHandleProvider(style);
-	this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
 	init();
 	this.device.registerResourceWithZoomSupport(this);
 }
@@ -160,28 +149,14 @@ public Cursor(Device device, int style) {
 public Cursor(Device device, ImageData source, ImageData mask, int hotspotX, int hotspotY) {
 	super(device);
 	this.cursorHandleProvider = new ImageDataWithMaskCursorHandleProvider(source, mask, hotspotX, hotspotY);
-	this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
 	init();
 	this.device.registerResourceWithZoomSupport(this);
 }
 
 private static CursorHandle setupCursorFromImageData(ImageData source, ImageData mask, int hotspotX, int hotspotY) {
-	if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
 	if (mask == null) {
-		if (source.getTransparencyType() != SWT.TRANSPARENCY_MASK) {
-			SWT.error(SWT.ERROR_NULL_ARGUMENT);
-		}
 		mask = source.getTransparencyMask();
 	}
-	/* Check the bounds. Mask must be the same size as source */
-	if (mask.width != source.width || mask.height != source.height) {
-		SWT.error(SWT.ERROR_INVALID_ARGUMENT);
-	}
-	/* Check the hotspots */
-	if (hotspotX >= source.width || hotspotX < 0 ||
-		hotspotY >= source.height || hotspotY < 0) {
-		SWT.error(SWT.ERROR_INVALID_ARGUMENT);
-	}
 	/* Convert depth to 1 */
 	mask = ImageData.convertMask(mask);
 	source = ImageData.convertMask(source);
@@ -229,18 +204,12 @@ private static CursorHandle setupCursorFromImageData(ImageData source, ImageData
 public Cursor(Device device, ImageData source, int hotspotX, int hotspotY) {
 	super(device);
 	this.cursorHandleProvider = new ImageDataCursorHandleProvider(source, hotspotX, hotspotY);
-	this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
 	init();
 	this.device.registerResourceWithZoomSupport(this);
 }
 
 private static CursorHandle setupCursorFromImageData(Device device, ImageData source, int hotspotX, int hotspotY) {
 	if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
-	/* Check the hotspots */
-	if (hotspotX >= source.width || hotspotX < 0 ||
-		hotspotY >= source.height || hotspotY < 0) {
-		SWT.error(SWT.ERROR_INVALID_ARGUMENT);
-	}
 	long hBitmap = 0;
 	long hMask = 0;
 	if (source.maskData == null && source.transparentPixel == -1 && (source.alpha != -1 || source.alphaData != null)) {
@@ -341,7 +310,6 @@ public Cursor(Device device, ImageDataProvider imageDataProvider, int hotspotX,
 	super(device);
 	if (imageDataProvider == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
 	this.cursorHandleProvider = new ImageDataProviderCursorHandleProvider(imageDataProvider, hotspotX, hotspotY);
-	this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
 	init();
 	this.device.registerResourceWithZoomSupport(this);
 }
@@ -363,7 +331,7 @@ public Cursor(Device device, ImageDataProvider imageDataProvider, int hotspotX,
  */
 public static Long win32_getHandle (Cursor cursor, int zoom) {
 	if (cursor.isDisposed()) {
-		return cursor.handle;
+		return 0L;
 	}
 	if (cursor.zoomLevelToHandle.get(zoom) != null) {
 		return cursor.zoomLevelToHandle.get(zoom).getHandle();
@@ -376,38 +344,19 @@ public static Long win32_getHandle (Cursor cursor, int zoom) {
 }
 
 private void setHandleForZoomLevel(CursorHandle handle, Integer zoom) {
-	if (this.handle == 0) {
-		this.handle = handle.getHandle();	// Set handle for default zoom level
-	}
 	if (zoom != null && !zoomLevelToHandle.containsKey(zoom)) {
 		zoomLevelToHandle.put(zoom, handle);
 	}
 }
 
 @Override
 void destroy () {
-	/*
-	* It is an error in Windows to destroy the current
-	* cursor.  Check that the cursor that is about to
-	* be destroyed is the current cursor.  If so, set
-	* the current cursor to be IDC_ARROW.  Note that
-	* Windows shares predefined cursors so the call to
-	* LoadCursor() does not leak.
-	*/
-	// TEMPORARY CODE
-//	if (OS.GetCursor() == handle) {
-//		OS.SetCursor(OS.LoadCursor(0, OS.IDC_ARROW));
-//	}
 	device.deregisterResourceWithZoomSupport(this);
-	destroyHandle();
-}
-
-private void destroyHandle () {
 	for (CursorHandle handle : zoomLevelToHandle.values()) {
 		handle.destroy();
 	}
 	zoomLevelToHandle.clear();
-	handle = 0;
+	this.isDestroyed = true;
 }
 
 /**
@@ -425,7 +374,7 @@ public boolean equals (Object object) {
 	if (object == this) return true;
 	if (!(object instanceof Cursor)) return false;
 	Cursor cursor = (Cursor) object;
-	return device == cursor.device && handle == cursor.handle;
+	return device == cursor.device && win32_getHandle(this, DEFAULT_ZOOM) == win32_getHandle(cursor, DEFAULT_ZOOM);
 }
 
 /**
@@ -440,7 +389,7 @@ public boolean equals (Object object) {
  */
 @Override
 public int hashCode () {
-	return (int)handle;
+	return win32_getHandle(this, DEFAULT_ZOOM).intValue();
 }
 
 /**
@@ -455,7 +404,7 @@ public int hashCode () {
  */
 @Override
 public boolean isDisposed() {
-	return handle == 0;
+	return isDestroyed;
 }
 
 /**
@@ -467,7 +416,7 @@ public boolean isDisposed() {
 @Override
 public String toString () {
 	if (isDisposed()) return "Cursor {*DISPOSED*}";
-	return "Cursor {" + handle + "}";
+	return "Cursor {" + zoomLevelToHandle + "}";
 }
 
 @Override
@@ -531,19 +480,23 @@ private static interface CursorHandleProvider {
 }
 
 private static class StyleCursorHandleProvider implements CursorHandleProvider {
-	private final int style;
+	private final long lpCursorName;
 
 	public StyleCursorHandleProvider(int style) {
-		this.style = style;
+		this.lpCursorName = getOSCursorIdFromStyle(style);
 	}
 
 	@Override
 	public CursorHandle createHandle(Device device, int zoom) {
 		// zoom ignored, LoadCursor handles scaling internally
-		return setupCursorFromStyle(this.style);
+		long handle = OS.LoadCursor(0, lpCursorName);
+		if (handle == 0) {
+			SWT.error(SWT.ERROR_NO_HANDLES);
+		}
+		return new CustomCursorHandle(handle);
 	}
 
-	private static final CursorHandle setupCursorFromStyle(int style) {
+	private static final long getOSCursorIdFromStyle(int style) {
 		long lpCursorName = 0;
 		switch (style) {
 		case SWT.CURSOR_HAND:
@@ -615,11 +568,7 @@ private static final CursorHandle setupCursorFromStyle(int style) {
 		default:
 			SWT.error(SWT.ERROR_INVALID_ARGUMENT);
 		}
-		long handle = OS.LoadCursor(0, lpCursorName);
-		if (handle == 0) {
-			SWT.error(SWT.ERROR_NO_HANDLES);
-		}
-		return new CustomCursorHandle(handle);
+		return lpCursorName;
 	}
 }
 
@@ -639,13 +588,24 @@ protected final int getHotpotXInPixels(int zoom) {
 	protected final int getHotpotYInPixels(int zoom) {
 		return Win32DPIUtils.pointToPixel(hotspotY, zoom);
 	}
+
+	protected static final void validateHotspotInsideImage(ImageData source, int hotspotX, int hotspotY) {
+		if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
+		/* Check the hotspots */
+		if (hotspotX >= source.width || hotspotX < 0 ||
+			hotspotY >= source.height || hotspotY < 0) {
+			SWT.error(SWT.ERROR_INVALID_ARGUMENT);
+		}
+	}
 }
 
 private static class ImageDataProviderCursorHandleProvider extends HotspotAwareCursorHandleProvider {
 	private final ImageDataProvider provider;
 
 	public ImageDataProviderCursorHandleProvider(ImageDataProvider provider, int hotspotX, int hotspotY) {
 		super(hotspotX, hotspotY);
+		ImageData source = provider.getImageData(DEFAULT_ZOOM);
+		validateHotspotInsideImage(source, hotspotX, hotspotY);
 		this.provider = provider;
 	}
 
@@ -668,6 +628,7 @@ private static class ImageDataCursorHandleProvider extends HotspotAwareCursorHan
 
 	public ImageDataCursorHandleProvider(ImageData source, int hotspotX, int hotspotY) {
 		super(hotspotX, hotspotY);
+		validateHotspotInsideImage(source, hotspotX, hotspotY);
 		this.source = source;
 	}
 
@@ -685,6 +646,21 @@ private static class ImageDataWithMaskCursorHandleProvider extends ImageDataCurs
 	public ImageDataWithMaskCursorHandleProvider(ImageData source, ImageData mask, int hotspotX, int hotspotY) {
 		super(source, hotspotX, hotspotY);
 		this.mask = mask;
+		validateMask(source, mask);
+	}
+
+	private void validateMask(ImageData source, ImageData mask) {
+		ImageData testMask = mask == null ? null : (ImageData) mask.clone();
+		if (testMask == null) {
+			if (source.getTransparencyType() != SWT.TRANSPARENCY_MASK) {
+				SWT.error(SWT.ERROR_NULL_ARGUMENT);
+			}
+			testMask = source.getTransparencyMask();
+		}
+		/* Check the bounds. Mask must be the same size as source */
+		if (testMask.width != source.width || testMask.height != source.height) {
+			SWT.error(SWT.ERROR_INVALID_ARGUMENT);
+		}
 	}
 
 	@Override

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Cursor.java

@@ -28,6 +28,8 @@
 import org.eclipse.swt.graphics.ImageData;
 import org.eclipse.swt.graphics.ImageDataProvider;
 import org.eclipse.swt.graphics.ImageLoader;
+import org.eclipse.swt.graphics.PaletteData;
+import org.eclipse.swt.graphics.RGB;
 import org.eclipse.swt.widgets.Display;
 import org.junit.Before;
 import org.junit.Test;
@@ -162,6 +164,66 @@ public void test_ConstructorWithImageDataProvider() {
 			IllegalArgumentException.class, () -> new Cursor(display, (ImageDataProvider) null, 0, 0));
 }
 
+@Test
+public void test_InvalidArgumentsForAllConstructors() {
+	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) }));
+
+	assertThrows("When wrong style was provided", IllegalArgumentException.class,
+			() -> {
+				Cursor cursor = new Cursor(Display.getDefault(), -99);
+				cursor.dispose();
+			});
+
+	assertThrows("When source is null", IllegalArgumentException.class, () -> {
+		Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), null, mask, 0, 0);
+		cursorFromImageAndMask.dispose();
+	});
+
+	assertThrows("When mask is null and source doesn't heve a mask",
+			IllegalArgumentException.class, () -> {
+				Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), source, null, 0, 0);
+				cursorFromImageAndMask.dispose();
+			});
+
+	assertThrows("When source and the mask are not the same size",
+			IllegalArgumentException.class, () -> {
+				ImageData source32 = new ImageData(32, 32, 1, new PaletteData(new RGB[] { new RGB(0, 0, 0) }));
+				ImageData mask16 = new ImageData(16, 16, 1, new PaletteData(new RGB[] { new RGB(0, 0, 0) }));
+
+				Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), source32, mask16, 0, 0);
+				cursorFromImageAndMask.dispose();
+			});
+
+	assertThrows("When hotspot is outside the bounds of the image",
+			IllegalArgumentException.class, () -> {
+				Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), source, mask, 18, 18);
+				cursorFromImageAndMask.dispose();
+			});
+
+	assertThrows("When source image data is null", IllegalArgumentException.class,
+			() -> {
+				ImageData nullImageData = null;
+				Cursor cursorFromSourceOnly = new Cursor(Display.getDefault(), nullImageData, 0, 0);
+				cursorFromSourceOnly.dispose();
+			});
+
+	assertThrows("When ImageDataProvider is null", IllegalArgumentException.class,
+			() -> {
+				ImageDataProvider provider = null;
+				Cursor cursorFromProvider = new Cursor(Display.getDefault(), provider, 0, 0);
+				cursorFromProvider.dispose();
+			});
+
+	assertThrows("When source in ImageDataProvider is null",
+			IllegalArgumentException.class, () -> {
+				ImageData nullSource = null;
+				ImageDataProvider provider = zoom -> nullSource;
+				Cursor cursorFromProvider = new Cursor(Display.getDefault(), provider, 0, 0);
+				cursorFromProvider.dispose();
+			});
+}
+
 @Test
 public void test_equalsLjava_lang_Object() {
 	/* Note: Two cursors are only considered equal if their handles are equal.