From 462dbb366c77fabe03df9a358e0d7a2913b441cb Mon Sep 17 00:00:00 2001 From: "Klare, Heiko" Date: Fri, 10 Apr 2026 13:41:09 +0200 Subject: [PATCH 1/2] Simplify and clean up Cursor.java (Win32) Several small improvements to the Win32 Cursor implementation: - Replace double map-lookup and setHandleForZoomLevel() helper with a single HashMap.computeIfAbsent() call in win32_getHandle(). The helper method contained redundant null/containsKey guards that were already enforced by the call-site; both are now unnecessary. - Change win32_getHandle() return type from boxed Long to primitive long, consistent with Font.win32_getHandle(), Image.win32_getHandle() and Region.win32_getHandle(). All existing call sites already consumed the result as a long, so no callers required changes. - Fix latent equality bug in equals(): the previous code compared two Long wrapper objects with ==, which only gives correct results for cached values in [-128, 127]. Cursor handles are OS memory addresses and fall well outside that range. With the primitive return type the comparison is now a straightforward == on long values, which is both correct and avoids boxing. - Use Java 16+ instanceof pattern matching in equals() to eliminate the explicit cast after the type check. - Remove unnecessary clone() in ImageDataWithMaskCursorHandleProvider .validateMask(): the cloned ImageData was only ever read for its width/height fields, so the defensive copy was pure overhead. - Remove redundant 'static' modifier on the nested CursorHandleProvider interface declaration; nested interfaces are implicitly static. - Remove redundant 'final' modifier on the private static method getOSCursorIdFromStyle(); private static methods cannot be overridden. - Remove redundant 'final' modifier on the lambda-captured local variable in destroyHandlesExcept(); the variable is effectively final without the explicit keyword. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../org/eclipse/swt/graphics/Cursor.java | 40 +++++++------------ 1 file changed, 14 insertions(+), 26 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 a406ea1ee9..7f5efa65ce 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 @@ -316,25 +316,14 @@ public Cursor(Device device, ImageDataProvider imageDataProvider, int hotspotX, * * @noreference This method is not intended to be referenced by clients. */ -public static Long win32_getHandle (Cursor cursor, int zoom) { +public static long win32_getHandle (Cursor cursor, int zoom) { if (cursor.isDisposed()) { return 0L; } - int zoomWithPointerSizeScaleFactor = (int) (zoom * getPointerSizeScaleFactor()); - if (cursor.zoomLevelToHandle.get(zoomWithPointerSizeScaleFactor) != null) { - return cursor.zoomLevelToHandle.get(zoomWithPointerSizeScaleFactor).getHandle(); - } - - CursorHandle handle = cursor.cursorHandleProvider.createHandle(cursor.device, zoomWithPointerSizeScaleFactor); - cursor.setHandleForZoomLevel(handle, zoomWithPointerSizeScaleFactor); - - return cursor.zoomLevelToHandle.get(zoomWithPointerSizeScaleFactor).getHandle(); -} - -private void setHandleForZoomLevel(CursorHandle handle, Integer zoom) { - if (zoom != null && !zoomLevelToHandle.containsKey(zoom)) { - zoomLevelToHandle.put(zoom, handle); - } + int scaledZoom = (int) (zoom * getPointerSizeScaleFactor()); + return cursor.zoomLevelToHandle + .computeIfAbsent(scaledZoom, z -> cursor.cursorHandleProvider.createHandle(cursor.device, z)) + .getHandle(); } /** @@ -389,8 +378,7 @@ void destroy () { @Override public boolean equals (Object object) { if (object == this) return true; - if (!(object instanceof Cursor)) return false; - Cursor cursor = (Cursor) object; + if (!(object instanceof Cursor cursor)) return false; return device == cursor.device && win32_getHandle(this, DEFAULT_ZOOM) == win32_getHandle(cursor, DEFAULT_ZOOM); } @@ -406,7 +394,7 @@ public boolean equals (Object object) { */ @Override public int hashCode () { - return win32_getHandle(this, DEFAULT_ZOOM).intValue(); + return (int) win32_getHandle(this, DEFAULT_ZOOM); } /** @@ -439,7 +427,7 @@ public String toString () { @Override void destroyHandlesExcept(Set zoomLevels) { zoomLevelToHandle.entrySet().removeIf(entry -> { - final Integer zoom = entry.getKey(); + Integer zoom = entry.getKey(); if (!zoomLevels.contains(zoom) && zoom != DPIUtil.getZoomForAutoscaleProperty(DEFAULT_ZOOM)) { entry.getValue().destroy(); return true; @@ -492,7 +480,7 @@ void destroy() { } } -private static interface CursorHandleProvider { +private interface CursorHandleProvider { CursorHandle createHandle(Device device, int zoom); } @@ -513,7 +501,7 @@ public CursorHandle createHandle(Device device, int zoom) { return new CustomCursorHandle(handle); } - private static final long getOSCursorIdFromStyle(int style) { + private static long getOSCursorIdFromStyle(int style) { long lpCursorName = 0; switch (style) { case SWT.CURSOR_HAND: @@ -663,15 +651,15 @@ public ImageDataWithMaskCursorHandleProvider(ImageData source, ImageData mask, i } private void validateMask(ImageData source, ImageData mask) { - ImageData testMask = mask == null ? null : (ImageData) mask.clone(); - if (testMask == null) { + ImageData effectiveMask = mask; + if (effectiveMask == null) { if (source.getTransparencyType() != SWT.TRANSPARENCY_MASK) { SWT.error(SWT.ERROR_NULL_ARGUMENT); } - testMask = source.getTransparencyMask(); + effectiveMask = source.getTransparencyMask(); } /* Check the bounds. Mask must be the same size as source */ - if (testMask.width != source.width || testMask.height != source.height) { + if (effectiveMask.width != source.width || effectiveMask.height != source.height) { SWT.error(SWT.ERROR_INVALID_ARGUMENT); } } From 68ef2f3207c6a54fd22cc65fe67cf2b25013a8dd Mon Sep 17 00:00:00 2001 From: "Klare, Heiko" Date: Fri, 10 Apr 2026 13:50:52 +0200 Subject: [PATCH 2/2] Add Win32-specific tests for Cursor CursorWin32Tests covers behaviour that is either Windows-only or directly exercises the internal implementation paths changed in the preceding cleanup commit: - testDisposedCursorReturnsZeroHandle: verifies the isDisposed() guard at the top of win32_getHandle() returns 0L after disposal. - testHandleIsCachedForSameZoomLevel: verifies the computeIfAbsent() caching - two calls with the same zoom must return the identical cached OS handle without re-creating it. - testImageDataCursorProducesDifferentHandlesForDifferentZoomLevels: verifies that ImageDataCursorHandleProvider scales the source image and produces a distinct OS cursor handle for each zoom level. - testDestroyHandlesExceptPreservesRetainedHandle: verifies that destroyHandlesExcept() leaves the retained zoom entry intact and does not mark the cursor as disposed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../swt/graphics/CursorWin32Tests.java | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/CursorWin32Tests.java diff --git a/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/CursorWin32Tests.java b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/CursorWin32Tests.java new file mode 100644 index 0000000000..a30fef2663 --- /dev/null +++ b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/CursorWin32Tests.java @@ -0,0 +1,102 @@ +/******************************************************************************* + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.swt.graphics; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.*; + +import org.eclipse.swt.*; +import org.eclipse.swt.internal.*; +import org.eclipse.swt.widgets.*; +import org.junit.jupiter.api.*; +import org.junit.jupiter.api.extension.*; + +@ExtendWith(PlatformSpecificExecutionExtension.class) +@ExtendWith(WithMonitorSpecificScalingExtension.class) +class CursorWin32Tests { + + private Display display; + + @BeforeEach + void setUp() { + display = Display.getDefault(); + } + + @Test + void testDisposedCursorReturnsZeroHandle() { + Cursor cursor = new Cursor(display, SWT.CURSOR_ARROW); + cursor.dispose(); + assertEquals(0L, Cursor.win32_getHandle(cursor, 100), + "A disposed cursor must return a zero handle"); + } + + @Test + void testHandleIsCachedForSameZoomLevel() { + ImageData source = new ImageData(16, 16, 32, + new PaletteData(0xFF00, 0xFF0000, 0xFF000000)); + source.alpha = 255; + + Cursor cursor = new Cursor(display, source, 0, 0); + try { + long first = Cursor.win32_getHandle(cursor, 100); + long second = Cursor.win32_getHandle(cursor, 100); + assertEquals(first, second, + "Repeated calls with the same zoom must return the cached handle"); + } finally { + cursor.dispose(); + } + } + + @Test + void testImageDataCursorProducesDifferentHandlesForDifferentZoomLevels() { + // 32bpp image with uniform alpha — takes the ARGB path in setupCursorFromImageData + ImageData source = new ImageData(16, 16, 32, + new PaletteData(0xFF00, 0xFF0000, 0xFF000000)); + source.alpha = 255; + + Cursor cursor = new Cursor(display, source, 0, 0); + try { + long handle100 = Cursor.win32_getHandle(cursor, 100); + long handle200 = Cursor.win32_getHandle(cursor, 200); + + assertNotEquals(0L, handle100, "Handle at 100% zoom must be non-zero"); + assertNotEquals(0L, handle200, "Handle at 200% zoom must be non-zero"); + assertNotEquals(handle100, handle200, + "Different zoom levels must produce distinct OS cursor handles (different physical sizes)"); + } finally { + cursor.dispose(); + } + } + + @Test + void testDestroyHandlesExceptPreservesRetainedHandle() { + // 32bpp ARGB source so we get a unique, non-shared OS handle per zoom level + ImageData source = new ImageData(16, 16, 32, + new PaletteData(0xFF00, 0xFF0000, 0xFF000000)); + source.alpha = 255; + + Cursor cursor = new Cursor(display, source, 0, 0); + try { + long handle100 = Cursor.win32_getHandle(cursor, 100); + Cursor.win32_getHandle(cursor, 200); // populate a second zoom level + + cursor.destroyHandlesExcept(Set.of(DPIUtil.getZoomForAutoscaleProperty(100))); + + // The cursor itself must still be alive and the retained handle unchanged + assertFalse(cursor.isDisposed(), "Cursor must not be disposed after destroyHandlesExcept"); + assertEquals(handle100, Cursor.win32_getHandle(cursor, 100), + "The handle for the retained zoom level must be unchanged after destroyHandlesExcept"); + } finally { + cursor.dispose(); + } + } +}