Bug 553240: ImageLoader save() creates striped images on Linux
A previous patch hardcoded all saved images to 32bit, meaning 4
bytes per pixel. However there was some leftover logic where a
24 bit image without an alpha channel could be loaded and saved
with 3 bytes per pixel. This caused weirdness as the data saving
algorithm was expecting 4 bytes per pixel and therefore some
pixels were not saved correctly.
Tested on Fedora 31, GTK3.24 with the snippet attached, as
well as previous ImageLoader snippets and a child Eclipse. No
AllNonBrowser JUnit tests fails.
Change-Id: I5c7184a2158f07879bd651cca81d8167c36f3d76
Signed-off-by: Eric Williams <ericwill@redhat.com>
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/ImageLoader.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/ImageLoader.java
index a1f6489..cb1c42c 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/ImageLoader.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/ImageLoader.java
@@ -465,7 +465,7 @@
int width = imgData.width;
int height = imgData.height;
int n_channels = imgData.bytesPerLine / width; // original n_channels 3 or 4
- int bytes_per_pixel = imgData.bytesPerLine / width; // n_channels for original ImageData (width * height * bytes_per_pixel) = imgData.length
+ int bytes_per_pixel = 4; // all images saved are 32 bit, if there is no alpha we set it to 255
/*
* Destination offsets, GdkPixbuf data is stored in RGBA format.
@@ -488,12 +488,8 @@
ob = 0;
}
- if (has_alpha && bytes_per_pixel == 3) {
- bytes_per_pixel = 4;
- }
-
- // We use alpha by default now so just hard code bytes per pixel to 4
- byte[] srcData = new byte[(width * height * 4)];
+ // We use alpha by default now so bytes_per_pixel is always 4
+ byte[] srcData = new byte[(width * height * bytes_per_pixel)];
int alpha_offset = n_channels == 4 ? 1 : 0;
if (has_alpha) {
diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug553240_ImageLoaderSavingStriped.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug553240_ImageLoaderSavingStriped.java
new file mode 100644
index 0000000..bd356fb
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug553240_ImageLoaderSavingStriped.java
@@ -0,0 +1,98 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Red Hat and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Red Hat - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.gtk.snippets;
+
+import java.io.File;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.graphics.ImageData;
+import org.eclipse.swt.graphics.ImageLoader;
+import org.eclipse.swt.layout.RowData;
+import org.eclipse.swt.layout.RowLayout;
+import org.eclipse.swt.widgets.Canvas;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Text;
+
+public class Bug553240_ImageLoaderSavingStriped {
+
+ public static void main(final String[] args) {
+ try {
+ initUI();
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ }
+
+ static void initUI() throws Exception {
+ final File file = File.createTempFile("swt", "example");
+ final Display display = new Display();
+ final Shell shell = new Shell();
+
+ shell.setLayout(new RowLayout());
+ shell.setText("Bug 553240: Saving a PNG results in a striped Image");
+
+ Label label = new Label(shell, SWT.BORDER);
+ label.setText("This snippet loads an Image, saves it, then loads it again.\n"
+ + "The first Image is saved in a temporary file shown in the Text widget.\n"
+ + "The Image loaded the second time is drawn on the Canvas.");
+ final Text text = new Text(shell, SWT.BORDER);
+ text.setBounds(0, 50, 450, 75);
+
+ ImageLoader loader = new ImageLoader();
+ final ImageData[] loadedImageData = loader.load("./images/map.png");
+ final ImageData tileImageData = loadedImageData[0];
+ final ImageLoader imageLoader = new ImageLoader();
+ imageLoader.data = new ImageData[] { tileImageData };
+
+ final int imageType = tileImageData.type;
+ file.delete();
+ final String imageFilePath = file.getAbsolutePath();
+ imageLoader.save(imageFilePath, imageType);
+ text.setText(file.getAbsolutePath());
+
+ Composite composite = new Composite(shell, SWT.NONE);
+ RowData layoutData = new RowData(256, 256);
+ composite.setLayoutData(layoutData);
+
+ composite.setLayout(new RowLayout());
+ Canvas canvas = new Canvas(composite, SWT.NONE);
+ layoutData = new RowData(256, 256);
+ canvas.setLayoutData(layoutData);
+
+ canvas.addPaintListener(e -> {
+ ImageLoader canvasLoader = new ImageLoader();
+ final ImageData[] cloadedImageData = canvasLoader.load(file.getAbsolutePath());
+ final ImageData ctileImageData = cloadedImageData[0];
+ canvasLoader.data = new ImageData[] { ctileImageData };
+
+ Image imageToDraw = new Image(display, ctileImageData);
+ e.gc.drawImage(imageToDraw, 0, 0);
+ imageToDraw.dispose();
+ });
+
+ shell.pack();
+ shell.open();
+
+ while (!shell.isDisposed()) {
+ if (!display.readAndDispatch()) {
+ display.sleep();
+ }
+ }
+ file.delete();
+ }
+}
diff --git a/tests/org.eclipse.swt.tests.gtk/images/map.png b/tests/org.eclipse.swt.tests.gtk/images/map.png
new file mode 100644
index 0000000..2117178
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.gtk/images/map.png
Binary files differ
