Bug 567898 - [JFace][HiDPI] ImageDescriptor support alternative naming
scheme for high dpi

Currently ImageDescriptor automatically provides higher DPI images if a
resource exits that ends with @x2 or @1.5

This works well if only one resolution is used across the application
(e.g. 16x16 pixel icons) but rapidly blows up if multiple resolutions
are used as there are duplications. The second limitation is that only
two fixed zoom levels are supported at the moment.

The current approach can be enhanced to support another common scheme
where icon resources are organized in folders holding the resolution in
folder-name (e.g. icons/16x16/icon1.png, icons/32x32/icon1.png and so
on). This also has the advantage that it is possible to implement other
scaling factors beside the currently fixed ones without any code
changes.

This commit includes the following:

- add/extend test cases for both schemes (current + fallback) to ensure
compatibility with both schemes
- add proposed alternative naming scheme for high-dpi icons as a
fallback if the @x2/@x1.5 does not find a suitable resource


Change-Id: Ib156aedf9db8ef115ab8dd566731124f3c6e1d48
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FileImageDescriptor.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FileImageDescriptor.java
index 472ac4c..01ec171 100644
--- a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FileImageDescriptor.java
+++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FileImageDescriptor.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2015 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Christoph Läubrich - Bug 567898 - [JFace][HiDPI] ImageDescriptor support alternative naming scheme for high dpi
  *******************************************************************************/
 package org.eclipse.jface.resource;
 
@@ -20,6 +21,8 @@
 import java.io.InputStream;
 import java.net.URL;
 import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.eclipse.core.runtime.FileLocator;
 import org.eclipse.core.runtime.IStatus;
@@ -39,6 +42,8 @@
  */
 class FileImageDescriptor extends ImageDescriptor {
 
+	private static final Pattern XPATH_PATTERN = Pattern.compile("(\\d+)x(\\d+)"); //$NON-NLS-1$
+
 	private class ImageProvider implements ImageFileNameProvider {
 		@Override
 		public String getImagePath(int zoom) {
@@ -94,31 +99,24 @@
 	 * <p>
 	 * The FileImageDescriptor implementation of this method is not used by
 	 * {@link ImageDescriptor#createImage(boolean, Device)} as of version
-	 * 3.4 so that the SWT OS optimised loading can be used.
+	 * 3.4 so that the SWT OS optimized loading can be used.
 	 */
 	@Override
 	public ImageData getImageData(int zoom) {
 		InputStream in = getStream(zoom);
-		ImageData result = null;
 		if (in != null) {
-			try {
-				result = new ImageData(in);
+			try (BufferedInputStream stream = new BufferedInputStream(in)) {
+				return new ImageData(stream);
 			} catch (SWTException e) {
 				if (e.code != SWT.ERROR_INVALID_IMAGE) {
 					throw e;
 					// fall through otherwise
 				}
-			} finally {
-				try {
-					in.close();
-				} catch (IOException e) {
-					// System.err.println(getClass().getName()+".getImageData():
-					// "+
-					// "Exception while closing InputStream : "+e);
-				}
+			} catch (IOException ioe) {
+				// fall through
 			}
 		}
-		return result;
+		return null;
 	}
 
 	/**
@@ -130,33 +128,62 @@
 	 *         file cannot be found
 	 */
 	private InputStream getStream(int zoom) {
-		String xName = getxName(name, zoom);
-		if (xName == null) {
-			return null;
+		if (zoom == 100) {
+			return getStream(name);
 		}
-		InputStream is = null;
 
-		if (location != null) {
-			is = location.getResourceAsStream(xName);
+		InputStream xstream = getStream(getxName(name, zoom));
+		if (xstream != null) {
+			return xstream;
+		}
 
-		} else {
+		InputStream xpath = getStream(getxPath(name, zoom));
+		if (xpath != null) {
+			return xpath;
+		}
+
+		return null;
+	}
+
+	/**
+	 * try to obtain a stream for a given name, if the name does not match a valid
+	 * resource null is returned
+	 *
+	 * @param fileName the filename to check
+	 * @return an {@link InputStream} to read from, or <code>null</code> if fileName
+	 *         does not denotes an existing resource
+	 */
+	private InputStream getStream(String fileName) {
+		if (fileName != null) {
+			if (location != null) {
+				return location.getResourceAsStream(fileName);
+			}
 			try {
-				is = new FileInputStream(xName);
+				return new FileInputStream(fileName);
 			} catch (FileNotFoundException e) {
 				return null;
 			}
 		}
-		if (is == null) {
-			return null;
+		return null;
+	}
+
+	static String getxPath(String name, int zoom) {
+		Matcher matcher = XPATH_PATTERN.matcher(name);
+		if (matcher.find()) {
+			try {
+				int current = Integer.parseInt(matcher.group(1));
+				int desired = (int) ((zoom / 100d) * current);
+				String lead = name.substring(0, matcher.start(1));
+				String tail = name.substring(matcher.end(2));
+				return lead + desired + "x" + desired + tail; //$NON-NLS-1$
+			} catch (RuntimeException e) {
+				// should never happen but if then we can't use the alternative name...
+			}
 		}
-		return new BufferedInputStream(is);
+		return null;
 	}
 
 	static String getxName(String name, int zoom) {
-		// see also URLImageDescriptor#getxURL(URL, int)
-		if (zoom == 100) {
-			return name;
-		}
 		int dot = name.lastIndexOf('.');
 		if (dot != -1 && (zoom == 150 || zoom == 200)) {
 			String lead = name.substring(0, dot);
diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/URLImageDescriptor.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/URLImageDescriptor.java
index eea96fe..d5b577b 100644
--- a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/URLImageDescriptor.java
+++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/URLImageDescriptor.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2016 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -11,6 +11,7 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Patrik Suzzi <psuzzi@gmail.com> - Bug 483465
+ *     Christoph Läubrich - Bug 567898 - [JFace][HiDPI] ImageDescriptor support alternative naming scheme for high dpi 
  *******************************************************************************/
 package org.eclipse.jface.resource;
 
@@ -74,7 +75,17 @@
 			if (tempURL != null) {
 				URL xUrl = getxURL(tempURL, zoom);
 				if (xUrl != null) {
-					return URLImageDescriptor.getImageData(xUrl);
+					ImageData xdata = URLImageDescriptor.getImageData(xUrl);
+					if (xdata != null) {
+						return xdata;
+					}
+				}
+				String xpath = FileImageDescriptor.getxPath(url, zoom);
+				if (xpath != null) {
+					URL xPathUrl = getURL(xpath);
+					if (xPathUrl != null) {
+						return URLImageDescriptor.getImageData(xPathUrl);
+					}
 				}
 			}
 			return null;
diff --git a/tests/org.eclipse.jface.tests/icons/imagetests/16x16/zoomIn.png b/tests/org.eclipse.jface.tests/icons/imagetests/16x16/zoomIn.png
new file mode 100644
index 0000000..c9766e0
--- /dev/null
+++ b/tests/org.eclipse.jface.tests/icons/imagetests/16x16/zoomIn.png
Binary files differ
diff --git a/tests/org.eclipse.jface.tests/icons/imagetests/32x32/zoomIn.png b/tests/org.eclipse.jface.tests/icons/imagetests/32x32/zoomIn.png
new file mode 100644
index 0000000..b18b291
--- /dev/null
+++ b/tests/org.eclipse.jface.tests/icons/imagetests/32x32/zoomIn.png
Binary files differ
diff --git a/tests/org.eclipse.jface.tests/icons/imagetests/zoomIn.png b/tests/org.eclipse.jface.tests/icons/imagetests/zoomIn.png
new file mode 100644
index 0000000..c9766e0
--- /dev/null
+++ b/tests/org.eclipse.jface.tests/icons/imagetests/zoomIn.png
Binary files differ
diff --git a/tests/org.eclipse.jface.tests/icons/imagetests/zoomIn@2x.png b/tests/org.eclipse.jface.tests/icons/imagetests/zoomIn@2x.png
new file mode 100644
index 0000000..b18b291
--- /dev/null
+++ b/tests/org.eclipse.jface.tests/icons/imagetests/zoomIn@2x.png
Binary files differ
diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/images/FileImageDescriptorTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/images/FileImageDescriptorTest.java
index ef751b4..cde773e 100644
--- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/images/FileImageDescriptorTest.java
+++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/images/FileImageDescriptorTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2017 IBM Corporation and others.
+ * Copyright (c) 2008, 2020 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -12,6 +12,7 @@
  *     IBM Corporation - initial API and implementation
  *     Karsten Stoeckmann <ngc2997@gmx.net> - Test case for Bug 220766
  *     		[JFace] ImageRegistry.get does not work as expected (crashes with NullPointerException)
+ *     Christoph Läubrich - Bug 567898 - [JFace][HiDPI] ImageDescriptor support alternative naming scheme for high dpi 
  ******************************************************************************/
 
 package org.eclipse.jface.tests.images;
@@ -26,6 +27,7 @@
 import org.eclipse.core.runtime.Path;
 import org.eclipse.jface.resource.ImageDescriptor;
 import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.graphics.ImageData;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.FrameworkUtil;
 
@@ -129,4 +131,24 @@
 		assertTrue("Did not find default image", image != null);
 	}
 
+	public void testGetxName() {
+		ImageDescriptor descriptor = ImageDescriptor.createFromFile(FileImageDescriptorTest.class,
+				"/icons/imagetests/zoomIn.png");
+		ImageData imageData = descriptor.getImageData(100);
+		assertNotNull(imageData);
+		ImageData imageDataZoomed = descriptor.getImageData(200);
+		assertNotNull(imageDataZoomed);
+		assertEquals(imageData.width * 2, imageDataZoomed.width);
+	}
+
+	public void testGetxPath() {
+		ImageDescriptor descriptor = ImageDescriptor.createFromFile(FileImageDescriptorTest.class,
+				"/icons/imagetests/16x16/zoomIn.png");
+		ImageData imageData = descriptor.getImageData(100);
+		assertNotNull(imageData);
+		ImageData imageDataZoomed = descriptor.getImageData(200);
+		assertNotNull(imageDataZoomed);
+		assertEquals(imageData.width * 2, imageDataZoomed.width);
+	}
+
 }
diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/images/UrlImageDescriptorTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/images/UrlImageDescriptorTest.java
new file mode 100644
index 0000000..97531bd
--- /dev/null
+++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/images/UrlImageDescriptorTest.java
@@ -0,0 +1,42 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Christoph Läubrich 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:
+ *     Christoph Läubrich - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.jface.tests.images;
+
+import org.eclipse.jface.resource.ImageDescriptor;
+import org.eclipse.swt.graphics.ImageData;
+
+import junit.framework.TestCase;
+
+public class UrlImageDescriptorTest extends TestCase {
+
+	public void testGetxName() {
+		ImageDescriptor descriptor = ImageDescriptor
+				.createFromURL(FileImageDescriptorTest.class.getResource("/icons/imagetests/zoomIn.png"));
+		ImageData imageData = descriptor.getImageData(100);
+		assertNotNull(imageData);
+		ImageData imageDataZoomed = descriptor.getImageData(200);
+		assertNotNull(imageDataZoomed);
+		assertEquals(imageData.width * 2, imageDataZoomed.width);
+	}
+
+	public void testGetxPath() {
+		ImageDescriptor descriptor = ImageDescriptor
+				.createFromURL(FileImageDescriptorTest.class.getResource("/icons/imagetests/16x16/zoomIn.png"));
+		ImageData imageData = descriptor.getImageData(100);
+		assertNotNull(imageData);
+		ImageData imageDataZoomed = descriptor.getImageData(200);
+		assertNotNull(imageDataZoomed);
+		assertEquals(imageData.width * 2, imageDataZoomed.width);
+	}
+}