Skip to content

Commit

Permalink
Fix FormatPainter not copying image
Browse files Browse the repository at this point in the history
- Copy the image bytes into FormatPainterInfo
- See #1075
  • Loading branch information
Phillipus committed Sep 11, 2024
1 parent 99cbef0 commit d1217d4
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@
import org.eclipse.swt.graphics.PaletteData;
import org.eclipse.swt.graphics.RGB;

import com.archimatetool.editor.model.IArchiveManager;
import com.archimatetool.editor.ui.ColorFactory;
import com.archimatetool.editor.ui.IArchiImages;
import com.archimatetool.editor.ui.ImageFactory;
import com.archimatetool.model.IDiagramModelComponent;
import com.archimatetool.model.IDiagramModelConnection;
import com.archimatetool.model.IDiagramModelObject;
import com.archimatetool.model.IIconic;



Expand All @@ -41,6 +43,8 @@ public class FormatPainterInfo {
public static FormatPainterInfo INSTANCE = new FormatPainterInfo();

private IDiagramModelComponent sourceComponent;
private byte[] sourceImageBytes;

private RGB cursorColor;
private Cursor coloredCursor, defaultCursor;
private PropertyChangeSupport listeners = new PropertyChangeSupport(this);
Expand Down Expand Up @@ -91,6 +95,10 @@ IDiagramModelComponent getSourceComponent() {
return sourceComponent;
}

byte[] getSourceImageBytes() {
return sourceImageBytes;
}

RGB getCursorColor() {
return cursorColor;
}
Expand All @@ -99,6 +107,10 @@ RGB getCursorColor() {
* Set the source component from which we will copy the formatting
*/
private void setSourceComponent(IDiagramModelComponent component) {
sourceComponent = null;
sourceImageBytes = null;
cursorColor = null;

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
Expand All @@ -108,12 +120,15 @@ private void setSourceComponent(IDiagramModelComponent component) {
// the FormatPainter would now hold that new value which might not be what the user expects.
sourceComponent = (IDiagramModelComponent)component.getCopy();
}
else {
sourceComponent = null;

// Copy image bytes, if any
if(component instanceof IIconic iconic && iconic.getImagePath() != null) {
IArchiveManager sourceArchiveManager = (IArchiveManager)component.getAdapter(IArchiveManager.class);
if(sourceArchiveManager != null) {
sourceImageBytes = sourceArchiveManager.getBytesFromEntry(iconic.getImagePath());
}
}

cursorColor = null;

if(sourceComponent instanceof IDiagramModelConnection dmc) {
// Line color
String colorValue = dmc.getLineColor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,11 @@ protected boolean handleButtonUp(int button) {
if(editpart != null && editpart.getModel() != null) {
Object object = editpart.getModel();
if(isPaintableObject(object)) {
IDiagramModelComponent sourceComponent = FormatPainterInfo.INSTANCE.getSourceComponent();
if(sourceComponent == null) {
if(FormatPainterInfo.INSTANCE.getSourceComponent() == null) {
FormatPainterInfo.INSTANCE.updateWithSourceComponent((IDiagramModelComponent)object);
}
else if(!isObjectLocked(object)) {
Command cmd = createCommand(sourceComponent, (IDiagramModelComponent)object);
Command cmd = createCommand((IDiagramModelComponent)object);
if(cmd.canExecute()) {
executeCommand(cmd);
}
Expand Down Expand Up @@ -109,10 +108,11 @@ protected boolean handleDoubleClick(int button) {
return false;
}

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

IObjectUIProvider provider = ObjectUIFactory.INSTANCE.getProvider(targetComponent);
IDiagramModelComponent sourceComponent = FormatPainterInfo.INSTANCE.getSourceComponent();
IObjectUIProvider targetUIProvider = ObjectUIFactory.INSTANCE.getProvider(targetComponent);

// IFontAttribute
if(sourceComponent instanceof IFontAttribute source && targetComponent instanceof IFontAttribute target) {
Expand All @@ -128,17 +128,17 @@ protected CompoundCommand createCommand(IDiagramModelComponent sourceComponent,
}

// ILineObject
if(sourceComponent instanceof ILineObject source && targetComponent instanceof ILineObject target && provider != null) {
if(sourceComponent instanceof ILineObject source && targetComponent instanceof ILineObject target && targetUIProvider != null) {
// Line color
if(provider.shouldExposeFeature(IArchimatePackage.Literals.LINE_OBJECT__LINE_COLOR.getName())) {
if(targetUIProvider.shouldExposeFeature(IArchimatePackage.Literals.LINE_OBJECT__LINE_COLOR.getName())) {
Command cmd = new LineColorCommand(target, source.getLineColor());
if(cmd.canExecute()) {
result.add(cmd);
}
}

// Line width
if(provider.shouldExposeFeature(IArchimatePackage.Literals.LINE_OBJECT__LINE_WIDTH.getName())) {
if(targetUIProvider.shouldExposeFeature(IArchimatePackage.Literals.LINE_OBJECT__LINE_WIDTH.getName())) {
Command cmd = new LineWidthCommand(target, source.getLineWidth());
if(cmd.canExecute()) {
result.add(cmd);
Expand Down Expand Up @@ -213,23 +213,23 @@ protected CompoundCommand createCommand(IDiagramModelComponent sourceComponent,
}

// Gradient
if(provider != null && provider.shouldExposeFeature(IDiagramModelObject.FEATURE_GRADIENT)) {
if(targetUIProvider != null && targetUIProvider.shouldExposeFeature(IDiagramModelObject.FEATURE_GRADIENT)) {
cmd = new FeatureCommand("", target, IDiagramModelObject.FEATURE_GRADIENT, source.getGradient(), IDiagramModelObject.FEATURE_GRADIENT_DEFAULT); //$NON-NLS-1$
if(cmd.canExecute()) {
result.add(cmd);
}
}

// Derive line color
if(provider != null && provider.shouldExposeFeature(IDiagramModelObject.FEATURE_DERIVE_ELEMENT_LINE_COLOR)) {
if(targetUIProvider != null && targetUIProvider.shouldExposeFeature(IDiagramModelObject.FEATURE_DERIVE_ELEMENT_LINE_COLOR)) {
cmd = new FeatureCommand("", target, IDiagramModelObject.FEATURE_DERIVE_ELEMENT_LINE_COLOR, source.getDeriveElementLineColor(), IDiagramModelObject.FEATURE_DERIVE_ELEMENT_LINE_COLOR_DEFAULT); //$NON-NLS-1$
if(cmd.canExecute()) {
result.add(cmd);
}
}

// Icon Visibility and Color, but paste only if the target object has an icon
if(provider instanceof IGraphicalObjectUIProvider && ((IGraphicalObjectUIProvider)provider).hasIcon()) {
if(targetUIProvider instanceof IGraphicalObjectUIProvider && ((IGraphicalObjectUIProvider)targetUIProvider).hasIcon()) {
// Icon visible
cmd = new FeatureCommand("", target, IDiagramModelObject.FEATURE_ICON_VISIBLE, source.getIconVisibleState(), IDiagramModelObject.FEATURE_ICON_VISIBLE_DEFAULT); //$NON-NLS-1$
if(cmd.canExecute()) {
Expand Down Expand Up @@ -273,14 +273,13 @@ protected CompoundCommand createCommand(IDiagramModelComponent sourceComponent,

// IIconic
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
// If we have an image path and the source has image bytes
String imagePath = source.getImagePath();
if(imagePath != null && source.getArchimateModel() != target.getArchimateModel()) {
IArchiveManager sourceArchiveManager = (IArchiveManager)source.getAdapter(IArchiveManager.class);
if(imagePath != null && FormatPainterInfo.INSTANCE.getSourceImageBytes() != null) {
IArchiveManager targetArchiveManager = (IArchiveManager)target.getAdapter(IArchiveManager.class);

try {
imagePath = targetArchiveManager.copyImageBytes(sourceArchiveManager, imagePath);
imagePath = targetArchiveManager.addByteContentEntry(imagePath, FormatPainterInfo.INSTANCE.getSourceImageBytes());
}
catch(IOException ex) {
ex.printStackTrace();
Expand All @@ -303,11 +302,11 @@ protected CompoundCommand createCommand(IDiagramModelComponent sourceComponent,
return result;
}

protected boolean isObjectLocked(Object object) {
private boolean isObjectLocked(Object object) {
return object instanceof ILockable lockable && lockable.isLocked();
}

protected boolean isPaintableObject(Object object) {
boolean isPaintableObject(Object object) {
// Junctions are a no-no
if(object instanceof IDiagramModelArchimateObject dmo) {
IArchimateElement element = dmo.getArchimateElement();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,13 @@ public void testUpdateWithSourceComponent() throws Exception {
info.updateWithSourceComponent(sourceComponent);

assertNotNull(info.getSourceComponent());

// FormatPaintInfo will make a copy of the source component
assertNotEquals(sourceComponent, info.getSourceComponent());

// Image bytes are null
assertNull(info.getSourceImageBytes());

// Colored cursor
assertSame(info.getCursor(), TestUtils.getPrivateField(info, "coloredCursor"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ public void testCreateCommandForDiagramModelArchimateObject() throws Exception {

// Execute command
FormatPainterTool tool = new FormatPainterTool();
CompoundCommand compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent);
CompoundCommand compoundCmd = tool.createCommand(targetComponent);

// Should be no commands
assertEquals(0, compoundCmd.getCommands().size());

// Set the source fill color to its default actual value.
// The fill color command will not be added because the target's fill color (null) is effectively the same.
sourceComponent.setFillColor("#ffffb5");
compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent);
compoundCmd = tool.createCommand(targetComponent);
assertEquals(0, compoundCmd.getCommands().size());

// Now change some attributes on the source component
Expand All @@ -69,7 +69,7 @@ public void testCreateCommandForDiagramModelArchimateObject() throws Exception {
// But we have to reset the FormatPainterInfo with the source component because it makes a copy of it
FormatPainterInfo.INSTANCE.updateWithSourceComponent(sourceComponent);

compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent);
compoundCmd = tool.createCommand(targetComponent);
assertEquals(12, compoundCmd.getCommands().size());
}

Expand All @@ -88,7 +88,7 @@ public void testCreateCommandForDiagramModelArchimateConnection() throws Excepti

// Execute command
FormatPainterTool tool = new FormatPainterTool();
CompoundCommand compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent);
CompoundCommand compoundCmd = tool.createCommand(targetComponent);

// Should be no commands
assertEquals(0, compoundCmd.getCommands().size());
Expand All @@ -103,7 +103,7 @@ public void testCreateCommandForDiagramModelArchimateConnection() throws Excepti
// But we have to reset the FormatPainterInfo with the source component because it makes a copy of it
FormatPainterInfo.INSTANCE.updateWithSourceComponent(sourceComponent);

compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent);
compoundCmd = tool.createCommand(targetComponent);
assertEquals(5, compoundCmd.getCommands().size());
}

Expand Down

0 comments on commit d1217d4

Please sign in to comment.