Bug 175526 [Sync View] Multiple copies of same image created by sync view

4 files changed, 266 insertions, 120 deletions

diff --git a/bundles/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java b/bundles/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java
index 9aaf26ce9..de522fde9 100644
--- a/bundles/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java
+++ b/bundles/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java
@@ -17,7 +17,7 @@ import org.eclipse.compare.structuremergeviewer.Differencer;
 import org.eclipse.compare.structuremergeviewer.ICompareInput;
 import org.eclipse.core.runtime.ListenerList;
 import org.eclipse.jface.preference.IPreferenceStore;
-import org.eclipse.jface.resource.ImageDescriptor;
+import org.eclipse.jface.resource.*;
 import org.eclipse.jface.util.IPropertyChangeListener;
 import org.eclipse.jface.util.PropertyChangeEvent;
 import org.eclipse.jface.viewers.*;
@@ -58,8 +58,6 @@ public class CompareConfiguration {
 	public static final String USE_OUTLINE_VIEW= "USE_OUTLINE_VIEW"; //$NON-NLS-1$
 	
 	private static ImageDescriptor[] fgImages= new ImageDescriptor[16];
-	private static Object fgDummy= new Object();
-	private static HashMap fgMap= new HashMap(20);
 	private static boolean fLeftIsLocal= true;
 
 	static {
@@ -103,9 +101,10 @@ public class CompareConfiguration {
 	private Image fAncestorImage;
 	private Image fRightImage;
 	private Image fLeftImage;
-	private Image[] fImages= new Image[16];
 	private ICompareContainer fContainer;
 	private DefaultLabelProvider labelProvider = new DefaultLabelProvider();
+	private boolean fDisposed;
+	private LocalResourceManager fResourceManager;
 	
 	private class DefaultLabelProvider extends LabelProvider implements ICompareInputLabelProvider, ILabelProviderListener {
 		private Map labelProviders = new HashMap();
@@ -286,19 +285,20 @@ public class CompareConfiguration {
 	 * @since 2.0
 	 */
 	public Image getImage(int kind) {
-		if (fImages == null)
-			// The configuration has been disposed
+		if (fDisposed)
 			return null;
-		Image image= fImages[kind & 15];
-		if (image == null) {
-			ImageDescriptor id= fgImages[kind & 15];
-			if (id != null)				
-				image= id.createImage();
-			fImages[kind & 15]= image;
-		}
-		return image;
+		ImageDescriptor id= fgImages[kind & 15];
+		ResourceManager rm = getResourceManager();
+		return rm.createImage(id);
 	}
 	
+	private synchronized ResourceManager getResourceManager() {
+		if (fResourceManager == null) {
+			fResourceManager = new LocalResourceManager(JFaceResources.getResources());
+		}
+		return fResourceManager;
+	}
+
 	/**
 	 * Returns an image showing the specified change kind applied to a
 	 * given base image. The different kind of changes are defined in the <code>Differencer</code>.
@@ -313,25 +313,12 @@ public class CompareConfiguration {
 	 * @see org.eclipse.compare.structuremergeviewer.Differencer
 	 */
 	public Image getImage(Image base, int kind) {
-
-		Object key= base;
-		if (key == null)
-			key= fgDummy;
-
+		if (fDisposed)
+			return null;
 		kind &= 15;
-
-		Image[] a= (Image[]) fgMap.get(key);
-		if (a == null) {
-			a= new Image[16];
-			fgMap.put(key, a);
-		}
-		Image b= a[kind];
-		if (b == null) {
-			b= new DiffImage(base, fgImages[kind], ICompareUIConstants.COMPARE_IMAGE_WIDTH, !fLeftIsLocal).createImage();
-			CompareUI.disposeOnShutdown(b);
-			a[kind]= b;
-		}
-		return b;
+		ImageDescriptor id = new DiffImage(base, fgImages[kind], ICompareUIConstants.COMPARE_IMAGE_WIDTH, !fLeftIsLocal);
+		ResourceManager rm = getResourceManager();
+		return rm.createImage(id);
 	}
 	
 	/**
@@ -340,14 +327,10 @@ public class CompareConfiguration {
 	 * An implementation must dispose of all resources.
 	 */
 	public void dispose() {
-		if (fImages != null) {
-			for (int i= 0; i < fImages.length; i++){
-				Image image= fImages[i];
-				if (image != null && !image.isDisposed())
-					image.dispose();
-			}
+		fDisposed = true;
+		if (fResourceManager != null) {
+			fResourceManager.dispose();
 		}
-		fImages= null;
 		labelProvider.dispose();
 	}
 
diff --git a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java
index 0d94bf006..aaebca3ef 100644
--- a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java
+++ b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java
@@ -22,35 +22,63 @@ public class DiffImage extends CompositeImageDescriptor {
 
 	static final int HEIGHT= 16;
 
-	private Image fBaseImage;
-	private ImageDescriptor fOverlayImage;
-	private int fWidth;
-	private boolean fLeft= true;
-
-	public DiffImage(Image base, ImageDescriptor overlay, int w) {
-		fBaseImage= base;
-		fOverlayImage= overlay;
-		fWidth= w;
-	}
+	private final ImageData fBaseImageData;
+	private final ImageDescriptor fOverlayImage;
+	private final int fWidth;
+	private final boolean fLeft;
+	private final int hashCode;
 
 	public DiffImage(Image base, ImageDescriptor overlay, int w, boolean onLeft) {
-		fBaseImage= base;
+		ImageData data = null;
+		if (base != null) {
+			data = base.getImageData();
+			if (data == null)
+				data = DEFAULT_IMAGE_DATA;
+		}
+		fBaseImageData = data;
 		fOverlayImage= overlay;
 		fWidth= w;
 		fLeft= onLeft;
+		hashCode = calculateHashCode();
 	}
 
+	private int calculateHashCode() {
+		int h1 = 0;
+		int h2 = 0;
+		if (fBaseImageData != null) {
+			h1 = calculateHash(fBaseImageData);
+		}
+		if (fOverlayImage != null) {
+			h2 = fOverlayImage.hashCode();
+		}
+		return h1 + h2 + fWidth;
+	}
+	
+	private int calculateHash(ImageData baseImageData) {
+		byte[] data = baseImageData.data;
+		int hash = baseImageData.width + baseImageData.height;
+		for (int i = 0; i < data.length; i++) {
+			byte b = data[i];
+			hash >>>= 1;
+			hash ^= b;
+		}
+		return hash;
+	}
+
+	/* (non-Javadoc)
+	 * @see org.eclipse.jface.resource.CompositeImageDescriptor#getSize()
+	 */
 	protected Point getSize() {
 		return new Point(fWidth, HEIGHT);
 	}
 
+	/* (non-Javadoc)
+	 * @see org.eclipse.jface.resource.CompositeImageDescriptor#drawCompositeImage(int, int)
+	 */
 	protected void drawCompositeImage(int width, int height) {
 		if (fLeft) {
-			if (fBaseImage != null) {
-				ImageData base= fBaseImage.getImageData();
-				if (base == null)
-					base= DEFAULT_IMAGE_DATA;
-				drawImage(base, fWidth - base.width, 0);
+			if (fBaseImageData != null) {
+				drawImage(fBaseImageData, fWidth - fBaseImageData.width, 0);
 			}
 	
 			if (fOverlayImage != null) {
@@ -60,11 +88,8 @@ public class DiffImage extends CompositeImageDescriptor {
 				drawImage(overlay, 0, (HEIGHT - overlay.height) / 2);
 			}
 		} else {
-			if (fBaseImage != null) {
-				ImageData base= fBaseImage.getImageData();
-				if (base == null)
-					base= DEFAULT_IMAGE_DATA;
-				drawImage(base, 0, 0);
+			if (fBaseImageData != null) {
+				drawImage(fBaseImageData, 0, 0);
 			}
 	
 			if (fOverlayImage != null) {
@@ -75,4 +100,69 @@ public class DiffImage extends CompositeImageDescriptor {
 			}
 		}
 	}
+	
+	/* (non-Javadoc)
+	 * @see java.lang.Object#hashCode()
+	 */
+	public int hashCode() {
+		return hashCode;
+	}
+
+	/* (non-Javadoc)
+	 * @see java.lang.Object#equals(java.lang.Object)
+	 */
+	public boolean equals(Object obj) {
+		if (obj == this)
+			return true;
+		if (obj instanceof DiffImage) {
+			DiffImage other = (DiffImage) obj;
+			return (other.hashCode == hashCode
+					&& isEqual(other.fOverlayImage, fOverlayImage)
+					&& other.fWidth == fWidth 
+					&& other.fLeft == fLeft
+					&& isEqual(other.fBaseImageData, fBaseImageData));
+		}
+		return false;
+	}
+	
+	private boolean isEqual(ImageData i1, ImageData i2) {
+		if (isEqual((Object) i1, (Object) i2)) {
+			return true;
+		}
+		if (i1 == null || i2 == null)
+			return false;
+		return (i1.width == i2.width && i1.height == i2.height
+				&& i1.depth == i2.depth && i1.scanlinePad == i2.scanlinePad
+				&& i1.bytesPerLine == i2.bytesPerLine
+				/* && i1.palette == i2.palette */
+				&& i1.transparentPixel == i2.transparentPixel
+				&& i1.maskPad == i2.maskPad
+	            && i1.alpha == i2.alpha
+				&& i1.type == i2.type && i1.x == i2.x && i1.y == i2.y
+				&& i1.disposalMethod == i2.disposalMethod && i1.delayTime == i2.delayTime
+				&& equals(i1.data,i2.data) && equals(i1.maskData, i2.maskData)
+				&& equals(i1.alphaData, i2.alphaData));
+	}
+	
+	private boolean equals(byte[] data, byte[] data2) {
+		if (isEqual(data, data2))
+			return true;
+		if (data == null || data2 == null)
+			return false;
+		if (data.length != data2.length)
+			return false;
+		for (int i = 0; i < data2.length; i++) {
+			if (data[i] != data2[i])
+				return false;
+		}
+		return true;
+	}
+
+	private boolean isEqual(Object o1, Object o2) {
+		if (o1 == o2)
+			return true;
+		if (o1 == null || o2 == null)
+			return false;
+		return o1.equals(o2);
+	}
 }
diff --git a/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java b/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java
index 9aaf26ce9..de522fde9 100644
--- a/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java
+++ b/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/CompareConfiguration.java
@@ -17,7 +17,7 @@ import org.eclipse.compare.structuremergeviewer.Differencer;
 import org.eclipse.compare.structuremergeviewer.ICompareInput;
 import org.eclipse.core.runtime.ListenerList;
 import org.eclipse.jface.preference.IPreferenceStore;
-import org.eclipse.jface.resource.ImageDescriptor;
+import org.eclipse.jface.resource.*;
 import org.eclipse.jface.util.IPropertyChangeListener;
 import org.eclipse.jface.util.PropertyChangeEvent;
 import org.eclipse.jface.viewers.*;
@@ -58,8 +58,6 @@ public class CompareConfiguration {
 	public static final String USE_OUTLINE_VIEW= "USE_OUTLINE_VIEW"; //$NON-NLS-1$
 	
 	private static ImageDescriptor[] fgImages= new ImageDescriptor[16];
-	private static Object fgDummy= new Object();
-	private static HashMap fgMap= new HashMap(20);
 	private static boolean fLeftIsLocal= true;
 
 	static {
@@ -103,9 +101,10 @@ public class CompareConfiguration {
 	private Image fAncestorImage;
 	private Image fRightImage;
 	private Image fLeftImage;
-	private Image[] fImages= new Image[16];
 	private ICompareContainer fContainer;
 	private DefaultLabelProvider labelProvider = new DefaultLabelProvider();
+	private boolean fDisposed;
+	private LocalResourceManager fResourceManager;
 	
 	private class DefaultLabelProvider extends LabelProvider implements ICompareInputLabelProvider, ILabelProviderListener {
 		private Map labelProviders = new HashMap();
@@ -286,19 +285,20 @@ public class CompareConfiguration {
 	 * @since 2.0
 	 */
 	public Image getImage(int kind) {
-		if (fImages == null)
-			// The configuration has been disposed
+		if (fDisposed)
 			return null;
-		Image image= fImages[kind & 15];
-		if (image == null) {
-			ImageDescriptor id= fgImages[kind & 15];
-			if (id != null)				
-				image= id.createImage();
-			fImages[kind & 15]= image;
-		}
-		return image;
+		ImageDescriptor id= fgImages[kind & 15];
+		ResourceManager rm = getResourceManager();
+		return rm.createImage(id);
 	}
 	
+	private synchronized ResourceManager getResourceManager() {
+		if (fResourceManager == null) {
+			fResourceManager = new LocalResourceManager(JFaceResources.getResources());
+		}
+		return fResourceManager;
+	}
+
 	/**
 	 * Returns an image showing the specified change kind applied to a
 	 * given base image. The different kind of changes are defined in the <code>Differencer</code>.
@@ -313,25 +313,12 @@ public class CompareConfiguration {
 	 * @see org.eclipse.compare.structuremergeviewer.Differencer
 	 */
 	public Image getImage(Image base, int kind) {
-
-		Object key= base;
-		if (key == null)
-			key= fgDummy;
-
+		if (fDisposed)
+			return null;
 		kind &= 15;
-
-		Image[] a= (Image[]) fgMap.get(key);
-		if (a == null) {
-			a= new Image[16];
-			fgMap.put(key, a);
-		}
-		Image b= a[kind];
-		if (b == null) {
-			b= new DiffImage(base, fgImages[kind], ICompareUIConstants.COMPARE_IMAGE_WIDTH, !fLeftIsLocal).createImage();
-			CompareUI.disposeOnShutdown(b);
-			a[kind]= b;
-		}
-		return b;
+		ImageDescriptor id = new DiffImage(base, fgImages[kind], ICompareUIConstants.COMPARE_IMAGE_WIDTH, !fLeftIsLocal);
+		ResourceManager rm = getResourceManager();
+		return rm.createImage(id);
 	}
 	
 	/**
@@ -340,14 +327,10 @@ public class CompareConfiguration {
 	 * An implementation must dispose of all resources.
 	 */
 	public void dispose() {
-		if (fImages != null) {
-			for (int i= 0; i < fImages.length; i++){
-				Image image= fImages[i];
-				if (image != null && !image.isDisposed())
-					image.dispose();
-			}
+		fDisposed = true;
+		if (fResourceManager != null) {
+			fResourceManager.dispose();
 		}
-		fImages= null;
 		labelProvider.dispose();
 	}
 
diff --git a/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java b/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java
index 0d94bf006..aaebca3ef 100644
--- a/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java
+++ b/bundles/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffImage.java
@@ -22,35 +22,63 @@ public class DiffImage extends CompositeImageDescriptor {
 
 	static final int HEIGHT= 16;
 
-	private Image fBaseImage;
-	private ImageDescriptor fOverlayImage;
-	private int fWidth;
-	private boolean fLeft= true;
-
-	public DiffImage(Image base, ImageDescriptor overlay, int w) {
-		fBaseImage= base;
-		fOverlayImage= overlay;
-		fWidth= w;
-	}
+	private final ImageData fBaseImageData;
+	private final ImageDescriptor fOverlayImage;
+	private final int fWidth;
+	private final boolean fLeft;
+	private final int hashCode;
 
 	public DiffImage(Image base, ImageDescriptor overlay, int w, boolean onLeft) {
-		fBaseImage= base;
+		ImageData data = null;
+		if (base != null) {
+			data = base.getImageData();
+			if (data == null)
+				data = DEFAULT_IMAGE_DATA;
+		}
+		fBaseImageData = data;
 		fOverlayImage= overlay;
 		fWidth= w;
 		fLeft= onLeft;
+		hashCode = calculateHashCode();
 	}
 
+	private int calculateHashCode() {
+		int h1 = 0;
+		int h2 = 0;
+		if (fBaseImageData != null) {
+			h1 = calculateHash(fBaseImageData);
+		}
+		if (fOverlayImage != null) {
+			h2 = fOverlayImage.hashCode();
+		}
+		return h1 + h2 + fWidth;
+	}
+	
+	private int calculateHash(ImageData baseImageData) {
+		byte[] data = baseImageData.data;
+		int hash = baseImageData.width + baseImageData.height;
+		for (int i = 0; i < data.length; i++) {
+			byte b = data[i];
+			hash >>>= 1;
+			hash ^= b;
+		}
+		return hash;
+	}
+
+	/* (non-Javadoc)
+	 * @see org.eclipse.jface.resource.CompositeImageDescriptor#getSize()
+	 */
 	protected Point getSize() {
 		return new Point(fWidth, HEIGHT);
 	}
 
+	/* (non-Javadoc)
+	 * @see org.eclipse.jface.resource.CompositeImageDescriptor#drawCompositeImage(int, int)
+	 */
 	protected void drawCompositeImage(int width, int height) {
 		if (fLeft) {
-			if (fBaseImage != null) {
-				ImageData base= fBaseImage.getImageData();
-				if (base == null)
-					base= DEFAULT_IMAGE_DATA;
-				drawImage(base, fWidth - base.width, 0);
+			if (fBaseImageData != null) {
+				drawImage(fBaseImageData, fWidth - fBaseImageData.width, 0);
 			}
 	
 			if (fOverlayImage != null) {
@@ -60,11 +88,8 @@ public class DiffImage extends CompositeImageDescriptor {
 				drawImage(overlay, 0, (HEIGHT - overlay.height) / 2);
 			}
 		} else {
-			if (fBaseImage != null) {
-				ImageData base= fBaseImage.getImageData();
-				if (base == null)
-					base= DEFAULT_IMAGE_DATA;
-				drawImage(base, 0, 0);
+			if (fBaseImageData != null) {
+				drawImage(fBaseImageData, 0, 0);
 			}
 	
 			if (fOverlayImage != null) {
@@ -75,4 +100,69 @@ public class DiffImage extends CompositeImageDescriptor {
 			}
 		}
 	}
+	
+	/* (non-Javadoc)
+	 * @see java.lang.Object#hashCode()
+	 */
+	public int hashCode() {
+		return hashCode;
+	}
+
+	/* (non-Javadoc)
+	 * @see java.lang.Object#equals(java.lang.Object)
+	 */
+	public boolean equals(Object obj) {
+		if (obj == this)
+			return true;
+		if (obj instanceof DiffImage) {
+			DiffImage other = (DiffImage) obj;
+			return (other.hashCode == hashCode
+					&& isEqual(other.fOverlayImage, fOverlayImage)
+					&& other.fWidth == fWidth 
+					&& other.fLeft == fLeft
+					&& isEqual(other.fBaseImageData, fBaseImageData));
+		}
+		return false;
+	}
+	
+	private boolean isEqual(ImageData i1, ImageData i2) {
+		if (isEqual((Object) i1, (Object) i2)) {
+			return true;
+		}
+		if (i1 == null || i2 == null)
+			return false;
+		return (i1.width == i2.width && i1.height == i2.height
+				&& i1.depth == i2.depth && i1.scanlinePad == i2.scanlinePad
+				&& i1.bytesPerLine == i2.bytesPerLine
+				/* && i1.palette == i2.palette */
+				&& i1.transparentPixel == i2.transparentPixel
+				&& i1.maskPad == i2.maskPad
+	            && i1.alpha == i2.alpha
+				&& i1.type == i2.type && i1.x == i2.x && i1.y == i2.y
+				&& i1.disposalMethod == i2.disposalMethod && i1.delayTime == i2.delayTime
+				&& equals(i1.data,i2.data) && equals(i1.maskData, i2.maskData)
+				&& equals(i1.alphaData, i2.alphaData));
+	}
+	
+	private boolean equals(byte[] data, byte[] data2) {
+		if (isEqual(data, data2))
+			return true;
+		if (data == null || data2 == null)
+			return false;
+		if (data.length != data2.length)
+			return false;
+		for (int i = 0; i < data2.length; i++) {
+			if (data[i] != data2[i])
+				return false;
+		}
+		return true;
+	}
+
+	private boolean isEqual(Object o1, Object o2) {
+		if (o1 == o2)
+			return true;
+		if (o1 == null || o2 == null)
+			return false;
+		return o1.equals(o2);
+	}
 }