Avoid re-reading the image file on every scaled drawImage

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

@@ -1909,6 +1909,8 @@ void executeOnImageAtSizeBestFittingSize(Consumer<Image> 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<ImageData> 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();

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;
-	}
-
 }

bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageLoader.java

@@ -209,16 +209,6 @@ List<ElementAtZoom<ImageData>> 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);
 }

bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java

@@ -72,8 +72,21 @@ private static Optional<FileFormat> 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<FileFormat> 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);
 	}
 

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<ImageData> 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();

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<ImageFileNameProvider> {
+	/** 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<ImageData> 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();
 	}

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<String> 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);
+	}
+}
+
 }