From 3443bc62529e8b5d900d07dcd30065b821fda65b Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 13 Aug 2026 20:45:12 +0200 Subject: [PATCH] [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);