diff --git a/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/ImagesWin32Tests.java b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/ImagesWin32Tests.java index 6b60c2a20df..cf48800de38 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/ImagesWin32Tests.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/ImagesWin32Tests.java @@ -13,7 +13,8 @@ *******************************************************************************/ package org.eclipse.swt.graphics; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import org.eclipse.swt.*; import org.eclipse.swt.internal.*; @@ -30,9 +31,140 @@ public void testImageIconTypeShouldNotChangeAfterCallingGetHandleForDifferentZoo Image icon = Display.getDefault().getSystemImage(SWT.ICON_ERROR); try { Image.win32_getHandle(icon, 200); - assertEquals("Image type should stay to SWT.ICON", SWT.ICON, icon.type); + assertEquals(SWT.ICON, icon.type, "Image type should stay to SWT.ICON"); } finally { icon.dispose(); } } + + /** + * Tests that a GC.drawImage() handle is reused across consecutive calls at + * different pixel sizes when the image only provides 100% zoom data. Because + * every zoom request falls back to the same 100% data, the effective (nearest + * available) zoom is always 100%, and a freshly allocated handle should not be + * required for each different draw size. + *

+ * See https://github.com/eclipse-platform/eclipse.platform.swt/issues/3419 + */ + @Test + public void testDrawingHandleIsReusedForSingleZoomImageAtDifferentSizes() { + PaletteData palette = new PaletteData(0xFF0000, 0xFF00, 0xFF); + ImageData imageData = new ImageData(10, 10, 24, palette); + // Provider only has 100% data; returns null for every other zoom + Image image = new Image(Display.getDefault(), (ImageDataProvider) zoom -> zoom == 100 ? imageData : null); + long[] firstHandle = {0}; + long[] secondHandle = {0}; + try { + // 20x20 pixels → 200% zoom equivalent for a 10x10 base image + image.executeOnImageHandleAtBestFittingSize(h -> firstHandle[0] = h.handle(), 20, 20); + // 30x30 pixels → 300% zoom equivalent; provider still falls back to 100%, + // so the nearest available zoom is still 100% and the handle must be reused + image.executeOnImageHandleAtBestFittingSize(h -> secondHandle[0] = h.handle(), 30, 30); + assertNotEquals(0L, firstHandle[0], "First handle should be non-zero"); + assertEquals(firstHandle[0], secondHandle[0], + "Consecutive GC.drawImage() calls at different sizes should reuse the same " + + "handle when the nearest available zoom is the same (100% in this case)"); + } finally { + image.dispose(); + } + } + + /** + * Tests that a GC.drawImage() handle is reused across consecutive calls at + * different pixel sizes when the image provides data at 100% and 200% zoom. + * Sizes that both map to the 200% nearest available zoom (e.g. 200% and 250%) + * should share the same underlying handle without re-allocating it. + *

+ * See https://github.com/eclipse-platform/eclipse.platform.swt/issues/3419 + */ + @Test + public void testDrawingHandleIsReusedForTwoZoomImageAtSizesWithSameNearestZoom() { + PaletteData palette = new PaletteData(0xFF0000, 0xFF00, 0xFF); + ImageData imageData100 = new ImageData(10, 10, 24, palette); + ImageData imageData200 = new ImageData(20, 20, 24, palette); + // Provider has explicit data at 100% and 200%; returns null for anything else + Image image = new Image(Display.getDefault(), (ImageDataProvider) zoom -> zoom == 100 ? imageData100 : zoom == 200 ? imageData200 : null); + long[] firstHandle = {0}; + long[] secondHandle = {0}; + try { + // 20x20 pixels → exactly 200% zoom for the 10x10 base image; uses 200% data + image.executeOnImageHandleAtBestFittingSize(h -> firstHandle[0] = h.handle(), 20, 20); + // 25x25 pixels → 250% zoom equivalent; nearest available is 200%, so the + // previously cached 200% handle should be reused + image.executeOnImageHandleAtBestFittingSize(h -> secondHandle[0] = h.handle(), 25, 25); + assertNotEquals(0L, firstHandle[0], "First handle should be non-zero"); + assertEquals(firstHandle[0], secondHandle[0], + "Consecutive GC.drawImage() calls at different sizes should reuse the same " + + "handle when the nearest available zoom is the same (200% in this case)"); + } finally { + image.dispose(); + } + } + + /** + * Tests that GC.drawImage() handles differ when consecutive calls at different + * pixel sizes land in different nearest-available-zoom regions for an image + * that provides distinct data at 100% and 200%. A size mapping to 100% and a + * size mapping to 200% must not share the same native handle, as they would + * represent different pixel content. + *

+ * See https://github.com/eclipse-platform/eclipse.platform.swt/issues/3419 + */ + @Test + public void testHandlesAreDifferentForTwoZoomImageAtDifferentNearestZooms() { + PaletteData palette = new PaletteData(0xFF0000, 0xFF00, 0xFF); + ImageData imageData100 = new ImageData(10, 10, 24, palette); + ImageData imageData200 = new ImageData(20, 20, 24, palette); + // Provider has explicit data at 100% and 200%; returns null for anything else + Image image = new Image(Display.getDefault(), (ImageDataProvider) zoom -> zoom == 100 ? imageData100 : zoom == 200 ? imageData200 : null); + long[] handle100Zone = {0}; + long[] handle200Zone = {0}; + try { + // 10x10 pixels → 100% zoom for the 10x10 base image; nearest available is 100% + image.executeOnImageHandleAtBestFittingSize(h -> handle100Zone[0] = h.handle(), 10, 10); + // 20x20 pixels → 200% zoom; nearest available is 200% → must differ from the + // 100% handle since the underlying pixel data is different + image.executeOnImageHandleAtBestFittingSize(h -> handle200Zone[0] = h.handle(), 20, 20); + assertNotEquals(0L, handle100Zone[0], "First handle should be non-zero"); + assertNotEquals(handle100Zone[0], handle200Zone[0], + "GC.drawImage() calls where the nearest available zoom differs must not " + + "reuse the same handle (100% data vs 200% data)"); + } finally { + image.dispose(); + } + } + + /** + * Tests that a persistent native handle already created via + * {@link Image#win32_getHandle(Image, int)} is found and reused by + * GC.drawImage() when the nearest available zoom for the requested draw size + * maps to the same zoom. This verifies that the + * {@code imageHandleManager.get(nearestAvailableZoom)} lookup is effective and + * avoids redundant handle allocation. + *

+ * See https://github.com/eclipse-platform/eclipse.platform.swt/issues/3419 + */ + @Test + public void testDrawImageReusesExistingPersistentHandleForNearestAvailableZoom() { + PaletteData palette = new PaletteData(0xFF0000, 0xFF00, 0xFF); + ImageData imageData = new ImageData(10, 10, 24, palette); + // Provider only has 100% data; every zoom falls back to 100% + Image image = new Image(Display.getDefault(), (ImageDataProvider) zoom -> zoom == 100 ? imageData : null); + try { + // Force creation of a persistent 100% handle (as would happen via GC.drawImage + // on a 100% zoom canvas or via Image.getImageData()) + long persistentHandle = Image.win32_getHandle(image, 100); + assertNotEquals(0L, persistentHandle, "Persistent handle should be non-zero"); + long[] drawHandle = {0}; + // 20x20 pixels → 200% zoom equivalent for the 10x10 base image; nearest + // available is still 100%, so the already-cached persistent handle must be + // returned without allocating a new one + image.executeOnImageHandleAtBestFittingSize(h -> drawHandle[0] = h.handle(), 20, 20); + assertEquals(persistentHandle, drawHandle[0], + "GC.drawImage() should reuse the existing persistent handle when the " + + "nearest available zoom matches the cached handle's zoom (100% here)"); + } finally { + image.dispose(); + } + } } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java index 12a78ed8464..29e6fc94aef 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java @@ -201,28 +201,39 @@ public String toString() { } private class HandleAtSize { + record TemporaryHandleForZoom(DestroyableImageHandle handle, int zoom) {} + private InternalImageHandle handleContainer = null; - private DestroyableImageHandle temporaryHandleContainer = null; + private TemporaryHandleForZoom temporaryHandleContainer = null; private int requestedWidth = -1; private int requestedHeight = -1; public void destroy() { - if (temporaryHandleContainer != null) { - temporaryHandleContainer.destroy(); - temporaryHandleContainer = null; + TemporaryHandleForZoom previousHandle = reset(); + if (previousHandle != null) { + previousHandle.handle().destroy(); } + } + + private TemporaryHandleForZoom reset() { + TemporaryHandleForZoom previousHandle = temporaryHandleContainer; + temporaryHandleContainer = null; handleContainer = null; requestedWidth = -1; requestedHeight = -1; + return previousHandle; } public ImageHandle refresh(int width, int height) { if (!isReusable(width, height)) { - destroy(); + TemporaryHandleForZoom previousHandle = reset(); requestedWidth = width; requestedHeight = height; handleContainer = createHandleAtExactSize(width, height) - .orElseGet(() -> getOrCreateImageHandleAtClosestSize(width, height)); + .orElseGet(() -> getOrCreateImageHandleAtClosestSize(width, height, previousHandle)); + if (previousHandle != null && previousHandle.handle() != handleContainer) { + previousHandle.handle().destroy(); + } } return handleContainer; } @@ -238,23 +249,32 @@ private boolean isReusable(int width, int height) { private Optional createHandleAtExactSize(int width, int height) { Optional imageData = imageProvider.loadImageDataAtExactSize(width, height); if (imageData.isPresent()) { - temporaryHandleContainer = init(imageData.get(), -1); - return Optional.of(temporaryHandleContainer); + temporaryHandleContainer = new TemporaryHandleForZoom(init(imageData.get(), -1), 0); + return Optional.of(temporaryHandleContainer.handle()); } return Optional.empty(); } - private InternalImageHandle getOrCreateImageHandleAtClosestSize(int widthHint, int heightHint) { + private InternalImageHandle getOrCreateImageHandleAtClosestSize(int widthHint, int heightHint, TemporaryHandleForZoom previousHandle) { Rectangle bounds = getBounds(100); int imageZoomForWidth = 100 * widthHint / bounds.width; int imageZoomForHeight = 100 * heightHint / bounds.height; int imageZoom = DPIUtil.getZoomForAutoscaleProperty(Math.max(imageZoomForWidth, imageZoomForHeight)); - InternalImageHandle bestFittingHandle = imageHandleManager.get(imageZoom); - if (bestFittingHandle == null) { - ImageData bestFittingImageData = imageProvider.loadImageData(imageZoom).element(); - bestFittingHandle = temporaryHandleContainer = init(bestFittingImageData, -1); + int nearestAvailableZoom = imageProvider.nearestAvailableZoom(imageZoom); + InternalImageHandle bestFittingHandle = imageHandleManager.get(nearestAvailableZoom); + if (bestFittingHandle != null) { + return bestFittingHandle; + } + if (nearestAvailableZoom == 100) { + return getHandleInternal(100, 100); } - return bestFittingHandle; + if (previousHandle != null && previousHandle.zoom() == nearestAvailableZoom) { + temporaryHandleContainer = previousHandle; + return previousHandle.handle(); + } + ElementAtZoom imageData = imageProvider.loadImageData(imageZoom); + temporaryHandleContainer = new TemporaryHandleForZoom(init(imageData.element(), -1), imageData.zoom()); + return temporaryHandleContainer.handle(); } } @@ -957,6 +977,10 @@ public static long win32_getHandle (Image image, int zoom) { } ImageHandle getHandle (int targetZoom, int nativeZoom) { + return getHandleInternal(targetZoom, nativeZoom); +} + +InternalImageHandle getHandleInternal (int targetZoom, int nativeZoom) { if (isDisposed()) { return null; } @@ -2090,6 +2114,8 @@ protected boolean isPersistentImageHandleRequriedForImageData() { return false; } + abstract int nearestAvailableZoom(int zoom); + /** * Returns image data at the best-fitting available zoom for the given zoom. * The returned data will have a potential gray/disable style applied. @@ -2102,7 +2128,7 @@ protected boolean isPersistentImageHandleRequriedForImageData() { ElementAtZoom getClosestAvailableImageData(int zoom) { TreeSet availableZooms = new TreeSet<>(imageHandleManager.getAllZooms()); - int closestZoom = Optional.ofNullable(availableZooms.higher(zoom)).orElse(availableZooms.lower(zoom)); + int closestZoom = availableZooms.contains(zoom) ? zoom : Optional.ofNullable(availableZooms.higher(zoom)).orElse(availableZooms.lower(zoom)); ImageData imageData = imageHandleManager.get(closestZoom).getImageData(); return new ElementAtZoom<>(imageData, closestZoom); } @@ -2179,6 +2205,11 @@ protected DestroyableImageHandle newImageHandle(ZoomContext zoomContext) { ImageData resizedData = newImageData (zoomContext.targetZoom()); return newImageHandle(resizedData, zoomContext); } + + @Override + int nearestAvailableZoom(int zoom) { + return zoomForHandle; + } } private abstract class ImageFromImageDataProviderWrapper extends AbstractImageProviderWrapper { @@ -2249,6 +2280,11 @@ protected ElementAtZoom loadImageData(int zoom) { AbstractImageProviderWrapper createCopy(Image image) { return image.new PlainImageDataProviderWrapper(this.imageDataAtBaseZoom); } + + @Override + int nearestAvailableZoom(int zoom) { + return baseZoom; + } } private class MaskedImageDataProviderWrapper extends ImageFromImageDataProviderWrapper { @@ -2281,6 +2317,11 @@ protected ElementAtZoom loadImageData(int zoom) { AbstractImageProviderWrapper createCopy(Image image) { return image.new MaskedImageDataProviderWrapper(this.srcAt100, this.maskAt100); } + + @Override + int nearestAvailableZoom(int zoom) { + return 100; + } } private class ImageDataLoaderStreamProviderWrapper extends ImageFromImageDataProviderWrapper { @@ -2325,6 +2366,14 @@ protected Optional loadImageDataAtExactSize(int targetWidth, int targ } return Optional.empty(); } + + @Override + int nearestAvailableZoom(int zoom) { + if (ImageDataLoader.isDynamicallySizable(new ByteArrayInputStream(this.inputStreamData))) { + return zoom; + } + return FileFormat.DEFAULT_ZOOM; + } } private class PlainImageProviderWrapper extends AbstractImageProviderWrapper { @@ -2388,6 +2437,11 @@ protected ElementAtZoom loadImageData(int zoom) { return getClosestAvailableImageData(zoom); } + @Override + int nearestAvailableZoom(int zoom) { + return getClosestAvailableImageData(zoom).zoom(); + } + @Override protected DestroyableImageHandle newImageHandle(ZoomContext zoomContext) { int targetZoom = zoomContext.targetZoom(); @@ -2533,6 +2587,7 @@ private class ImageFileNameProviderWrapper extends BaseImageProviderWrapper loadImageData(int zoom) { ElementAtZoom fileForZoom = DPIUtil.validateAndGetImagePathAtZoom(provider, zoom); @@ -2564,6 +2619,14 @@ protected ElementAtZoom loadImageData(int zoom) { return adaptImageDataIfDisabledOrGray(imageDataAtZoom); } + @Override + int nearestAvailableZoom(int zoom) { + if (provider instanceof ImageDataAtSizeProvider) { + return zoom; + } + return DPIUtil.validateAndGetImagePathAtZoom(provider, zoom).zoom(); + } + @Override public int hashCode() { return Objects.hash(provider, styleFlag); @@ -2807,6 +2870,14 @@ protected Optional loadImageDataAtExactSize(int targetWidth, int targ } return Optional.empty(); } + + @Override + int nearestAvailableZoom(int zoom) { + if (provider instanceof ImageDataAtSizeProvider) { + return zoom; + } + return DPIUtil.validateAndGetImageDataAtZoom (provider, zoom).zoom(); + } } private class ImageGcDrawerWrapper extends DynamicImageProviderWrapper { @@ -2848,6 +2919,11 @@ protected ElementAtZoom loadImageData(int zoom) { return new ElementAtZoom<>(loadImageData(new ZoomContext(zoom)), zoom); } + @Override + int nearestAvailableZoom(int zoom) { + return zoom; + } + private ImageData loadImageData(ZoomContext zoomContext) { currentZoom = zoomContext; int targetZoom = zoomContext.targetZoom();