From af9dd04bd6bc1e41b77859c5487ed8367a564edd Mon Sep 17 00:00:00 2001 From: Amartya Parijat Date: Mon, 3 Mar 2025 14:27:47 +0100 Subject: [PATCH] Image Rounding Error with smooth scaling #62 This commit contributes to fixing the logic for scaling ImageData with Smooth scaling strategy. The logic of DPIUtil::autoScaleImageData is replicated in the Image class and modified to have properly defined current and target zoom. The previous implementation scales the bounds of image up and down several times which can lead to rounding error in case of scaling factor being a fractional value. With this implementation, the obtained imageData has no rounding error after ruling out those scale ups and downs and hence improves the menu item icons. contributes to https://github.com/eclipse-platform/eclipse.platform.swt/issues/62 and https://github.com/eclipse-platform/eclipse.platform.swt/issues/127 --- .../org/eclipse/swt/internal/win32/OS.java | 1 + .../org/eclipse/swt/internal/DPIUtil.java | 18 +++++- .../win32/org/eclipse/swt/graphics/Image.java | 61 +++++++++++++++---- .../org/eclipse/swt/widgets/MenuItem.java | 13 +++- 4 files changed, 78 insertions(+), 15 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java index 444bbb54250..7ab15ed3b32 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java @@ -1246,6 +1246,7 @@ public class OS extends C { public static final int SM_CYFOCUSBORDER = 84; public static final int SM_CYHSCROLL = 0x3; public static final int SM_CYMENU = 0xf; + public static final int SM_CYMENUCHECK = 72; public static final int SM_CXMINTRACK = 34; public static final int SM_CYMINTRACK = 35; public static final int SM_CXMAXTRACK = 59; diff --git a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/DPIUtil.java b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/DPIUtil.java index 0f08710d468..a5ad1846cde 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/DPIUtil.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/DPIUtil.java @@ -292,7 +292,7 @@ private static ImageData autoScaleImageData (Device device, final ImageData imag int height = imageData.height; int scaledWidth = Math.round (width * scaleFactor); int scaledHeight = Math.round (height * scaleFactor); - boolean useSmoothScaling = autoScaleMethod == AutoScaleMethod.SMOOTH && imageData.getTransparencyType() != SWT.TRANSPARENCY_MASK; + boolean useSmoothScaling = isSmoothScalingEnabled() && imageData.getTransparencyType() != SWT.TRANSPARENCY_MASK; if (useSmoothScaling) { Image original = new Image (device, (ImageDataProvider) zoom -> imageData); /* Create a 24 bit image data with alpha channel */ @@ -316,6 +316,10 @@ private static ImageData autoScaleImageData (Device device, final ImageData imag } } +public static boolean isSmoothScalingEnabled() { + return autoScaleMethod == AutoScaleMethod.SMOOTH; +} + /** * Returns a new rectangle as per the scaleFactor. */ @@ -631,7 +635,19 @@ public static boolean useCairoAutoScale() { return useCairoAutoScale; } +public static int getZoomForMenuItemImage(int nativeDeviceZoom) { + String autoScaleValueForMenuItemImage = DPIUtil.autoScaleValue; + if(autoScaleValueForMenuItemImage.equals("quarter") || autoScaleValueForMenuItemImage.equals("exact")) { + autoScaleValueForMenuItemImage = "half"; + } + return getZoomForAutoscaleProperty(nativeDeviceZoom, autoScaleValueForMenuItemImage); +} + public static int getZoomForAutoscaleProperty (int nativeDeviceZoom) { + return getZoomForAutoscaleProperty(nativeDeviceZoom, autoScaleValue); +} + +private static int getZoomForAutoscaleProperty (int nativeDeviceZoom, String autoScaleValue) { int zoom = 0; if (autoScaleValue != null) { if ("false".equalsIgnoreCase (autoScaleValue)) { diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java index e948c14f217..be39fdbfdb7 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java @@ -22,6 +22,7 @@ import org.eclipse.swt.internal.DPIUtil.*; import org.eclipse.swt.internal.gdip.*; import org.eclipse.swt.internal.win32.*; +import org.eclipse.swt.widgets.*; /** * Instances of this class are graphics which have been prepared @@ -363,7 +364,7 @@ public Image(Device device, ImageData data) { super(device); if (data == null) SWT.error(SWT.ERROR_NULL_ARGUMENT); initialNativeZoom = DPIUtil.getNativeDeviceZoom(); - data = DPIUtil.autoScaleUp(device, new ElementAtZoom<>(data, 100)); + data = scaleImageData(device, 100, getZoom(), data); init(data, getZoom()); init(); this.device.registerResourceWithZoomSupport(this); @@ -407,8 +408,8 @@ public Image(Device device, ImageData source, ImageData mask) { SWT.error(SWT.ERROR_INVALID_ARGUMENT); } initialNativeZoom = DPIUtil.getNativeDeviceZoom(); - source = DPIUtil.autoScaleUp(device, source); - mask = DPIUtil.autoScaleUp(device, mask); + source = scaleImageData(device, 100, getZoom(), source); + mask = scaleImageData(device, 100, getZoom(), mask); mask = ImageData.convertMask(mask); init(this.device, this, source, mask, getZoom()); init(); @@ -471,7 +472,7 @@ public Image(Device device, ImageData source, ImageData mask) { public Image (Device device, InputStream stream) { super(device); initialNativeZoom = DPIUtil.getNativeDeviceZoom(); - ImageData data = DPIUtil.autoScaleUp(device, new ElementAtZoom<>(new ImageData (stream), 100)); + ImageData data = scaleImageData(device, 100, getZoom(), new ImageData (stream)); init(data, getZoom()); init(); this.device.registerResourceWithZoomSupport(this); @@ -513,7 +514,7 @@ public Image (Device device, String filename) { super(device); if (filename == null) SWT.error(SWT.ERROR_NULL_ARGUMENT); initialNativeZoom = DPIUtil.getNativeDeviceZoom(); - ImageData data = DPIUtil.autoScaleUp(device, new ElementAtZoom<>(new ImageData (filename), 100)); + ImageData data = scaleImageData(device, 100, getZoom(), new ImageData (filename)); init(data, getZoom()); init(); this.device.registerResourceWithZoomSupport(this); @@ -559,7 +560,7 @@ public Image(Device device, ImageFileNameProvider imageFileNameProvider) { init(new ImageData (fileName.element()), getZoom()); } } else { - ImageData resizedData = DPIUtil.autoScaleImageData (device, new ImageData (fileName.element()), fileName.zoom()); + ImageData resizedData = scaleImageData(device, fileName.zoom(), getZoom(), new ImageData (fileName.element())); init(resizedData, getZoom()); } init(); @@ -600,7 +601,7 @@ public Image(Device device, ImageDataProvider imageDataProvider) { this.imageProvider = new ImageDataProviderWrapper(imageDataProvider); initialNativeZoom = DPIUtil.getNativeDeviceZoom(); ElementAtZoom data = DPIUtil.validateAndGetImageDataAtZoom(imageDataProvider, getZoom()); - ImageData resizedData = DPIUtil.scaleImageData(device, data.element(), getZoom(), data.zoom()); + ImageData resizedData = scaleImageData(device, data.zoom(), getZoom(), data.element()); init (resizedData, getZoom()); init(); this.device.registerResourceWithZoomSupport(this); @@ -1420,7 +1421,7 @@ public ImageData getImageData (int zoom) { } TreeSet availableZooms = new TreeSet<>(zoomLevelToImageHandle.keySet()); int closestZoom = Optional.ofNullable(availableZooms.higher(zoom)).orElse(availableZooms.lower(zoom)); - return DPIUtil.scaleImageData (device, getImageMetadata(closestZoom).getImageData(), zoom, closestZoom); + return scaleImageData(device, closestZoom, zoom, getImageMetadata(closestZoom).getImageData()); } /** @@ -2048,6 +2049,41 @@ private void setBackground(Color color, long handle) { device.internal_dispose_GC(hDC, null); } +private ImageData scaleImageData(Device device, int currentZoom, int targetZoom, ImageData imageData) { + if (imageData == null || targetZoom == currentZoom || (device != null && !device.isAutoScalable())) return imageData; + float scaleFactor = (float) targetZoom / (float) currentZoom; + int width = imageData.width; + int height = imageData.height; + int scaledWidth = Math.round (width * scaleFactor); + int scaledHeight = Math.round (height * scaleFactor); + boolean useSmoothScaling = DPIUtil.isSmoothScalingEnabled() && imageData.getTransparencyType() != SWT.TRANSPARENCY_MASK; + if (useSmoothScaling) { + return scaleToUsingSmoothScaling(scaledWidth, scaledHeight, imageData); + } else { + return imageData.scaledTo (scaledWidth, scaledHeight); + } +} + +private ImageData scaleToUsingSmoothScaling(int width, int height, ImageData imageData) { + Image original = new Image (Display.getCurrent(), (ImageDataProvider) zoom -> imageData); + /* Create a 24 bit image data with alpha channel */ + final ImageData resultData = new ImageData (width, height, 24, new PaletteData (0xFF, 0xFF00, 0xFF0000)); + resultData.alphaData = new byte [width * height]; + Image resultImage = new Image (Display.getCurrent(), (ImageDataProvider) zoom -> resultData); + GC gc = new GC (resultImage); + gc.setAntialias (SWT.ON); + gc.drawImage (original, 0, 0, imageData.width, imageData.height, + /* E.g. destWidth here is effectively DPIUtil.autoScaleDown (scaledWidth), but avoiding rounding errors. + * Nevertheless, we still have some rounding errors due to the point-based API GC#drawImage(..). + */ + 0, 0, width, height, false); + gc.dispose (); + original.dispose (); + ImageData result = resultImage.getImageData (resultImage.getZoom()); + resultImage.dispose (); + return result; +} + private int getZoom() { return DPIUtil.getZoomForAutoscaleProperty(initialNativeZoom); } @@ -2138,7 +2174,8 @@ protected Rectangle getBounds(int zoom) { @Override ImageData getImageData(int zoom) { ElementAtZoom fileName = DPIUtil.validateAndGetImagePathAtZoom (provider, zoom); - return DPIUtil.scaleImageData (device, new ImageData (fileName.element()), zoom, fileName.zoom()); + ImageData imageData = new ImageData (fileName.element()); + return scaleImageData(device, fileName.zoom(), zoom, imageData); } @Override @@ -2151,7 +2188,7 @@ ImageHandle getImageMetadata(int zoom) { if (imageMetadata == null) init(imageData, zoom); init(); } else { - ImageData resizedData = DPIUtil.scaleImageData(device, imageData, zoom, imageCandidate.zoom()); + ImageData resizedData = scaleImageData(device, imageCandidate.zoom(), zoom, imageData); ImageData newData = adaptImageDataIfDisabledOrGray(resizedData); init(newData, zoom); } @@ -2201,13 +2238,13 @@ protected Rectangle getBounds(int zoom) { @Override ImageData getImageData(int zoom) { ElementAtZoom data = DPIUtil.validateAndGetImageDataAtZoom (provider, zoom); - return DPIUtil.scaleImageData (device, data.element(), zoom, data.zoom()); + return scaleImageData(device, data.zoom(), zoom, data.element()); } @Override ImageHandle getImageMetadata(int zoom) { ElementAtZoom imageCandidate = DPIUtil.validateAndGetImageDataAtZoom (provider, zoom); - ImageData resizedData = DPIUtil.scaleImageData (device, imageCandidate.element(), zoom, imageCandidate.zoom()); + ImageData resizedData = scaleImageData(device, imageCandidate.zoom(), zoom, imageCandidate.element()); ImageData newData = adaptImageDataIfDisabledOrGray(resizedData); init(newData, zoom); init(); diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/MenuItem.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/MenuItem.java index 1729a2a9a43..b9a06fe85cc 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/MenuItem.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/MenuItem.java @@ -781,8 +781,7 @@ public void setImage (Image image) { info.hbmpItem = OS.HBMMENU_CALLBACK; } else { if (OS.IsAppThemed ()) { - if (hBitmap != 0) OS.DeleteObject (hBitmap); - info.hbmpItem = hBitmap = image != null ? Display.create32bitDIB (image, getZoom()) : 0; + info.hbmpItem = hBitmap = getMenuItemIconBitmapHandle(image); } else { info.hbmpItem = image != null ? OS.HBMMENU_CALLBACK : 0; } @@ -792,6 +791,16 @@ public void setImage (Image image) { parent.redraw (); } +private long getMenuItemIconBitmapHandle(Image image) { + if(image == null) { + return 0; + } + if (hBitmap != 0) OS.DeleteObject (hBitmap); + int desiredSize = getSystemMetrics(OS.SM_CYMENUCHECK); + int zoom = (int) (((double) desiredSize / image.getBounds().height) * 100); + return Display.create32bitDIB (image, zoom); +} + /** * Sets the receiver's pull down menu to the argument. * Only CASCADE menu items can have a