From d295dd2fd17fe36e0251f30b59cd3ed46e718006 Mon Sep 17 00:00:00 2001
From: Colin Rofls <colin@cmyr.net>
Date: Wed, 13 Sep 2023 16:03:21 -0400
Subject: [PATCH] [write-fonts] Graphviz improvements

This adds the ability to set an env var ('FONTC_PRUNE_GRAPH') that
tells write-fonts to prune a generated dotviz file to only include
spaces that contain an overflow.

It also cleanups up our graphviz code to not use subgraphs, which
weren't adding much if anything.
---
 write-fonts/src/graph.rs          |  5 ++-
 write-fonts/src/graph/graphviz.rs | 63 +++++++++++++++++++------------
 2 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/write-fonts/src/graph.rs b/write-fonts/src/graph.rs
index af1af822e..9acec80f4 100644
--- a/write-fonts/src/graph.rs
+++ b/write-fonts/src/graph.rs
@@ -1169,7 +1169,10 @@ impl Graph {
 
     #[cfg(feature = "dot2")]
     pub(crate) fn write_graph_viz(&self, path: impl AsRef<std::path::Path>) -> std::io::Result<()> {
-        graphviz::GraphVizGraph::from_graph(self).write_to_file(path)
+        // if this is set then we prune the generated graph
+        const PRUNE_GRAPH_ENV_VAR: &str = "FONTC_PRUNE_GRAPH";
+        let try_trim_graph = std::env::var_os(PRUNE_GRAPH_ENV_VAR).is_some();
+        graphviz::GraphVizGraph::from_graph(self, try_trim_graph).write_to_file(path)
     }
 }
 
diff --git a/write-fonts/src/graph/graphviz.rs b/write-fonts/src/graph/graphviz.rs
index b69a4d6d6..ce20d25c1 100644
--- a/write-fonts/src/graph/graphviz.rs
+++ b/write-fonts/src/graph/graphviz.rs
@@ -1,18 +1,45 @@
 //! Support for generating graphviz files from our object graph
 
-use super::{Graph, ObjectId, OffsetLen, Space};
+use std::collections::{BTreeSet, HashSet};
+
+use super::{Graph, ObjectId, OffsetLen};
 
 pub struct GraphVizGraph<'a> {
     graph: &'a Graph,
+    nodes: Vec<ObjectId>,
     edges: Vec<GraphVizEdge>,
 }
 
 impl<'a> GraphVizGraph<'a> {
-    pub(crate) fn from_graph(graph: &'a Graph) -> Self {
+    pub(crate) fn from_graph(graph: &'a Graph, prune_non_overflows: bool) -> Self {
         let mut edges = Vec::new();
+
+        // if we are pruning it means that we remove all nodes in spaces
+        // that do not include overflows.
+        let nodes: BTreeSet<_> = if !prune_non_overflows {
+            graph.objects.keys().copied().collect()
+        } else {
+            let overflows = graph.find_overflows();
+            let overflow_spaces = overflows
+                .iter()
+                .map(|overflow| graph.nodes.get(&overflow.child).unwrap().space)
+                .collect::<HashSet<_>>();
+            graph
+                .nodes
+                .iter()
+                .filter_map(|(id, node)| overflow_spaces.contains(&node.space).then_some(*id))
+                .collect()
+        };
+
         for (parent_id, table) in &graph.objects {
+            if !nodes.contains(parent_id) {
+                continue;
+            }
             let parent = &graph.nodes[parent_id];
             for link in &table.offsets {
+                if !nodes.contains(&link.object) {
+                    continue;
+                }
                 let child = &graph.nodes[&link.object];
                 let len = child.position - parent.position;
                 edges.push(GraphVizEdge {
@@ -23,7 +50,12 @@ impl<'a> GraphVizGraph<'a> {
                 });
             }
         }
-        GraphVizGraph { graph, edges }
+
+        GraphVizGraph {
+            graph,
+            edges,
+            nodes: nodes.into_iter().collect(),
+        }
     }
 
     /// Write out this graph as a graphviz file to the provided path.
@@ -46,13 +78,11 @@ pub struct GraphVizEdge {
 
 impl<'a> dot2::GraphWalk<'a> for GraphVizGraph<'a> {
     type Node = ObjectId;
-
     type Edge = GraphVizEdge;
-
-    type Subgraph = Space;
+    type Subgraph = ();
 
     fn nodes(&'a self) -> dot2::Nodes<'a, Self::Node> {
-        self.graph.order.as_slice().into()
+        self.nodes.as_slice().into()
     }
 
     fn edges(&'a self) -> dot2::Edges<'a, Self::Edge> {
@@ -66,29 +96,12 @@ impl<'a> dot2::GraphWalk<'a> for GraphVizGraph<'a> {
     fn target(&'a self, edge: &Self::Edge) -> Self::Node {
         edge.target
     }
-
-    fn subgraphs(&'a self) -> dot2::Subgraphs<'a, Self::Subgraph> {
-        let mut spaces: Vec<_> = self.graph.nodes.values().map(|x| x.space).collect();
-        spaces.sort_unstable();
-        spaces.dedup();
-        spaces.into()
-    }
-
-    fn subgraph_nodes(&'a self, s: &Self::Subgraph) -> dot2::Nodes<'a, Self::Node> {
-        self.graph
-            .nodes
-            .iter()
-            .filter_map(|(id, node)| (node.space == *s).then_some(*id))
-            .collect()
-    }
 }
 
 impl<'a> dot2::Labeller<'a> for GraphVizGraph<'a> {
     type Node = ObjectId;
-
     type Edge = GraphVizEdge;
-
-    type Subgraph = Space;
+    type Subgraph = ();
 
     fn graph_id(&'a self) -> dot2::Result<dot2::Id<'a>> {
         dot2::Id::new("TablePacking")