Skip to content

Commit

Permalink
Fix adding a group annotation automatically adds all its children. Refs
Browse files Browse the repository at this point in the history 
#4314
  • Loading branch information
@mikekucera
mikekucera committed Sep 17, 2018
1 parent fcabf18 commit 9675b5c
Show file tree
Hide file tree
Showing 10 changed files with 296 additions and 86 deletions.
3 changes: 3 additions & 0 deletions ...ding-presentation-impl/src/main/java/org/cytoscape/ding/impl/ArbitraryGraphicsCanvas.java
View file
Expand Up @@ -44,6 +44,7 @@

import javax.swing.JComponent;

import org.cytoscape.ding.impl.DGraphView.Canvas;
import org.cytoscape.ding.impl.cyannotator.annotations.DingAnnotation;
import org.cytoscape.ding.impl.events.ViewportChangeListener;
import org.cytoscape.model.CyNode;
Expand Down Expand Up @@ -100,9 +101,11 @@ public class ArbitraryGraphicsCanvas extends DingCanvas implements ViewportChang
* @param isOpaque boolean
*/
public ArbitraryGraphicsCanvas(DGraphView dGraphView,
Canvas canvasId,
InnerCanvas innerCanvas,
Color backgroundColor,
boolean isOpaque) {
super(canvasId);
m_dGraphView = dGraphView;
m_innerCanvas = innerCanvas;
m_backgroundColor = backgroundColor;
Expand Down
4 changes: 2 additions & 2 deletions ding-impl/ding-presentation-impl/src/main/java/org/cytoscape/ding/impl/DGraphView.java
View file
Expand Up @@ -429,9 +429,9 @@ public DGraphView(
m_defaultNodeXMax = m_defaultNodeXMin + DNodeView.DEFAULT_WIDTH;
m_defaultNodeYMax = m_defaultNodeYMin + DNodeView.DEFAULT_HEIGHT;
m_networkCanvas = new InnerCanvas(m_lock, this, registrar);
m_backgroundCanvas = new ArbitraryGraphicsCanvas(this, m_networkCanvas, Color.white, true);
m_backgroundCanvas = new ArbitraryGraphicsCanvas(this, Canvas.BACKGROUND_CANVAS, m_networkCanvas, Color.white, true);
addViewportChangeListener(m_backgroundCanvas);
m_foregroundCanvas = new ArbitraryGraphicsCanvas(this, m_networkCanvas, Color.white, false);
m_foregroundCanvas = new ArbitraryGraphicsCanvas(this, Canvas.FOREGROUND_CANVAS, m_networkCanvas, Color.white, false);
addViewportChangeListener(m_foregroundCanvas);
m_selectedNodes = new LongBTree();
m_selectedEdges = new LongBTree();
Expand Down
13 changes: 13 additions & 0 deletions ding-impl/ding-presentation-impl/src/main/java/org/cytoscape/ding/impl/DingCanvas.java
View file
Expand Up @@ -30,6 +30,8 @@

import javax.swing.JComponent;

import org.cytoscape.ding.impl.DGraphView.Canvas;


/**
* This class is meant to be extended by a class which
Expand Down Expand Up @@ -57,7 +59,18 @@ public abstract class DingCanvas extends JComponent {
* ref to opaque boolean
*/
protected boolean m_isOpaque;

protected final Canvas canvasId;


public DingCanvas(Canvas canvasId) {
this.canvasId = canvasId;
}

public Canvas getCanvasId() {
return canvasId;
}

/**
* Sets opacity of component
*
Expand Down
2 changes: 2 additions & 0 deletions ding-impl/ding-presentation-impl/src/main/java/org/cytoscape/ding/impl/InnerCanvas.java
View file
Expand Up @@ -41,6 +41,7 @@
import org.cytoscape.ding.EdgeView;
import org.cytoscape.ding.NodeView;
import org.cytoscape.ding.ViewChangeEdit;
import org.cytoscape.ding.impl.DGraphView.Canvas;
import org.cytoscape.ding.impl.events.ViewportChangeListener;
import org.cytoscape.graph.render.export.ImageImposter;
import org.cytoscape.graph.render.immed.EdgeAnchors;
Expand Down Expand Up @@ -154,6 +155,7 @@ public class InnerCanvas extends DingCanvas implements MouseListener, MouseMotio
private final CyServiceRegistrar serviceRegistrar;

InnerCanvas(Object lock, DGraphView view, CyServiceRegistrar serviceRegistrar) {
super(Canvas.NETWORK_CANVAS);
m_lock = lock;
m_view = view;
this.serviceRegistrar = serviceRegistrar;
Expand Down
142 changes: 82 additions & 60 deletions ...ntation-impl/src/main/java/org/cytoscape/ding/impl/cyannotator/AnnotationManagerImpl.java
View file
Expand Up @@ -4,13 +4,19 @@
import static java.util.stream.Collectors.toList;
import static org.cytoscape.ding.internal.util.ViewUtil.invokeOnEDTAndWait;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.cytoscape.ding.impl.ArbitraryGraphicsCanvas;
import org.cytoscape.ding.impl.DGraphView;
import org.cytoscape.ding.impl.DGraphView.Canvas;
import org.cytoscape.ding.impl.cyannotator.annotations.DingAnnotation;
import org.cytoscape.service.util.CyServiceRegistrar;
import org.cytoscape.view.model.CyNetworkView;
Expand Down Expand Up @@ -55,45 +61,44 @@ public AnnotationManagerImpl(final CyServiceRegistrar serviceRegistrar) {

@Override
public void addAnnotation(final Annotation annotation) {
if (!(annotation instanceof DingAnnotation))
return;

DingAnnotation dAnnotation = (DingAnnotation) annotation;
CyNetworkView view = annotation.getNetworkView();
CyAnnotator cyAnnotator = ((DGraphView)view).getCyAnnotator();

cyAnnotator.checkCycle(annotation);

invokeOnEDTAndWait(() -> {
if (dAnnotation.getCanvas() != null)
dAnnotation.getCanvas().add(dAnnotation.getComponent());
else
cyAnnotator.getForeGroundCanvas().add(dAnnotation.getComponent());

cyAnnotator.addAnnotation(annotation);
});

// TODO
// final CyEventHelper eventHelper = serviceRegistrar.getService(CyEventHelper.class);
// eventHelper.addEventPayload(view, annotation, AnnotationsAddedEvent.class);
addAnnotations(Arrays.asList(annotation));
}



@Override
public void addAnnotations(Collection<? extends Annotation> annotations) {
Map<DGraphView, List<DingAnnotation>> annotationsByView = groupByView(annotations);
Map<DGraphView,Map<Canvas,List<DingAnnotation>>> annotationsByView = groupByViewAndCanvasAndFlatten(annotations, false);
if (annotationsByView.isEmpty())
return;

for(Map.Entry<DGraphView, List<DingAnnotation>> entry : annotationsByView.entrySet()) {
// this throws an exception so we don't want to use forEach here
entry.getKey().getCyAnnotator().checkCycle(entry.getValue());
}
// checkCycle throws IllegalAnnotationStructureException
// we don't want this thrown from inside the invokeOnEDTAndWait call because it will get wrapped
// note all the groups have to be on the same canvas for this to work
annotationsByView.forEach((view, annotationsByCanvas) -> {
annotationsByCanvas.forEach((canvas, canvasAnnotations) -> {
view.getCyAnnotator().checkCycle(canvasAnnotations);
});
});

invokeOnEDTAndWait(() -> {
annotationsByView.forEach((view, viewAnnotations) -> {
Map<ArbitraryGraphicsCanvas, List<DingAnnotation>> annotationsByCanvas = groupByCanvas(view, viewAnnotations);
annotationsByCanvas.forEach(ArbitraryGraphicsCanvas::addAnnotations);
view.getCyAnnotator().addAnnotations(viewAnnotations);
annotationsByView.forEach((view, annotationsByCanvas) -> {
// We have to make sure the foreground annotations are added before the background annotations.
// Because group annotations must be on the foreground canvas, if we add annotations to the background
// before the groups are added it can lead to bad things happening.
List<DingAnnotation> all = new ArrayList<>();

if(annotationsByCanvas.containsKey(Canvas.FOREGROUND_CANVAS)) {
List<DingAnnotation> foregroundAnnotations = annotationsByCanvas.get(Canvas.FOREGROUND_CANVAS);
((ArbitraryGraphicsCanvas)view.getCanvas(Canvas.FOREGROUND_CANVAS)).addAnnotations(foregroundAnnotations);
all.addAll(foregroundAnnotations);
}
if(annotationsByCanvas.containsKey(Canvas.BACKGROUND_CANVAS)) {
List<DingAnnotation> backgroundAnnotations = annotationsByCanvas.get(Canvas.BACKGROUND_CANVAS);
((ArbitraryGraphicsCanvas)view.getCanvas(Canvas.BACKGROUND_CANVAS)).addAnnotations(backgroundAnnotations);
all.addAll(backgroundAnnotations);
}

view.getCyAnnotator().addAnnotations(all);
});
});

Expand All @@ -108,48 +113,30 @@ public void addAnnotations(Collection<? extends Annotation> annotations) {

@Override
public void removeAnnotation(final Annotation annotation) {
CyNetworkView view = annotation.getNetworkView();

if (!(view instanceof DGraphView))
return;

invokeOnEDTAndWait(() -> {
annotation.removeAnnotation();
((DGraphView)view).getCyAnnotator().removeAnnotation(annotation);
});

// TODO
// final CyEventHelper eventHelper = serviceRegistrar.getService(CyEventHelper.class);
// eventHelper.addEventPayload(view, annotation, AnnotationsRemovedEvent.class);
removeAnnotations(Arrays.asList(annotation));
}


@Override
public void removeAnnotations(Collection<? extends Annotation> annotations) {
Map<DGraphView, List<DingAnnotation>> annotationsByView = groupByView(annotations);

// throws IllegalAnnotationStructureException
Map<DGraphView,Map<Canvas,List<DingAnnotation>>> annotationsByView = groupByViewAndCanvasAndFlatten(annotations, true);
if (annotationsByView.isEmpty())
return;

invokeOnEDTAndWait(() -> {
annotationsByView.forEach((view, viewAnnotations) -> {
Map<ArbitraryGraphicsCanvas, List<DingAnnotation>> annotationsByCanvas = groupByCanvas(view,
viewAnnotations);
annotationsByCanvas.forEach((canvas, dingAnnotations) -> {
annotationsByView.forEach((view, annotationsByCanvas) -> {
annotationsByCanvas.forEach((canvasId, dingAnnotations) -> {
// The following code is a batch version of Annotation.removeAnnotation()
for (DingAnnotation a : dingAnnotations) {
if (a instanceof GroupAnnotation) {
// Remove all of its children
for (Annotation aa: ((GroupAnnotation) a).getMembers())
removeAnnotation(aa);
}

GroupAnnotation parent = a.getGroupParent();

if (parent != null)
parent.removeMember(a);
}

List<DingAnnotation> arrows = getArrows(dingAnnotations);

ArbitraryGraphicsCanvas canvas = (ArbitraryGraphicsCanvas)view.getCanvas(canvasId);
canvas.removeAnnotations(arrows);
canvas.removeAnnotations(dingAnnotations);
canvas.repaint();
Expand All @@ -167,20 +154,55 @@ public void removeAnnotations(Collection<? extends Annotation> annotations) {
// });
}


private static Map<DGraphView,Map<Canvas,List<DingAnnotation>>> groupByViewAndCanvasAndFlatten(Collection<? extends Annotation> annotations, boolean incudeExisting) {
Map<DGraphView,Map<Canvas,List<DingAnnotation>>> map = new HashMap<>();

groupByView(annotations).forEach((view, as) -> {
Set<DingAnnotation> flattened = flattenAnnotations(view, as, incudeExisting);
map.put(view, groupByCanvas(view, flattened));
});

return map;
}


private static Map<DGraphView,List<DingAnnotation>> groupByView(Collection<? extends Annotation> annotations) {
return annotations.stream()
.filter(a -> a instanceof DingAnnotation)
.map(a -> (DingAnnotation) a)
.collect(groupingBy(da -> (DGraphView)da.getNetworkView()));
}

private static Map<ArbitraryGraphicsCanvas,List<DingAnnotation>> groupByCanvas(DGraphView view, List<DingAnnotation> dingAnnotations) {
private static Map<Canvas,List<DingAnnotation>> groupByCanvas(DGraphView view, Collection<DingAnnotation> dingAnnotations) {
return dingAnnotations.stream().collect(groupingBy(da -> getCanvas(view, da)));
}

private static Set<DingAnnotation> flattenAnnotations(DGraphView view, List<DingAnnotation> annotaitons, boolean incudeExisting) {
Set<DingAnnotation> collector = new HashSet<>();
for(DingAnnotation a : annotaitons) {
flattenAnnotations(view, a, collector, incudeExisting);
}
return collector;
}

private static void flattenAnnotations(DGraphView view, DingAnnotation a, Set<DingAnnotation> collector, boolean includeExisting) {
if(!includeExisting && view.getCyAnnotator().contains(a))
return;
if(!collector.add(a))
return;

collector.add(a);
if(a instanceof GroupAnnotation) {
for(Annotation member : ((GroupAnnotation)a).getMembers()) {
flattenAnnotations(view, (DingAnnotation)member, collector, includeExisting);
}
}
}


private static ArbitraryGraphicsCanvas getCanvas(DGraphView view, DingAnnotation annotation) {
return annotation.getCanvas() == null ? view.getCyAnnotator().getForeGroundCanvas() : annotation.getCanvas();
private static Canvas getCanvas(DGraphView view, DingAnnotation annotation) {
return annotation.getCanvas() == null ? Canvas.FOREGROUND_CANVAS : annotation.getCanvas().getCanvasId();
}

private static List<DingAnnotation> getArrows(Collection<DingAnnotation> annotations) {
Expand Down
21 changes: 12 additions & 9 deletions ...g-presentation-impl/src/main/java/org/cytoscape/ding/impl/cyannotator/AnnotationTree.java
View file
Expand Up @@ -135,10 +135,10 @@ public static AnnotationTree buildTree(Collection<? extends Annotation> annotati
Map<Annotation,AnnotationNode> backgroundNodes = new HashMap<>();

for(Annotation a : layers.get(Annotation.FOREGROUND)) {
addNode(a, foregroundTree, foregroundNodes);
addNode(a, foregroundTree, foregroundNodes, cyAnnotator);
}
for(Annotation a : layers.get(Annotation.BACKGROUND)) {
addNode(a, backgroundTree, backgroundNodes);
addNode(a, backgroundTree, backgroundNodes, cyAnnotator);
}

foregroundTree.removeEmptyGroups();
Expand All @@ -151,18 +151,22 @@ public static AnnotationTree buildTree(Collection<? extends Annotation> annotati
return head;
}

private static void addNode(Annotation a, AnnotationNode root, Map<Annotation,AnnotationNode> all) {
private static void addNode(Annotation a, AnnotationNode root, Map<Annotation,AnnotationNode> all, CyAnnotator cyAnnotator) {
if(!(a instanceof DingAnnotation))
return;

AnnotationNode n = all.computeIfAbsent(a, AnnotationNode::new);

if(a instanceof DingAnnotation && ((DingAnnotation)a).getGroupParent() != null) {
DingAnnotation ga = (DingAnnotation)((DingAnnotation)a).getGroupParent();
AnnotationNode pn = all.get(ga);
DingAnnotation groupParent = (DingAnnotation)((DingAnnotation)a).getGroupParent();

if(groupParent != null && cyAnnotator.contains(groupParent)) {
AnnotationNode pn = all.get(groupParent);
if(pn == null) {
// Now we can create the Nodes for each GroupAnnotation we find,
// because a group node can be added to both background and foreground trees,
// since it may contain child annotations from different canvases
all.put(ga, pn = new AnnotationNode(ga));
addNode(ga, root, all);
all.put(groupParent, pn = new AnnotationNode(groupParent));
addNode(groupParent, root, all, cyAnnotator);
}

if(pn.getIndex(n) < 0)
Expand All @@ -181,7 +185,6 @@ private static Map<String, List<Annotation>> separateByLayers(Collection<? exten
if (list != null) {
for (Annotation a : list) {
if(a instanceof GroupAnnotation) {
// MKTODO groups that only contain annotations on one canvas should only show up on that canvas
map.get(Annotation.FOREGROUND).add(a);
map.get(Annotation.BACKGROUND).add(a);
} else {
Expand Down
8 changes: 6 additions & 2 deletions ...ding-presentation-impl/src/main/java/org/cytoscape/ding/impl/cyannotator/CyAnnotator.java
View file
Expand Up @@ -18,14 +18,12 @@
import java.util.Set;
import java.util.TreeMap;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.swing.event.SwingPropertyChangeSupport;

import org.cytoscape.application.CyUserLog;
import org.cytoscape.application.events.SetSelectedNetworksEvent;
import org.cytoscape.ding.impl.ArbitraryGraphicsCanvas;
import org.cytoscape.ding.impl.DGraphView;
import org.cytoscape.ding.impl.InnerCanvas;
Expand Down Expand Up @@ -444,6 +442,12 @@ public void removeAnnotations(Collection<? extends Annotation> annotations) {
public List<Annotation> getAnnotations() {
return annotationSet.isEmpty() ? Collections.emptyList() : new ArrayList<>(annotationSet);
}

public boolean contains(Annotation a) {
if(a == null)
return false;
return annotationSet.contains(a);
}

public void setSelectedAnnotation(final DingAnnotation a, final boolean selected) {
invokeOnEDT(() -> {
Expand Down
12 changes: 11 additions & 1 deletion ...pl/src/main/java/org/cytoscape/ding/impl/cyannotator/annotations/GroupAnnotationImpl.java
View file
Expand Up @@ -80,7 +80,7 @@ public GroupAnnotationImpl(
}

public GroupAnnotationImpl(DGraphView view, Map<String, String> argMap) {
super(view, argMap);
super(view, processArgs(argMap));

// Get the UUIDs of all of the annotations
if (argMap.containsKey(MEMBERS)) {
Expand All @@ -98,6 +98,16 @@ public GroupAnnotationImpl(DGraphView view, Map<String, String> argMap) {
}
}
}

@Override
public void setCanvas(String cnvs) {
// do nothing, must be on the foreground canvas
}

private static Map<String,String> processArgs(Map<String,String> argMap) {
argMap.remove(CANVAS);
return argMap;
}

@Override
public Class<? extends Annotation> getType() {
Expand Down

0 comments on commit 9675b5c

Please sign in to comment.