[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 #2052

Author: HeikoKlare

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java

@@ -1892,14 +1892,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);
 		}
 	}
 
@@ -1947,26 +1951,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 = scaleImageData(imageDataAtZoom.element(), zoom, imageDataAtZoom.zoom());
+		imageData = adaptImageDataIfDisabledOrGray(imageData);
+		return newImageHandle(imageData, zoom);
+	}
+
 }
 
 private class PlainImageDataProviderWrapper extends ImageFromImageDataProviderWrapper {

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) //