Skip to content

Commit de3812d

Browse files
ShahzaibIbrahimHeikoKlare
authored andcommitted
[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.
1 parent ad53b13 commit de3812d

File tree

2 files changed

+107
-68
lines changed

2 files changed

+107
-68
lines changed

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

Lines changed: 45 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,6 @@
4949
*/
5050
public final class Cursor extends Resource {
5151

52-
/**
53-
* the handle to the OS cursor resource
54-
* (Warning: This field is platform dependent)
55-
* <p>
56-
* <b>IMPORTANT:</b> This field is <em>not</em> part of the SWT
57-
* public API. It is marked public only so that it can be shared
58-
* within the packages provided by SWT. It is not available on all
59-
* platforms and should never be accessed from application code.
60-
* </p>
61-
*
62-
*/
63-
private long handle;
6452
/**
6553
* Attribute to cache current native zoom level
6654
*/
@@ -70,6 +58,8 @@ public final class Cursor extends Resource {
7058

7159
private final CursorHandleProvider cursorHandleProvider;
7260

61+
private boolean isDestroyed;
62+
7363
/**
7464
* Constructs a new cursor given a device and a style
7565
* constant describing the desired cursor appearance.
@@ -119,7 +109,6 @@ public final class Cursor extends Resource {
119109
public Cursor(Device device, int style) {
120110
super(device);
121111
this.cursorHandleProvider = new StyleCursorHandleProvider(style);
122-
this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
123112
init();
124113
this.device.registerResourceWithZoomSupport(this);
125114
}
@@ -160,28 +149,14 @@ public Cursor(Device device, int style) {
160149
public Cursor(Device device, ImageData source, ImageData mask, int hotspotX, int hotspotY) {
161150
super(device);
162151
this.cursorHandleProvider = new ImageDataWithMaskCursorHandleProvider(source, mask, hotspotX, hotspotY);
163-
this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
164152
init();
165153
this.device.registerResourceWithZoomSupport(this);
166154
}
167155

168156
private static CursorHandle setupCursorFromImageData(ImageData source, ImageData mask, int hotspotX, int hotspotY) {
169-
if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
170157
if (mask == null) {
171-
if (source.getTransparencyType() != SWT.TRANSPARENCY_MASK) {
172-
SWT.error(SWT.ERROR_NULL_ARGUMENT);
173-
}
174158
mask = source.getTransparencyMask();
175159
}
176-
/* Check the bounds. Mask must be the same size as source */
177-
if (mask.width != source.width || mask.height != source.height) {
178-
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
179-
}
180-
/* Check the hotspots */
181-
if (hotspotX >= source.width || hotspotX < 0 ||
182-
hotspotY >= source.height || hotspotY < 0) {
183-
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
184-
}
185160
/* Convert depth to 1 */
186161
mask = ImageData.convertMask(mask);
187162
source = ImageData.convertMask(source);
@@ -229,18 +204,12 @@ private static CursorHandle setupCursorFromImageData(ImageData source, ImageData
229204
public Cursor(Device device, ImageData source, int hotspotX, int hotspotY) {
230205
super(device);
231206
this.cursorHandleProvider = new ImageDataCursorHandleProvider(source, hotspotX, hotspotY);
232-
this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
233207
init();
234208
this.device.registerResourceWithZoomSupport(this);
235209
}
236210

237211
private static CursorHandle setupCursorFromImageData(Device device, ImageData source, int hotspotX, int hotspotY) {
238212
if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
239-
/* Check the hotspots */
240-
if (hotspotX >= source.width || hotspotX < 0 ||
241-
hotspotY >= source.height || hotspotY < 0) {
242-
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
243-
}
244213
long hBitmap = 0;
245214
long hMask = 0;
246215
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,
341310
super(device);
342311
if (imageDataProvider == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
343312
this.cursorHandleProvider = new ImageDataProviderCursorHandleProvider(imageDataProvider, hotspotX, hotspotY);
344-
this.handle = this.cursorHandleProvider.createHandle(this.device, DEFAULT_ZOOM).getHandle();
345313
init();
346314
this.device.registerResourceWithZoomSupport(this);
347315
}
@@ -363,7 +331,7 @@ public Cursor(Device device, ImageDataProvider imageDataProvider, int hotspotX,
363331
*/
364332
public static Long win32_getHandle (Cursor cursor, int zoom) {
365333
if (cursor.isDisposed()) {
366-
return cursor.handle;
334+
return 0L;
367335
}
368336
if (cursor.zoomLevelToHandle.get(zoom) != null) {
369337
return cursor.zoomLevelToHandle.get(zoom).getHandle();
@@ -376,38 +344,19 @@ public static Long win32_getHandle (Cursor cursor, int zoom) {
376344
}
377345

378346
private void setHandleForZoomLevel(CursorHandle handle, Integer zoom) {
379-
if (this.handle == 0) {
380-
this.handle = handle.getHandle(); // Set handle for default zoom level
381-
}
382347
if (zoom != null && !zoomLevelToHandle.containsKey(zoom)) {
383348
zoomLevelToHandle.put(zoom, handle);
384349
}
385350
}
386351

387352
@Override
388353
void destroy () {
389-
/*
390-
* It is an error in Windows to destroy the current
391-
* cursor. Check that the cursor that is about to
392-
* be destroyed is the current cursor. If so, set
393-
* the current cursor to be IDC_ARROW. Note that
394-
* Windows shares predefined cursors so the call to
395-
* LoadCursor() does not leak.
396-
*/
397-
// TEMPORARY CODE
398-
// if (OS.GetCursor() == handle) {
399-
// OS.SetCursor(OS.LoadCursor(0, OS.IDC_ARROW));
400-
// }
401354
device.deregisterResourceWithZoomSupport(this);
402-
destroyHandle();
403-
}
404-
405-
private void destroyHandle () {
406355
for (CursorHandle handle : zoomLevelToHandle.values()) {
407356
handle.destroy();
408357
}
409358
zoomLevelToHandle.clear();
410-
handle = 0;
359+
this.isDestroyed = true;
411360
}
412361

413362
/**
@@ -425,7 +374,7 @@ public boolean equals (Object object) {
425374
if (object == this) return true;
426375
if (!(object instanceof Cursor)) return false;
427376
Cursor cursor = (Cursor) object;
428-
return device == cursor.device && handle == cursor.handle;
377+
return device == cursor.device && win32_getHandle(this, DEFAULT_ZOOM) == win32_getHandle(cursor, DEFAULT_ZOOM);
429378
}
430379

431380
/**
@@ -440,7 +389,7 @@ public boolean equals (Object object) {
440389
*/
441390
@Override
442391
public int hashCode () {
443-
return (int)handle;
392+
return win32_getHandle(this, DEFAULT_ZOOM).intValue();
444393
}
445394

446395
/**
@@ -455,7 +404,7 @@ public int hashCode () {
455404
*/
456405
@Override
457406
public boolean isDisposed() {
458-
return handle == 0;
407+
return isDestroyed;
459408
}
460409

461410
/**
@@ -467,7 +416,7 @@ public boolean isDisposed() {
467416
@Override
468417
public String toString () {
469418
if (isDisposed()) return "Cursor {*DISPOSED*}";
470-
return "Cursor {" + handle + "}";
419+
return "Cursor {" + zoomLevelToHandle + "}";
471420
}
472421

473422
@Override
@@ -531,19 +480,23 @@ private static interface CursorHandleProvider {
531480
}
532481

533482
private static class StyleCursorHandleProvider implements CursorHandleProvider {
534-
private final int style;
483+
private final long lpCursorName;
535484

536485
public StyleCursorHandleProvider(int style) {
537-
this.style = style;
486+
this.lpCursorName = getOSCursorIdFromStyle(style);
538487
}
539488

540489
@Override
541490
public CursorHandle createHandle(Device device, int zoom) {
542491
// zoom ignored, LoadCursor handles scaling internally
543-
return setupCursorFromStyle(this.style);
492+
long handle = OS.LoadCursor(0, lpCursorName);
493+
if (handle == 0) {
494+
SWT.error(SWT.ERROR_NO_HANDLES);
495+
}
496+
return new CustomCursorHandle(handle);
544497
}
545498

546-
private static final CursorHandle setupCursorFromStyle(int style) {
499+
private static final long getOSCursorIdFromStyle(int style) {
547500
long lpCursorName = 0;
548501
switch (style) {
549502
case SWT.CURSOR_HAND:
@@ -615,11 +568,7 @@ private static final CursorHandle setupCursorFromStyle(int style) {
615568
default:
616569
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
617570
}
618-
long handle = OS.LoadCursor(0, lpCursorName);
619-
if (handle == 0) {
620-
SWT.error(SWT.ERROR_NO_HANDLES);
621-
}
622-
return new CustomCursorHandle(handle);
571+
return lpCursorName;
623572
}
624573
}
625574

@@ -639,13 +588,24 @@ protected final int getHotpotXInPixels(int zoom) {
639588
protected final int getHotpotYInPixels(int zoom) {
640589
return Win32DPIUtils.pointToPixel(hotspotY, zoom);
641590
}
591+
592+
protected static final void validateHotspotInsideImage(ImageData source, int hotspotX, int hotspotY) {
593+
/* Check the hotspots */
594+
if (hotspotX >= source.width || hotspotX < 0 ||
595+
hotspotY >= source.height || hotspotY < 0) {
596+
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
597+
}
598+
}
642599
}
643600

644601
private static class ImageDataProviderCursorHandleProvider extends HotspotAwareCursorHandleProvider {
645602
private final ImageDataProvider provider;
646603

647604
public ImageDataProviderCursorHandleProvider(ImageDataProvider provider, int hotspotX, int hotspotY) {
648605
super(hotspotX, hotspotY);
606+
ImageData source = provider.getImageData(DEFAULT_ZOOM);
607+
if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
608+
validateHotspotInsideImage(source, hotspotX, hotspotY);
649609
this.provider = provider;
650610
}
651611

@@ -668,6 +628,8 @@ private static class ImageDataCursorHandleProvider extends HotspotAwareCursorHan
668628

669629
public ImageDataCursorHandleProvider(ImageData source, int hotspotX, int hotspotY) {
670630
super(hotspotX, hotspotY);
631+
if (source == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
632+
validateHotspotInsideImage(source, hotspotX, hotspotY);
671633
this.source = source;
672634
}
673635

@@ -685,6 +647,21 @@ private static class ImageDataWithMaskCursorHandleProvider extends ImageDataCurs
685647
public ImageDataWithMaskCursorHandleProvider(ImageData source, ImageData mask, int hotspotX, int hotspotY) {
686648
super(source, hotspotX, hotspotY);
687649
this.mask = mask;
650+
validateMask(source, mask);
651+
}
652+
653+
private void validateMask(ImageData source, ImageData mask) {
654+
ImageData testMask = mask == null ? null : (ImageData) mask.clone();
655+
if (testMask == null) {
656+
if (source.getTransparencyType() != SWT.TRANSPARENCY_MASK) {
657+
SWT.error(SWT.ERROR_NULL_ARGUMENT);
658+
}
659+
testMask = source.getTransparencyMask();
660+
}
661+
/* Check the bounds. Mask must be the same size as source */
662+
if (testMask.width != source.width || testMask.height != source.height) {
663+
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
664+
}
688665
}
689666

690667
@Override

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.eclipse.swt.graphics.ImageData;
2929
import org.eclipse.swt.graphics.ImageDataProvider;
3030
import org.eclipse.swt.graphics.ImageLoader;
31+
import org.eclipse.swt.graphics.PaletteData;
32+
import org.eclipse.swt.graphics.RGB;
3133
import org.eclipse.swt.widgets.Display;
3234
import org.junit.Before;
3335
import org.junit.Test;
@@ -162,6 +164,66 @@ public void test_ConstructorWithImageDataProvider() {
162164
IllegalArgumentException.class, () -> new Cursor(display, (ImageDataProvider) null, 0, 0));
163165
}
164166

167+
@Test
168+
public void test_InvalidArgumentsForAllConstructors() {
169+
ImageData source = new ImageData(16, 16, 1, new PaletteData(new RGB[] { new RGB(0, 0, 0) }));
170+
ImageData mask = new ImageData(16, 16, 1, new PaletteData(new RGB[] { new RGB(0, 0, 0) }));
171+
172+
assertThrows("When wrong style was provided", IllegalArgumentException.class,
173+
() -> {
174+
Cursor cursor = new Cursor(Display.getDefault(), -99);
175+
cursor.dispose();
176+
});
177+
178+
assertThrows("When source is null", IllegalArgumentException.class, () -> {
179+
Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), null, mask, 0, 0);
180+
cursorFromImageAndMask.dispose();
181+
});
182+
183+
assertThrows("When mask is null and source doesn't heve a mask",
184+
IllegalArgumentException.class, () -> {
185+
Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), source, null, 0, 0);
186+
cursorFromImageAndMask.dispose();
187+
});
188+
189+
assertThrows("When source and the mask are not the same size",
190+
IllegalArgumentException.class, () -> {
191+
ImageData source32 = new ImageData(32, 32, 1, new PaletteData(new RGB[] { new RGB(0, 0, 0) }));
192+
ImageData mask16 = new ImageData(16, 16, 1, new PaletteData(new RGB[] { new RGB(0, 0, 0) }));
193+
194+
Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), source32, mask16, 0, 0);
195+
cursorFromImageAndMask.dispose();
196+
});
197+
198+
assertThrows("When hotspot is outside the bounds of the image",
199+
IllegalArgumentException.class, () -> {
200+
Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), source, mask, 18, 18);
201+
cursorFromImageAndMask.dispose();
202+
});
203+
204+
assertThrows("When source image data is null", IllegalArgumentException.class,
205+
() -> {
206+
ImageData nullImageData = null;
207+
Cursor cursorFromSourceOnly = new Cursor(Display.getDefault(), nullImageData, 0, 0);
208+
cursorFromSourceOnly.dispose();
209+
});
210+
211+
assertThrows("When ImageDataProvider is null", IllegalArgumentException.class,
212+
() -> {
213+
ImageDataProvider provider = null;
214+
Cursor cursorFromProvider = new Cursor(Display.getDefault(), provider, 0, 0);
215+
cursorFromProvider.dispose();
216+
});
217+
218+
assertThrows("When source in ImageDataProvider is null",
219+
IllegalArgumentException.class, () -> {
220+
ImageData nullSource = null;
221+
ImageDataProvider provider = zoom -> nullSource;
222+
Cursor cursorFromProvider = new Cursor(Display.getDefault(), provider, 0, 0);
223+
cursorFromProvider.dispose();
224+
});
225+
}
226+
165227
@Test
166228
public void test_equalsLjava_lang_Object() {
167229
/* Note: Two cursors are only considered equal if their handles are equal.

0 commit comments

Comments
 (0)