From 76569d3ca9a877554cc4a48684845d53fe8dfaab Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Wed, 12 May 2021 11:02:30 -0400 Subject: [PATCH] GP-940 - Function Graph - fixed caching satellite view not showing primary view lens; fixed occasional bad vertex clipping --- .../graph/layout/DecompilerNestedLayout.java | 8 ++- .../layout/AbstractVisualGraphLayout.java | 60 ++++++++++++------- .../CachingSatelliteGraphViewer.java | 15 +++-- 3 files changed, 55 insertions(+), 28 deletions(-) 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 39c2744d80..5b9facc738 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 @@ -112,6 +112,12 @@ public class DecompilerNestedLayout extends AbstractFGLayout { return new DNLEdgeLabelRenderer<>(getCondenseFactor()); } + @Override + protected void condenseEdges(List> rows, + Map> newEdgeArticulations, double centerX, double centerY) { + // do not condense, as we route our edges at the preferred positions + } + @Override protected double getCondenseFactor() { // our layout needs more spacing because we have custom edge routing that we want to @@ -714,7 +720,7 @@ public class DecompilerNestedLayout extends AbstractFGLayout { -p2 - just past the left edge -p3 - just past the bottom of the vertex -p4 - back at the original x value - + | .___| | .-----. diff --git a/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/layout/AbstractVisualGraphLayout.java b/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/layout/AbstractVisualGraphLayout.java index afecdf2477..46ef566245 100644 --- a/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/layout/AbstractVisualGraphLayout.java +++ b/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/layout/AbstractVisualGraphLayout.java @@ -313,20 +313,27 @@ public abstract class AbstractVisualGraphLayout vertexLayoutLocations = positionVerticesInLayoutSpace(transformer, vertices, layoutLocations); - Map> edgeLayoutArticulationLocations = new HashMap<>(); - Rectangle graphBounds = - getTotalGraphSize(vertexLayoutLocations, edgeLayoutArticulationLocations, transformer); + Rectangle graphBounds = getTotalGraphSize(vertexLayoutLocations, transformer); double centerX = graphBounds.getCenterX(); double centerY = graphBounds.getCenterY(); + // + // Condense vertices before placing edges. This allows layouts to perform custom routing + // of edges around vertices *after* condensing. + // if (isCondensed) { List> rows = gridLocations.rows(); - condense(rows, vertexLayoutLocations, edgeLayoutArticulationLocations, transformer, - centerX, centerY); + condenseVertices(rows, vertexLayoutLocations, transformer, centerX, centerY); } - edgeLayoutArticulationLocations = positionEdgeArticulationsInLayoutSpace(transformer, - vertexLayoutLocations, edges, layoutLocations); + Map> edgeLayoutArticulations = positionEdgeArticulationsInLayoutSpace( + transformer, vertexLayoutLocations, edges, layoutLocations); + + if (isCondensed) { + // note: some layouts will not condense the edges, as they perform custom routing + List> rows = gridLocations.rows(); + condenseEdges(rows, edgeLayoutArticulations, centerX, centerY); + } // DEGUG triggers grid lines to be printed; useful for debugging // VisualGraphRenderer.DEBUG_ROW_COL_MAP.put(this, layoutLocations.copy()); @@ -334,8 +341,7 @@ public abstract class AbstractVisualGraphLayout positionVerticesInLayoutSpace( @@ -430,11 +436,12 @@ public abstract class AbstractVisualGraphLayout vertexLocationMap, - Map> edgeArticulations, com.google.common.base.Function vertexShapeTransformer) { + // note: do not include edges in the size of the graph at this point, as some layouts use + // custom edge routing after this method is called Set vertices = vertexLocationMap.keySet(); - Set edges = edgeArticulations.keySet(); + Set edges = Collections.emptySet(); Function vertexToBounds = v -> { @@ -452,14 +459,13 @@ public abstract class AbstractVisualGraphLayout> edgeToArticulations = e -> edgeArticulations.get(e); + Function> edgeToArticulations = e -> Collections.emptyList(); Rectangle bounds = GraphViewerUtils.getTotalGraphSizeInLayoutSpace(vertices, edges, vertexToBounds, edgeToArticulations); return bounds; } - private void condense(List> rows, Map newLocations, - Map> newEdgeArticulations, + protected void condenseVertices(List> rows, Map newLocations, VisualGraphVertexShapeTransformer transformer, double centerX, double centerY) { // @@ -480,6 +486,22 @@ public abstract class AbstractVisualGraphLayout> rows, Map> newEdgeArticulations, + double centerX, double centerY) { + + // + // Note: we move the articulations and vertices closer together on the x-axis. We do + // not move the y-axis, as that is already as close together as we would like at + // this point. + // + double condenseFactor = getCondenseFactor(); Collection> edgeArticulations = newEdgeArticulations.values(); for (List edgePoints : edgeArticulations) { for (Point2D point : edgePoints) { @@ -493,12 +515,6 @@ public abstract class AbstractVisualGraphLayout row, int moveLeftStartIndex, Map vertexLocations, VisualGraphVertexShapeTransformer transformer) { - for (int i = moveLeftStartIndex; i >= 0; i--) { + for (int i = moveLeftStartIndex; i >= row.getStartColumn(); i--) { V vertex = row.getVertex(i); V rightVertex = getRightVertex(row, i); moveLeftIfOverlaps(vertexLocations, transformer, vertex, rightVertex); @@ -537,7 +553,7 @@ public abstract class AbstractVisualGraphLayout row, int moveRightStartIndex, Map vertexLocations, VisualGraphVertexShapeTransformer transformer) { - for (int i = moveRightStartIndex; i < row.getColumnCount(); i++) { + for (int i = moveRightStartIndex; i <= row.getEndColumn(); i++) { V vertex = row.getVertex(i); V leftVertex = getLeftVertex(row, i); moveRightIfOverlaps(vertexLocations, transformer, vertex, leftVertex); diff --git a/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/satellite/CachingSatelliteGraphViewer.java b/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/satellite/CachingSatelliteGraphViewer.java index 59411db56a..466984369e 100644 --- a/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/satellite/CachingSatelliteGraphViewer.java +++ b/Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/satellite/CachingSatelliteGraphViewer.java @@ -39,6 +39,8 @@ public class CachingSatelliteGraphViewer highlightRenderer = new VisualVertexSatelliteRenderer<>(); @@ -53,7 +55,10 @@ public class CachingSatelliteGraphViewer layout = masterViewer.getGraphLayout(); if (layout instanceof ObservableCachingLayout) { @@ -75,7 +80,7 @@ public class CachingSatelliteGraphViewer rc, V vertex, GraphicsDecorator g, Rectangle bounds) { - // Stub--we don't want the render to paint highlights, as we use a static, + // Stub--we don't want the render to paint highlights, as we use a static, // cached image. We will manually paint highlights in the paint routine of this // viewer. } @@ -95,16 +100,16 @@ public class CachingSatelliteGraphViewer