IllegalArgumentException: Argument cannot be null when creating SharedCursor

[Phillipus]

Cursors for the Palette are created in the SharedCursors class. The ImageData is returned from the ImageDescriptor at the current device zoom level. But if the zoom level is not 100% or 200% then ImageData will be null and an exception is thrown.

1. On Windows launch a GEF-based app with a Palette with the VM argument -Dswt.autoScale=150
2. Select a tool from the palette

This is caused by the ImageDescriptor not finding an image with a @1.5x suffix.

Perhaps a fallback is required so that if ImageData is null 100% zoom is used instead?

[Phillipus]

Or another possibility is to get the ImageData from an Image rather than an ImageDescriptor as that will not be null:

    private static Cursor createCursor(String sourceName) {
        // ImageDescriptor will load @2x image
        ImageDescriptor src = ImageDescriptor.createFromFile(Internal.class, sourceName);
        Image img = src.createImage();        
        ImageData id = img.getImageData(getDeviceZoom());
        img.dispose();
        return new Cursor(null, id, 0, 0);
    }

[Phillipus]

There are other cursors created in FlyoutPaletteComposite here - https://github.com/eclipse/gef-classic/blob/e1d163b08c72039ce96efd2692edbda1ff06d6f9/org.eclipse.gef/src/org/eclipse/gef/ui/palette/FlyoutPaletteComposite.java#L1540

Because these are created with ImageDescriptor.getImageData() with no zoom level they are too small on Windows at hi-dpi 200% scale. These would need to be changed as well.

[ptziegler]

I agree that throwing an exception is definitely not the way to go. My initial approach would've been to fall back to the 100% zoom level, as you suggested. Alternatively, one could also make it so that it goes to the "closest" zoom level instead. e.g. from 125% to 100% and from 175% to 200%.

private static Cursor createCursor(String sourceName) {
    ImageDescriptor src = ImageDescriptor.createFromFile(Internal.class, sourceName);
    ImageData data = src.getImageData(getDeviceZoom());
    if (data == null) {
        // if no image for that zoom level exists use the one for the default zoom level
        data = src.getImageData(100);
    }
    return new Cursor(null, data, 0, 0);
}

Or another possibility is to get the ImageData from an Image rather than an ImageDescriptor as that will not be null:

    private static Cursor createCursor(String sourceName) {
        // ImageDescriptor will load @2x image
        ImageDescriptor src = ImageDescriptor.createFromFile(Internal.class, sourceName);
        Image img = src.createImage();        
        ImageData id = img.getImageData(getDeviceZoom());
        img.dispose();
        return new Cursor(null, id, 0, 0);
    }

Wouldn't that cause the cursor to be scaled to the current zoom level? That might also work, though I'm a little concerned how good those scaled cursors will look...

Because these are created with ImageDescriptor.getImageData() with no zoom level they are too small on Windows at hi-dpi 200% scale. These would need to be changed as well.

Apparently, those icons come from here and haven't been updated in... 21 years. Yikes. https://github.com/eclipse-platform/eclipse.platform.ui/blob/master/bundles/org.eclipse.ui/icons/full/pointer/

There are no @2x variants for those icons, so we would have to replace them with something else entirely.

[Phillipus]

Wouldn't that cause the cursor to be scaled to the current zoom level? That might also work, though I'm a little concerned how good those scaled cursors will look...

Yep, and they look a bit scrunched. But then so do the icons in the palette at that setting so they complement each other ;-).

I experimented with things like "if displayZoom <= 150 then use 100" and "if displayZoom > 150 then use 200" but if the setting is -Dswt.autoScale=300 then even the double size cursors are too small relative to everything else in the UI. Icons are scaled by SWT's DPIUtil magic to match the scale setting, so I thought maybe it should be the same for cursors.