From c8b7246454d5367e18f32766624cd6a1d7b6485d Mon Sep 17 00:00:00 2001 From: Christian Schneider Date: Thu, 16 Jul 2020 15:42:31 +0200 Subject: [PATCH] klighd +.piccolo: enabled support of multiple KTexts per KLabel as demanded in #43 * introduced guarding property 'MULTIPLE_KTEXTS_PER_KLABEL' preserving the existing behavior by default if not explicitly set to 'true' * relaxed handling of label text, may now be specified within the first KText alternatively * dropped dead code as discussed in #54 --- .../kieler/klighd/kgraph/util/KGraphUtil.java | 6 ----- .../controller/DiagramController.java | 19 +++++++++++--- .../controller/KLabelRenderingController.java | 25 +++++++++---------- .../piccolo/internal/nodes/KLabelNode.java | 2 +- .../internal/nodes/KlighdStyledText.java | 2 +- .../KlighdDiagramLayoutConnector.java | 16 ++++++++++-- .../kieler/klighd/util/KlighdProperties.java | 16 +++++++++--- 7 files changed, 57 insertions(+), 29 deletions(-) diff --git a/plugins/de.cau.cs.kieler.klighd.kgraph/src/de/cau/cs/kieler/klighd/kgraph/util/KGraphUtil.java b/plugins/de.cau.cs.kieler.klighd.kgraph/src/de/cau/cs/kieler/klighd/kgraph/util/KGraphUtil.java index 713646890..7d900bf8d 100644 --- a/plugins/de.cau.cs.kieler.klighd.kgraph/src/de/cau/cs/kieler/klighd/kgraph/util/KGraphUtil.java +++ b/plugins/de.cau.cs.kieler.klighd.kgraph/src/de/cau/cs/kieler/klighd/kgraph/util/KGraphUtil.java @@ -153,12 +153,6 @@ public static void validate(final KNode graph) { if (node.getInsets() == null) { node.setInsets(FACTORY.createKInsets()); } - // Make sure labels are OK - } else if (element instanceof KLabel) { - KLabel label = (KLabel) element; - if (label.getText() == null) { - label.setText(""); - } // Make sure edges are OK } else if (element instanceof KEdge) { KEdge edge = (KEdge) element; diff --git a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/DiagramController.java b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/DiagramController.java index 991ebaa71..66caefd60 100644 --- a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/DiagramController.java +++ b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/DiagramController.java @@ -148,6 +148,9 @@ public class DiagramController { /** whether edges are drawn before nodes, i.e. nodes have priority over edges. */ private final boolean edgesFirst; + /** whether figure descriptions of KLabels may contain multiple KTexts. */ + private final boolean multipleKTextsPerKLabel; + /** whether to record layout changes, will be set to true by the KlighdLayoutManager. */ private boolean record = false; @@ -175,6 +178,7 @@ public DiagramController(final KNode graph, final KlighdMainCamera camera, final final ViewContext viewContext) { this(graph, camera, sync, getProperty(viewContext, KlighdProperties.EDGES_FIRST).booleanValue(), + getProperty(viewContext, KlighdProperties.MULTIPLE_KTEXTS_PER_KLABEL).booleanValue(), getProperty(viewContext, KlighdProperties.ZOOM_TO_FIT_CONTENT_SPACING)); camera.initClipsPortAndLabelsVisibility( @@ -199,13 +203,22 @@ private static T getProperty(ViewContext context, IProperty property) { * @param edgesFirst * determining whether edges are drawn before nodes, i.e. nodes have priority over * edges + * @param multipleKTextsPerKLabel + * whether figure descriptions of KLabels may contain multiple KTexts. + * @param defaultZoomToFitContentSpacing + * default spacing to be applied if {@link ZoomStyle#ZOOM_TO_FIT_CONTENT} is + * demanded, see also + * {@link de.cau.cs.kieler.klighd.util.KlighdProperties#ZOOM_TO_FIT_CONTENT_SPACING}, + * may be null */ - protected DiagramController(final KNode graph, final KlighdMainCamera camera, final boolean sync, - final boolean edgesFirst, final Spacing defaultZoomToFitContentSpacing) { + protected DiagramController(final KNode graph, final KlighdMainCamera camera, + final boolean sync, final boolean edgesFirst, final boolean multipleKTextsPerKLabel, + final Spacing defaultZoomToFitContentSpacing) { DiagramControllerHelper.resetGraphElement(graph); this.sync = sync; this.edgesFirst = edgesFirst; + this.multipleKTextsPerKLabel = multipleKTextsPerKLabel; this.canvasCamera = camera; @@ -1884,7 +1897,7 @@ private void updateRendering(final KLabelNode labelRep) { if (renderingController == null) { // the new rendering controller is attached to nodeRep in the constructor of // AbstractRenderingController - renderingController = new KLabelRenderingController(labelRep); + renderingController = new KLabelRenderingController(labelRep, multipleKTextsPerKLabel); // labelRep.addAttribute(RENDERING_KEY, renderingController); renderingController.initialize(this, sync); } else { diff --git a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/KLabelRenderingController.java b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/KLabelRenderingController.java index 9d25379f5..e90b0339f 100644 --- a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/KLabelRenderingController.java +++ b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/controller/KLabelRenderingController.java @@ -29,7 +29,6 @@ import de.cau.cs.kieler.klighd.microlayout.PlacementUtil; import de.cau.cs.kieler.klighd.piccolo.internal.nodes.KLabelNode; import de.cau.cs.kieler.klighd.piccolo.internal.nodes.KlighdStyledText; -import de.cau.cs.kieler.klighd.util.KlighdProperties; import edu.umd.cs.piccolo.PNode; /** @@ -37,14 +36,20 @@ */ public class KLabelRenderingController extends AbstractKGERenderingController { + /** whether figure descriptions shall be checked for at most one {@link KText} element. */ + private final boolean validateSingleKText; + /** * Constructs a rendering controller for a label. * * @param label * the Piccolo node representing a label + * @param multipleKTextsPerKLabel + * whether figure descriptions of KLabels may contain multiple KTexts. */ - public KLabelRenderingController(final KLabelNode label) { + public KLabelRenderingController(final KLabelNode label, final boolean multipleKTextsPerKLabel) { super(label.getViewModelElement(), label); + this.validateSingleKText = !multipleKTextsPerKLabel; } /** @@ -88,19 +93,11 @@ private PNodeController handleLabelRendering(final KRendering rendering, final KText kText = Iterators.getNext(kTexts, null); - if (kText == null || kTexts.hasNext()) { + if (kText == null || validateSingleKText && kTexts.hasNext()) { throw new RuntimeException("KLabel " + getGraphElement() + " must (deeply) contain exactly 1 KText element."); } - // Transfer selectability from KLabel to KText to properly handle the selection handlers - // Should only apply if the KText is not configured but KLabel is configured - if (!kText.getProperties().contains(KlighdProperties.NOT_SELECTABLE) - && getGraphElement().getProperties().contains(KlighdProperties.NOT_SELECTABLE)) { - kText.setProperty(KlighdProperties.NOT_SELECTABLE, - getGraphElement().getProperty(KlighdProperties.NOT_SELECTABLE)); - } - // create the rendering final PNodeController controller = createRendering(rendering, parent, Bounds.of(parent.getBoundsReference())); @@ -113,14 +110,16 @@ && getGraphElement().getProperties().contains(KlighdProperties.NOT_SELECTABLE)) // the opposite should never happen (see test above), this test is just for preventing // null pointer exceptions - textController.getNode().setText(parent.getText()); + textController.getNode().setText( + parent.getText() != null ? parent.getText() : kText.getText()); // add a listener on the parent's bend points addListener(KLabelNode.PROPERTY_TEXT, parent, controller.getNode(), new PropertyChangeListener() { public void propertyChange(final PropertyChangeEvent e) { - textController.getNode().setText(parent.getText()); + textController.getNode().setText( + parent.getText() != null ? parent.getText() : kText.getText()); textController.getNode().repaint(); } }); diff --git a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KLabelNode.java b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KLabelNode.java index 402e8d0ca..99a4836a0 100644 --- a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KLabelNode.java +++ b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KLabelNode.java @@ -34,7 +34,7 @@ public class KLabelNode extends KGraphElementNode { private KLabelRenderingController renderingController; /** the text. */ - private String text = ""; + private String text = null; /** * Constructs a Piccolo2D node for representing a {@code KLabel}. diff --git a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KlighdStyledText.java b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KlighdStyledText.java index b2752eeec..5bb48de3f 100644 --- a/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KlighdStyledText.java +++ b/plugins/de.cau.cs.kieler.klighd.piccolo/src/de/cau/cs/kieler/klighd/piccolo/internal/nodes/KlighdStyledText.java @@ -150,7 +150,7 @@ public String getText() { * The text string to be displayed. */ public void setText(final String theText) { - this.text = theText; + this.text = theText == null ? "" : theText; updateBounds(); } diff --git a/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/internal/macrolayout/KlighdDiagramLayoutConnector.java b/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/internal/macrolayout/KlighdDiagramLayoutConnector.java index 8cbb91268..836bf87e9 100644 --- a/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/internal/macrolayout/KlighdDiagramLayoutConnector.java +++ b/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/internal/macrolayout/KlighdDiagramLayoutConnector.java @@ -75,6 +75,8 @@ import de.cau.cs.kieler.klighd.krendering.KRenderingFactory; import de.cau.cs.kieler.klighd.krendering.KRenderingOptions; import de.cau.cs.kieler.klighd.krendering.KRenderingRef; +import de.cau.cs.kieler.klighd.krendering.KRenderingUtil; +import de.cau.cs.kieler.klighd.krendering.KText; import de.cau.cs.kieler.klighd.labels.management.LabelManagementResult; import de.cau.cs.kieler.klighd.microlayout.Bounds; import de.cau.cs.kieler.klighd.microlayout.PlacementUtil; @@ -552,9 +554,19 @@ private void createLabel(final LayoutMapping mapping, final KLabel label, final ElkGraphElement layoutLabeledElement, final boolean estimateSize, final boolean setFontLayoutOptions) { + final KText kText = Iterators.getNext( + Iterators.filter( + KRenderingUtil.selfAndAllChildren(label.getData(KRendering.class)), + KText.class), + null); + + final String labelText = + label.getText() != null ? label.getText() + : kText != null ? kText.getText() : ""; + final ElkLabel layoutLabel = - ElkGraphUtil.createLabel(label.getText(), layoutLabeledElement); - + ElkGraphUtil.createLabel(labelText, layoutLabeledElement); + KIdentifier id = label.getData(KIdentifier.class); if (id != null && !Strings.isNullOrEmpty(id.getId())) { layoutLabel.setIdentifier(id.getId()); diff --git a/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/util/KlighdProperties.java b/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/util/KlighdProperties.java index 2bd78fb7d..66116c662 100644 --- a/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/util/KlighdProperties.java +++ b/plugins/de.cau.cs.kieler.klighd/src/de/cau/cs/kieler/klighd/util/KlighdProperties.java @@ -98,7 +98,7 @@ private KlighdProperties() { * {@link de.cau.cs.kieler.klighd.kgraph.KGraphElement KGraphElement} (kge) into the corresponding * diagram if the value is true.
* This property is currently to be attached to the kge's - * {@link de.cau.cs.kieler.kiml.klayoutdata.KLayoutData } data during the view synthesis process. + * {@link de.cau.cs.kieler.klighd.kgraph.KLayoutData KLayoutData} data during the view synthesis process. * If it is absent the kge is incorporated, anyway. */ public static final IProperty SHOW = new Property( @@ -256,8 +256,8 @@ public static boolean isSelectable(final EObject viewElement) { * Property determining whether the diagram zoom scale-based visibility configurations of the * {@link de.cau.cs.kieler.klighd.krendering.KRendering KRendering} it is defined on shall apply * to its children, as well. Configuring this property on {@link KGraphElement KGraphElements} - * (' {@link KLayoutData}) will have no effect, {@link KGraphElement KGraphElements'} children - * are automatically skipped by default. + * (' {@link de.cau.cs.kieler.klighd.kgraph.KLayoutData KLayoutData}) will have no effect, + * {@link KGraphElement KGraphElements'} children are automatically skipped by default. */ public static final IProperty VISIBILITY_PROPAGATE_TO_CHILDREN = new Property( "de.cau.cs.kieler.klighd.visibilityPropagateToChildren", false); @@ -308,6 +308,16 @@ public static boolean isSelectable(final EObject viewElement) { public static final IProperty EDGES_FIRST = new Property( "klighd.edgesFirst", false); + /** + * Property for globally determining whether figure descriptions of + * {@link de.cau.cs.kieler.klighd.kgraph.KLabel KLabel}s may contain multiple KTexts. Enabling + * this has implications on the applicability of layout algorithms performing the computation of + * the label size on their own, as well as the on the applicability of + * {@link org.eclipse.elk.core.labels.ILabelManager ILabelManager}s. + */ + public static final IProperty MULTIPLE_KTEXTS_PER_KLABEL = new Property( + "klighd.multipleKTextsPerKLabel", false); + /** * Determines whether the ports and port labels of clipped nodes should be shown or not. * By default the ports are shown, preventing issues with links to external ports.