From ab716d9ede05dd4490d90131f572dc711352a243 Mon Sep 17 00:00:00 2001 From: Heiko Klare <heiko.klare@vector.com> Date: Thu, 3 Apr 2025 18:28:28 +0200 Subject: [PATCH] [Win32] Ensure consistent image data returned for data-based images Due to the on-demand creation of image handles, there is not necessarily a handles anymore from which image data is retrieved when requesting is via the getImageData(...) methods. This results in potentially different kinds of image data (including different anti-aliasing results) depending on whether a handle has already been created for an image at the given zoom or not. This change adapts the implementation of Image based on static ImageData and streams to always use the image data retrieved from a native handle. To this end, it temporarily creates a handle if necessary. In order to avoid repeated loading and handle creation for the same source of image, a cache for the already retrieved image data is introduced. Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/2052 --- .../win32/org/eclipse/swt/graphics/Image.java | 43 ++++++++++++++----- .../Test_org_eclipse_swt_graphics_Image.java | 25 +++++++++++ 2 files changed, 57 insertions(+), 11 deletions(-) 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 d45c6a75bd..f74612574d 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 @@ -1878,14 +1878,18 @@ ImageData getScaledImageData (int zoom) { protected ImageHandle newImageHandle(int zoom) { ImageData resizedData = getImageData(zoom); - if (type == SWT.ICON && resizedData.getTransparencyType() != SWT.TRANSPARENCY_MASK) { + return newImageHandle(resizedData, zoom); + } + + protected final ImageHandle newImageHandle(ImageData data, int zoom) { + if (type == SWT.ICON && data.getTransparencyType() != SWT.TRANSPARENCY_MASK) { // If the original type was an icon with transparency mask and re-scaling leads // to image data without transparency mask, this will create invalid images // so this fallback will "repair" the image data by explicitly passing // the transparency mask created from the scaled image data - return initIconHandle(device, resizedData, resizedData.getTransparencyMask(), zoom); + return initIconHandle(device, data, data.getTransparencyMask(), zoom); } else { - return init(resizedData, zoom); + return init(data, zoom); } } @@ -1933,26 +1937,43 @@ AbstractImageProviderWrapper createCopy(Image image) { } private abstract class ImageFromImageDataProviderWrapper extends AbstractImageProviderWrapper { + private final Map<Integer, ImageData> cachedImageData = new HashMap<>(); protected abstract ElementAtZoom<ImageData> loadImageData(int zoom); void initImage() { // As the init call configured some Image attributes (e.g. type) // it must be called - ImageData imageDataAt100 = getImageData(100); - init(imageDataAt100, 100); - destroyHandleForZoom(100); + getImageData(100); } @Override ImageData newImageData(int zoom) { - if (!zoomLevelToImageHandle.isEmpty()) { - return getScaledImageData(zoom); + Function<Integer, ImageData> imageDataRetrieval = zoomToRetrieve -> { + ImageHandle handle = initializeHandleFromSource(zoomToRetrieve); + ImageData data = handle.getImageData(); + destroyHandleForZoom(zoomToRetrieve); + return data; + }; + return cachedImageData.computeIfAbsent(zoom, imageDataRetrieval); + } + + @Override + protected ImageHandle newImageHandle(int zoom) { + ImageData cachedData = cachedImageData.remove(zoom); + if (cachedData != null) { + return newImageHandle(cachedData, zoom); } - ElementAtZoom<ImageData> loadedImageData = loadImageData(zoom); - ImageData scaledImageData = DPIUtil.scaleImageData(device, loadedImageData, zoom); - return adaptImageDataIfDisabledOrGray(scaledImageData); + return initializeHandleFromSource(zoom); + } + + private ImageHandle initializeHandleFromSource(int zoom) { + ElementAtZoom<ImageData> imageDataAtZoom = loadImageData(zoom); + ImageData imageData = DPIUtil.scaleImageData(device, imageDataAtZoom.element(), zoom, imageDataAtZoom.zoom()); + imageData = adaptImageDataIfDisabledOrGray(imageData); + return newImageHandle(imageData, zoom); } + } private class PlainImageDataProviderWrapper extends ImageFromImageDataProviderWrapper { diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java index 5bc9cc034c..c18b01add6 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java @@ -1033,6 +1033,31 @@ public void test_imageDataSameViaDifferentProviders() { dataProviderImage.dispose(); } +@Test +public void test_imageDataSameViaProviderAndSimpleData() { + assumeFalse("Cocoa generates inconsistent image data", SwtTestUtil.isCocoa); + String imagePath = getPath("collapseall.png"); + ImageFileNameProvider imageFileNameProvider = __ -> { + return imagePath; + }; + ImageDataProvider dataProvider = __ -> { + try (InputStream imageStream = Files.newInputStream(Path.of(imagePath))) { + return new ImageData(imageStream); + } catch (IOException e) { + } + return null; + }; + Image fileNameProviderImage = new Image(display, imageFileNameProvider); + Image dataImage = new Image(display, dataProvider.getImageData(100)); + ImageData dataFromFileNameProviderImage = fileNameProviderImage.getImageData(100); + ImageData dataFromImageWithSimpleData = dataImage.getImageData(100); + assertEquals(0, imageDataComparator().compare(dataFromFileNameProviderImage, dataFromImageWithSimpleData)); + + fileNameProviderImage.dispose(); + dataImage.dispose(); +} + + private Comparator<ImageData> imageDataComparator() { return Comparator.<ImageData>comparingInt(d -> d.width) // .thenComparing(d -> d.height) //