[repacker] improve resolution of overflows to shared nodes.

Prior to this change overflows to shared nodes were handled by duplicating the link from only a single parent on each iteration. However, I've encountered fonts where there is a large number of parents sharing a single child. Using the prior strategy requires large number of overflow resolution iterations to resolve overflows. This changes shared overflow resolution to duplicate the shared child and re-assign multiple overflowing parents in a single iteration. This reduces total packing size in these cases and allows resolution to complete in far fewer iterations.

src/graph/graph.hh

@@ -27,6 +27,7 @@
 #include "../hb-set.hh"
 #include "../hb-priority-queue.hh"
 #include "../hb-serialize.hh"
+#include "hb.h"
 
 #ifndef GRAPH_GRAPH_HH
 #define GRAPH_GRAPH_HH
@@ -195,6 +196,15 @@ struct graph_t
       return incoming_edges_;
     }
 
+    unsigned incoming_edges_from_parent (unsigned parent_index) const {
+      if (single_parent != (unsigned) -1) {
+        return single_parent == parent_index ? 1 : 0;
+      }
+
+      unsigned* count;
+      return  parents.has(parent_index, &count) ? *count : 0;
+    }
+
     void reset_parents ()
     {
       incoming_edges_ = 0;
@@ -1023,6 +1033,11 @@ struct graph_t
    * Creates a copy of child and re-assigns the link from
    * parent to the clone. The copy is a shallow copy, objects
    * linked from child are not duplicated.
+   *
+   * Returns the index of the newly created duplicate.
+   *
+   * If the child_idx only has incoming edges from parent_idx, this
+   * will do nothing and return the original child_idx.
    */
   unsigned duplicate_if_shared (unsigned parent_idx, unsigned child_idx)
   {
@@ -1036,18 +1051,20 @@ struct graph_t
    * Creates a copy of child and re-assigns the link from
    * parent to the clone. The copy is a shallow copy, objects
    * linked from child are not duplicated.
+   *
+   * Returns the index of the newly created duplicate.
+   *
+   * If the child_idx only has incoming edges from parent_idx,
+   * duplication isn't possible and this will return -1.
    */
   unsigned duplicate (unsigned parent_idx, unsigned child_idx)
   {
     update_parents ();
 
-    unsigned links_to_child = 0;
-    for (const auto& l : vertices_[parent_idx].obj.all_links ())
-    {
-      if (l.objidx == child_idx) links_to_child++;
-    }
+    const auto& child = vertices_[child_idx];
+    unsigned links_to_child = child.incoming_edges_from_parent(parent_idx);
 
-    if (vertices_[child_idx].incoming_edges () <= links_to_child)
+    if (child.incoming_edges () <= links_to_child)
     {
       // Can't duplicate this node, doing so would orphan the original one as all remaining links
       // to child are from parent.
@@ -1060,7 +1077,7 @@ struct graph_t
                parent_idx, child_idx);
 
     unsigned clone_idx = duplicate (child_idx);
-    if (clone_idx == (unsigned) -1) return false;
+    if (clone_idx == (unsigned) -1) return -1;
     // duplicate shifts the root node idx, so if parent_idx was root update it.
     if (parent_idx == clone_idx) parent_idx++;
 
@@ -1076,6 +1093,62 @@ struct graph_t
     return clone_idx;
   }
 
+  /*
+   * Creates a copy of child and re-assigns the links from
+   * parents to the clone. The copy is a shallow copy, objects
+   * linked from child are not duplicated.
+   *
+   * Returns the index of the newly created duplicate.
+   *
+   * If the child_idx only has incoming edges from parents,
+   * duplication isn't possible or duplication fails and this will
+   * return -1.
+   */
+  unsigned duplicate (const hb_set_t* parents, unsigned child_idx)
+  {
+    if (parents->is_empty()) {
+      return -1;
+    }
+
+    update_parents ();
+
+    const auto& child = vertices_[child_idx];
+    unsigned links_to_child = 0;
+    unsigned last_parent = parents->get_max();
+    unsigned first_parent = parents->get_min();
+    for (unsigned parent_idx : *parents) {
+      links_to_child += child.incoming_edges_from_parent(parent_idx);
+    }
+
+    if (child.incoming_edges () <= links_to_child)
+    {
+      // Can't duplicate this node, doing so would orphan the original one as all remaining links
+      // to child are from parent.
+      DEBUG_MSG (SUBSET_REPACK, nullptr, "  Not duplicating %u, ..., %u => %u", first_parent, last_parent, child_idx);
+      return -1;
+    }
+
+    DEBUG_MSG (SUBSET_REPACK, nullptr, "  Duplicating %u, ..., %u => %u", first_parent, last_parent, child_idx);
+
+    unsigned clone_idx = duplicate (child_idx);
+    if (clone_idx == (unsigned) -1) return false;
+    
+    for (unsigned parent_idx : *parents) {
+      // duplicate shifts the root node idx, so if parent_idx was root update it.
+      if (parent_idx == clone_idx) parent_idx++;
+      auto& parent = vertices_[parent_idx];
+      for (auto& l : parent.obj.all_links_writer ())
+      {
+        if (l.objidx != child_idx)
+          continue;
+
+        reassign_link (l, parent_idx, clone_idx);
+      }
+    }
+
+    return clone_idx;
+  }
+
 
   /*
    * Adds a new node to the graph, not connected to anything.

src/hb-repacker.hh

@@ -238,6 +238,37 @@ bool _try_isolating_subgraphs (const hb_vector_t<graph::overflow_record_t>& over
   return true;
 }
 
+static inline
+bool _resolve_shared_overflow(const hb_vector_t<graph::overflow_record_t>& overflows,
+                              int overflow_index,
+                              graph_t& sorted_graph)
+{
+  const graph::overflow_record_t& r = overflows[overflow_index];
+
+  // Find all of the parents in overflowing links that link to this
+  // same child node. We will then try duplicating the child node and
+  // re-assigning all of these parents to the duplicate.
+  hb_set_t parents;
+  parents.add(r.parent);
+  for (int i = overflow_index - 1; i >= 0; i--) {
+    const graph::overflow_record_t& r2 = overflows[i];
+    if (r2.child == r.child) {
+      parents.add(r2.parent);
+    }
+  }
+
+  unsigned result = sorted_graph.duplicate(&parents, r.child);
+  if (result == (unsigned) -1 && parents.get_population() > 2) {
+    // All links to the child are overflowing, so we can't include all
+    // in the duplication. Remove one parent from the duplication.
+    // Remove the lowest index parent, which will be the closest to the child.
+    parents.del(parents.get_min());
+    result = sorted_graph.duplicate(&parents, r.child);
+  }
+
+  return result != (unsigned) -1;
+}
+
 static inline
 bool _process_overflows (const hb_vector_t<graph::overflow_record_t>& overflows,
                          hb_set_t& priority_bumped_parents,
@@ -254,7 +285,7 @@ bool _process_overflows (const hb_vector_t<graph::overflow_record_t>& overflows,
     {
       // The child object is shared, we may be able to eliminate the overflow
       // by duplicating it.
-      if (sorted_graph.duplicate (r.parent, r.child) == (unsigned) -1) continue;
+      if (!_resolve_shared_overflow(overflows, i, sorted_graph)) continue;
       return true;
     }