diff options
author | Cedric Notot | 2013-04-08 08:08:25 +0000 |
---|---|---|
committer | Mikaƫl Barbero | 2013-05-28 09:59:10 +0000 |
commit | fe19d16b41a16195d8c14e8e97a4b6381677aed9 (patch) | |
tree | ade4083fe35eaa4b6e338572726d2b94ed87efc0 | |
parent | b679347d9d0ae08bf989961d96d00d656220c832 (diff) | |
download | org.eclipse.emf.compare-fe19d16b41a16195d8c14e8e97a4b6381677aed9.tar.gz org.eclipse.emf.compare-fe19d16b41a16195d8c14e8e97a4b6381677aed9.tar.xz org.eclipse.emf.compare-fe19d16b41a16195d8c14e8e97a4b6381677aed9.zip |
Fix bug in displaying diagrams in multi-diagram case and other fixes.
As now, decorators are computed again at each difference selection.
Besides, the diagram edit parts are computed again at each change of
diagram (all these changes are useful for bug on multi-diagram).
Synchronized lists (and synchronized areas) are not needed.
Fix of a regression on the diagram match accessor.
Change-Id: I2b9fff5c8d823f0dbff392ad035ed12724710ca5
4 files changed, 137 insertions, 115 deletions
diff --git a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramDiffAccessorImpl.java b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramDiffAccessorImpl.java index b050dad48..0894eb533 100644 --- a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramDiffAccessorImpl.java +++ b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramDiffAccessorImpl.java @@ -52,6 +52,11 @@ public class DiagramDiffAccessorImpl extends DiagramMatchAccessorImpl implements return fDiff;
}
+ /**
+ * It returns the view of the diagram difference.
+ *
+ * @return The view of the diagram difference.
+ */
private EObject getEObject() {
if (fDiff instanceof DiagramDiff) {
return ((DiagramDiff)fDiff).getView();
diff --git a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramMatchAccessorImpl.java b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramMatchAccessorImpl.java index 096d45f69..bcb71e0c7 100644 --- a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramMatchAccessorImpl.java +++ b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/accessor/DiagramMatchAccessorImpl.java @@ -10,6 +10,8 @@ *******************************************************************************/
package org.eclipse.emf.compare.diagram.ide.ui.internal.accessor;
+import com.google.common.collect.Lists;
+
import java.util.List;
import org.eclipse.emf.compare.Comparison;
@@ -132,12 +134,15 @@ public class DiagramMatchAccessorImpl implements IDiagramNodeAccessor, ITypedEle if (obj != null) {
diagram = getDiagram(obj);
} else {
- obj = getEObject(side.opposite());
- if (obj != null) {
- diagram = getDiagram(obj);
- if (diagram != null) {
- Match diagramMatch = fComparison.getMatch(diagram);
- diagram = (Diagram)getEObject(diagramMatch, side);
+ for (MergeViewerSide oppositeSide : getOtherSides(side)) {
+ obj = getEObject(oppositeSide);
+ if (obj != null) {
+ diagram = getDiagram(obj);
+ if (diagram != null) {
+ Match diagramMatch = fComparison.getMatch(diagram);
+ diagram = (Diagram)getEObject(diagramMatch, side);
+ break;
+ }
}
}
}
@@ -223,4 +228,30 @@ public class DiagramMatchAccessorImpl implements IDiagramNodeAccessor, ITypedEle public List<Diff> getAllDiffs() {
return fComparison.getDifferences();
}
+
+ /**
+ * Get the opposite sides of the given one.
+ *
+ * @param side
+ * The given side.
+ * @return the opposite ones.
+ */
+ private List<MergeViewerSide> getOtherSides(MergeViewerSide side) {
+ List<MergeViewerSide> ret = null;
+ switch (side) {
+ case ANCESTOR:
+ ret = Lists.newArrayList(MergeViewerSide.LEFT, MergeViewerSide.RIGHT);
+ break;
+ case LEFT:
+ ret = Lists.newArrayList(MergeViewerSide.RIGHT, MergeViewerSide.ANCESTOR);
+ break;
+ case RIGHT:
+ ret = Lists.newArrayList(MergeViewerSide.LEFT, MergeViewerSide.ANCESTOR);
+ break;
+ default:
+ ret = Lists.newArrayList();
+ }
+ return ret;
+ }
+
}
diff --git a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramContentMergeViewer.java b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramContentMergeViewer.java index ac1324eb8..94b9bd05c 100644 --- a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramContentMergeViewer.java +++ b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramContentMergeViewer.java @@ -19,9 +19,11 @@ import static org.eclipse.emf.compare.utils.EMFComparePredicates.valueIs; import com.google.common.base.Predicate; import com.google.common.collect.Collections2; +import com.google.common.collect.HashMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; import java.util.ArrayList; import java.util.Collection; @@ -77,7 +79,6 @@ import org.eclipse.gmf.runtime.notation.Diagram; import org.eclipse.gmf.runtime.notation.Edge; import org.eclipse.gmf.runtime.notation.NotationPackage; import org.eclipse.gmf.runtime.notation.View; -import org.eclipse.jface.viewers.SelectionChangedEvent; import org.eclipse.swt.SWT; import org.eclipse.swt.graphics.GC; import org.eclipse.swt.widgets.Composite; @@ -127,7 +128,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { void removeDecorators(Diff difference); /** - * It removes all the displayed decorators. + * It removes all the displayed decorators from cache. */ void removeAll(); } @@ -340,7 +341,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { * The difference. */ public void hideDecorators(Diff difference) { - List<? extends AbstractDecorator> oldDecorators = getDecorators(difference); + Collection<? extends AbstractDecorator> oldDecorators = getDecorators(difference); if (oldDecorators != null && !oldDecorators.isEmpty() && getComparison() != null) { handleDecorators(oldDecorators, false, true); } @@ -354,7 +355,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { */ public void revealDecorators(Diff difference) { - List<? super AbstractDecorator> decorators = (List<? super AbstractDecorator>)getDecorators(difference); + Collection<? super AbstractDecorator> decorators = (Collection<? super AbstractDecorator>)getDecorators(difference); // Create decorators only if they do not already exist and if the selected difference is a good // candidate for that. @@ -388,7 +389,8 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { // The selected difference is a good candidate and decorators exist for it if (decorators != null && !decorators.isEmpty()) { - revealDecorators((List<? extends AbstractDecorator>)decorators); + revealDecorators((Collection<? extends AbstractDecorator>)decorators); + // removeAll(); } } @@ -412,7 +414,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { * @param decorators * The main decorators. */ - protected void revealDecorators(List<? extends AbstractDecorator> decorators) { + protected void revealDecorators(Collection<? extends AbstractDecorator> decorators) { handleDecorators(decorators, true, true); } @@ -443,7 +445,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { * It indicates if the given decorators to handle are considered as the main ones (the ones * directly linked to the selected difference). */ - protected void handleDecorators(List<? extends AbstractDecorator> decorators, boolean isAdd, + protected void handleDecorators(Collection<? extends AbstractDecorator> decorators, boolean isAdd, boolean areMain) { for (AbstractDecorator decorator : decorators) { handleDecorator(decorator, isAdd, areMain); @@ -687,7 +689,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { * The difference. * @return The list of main decorators. */ - protected abstract List<? extends AbstractDecorator> getDecorators(Diff difference); + protected abstract Collection<? extends AbstractDecorator> getDecorators(Diff difference); } @@ -795,8 +797,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { } /** Registry of created phantoms, indexed by difference. */ - private final Map<Diff, Phantom> fPhantomRegistry = Collections - .synchronizedMap(new HashMap<Diff, Phantom>()); + private final Map<Diff, Phantom> fPhantomRegistry = new HashMap<Diff, Phantom>(); /** Predicate witch checks that the given difference is an ADD or DELETE of a graphical object. */ private Predicate<Diff> isAddOrDelete = and(instanceOf(DiagramDiff.class), or( @@ -1393,21 +1394,19 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { * @see org.eclipse.emf.compare.diagram.ide.ui.internal.contentmergeviewer.diagram.DiagramContentMergeViewer.IDecoratorManager#hideAll() */ public void hideAll() { - synchronized(fPhantomRegistry) { - Collection<Phantom> phantoms = fPhantomRegistry.values(); - synchronized(phantoms) { - Iterator<Phantom> visiblePhantoms = Iterators.filter(phantoms.iterator(), - new Predicate<Phantom>() { - public boolean apply(Phantom phantom) { - return phantom.getFigure().getParent() != null; - } - }); - while (visiblePhantoms.hasNext()) { - Phantom phantom = (Phantom)visiblePhantoms.next(); - handleDecorator(phantom, false, true); - } - } + Collection<Phantom> phantoms = fPhantomRegistry.values(); + + Iterator<Phantom> visiblePhantoms = Iterators.filter(phantoms.iterator(), + new Predicate<Phantom>() { + public boolean apply(Phantom phantom) { + return phantom.getFigure().getParent() != null; + } + }); + while (visiblePhantoms.hasNext()) { + Phantom phantom = (Phantom)visiblePhantoms.next(); + handleDecorator(phantom, false, true); } + } } @@ -1454,8 +1453,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { } /** Registry of created markers, indexed by difference. */ - private Map<Diff, List<Marker>> fMarkerRegistry = Collections - .synchronizedMap(new HashMap<Diff, List<Marker>>()); + private Multimap<Diff, Marker> fMarkerRegistry = HashMultimap.create(); /** * {@inheritDoc} @@ -1511,12 +1509,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { MergeViewerSide targetSide) { Marker marker = createMarker(diff, referenceView, referenceFigure, targetSide); if (marker != null) { - List<Marker> markers = fMarkerRegistry.get(diff); - if (markers == null) { - markers = Collections.synchronizedList(new ArrayList<Marker>()); - fMarkerRegistry.put(diff, markers); - } - markers.add(marker); + fMarkerRegistry.put(diff, marker); } return marker; } @@ -1528,7 +1521,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { */ @Override public void removeDecorators(Diff difference) { - fMarkerRegistry.remove(difference); + fMarkerRegistry.removeAll(difference); } /** @@ -1547,7 +1540,7 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { * @see org.eclipse.emf.compare.diagram.ide.ui.internal.contentmergeviewer.diagram.DiagramContentMergeViewer.AbstractDecoratorManager#getDecorators(org.eclipse.emf.compare.Diff) */ @Override - protected List<Marker> getDecorators(Diff difference) { + protected Collection<Marker> getDecorators(Diff difference) { return fMarkerRegistry.get(difference); } @@ -1637,22 +1630,14 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { * @see org.eclipse.emf.compare.diagram.ide.ui.internal.contentmergeviewer.diagram.DiagramContentMergeViewer.IDecoratorManager#hideAll() */ public void hideAll() { - synchronized(fMarkerRegistry) { - Collection<List<Marker>> listMarkers = fMarkerRegistry.values(); - List<Marker> markers = Lists.newArrayList(Iterables.concat(listMarkers)); - List<Marker> sMarkers = Collections.synchronizedList(markers); - synchronized(sMarkers) { - Iterator<Marker> visibleMarkers = Iterators.filter(sMarkers.iterator(), - new Predicate<Marker>() { - public boolean apply(Marker marker) { - return marker.getFigure().getParent() != null; - } - }); - while (visibleMarkers.hasNext()) { - Marker marker = (Marker)visibleMarkers.next(); - handleDecorator(marker, false, true); - } + Collection<Marker> listMarkers = fMarkerRegistry.values(); + Iterable<Marker> visibleMarkers = Iterables.filter(listMarkers, new Predicate<Marker>() { + public boolean apply(Marker marker) { + return marker.getFigure().getParent() != null; } + }); + for (Marker marker : visibleMarkers) { + handleDecorator(marker, false, true); } } @@ -1857,31 +1842,40 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { */ @Override protected void updateContent(Object ancestor, Object left, Object right) { + + // Delete decorators at each selection of a difference (force the computation) + fDecoratorsManager.hideAll(); + fDecoratorsManager.removeAll(); + super.updateContent(ancestor, left, right); getLeftMergeViewer().getGraphicalViewer().flush(); getRightMergeViewer().getGraphicalViewer().flush(); getAncestorMergeViewer().getGraphicalViewer().flush(); - if (left instanceof IDiagramDiffAccessor) { - IDiagramDiffAccessor input = (IDiagramDiffAccessor)left; + if (left instanceof IDiagramNodeAccessor) { - // initialization: reset the current difference selection hiding potential visible phantoms - if (fCurrentSelectedDiff != null && fCurrentSelectedDiff.getState() != DifferenceState.MERGED) { - fDecoratorsManager.hideDecorators(fCurrentSelectedDiff); - } + // if (fCurrentSelectedDiff != null && fCurrentSelectedDiff.getState() != DifferenceState.MERGED) + // { + // fDecoratorsManager.hideDecorators(fCurrentSelectedDiff); + // } + // fCurrentSelectedDiff = null; - Diff diff = input.getDiff(); // equivalent to getInput().getTarget() - fCurrentSelectedDiff = diff; + // Compute and display the decorators related to the selected difference (if not merged and + // different from the current one) + if (left instanceof IDiagramDiffAccessor) { + IDiagramDiffAccessor input = (IDiagramDiffAccessor)left; - if (diff.getState() != DifferenceState.MERGED) { - fDecoratorsManager.revealDecorators(diff); - } - } else if (left instanceof IDiagramNodeAccessor) { - if (fCurrentSelectedDiff != null && fCurrentSelectedDiff.getState() != DifferenceState.MERGED) { - fDecoratorsManager.hideDecorators(fCurrentSelectedDiff); + Diff diff = input.getDiff(); // equivalent to getInput().getTarget() + + if (diff.getState() != DifferenceState.MERGED && diff != fCurrentSelectedDiff) { + fDecoratorsManager.revealDecorators(diff); + } + + fCurrentSelectedDiff = diff; + } else { + fCurrentSelectedDiff = null; } - fCurrentSelectedDiff = null; } updateToolItems(); @@ -1896,6 +1890,8 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { @Override public void commandStackChanged(EventObject event) { super.commandStackChanged(event); + + // Delete decorators at each change of the input models (after merging or CTRL-Z, CTRL-Y) Object source = event.getSource(); if (source instanceof CommandStack) { Command command = ((CommandStack)source).getMostRecentCommand(); @@ -1915,16 +1911,6 @@ public class DiagramContentMergeViewer extends EMFCompareContentMergeViewer { /** * {@inheritDoc} * - * @see org.eclipse.emf.compare.ide.ui.internal.contentmergeviewer.EMFCompareContentMergeViewer#selectionChanged(org.eclipse.jface.viewers.SelectionChangedEvent) - */ - @Override - public void selectionChanged(SelectionChangedEvent event) { - // No selection synchronization (content to structure merge viewer). - } - - /** - * {@inheritDoc} - * * @see org.eclipse.emf.compare.ide.ui.internal.contentmergeviewer.EMFCompareContentMergeViewer#getDiffFrom(org.eclipse.emf.compare.rcp.ui.internal.mergeviewer.IMergeViewer) */ @Override diff --git a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramMergeViewer.java b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramMergeViewer.java index f204398b7..6c17500e2 100644 --- a/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramMergeViewer.java +++ b/plugins/org.eclipse.emf.compare.diagram.ide.ui/src/org/eclipse/emf/compare/diagram/ide/ui/internal/contentmergeviewer/diagram/DiagramMergeViewer.java @@ -29,7 +29,6 @@ import org.eclipse.gef.EditPartViewer; import org.eclipse.gef.MouseWheelHandler;
import org.eclipse.gef.MouseWheelZoomHandler;
import org.eclipse.gef.SelectionManager;
-import org.eclipse.gef.editparts.ZoomManager;
import org.eclipse.gef.ui.parts.DomainEventDispatcher;
import org.eclipse.gmf.runtime.diagram.ui.editparts.DiagramRootEditPart;
import org.eclipse.gmf.runtime.diagram.ui.parts.DiagramCommandStack;
@@ -186,25 +185,44 @@ class DiagramMergeViewer extends AbstractGraphicalMergeViewer { public void setInput(final Object input) {
if (input instanceof IDiagramNodeAccessor) {
Diagram diagram = ((IDiagramNodeAccessor)input).getOwnedDiagram();
- initEditingDomain(diagram);
- EditPart editPart = null;
- View view = ((IDiagramNodeAccessor)input).getOwnedView();
- if (view != null) {
- editPart = getEditPart(view);
- }
- fGraphicalViewer.deselectAll();
- // Selection only on matches.
- if (!(input instanceof IDiagramDiffAccessor) && editPart != null) {
- while (editPart != null && !editPart.isSelectable()) {
- editPart = editPart.getParent();
+ if (diagram != null) {
+ initEditingDomain(diagram);
+ if (currentDiag != diagram) {
+ DiagramRootEditPart rootEditPart = new DiagramRootEditPart(diagram.getMeasurementUnit());
+ // rootEditPart.getZoomManager().setZoomAnimationStyle(ZoomManager.ANIMATE_NEVER);
+ // rootEditPart.getZoomManager().setZoom(ZOOM_FACTOR);
+ fGraphicalViewer.setRootEditPart(rootEditPart);
+ fGraphicalViewer.setContents(diagram);
}
-
- if (editPart != null) {
- setSelection(new StructuredSelection(editPart));
- fGraphicalViewer.reveal(editPart);
+ View view = ((IDiagramNodeAccessor)input).getOwnedView();
+ EditPart editPart = getEditPart(view);
+ fGraphicalViewer.deselectAll();
+ // Selection only on matches.
+ if (!(input instanceof IDiagramDiffAccessor) && editPart != null) {
+ setSelection(editPart);
}
+ } else {
+ fGraphicalViewer.setContents((EditPart)null);
}
+ currentDiag = diagram;
+ }
+ }
+ /**
+ * Set the selection of the first selectable object from the given edit part.
+ *
+ * @param editPart
+ * The edit part.
+ */
+ private void setSelection(EditPart editPart) {
+ EditPart ep = editPart;
+ while (ep != null && !ep.isSelectable()) {
+ ep = ep.getParent();
+ }
+
+ if (ep != null) {
+ setSelection(new StructuredSelection(ep));
+ fGraphicalViewer.reveal(ep);
}
}
@@ -226,31 +244,13 @@ class DiagramMergeViewer extends AbstractGraphicalMergeViewer { }
/**
- * Get the editpart from the given view.
+ * Return the editpart from the given view.
*
* @param view
* The view.
* @return The editpart.
*/
public EditPart getEditPart(final EObject view) {
- final EditPart editPart = (EditPart)fGraphicalViewer.getEditPartRegistry().get(view);
- if (editPart == null) {
- Diagram diagram = null;
- if (view instanceof Diagram) {
- diagram = (Diagram)view;
- } else if (view instanceof View) {
- diagram = ((View)view).getDiagram();
- }
- if (diagram != null && !diagram.equals(currentDiag)) {
- currentDiag = diagram;
- fGraphicalViewer.getEditPartRegistry().clear();
- final DiagramRootEditPart rootEditPart = new DiagramRootEditPart(diagram.getMeasurementUnit());
- fGraphicalViewer.setRootEditPart(rootEditPart);
- fGraphicalViewer.setContents(diagram);
- rootEditPart.getZoomManager().setZoomAnimationStyle(ZoomManager.ANIMATE_NEVER);
- rootEditPart.getZoomManager().setZoom(ZOOM_FACTOR);
- }
- }
return (EditPart)fGraphicalViewer.getEditPartRegistry().get(view);
}
|