IllegalArgumentException for GMF cursors if display zoom is not 100%/200%

[ptziegler]

See eclipse-emf/org.eclipse.emf#73 for context. The SEG_ADD and SEG_MOVE cursors are created using the following snippet:

gmf-runtime/bundles/org.eclipse.gmf.runtime.gef.ui/src/org/eclipse/gmf/runtime/gef/ui/internal/l10n/Cursors.java

Lines 39 to 42 in f8ad590

	static {
	CURSOR_SEG_ADD = new Cursor(null, GefUIPluginImages.DESC_SEG_ADD.getImageData(getDeviceZoom()), 0, 0);
	CURSOR_SEG_MOVE = new Cursor(null, GefUIPluginImages.DESC_SEG_MOVE.getImageData(getDeviceZoom()), 0, 0);
	}

The problem: The source are two PNG files for 100% and 200% display zoom, respectively. If the zoom level is e.g. 125%, getImageData(125) returns null, which then triggers an IllegalArgumentException by SWT.

Suggestion:

Check if the image data for the given zoom level is null and if so, artificially scale it.

public static ImageData scaledImageData(ImageDescriptor descriptor, int zoom) {
    // Default case: Image in matching resolution has been found
    ImageData data = descriptor.getImageData(zoom);
    if (data != null) {
        return data;
    }
    // Otherwise artifically scale the image
    Image image = descriptor.createImage();
    try {
        return image.getImageData(zoom);
    } finally {
        image.dispose();
    }
}

Though I believe the provided ImageData should generally be at 100% zoom. Otherwise the scaling is done twice: Once by GMF and once by SWT (which would then lead to e.g. a 400% zoom cursor at 200% zoom).

[pcdavid]

Thanks for reporting this here. I'll have a look, but probably not before next week.

[pcdavid]

@ptziegler I've created #92 with your proposed fix, but I'm blind here. I can't reproduce the issue myself, it seems that under Linux (Wayland at least), when I set my display zoom level to e.g. 125%, it's actually set internally to 200% (org.eclipse.gmf.runtime.gef.ui.internal.l10n.Cursors.getDeviceZoom() returns 200). My undestanding is that in this case the Wayland compositor tells applications they are running at 200% and downscales the result appropriatly itself.

[ptziegler]

As far as I understand the SWT implementation, smooth scaling is primarily implemented for Windows. On Linux/Mac, it is likely still rounded up to next multiple of 100, as you said. I think @HeikoKlare would be able to go into more detail when and how the images are scaled.

But at least for Windows, SWT expects the image data for the cursor to be at 100% zoom and then artificially scales it to the device zoom. Proper scaling was only implemented with eclipse-platform/eclipse.platform.swt#2271 when providing the constructor with an ÌmageDataProvider.

We used to do the same as #92 for GEF, but then we quickly ran into issues where the cursors were twice as large, because the Cursor class doesn't know whether the image data is pre-scaled or not.

Note: If I understand this comment correctly, cursors weren't scaled at all on Linux. So the current behavior sounds like a bug in SWT and you'd also see the twice-as-large cursor icons on Linux, with the next Eclipse version.

Previously, cursors were initialized with a single ImageData, which caused issues on systems with varying zoom levels, for example, cursors were not scaled at all on Linux, or were blurrily auto-scaled on Windows.

To summarize:
I now think there should never be a need for clients to check the "device zoom" system property and one simply has to always pass the image data at 100% zoom. At least with the next Eclipse version.

[pcdavid]

I see. It's a little confusing given all the possible combinations of Eclipse versions and platforms (we try to keep support for older versions of Eclipse in GMF and Sirius, but it adds a lot of overhead).

I notice you've started to move to SVG in GEF, bug the rasterizer seems to have it own quirks, which adds another layer...

[ptziegler]

It's a little confusing given all the possible combinations of Eclipse versions and platforms (we try to keep support for older versions of Eclipse in GMF and Sirius, but it adds a lot of overhead).

The same is also the case for GEF, where I insisted on keeping the support for the 2024-09. But the HighDPI functionality makes this quite a challenge.

I think in general, the new constructor with the ImageDataProvider that will be added with the 2025-09 is the most stable solution., which is also what we switched to in GEF. To remain backwards compatible, we only access this constructor via reflection and fall back to the previous behavior, when running on older Eclipse versions.

So my proposal is to structure it as follows:

For >= 2025-09, use the new constructor with the ImageDataProvider
For older versions, scale icons at 100% for Windows (when monitor-specific UI scaling is enabled), otherwise use the device zoom

I can try to draft a PR later this afternoon.

bug the rasterizer seems to have it own quirks, which adds another layer...

Yes, that sadly seems to be an edge-case where jSVG doesn't properly handle some of the SVG attributes. There are ways around this and it's probably possible to integrate the cursor into the SVG but in the end, I simply got fed up.

[ptziegler]

I've player around for a while and depending on whether you use the Eclipse 2024-12, 2025-03, 2025-06 or 2025-09, you get different result. Expect all results starting with the 2025-03 are wrong, always in a different way.

See eclipse-platform/eclipse.platform.swt#2308 and eclipse-platform/eclipse.platform.swt#2309 for details. I think the SWT team needs to explain to me how this is supposed to work, because I get the impression that it doesn't. Unless you use SVGs and the new API, that is.

Or to rephrase: Unless you target the 2025-09 where you can use the ImageDataProvider constructor, you either get a cursor with the wrong size, or no cursor at all... All on Windows, of course.

[ptziegler]

@pcdavid FYI: #93

[HeikoKlare]

So my proposal is to structure it as follows:

For >= 2025-09, use the new constructor with the ImageDataProvider
For older versions, scale icons at 100% for Windows (when monitor-specific UI scaling is enabled), otherwise use the device zoom

I think that's the best that can be achieved. Unfortunately, there is a bunch of issues involved here, which makes it hard to distangle everything. But it boils down to two things:

1. Cursor API: cursors were not properly made for HiDPI support (not even before the monitor-specific support was introduced). A cursors was created based on a single image data at some specific zoom. It's even undocumented at which zoom that image data is supposed to be. That's a limitation that was now removed by introducing a constructor based on an ImageDataProvider but if compatibilits with older SWT releases is desired, you obviously need to still rely on or at least support usage of the old constructors as a fallback.
2. Cursor zoom determination in Win32 SWT implementation: as Patrick pointed out, the Win32 implementation does not use cursors at the proper size when used inside some widget. It seems to have been broken in different ways throughout the recent releases (see Cursors not correctly sized when using swt.autoScale on a HighDPI screen. eclipse-platform/eclipse.platform.swt#2308). It seems quite clear to me what is currently broken (as documented in the issue), so we will fix that issue, but I am not sure about all the different ways in which it was broken in previous releases.

On Linux nothing of this is really an issue as there is not much auto-scaling done by SWT in place anymore. Most scaling is done natively, so that SWT does not have to care about it (except for using images in proper sizes).

Regarding limited capabilities of JSVG, note that the project is open-source and one can also raise issues or even propose fixes for missing behavior: https://github.com/weisJ/jsvg
We can then update the JSVG version deployed with SWT once required features are in there.

[pcdavid]

Thanks @ptziegler and @HeikoKlare for the time taken investigating this.
I'm busy with other projects at the moment but I'll still try to have a look soon-ish (hopefully this week, but that's not 100% sure).