diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java index e6e18af6e1..454afc77f0 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java @@ -491,13 +491,38 @@ public void copyArea (Image image, int x, int y) { storeAndApplyOperationForExistingHandle(new CopyAreaToImageOperation(image, x, y)); } -private class CopyAreaToImageOperation extends Operation { - private final Image image; +private abstract class ImageOperation extends Operation { + private Image image; + + ImageOperation(Image image) { + setImage(image); + image.addOnDisposeListener(this::setCopyOfImage); + } + + private void setImage(Image image) { + this.image = image; + } + + private void setCopyOfImage(Image image) { + if (!GC.this.isDisposed()) { + Image copiedImage = new Image(image.device, image, SWT.IMAGE_COPY); + setImage(copiedImage); + registerForDisposal(copiedImage); + } + } + + protected Image getImage() { + return image; + } + +} + +private class CopyAreaToImageOperation extends ImageOperation { private final int x; private final int y; CopyAreaToImageOperation(Image image, int x, int y) { - this.image = image; + super(image); this.x = x; this.y = y; } @@ -507,7 +532,7 @@ void apply() { int zoom = getZoom(); int scaledX = Win32DPIUtils.pointToPixel(drawable, this.x, zoom); int scaledY = Win32DPIUtils.pointToPixel(drawable, this.y, zoom); - copyAreaInPixels(this.image, scaledX, scaledY); + copyAreaInPixels(getImage(), scaledX, scaledY); } } @@ -1013,18 +1038,17 @@ public void drawImage (Image image, int x, int y) { storeAndApplyOperationForExistingHandle(new DrawImageOperation(image, new Point(x, y))); } -private class DrawImageOperation extends Operation { - private final Image image; +private class DrawImageOperation extends ImageOperation { private final Point location; DrawImageOperation(Image image, Point location) { - this.image = image; + super(image); this.location = location; } @Override void apply() { - drawImageInPixels(this.image, Win32DPIUtils.pointToPixel(drawable, this.location, getZoom())); + drawImageInPixels(getImage(), Win32DPIUtils.pointToPixel(drawable, this.location, getZoom())); } private void drawImageInPixels(Image image, Point location) { @@ -1081,13 +1105,12 @@ void drawImage(Image srcImage, int srcX, int srcY, int srcWidth, int srcHeight, storeAndApplyOperationForExistingHandle(new DrawImageToImageOperation(srcImage, new Rectangle(srcX, srcY, srcWidth, srcHeight), new Rectangle(destX, destY, destWidth, destHeight), simple)); } -private class DrawScalingImageToImageOperation extends Operation { - private final Image image; +private class DrawScalingImageToImageOperation extends ImageOperation { private final Rectangle source; private final Rectangle destination; DrawScalingImageToImageOperation(Image image, Rectangle source, Rectangle destination) { - this.image = image; + super(image); this.source = source; this.destination = destination; } @@ -1096,7 +1119,7 @@ private class DrawScalingImageToImageOperation extends Operation { void apply() { int gcZoom = getZoom(); int srcImageZoom = calculateZoomForImage(gcZoom, source.width, source.height, destination.width, destination.height); - drawImage(image, source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, gcZoom, srcImageZoom); + drawImage(getImage(), source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, gcZoom, srcImageZoom); } private Collection getAllCurrentMonitorZooms() { @@ -1154,14 +1177,13 @@ private void drawImage(Image image, int srcX, int srcY, int srcWidth, int srcHei drawImage(image, src.x, src.y, src.width, src.height, dest.x, dest.y, dest.width, dest.height, false, scaledImageZoom); } -private class DrawImageToImageOperation extends Operation { - private final Image image; +private class DrawImageToImageOperation extends ImageOperation { private final Rectangle source; private final Rectangle destination; private final boolean simple; DrawImageToImageOperation(Image image, Rectangle source, Rectangle destination, boolean simple) { - this.image = image; + super(image); this.source = source; this.destination = destination; this.simple = simple; @@ -1169,7 +1191,7 @@ private class DrawImageToImageOperation extends Operation { @Override void apply() { - drawImage(image, source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, simple, getZoom()); + drawImage(getImage(), source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, simple, getZoom()); } } 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 e63ef4a913..28bb30fd71 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 @@ -139,6 +139,8 @@ public final class Image extends Resource implements Drawable { private Map zoomLevelToImageHandle = new HashMap<>(); + private List> onDisposeListeners; + private Image (Device device, int type, long handle, int nativeZoom) { super(device); this.type = type; @@ -997,6 +999,21 @@ public static void drawScaled(GC gc, Image original, int width, int height, floa return null; } +void addOnDisposeListener(Consumer onDisposeListener) { + if (onDisposeListeners == null) { + onDisposeListeners = new ArrayList<>(); + } + onDisposeListeners.add(onDisposeListener); +} + +@Override +public void dispose() { + if (onDisposeListeners != null) { + onDisposeListeners.forEach(listener -> listener.accept(this)); + } + super.dispose(); +} + @Override void destroy () { device.deregisterResourceWithZoomSupport(this); diff --git a/tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/graphics/ImageWin32Tests.java b/tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/graphics/ImageWin32Tests.java index c00d5a9e4d..da21556b1a 100644 --- a/tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/graphics/ImageWin32Tests.java +++ b/tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/graphics/ImageWin32Tests.java @@ -17,10 +17,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import org.eclipse.swt.SWT; import org.eclipse.swt.internal.DPIUtil; import org.eclipse.swt.widgets.Display; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Automated Tests for class org.eclipse.swt.graphics.Image @@ -67,4 +70,108 @@ public void testImageShouldHaveDimesionAsPerZoomLevel() { } } + @ParameterizedTest + @ValueSource(booleans = { true, false } ) + public void testRetrieveImageDataAtDifferentZooms(boolean concurrentlyDisposeDrawnImage) { + Color imageColor = display.getSystemColor(SWT.COLOR_RED); + Image image = new Image (display, 1, 1); + GC gc = new GC(image); + + try { + Image temporaryImage = createImageOfSizeOne(imageColor); + gc.drawImage(temporaryImage, 0, 0); + + if (concurrentlyDisposeDrawnImage) { + temporaryImage.dispose(); + } + ImageData resultImageDataAtDifferentZoom = image.getImageData(200); + + assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageDataAtDifferentZoom, 0, 0)); + + if (!concurrentlyDisposeDrawnImage) { + temporaryImage.dispose(); + } + } finally { + gc.dispose(); + image.dispose(); + } + } + + private Image createImageOfSizeOne(Color imageColor) { + Image imageToDraw = new Image (display, 1, 1); + GC imageToDrawGc = new GC(imageToDraw); + imageToDrawGc.setBackground(imageColor); + imageToDrawGc.fillRectangle(0, 0, 1, 1); + imageToDrawGc.dispose(); + return imageToDraw; + } + + private RGB getRGBAtPixel(ImageData resultImageDataAtDifferentZoom, int x, int y) { + return resultImageDataAtDifferentZoom.palette.getRGB(resultImageDataAtDifferentZoom.getPixel(x, y)); + } + + @ParameterizedTest + @ValueSource(booleans = { true, false } ) + public void testDrawImageAtDifferentZooms(boolean concurrentlyDisposeDrawnImage) { + Color imageColor = display.getSystemColor(SWT.COLOR_RED); + Image image = new Image (display, 1, 1); + GC gc = new GC(image); + Image temporaryImage = createImageOfSizeOne(imageColor); + try { + gc.drawImage(temporaryImage, 0, 0); + } finally { + if (concurrentlyDisposeDrawnImage) { + temporaryImage.dispose(); + } + } + + Image targetCanvas = new Image(display, 5, 5); + GC targetCanvasGc = new GC(targetCanvas); + try { + targetCanvasGc.getGCData().nativeZoom = 200; + targetCanvasGc.drawImage(image, 0, 0); + targetCanvasGc.getGCData().nativeZoom = 100; + targetCanvasGc.drawImage(image, 3, 0); + ImageData resultImageData = targetCanvas.getImageData(100); + + assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 0, 0)); + assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 1, 0)); + assertNotEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 2, 0)); + assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 3, 0)); + assertNotEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 4, 0)); + } finally { + if (!concurrentlyDisposeDrawnImage) { + temporaryImage.dispose(); + } + image.dispose(); + targetCanvasGc.dispose(); + gc.dispose(); + } + } + + @Test + public void testDisposeDrawnImageBeforeRequestingTargetForOtherZoom() { + Color imageColor = display.getSystemColor(SWT.COLOR_RED); + Image imageToDraw = new Image (display, 1, 1); + GC imageToDrawGc = new GC(imageToDraw); + imageToDrawGc.setBackground(imageColor); + imageToDrawGc.fillRectangle(0, 0, 1, 1); + + Image targetCanvas = new Image(display, 5, 5); + GC targetCanvasGc = new GC(targetCanvas); + try { + targetCanvasGc.drawImage(imageToDraw, 0, 0); + imageToDrawGc.dispose(); + imageToDraw.dispose(); + ImageData resultImageData = targetCanvas.getImageData(200); + + assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 0, 0)); + assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 1, 0)); + assertNotEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 2, 0)); + } finally { + targetCanvasGc.dispose(); + targetCanvas.dispose(); + } + } + } \ No newline at end of file diff --git a/tests/org.eclipse.swt.tests.win32/META-INF/MANIFEST.MF b/tests/org.eclipse.swt.tests.win32/META-INF/MANIFEST.MF index a4f6218d9f..2c3b7b737f 100644 --- a/tests/org.eclipse.swt.tests.win32/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.swt.tests.win32/META-INF/MANIFEST.MF @@ -9,4 +9,6 @@ Eclipse-BundleShape: dir Bundle-RequiredExecutionEnvironment: JavaSE-17 Automatic-Module-Name: org.eclipse.swt.tests.win32 Import-Package: org.junit.jupiter.api;version="[5.13.0,6.0.0)", + org.junit.jupiter.params;version="[5.13.0,6.0.0)", + org.junit.jupiter.params.provider;version="[5.13.0,6.0.0)", org.junit.platform.suite.api;version="[1.13.0,2.0.0)"