diff --git a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Image.java b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Image.java index dbf1b852f5c..07f96f43691 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Image.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Image.java @@ -1909,6 +1909,8 @@ void executeOnImageAtSizeBestFittingSize(Consumer imageAtBestFittingSizeC private class CachedImageAtSize { private Image image; + /** File already found not to be dynamically sizable, so re-reading it cannot help. */ + private String nonSizableFileName; public void destroy() { if (image != null) { @@ -1959,9 +1961,17 @@ private Optional loadImageDataAtExactSize(int targetWidth, int target } if (imageFileNameProvider != null) { String fileName = DPIUtil.validateAndGetImagePathAtZoom(imageFileNameProvider, 100).element(); - if (ImageDataLoader.isDynamicallySizable(fileName)) { - ImageData imageDataAtSize = ImageDataLoader.loadBySize(fileName, targetWidth, targetHeight); - return Optional.of(imageDataAtSize); + if (fileName.equals(nonSizableFileName)) { + return Optional.empty(); + } + try (InputStream stream = new BufferedInputStream(new FileInputStream(fileName))) { + if (ImageDataLoader.isDynamicallySizable(stream)) { + nonSizableFileName = null; + return Optional.of(ImageDataLoader.loadBySize(stream, targetWidth, targetHeight)); + } + nonSizableFileName = fileName; + } catch (IOException e) { + SWT.error(SWT.ERROR_IO, e); } } return Optional.empty(); diff --git a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataLoader.java b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataLoader.java index 5f3bbc118f8..8df4c2f1a40 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataLoader.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataLoader.java @@ -45,10 +45,6 @@ public static boolean canLoadAtZoom(String filename, int fileZoom, int targetZoo return ImageLoader.canLoadAtZoom(filename, fileZoom, targetZoom); } - static boolean isDynamicallySizable(String filename) { - return ImageLoader.isDynamicallySizable(filename); - } - static boolean isDynamicallySizable(InputStream stream) { return ImageLoader.isDynamicallySizable(stream); } @@ -71,10 +67,4 @@ public static ImageData loadBySize(InputStream stream, int width, int height) { return data; } - public static ImageData loadBySize(String filename, int width, int height) { - ImageData data = new ImageLoader().loadBySize(filename, width, height); - if (data == null) SWT.error(SWT.ERROR_INVALID_IMAGE); - return data; - } - } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageLoader.java b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageLoader.java index 66e8e294f44..2d37215599e 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageLoader.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageLoader.java @@ -209,16 +209,6 @@ List> loadByZoom(String filename, int fileZoom, int tar return null; } -ImageData loadBySize(String filename, int width, int height) { - if (filename == null) SWT.error(SWT.ERROR_NULL_ARGUMENT); - try (InputStream stream = new FileInputStream(filename)) { - return loadBySize(stream, width, height); - } catch (IOException e) { - SWT.error(SWT.ERROR_IO, e); - } - return null; -} - static boolean canLoadAtZoom(String filename, int fileZoom, int targetZoom) { if (filename == null) SWT.error(SWT.ERROR_NULL_ARGUMENT); try (InputStream stream = new FileInputStream(filename)) { @@ -229,15 +219,6 @@ static boolean canLoadAtZoom(String filename, int fileZoom, int targetZoom) { return false; } -static boolean isDynamicallySizable(String filename) { - try (InputStream stream = new FileInputStream(filename)) { - return FileFormat.isDynamicallySizableFormat(stream); - } catch (IOException e) { - SWT.error(SWT.ERROR_IO, e); - } - return false; -} - static boolean isDynamicallySizable(InputStream stream) { return FileFormat.isDynamicallySizableFormat(stream); } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java index dfd5e265199..6d33d7c2ca0 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java @@ -72,8 +72,21 @@ private static Optional determineFileFormat(LEDataInputStream stream private static final int MAX_SIGNATURE_BYTES = 18 + 2; // e.g. Win-BMP or OS2-BMP plus a safety-margin + /** + * Answers whether the stream holds a format that can be loaded at an arbitrary size. + * A stream that supports mark is rewound to its current position afterwards. + */ public static boolean isDynamicallySizableFormat(InputStream is) { + boolean rewind = is.markSupported(); + if (rewind) is.mark(MAX_SIGNATURE_BYTES); Optional format = determineFileFormat(new LEDataInputStream(is, MAX_SIGNATURE_BYTES)); + if (rewind) { + try { + is.reset(); + } catch (IOException e) { + SWT.error(SWT.ERROR_IO, e); + } + } return format.isPresent() && !(format.get() instanceof StaticImageFileFormat); } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Image.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Image.java index 4c8593108c6..60c4ce3b2b7 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Image.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Image.java @@ -970,6 +970,8 @@ void destroy() { private class CachedImageAtSize { private Image image; + /** File already found not to be dynamically sizable, so re-reading it cannot help. */ + private String nonSizableFileName; public void destroy() { if (image != null) { @@ -1020,9 +1022,17 @@ private Optional loadImageDataAtExactSize(int targetWidth, int target } if (imageFileNameProvider != null) { String fileName = DPIUtil.validateAndGetImagePathAtZoom(imageFileNameProvider, 100).element(); - if (ImageDataLoader.isDynamicallySizable(fileName)) { - ImageData imageDataAtSize = ImageDataLoader.loadBySize(fileName, targetWidth, targetHeight); - return Optional.of(imageDataAtSize); + if (fileName.equals(nonSizableFileName)) { + return Optional.empty(); + } + try (InputStream stream = new BufferedInputStream(new FileInputStream(fileName))) { + if (ImageDataLoader.isDynamicallySizable(stream)) { + nonSizableFileName = null; + return Optional.of(ImageDataLoader.loadBySize(stream, targetWidth, targetHeight)); + } + nonSizableFileName = fileName; + } catch (IOException e) { + SWT.error(SWT.ERROR_IO, e); } } return Optional.empty(); 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 51da0aee6ff..10acb14295b 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 @@ -2630,6 +2630,9 @@ protected Rectangle getBounds(int zoom) { } private class ImageFileNameProviderWrapper extends BaseImageProviderWrapper { + /** File already found not to be dynamically sizable, so re-reading it cannot help. */ + private String nonSizableFileName; + ImageFileNameProviderWrapper(ImageFileNameProvider provider) { super(provider, ImageFileNameProvider.class); // Checks for the contract of the passed provider require @@ -2879,10 +2882,18 @@ private long extractHandleForPixelFormat(int width, int height, int pixelFormat) @Override protected Optional loadImageDataAtExactSize(int targetWidth, int targetHeight) { String fileName = DPIUtil.validateAndGetImagePathAtZoom(this.provider, 100).element(); - if (ImageDataLoader.isDynamicallySizable(fileName)) { - ImageData imageDataAtSize = ImageDataLoader.loadBySize(fileName, targetWidth, targetHeight); - ImageData adaptedImageDataAtSize = adaptImageDataIfDisabledOrGray(imageDataAtSize); - return Optional.of(adaptedImageDataAtSize); + if (fileName.equals(nonSizableFileName)) { + return Optional.empty(); + } + try (InputStream stream = new BufferedInputStream(new FileInputStream(fileName))) { + if (ImageDataLoader.isDynamicallySizable(stream)) { + nonSizableFileName = null; + ImageData imageDataAtSize = ImageDataLoader.loadBySize(stream, targetWidth, targetHeight); + return Optional.of(adaptImageDataIfDisabledOrGray(imageDataAtSize)); + } + nonSizableFileName = fileName; + } catch (IOException e) { + SWT.error(SWT.ERROR_IO, e); } return Optional.empty(); } 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 020dd94af01..df6a8b7fd8b 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 @@ -34,6 +34,7 @@ import java.nio.file.Path; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import org.eclipse.swt.SWT; @@ -1224,5 +1225,77 @@ public void test_gcOnImageGcDrawer_imageDataAtNonDeviceZoom() { } } +/** + * Whether a file can be loaded at an arbitrary size never changes, so the scaled + * drawImage must determine it once instead of re-reading the file on every draw. + * Deleting the file after the first draw makes any further read fail loudly. + */ +@Test +public void test_drawImageAtSize_doesNotReReadFileOfNonSizableFormat() throws IOException { + Path file = tempFolder.resolve("volatile-collapseall.png"); + Files.copy(Path.of(getPath("collapseall.png")), file); + int previousDeviceZoom = DPIUtil.getDeviceZoom(); + DPIUtil.setDeviceZoom(100); + try { + Image image = new Image(display, file.toString()); + Image target = new Image(display, 64, 64); + GC gc = new GC(target); + try { + Rectangle bounds = image.getBounds(); + gc.drawImage(image, 0, 0, bounds.width * 2, bounds.height * 2); + + Files.delete(file); + + gc.drawImage(image, 0, 0, bounds.width * 2, bounds.height * 2); + gc.drawImage(image, 0, 0, bounds.width * 3, bounds.height * 3); + } finally { + gc.dispose(); + target.dispose(); + image.dispose(); + } + } finally { + DPIUtil.setDeviceZoom(previousDeviceZoom); + Files.deleteIfExists(file); + } +} + +/** + * An Image may be backed by different files over its lifetime, so what is known about + * one file must not be applied to the next one. + */ +@Test +public void test_drawImageAtSize_reevaluatesSizabilityWhenFileNameChanges() throws IOException { + Path sizableFile = tempFolder.resolve("switchable-collapseall.svg"); + Files.copy(Path.of(getPath("collapseall.svg")), sizableFile); + AtomicReference currentFile = new AtomicReference<>(getPath("collapseall.png")); + ImageFileNameProvider switchingProvider = zoom -> zoom == 100 ? currentFile.get() : null; + int previousDeviceZoom = DPIUtil.getDeviceZoom(); + DPIUtil.setDeviceZoom(100); + try { + Image image = new Image(display, switchingProvider); + Image target = new Image(display, 64, 64); + GC gc = new GC(target); + try { + // learns that the PNG cannot be loaded at an arbitrary size + gc.drawImage(image, 0, 0, 20, 20); + + currentFile.set(sizableFile.toString()); + gc.drawImage(image, 0, 0, 20, 20); + + // if the SVG were still treated as non-sizable, it would never be read at all + Files.delete(sizableFile); + assertThrows(SWTException.class, () -> gc.drawImage(image, 0, 0, 24, 24), + "the file of a sizable image must be read again for a new size"); + } finally { + gc.dispose(); + target.dispose(); + image.dispose(); + } + } finally { + DPIUtil.setDeviceZoom(previousDeviceZoom); + Files.deleteIfExists(sizableFile); + } +} + }