Skip to content

Commit

Permalink
Refactor FormatPainter code and don't reference the original diagram …
Browse files Browse the repository at this point in the history 
…object

- Don't reference the original diagram object in case the user closes its containing model. Instead make a snapshot copy of it. It might be that the original object is in a large model consuming a lot of memory. If the user closes that model it could not be garbage collected as we would hold a reference to it.

- This has the side effect that we now make a snapshot of the object when the FormatPainter copies the object. Thereafter, any pasted attributes come from the snapshot, whereas before if the user changed an attribute of the original object that would be pasted.

- When pasting the fill color and the source or target fill is null ("default"), compare on actual colors. This avoids the problem of copying a null fill color and pasting an actual fill color on an object with the same default fill color.

- We're not going to sub-class FormatPainterToolEntry so remove that support. This was added in 2011 and, 13 years later, we haven't sub-classed it. :-)

- Get rid of PaintFormat inner class

- Modernise code
  • Loading branch information
@Phillipus
Phillipus committed Aug 1, 2024
1 parent 520caa2 commit 18ea669
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 178 deletions.
139 changes: 77 additions & 62 deletions com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,85 +23,52 @@


/**
* FormatPainter Singleton state of format and cursor
* FormatPainter Singleton state of format and cursor.
*
* This is a a global class that stores the component to copy formatting from and the cursor to be used
* by each instance of FormatPainterToolEntry and FormatPainterTool.
*
* Via the listener list it notifies each instance of FormatPainterToolEntry so that each can update
* labels, icons and cursors.
*
* @author Phillip Beauvoir
*/
public class FormatPainterInfo {

public static FormatPainterInfo INSTANCE = new FormatPainterInfo();

/**
* Paint format information containing cursor color and source component
* Global static instance
*/
public static class PaintFormat {
private IDiagramModelComponent component;
private RGB cursorColor;

public PaintFormat(IDiagramModelComponent component) {
this.component = component;

// Default
cursorColor = new RGB(255, 255, 255);

if(component instanceof IDiagramModelConnection) {
// Line color
String colorValue = ((IDiagramModelConnection)component).getLineColor();
cursorColor = ColorFactory.convertStringToRGB(colorValue);
if(cursorColor == null) {
cursorColor = ColorFactory.getDefaultLineColor(component).getRGB();
}
}
else if(component instanceof IDiagramModelObject) {
// Fill color
String colorValue = ((IDiagramModelObject)component).getFillColor();
cursorColor = ColorFactory.convertStringToRGB(colorValue);
if(cursorColor == null) {
cursorColor = ColorFactory.getDefaultFillColor(component).getRGB();
}
}
}

public IDiagramModelComponent getSourceComponent() {
return component;
}

public RGB getCursorColor() {
return cursorColor;
}
}

FormatPainterInfo() {}
public static FormatPainterInfo INSTANCE = new FormatPainterInfo();

private PaintFormat pf;
private IDiagramModelComponent sourceComponent;
private RGB cursorColor;
private Cursor coloredCursor, defaultCursor;
private PropertyChangeSupport listeners = new PropertyChangeSupport(this);

PaintFormat getPaintFormat() {
return pf;
}
private FormatPainterInfo() {}

void updatePaintFormat(IDiagramModelComponent component) {
if(component != null) {
pf = new PaintFormat(component);
}
else {
pf = null;
}
updateColoredCursor();
/**
* Reset source component to null and notify listeners.
*/
public void reset() {
setSourceComponent(null);
fireUpdated();
}

/**
* Reset all copied information
* Set the source component that we will copy formatting from and update the cursor.
*/
public void reset() {
pf = null;
void updateWithSourceComponent(IDiagramModelComponent component) {
setSourceComponent(component);
updateColoredCursor();
fireUpdated();
}

/**
* Get the cursor to use for the tool.
*/
Cursor getCursor() {
return pf == null ? getDefaultCursor() : coloredCursor;
return hasSourceComponent() ? coloredCursor : getDefaultCursor();
}

private Cursor getDefaultCursor() {
Expand All @@ -113,8 +80,56 @@ private Cursor getDefaultCursor() {
return defaultCursor;
}

boolean isFat() {
return pf != null;
/**
* @return true if we have a source component that we will copy the format from.
*/
boolean hasSourceComponent() {
return getSourceComponent() != null;
}

IDiagramModelComponent getSourceComponent() {
return sourceComponent;
}

RGB getCursorColor() {
return cursorColor;
}

/**
* Set the source component from which we will copy the formatting
*/
private void setSourceComponent(IDiagramModelComponent component) {
if(component != null) {
// Make a snapshot copy of the source component so we don't reference the original object.
// Before this change we used to hold a reference to the original object
// but if the model was closed we still held a reference to it and it couldn't be garbage collected
// until the user cleared the FormatPainter which they might forget to do.
// Another side effect of that old way was if the user changed an attribute of the source component
// the FormatPainter would now hold that new value which might not be what the user expects.
sourceComponent = (IDiagramModelComponent)component.getCopy();
}
else {
sourceComponent = null;
}

cursorColor = null;

if(sourceComponent instanceof IDiagramModelConnection dmc) {
// Line color
String colorValue = dmc.getLineColor();
cursorColor = ColorFactory.convertStringToRGB(colorValue);
if(cursorColor == null) {
cursorColor = ColorFactory.getDefaultLineColor(sourceComponent).getRGB();
}
}
else if(sourceComponent instanceof IDiagramModelObject dmo) {
// Fill color
String colorValue = dmo.getFillColor();
cursorColor = ColorFactory.convertStringToRGB(colorValue);
if(cursorColor == null) {
cursorColor = ColorFactory.getDefaultFillColor(sourceComponent).getRGB();
}
}
}

/**
Expand All @@ -127,10 +142,10 @@ private void updateColoredCursor() {

ImageData cursorImageData = IArchiImages.ImageFactory.getImage(IArchiImages.CURSOR_FORMAT_PAINTER).getImageData(ImageFactory.getCursorDeviceZoom());

if(pf.getCursorColor() != null) {
if(getCursorColor() != null) {
PaletteData pData = cursorImageData.palette;
int whitePixel = pData.getPixel(new RGB(255, 255, 255));
int fillColor = pData.getPixel(pf.getCursorColor());
int fillColor = pData.getPixel(getCursorColor());

for(int i = 0; i < cursorImageData.width; i++) {
for(int j = 0; j < cursorImageData.height; j++) {
Expand Down
58 changes: 33 additions & 25 deletions com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterTool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package com.archimatetool.editor.diagram.tools;

import java.io.IOException;
import java.util.Objects;

import org.eclipse.draw2d.geometry.Point;
import org.eclipse.gef.EditPart;
Expand All @@ -26,7 +27,6 @@
import com.archimatetool.editor.diagram.commands.LineWidthCommand;
import com.archimatetool.editor.diagram.commands.TextAlignmentCommand;
import com.archimatetool.editor.diagram.commands.TextPositionCommand;
import com.archimatetool.editor.diagram.tools.FormatPainterInfo.PaintFormat;
import com.archimatetool.editor.model.IArchiveManager;
import com.archimatetool.editor.model.commands.EObjectFeatureCommand;
import com.archimatetool.editor.model.commands.FeatureCommand;
Expand Down Expand Up @@ -73,12 +73,12 @@ protected boolean handleButtonUp(int button) {
if(editpart != null && editpart.getModel() != null) {
Object object = editpart.getModel();
if(isPaintableObject(object)) {
PaintFormat pf = FormatPainterInfo.INSTANCE.getPaintFormat();
if(pf == null) {
FormatPainterInfo.INSTANCE.updatePaintFormat((IDiagramModelComponent)object);
IDiagramModelComponent sourceComponent = FormatPainterInfo.INSTANCE.getSourceComponent();
if(sourceComponent == null) {
FormatPainterInfo.INSTANCE.updateWithSourceComponent((IDiagramModelComponent)object);
}
else if(!isObjectLocked(object)) {
Command cmd = createCommand(pf, (IDiagramModelComponent)object);
Command cmd = createCommand(sourceComponent, (IDiagramModelComponent)object);
if(cmd.canExecute()) {
executeCommand(cmd);
}
Expand All @@ -104,18 +104,18 @@ protected boolean handleDoubleClick(int button) {
FormatPainterInfo.INSTANCE.reset();
return true;
}
else return handleButtonUp(1);
}

return false;
}

protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent targetComponent) {
protected CompoundCommand createCommand(IDiagramModelComponent sourceComponent, IDiagramModelComponent targetComponent) {
CompoundCommand result = new CompoundCommand(Messages.FormatPainterTool_0);

IObjectUIProvider provider = ObjectUIFactory.INSTANCE.getProvider(targetComponent);

// IFontAttribute
if(pf.getSourceComponent() instanceof IFontAttribute source && targetComponent instanceof IFontAttribute target) {
if(sourceComponent instanceof IFontAttribute source && targetComponent instanceof IFontAttribute target) {
Command cmd = new FontStyleCommand(target, source.getFont());
if(cmd.canExecute()) {
result.add(cmd);
Expand All @@ -128,7 +128,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t
}

// ILineObject
if(pf.getSourceComponent() instanceof ILineObject source && targetComponent instanceof ILineObject target && provider != null) {
if(sourceComponent instanceof ILineObject source && targetComponent instanceof ILineObject target && provider != null) {
// Line color
if(provider.shouldExposeFeature(IArchimatePackage.Literals.LINE_OBJECT__LINE_COLOR.getName())) {
Command cmd = new LineColorCommand(target, source.getLineColor());
Expand All @@ -147,48 +147,57 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t
}

// IBorderObject
if(pf.getSourceComponent() instanceof IBorderObject source && targetComponent instanceof IBorderObject target) {
if(sourceComponent instanceof IBorderObject source && targetComponent instanceof IBorderObject target) {
Command cmd = new BorderColorCommand(target, source.getBorderColor());
if(cmd.canExecute()) {
result.add(cmd);
}
}

// IBorderType
if(pf.getSourceComponent() instanceof IBorderType source && targetComponent instanceof IBorderType target && source.eClass() == target.eClass()) {
if(sourceComponent instanceof IBorderType source && targetComponent instanceof IBorderType target && source.eClass() == target.eClass()) {
Command cmd = new EObjectFeatureCommand("", target, IArchimatePackage.Literals.BORDER_TYPE__BORDER_TYPE, source.getBorderType()); //$NON-NLS-1$
if(cmd.canExecute()) {
result.add(cmd);
}
}

// ITextPosition
if(pf.getSourceComponent() instanceof ITextPosition source && targetComponent instanceof ITextPosition target) {
if(sourceComponent instanceof ITextPosition source && targetComponent instanceof ITextPosition target) {
Command cmd = new TextPositionCommand(target, source.getTextPosition());
if(cmd.canExecute()) {
result.add(cmd);
}
}

// ITextAlignment
if(pf.getSourceComponent() instanceof ITextAlignment source && targetComponent instanceof ITextAlignment target) {
if(sourceComponent instanceof ITextAlignment source && targetComponent instanceof ITextAlignment target) {
Command cmd = new TextAlignmentCommand(target, source.getTextAlignment());
if(cmd.canExecute()) {
result.add(cmd);
}
}

// IDiagramModelObject
if(pf.getSourceComponent() instanceof IDiagramModelObject source && targetComponent instanceof IDiagramModelObject target) {
// Source fill colour is null which is "default"
String fillColorString = source.getFillColor();
if(fillColorString == null) {
fillColorString = ColorFactory.convertColorToString(ColorFactory.getDefaultFillColor(source));
if(sourceComponent instanceof IDiagramModelObject source && targetComponent instanceof IDiagramModelObject target) {
Command cmd;

String sourcefillColor = source.getFillColor();
if(sourcefillColor == null) { // Source fill colour is null which is "default"
sourcefillColor = ColorFactory.convertColorToString(ColorFactory.getDefaultFillColor(source));
}

Command cmd = new FillColorCommand(target, fillColorString);
if(cmd.canExecute()) {
result.add(cmd);
String targetfillColor = target.getFillColor();
if(targetfillColor == null) { // target fill colour is null which is "default"
targetfillColor = ColorFactory.convertColorToString(ColorFactory.getDefaultFillColor(target));
}

// Compare actual fill colours rather than null fill colors
if(!Objects.equals(sourcefillColor, targetfillColor)) {
cmd = new FillColorCommand(target, sourcefillColor);
if(cmd.canExecute()) {
result.add(cmd);
}
}

// Alpha fill opacity
Expand Down Expand Up @@ -236,7 +245,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t
}

// Archimate objects
if(pf.getSourceComponent() instanceof IDiagramModelArchimateObject source && targetComponent instanceof IDiagramModelArchimateObject target) {
if(sourceComponent instanceof IDiagramModelArchimateObject source && targetComponent instanceof IDiagramModelArchimateObject target) {
// Image Source
Command cmd = new FeatureCommand("", target, IDiagramModelArchimateObject.FEATURE_IMAGE_SOURCE, //$NON-NLS-1$
source.getImageSource(), IDiagramModelArchimateObject.FEATURE_IMAGE_SOURCE_DEFAULT);
Expand All @@ -246,7 +255,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t
}

// IDiagramModelConnection
if(pf.getSourceComponent() instanceof IDiagramModelConnection source && targetComponent instanceof IDiagramModelConnection target) {
if(sourceComponent instanceof IDiagramModelConnection source && targetComponent instanceof IDiagramModelConnection target) {
// Connection text position
Command cmd = new ConnectionTextPositionCommand(target, source.getTextPosition());
if(cmd.canExecute()) {
Expand All @@ -263,7 +272,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t
}

// IIconic
if(pf.getSourceComponent() instanceof IIconic source && targetComponent instanceof IIconic target) {
if(sourceComponent instanceof IIconic source && targetComponent instanceof IIconic target) {
// If we have an image path and the source and target models are different, copy the image bytes
String imagePath = source.getImagePath();
if(imagePath != null && source.getArchimateModel() != target.getArchimateModel()) {
Expand Down Expand Up @@ -317,5 +326,4 @@ protected boolean isPaintableObject(Object object) {
protected String getCommandName() {
return "FormatPaint"; //$NON-NLS-1$
}

}
Loading

0 comments on commit 18ea669

Please sign in to comment.