diff --git a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java index 9c6ce63b66..04a7c3c209 100644 --- a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java +++ b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java @@ -855,7 +855,7 @@ class FGActionManager { HelpLocation layoutHelpLocation = new HelpLocation("FunctionGraphPlugin", "Function_Graph_Action_Layout"); - layoutAction = new MultiStateDockingAction<>("Relayout Graph", plugin.getName()) { + layoutAction = new MultiStateDockingAction<>("Relayout Graph", plugin.getName(), true) { @Override protected void doActionPerformed(ActionContext context) { diff --git a/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLEdgeLabelRenderer.java b/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLEdgeLabelRenderer.java index 20abdb22c2..1675135060 100644 --- a/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLEdgeLabelRenderer.java +++ b/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLEdgeLabelRenderer.java @@ -33,6 +33,7 @@ import edu.uci.ics.jung.visualization.transform.shape.GraphicsDecorator; import ghidra.app.plugin.core.functiongraph.graph.FGEdge; import ghidra.app.plugin.core.functiongraph.graph.vertex.FGVertex; import ghidra.graph.viewer.vertex.VisualGraphVertexShapeTransformer; +import ghidra.program.model.symbol.FlowType; /** * An edge label renderer used with the {@link DecompilerNestedLayout} @@ -86,11 +87,18 @@ class DNLEdgeLabelRenderer double cx = start.getX(); double cy = start.getY(); + EdgeLabelRenderer labelRenderer = rc.getEdgeLabelRenderer(); + Font font = rc.getEdgeFontTransformer().apply(e); + boolean isSelected = rc.getPickedEdgeState().isPicked(e); + Component component = labelRenderer.getEdgeLabelRendererComponent(rc.getScreenDevice(), + text, font, isSelected, e); + int labelWidth = component.getPreferredSize().width; + List articulationPoints = e.getArticulationPoints(); if (articulationPoints.isEmpty()) { double vertexBottom = start.getY() + (vertexBounds.height >> 1); // location is centered - int textY = (int) (vertexBottom + edgeOffset); // below the vertex; above the bend - int textX = (int) (start.getX() + xDisplacement); // right of the edge + double textY = (int) (vertexBottom + edgeOffset); // below the vertex; above the bend + double textX = (int) (start.getX() + xDisplacement); // right of the edge labelPointOffset.setLocation(textX, textY); } else if (articulationPoints.size() == 1) { @@ -109,29 +117,26 @@ class DNLEdgeLabelRenderer double bx1 = bend1.getX(); + FlowType flow = e.getFlowType(); + boolean isRight = flow.isFallthrough() || flow.isUnConditional(); + if (articulationPoints.size() == 2) { - int textX = (int) (vertexSide + edgeOffset); // right of the vertex - int textY = (int) (cy + edgeOffset); // above the edge + double textX = (int) (vertexSide + edgeOffset); // right of the vertex + double textY = (int) (cy + edgeOffset); // above the edge labelPointOffset.setLocation(textX, textY); } - else if (articulationPoints.size() == 3) { + else { // 3 or 4 articulations + + double textY = (int) (vertexBottom + edgeOffset); // below the vertex; above the bend + double textX = (int) (bx1 + xDisplacement); // right of the edge + if (!isRight) { + textX = bx1 - xDisplacement - labelWidth; + } - int textY = (int) (vertexBottom + edgeOffset); // below the vertex; above the bend - int textX = (int) (bx1 + xDisplacement); // right of the edge - labelPointOffset.setLocation(textX, textY); - } - else { // if (articulationPoints.size() == 4) { - int textY = (int) (vertexBottom + edgeOffset); // below the vertex; above the bend - int textX = (int) (bx1 + xDisplacement); // right of the edge labelPointOffset.setLocation(textX, textY); } } - EdgeLabelRenderer labelRenderer = rc.getEdgeLabelRenderer(); - Font font = rc.getEdgeFontTransformer().apply(e); - boolean isSelected = rc.getPickedEdgeState().isPicked(e); - Component component = labelRenderer.getEdgeLabelRendererComponent(rc.getScreenDevice(), - text, font, isSelected, e); Dimension d = component.getPreferredSize(); @@ -142,7 +147,37 @@ class DNLEdgeLabelRenderer g.setTransform(xform); g.draw(component, rc.getRendererPane(), 0, 0, d.width, d.height, true); - g.setTransform(old); + + // debug + //labelArticulations(component, g, rc, e); + } + + @SuppressWarnings("unused") // used during debug + private void labelArticulations(Component component, GraphicsDecorator g, + RenderContext rc, E e) { + + int offset = 5; + int counter = 1; + List points = e.getArticulationPoints(); + for (Point2D p : points) { + + MultiLayerTransformer multiLayerTransformer = rc.getMultiLayerTransformer(); + p = multiLayerTransformer.transform(Layer.LAYOUT, p); + + EdgeLabelRenderer labelRenderer = rc.getEdgeLabelRenderer(); + Font font = rc.getEdgeFontTransformer().apply(e); + boolean isSelected = rc.getPickedEdgeState().isPicked(e); + component = labelRenderer.getEdgeLabelRendererComponent(rc.getScreenDevice(), + "p" + counter++, font, isSelected, e); + + Dimension d = component.getPreferredSize(); + AffineTransform old = g.getTransform(); + AffineTransform xform = new AffineTransform(old); + xform.translate(p.getX() + offset, p.getY()); + g.setTransform(xform); + g.draw(component, rc.getRendererPane(), 0, 0, d.width, d.height, true); + g.setTransform(old); + } } } diff --git a/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DecompilerNestedLayout.java b/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DecompilerNestedLayout.java index 89e4706a4d..39062c64c8 100644 --- a/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DecompilerNestedLayout.java +++ b/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DecompilerNestedLayout.java @@ -56,11 +56,17 @@ import ghidra.util.task.TaskMonitor; *

Edges returning to the default code flow are painted lighter to de-emphasize them. This * could be made into an option. * + *

Edge routing herein defaults to 'simple routing'; 'complex routing' is a user option. + * Simple routing will reduce edge noise as much as possible by combining/overlapping edges that + * flow towards the bottom of the function (returning code flow). Also, edges may fall behind + * vertices for some functions. Complex routing allows the user to visually follow the flow + * of an individual edge. Complex routing will prevent edges from overlapping and will route + * edges around vertices. Simple routing is better when the layout of the vertices is + * important to the user; complex routing is better when edges/relationships are more + * important to the user. + * * TODO ideas: * -paint fallthrough differently for all, or just for those returning to the baseline - * -using 'edge routing mode' that avoids vertices can be tweaked so that all returning edges also - * will not overlap (do this by keeping some sort of mapping for a given edge's column); when - * not routing edges around vertices, the edges intentionally overlap, to reduce noise */ public class DecompilerNestedLayout extends AbstractFGLayout { @@ -265,9 +271,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { Column loopEndColumn = layoutToGridMap.nextColumn(outermostCol); List articulations = routeLoopEdge(start, end, loopEndColumn); newEdgeArticulations.put(e, articulations); - - vertex2dFactory.dispose(); - return newEdgeArticulations; + continue; } } @@ -340,7 +344,11 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // int delta = start.rowIndex - end.rowIndex; - int distanceSpacing = delta * EDGE_ENDPOINT_DISTANCE_MULTIPLIER; + int multiplier = EDGE_ENDPOINT_DISTANCE_MULTIPLIER; + if (useSimpleRouting()) { + multiplier = 1; // we allow edges to overlap with 'simple routing' + } + int distanceSpacing = delta * multiplier; // Condensing Note: we have guilty knowledge that our parent class my condense the // vertices and edges towards the center of the graph after we calculate positions. @@ -375,6 +383,9 @@ public class DecompilerNestedLayout extends AbstractFGLayout { double y2 = y1; articulations.add(new Point2D.Double(x2, y2)); + Column column = vertex2dFactory.getColumn(x1); + routeAroundColumnVertices(start, end, column.index, vertex2dFactory, articulations, x2); + double x3 = x2; double y3 = end.getBottom() - VERTEX_BORDER_THICKNESS; @@ -386,9 +397,6 @@ public class DecompilerNestedLayout extends AbstractFGLayout { y3 = Math.max(y3, endYLimit); articulations.add(new Point2D.Double(x3, y3)); - Column column = vertex2dFactory.getColumn(x1); - routeAroundColumnVertices(start, end, column.index, vertex2dFactory, articulations, x3); - double x4 = end.getX(); double y4 = y3; articulations.add(new Point2D.Double(x4, y4)); // point is hidden behind the vertex @@ -431,7 +439,11 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // int delta = end.rowIndex - start.rowIndex; - int distanceSpacing = delta * EDGE_ENDPOINT_DISTANCE_MULTIPLIER; + int multiplier = EDGE_ENDPOINT_DISTANCE_MULTIPLIER; + if (useSimpleRouting()) { + multiplier = 1; // we allow edges to overlap with 'simple routing' + } + int distanceSpacing = delta * multiplier; double x1 = start.getX() - VERTEX_BORDER_THICKNESS; // start at the center @@ -462,7 +474,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // restrict x from moving past the end vertex center x int edgeOffset = 0; - if (getLayoutOptions().useEdgeRoutingAroundVertices()) { + if (usesEdgeArticulations()) { // for now, only offset edge lines when we are performing complex routing edgeOffset = EDGE_OFFSET_INCOMING_FROM_LEFT; } @@ -489,7 +501,11 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // int delta = end.rowIndex - start.rowIndex; - int distanceSpacing = delta * EDGE_ENDPOINT_DISTANCE_MULTIPLIER; + int multiplier = EDGE_ENDPOINT_DISTANCE_MULTIPLIER; + if (useSimpleRouting()) { + multiplier = 1; // we allow edges to overlap with 'simple routing' + } + int distanceSpacing = delta * multiplier; double startRightX = start.getRight(); double x1 = startRightX - VERTEX_BORDER_THICKNESS; // start at the end @@ -540,14 +556,15 @@ public class DecompilerNestedLayout extends AbstractFGLayout { private void routeAroundColumnVertices(Vertex2d start, Vertex2d end, int column, Vertex2dFactory vertex2dFactory, List articulations, double edgeX) { - if (!getLayoutOptions().useEdgeRoutingAroundVertices()) { + if (useSimpleRouting()) { return; } + boolean goingDown = true; int startRow = start.rowIndex; int endRow = end.rowIndex; - if (startRow > endRow) { // going upwards + goingDown = false; endRow = start.rowIndex; startRow = end.rowIndex; } @@ -578,15 +595,20 @@ public class DecompilerNestedLayout extends AbstractFGLayout { } double centerX = otherVertex.getX(); - boolean isLeft = edgeX < centerX; + boolean goingLeft = edgeX < centerX; - // no need to check the 'y' value, as the end vertex is below this one + if (!goingDown) { + // for now, any time an edge goes up, we route it to the right + goingLeft = false; + } - if (isLeft) { - if (edgeX >= otherVertex.getLeft()) { - // less than center; after the start of the vertex + VertexClipper vertexClipper = new VertexClipper(goingLeft, goingDown); - /* + // no need to check the 'y' value, as the end vertex is above/below this one + if (vertexClipper.isClippingX(otherVertex, edgeX)) { + + /* + Must route around this vertex - new points: -p1 - just above the intersection point -p2 - just past the left edge @@ -599,89 +621,60 @@ public class DecompilerNestedLayout extends AbstractFGLayout { | | | | '-----' '---. - | + | + */ + + // p1 - same x; y just above vertex + double x = edgeX; + double y = vertexClipper.getTopOffset(otherVertex, vertexToEdgeOffset); + articulations.add(new Point2D.Double(x, y)); + + // Maybe merge points if they are too close together. Visually, many lines + // moving around intersecting vertices looks busy. When the intersecting + // vertices are close together, we remove some of the articulations in order to + // smooth out the edges. + if (articulations.size() > 2) { + + /* + The last articulation is the one added before this method was called, which + lies just below the intersecting vertex. The articulation before that is + the one that is the one that is sending the x value straight into the + intersecting vertex. Delete that point as well so that the entire edge is + shifted to the outside of the intersecting vertex. This will get repeated + for each vertex that is intersecting. */ - - // p1 - same x; y just above vertex - double x = edgeX; - double y = otherVertex.getTop() - vertexToEdgeOffset; - articulations.add(new Point2D.Double(x, y)); - - // maybe merge points if they are too close together - if (articulations.size() > 2) { - Point2D previousArticulation = articulations.get(articulations.size() - 2); - int closenessHeight = 50; - double previousY = previousArticulation.getY(); - if (y - previousY < closenessHeight) { - // remove our first articulation and the one before that so that - // the get merged into on line on the outside of the vertex - articulations.remove(articulations.size() - 1); - articulations.remove(articulations.size() - 1); - Point2D newPrevious = articulations.get(articulations.size() - 1); - y = newPrevious.getY(); - } + Point2D previousArticulation = articulations.get(articulations.size() - 2); + int closenessHeight = 50; + double previousY = previousArticulation.getY(); + if (vertexClipper.isTooCloseY(y, previousY, closenessHeight)) { + articulations.remove(articulations.size() - 1); + articulations.remove(articulations.size() - 1); + Point2D newPrevious = articulations.get(articulations.size() - 1); + y = newPrevious.getY(); } - - // p2 - move over; same y - int offset = Math.max(vertexToEdgeOffset, distanceSpacing); - offset *= exaggerationFactor; - x = otherVertex.getLeft() - offset; - articulations.add(new Point2D.Double(x, y)); - - // p3 - same x; move y below the vertex - y = otherVertex.getBottom() + vertexToEdgeOffset; - articulations.add(new Point2D.Double(x, y)); - - // p4 - move over back to our original x; same y - x = edgeX; - articulations.add(new Point2D.Double(x, y)); } - } - else { // right - double otherEnd = otherVertex.getRight(); - if (edgeX < otherEnd) { - // greater than center; before the end of the vertex - // (see not above for routing info) + // p2 - move over; same y + int offset = Math.max(vertexToEdgeOffset, distanceSpacing); + offset *= exaggerationFactor; + x = vertexClipper.getSideOffset(otherVertex, offset); + articulations.add(new Point2D.Double(x, y)); - // p1 - same x; y just above vertex - double x = edgeX; - double y = otherVertex.getTop() - vertexToEdgeOffset; - articulations.add(new Point2D.Double(x, y)); + // p3 - same x; move y above/below the vertex + y = vertexClipper.getBottomOffset(otherVertex, vertexToEdgeOffset); + articulations.add(new Point2D.Double(x, y)); - // maybe merge points if they are too close together - if (articulations.size() > 2) { - Point2D previousArticulation = articulations.get(articulations.size() - 2); - int closenessHeight = 50; - double previousY = previousArticulation.getY(); - if (y - previousY < closenessHeight) { - // remove our first articulation and the one before that so that - // the get merged into on line on the outside of the vertex - articulations.remove(articulations.size() - 1); - articulations.remove(articulations.size() - 1); - Point2D newPrevious = articulations.get(articulations.size() - 1); - y = newPrevious.getY(); - } - } - - // p2 - move over; same y - int offset = Math.max(vertexToEdgeOffset, distanceSpacing); - offset *= exaggerationFactor; - x = otherEnd + offset; - articulations.add(new Point2D.Double(x, y)); - - // p3 - same x; move y below the vertex - y = otherVertex.getBottom() + vertexToEdgeOffset; - articulations.add(new Point2D.Double(x, y)); - - // p4 - move over back to our original x; same y - x = edgeX; - articulations.add(new Point2D.Double(x, y)); - } + // p4 - move over back to our original x; same y + x = edgeX; + articulations.add(new Point2D.Double(x, y)); } } } + private boolean useSimpleRouting() { + return !getLayoutOptions().useEdgeRoutingAroundVertices(); + } + private List routeLoopEdge(Vertex2d start, Vertex2d end, Column loopEndColumn) { // going backwards @@ -691,8 +684,17 @@ public class DecompilerNestedLayout extends AbstractFGLayout { int halfWidth = loopEndColumn.getPaddedWidth(isCondensedLayout()) >> 1; double x = loopEndColumn.x + halfWidth; // middle of the column - Point2D startVertexPoint = start.center; + int startRow = start.rowIndex; + int endRow = end.rowIndex; + if (startRow > endRow) { // going upwards + endRow = start.rowIndex; + startRow = end.rowIndex; + } + int delta = endRow - startRow; + x += delta; // adding the delta makes overlap less likely + + Point2D startVertexPoint = start.center; double y1 = startVertexPoint.getY(); Point2D first = new Point2D.Double(x, y1); articulations.add(first); @@ -745,7 +747,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { } private void debug(String text) { -// System.err.println(text); + // System.err.println(text); } private void printParts(int depth, BlockGraph block) { @@ -937,6 +939,59 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // Inner Classes //================================================================================================== + /** + * Encapsulates knowledge of edge direction (up/down, left/right) and uses that knowledge + * to report vertex offsets from the appropriate side and top/bottom + */ + private class VertexClipper { + + boolean goingLeft; + boolean goingDown; + + VertexClipper(boolean isLeft, boolean isBottom) { + this.goingLeft = isLeft; + this.goingDown = isBottom; + } + + private double getSide(Vertex2d v) { + return goingLeft ? v.getLeft() : v.getRight(); + } + + double getTopOffset(Vertex2d v, int offset) { + return goingDown ? v.getTop() - offset : v.getBottom() + offset; + } + + double getBottomOffset(Vertex2d v, int offset) { + return goingDown ? v.getBottom() + offset : v.getTop() - offset; + } + + double getSideOffset(Vertex2d v, int offset) { + + double side = getSide(v); + if (goingLeft) { + return side - offset; + } + return side + offset; + } + + boolean isTooCloseY(double topY, double bottomY, double threshold) { + double delta = goingDown ? topY - bottomY : bottomY - topY; + return delta < threshold; + } + + boolean isClippingX(Vertex2d v, double x) { + + double side = getSide(v); + if (goingLeft) { + return x >= side; + } + return x < side; + } + } + + /** + * Factory for creating and caching {@link Vertex2d} objects + */ private class Vertex2dFactory { private VisualGraphVertexShapeTransformer vertexShaper; @@ -1216,7 +1271,8 @@ public class DecompilerNestedLayout extends AbstractFGLayout { @Override public String toString() { - return PcodeBlock.typeToName(pcodeBlock.getType()) + " - " + pcodeBlock.getStart(); + return PcodeBlock.typeToName(pcodeBlock.getType()) + " - " + getName() + " - " + + pcodeBlock.getStart(); } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/menu/MultiStateDockingAction.java b/Ghidra/Framework/Docking/src/main/java/docking/menu/MultiStateDockingAction.java index feec38cda4..468a934ca0 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/menu/MultiStateDockingAction.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/menu/MultiStateDockingAction.java @@ -35,6 +35,11 @@ import resources.icons.EmptyIcon; * drop-down icon that allows users to change the state of the button. Also, by default, as * the user presses the button, it will execute the action corresponding to the current * state. + * + *

Warning: if you use this action in a toolbar, then be sure to call the + * {@link #MultiStateDockingAction(String, String, boolean) correct constructor}. If you call + * another constructor, or pass false for this boolean above, your + * {@link #doActionPerformed(ActionContext)} method will get called twice. * * @param the type of the user data * @see MultiActionDockingAction @@ -56,10 +61,25 @@ public abstract class MultiStateDockingAction extends DockingAction { // stub for toolbar actions }; + /** + * Call this constructor with this action will not be added to a toolbar + * + * @param name the action name + * @param owner the owner + * @see #MultiStateDockingAction(String, String, boolean) + */ public MultiStateDockingAction(String name, String owner) { this(name, owner, false); } + /** + * Use this constructor explicitly when this action is used in a toolbar, passing true + * for isToolbarAction (see the javadoc header note). + * + * @param name the action name + * @param owner the owner + * @param isToolbarAction true if this action is a toolbar action + */ protected MultiStateDockingAction(String name, String owner, boolean isToolbarAction) { super(name, owner); multiActionGenerator = context -> getStateActions(); @@ -87,6 +107,9 @@ public abstract class MultiStateDockingAction extends DockingAction { *

* Also, if the parameter is true, then the button will behave like a button in terms of * mouse feedback. If false, then the button will behave more like a label. + * + * @param doPerformAction true to call {@link #doActionPerformed(ActionContext)} when the + * user presses the button for this action (not the drop-down menu; see above) */ public void setPerformActionOnPrimaryButtonClick(boolean doPerformAction) { performActionOnPrimaryButtonClick = doPerformAction; @@ -116,6 +139,8 @@ public abstract class MultiStateDockingAction extends DockingAction { * This is the callback to be overridden when the child wishes to respond to user button * presses that are on the button and not the drop-down. This will only be called if * {@link #performActionOnPrimaryButtonClick} is true. + * + * @param context the action context */ protected void doActionPerformed(ActionContext context) { // override me to do work diff --git a/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/renderer/VisualGraphRenderer.java b/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/renderer/VisualGraphRenderer.java index e697c9da4d..713a186f33 100644 --- a/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/renderer/VisualGraphRenderer.java +++ b/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/renderer/VisualGraphRenderer.java @@ -34,6 +34,9 @@ import ghidra.graph.viewer.layout.*; * This was created to add the ability to paint selected vertices above other vertices. We need * this since the Jung Graph has no notion of Z-order and thus does not let us specify that any * particular vertex should be above another one. + * + * @param the vertex type + * @param the edge type */ public class VisualGraphRenderer> extends edu.uci.ics.jung.visualization.renderers.BasicRenderer {