Is there already a test utility to check two image data's contents are effectively the same

[jonahgraham]

I am working on adding missing tests for the Clipboard (as part of my work on #2126)

I am currently trying to verify that we copy images to/from the clipboard properly. When that copy happens, we may not have the same binary contents for ImageData, so comparing the data field of the ImageData as ImageDataTestHelper.assertImageDataEqual will show them not-equal. Similarly, Test_org_eclipse_swt_graphics_ImageData.test_clone() has a comparison that also will fail in my case.

What I am looking to know is if there is in the testsuite already some code that compares two ImageDatas for their rendered values.

I think the code looks something like this, but perhaps I am missing something.

	public static void assertImageDataEquals(ImageData expected, ImageData actual) {
		if (expected == null) {
			assertNull(actual);
		}
		assertEquals(expected.width, actual.width);
		assertEquals(expected.height, actual.height);
		if (expected.getRGBs() != null || actual.getRGBs() != null) {
			fail("We don't have code yet for comparing non-direct color palettes");
		}
		for (int x = 0; x < expected.width; x++) {
			for (int y = 0; y < expected.width; y++) {
				assertEquals(expected.getPixel(x, y), actual.getPixel(x, y));
				assertEquals(expected.getAlpha(x, y), actual.getAlpha(x, y));
			}
		}
	}

Do we need to check any other values? Maybe transparencies? I also need to determine what is expected when copying/pasting (or drag and drop which uses much of the same code). Should everything be preserved? Even the colour depth?

[ShahzaibIbrahim]

Hi, We already have a comparator in the test for the images, that does more or less as your snippet in Test_org_eclipse_swt_graphics_Image

private Comparator<ImageData> imageDataComparator() {
	return Comparator.<ImageData>comparingInt(d -> d.width) //
			.thenComparing(d -> d.height) //
			.thenComparing((ImageData firstData, ImageData secondData) -> {
				for (int x = 0; x < firstData.width; x++) {
					for (int y = 0; y < firstData.height; y++) {
						if (firstData.getPixel(x, y) != secondData.getPixel(x, y)) {
							return -1;
						}
						if (firstData.getAlpha(x, y) != secondData.getAlpha(x, y)) {
							return -1;
						}
					}
				}
				return 0;
			});
}

I am not 100% sure about your copy implementation but I think this comparator is not enough. In case of different type of transparency for example we don't have alpha data at all. Some images like GIF uses mask data to define transparency. So to answer your question, I would preserve everything.

[jonahgraham]

Thanks @ShahzaibIbrahim that is exactly what I wanted.

So to answer your question, I would preserve everything.

I am writing tests on the existing implementations of the Transfer classes. Specifically this is to test that ImageTransfer works "as expected".

Currently (at least on GTK) it does not preserve everything, if I copy a 24-bit image to the clipboard, it comes back as a 32-bit image. I have not yet determined if it is in the serializing or deserializing that this changes.

If this sounds wrong to you, then I can create a new issue on this and also compare how other programs and OSes deal with this.

[ShahzaibIbrahim]

I think it's how Image was implemented. We convert RGB Images to ARGB (32-bit) if the image has alpha data. So that could be the one reason.

[jonahgraham]

That makes sense - so I think this line is when the change happens based on your info

eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/gtk/org/eclipse/swt/dnd/ImageTransfer.java

Line 93 in b942c4c

Image image = new Image(Display.getCurrent(), imgData);

I don't think that I will look at trying to change this though, but mostly because it has been like this forever (since code was first written in 662c4fb) so while I think it is wrong, it is of lower priority for now.

However that may change because GTK is deprecating pixbuf so this code may need to be rewritten for GTK5 support in the future.

[jonahgraham]

We already have a comparator in the test for the images, that does more or less as your snippet in Test_org_eclipse_swt_graphics_Image

I moved the implementation to common util class in #2672 (at the moment the second commit in the PR)

[jonahgraham]

We already have a comparator in the test for the images, that does more or less as your snippet in Test_org_eclipse_swt_graphics_Image

I moved the implementation to common util class in #2672 (at the moment the second commit in the PR)

In the end I merged this as part of #2673