Skip to content

Commit

Permalink
Fix errors in drag cursors when using high-DPI screens
Browse files Browse the repository at this point in the history 
The cursors used in the flyout palette only exist for the default 100%.
If a system is using a higher zoom level, the creation of those cursors
fails with an IllegalArgumentException. Instead, we should instead scale
the cursor image to match the current level.

This change moves the code for detecting the device zoom from the
SharedCursors class to the InternalGEFPlugin, so that it becomes
available to the DragCursors class. From there, we call
Image.getImageData(...) instead of ImageDescriptor.getImageData(), in
order to get a scaled ImageData object via DPIUtil magic.

See #464
  • Loading branch information
@ptziegler
ptziegler committed Jul 16, 2024
1 parent 46ecf6f commit 8963cef
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 45 deletions.
30 changes: 5 additions & 25 deletions org.eclipse.gef/src/org/eclipse/gef/SharedCursors.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2010 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
Expand All @@ -13,12 +13,14 @@
package org.eclipse.gef;

import org.eclipse.swt.graphics.Cursor;
import org.eclipse.swt.graphics.Image;

import org.eclipse.jface.resource.ImageDescriptor;
import org.eclipse.core.runtime.Platform;

import org.eclipse.draw2d.Cursors;

import org.eclipse.gef.internal.Internal;
import org.eclipse.gef.internal.InternalGEFPlugin;

/**
* A shared collection of Cursors.
Expand All @@ -44,8 +46,6 @@ public class SharedCursors extends Cursors {
*/
public static final Cursor CURSOR_TREE_MOVE;

private static int deviceZoom = -1;

static {
CURSOR_PLUG = createCursor("icons/plug-cursor.png"); //$NON-NLS-1$
CURSOR_PLUG_NOT = createCursor("icons/plugnot-cursor.png"); //$NON-NLS-1$
Expand All @@ -55,26 +55,6 @@ public class SharedCursors extends Cursors {

private static Cursor createCursor(String sourceName) {
ImageDescriptor src = ImageDescriptor.createFromFile(Internal.class, sourceName);
return new Cursor(null, src.getImageData(getDeviceZoom()), 0, 0);
return new Cursor(null, src.getImageData(InternalGEFPlugin.getDeviceZoom()), 0, 0);
}

private static int getDeviceZoom() {
if (deviceZoom == -1) {
deviceZoom = 100; // default value
String deviceZoomProperty = System.getProperty("org.eclipse.swt.internal.deviceZoom"); //$NON-NLS-1$
if (deviceZoomProperty != null) {
try {
deviceZoom = Integer.parseInt(deviceZoomProperty);
} catch (NumberFormatException ex) {
// if the property can not be parsed we keep the default 100% zoom level
}
}
}
// On Mac and Linux X11 ImageData for cursors should always be created with 100% device zoom
return Platform.getOS().equals(Platform.OS_MACOSX) ||
(Platform.getOS().equals(Platform.OS_LINUX) && "x11".equalsIgnoreCase(System.getenv("XDG_SESSION_TYPE"))) //$NON-NLS-1$ //$NON-NLS-2$
? 100
: deviceZoom;
}

}
22 changes: 22 additions & 0 deletions org.eclipse.gef/src/org/eclipse/gef/internal/InternalGEFPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

package org.eclipse.gef.internal;

import org.eclipse.core.runtime.Platform;
import org.eclipse.jface.resource.ImageRegistry;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.plugin.AbstractUIPlugin;
Expand All @@ -28,6 +29,7 @@ public class InternalGEFPlugin extends AbstractUIPlugin {

private static BundleContext context;
private static AbstractUIPlugin singleton;
private static int deviceZoom = -1;

public InternalGEFPlugin() {
singleton = this;
Expand Down Expand Up @@ -58,6 +60,26 @@ public static AbstractUIPlugin getDefault() {
return singleton;
}

public static int getDeviceZoom() {
if (deviceZoom == -1) {
deviceZoom = 100; // default value
String deviceZoomProperty = System.getProperty("org.eclipse.swt.internal.deviceZoom"); //$NON-NLS-1$
if (deviceZoomProperty != null) {
try {
deviceZoom = Integer.parseInt(deviceZoomProperty);
} catch (NumberFormatException ex) {
// if the property can not be parsed we keep the default 100% zoom level
}
}
}
// On Mac and Linux X11 ImageData for cursors should always be created with 100%
// device zoom
return Platform.getOS().equals(Platform.OS_MACOSX) || (Platform.getOS().equals(Platform.OS_LINUX)
&& "x11".equalsIgnoreCase(System.getenv("XDG_SESSION_TYPE"))) //$NON-NLS-1$ //$NON-NLS-2$
? 100
: deviceZoom;
}

/**
* @see org.osgi.framework.BundleActivator#stop(org.osgi.framework.BundleContext)
*/
Expand Down
42 changes: 22 additions & 20 deletions org.eclipse.gef/src/org/eclipse/gef/ui/palette/FlyoutPaletteComposite.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2004, 2010 IBM Corporation and others.
* Copyright (c) 2004, 2024 IBM Corporation and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
Expand Down Expand Up @@ -32,6 +32,8 @@
import org.eclipse.swt.graphics.Cursor;
import org.eclipse.swt.graphics.Font;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.ImageData;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.Rectangle;
import org.eclipse.swt.widgets.Canvas;
Expand All @@ -51,7 +53,6 @@
import org.eclipse.jface.action.IAction;
import org.eclipse.jface.action.IContributionItem;
import org.eclipse.jface.action.MenuManager;
import org.eclipse.jface.resource.ImageDescriptor;
import org.eclipse.jface.resource.JFaceResources;
import org.eclipse.jface.util.IPropertyChangeListener;
import org.eclipse.jface.util.TransferDropTargetListener;
Expand Down Expand Up @@ -84,6 +85,7 @@
import org.eclipse.gef.GraphicalViewer;
import org.eclipse.gef.dnd.TemplateTransfer;
import org.eclipse.gef.internal.GEFMessages;
import org.eclipse.gef.internal.InternalGEFPlugin;
import org.eclipse.gef.internal.InternalImages;
import org.eclipse.gef.internal.ui.palette.PaletteColorUtil;
import org.eclipse.gef.ui.views.palette.PaletteView;
Expand Down Expand Up @@ -1557,37 +1559,37 @@ private static class DragCursors {
* @return the cursor
*/
public static Cursor getCursor(int code) {
Display display = Display.getCurrent();
if (cursors[code] == null) {
ImageDescriptor source = null;
ImageDescriptor mask = null;
switch (code) {
case LEFT:
source = PlatformUI.getWorkbench().getSharedImages()
.getImageDescriptor(ISharedImages.IMG_OBJS_DND_LEFT_SOURCE);
mask = PlatformUI.getWorkbench().getSharedImages()
.getImageDescriptor(ISharedImages.IMG_OBJS_DND_LEFT_MASK);
cursors[LEFT] = new Cursor(display, source.getImageData(), mask.getImageData(), 16, 16);
cursors[LEFT] = createCursor(ISharedImages.IMG_OBJS_DND_LEFT_SOURCE,
ISharedImages.IMG_OBJS_DND_LEFT_MASK);
break;
case RIGHT:
source = PlatformUI.getWorkbench().getSharedImages()
.getImageDescriptor(ISharedImages.IMG_OBJS_DND_RIGHT_SOURCE);
mask = PlatformUI.getWorkbench().getSharedImages()
.getImageDescriptor(ISharedImages.IMG_OBJS_DND_RIGHT_MASK);
cursors[RIGHT] = new Cursor(display, source.getImageData(), mask.getImageData(), 16, 16);
cursors[RIGHT] = createCursor(ISharedImages.IMG_OBJS_DND_RIGHT_SOURCE,
ISharedImages.IMG_OBJS_DND_RIGHT_MASK);
break;
default:
case INVALID:
source = PlatformUI.getWorkbench().getSharedImages()
.getImageDescriptor(ISharedImages.IMG_OBJS_DND_INVALID_SOURCE);
mask = PlatformUI.getWorkbench().getSharedImages()
.getImageDescriptor(ISharedImages.IMG_OBJS_DND_INVALID_MASK);
cursors[INVALID] = new Cursor(display, source.getImageData(), mask.getImageData(), 16, 16);
cursors[INVALID] = createCursor(ISharedImages.IMG_OBJS_DND_INVALID_SOURCE,
ISharedImages.IMG_OBJS_DND_INVALID_MASK);
break;
}
}
return cursors[code];
}

private static Cursor createCursor(String sourceName, String maskName) {
Image source = PlatformUI.getWorkbench().getSharedImages().getImage(sourceName);
Image mask = PlatformUI.getWorkbench().getSharedImages().getImage(maskName);
// Scales the image if the display is using neither 100% nor 200% zoom
ImageData sourceData = source.getImageData(InternalGEFPlugin.getDeviceZoom());
ImageData maskData = mask.getImageData(InternalGEFPlugin.getDeviceZoom());
// Hotspot should be the center of the image. e.g. (16, 16) on 100% zoom
int hotspotX = sourceData.width / 2;
int hotspotY = sourceData.height / 2;
return new Cursor(null, sourceData, maskData, hotspotX, hotspotY);
}

}
}

0 comments on commit 8963cef

Please sign in to comment.