[repacker] add tests for multi-duplication.

Further improve the resolution attempt by pre-emptively raising priority of a duplicated shared node.

src/graph/graph.hh

@@ -27,7 +27,6 @@
 #include "../hb-set.hh"
 #include "../hb-priority-queue.hh"
 #include "../hb-serialize.hh"
-#include "hb.h"
 
 #ifndef GRAPH_GRAPH_HH
 #define GRAPH_GRAPH_HH
@@ -344,6 +343,16 @@ struct graph_t
       return true;
     }
 
+    bool give_max_priority ()
+    {
+      bool result = false;
+      while (!has_max_priority()) {
+        result = true;
+        priority++;
+      }
+      return result;
+    }
+
     bool has_max_priority () const {
       return priority >= 3;
     }

src/hb-repacker.hh

@@ -266,7 +266,24 @@ bool _resolve_shared_overflow(const hb_vector_t<graph::overflow_record_t>& overf
     result = sorted_graph.duplicate(&parents, r.child);
   }
 
-  return result != (unsigned) -1;
+  if (result == (unsigned) -1) return result;
+
+  if (parents.get_population() > 1) {
+    // If the duplicated node has more than one parent pre-emptively raise it's priority to the maximum.
+    // This will place it close to the parents. Node's with only one parent, don't need this as normal overflow
+    // resolution will raise priority if needed.
+    //
+    // Reasoning: most of the parents to this child are likely at the same layer in the graph. Duplicating
+    // the child will theoretically allow it to be placed closer to it's parents. However, due to the shortest
+    // distance sort by default it's placement will remain in the same layer, thus it will remain in roughly the
+    // same position (and distance from parents) as the original child node. The overflow resolution will attempt
+    // to move nodes closer, but only for non-shared nodes. Since this node is shared, it will simply be given
+    // further duplication which defeats the attempt to duplicate with multiple parents. To fix this we
+    // pre-emptively raise priority now which allows the duplicated node to pack into the same layer as it's parents.
+    sorted_graph.vertices_[result].give_max_priority();
+  }
+
+  return result;
 }
 
 static inline

src/test-repacker.cc

@@ -596,6 +596,31 @@ populate_serializer_with_dedup_overflow (hb_serialize_context_t* c)
   c->end_serialize();
 }
 
+static void
+populate_serializer_with_multiple_dedup_overflow (hb_serialize_context_t* c)
+{
+  std::string large_string(70000, 'a');
+  c->start_serialize<char> ();
+
+  unsigned leaf = add_object("def", 3, c);
+
+  constexpr unsigned num_mid_nodes = 20;
+  unsigned mid_nodes[num_mid_nodes];
+  for (unsigned i = 0; i < num_mid_nodes; i++) {
+    start_object(large_string.c_str(), 10000 + i, c);
+    add_offset(leaf, c);
+    mid_nodes[i] = c->pop_pack(false);
+  }
+
+  start_object("abc", 3, c);
+  for (unsigned i = 0; i < num_mid_nodes; i++) {
+    add_wide_offset(mid_nodes[i], c);
+  }
+  c->pop_pack(false);
+
+  c->end_serialize();
+}
+
 static void
 populate_serializer_with_isolation_overflow (hb_serialize_context_t* c)
 {
@@ -1682,6 +1707,21 @@ static void test_resolve_overflows_via_duplication ()
   hb_blob_destroy (out);
 }
 
+static void test_resolve_overflows_via_multiple_duplications ()
+{
+  size_t buffer_size = 300000;
+  void* buffer = malloc (buffer_size);
+  hb_serialize_context_t c (buffer, buffer_size);
+  populate_serializer_with_multiple_dedup_overflow (&c);
+  graph_t graph (c.object_graph ());
+
+  hb_blob_t* out = hb_resolve_overflows (c.object_graph (), HB_TAG_NONE, 5);
+  assert (out);
+
+  free (buffer);
+  hb_blob_destroy (out);
+}
+
 static void test_resolve_overflows_via_space_assignment ()
 {
   size_t buffer_size = 160000;
@@ -2141,6 +2181,7 @@ main (int argc, char **argv)
   test_will_overflow_3 ();
   test_resolve_overflows_via_sort ();
   test_resolve_overflows_via_duplication ();
+  test_resolve_overflows_via_multiple_duplications ();
   test_resolve_overflows_via_priority ();
   test_resolve_overflows_via_space_assignment ();
   test_resolve_overflows_via_isolation ();