diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Cursor.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Cursor.java index 3450118afdc..52cb8d8002e 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Cursor.java +++ b/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) - *

- * IMPORTANT: This field is not 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. - *

- * - */ - 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,9 +344,6 @@ 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); } @@ -386,28 +351,12 @@ private void setHandleForZoomLevel(CursorHandle handle, Integer zoom) { @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,6 +588,14 @@ 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) { + /* 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 { @@ -646,6 +603,9 @@ private static class ImageDataProviderCursorHandleProvider extends HotspotAwareC public ImageDataProviderCursorHandleProvider(ImageDataProvider provider, int hotspotX, int hotspotY) { super(hotspotX, hotspotY); + ImageData source = provider.getImageData(DEFAULT_ZOOM); + if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT); + validateHotspotInsideImage(source, hotspotX, hotspotY); this.provider = provider; } @@ -668,6 +628,8 @@ private static class ImageDataCursorHandleProvider extends HotspotAwareCursorHan public ImageDataCursorHandleProvider(ImageData source, int hotspotX, int hotspotY) { super(hotspotX, hotspotY); + if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT); + validateHotspotInsideImage(source, hotspotX, hotspotY); this.source = source; } @@ -685,6 +647,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 diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Cursor.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Cursor.java index 7c5f40d48a3..4d523a22d2d 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Cursor.java +++ b/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.