Improve and extend tests for Image#getImageData()

- makes multiple test methods with more comprehensive names out of a
combined single one
- adds test method for additional use case
- adds win32-specific tests methods for specifics in Windows image
handling

Author: HeikoKlare

tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/graphics/ImageWin32Tests.java

@@ -14,6 +14,7 @@
 package org.eclipse.swt.graphics;
 
 
+import static org.eclipse.swt.tests.win32.SwtWin32TestUtil.assertImageDataEqualsIgnoringAlphaInData;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
@@ -174,4 +175,42 @@ public void testDisposeDrawnImageBeforeRequestingTargetForOtherZoom() {
 		}
 	}
 
+	// On Windows, the first used Image constructor creates a DDB, while the second
+	// transforms it into a DIB, still pixel RGB and alpha values should be the same.
+	@Test
+	public void test_getImageData_fromImageForImageDataFromImage() {
+		int width = 10;
+		int height = 10;
+		Color color = new Color(0, 0xff, 0);
+		Image image = new Image(display, width, height);
+		fillImage(image, color);
+		ImageData imageData = image.getImageData();
+		ImageData recreatedImageData = new Image(display, imageData).getImageData();
+		assertImageDataEqualsIgnoringAlphaInData(imageData, recreatedImageData);
+		image.dispose();
+	}
+
+	// On Windows, the first used Image constructor creates a DDB, while the second
+	// transforms it into a DIB, still pixel RGB and alpha values should be the same.
+	@Test
+	public void test_getImageData_fromCopiedImage() {
+		int width = 10;
+		int height = 10;
+		Color color = new Color(0, 0xff, 0);
+		Image image = new Image(display, width, height);
+		fillImage(image, color);
+		ImageData imageData = image.getImageData();
+		ImageData copiedImageData = new Image(display, image, SWT.IMAGE_COPY).getImageData();
+		assertImageDataEqualsIgnoringAlphaInData(imageData, copiedImageData);
+		image.dispose();
+	}
+
+	private static void fillImage(Image image, Color fillColor) {
+		GC gc = new GC(image);
+		gc.setBackground(fillColor);
+		gc.setForeground(fillColor);
+		gc.fillRectangle(image.getBounds());
+		gc.dispose();
+	}
+
 }

tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/tests/win32/SwtWin32TestUtil.java

@@ -13,8 +13,12 @@
  *******************************************************************************/
 package org.eclipse.swt.tests.win32;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
 import java.util.function.BooleanSupplier;
 
+import org.eclipse.swt.graphics.ImageData;
 import org.eclipse.swt.widgets.Display;
 
 public class SwtWin32TestUtil {
@@ -41,4 +45,45 @@ public static void processEvents(Display display, int timeoutMs, BooleanSupplier
 		}
 	}
 }
+
+/**
+ * Asserts that both given ImageData are equal, i.e. that:
+ * <ul>
+ *   <li>depths are equal (considering 24/32 bit as equals since alpha data is stored separately)</li>
+ *   <li>width and height are equal</li>
+ *   <li>all pixel RGB values are equal</li>
+ *   <li>all pixel alpha values in the alphaData are equal</li>
+ * </ul>
+ * In case any of these properties differ, the test will fail.
+ *
+ * @param expected the expected ImageData
+ * @param actual the actual ImageData
+ */
+// This method is necessary because ImageData has no custom equals method and the default one isn't sufficient.
+public static void assertImageDataEqualsIgnoringAlphaInData(final ImageData expected, final ImageData actual) {
+	assertNotNull(expected, "expected data must not be null");
+	assertNotNull(actual, "actual data must not be null");
+	if (expected == actual) {
+		return;
+	}
+	assertEquals(expected.height, actual.height, "height of expected image is different from actual image");
+	// Alpha values are taken from alpha data, so ignore whether data depth is 24 or 32 bits
+	int expectedNormalizedDepth = expected.depth == 32 ? 24 : expected.depth;
+	int actualNormalizedDepth = expected.depth == 32 ? 24 : expected.depth;
+	assertEquals(expectedNormalizedDepth, actualNormalizedDepth, "depth of image data to compare must be equal");
+	assertEquals(expected.width, actual.width, "width of expected image is different from actual image");
+
+	for (int y = 0; y < expected.height; y++) {
+		for (int x = 0; x < expected.width; x++) {
+			// FIXME win32: dragged ALPHA=FF, dropped ALPHA=00, but other transparencyType
+			// => alpha stored in ImageData.alphaData
+			String expectedPixel = String.format("0x%08X", expected.getPixel(x, y) >> (expected.depth == 32 ? 8 : 0));
+			String actualPixel = String.format("0x%08X", actual.getPixel(x, y) >> (actual.depth == 32 ? 8 : 0));
+			assertEquals(expectedPixel, actualPixel, "actual pixel at x=" + x + " y=" + y + " is different from expected pixel");
+			int expectedAlpha = expected.getAlpha(x, y);
+			int actualAlpha = actual.getAlpha(x, y);
+			assertEquals(expectedAlpha, actualAlpha, "actual pixel alpha at x=" + x + " y=" + y + " is different from expected pixel");
+		}
+	}
+}
 }

tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/tests/win32/Test_org_eclipse_swt_dnd_DND.java

@@ -13,6 +13,7 @@
  *******************************************************************************/
 package org.eclipse.swt.tests.win32;
 
+import static org.eclipse.swt.tests.win32.SwtWin32TestUtil.assertImageDataEqualsIgnoringAlphaInData;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -144,7 +145,7 @@ public void testImageTransfer_fromImage() throws InterruptedException {
 	try {
 		final ImageData drag = image.getImageData();
 		final ImageData drop = testTransferRoundtrip(ImageTransfer.getInstance(), drag);
-		assertImageDataEqualsIgoringAlphaInData(drag, drop);
+		assertImageDataEqualsIgnoringAlphaInData(drag, drop);
 	} finally {
 		image.dispose();
 	}
@@ -159,7 +160,7 @@ public void testImageTransfer_fromCopiedImage() throws InterruptedException {
 	try {
 		final ImageData drag = new Image(shell.getDisplay(), image, SWT.IMAGE_COPY).getImageData();
 		final ImageData drop = testTransferRoundtrip(ImageTransfer.getInstance(), drag);
-		assertImageDataEqualsIgoringAlphaInData(drag, drop);
+		assertImageDataEqualsIgnoringAlphaInData(drag, drop);
 	} finally {
 		image.dispose();
 	}
@@ -176,7 +177,7 @@ public void testImageTransfer_fromImageData() throws InterruptedException {
 	}
 	final ImageData drag = imageData;
 	final ImageData drop = testTransferRoundtrip(ImageTransfer.getInstance(), drag);
-	assertImageDataEqualsIgoringAlphaInData(drag, drop);
+	assertImageDataEqualsIgnoringAlphaInData(drag, drop);
 }
 
 /**
@@ -190,7 +191,7 @@ public void testImageTransfer_fromImageDataFromImage() throws InterruptedExcepti
 		try {
 			final ImageData drag = imageFromImageData.getImageData();
 			final ImageData drop = testTransferRoundtrip(ImageTransfer.getInstance(), drag);
-			assertImageDataEqualsIgoringAlphaInData(drag, drop);
+			assertImageDataEqualsIgnoringAlphaInData(drag, drop);
 		} finally {
 			imageFromImageData.dispose();
 		}
@@ -252,47 +253,6 @@ private Image createTestImage() {
 	return null;
 }
 
-/**
- * Asserts that both given ImageData are equal, i.e. that:
- * <ul>
- *   <li>depths are equal (considering 24/32 bit as equals since alpha data is stored separately)</li>
- *   <li>width and height are equal</li>
- *   <li>all pixel RGB values are equal</li>
- *   <li>all pixel alpha values in the alphaData are equal</li>
- * </ul>
- * In case any of these properties differ, the test will fail.
- *
- * @param expected the expected ImageData
- * @param actual the actual ImageData
- */
-// This method is necessary because ImageData has no custom equals method and the default one isn't sufficient.
-private void assertImageDataEqualsIgoringAlphaInData(final ImageData expected, final ImageData actual) {
-	assertNotNull(expected, "expected data must not be null");
-	assertNotNull(actual, "actual data must not be null");
-	if (expected == actual) {
-		return;
-	}
-	assertEquals(expected.height, actual.height, "height of expected image is different from actual image");
-	// Alpha values are taken from alpha data, so ignore whether data depth is 24 or 32 bits
-	int expectedNormalizedDepth = expected.depth == 32 ? 24 : expected.depth;
-	int actualNormalizedDepth = expected.depth == 32 ? 24 : expected.depth;
-	assertEquals(expectedNormalizedDepth, actualNormalizedDepth, "depth of image data to compare must be equal");
-	assertEquals(expected.width, actual.width, "width of expected image is different from actual image");
-
-	for (int y = 0; y < expected.height; y++) {
-		for (int x = 0; x < expected.width; x++) {
-			// FIXME win32: dragged ALPHA=FF, dropped ALPHA=00, but other transparencyType
-			// => alpha stored in ImageData.alphaData
-			String expectedPixel = String.format("0x%08X", expected.getPixel(x, y) >> (expected.depth == 32 ? 8 : 0));
-			String actualPixel = String.format("0x%08X", actual.getPixel(x, y) >> (actual.depth == 32 ? 8 : 0));
-			assertEquals(expectedPixel, actualPixel, "actual pixel at x=" + x + " y=" + y + " is different from expected pixel");
-			int expectedAlpha = expected.getAlpha(x, y);
-			int actualAlpha = actual.getAlpha(x, y);
-			assertEquals(expectedAlpha, actualAlpha, "actual pixel alpha at x=" + x + " y=" + y + " is different from expected pixel");
-		}
-	}
-}
-
 /**
  * Test transfer implementation by dragging and dropping some data onto ourself.
  *

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

@@ -677,13 +677,6 @@ public void test_getImageDataCurrentZoom() {
 	assertEquals(":d: Size of ImageData returned from Image.getImageDataAtCurrentZoom method doesn't return matches with bounds in Pixel values.", boundsAtCurrentZoom, bounds);
 }
 
-@Test
-public void test_getImageData() {
-	getImageData1();
-	getImageData2(24, new PaletteData(0xff0000, 0xff00, 0xff));
-	getImageData2(32, new PaletteData(0xff0000, 0xff00, 0xff));
-}
-
 @Test
 public void test_getImageData_changingImageDataDoesNotAffectImage() {
 	List<Image> images = List.of( //
@@ -920,11 +913,12 @@ public void test_toString() {
 /* custom */
 Display display;
 
-/** Test implementation **/
-
-void getImageData1() {
+@Test
+public void test_getImageData_fromFiles() {
+	int numFormats = SwtTestUtil.imageFormats.length;
 	String fileName = SwtTestUtil.imageFilenames[0];
-	for (String format : SwtTestUtil.imageFormats) {
+	for (int i=0; i<numFormats; i++) {
+		String format = SwtTestUtil.imageFormats[i];
 		try (InputStream stream = SwtTestUtil.class.getResourceAsStream(fileName + "." + format)) {
 			ImageData data1 = new ImageData(stream);
 			Image image = new Image(display, data1);
@@ -942,31 +936,53 @@ void getImageData1() {
  * Verify Image.getImageData returns pixels with the same RGB value as the
  * source image. This test only makes sense with depth of 24 and 32 bits.
  */
-void getImageData2(int depth, PaletteData palette) {
+@Test
+public void test_getImageData_fromImageForCustomImageData() {
 	int width = 10;
 	int height = 10;
 	Color color = new Color(0, 0xff, 0);
-	RGB colorRGB = color.getRGB();
+	PaletteData palette = new PaletteData(0xff0000, 0xff00, 0xff);
+	int[] depths = new int[] { 24, 32 };
+	for (int depth : depths) {
+		ImageData imageData = new ImageData(width, height, depth, palette);
+		Image image = new Image(display, imageData);
+		fillImage(image, color);
+		ImageData newData = image.getImageData();
+		assertAllPixelsHaveColor(newData, color);
+		image.dispose();
+	}
+}
 
-	ImageData imageData = new ImageData(width, height, depth, palette);
-	Image image = new Image(display, imageData);
+@Test
+public void test_getImageData_fromImage() {
+	int width = 10;
+	int height = 10;
+	Color color = new Color(0, 0xff, 0);
+	Image image = new Image(display, width, height);
+	fillImage(image, color);
+	ImageData imageData = image.getImageData();
+	assertAllPixelsHaveColor(imageData, color);
+	image.dispose();
+}
 
+private static void fillImage(Image image, Color fillColor) {
 	GC gc = new GC(image);
-	gc.setBackground(color);
-	gc.setForeground(color);
-	gc.fillRectangle(0, 0, 10, 10);
-
-	ImageData newData = image.getImageData();
-	PaletteData newPalette = newData.palette;
-	for (int i = 0; i < width; i++) {
-		for (int j = 0; j < height; j++) {
-			int pixel = newData.getPixel(i, j);
+	gc.setBackground(fillColor);
+	gc.setForeground(fillColor);
+	gc.fillRectangle(image.getBounds());
+	gc.dispose();
+}
+
+private static PaletteData assertAllPixelsHaveColor(ImageData imageData, Color expectedColor) {
+	PaletteData newPalette = imageData.palette;
+	for (int x = 0; x < imageData.width; x++) {
+		for (int y = 0; y < imageData.height; y++) {
+			int pixel = imageData.getPixel(x, y);
 			RGB rgb = newPalette.getRGB(pixel);
-			assertTrue("rgb.equals(colorRGB)", rgb.equals(colorRGB));
+			assertEquals("pixel at x=" + x + " y=" + y + " does not have expected color", expectedColor.getRGB(), rgb);
 		}
 	}
-	gc.dispose();
-	image.dispose();
+	return newPalette;
 }
 
 RGB getRealRGB(Color color) {