Avoid re-reading the image file on every scaled drawImage

[vogella]

The scaled GC.drawImage overload sniffed the backing file on every draw to ask whether it can be loaded at an arbitrary size, then discarded the answer, so every PNG, GIF or JPEG icon was reopened on each repaint.
Sizability is a property of the file, so the Image now remembers a file found not to be sizable, keyed on the resolved file name because an Image may be backed by different files over its lifetime.
For sizable files the sniff and the load share one open: Image opens the stream once and FileFormat rewinds it after reading the signature.

Measured with strace on Linux/GTK at zoom 100, opens per scaled draw drop from 1.00 to 0.01 for PNG and from 2.00 to 1.00 for SVG at alternating sizes, with bit-identical rendering.
At zoom 200 one open per draw remains from #3507, fixed separately in #3510, and the size cache never hitting there is #3511.
A file replaced in place at the same path is no longer re-sniffed for the lifetime of the Image.

Fixes #3505

[github-actions]

Test Results

  212 files  ± 0    212 suites  ±0   26m 30s ⏱️ - 3m 5s
4 952 tests + 2  4 926 ✅ + 2   26 💤 ±0  0 ❌ ±0 
7 150 runs  +12  6 970 ✅ +12  180 💤 ±0  0 ❌ ±0 

Results for commit 8be0f40. ± Comparison against base commit 0ba3aa6.

♻️ This comment has been updated with latest results.

[Copilot]

Pull request overview

This PR reduces repeated disk reads during scaled GC.drawImage(Image, int, int, int, int) when the source Image is backed by a file that cannot be loaded at arbitrary sizes (e.g., PNG/GIF/JPEG). It does this by remembering when a resolved filename has already been proven “non-dynamically-sizable”, and by performing the “format sniff” + “load at size” using a single stream open.

Changes:

• Cache “known non-sizable” resolved filenames in the platform Image scaled-draw caches/wrappers to avoid re-sniffing the same static-format file on every repaint.
• Add a single-pass loader path (loadBySizeIfDynamicallySizable) that determines format sizability and loads-at-size from the same opened stream.
• Add JUnit coverage asserting that static-format images are not re-read and that sizability is re-evaluated when an Image’s backing filename changes.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java	Adds tests validating the “no re-read for static formats” behavior and re-evaluation when the backing filename changes.
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java	Memoizes the last resolved non-sizable filename in the file-name provider wrapper to skip repeated sniffing/loads for static formats.
bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Image.java	Adds non-sizable filename memoization in CachedImageAtSize to avoid repeated format checks on static files.
bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Image.java	Mirrors GTK’s CachedImageAtSize memoization for static-format files.
bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java	Introduces an “if known” format-sizability check that can return “unknown” when the signature can’t be identified.
bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageLoader.java	Adds loadBySizeIfDynamicallySizable using mark/reset on a buffered stream to avoid double-opening the file.
bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataLoader.java	Adds an internal AtSizeLoadResult wrapper and a delegating entry point for the single-pass “check + load” path.

Suppressed comments (2)

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java:1266

• This test also depends on device zoom being 100; otherwise the separate zoom-related file decode path can cause file reads that mask the behavior being asserted. Capture/force device zoom to 100 at the start so the test only exercises the scaled-draw sizability caching.

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;

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java:1287

• If the test forces the device zoom, it should be restored in the finally block. Restore it before deleting the temp file so an IOException from delete doesn't leave the global zoom modified for subsequent tests.

		gc.dispose();
		target.dispose();
		image.dispose();
		Files.deleteIfExists(sizableFile);
	}

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[HeikoKlare]

I wonder if it wouldn't be easier to just reduce the ImageLoader methods for loading by size to those accepting streams and creating the stream on which isDynamicallySizable() and loadBySize() are executed inside the Image class. That would avoid introducing the AtSizeLoadResult type which seems somehow redundant to ElementAtZoom used for image loading results so far, it would preseve the level of abstraction at which ImageLoader operates and would not introduce a larger distinction between by zoom and by size loading operations etc.

Maybe I miss something that prevents that simpler solution. Then at least I would expect the design of the solution to be cleaned up rather than introducing further auxiliary records and inconsistent methods at the ImageLoader.

[vogella]

Thanks @HeikoKlare for the feedback, I'm currently working on other things but plan to return to SWT next week, so I will look at the feedback a little bit later.

[vogella]

@HeikoKlareThanks, good suggestions. I reworked the PR accordingly: ImageLoader keeps only its stream-based isDynamicallySizable and loadBySize, and Image opens the file once and hands the same stream to both.

AtSizeLoadResult and isDynamicallySizableFormatIfKnown are removed.

Please check again.

[HeikoKlare]

Thank you for the update. This looks like a sound and simple enhancement now.

[vogella]

And it shows up im my performance traces as a annoying baseline making the UI sluggisch. Thanks for the review @HeikoKlare