Skip to content

[Win32] Ensure consistent image data returned for filename/data provider #1989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 4, 2025
Merged

[Win32] Ensure consistent image data returned for filename/data provider #1989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 38 additions & 41 deletions bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import java.io.*;
import java.util.*;
import java.util.function.*;

import org.eclipse.swt.*;
import org.eclipse.swt.internal.*;
Expand Down Expand Up @@ -2176,8 +2177,9 @@ public boolean equals(Object otherProvider) {
}

private abstract class BaseImageProviderWrapper<T> extends DynamicImageProviderWrapper {
private final Map<Integer, ImageData> cachedImageData = new HashMap<>();

protected final T provider;
private final Map<Integer, Rectangle> zoomToBounds = new HashMap<>();

BaseImageProviderWrapper(T provider, Class<T> expectedClass) {
checkProvider(provider, expectedClass);
Expand All @@ -2190,17 +2192,38 @@ Object getProvider() {
}

@Override
protected Rectangle getBounds(int zoom) {
if (zoomLevelToImageHandle.containsKey(zoom)) {
ImageHandle imgHandle = zoomLevelToImageHandle.get(zoom);
return new Rectangle(0, 0, imgHandle.width, imgHandle.height);
}
if (!zoomToBounds.containsKey(zoom)) {
ImageData imageData = getImageData(zoom);
Rectangle rectangle = new Rectangle(0, 0, imageData.width, imageData.height);
zoomToBounds.put(zoom, rectangle);
final ImageData getImageData(int zoom) {
Function<Integer, ImageData> imageDataRetrival = zoomToRetrieve -> {
ImageHandle handle = initializeHandleFromSource(zoomToRetrieve);
ImageData data = handle.getImageData();
destroyHandleForZoom(zoomToRetrieve);
return data;
};
return cachedImageData.computeIfAbsent(zoom, imageDataRetrival);
}

@Override
final ImageHandle getImageMetadata(int zoom) {
ImageData cachedData = cachedImageData.remove(zoom);
if (cachedData != null) {
return init(cachedData, zoom);
}
return zoomToBounds.get(zoom);
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 init(imageData, zoom);
}

protected abstract ElementAtZoom<ImageData> loadImageData(int zoom);

@Override
protected Rectangle getBounds(int zoom) {
ImageData imageData = getImageData(zoom);
return new Rectangle(0, 0, imageData.width, imageData.height);
}
}

Expand All @@ -2210,7 +2233,7 @@ private class ImageFileNameProviderWrapper extends BaseImageProviderWrapper<Imag
}

@Override
ImageData getImageData(int zoom) {
protected ElementAtZoom<ImageData> loadImageData(int zoom) {
ElementAtZoom<String> fileForZoom = DPIUtil.validateAndGetImagePathAtZoom(provider, zoom);
ImageHandle nativeInitializedImage;
if (zoomLevelToImageHandle.containsKey(fileForZoom.zoom())) {
Expand All @@ -2226,23 +2249,7 @@ ImageData getImageData(int zoom) {
imageDataAtZoom = new ElementAtZoom<>(nativeInitializedImage.getImageData(), fileForZoom.zoom());
destroyHandleForZoom(zoom);
}
ImageData imageData = scaleIfNecessary(imageDataAtZoom, zoom);
imageData = adaptImageDataIfDisabledOrGray(imageData);
return imageData;
}

private ImageData scaleIfNecessary(ElementAtZoom<ImageData> imageDataAtZoom, int zoom) {
if (imageDataAtZoom.zoom() != zoom) {
return scaleImageData(imageDataAtZoom.element(), zoom, imageDataAtZoom.zoom());
}
return imageDataAtZoom.element();
}

@Override
ImageHandle getImageMetadata(int zoom) {
ImageData imageData = getImageData(zoom);
init(imageData, zoom);
return zoomLevelToImageHandle.get(zoom);
return imageDataAtZoom;
}

@Override
Expand Down Expand Up @@ -2455,18 +2462,8 @@ private class ImageDataProviderWrapper extends BaseImageProviderWrapper<ImageDat
}

@Override
ImageData getImageData(int zoom) {
ElementAtZoom<ImageData> data = DPIUtil.validateAndGetImageDataAtZoom (provider, zoom);
return scaleImageData(data.element(), zoom, data.zoom());
}

@Override
ImageHandle getImageMetadata(int zoom) {
ElementAtZoom<ImageData> imageCandidate = DPIUtil.validateAndGetImageDataAtZoom (provider, zoom);
ImageData resizedData = scaleImageData(imageCandidate.element(), zoom, imageCandidate.zoom());
ImageData newData = adaptImageDataIfDisabledOrGray(resizedData);
init(newData, zoom);
return zoomLevelToImageHandle.get(zoom);
protected ElementAtZoom<ImageData> loadImageData(int zoom) {
return DPIUtil.validateAndGetImageDataAtZoom (provider, zoom);
}

@Override
Expand Down
60 changes: 60 additions & 0 deletions ...wt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Comparator;
import java.util.function.Consumer;

import org.eclipse.swt.SWT;
Expand Down Expand Up @@ -1262,4 +1268,58 @@ public void test_updateWidthHeightAfterDPIChange() {
DPIUtil.setDeviceZoom(deviceZoom);
}
}

@Test
public void test_imageDataIsCached() {
assumeTrue("On-demand image creation only implemented for Windows", SwtTestUtil.isWindows);
String imagePath = getPath("collapseall.png");
ImageFileNameProvider imageFileNameProvider = __ -> {
return imagePath;
};
Image fileNameProviderImage = new Image(display, imageFileNameProvider);
assertSame(fileNameProviderImage.getImageData(100), fileNameProviderImage.getImageData(100));
}

@Test
public void test_imageDataSameViaDifferentProviders() {
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 dataProviderImage = new Image(display, dataProvider);
ImageData dataFromFileNameProviderImage = fileNameProviderImage.getImageData(100);
ImageData dataFromImageDescriptorImage = dataProviderImage.getImageData(100);
assertEquals(0, imageDataComparator().compare(dataFromFileNameProviderImage, dataFromImageDescriptorImage));

fileNameProviderImage.dispose();
dataProviderImage.dispose();
}

private Comparator<ImageData> imageDataComparator() {
return Comparator.<ImageData>comparingInt(d -> d.width) //
.thenComparing(d -> d.height) //
.thenComparing((ImageData firstData, ImageData secondData) -> {
for (int x = 0; x < firstData.width; x++) {
for (int y = 0; y < firstData.height; y++) {
if (firstData.getPixel(x, y) != secondData.getPixel(x, y)) {
return -1;
}
if (firstData.getAlpha(x, y) != secondData.getAlpha(x, y)) {
return -1;
}
}
}
return 0;
});
}

}
Loading