From e19d59cac74a0cee509f1e559258098cd064b6a0 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Fri, 12 Mar 2021 15:26:35 -0500 Subject: [PATCH] GP-769 - Function Graph - added option to not used dimming for return flow edges --- .../FunctionGraphPlugin/Function_Graph.html | 16 +++ .../Function_Graph_Layouts.html | 7 ++ .../mvc/FunctionGraphOptions.java | 6 +- .../graph/layout/DNLayoutOptions.java | 18 ++- .../graph/layout/DecompilerNestedLayout.java | 118 +++++++++--------- 5 files changed, 103 insertions(+), 62 deletions(-) diff --git a/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph.html b/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph.html index 9e5de3196f..b66dce8c8f 100644 --- a/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph.html +++ b/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph.html @@ -840,6 +840,22 @@ location when zooming from the middle-mouse. The default for this option is off, which triggers zoom to work from the center of the graph, regardless of the mouse location.

+

The View Settings option describes how the graph will be zoomed when it is first + loaded. The values are:

+ + +
+
+

There are various edge color and highlight color options available to change. The highlight colors are those to be used when the flow animations take place.

diff --git a/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph_Layouts.html b/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph_Layouts.html index 2f2eab68a8..6e71260598 100644 --- a/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph_Layouts.html +++ b/Ghidra/Features/FunctionGraph/src/main/help/help/topics/FunctionGraphPlugin/Function_Graph_Layouts.html @@ -49,6 +49,13 @@ notes on how edges are routed for this layout.)

+ +
+

The Use Dim Return Edges option makes default code block return flow edges + lighter than conditional edges. This makes it easier for users to scan the + graph and ignore return flows. +

+
diff --git a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphOptions.java b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphOptions.java index 51b6773a9a..ea22f1cf44 100644 --- a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphOptions.java +++ b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphOptions.java @@ -42,7 +42,7 @@ public class FunctionGraphOptions extends VisualGraphOptions { private static final String EDGE_COLOR_CONDITIONAL_JUMP_KEY = "Edge Color - Conditional Jump "; //@formatter:off - private static final String NAVIGATION_HISTORY_KEY = "Navigation History"; + private static final String NAVIGATION_HISTORY_KEY = "Navigation History"; private static final String NAVIGATION_HISTORY_DESCRIPTION = "Determines how the navigation history will be updated when using the Function Graph. " + "The basic options are:" + @@ -185,8 +185,8 @@ public class FunctionGraphOptions extends VisualGraphOptions { options.registerOption(SCROLL_WHEEL_PANS_KEY, getScrollWheelPans(), help, SCROLL_WHEEL_PANS_DESCRIPTION); - options.registerOption(GRAPH_BACKGROUND_COLOR_KEY, DEFAULT_GRAPH_BACKGROUND_COLOR, - help, GRAPH_BACKGROUND_COLOR_DESCRPTION); + options.registerOption(GRAPH_BACKGROUND_COLOR_KEY, DEFAULT_GRAPH_BACKGROUND_COLOR, help, + GRAPH_BACKGROUND_COLOR_DESCRPTION); options.registerOption(DEFAULT_VERTEX_BACKGROUND_COLOR_KEY, DEFAULT_VERTEX_BACKGROUND_COLOR, help, DEFAULT_VERTEX_BACKGROUND_COLOR_DESCRPTION); diff --git a/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLayoutOptions.java b/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLayoutOptions.java index 1467a84781..1549c2cd9a 100644 --- a/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLayoutOptions.java +++ b/Ghidra/Features/FunctionGraphDecompilerExtension/src/main/java/ghidra/app/plugin/core/functiongraph/graph/layout/DNLayoutOptions.java @@ -31,7 +31,12 @@ public class DNLayoutOptions implements FGLayoutOptions { "edges should be routed around any intersecting vertex. When toggled off, edges will " + "pass through any intersecting vertices."; + private static final String DIM_RETURN_EDGES_KEY = "Use Dim Return Edges"; + private static final String DIM_RETURN_EDGES_DESCRIPTION = + "Signals to lighten the default return edges."; + private boolean useEdgeRoutingAroundVertices; + private boolean useDimmedReturnEdges = true; @Override public void registerOptions(Options options) { @@ -40,21 +45,32 @@ public class DNLayoutOptions implements FGLayoutOptions { options.registerOption(USE_EDGE_ROUTING_AROUND_VERTICES_KEY, useEdgeRoutingAroundVertices, help, USE_EDGE_ROUTING_AROUND_VERTICES_DESCRIPTION); + + options.registerOption(DIM_RETURN_EDGES_KEY, useDimmedReturnEdges, help, + DIM_RETURN_EDGES_DESCRIPTION); } @Override public void loadOptions(Options options) { useEdgeRoutingAroundVertices = options.getBoolean(USE_EDGE_ROUTING_AROUND_VERTICES_KEY, useEdgeRoutingAroundVertices); + + useDimmedReturnEdges = options.getBoolean(DIM_RETURN_EDGES_KEY, useDimmedReturnEdges); + } public boolean useEdgeRoutingAroundVertices() { return useEdgeRoutingAroundVertices; } + public boolean useDimmedReturnEdges() { + return useDimmedReturnEdges; + } + @Override public boolean optionChangeRequiresRelayout(String optionName) { // format: 'Nested Code Layout.Route Edges....' - return optionName.endsWith(USE_EDGE_ROUTING_AROUND_VERTICES_KEY); + return optionName.endsWith(USE_EDGE_ROUTING_AROUND_VERTICES_KEY) || + optionName.endsWith(DIM_RETURN_EDGES_KEY); } } 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 761c18f743..365444132e 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 @@ -52,19 +52,19 @@ import ghidra.util.task.TaskMonitor; /** * A layout that uses the decompiler to show code nesting based upon conditional logic. - * + * *

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. + * + *

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 + * 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 */ @@ -113,7 +113,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { @Override protected double getCondenseFactor() { - // our layout needs more spacing because we have custom edge routing that we want to + // our layout needs more spacing because we have custom edge routing that we want to // stand out return .3; } @@ -201,7 +201,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { continue; } - // + // // Special case: fallthrough--don't label this...not sure how to tell fallthrough. For // now assume that any column below or backwards is fallthrough. However, // do label fallthrough if it is the only edge. @@ -231,10 +231,10 @@ public class DecompilerNestedLayout extends AbstractFGLayout { Map> newEdgeArticulations = new HashMap<>(); - // Condensing Note: we have guilty knowledge that our parent class my condense the + // 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. // To prevent the edges from moving to far behind the vertices, we will compensate a - // bit for that effect using this offset value. The getEdgeOffset() method below is + // bit for that effect using this offset value. The getEdgeOffset() method below is // updated for the condense factor. int edgeOffset = isCondensedLayout() ? (int) (VERTEX_TO_EDGE_ARTICULATION_PADDING * (1 - getCondenseFactor())) @@ -242,7 +242,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { Vertex2dFactory vertex2dFactory = new Vertex2dFactory(transformer, vertexLayoutLocations, layoutToGridMap, edgeOffset); - // + // // Route our edges! // for (FGEdge e : edges) { @@ -275,31 +275,31 @@ public class DecompilerNestedLayout extends AbstractFGLayout { List articulations = new ArrayList<>(); // - // Basic routing: + // Basic routing: // -leave the bottom of the start vertex // -first bend at some constant offset // -move to right or left, to above the end vertex // -second bend above the end vertex at previous constant offset - // - // Edges start/end on the vertex center. If we offset them to avoid + // + // Edges start/end on the vertex center. If we offset them to avoid // overlapping, then they produce angles when only using two articulations. // Thus, we create articulations that are behind the vertices to remove // the angles. This points will not be seen. // - // + // // Complex routing: // -this mode will route edges around vertices - // + // // One goal for complex edge routing is to prevent overlapping (simple edge routing // prefers overlapping to reduce lines). To prevent overlapping we will use different - // offset x and y values, depending upon the start and end vertex row and column + // offset x and y values, depending upon the start and end vertex row and column // locations. Specifically, for a given edge direction there will be a bias: // -Edge to the right - leave from the right; arrive to the left // -Edge to the left - leave from the left; arrive to the right // -Edge straight down - go straight down // // For each of the above offsets, there will be an amplifier based upon row/column - // distance from start to end vertex. This has the effect that larger vertex + // distance from start to end vertex. This has the effect that larger vertex // distances will have a larger offset/spacing. // @@ -335,10 +335,10 @@ public class DecompilerNestedLayout extends AbstractFGLayout { Vertex2dFactory vertex2dFactory, List articulations) { // - // For routing to the right and back up we will leave the start vertex from the right side + // For routing to the right and back up we will leave the start vertex from the right side // and enter the end vertex on the right side. As the vertices get further apart, we will - // space them further in towards the center. - // + // space them further in towards the center. + // int delta = start.rowIndex - end.rowIndex; int multiplier = EDGE_ENDPOINT_DISTANCE_MULTIPLIER; @@ -347,10 +347,10 @@ public class DecompilerNestedLayout extends AbstractFGLayout { } int distanceSpacing = delta * multiplier; - // Condensing Note: we have guilty knowledge that our parent class my condense the + // 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. // To prevent the edges from moving to far behind the vertices, we will compensate a - // bit for that effect using this offset value. The getEdgeOffset() method is + // bit for that effect using this offset value. The getEdgeOffset() method is // updated for the condense factor. int exaggerationFactor = 1; if (isCondensedLayout()) { @@ -369,7 +369,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { y1 = Math.min(y1, startCenterY); articulations.add(new Point2D.Double(x1, y1)); // point is hidden behind the vertex - // Use the spacing to move the y value towards the top of the vertex. Just like with + // Use the spacing to move the y value towards the top of the vertex. Just like with // the x value, restrict the y to the range between the edge and the center. double startRightX = start.getRight(); double x2 = startRightX + VERTEX_BORDER_THICKNESS; // start at the end @@ -434,10 +434,10 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // // For routing to the left we will leave the start vertex from just left of center and - // enter the end vertex on the top, towards the right. As the vertices get further apart, - // we will space them further in towards the center of the end vertex. This will keep + // enter the end vertex on the top, towards the right. As the vertices get further apart, + // we will space them further in towards the center of the end vertex. This will keep // edges with close endpoints from intersecting edges with distant endpoints. - // + // int delta = end.rowIndex - start.rowIndex; int multiplier = EDGE_ENDPOINT_DISTANCE_MULTIPLIER; @@ -499,7 +499,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // enter the end vertex on the left side. As the vertices get further apart, we will // space them further in towards the center. This will keep edges with close endpoints // from intersecting edges with distant endpoints. - // + // int delta = end.rowIndex - start.rowIndex; if (delta < 0) { @@ -529,7 +529,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { double y1 = start.getY(); articulations.add(new Point2D.Double(x1, y1)); // point is hidden behind the vertex - // Use the spacing to move the y value towards the top of the vertex. Just like with + // Use the spacing to move the y value towards the top of the vertex. Just like with // the x value, restrict the y to the range between the edge and the center. double x2 = x1; double y2 = end.getTop() + VERTEX_BORDER_THICKNESS; @@ -605,10 +605,10 @@ public class DecompilerNestedLayout extends AbstractFGLayout { int padding = VERTEX_TO_EDGE_AVOIDANCE_PADDING; int distanceSpacing = padding + delta; // adding the delta makes overlap less likely - // Condensing Note: we have guilty knowledge that our parent class my condense the + // 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. // To prevent the edges from moving to far behind the vertices, we will compensate a - // bit for that effect using this offset value. The getEdgeOffset() method is + // bit for that effect using this offset value. The getEdgeOffset() method is // updated for the condense factor. int vertexToEdgeOffset = otherVertex.getEdgeOffset(); int exaggerationFactor = 1; @@ -629,20 +629,20 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // 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 -p3 - just past the bottom of the vertex -p4 - back at the original x value - + | .___| | .-----. | | | | '-----' '---. - | + | */ // p1 - same x; y just above vertex @@ -650,19 +650,19 @@ public class DecompilerNestedLayout extends AbstractFGLayout { 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 + // 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 + 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. + for each vertex that is intersecting. */ Point2D previousArticulation = articulations.get(articulations.size() - 2); int closenessHeight = 50; @@ -720,7 +720,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { Point2D first = new Point2D.Double(x, y1); articulations.add(first); - // loop second point - same y coord as destination; + // loop second point - same y coord as destination; // x is the col after the outermost dominated vertex Point2D endVertexPoint = end.center; @@ -733,6 +733,10 @@ public class DecompilerNestedLayout extends AbstractFGLayout { private void lighten(FGEdge e) { + if (!getLayoutOptions().useDimmedReturnEdges()) { + return; + } + // assumption: edges that move to the left in this layout are return flows that happen // after the code block has been executed. We dim those a bit so that they // produce less clutter. @@ -840,8 +844,11 @@ public class DecompilerNestedLayout extends AbstractFGLayout { BlockCopy copy = (BlockCopy) child; StringBuilder buffy = new StringBuilder(); - buffy.append(printDepth(depth, depth + 1)).append(' ').append(ID).append( - " plain - ").append(copy.getRef()); + buffy.append(printDepth(depth, depth + 1)) + .append(' ') + .append(ID) + .append(" plain - ") + .append(copy.getRef()); debug(buffy.toString()); } @@ -958,7 +965,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { //================================================================================================== // Inner Classes -//================================================================================================== +//================================================================================================== /** * Encapsulates knowledge of edge direction (up/down, left/right) and uses that knowledge @@ -1060,7 +1067,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { } /** - * A class that represents 2D information about the contained vertex, such as location, + * A class that represents 2D information about the contained vertex, such as location, * bounds, row and column of the layout grid. */ private class Vertex2d { @@ -1207,8 +1214,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { int row = startRow; - for (int i = 0; i < allChildren.size(); i++) { - DecompilerBlock block = allChildren.get(i); + for (DecompilerBlock block : allChildren) { if (block instanceof DecompilerBlockGraph) { row = ((DecompilerBlockGraph) block).setRows(row); } @@ -1229,8 +1235,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { String getChildrenString(int depth) { StringBuilder buffy = new StringBuilder(); int childCount = 0; - for (int i = 0; i < allChildren.size(); i++) { - DecompilerBlock block = allChildren.get(i); + for (DecompilerBlock block : allChildren) { if (block instanceof DecompilerBlockGraph) { String blockName = block.getName(); @@ -1315,8 +1320,8 @@ public class DecompilerNestedLayout extends AbstractFGLayout { @Override DecompilerBlock getBlock(FGVertex vertex) { // - // Note: we currently allow grouping in this layout. When we search for a vertex, - // we have to check each vertex inside of the given group *and* each vertex + // Note: we currently allow grouping in this layout. When we search for a vertex, + // we have to check each vertex inside of the given group *and* each vertex // inside of the vertex that belongs to this decompiler block. // if (vertex instanceof GroupedFunctionGraphVertex) { @@ -1447,9 +1452,8 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // The 'list' structure for children's nesting: // -all nodes are at the same level // - for (int i = 0; i < allChildren.size(); i++) { + for (DecompilerBlock block : allChildren) { int column = col; - DecompilerBlock block = allChildren.get(i); block.setCol(column); } @@ -1477,8 +1481,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // -each successive condition is another level nested // int column = col; - for (int i = 0; i < allChildren.size(); i++) { - DecompilerBlock block = allChildren.get(i); + for (DecompilerBlock block : allChildren) { block.setCol(column); column++; } @@ -1515,11 +1518,10 @@ public class DecompilerNestedLayout extends AbstractFGLayout { // // The 'do' structure for children's nesting: - // -all blocks nested + // -all blocks nested // int column = col + 1; - for (int i = 0; i < allChildren.size(); i++) { - DecompilerBlock block = allChildren.get(i); + for (DecompilerBlock block : allChildren) { block.setCol(column); }