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,10 +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();
 			}
+			ImageDataLoader.AtSizeLoadResult imageDataAtSize = ImageDataLoader
+					.loadBySizeIfDynamicallySizable(fileName, targetWidth, targetHeight);
+			if (imageDataAtSize.staticFormat()) {
+				nonSizableFileName = fileName;
+			} else if (imageDataAtSize.data().isPresent()) {
+				nonSizableFileName = null;
+			}
+			return imageDataAtSize.data();
 		}
 		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,19 @@ 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;
+	public static AtSizeLoadResult loadBySizeIfDynamicallySizable(String filename, int width, int height) {
+		return new ImageLoader().loadBySizeIfDynamicallySizable(filename, width, height);
+	}
+
+	/**
+	 * Outcome of an at-size load attempt. {@code staticFormat} is only set when the file was
+	 * positively identified as a format that cannot be loaded at an arbitrary size, so that a
+	 * format which merely could not be determined is not remembered as non-sizable.
+	 */
+	record AtSizeLoadResult(Optional<ImageData> data, boolean staticFormat) {
+
+		static final AtSizeLoadResult STATIC_FORMAT = new AtSizeLoadResult(Optional.empty(), true);
+		static final AtSizeLoadResult UNKNOWN_FORMAT = new AtSizeLoadResult(Optional.empty(), false);
 	}
 
 }

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

@@ -55,6 +55,9 @@
  */
 public class ImageLoader {
 
+	/** Upper bound on the bytes a format signature check may consume before the stream is rewound. */
+	private static final int SIGNATURE_LOOKAHEAD_LIMIT = 8192;
+
 	/**
 	 * the array of ImageData objects in this ImageLoader.
 	 * This array is read in when the load method is called,
@@ -209,16 +212,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,13 +222,29 @@ 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);
+/**
+ * Loads the image at the given size if the file is a dynamically sizable format,
+ * reading the file only once for both the format check and the load.
+ */
+ImageDataLoader.AtSizeLoadResult loadBySizeIfDynamicallySizable(String filename, int width, int height) {
+	if (filename == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
+	try (InputStream stream = new BufferedInputStream(new FileInputStream(filename))) {
+		stream.mark(SIGNATURE_LOOKAHEAD_LIMIT);
+		Optional<Boolean> sizable = FileFormat.isDynamicallySizableFormatIfKnown(stream);
+		if (sizable.isEmpty()) {
+			return ImageDataLoader.AtSizeLoadResult.UNKNOWN_FORMAT;
+		}
+		if (!sizable.get()) {
+			return ImageDataLoader.AtSizeLoadResult.STATIC_FORMAT;
+		}
+		stream.reset();
+		ImageData data = loadBySize(stream, width, height);
+		if (data == null) SWT.error(SWT.ERROR_INVALID_IMAGE);
+		return new ImageDataLoader.AtSizeLoadResult(Optional.of(data), false);
 	} catch (IOException e) {
 		SWT.error(SWT.ERROR_IO, e);
 	}
-	return false;
+	return ImageDataLoader.AtSizeLoadResult.UNKNOWN_FORMAT;
 }
 
 static boolean isDynamicallySizable(InputStream stream) {

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

@@ -73,8 +73,15 @@ 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
 
 	public static boolean isDynamicallySizableFormat(InputStream is) {
-		Optional<FileFormat> format = determineFileFormat(new LEDataInputStream(is, MAX_SIGNATURE_BYTES));
-		return format.isPresent() && !(format.get() instanceof StaticImageFileFormat);
+		return isDynamicallySizableFormatIfKnown(is).orElse(Boolean.FALSE);
+	}
+
+	/**
+	 * @return empty if no format could be identified, so sizability is unknown
+	 */
+	public static Optional<Boolean> isDynamicallySizableFormatIfKnown(InputStream is) {
+		return determineFileFormat(new LEDataInputStream(is, MAX_SIGNATURE_BYTES))
+				.map(format -> !(format instanceof StaticImageFileFormat));
 	}
 
 	static abstract class StaticImageFileFormat extends FileFormat {

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,10 +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();
 			}
+			ImageDataLoader.AtSizeLoadResult imageDataAtSize = ImageDataLoader
+					.loadBySizeIfDynamicallySizable(fileName, targetWidth, targetHeight);
+			if (imageDataAtSize.staticFormat()) {
+				nonSizableFileName = fileName;
+			} else if (imageDataAtSize.data().isPresent()) {
+				nonSizableFileName = null;
+			}
+			return imageDataAtSize.data();
 		}
 		return Optional.empty();
 	}

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

@@ -2619,6 +2619,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
@@ -2871,12 +2874,17 @@ 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();
 		}
-		return Optional.empty();
+		ImageDataLoader.AtSizeLoadResult imageDataAtSize = ImageDataLoader
+				.loadBySizeIfDynamicallySizable(fileName, targetWidth, targetHeight);
+		if (imageDataAtSize.staticFormat()) {
+			nonSizableFileName = fileName;
+		} else if (imageDataAtSize.data().isPresent()) {
+			nonSizableFileName = null;
+		}
+		return imageDataAtSize.data().map(Image.this::adaptImageDataIfDisabledOrGray);
 	}
 }
 

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,67 @@ 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);
+
+	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();
+		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;
+
+	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();
+		Files.deleteIfExists(sizableFile);
+	}
+}
+
 }