Skip to content

Commit b54aefd

Browse files
committed
Only cache the images in PriorityDescriptor
Fix image size in ViolationOverview
1 parent b2520ad commit b54aefd

File tree

7 files changed

+71
-54
lines changed

7 files changed

+71
-54
lines changed

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/plugin/PMDPlugin.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import net.sourceforge.pmd.eclipse.ui.ShapePainter;
8282
import net.sourceforge.pmd.eclipse.ui.nls.StringKeys;
8383
import net.sourceforge.pmd.eclipse.ui.nls.StringTable;
84+
import net.sourceforge.pmd.eclipse.ui.priority.PriorityDescriptorCache;
8485
import net.sourceforge.pmd.eclipse.util.ResourceManager;
8586
import net.sourceforge.pmd.lang.LanguageRegistry;
8687
import net.sourceforge.pmd.lang.LanguageVersion;
@@ -330,8 +331,8 @@ public static IViewPart getView(String id) {
330331
*
331332
* @param viewId id of the view
332333
*/
333-
public void refreshView(final String viewId) {
334-
Display.getDefault().asyncExec(new Runnable() {
334+
public void refreshView(final String viewId) {
335+
Display.getCurrent().asyncExec(new Runnable() {
335336
@Override
336337
public void run() {
337338
try {
@@ -371,6 +372,7 @@ public void stop(BundleContext context) throws Exception {
371372
disposeResources();
372373
ShapePainter.disposeAll();
373374
ResourceManager.dispose();
375+
PriorityDescriptorCache.INSTANCE.dispose();
374376
super.stop(context);
375377
}
376378

@@ -734,5 +736,4 @@ public void removedMarkersIn(IResource resource) {
734736

735737
decorator.changed(changes);
736738
}
737-
738739
}

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/ShapePainter.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44

55
package net.sourceforge.pmd.eclipse.ui;
66

7-
import java.util.HashMap;
8-
import java.util.Map;
9-
107
import org.eclipse.swt.SWT;
118
import org.eclipse.swt.graphics.GC;
129
import org.eclipse.swt.graphics.Image;
@@ -22,10 +19,6 @@
2219
*
2320
*/
2421
public class ShapePainter {
25-
26-
/** Provides a simple cache for the images. */
27-
private static Map<String, Image> shapes = new HashMap<String, Image>();
28-
2922
private ShapePainter() {
3023
}
3124

@@ -36,11 +29,6 @@ private ShapePainter() {
3629
*/
3730
public static Image newDrawnImage(Display display, int width, int height, Shape shape, RGB transparentColour,
3831
RGB fillColour) {
39-
String key = width + "x" + height + " " + shape + " " + transparentColour + " " + fillColour;
40-
if (shapes.containsKey(key)) {
41-
return shapes.get(key);
42-
}
43-
4432
Image image = new Image(display, width, height);
4533
GC gc = new GC(image);
4634

@@ -61,15 +49,11 @@ public static Image newDrawnImage(Display display, int width, int height, Shape
6149

6250
gc.dispose();
6351

64-
shapes.put(key, newImage);
6552
return newImage;
6653
}
6754

55+
@Deprecated
6856
public static void disposeAll() {
69-
for (Image i : shapes.values()) {
70-
i.dispose();
71-
}
72-
shapes.clear();
7357
}
7458

7559
public static void drawShape(int width, int height, Shape shapeId, GC gc, int x, int y, String optionalText) {

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/preferences/GeneralPreferencesPage.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -823,21 +823,13 @@ private void updateMarkerIcons() {
823823

824824
PriorityDescriptorCache.INSTANCE.storeInPreferences();
825825

826-
// ensure that the decorator gets these new images...
827-
RuleLabelDecorator decorator = PMDPlugin.getDefault().ruleLabelDecorator();
828-
if (decorator != null) {
829-
decorator.reloadDecorators();
830-
}
831-
832826
RootRecord root = new RootRecord(ResourcesPlugin.getWorkspace().getRoot());
833827
Set<IFile> files = MarkerUtil.allMarkedFiles(root);
834828
PMDPlugin.getDefault().changedFiles(files);
835829

836830
/* Refresh the views to pick up the marker change */
837831
PMDPlugin.getDefault().refreshView(PMDPlugin.VIOLATIONS_OVERVIEW_ID);
838832
PMDPlugin.getDefault().refreshView(PMDPlugin.VIOLATIONS_OUTLINE_ID);
839-
PMDPlugin.getDefault().refreshView(IPageLayout.ID_PROJECT_EXPLORER);
840-
PMDPlugin.getDefault().refreshView(IPageLayout.ID_OUTLINE);
841833
}
842834

843835
public boolean performCancel() {

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/priority/PriorityDescriptor.java

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
package net.sourceforge.pmd.eclipse.ui.priority;
66

77
import java.util.EnumSet;
8+
import java.util.HashMap;
9+
import java.util.Map;
810

911
import org.eclipse.jface.resource.ImageDescriptor;
1012
import org.eclipse.swt.graphics.Image;
@@ -172,7 +174,7 @@ public Image getImage(Display display, int maxDimension) {
172174

173175
public String toString() {
174176
StringBuilder sb = new StringBuilder();
175-
sb.append("RuleDescriptor: ");
177+
sb.append("PriorityDescriptor: ");
176178
sb.append(priority).append(", ");
177179
sb.append(label).append(", ");
178180
sb.append(description).append(", ");
@@ -182,37 +184,68 @@ public String toString() {
182184
return sb.toString();
183185
}
184186

187+
private Map<Integer, Image> cachedImages = new HashMap<>(3);
185188
private static final int ANNOTATION_IMAGE_DIMENSION = 9;
186-
private Image cachedAnnotationImage = null;
187-
private ImageDescriptor cachedAnnotationImageDescriptor = null;
188-
public void createAnnotationImageDescriptor() {
189-
if (cachedAnnotationImage != null) {
190-
cachedAnnotationImage.dispose();
191-
}
192-
cachedAnnotationImage = createAnnotationImage();
193-
cachedAnnotationImageDescriptor = ImageDescriptor.createFromImage(cachedAnnotationImage);
194-
}
195189
public Image getAnnotationImage() {
196-
return cachedAnnotationImageDescriptor.createImage();
190+
return getImage(ANNOTATION_IMAGE_DIMENSION);
197191
}
198192
public ImageDescriptor getAnnotationImageDescriptor() {
199-
return cachedAnnotationImageDescriptor;
193+
return ImageDescriptor.createFromImage(getAnnotationImage());
200194
}
201-
private Image createAnnotationImage() {
202-
return ShapePainter.newDrawnImage(Display.getCurrent(), Math.min(shape.size, ANNOTATION_IMAGE_DIMENSION),
203-
Math.min(shape.size, ANNOTATION_IMAGE_DIMENSION), shape.shape, PROTO_TRANSPARENT_COLOR, shape.rgbColor // fillColour
195+
private Image createImage(final int size) {
196+
return ShapePainter.newDrawnImage(Display.getCurrent(), size, size, shape.shape, PROTO_TRANSPARENT_COLOR,
197+
shape.rgbColor // fillColour
204198
);
205199
}
200+
/**
201+
* Gets the marker image in a specific size. The image is cached and reused.
202+
* @param size
203+
* @return
204+
*/
205+
public Image getImage(int size) {
206+
Image cached = cachedImages.get(size);
207+
if (cached == null) {
208+
cached = createImage(size);
209+
cachedImages.put(size, cached);
210+
}
211+
return cached;
212+
}
206213

207214
/**
208215
* Creates a new image with the current setting. This is needed during the configuration, when
209216
* the priority descriptor is changed, but not persisted yet. The cached image returned by
210217
* {@link #getAnnotationImage()} would not reflect that change and a preview is not possible.
218+
*
219+
* <p>The image is not cached.
211220
* @return
212221
*/
213222
public Image createImage() {
214-
return ShapePainter.newDrawnImage(Display.getCurrent(), shape.size, shape.size, shape.shape, PROTO_TRANSPARENT_COLOR,
215-
shape.rgbColor // fillColour
216-
);
223+
return createImage(shape.size);
224+
}
225+
226+
public void dispose() {
227+
for (Image image : cachedImages.values()) {
228+
image.dispose();
229+
}
230+
cachedImages.clear();
231+
}
232+
233+
/**
234+
* Eagerly create the images. This must be called in a UI thread!
235+
*/
236+
public void refreshImages() {
237+
if (Display.getCurrent() == null) {
238+
throw new IllegalStateException("Must be called in the UI thread");
239+
}
240+
241+
dispose();
242+
Image image = getAnnotationImage();
243+
if (image == null) {
244+
throw new RuntimeException("Could not create annotation image");
245+
}
246+
image = getImage(16);
247+
if (image == null) {
248+
throw new RuntimeException("Could not create marker image");
249+
}
217250
}
218251
}

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/priority/PriorityDescriptorCache.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,13 @@
1919
* @author Brian Remedios
2020
*/
2121
public class PriorityDescriptorCache {
22-
2322
private Map<RulePriority, PriorityDescriptor> uiDescriptorsByPriority;
2423

2524
public static final PriorityDescriptorCache INSTANCE = new PriorityDescriptorCache();
2625

2726
private PriorityDescriptorCache() {
2827
uiDescriptorsByPriority = new HashMap<RulePriority, PriorityDescriptor>(RulePriority.values().length);
2928
loadFromPreferences();
30-
recreateAnnotationImages();
3129
}
3230

3331
private IPreferencesManager preferencesManager() {
@@ -46,6 +44,7 @@ public void loadFromPreferences() {
4644
for (RulePriority rp : UISettings.currentPriorities(true)) {
4745
uiDescriptorsByPriority.put(rp, preferences.getPriorityDescriptor(rp).clone());
4846
}
47+
refreshImages();
4948
}
5049

5150
public void storeInPreferences() {
@@ -56,15 +55,15 @@ public void storeInPreferences() {
5655
prefs.setPriorityDescriptor(entry.getKey(), entry.getValue());
5756
}
5857
mgr.storePreferences(prefs);
59-
recreateAnnotationImages();
58+
// recreate images with the changed settings
59+
refreshImages();
6060
}
6161

6262
public PriorityDescriptor descriptorFor(RulePriority priority) {
6363
return uiDescriptorsByPriority.get(priority);
6464
}
6565

6666
public boolean hasChanges() {
67-
6867
IPreferences preferences = preferencesManager().reloadPreferences();
6968

7069
for (RulePriority rp : UISettings.currentPriorities(true)) {
@@ -78,9 +77,15 @@ public boolean hasChanges() {
7877
return false;
7978
}
8079

81-
private void recreateAnnotationImages() {
80+
public void dispose() {
8281
for (PriorityDescriptor pd : uiDescriptorsByPriority.values()) {
83-
pd.createAnnotationImageDescriptor();
82+
pd.dispose();
8483
}
8584
}
85+
86+
private void refreshImages() {
87+
for (PriorityDescriptor pd : uiDescriptorsByPriority.values()) {
88+
pd.refreshImages();
89+
}
90+
}
8691
}

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/views/ViolationOverviewLabelProvider.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ public Image getColumnImage(Object element, int columnIndex) {
6363
} else if (element instanceof MarkerRecord) {
6464
MarkerRecord markerRecord = (MarkerRecord) element;
6565
int priority = markerRecord.getPriority();
66-
image = PriorityDescriptorCache.INSTANCE.descriptorFor(RulePriority.valueOf(priority)).getAnnotationImage();
66+
// image must be 16x16 pixels, since the other images in the table are 16x16 as well (ICON_PACKAGE)
67+
image = PriorityDescriptorCache.INSTANCE.descriptorFor(RulePriority.valueOf(priority)).getImage(16);
6768
}
6869
}
6970

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/views/actions/PriorityFilterAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import org.apache.log4j.Logger;
88
import org.eclipse.jface.action.Action;
9+
import org.eclipse.jface.resource.ImageDescriptor;
910
import org.eclipse.jface.viewers.ViewerFilter;
1011

1112
import net.sourceforge.pmd.RulePriority;
@@ -79,7 +80,7 @@ private void setFilterFrom(ViewerFilter[] filters) {
7980
*/
8081
private void setupActionLook() {
8182
PriorityDescriptor desc = PriorityDescriptorCache.INSTANCE.descriptorFor(priority);
82-
setImageDescriptor(desc.getAnnotationImageDescriptor());
83+
setImageDescriptor(ImageDescriptor.createFromImage(desc.getImage(16)));
8384
setText(desc.label);
8485
String toolTip = String.format(desc.filterText, UISettings.labelFor(priority));
8586
setToolTipText(toolTip);

0 commit comments

Comments
 (0)