Skip to content

Refactor Cursor to lazy load Cursor handle #2362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactor Cursor to lazy load Cursor handle #2362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 40 additions & 34 deletions bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Cursor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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.
Expand Down Expand Up @@ -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(device, DEFAULT_ZOOM).getHandle();
init();
this.device.registerResourceWithZoomSupport(this);
}
Expand Down Expand Up @@ -160,7 +149,6 @@ 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(device, DEFAULT_ZOOM).getHandle();
init();
this.device.registerResourceWithZoomSupport(this);
}
Expand Down Expand Up @@ -229,7 +217,6 @@ 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(device, DEFAULT_ZOOM).getHandle();
init();
this.device.registerResourceWithZoomSupport(this);
}
Expand Down Expand Up @@ -341,7 +328,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(device, DEFAULT_ZOOM).getHandle();
init();
this.device.registerResourceWithZoomSupport(this);
}
Expand All @@ -363,7 +349,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();
Expand All @@ -376,9 +362,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);
}
Expand Down Expand Up @@ -407,7 +390,7 @@ private void destroyHandle () {
handle.destroy();
}
zoomLevelToHandle.clear();
handle = 0;
this.isDestroyed = true;
}

/**
Expand All @@ -425,7 +408,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);
}

/**
Expand All @@ -440,7 +423,7 @@ public boolean equals (Object object) {
*/
@Override
public int hashCode () {
return (int)handle;
return win32_getHandle(this, DEFAULT_ZOOM).intValue();
}

/**
Expand All @@ -455,7 +438,7 @@ public int hashCode () {
*/
@Override
public boolean isDisposed() {
return handle == 0;
return isDestroyed;
}

/**
Expand All @@ -467,7 +450,7 @@ public boolean isDisposed() {
@Override
public String toString () {
if (isDisposed()) return "Cursor {*DISPOSED*}";
return "Cursor {" + handle + "}";
return "Cursor {" + zoomLevelToHandle + "}";
}

@Override
Expand Down Expand Up @@ -531,19 +514,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 = setupCursorFromStyle(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 setupCursorFromStyle(int style) {
long lpCursorName = 0;
switch (style) {
case SWT.CURSOR_HAND:
Expand Down Expand Up @@ -615,11 +602,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;
}
}

Expand All @@ -646,6 +629,13 @@ 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);
/* Check the hotspots */
if (hotspotX >= source.width || hotspotX < 0 ||
hotspotY >= source.height || hotspotY < 0) {
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
}
this.provider = provider;
}

Expand All @@ -668,6 +658,12 @@ 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);
/* Check the hotspots */
if (hotspotX >= source.width || hotspotX < 0 ||
hotspotY >= source.height || hotspotY < 0) {
SWT.error(SWT.ERROR_INVALID_ARGUMENT);
}
this.source = source;
}

Expand All @@ -684,6 +680,16 @@ private static class ImageDataWithMaskCursorHandleProvider extends ImageDataCurs

public ImageDataWithMaskCursorHandleProvider(ImageData source, ImageData mask, int hotspotX, int hotspotY) {
super(source, hotspotX, hotspotY);
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);
}
this.mask = mask;
}

Expand Down
63 changes: 63 additions & 0 deletions ...t.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Cursor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@


import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand All @@ -28,6 +29,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;
Expand Down Expand Up @@ -172,6 +175,66 @@ public void test_ConstructorWithImageDataProvider() {
}
}

@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("IllegalArgumentException Expected: when wrong style was provided", IllegalArgumentException.class,
() -> {
Cursor cursor = new Cursor(Display.getDefault(), -99);
cursor.dispose();
});

assertThrows("IllegalArgumentException Expected: when source is null", IllegalArgumentException.class, () -> {
Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), null, mask, 0, 0);
cursorFromImageAndMask.dispose();
});

assertThrows("IllegalArgumentException Expected: 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("IllegalArgumentException Expected: 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("IllegalArgumentException Expected: when hotspot is outside the bounds of the image",
IllegalArgumentException.class, () -> {
Cursor cursorFromImageAndMask = new Cursor(Display.getDefault(), source, mask, 18, 18);
cursorFromImageAndMask.dispose();
});

assertThrows("IllegalArgumentException Expected: when source image data is null", IllegalArgumentException.class,
() -> {
ImageData nullImageData = null;
Cursor cursorFromSourceOnly = new Cursor(Display.getDefault(), nullImageData, 0, 0);
cursorFromSourceOnly.dispose();
});

assertThrows("IllegalArgumentException Expected: when ImageDataProvider is null", IllegalArgumentException.class,
() -> {
ImageDataProvider provider = null;
Cursor cursorFromProvider = new Cursor(Display.getDefault(), provider, 0, 0);
cursorFromProvider.dispose();
});

assertThrows("IllegalArgumentException Expected: 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.
Expand Down
Loading