Skip to content

Execute org.eclipse.swt.graphics.GC call in UI thread in ShapePainter #157

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jul 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ This is a minor release.

### Fixed Issues

* [#148](https://github.com/pmd/pmd-eclipse-plugin/issues/148): Eclipse nearly-consistently crashes on startup when workspace contains PMD enabled projects
* [#150](https://github.com/pmd/pmd-eclipse-plugin/issues/150): Error executing command ReviewCode: java.util.regex.PatternSyntaxException: Illegal/unsupported escape sequence near index
* [#153](https://github.com/pmd/pmd-eclipse-plugin/issues/153): Not properly disposed SWT resource

Expand All @@ -28,6 +29,7 @@ This is a minor release.
* `net.sourceforge.pmd.eclipse.runtime.cmd.BaseVisitor.setUseTaskMarker(boolean)`
* `net.sourceforge.pmd.eclipse.ui.preferences.br.FilterManager`
* `net.sourceforge.pmd.eclipse.util.IOUtil`
* `net.sourceforge.pmd.eclipse.ui.priority.PriorityDescriptor.refreshImages()`
* All classes that couldn't be instantiated because they had a private constructor only
are now also `final`.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public static IViewPart getView(String id) {
* @param viewId id of the view
*/
public void refreshView(final String viewId) {
Display.getCurrent().asyncExec(new Runnable() {
Display.getDefault().asyncExec(new Runnable() {
@Override
public void run() {
try {
Expand Down Expand Up @@ -447,7 +447,7 @@ public void showError(final String message, final Throwable t) {
@Override
public void run() {
String errTitle = getStringTable().getString(StringKeys.ERROR_TITLE);
MessageDialog.openError(Display.getCurrent().getActiveShell(), errTitle,
MessageDialog.openError(Display.getDefault().getActiveShell(), errTitle,
message + "\n" + String.valueOf(t));
}
});
Expand All @@ -462,7 +462,7 @@ public void showUserError(final String message) {
@Override
public void run() {
String errTitle = getStringTable().getString(StringKeys.ERROR_TITLE);
MessageDialog.openError(Display.getCurrent().getActiveShell(), errTitle, message);
MessageDialog.openError(Display.getDefault().getActiveShell(), errTitle, message);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
*/
public class PageBuilder {
private static final char CR = '\n';
private static final Color BACKGROUND = Display.getCurrent().getSystemColor(SWT.COLOR_WHITE);
private static final Color BACKGROUND = Display.getDefault().getSystemColor(SWT.COLOR_WHITE);

private static final Comparator<StyleRange> STYLE_COMPARATOR = new Comparator<StyleRange>() {
@Override
Expand All @@ -62,7 +62,7 @@ public PageBuilder(int textIndent, int headingColorIndex, FontBuilder codeFontBu
buffer = new StringBuilder(500);
indentDepth = textIndent;

Display display = Display.getCurrent();
Display display = Display.getDefault();
headingColor = display.getSystemColor(headingColorIndex);
codeStyle = codeFontBuilder.style(display);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

package net.sourceforge.pmd.eclipse.ui;

import java.util.concurrent.atomic.AtomicReference;

import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.ImageData;
Expand All @@ -28,29 +30,38 @@ private ShapePainter() {
* drawn within. It will return cached images to avoid creating new images
* (and using the limited GDI handles under Windows).
*/
public static Image newDrawnImage(Display display, int width, int height, Shape shape, RGB transparentColour,
RGB fillColour) {
Image image = new Image(display, width, height);
GC gc = new GC(image);
public static Image newDrawnImage(final Display displayNotUsed, final int width, final int height, final Shape shape,
final RGB transparentColour, final RGB fillColour) {

final AtomicReference<Image> result = new AtomicReference<>();
Display.getDefault().syncExec(new Runnable() {
@Override
public void run() {
Display display = Display.getDefault();
Image image = new Image(display, width, height);
GC gc = new GC(image);

gc.setBackground(PMDPlugin.getDefault().colorFor(transparentColour));
gc.fillRectangle(0, 0, width, height);
gc.setBackground(PMDPlugin.getDefault().colorFor(transparentColour));
gc.fillRectangle(0, 0, width, height);

gc.setForeground(PMDPlugin.getDefault().colorFor(RGB_BLACK));
gc.setBackground(PMDPlugin.getDefault().colorFor(fillColour));
gc.setForeground(PMDPlugin.getDefault().colorFor(RGB_BLACK));
gc.setBackground(PMDPlugin.getDefault().colorFor(fillColour));

drawShape(width - 1, height - 1, shape, gc, 0, 0, null);
drawShape(width - 1, height - 1, shape, gc, 0, 0, null);

ImageData data = image.getImageData();
int clrIndex = data.palette.getPixel(transparentColour);
data.transparentPixel = clrIndex;
ImageData data = image.getImageData();
int clrIndex = data.palette.getPixel(transparentColour);
data.transparentPixel = clrIndex;

Image newImage = new Image(display, data);
image.dispose();
Image newImage = new Image(display, data);
image.dispose();

gc.dispose();
gc.dispose();

return newImage;
result.set(newImage);
}
});
return result.get();
}

@Deprecated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public void focusLost(FocusEvent e) {
private Color colourFor(int itemIndex) {
ShapeDescriptor desc = shapeDescriptorsByItem.get(items[itemIndex]);
if (desc == null) {
return Display.getCurrent().getSystemColor(SWT.COLOR_BLACK);
return Display.getDefault().getSystemColor(SWT.COLOR_BLACK);
}

RGB rgb = desc.rgbColor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void init(IViewPart view) {
@Override
public void run(IAction action) {
LOG.info("Remove violation reviews requested.");
ProgressMonitorDialog monitorDialog = new ProgressMonitorDialog(Display.getCurrent().getActiveShell());
ProgressMonitorDialog monitorDialog = new ProgressMonitorDialog(Display.getDefault().getActiveShell());
try {
monitorDialog.run(false, false, new IRunnableWithProgress() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ public class StyleExtractor {
private SyntaxData syntaxData;
private List<int[]> commentOffsets;

private static final Color COMMENT_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GREEN);
private static final Color REFERENCED_VAR_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GREEN);
private static final Color COMMENT_BACKGROUND = Display.getCurrent().getSystemColor(SWT.COLOR_WHITE);
private static final Color PUNCTUATION_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_BLACK);
private static final Color KEYWORD_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_DARK_MAGENTA);
private static final Color STRING_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_BLUE);
private static final Color COMMENT_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_DARK_GREEN);
private static final Color REFERENCED_VAR_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_DARK_GREEN);
private static final Color COMMENT_BACKGROUND = Display.getDefault().getSystemColor(SWT.COLOR_WHITE);
private static final Color PUNCTUATION_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_BLACK);
private static final Color KEYWORD_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_DARK_MAGENTA);
private static final Color STRING_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_BLUE);

public StyleExtractor(SyntaxData theSyntaxData) {
syntaxData = theSyntaxData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicReference;

import org.eclipse.jface.viewers.CheckboxTreeViewer;
import org.eclipse.jface.viewers.ISelectionChangedListener;
Expand Down Expand Up @@ -129,16 +130,21 @@ protected Map<Integer, List<Listener>> paintListeners() {
}

protected void createCheckBoxColumn(Tree tree) {

TreeColumn tc = new TreeColumn(tree, 0);

Image image = new Image(Display.getCurrent(), 15, 15);
GC gc = new GC(image);
Point textExtent = gc.textExtent("m");
gc.dispose();
image.dispose();
final AtomicReference<Point> textExtent = new AtomicReference<>();
Display.getDefault().syncExec(new Runnable() {
@Override
public void run() {
Image image = new Image(Display.getDefault(), 15, 15);
GC gc = new GC(image);
textExtent.set(gc.textExtent("m"));
gc.dispose();
image.dispose();
}
});

tc.setWidth(textExtent.x * 4);
tc.setWidth(textExtent.get().x * 4);

tc.setResizable(true);
tc.pack();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ protected AbstractEditorFactory() {
// private static ColourManager colourManager() {
//
// if (colourManager != null) return colourManager;
// colourManager = ColourManager.managerFor(Display.getCurrent());
// colourManager = ColourManager.managerFor(Display.getDefault());
// return colourManager;
// }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
package net.sourceforge.pmd.eclipse.ui.priority;

import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import org.apache.commons.lang3.StringUtils;
import org.eclipse.jface.resource.ImageDescriptor;
Expand Down Expand Up @@ -44,10 +46,6 @@ public PriorityDescriptor(RulePriority thePriority, String theLabelKey, String t
this(thePriority, theLabelKey, theFilterTextKey, theIconId, new ShapeDescriptor(theShape, theColor, theSize));
}

private PriorityDescriptor(RulePriority thePriority) {
priority = thePriority;
}

public PriorityDescriptor(RulePriority thePriority, String theLabelKey, String theFilterTextKey, String theIconId,
ShapeDescriptor theShape) {
priority = thePriority;
Expand Down Expand Up @@ -166,6 +164,7 @@ public PriorityDescriptor clone() {
copy.filterText = filterText;
copy.iconId = iconId;
copy.shape = shape.clone();
copy.cachedImages = new ConcurrentHashMap<>(3);
return copy;
} catch (CloneNotSupportedException e) {
throw new RuntimeException(e);
Expand All @@ -185,7 +184,7 @@ public Image getImage(Display display) {
*/
@Deprecated
public Image getImage(Display display, int maxDimension) {
return ShapePainter.newDrawnImage(display, Math.min(shape.size, maxDimension),
return ShapePainter.newDrawnImage(null, Math.min(shape.size, maxDimension),
Math.min(shape.size, maxDimension), shape.shape, PROTO_TRANSPARENT_COLOR, shape.rgbColor // fillColour
);
}
Expand All @@ -203,7 +202,7 @@ public String toString() {
return sb.toString();
}

private Map<Integer, Image> cachedImages = new HashMap<>(3);
private ConcurrentMap<Integer, Image> cachedImages = new ConcurrentHashMap<>(3);
private static final int ANNOTATION_IMAGE_DIMENSION = 9;

public Image getAnnotationImage() {
Expand All @@ -216,7 +215,7 @@ public ImageDescriptor getAnnotationImageDescriptor() {

private Image createImage(final int size) {
if (iconId == null || iconId.isEmpty() || "null".equals(iconId)) {
return ShapePainter.newDrawnImage(Display.getCurrent(), size, size, shape.shape, PROTO_TRANSPARENT_COLOR,
return ShapePainter.newDrawnImage(null, size, size, shape.shape, PROTO_TRANSPARENT_COLOR,
shape.rgbColor // fillColour
);
} else {
Expand All @@ -239,8 +238,16 @@ private Image createImage(final int size) {
public Image getImage(int size) {
Image cached = cachedImages.get(size);
if (cached == null) {
cached = createImage(size);
cachedImages.put(size, cached);
Image newImage = createImage(size);
cached = cachedImages.putIfAbsent(size, newImage);
if (cached == null) {
cached = newImage;
} else {
// two threads might have created the same image, only one is needed
// use the already cached image and dispose the new image, that was
// unnecessarily created
newImage.dispose();
}
}
return cached;
}
Expand All @@ -259,24 +266,20 @@ public Image createImage() {
}

public void dispose() {
for (Image image : cachedImages.values()) {
Set<Image> copy = new HashSet<>(cachedImages.values());
cachedImages.clear();

for (Image image : copy) {
image.dispose();
}
cachedImages.clear();
}

/**
* Eagerly create the images.
* @deprecated not used anymore, it does nothing anymore. The images are created on demand now.
*/
@Deprecated
public void refreshImages() {
dispose();
Image image = getAnnotationImage();
if (image == null) {
throw new RuntimeException("Could not create annotation image");
}
image = getImage(16);
if (image == null) {
throw new RuntimeException("Could not create marker image");
}
// does nothing
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
package net.sourceforge.pmd.eclipse.ui.priority;

import java.io.PrintStream;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import net.sourceforge.pmd.RulePriority;
import net.sourceforge.pmd.eclipse.plugin.PMDPlugin;
Expand All @@ -19,12 +20,11 @@
* @author Brian Remedios
*/
public final class PriorityDescriptorCache {
private Map<RulePriority, PriorityDescriptor> uiDescriptorsByPriority;
private final ConcurrentMap<RulePriority, PriorityDescriptor> uiDescriptorsByPriority = new ConcurrentHashMap<>(RulePriority.values().length);

public static final PriorityDescriptorCache INSTANCE = new PriorityDescriptorCache();

private PriorityDescriptorCache() {
uiDescriptorsByPriority = new HashMap<>(RulePriority.values().length);
loadFromPreferences();
}

Expand All @@ -47,7 +47,6 @@ public void loadFromPreferences() {
// preferences, but the user might cancel.
uiDescriptorsByPriority.put(rp, preferences.getPriorityDescriptor(rp).clone());
}
refreshImages();
}

public void storeInPreferences() {
Expand All @@ -61,8 +60,8 @@ public void storeInPreferences() {
prefs.setPriorityDescriptor(entry.getKey(), entry.getValue().clone());
}
prefs.sync();
// recreate images with the changed settings
refreshImages();
// remove old images so that the images are recreated with the changed settings
dispose();
}

public PriorityDescriptor descriptorFor(RulePriority priority) {
Expand All @@ -89,10 +88,4 @@ public void dispose() {
pd.dispose();
}
}

private void refreshImages() {
for (PriorityDescriptor pd : uiDescriptorsByPriority.values()) {
pd.refreshImages();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private TreeViewer getViewer() {
@Override
public void run() {
try {
final ProgressMonitorDialog dialog = new ProgressMonitorDialog(Display.getCurrent().getActiveShell());
final ProgressMonitorDialog dialog = new ProgressMonitorDialog(Display.getDefault().getActiveShell());
dialog.setCancelable(true);

dialog.run(false, false, new IRunnableWithProgress() {
Expand Down
Loading