Skip to content

Commit

Permalink
Minor code and doc improvements in jface.resource
Browse files Browse the repository at this point in the history
  • Loading branch information
HannesWell committed Jun 23, 2023
1 parent 91e19e2 commit ed8ec90
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 154 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
*******************************************************************************/
package org.eclipse.jface.resource;

import java.util.Collection;
import java.util.HashMap;
import java.util.Map.Entry;
import java.util.Map;

/**
* Abstract implementation of ResourceManager. Maintains reference counts for all previously
Expand All @@ -30,13 +29,13 @@ abstract class AbstractResourceManager extends ResourceManager {
/**
* Map of ResourceDescriptor onto RefCount. (null when empty)
*/
private HashMap<DeviceResourceDescriptor, RefCount> map = null;
private Map<DeviceResourceDescriptor, RefCount> map = null;

/**
* Holds a reference count for a previously-allocated resource
*/
private static class RefCount {
Object resource;
final Object resource;
int count = 1;

RefCount(Object resource) {
Expand Down Expand Up @@ -131,16 +130,7 @@ public void dispose() {
if (map == null) {
return;
}

Collection<Entry<DeviceResourceDescriptor, RefCount>> entries = map.entrySet();

for (Entry<DeviceResourceDescriptor, RefCount> next : entries) {
Object key = next.getKey();
RefCount val = next.getValue();

deallocate(val.resource, (DeviceResourceDescriptor)key);
}

map.forEach((key, val) -> deallocate(val.resource, key));
map = null;
}

Expand All @@ -150,8 +140,9 @@ public Object find(DeviceResourceDescriptor descriptor) {
return null;
}
RefCount refCount = map.get(descriptor);
if (refCount == null)
if (refCount == null) {
return null;
}
return refCount.resource;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,7 @@ public int hashCode() {

@Override
public boolean equals(Object arg0) {
if (arg0 instanceof DerivedImageDescriptor) {
DerivedImageDescriptor desc = (DerivedImageDescriptor)arg0;

return desc.original.equals(original) && flags == desc.flags;
}

return false;
return arg0 instanceof DerivedImageDescriptor desc && desc.original.equals(original) && flags == desc.flags;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
*/
public final class DeviceResourceManager extends AbstractResourceManager {

private Device device;
private final Device device;
private Image missingImage;

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,21 @@
*/
class ImageDataImageDescriptor extends ImageDescriptor {

private ImageDataProvider dataProvider;
private final ImageDataProvider dataProvider;

/**
* Original image being described, or null if this image is described
* completely using its ImageData
*/
private Image originalImage = null;
private final Image originalImage;

/**
* Creates an image descriptor, given an image and the device it was created on.
*
* @param originalImage
*/
ImageDataImageDescriptor(Image originalImage) {
this(originalImage::getImageData);
this.dataProvider = originalImage::getImageData;
this.originalImage = originalImage;
}

Expand All @@ -60,30 +60,26 @@ class ImageDataImageDescriptor extends ImageDescriptor {
*/
ImageDataImageDescriptor(ImageDataProvider provider) {
dataProvider = provider;
originalImage = null;
}

@Override
public Object createResource(Device device) throws DeviceResourceException {

// If this descriptor is based on an existing image, then we can return the original image
// if this is the same device.
if (originalImage != null) {
if (originalImage != null && originalImage.getDevice() == device) {
// If we're allocating on the same device as the original image, return the original.
if (originalImage.getDevice() == device) {
return originalImage;
}
return originalImage;
}

return super.createResource(device);
}

@Override
public void destroyResource(Object previouslyCreatedObject) {
if (previouslyCreatedObject == originalImage) {
return;
if (previouslyCreatedObject != originalImage) {
super.destroyResource(previouslyCreatedObject);
}

super.destroyResource(previouslyCreatedObject);
}

@Override
Expand All @@ -101,20 +97,16 @@ public int hashCode() {

@Override
public boolean equals(Object obj) {
if (!(obj instanceof ImageDataImageDescriptor)) {
if (!(obj instanceof ImageDataImageDescriptor imgWrap)) {
return false;
}

ImageDataImageDescriptor imgWrap = (ImageDataImageDescriptor) obj;

//Intentionally using == instead of equals() as Image.hashCode() changes
//when the image is disposed and so leaks may occur with equals()

if (originalImage != null) {
return imgWrap.originalImage == originalImage;
}

return (imgWrap.originalImage == null && dataProvider.equals(imgWrap.dataProvider));
return imgWrap.originalImage == null && dataProvider.equals(imgWrap.dataProvider);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.MissingResourceException;
import java.util.Objects;
import java.util.ResourceBundle;
import java.util.function.Supplier;

import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.FileLocator;
Expand Down Expand Up @@ -199,7 +198,7 @@ public static ColorRegistry getColorRegistry() {
* 300 is big enough to cache common images of eclipse IDE, and small enough to
* not blow OS.
*/
private static final int cacheSize = Integer.getInteger("org.eclipse.jface.resource.cacheSize", 300).intValue(); //$NON-NLS-1$
private static final int CACHE_SIZE = Integer.getInteger("org.eclipse.jface.resource.cacheSize", 300).intValue(); //$NON-NLS-1$

/**
* Returns the global resource manager for the given display
Expand All @@ -214,20 +213,18 @@ public static ResourceManager getResources(final Display toQuery) {
ResourceManager reg = registries.get(toQuery);

if (reg == null) {
final ResourceManager mgr;
if (cacheSize == 0) {
mgr = new DeviceResourceManager(toQuery);
if (CACHE_SIZE == 0) {
reg = new DeviceResourceManager(toQuery);
} else {
mgr = new LazyResourceManager(cacheSize, new DeviceResourceManager(toQuery));
reg = new LazyResourceManager(CACHE_SIZE, new DeviceResourceManager(toQuery));
}
reg = mgr;
registries.put(toQuery, mgr);
registries.put(toQuery, reg);
final ResourceManager mgr = reg;
toQuery.disposeExec(() -> {
mgr.dispose();
registries.remove(toQuery);
});
}

return reg;
}

Expand Down Expand Up @@ -482,17 +479,15 @@ private static void initializeDefaultImages() {
private static final void declareImage(Object bundle, String key, String path, Class<?> fallback,
String fallbackPath) {

Supplier<URL> supplier = () -> {
imageRegistry.put(key, ImageDescriptor.createFromURLSupplier(false, () -> {
if (bundle != null) {
URL url = FileLocator.find((Bundle) bundle, new Path(path), null);
if (url != null)
if (url != null) {
return url;
}
}
URL url = fallback.getResource(fallbackPath);
return url;
};

imageRegistry.put(key, ImageDescriptor.createFromURLSupplier(false, supplier));
return fallback.getResource(fallbackPath);
}));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,13 @@ protected Image getDefaultImage() {
return parent.getDefaultImage();
}

private static Integer createOrIncrease(@SuppressWarnings("unused") DeviceResourceDescriptor key, Integer refs) {
return Integer.valueOf(refs == null ? 1 : refs.intValue() + 1);
}

private static Integer decreaseOrRemove(@SuppressWarnings("unused") DeviceResourceDescriptor key, Integer refs) {
return refs.intValue() == 1 ? null : Integer.valueOf(refs.intValue() - 1);

}
@Override
public Object create(DeviceResourceDescriptor descriptor) {
if (!shouldBeCached(descriptor)) {
return parent.create(descriptor);
}
int updatedRefs = refCount.compute(descriptor, LazyResourceManager::createOrIncrease).intValue();
@SuppressWarnings("boxing")
int updatedRefs = refCount.compute(descriptor, (k, refs) -> refs == null ? 1 : refs + 1);
if (updatedRefs == 1) {
ResourceManager cached = unreferenced.remove(descriptor);
if (cached == null) {
Expand All @@ -115,7 +108,8 @@ public void destroy(DeviceResourceDescriptor descriptor) {
parent.destroy(descriptor);
return;
}
Integer refsLeft = refCount.computeIfPresent(descriptor, LazyResourceManager::decreaseOrRemove);
@SuppressWarnings("boxing")
Integer refsLeft = refCount.computeIfPresent(descriptor, (k, refs) -> refs == 1 ? null : (refs - 1));
if (refsLeft == null) {
// defer destroy:
ResourceManager old = unreferenced.put(descriptor, parent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,12 @@ public Device getDevice() {
}

@Override
protected Object allocate(DeviceResourceDescriptor descriptor)
throws DeviceResourceException {
protected Object allocate(DeviceResourceDescriptor descriptor) throws DeviceResourceException {
return parentRegistry.create(descriptor);
}

@Override
protected void deallocate(Object resource,
DeviceResourceDescriptor descriptor) {

protected void deallocate(Object resource, DeviceResourceDescriptor descriptor) {
parentRegistry.destroy(descriptor);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@
*/
class RGBColorDescriptor extends ColorDescriptor {

private RGB color;
private final RGB color;

/**
* Color being copied, or null if none
*/
private Color originalColor = null;
private final Color originalColor;

/**
* Creates a new RGBColorDescriptor given some RGB values
Expand All @@ -38,6 +38,7 @@ class RGBColorDescriptor extends ColorDescriptor {
*/
public RGBColorDescriptor(RGB color) {
this.color = color;
this.originalColor = null;
}

/**
Expand All @@ -48,19 +49,14 @@ public RGBColorDescriptor(RGB color) {
* @param originalColor a color to describe
*/
public RGBColorDescriptor(Color originalColor) {
this(originalColor.getRGB());
this.color = originalColor.getRGB();
this.originalColor = originalColor;
}

@Override
public boolean equals(Object obj) {
if (obj instanceof RGBColorDescriptor) {
RGBColorDescriptor other = (RGBColorDescriptor) obj;

return other.color.equals(color) && other.originalColor == originalColor;
}

return false;
return obj instanceof RGBColorDescriptor other //
&& other.color.equals(color) && other.originalColor == originalColor;
}

@Override
Expand All @@ -72,13 +68,10 @@ public int hashCode() {
public Color createColor(Device device) {
// If this descriptor is wrapping an existing color, then we can return the original color
// if this is the same device.
if (originalColor != null) {
if (originalColor != null && originalColor.getDevice() == device) {
// If we're allocating on the same device as the original color, return the original.
if (originalColor.getDevice() == device) {
return originalColor;
}
return originalColor;
}

return new Color(device, color);
}
}
Loading

0 comments on commit ed8ec90

Please sign in to comment.