From 3443bc62529e8b5d900d07dcd30065b821fda65b Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 13 Aug 2026 20:45:12 +0200 Subject: [PATCH 1/2] [GTK] Stop decoding the image file on every draw at zoom != 100 The internal drawImage that every public overload funnels into read the source dimensions from srcImage.getImageData(), which is getImageData(100). At currentDeviceZoom != 100 that misses the zoom == currentDeviceZoom fast path and loads new ImageData(fileName): a full open and full decode, for SVG a full re-parse and re-rasterize, to obtain two integers the Image already knows. The result was used for nothing else, the drawing itself goes through srcImage.surface. An ImageGcDrawer image paid the same way, by running the drawer callback once per draw. Take the dimensions from the Image, falling back to ImageData for images wrapped around a native handle by Image.gtk_new, which carry none. Those have no provider, so the fallback reads the cairo surface and never a file. Cocoa already takes its dimensions from NSImage.size() and win32 from getBounds(), so this brings GTK in line with both. Opens per draw over 100 draws of one Image at zoom 200, strace on Linux/GTK: 9 arg overload, PNG 1.00 -> 0.00 5 arg overload, PNG 2.00 -> 1.00 The open left on the 5 arg overload is the CachedImageAtSize path of issue 3505. Rendering is unchanged wherever the Image dimensions agree with the decoded ones, verified as identical SHA-256 of the drawn ImageData over both overloads, three files and zoom 100, 150 and 200, and again for an ImageFileNameProvider returning one path at every zoom. Two cases change, both where getBounds() and getImageData() already disagreed. For an asset set that is not exactly proportional, 16 pixels at 100% and 33 at 200%, the width field is round(33/2) = 17 while getImageData(100) gives 16, so at zoom 200 the unscaled draw now paints the last point row and column that were previously clipped. For a provider handing out the same file at every zoom, a 16 pixel file at zoom 200 gives bounds 8 and getImageData 16, so passing getImageData() dimensions as the source rectangle now raises ERROR_INVALID_ARGUMENT where it previously drew. In both cases the new value is the one that agrees with getBounds(). Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/3507 --- .../gtk/org/eclipse/swt/graphics/GC.java | 11 +++- .../Test_org_eclipse_swt_graphics_GC.java | 61 +++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java index 73f798cd5dc..9b7aa44d444 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java @@ -913,9 +913,14 @@ void drawImage(Image srcImage, int srcX, int srcY, int srcWidth, int srcHeight, /* Refresh Image as per zoom level, if required. */ srcImage.refreshImageForZoom (); - ImageData srcImageData = srcImage.getImageData(); - int imgWidth = srcImageData.width; - int imgHeight = srcImageData.height; + int imgWidth = srcImage.width; + int imgHeight = srcImage.height; + if (imgWidth == -1 || imgHeight == -1) { + /* Images wrapped around a native handle carry no dimensions, see Image.gtk_new. */ + ImageData srcImageData = srcImage.getImageData(); + imgWidth = srcImageData.width; + imgHeight = srcImageData.height; + } if (srcWidth == 0 && srcHeight == 0) { srcWidth = imgWidth; srcHeight = imgHeight; diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java index 5d7449c8e36..78c7efaed5a 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java @@ -30,6 +30,9 @@ import java.io.IOException; import java.io.InputStream; import java.lang.ref.WeakReference; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -59,6 +62,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -72,6 +76,9 @@ public class Test_org_eclipse_swt_graphics_GC { private static final int IMAGE_SIZE = 200; +@TempDir +static Path tempFolder; + @BeforeEach public void setUp() { display = Display.getDefault(); @@ -1208,6 +1215,60 @@ RGB getRealRGB(Color color) { return palette.getRGB(pixel); } +/** + * Drawing reads the image dimensions from the image itself. Obtaining them through + * ImageData used to decode the file again on every draw at a device zoom other than 100. + */ +@Test +public void test_drawImage_doesNotReReadImageFileAtNonDefaultZoom() throws IOException { + Path file = tempFolder.resolve("volatile-collapseall.png"); + Files.copy(SwtTestUtil.getPath("collapseall.png", tempFolder), file); + int previousDeviceZoom = DPIUtil.getDeviceZoom(); + Image fileImage = null; + try { + DPIUtil.setDeviceZoom(200); + gc.dispose(); + gc = new GC(image); + fileImage = new Image(display, file.toString()); + ImageData beforeDelete = drawToFreshTarget(fileImage); + assertFalse(Arrays.equals(blankTargetData(), beforeDelete.data), "the reference draw produced no pixels"); + + Files.delete(file); + + ImageData afterDelete = drawToFreshTarget(fileImage); + ImageDataTestHelper.assertImageDataEqual(beforeDelete, afterDelete, beforeDelete); + gc.drawImage(fileImage, 0, 0); + } finally { + if (fileImage != null) { + fileImage.dispose(); + } + DPIUtil.setDeviceZoom(previousDeviceZoom); + Files.deleteIfExists(file); + } +} + +private byte[] blankTargetData() { + Image target = new Image(display, IMAGE_SIZE, IMAGE_SIZE); + try { + return target.getImageData().data; + } finally { + target.dispose(); + } +} + +private ImageData drawToFreshTarget(Image source) { + Rectangle bounds = source.getBounds(); + Image target = new Image(display, IMAGE_SIZE, IMAGE_SIZE); + GC targetGc = new GC(target); + try { + targetGc.drawImage(source, 0, 0, bounds.width, bounds.height, 0, 0, bounds.width * 2, bounds.height * 2); + return target.getImageData(); + } finally { + targetGc.dispose(); + target.dispose(); + } +} + private void executeWithNonDefaultDeviceZoom(Runnable executable) { int previousDeviceZoom = DPIUtil.getDeviceZoom(); DPIUtil.setDeviceZoom(200); From ee4ddac24f17f76e1b1760fd231f2711eea9392f Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 13 Aug 2026 22:16:12 +0200 Subject: [PATCH 2/2] [GTK] Fix the image size cache never hitting at zoom != 100 CachedImageAtSize.refresh converted the requested draw size to pixels with DPIUtil.pointToPixel and handed that to isReusable, which compared it against the cached image's width and height. Those are points: the cached image is built with new Image(device, imageData, getDeviceZoom()) and Image.init(ImageData, zoom) divides by the scale factor. At zoom 200 the comparison is scaledWidth/2 == scaledWidth, never true, so the cache never hit and every scaled draw reloaded the image. Remember the requested size in pixels alongside the cached image and compare against that, as the win32 HandleAtSize already does. Opens per draw over 100 draws of one Image at a stable draw size through the 5 argument overload, SVG, strace on Linux/GTK: zoom 100 0.02 -> 0.02 zoom 200 2.00 -> 0.02 Rendering is unchanged, verified as identical SHA-256 of the drawn ImageData over both overloads, three files and zoom 100, 150 and 200. Cocoa has the same comparison but is not affected: its cached image is built with new Image(device, imageData), which passes zoom 100 to init and so keeps the value in pixels. Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/3511 --- .../gtk/org/eclipse/swt/graphics/Image.java | 11 +++++- .../Test_org_eclipse_swt_graphics_Image.java | 38 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) 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..b81b5446cd9 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,12 +970,17 @@ void destroy() { private class CachedImageAtSize { private Image image; + /** Size in pixels the cached image was requested at; the image itself carries points. */ + private int requestedWidth = -1; + private int requestedHeight = -1; public void destroy() { if (image != null) { image.dispose(); image = null; } + requestedWidth = -1; + requestedHeight = -1; } private Optional refresh(int destWidth, int destHeight) { @@ -987,12 +992,16 @@ private Optional refresh(int destWidth, int destHeight) { destroy(); Optional imageAtSize = loadImageAtSize(scaledWidth, scaledHeight); image = imageAtSize.orElse(null); + if (image != null) { + requestedWidth = scaledWidth; + requestedHeight = scaledHeight; + } return imageAtSize; } } private boolean isReusable(int width, int height) { - return image != null && image.height == height && image.width == width; + return image != null && requestedHeight == height && requestedWidth == width; } private Optional loadImageAtSize(int destWidth, int destHeight) { 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..de50d2acfe1 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 @@ -1224,5 +1224,43 @@ public void test_gcOnImageGcDrawer_imageDataAtNonDeviceZoom() { } } +/** + * The size cache must hit at any device zoom. Comparing the requested size in pixels + * against the cached image's size in points made every scaled draw a miss at zoom != 100. + */ +@Test +public void test_drawImageAtSize_cacheIsReusedAtNonDefaultZoom() throws IOException { + Path file = tempFolder.resolve("cached-collapseall.svg"); + Files.copy(Path.of(getPath("collapseall.svg")), file); + int originalDeviceZoom = DPIUtil.getDeviceZoom(); + Image image = null; + Image target = null; + GC gc = null; + try { + DPIUtil.setDeviceZoom(200); + image = new Image(display, file.toString()); + target = new Image(display, 64, 64); + gc = new GC(target); + gc.drawImage(image, 0, 0, 20, 20); + + // a second draw at the same size must come from the cache, so the file is not needed + Files.delete(file); + + gc.drawImage(image, 0, 0, 20, 20); + } finally { + if (gc != null) { + gc.dispose(); + } + if (target != null) { + target.dispose(); + } + if (image != null) { + image.dispose(); + } + DPIUtil.setDeviceZoom(originalDeviceZoom); + Files.deleteIfExists(file); + } +} + }