Use OS.CreateIconIndirect for colored cursors with source and mask

[ShahzaibIbrahim]

Use OS.CreateIconIndirect for colored cursors with source and mask

Previously, the Cursor constructor that accepted both source and mask used OS.CreateCursor, which only supports monochrome cursors. As a result, any color information in the source was ignored and the cursor always appeared black.

This change updates the constructor to use OS.CreateIconIndirect, allowing full-color cursors while still respecting the mask for transparency. DPI scaling of the source and mask now works correctly with colored cursors.

How to Test

Run the following snippet:

Snippet386

import org.eclipse.swt.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.widgets.*;

public class Snippet386 {

	private static final int IMAGE_SIZE_IN_POINTS = 16;

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = createShell(display);

		Combo combo = createConstructorCombo(shell);
		Label zoomLabel = createZoomLabel(shell);

		addZoomChangedListener(shell, zoomLabel);
		Group section = new Group(shell, SWT.NONE);
		section.setText("Scale");
		section.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
		section.setLayout(new FillLayout());
		addPaintTicks(section);

		CursorManager cursorManager = new CursorManager(display, shell, combo);
		combo.addListener(SWT.Selection, e -> cursorManager.updateCursor());
		cursorManager.updateCursor();

		shell.setSize(400, 400);
		shell.open();

		eventLoop(display, shell);
		display.dispose();
	}

	private static Shell createShell(Display display) {
		Shell shell = new Shell(display);
		shell.setText("Snippet 386");
		shell.setLayout(new GridLayout(1, false));
		return shell;
	}

	private static Combo createConstructorCombo(Composite parent) {
		Label label = new Label(parent, SWT.NONE);
		label.setText("Choose Cursor Constructor:");

		Combo combo = new Combo(parent, SWT.READ_ONLY);
		combo.setItems("Cursor(Device, int)", "Cursor(Device, ImageData, ImageData, int, int)",
				"Cursor(Device, ImageData, int, int)", "Cursor(Device, ImageDataProvider, int, int)");
		combo.select(0);
		return combo;
	}

	private static Label createZoomLabel(Shell parent) {
		Label zoomLabel = new Label(parent, SWT.NONE);
		zoomLabel.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false));
		setZoomLabelText(parent, zoomLabel);
		return zoomLabel;
	}

	private static void setZoomLabelText(Shell shell, Label label) {
		int zoom = shell.getMonitor().getZoom();
		int expectedCursorSize = Math.round(IMAGE_SIZE_IN_POINTS * (zoom / 100f));
		label.setText("Current zoom: " + zoom + "% \nExpected Cursor Size = " + expectedCursorSize);
	}

	private static void addZoomChangedListener(Shell shell, Label zoomLabel) {
		shell.addListener(SWT.Resize, event -> {
			setZoomLabelText(shell, zoomLabel);
			shell.layout();
		});
	}

	private static void addPaintTicks(Composite composite) {
		composite.addPaintListener(event -> {
			drawTicks(composite, event.gc);
		});
	}

	private static void drawTicks(Composite shell, GC gc) {
		int deviceZoom = shell.getMonitor().getZoom();
		float devScale = deviceZoom / 100f;
		Point client = shell.getSize();
		int xPos = (int) ((client.x / 2) - (6 * (IMAGE_SIZE_IN_POINTS / devScale)));
		int tickHeight = 10;
		int yPos = 20;

		for (int tickIndex = 0; tickIndex < 6; tickIndex++) {
			xPos += (IMAGE_SIZE_IN_POINTS / devScale);
			int yOffset = (tickIndex % 3 == 1) ? 20 : (tickIndex % 3 == 2) ? 40 : 0;

			gc.drawLine(xPos, yPos, xPos, yPos + tickHeight);
			gc.drawText(Integer.toString(tickIndex * IMAGE_SIZE_IN_POINTS), xPos - 5, yPos + 12 + yOffset);
		}
	}

	private static void eventLoop(Display display, Shell shell) {
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch())
				display.sleep();
		}
	}

	public static ImageData createSolidColorImageData(int size, RGB color) {
		PaletteData palette = new PaletteData(0xFF0000, 0x00FF00, 0x0000FF);
		ImageData imageData = new ImageData(size, size, 24, palette);

		int pixel = palette.getPixel(color);
		for (int y = 0; y < size; y++) {
			for (int x = 0; x < size; x++) {
				imageData.setPixel(x, y, pixel);
			}
		}
		return imageData;
	}

	private static class CursorManager {
		private final Display display;
		private final Shell shell;
		private final Combo combo;

		CursorManager(Display display, Shell shell, Combo combo) {
			this.display = display;
			this.shell = shell;
			this.combo = combo;
		}

		void updateCursor() {
			int selection = combo.getSelectionIndex();
			Cursor oldCursor = shell.getCursor();
			if (oldCursor != null && !oldCursor.isDisposed()) {
				oldCursor.dispose();
			}
			Cursor cursor = createCursor(selection);
			if (cursor != null) {
				shell.setCursor(cursor);
			}
		}

		private Cursor createCursor(int selection) {
			switch (selection) {
			case 0:
				return new Cursor(display, SWT.CURSOR_HAND);
			case 1: {
				PaletteData rgbPalette = new PaletteData(0xFF0000, 0x00FF00, 0x0000FF);
				int bluePixel = rgbPalette.getPixel(new RGB(0, 0, 255));
				ImageData source = new ImageData(IMAGE_SIZE_IN_POINTS, IMAGE_SIZE_IN_POINTS, 24, new PaletteData(0xFF0000, 0x00FF00, 0x0000FF));

				for (int x = 0; x < IMAGE_SIZE_IN_POINTS; x++) {
					for (int y = 0; y < IMAGE_SIZE_IN_POINTS; y++) {
						source.setPixel(x, y, bluePixel);
					}
				}

				ImageData mask = new ImageData(IMAGE_SIZE_IN_POINTS, IMAGE_SIZE_IN_POINTS, 1, new PaletteData(new RGB[] {new RGB(0,0,0), new RGB(255,255,255)}));
				for (int x = 0; x < IMAGE_SIZE_IN_POINTS; x++) {
					for (int y = 0; y < IMAGE_SIZE_IN_POINTS; y++) {
						mask.setPixel(x, y, x % 2);
					}
				}

				return new Cursor(display, source, mask, IMAGE_SIZE_IN_POINTS / 2, IMAGE_SIZE_IN_POINTS / 2);
			}
			case 2: {
				RGB red = new RGB(255, 0, 0);
				return new Cursor(display, createSolidColorImageData(IMAGE_SIZE_IN_POINTS, red), 0, 0);
			}
			case 3: {
				RGB green = new RGB(0, 255, 0);
				ImageDataProvider provider = zoom -> {
					return createSolidColorImageData(IMAGE_SIZE_IN_POINTS * zoom / 100, green);
				};
				return new Cursor(display, provider, 0, 0);
			}
			default:
				return null;
			}
		}
	}
}

• Select the Cursor(Device, ImageData, ImageData, int, int) value from drop down
• See if the cursor is blue and transparency is also applied.

Result

Before:

After:

Requires

• SWT: Respect Windows accessibility cursor size setting #2493

[github-actions]

Test Results

  118 files  ±0    118 suites  ±0   10m 54s ⏱️ +48s
4 432 tests ±0  4 410 ✅  - 1  17 💤 ±0  5 ❌ +1 
  298 runs  ±0    289 ✅  - 1   4 💤 ±0  5 ❌ +1 

For more details on these failures, see this check.

Results for commit f271319. ± Comparison against base commit 35119f0.

♻️ This comment has been updated with latest results.

[ShahzaibIbrahim]

Failing test are unrelated, see #2516

[HeikoKlare]

I have some concerns about this change:

• It introduces unnecessary complexity to setupCursorFromIamgeData as that internal utility method is made capable of handling cases that can not occur (or at least that were also not supported) by the existing API.
• It change the return type of getPointerSizeScaleFactor() without any obvious need, increasing the complexity of consumer code.
• The code is not properly formatted.

[HeikoKlare]

Why change the return type of getPointerSizeScaleFactor() to float and have all these integer conversions / roundings in this method?

[fedejeanne]

See #2517 (comment)

[HeikoKlare]

Thanks for the pointer. But then it's an issue unrelated to this PR but an issue with the existing implementation, isn't it? So please extract an appropriate PR for a separate issue out of this.

[fedejeanne]

I think we could simply fix it here. The method was introduced 2 weeks ago in 6e4241d. That's probably why nobody noticed so far.

[HeikoKlare]

The issue is unrelated to this PR, so please have separate PRs for unrelated concerns.

[HeikoKlare]

If I am not mistaken, this will apply the mask data as a mask to itself, then extract the mask data out of the combined image to finally just execute OS.CreateBitmap(i.width, i.height, 1, 1, maskData). Why not directly do that without all the unnecessary complexity?

[HeikoKlare]

And if you pass mask data, why should the source data also contain alpha information? This was not supported by the existing API, so I don't think we should implement it here. It should be valid to assume that if a mask is passed the source does not contain mask data. I think we should even deprecate the source+mask constructor and indicate that better other constructors taking transparency-aware data or even an ImageDataProvider should be used.

[fedejeanne]

My comment was addressed 👍
See Heiko's comments