From a3be4bdae233ee62dcc54c8ca9542250d5ee9956 Mon Sep 17 00:00:00 2001 From: Shahzaib Ibrahim Date: Tue, 2 Sep 2025 14:34:07 +0200 Subject: [PATCH] [Win32] Lazy load cursor handles This change updates the Win32 implementation of the Cursor class to defer creation of the native cursor handle until first use, enabling lazy loading instead of initializing the handle at construction time. Key changes: - Move initialization of the first handle creation from the constructors to the first handle retrieval. - Update equals(), hashCode(), isDisposed(), and toString() to work with zoom-aware handles and a new isDestroyed flag. - Add comprehensive unit tests for invalid constructor arguments to improve coverage and guard against improper usage. This change reduces unnecessary resource creation. --- .../org/eclipse/swt/graphics/Cursor.java | 113 +++++++----------- .../Test_org_eclipse_swt_graphics_Cursor.java | 62 ++++++++++ 2 files changed, 107 insertions(+), 68 deletions(-) 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.