[subset] finish up BFS sort implementation.

src/hb-repacker.hh

@@ -28,6 +28,7 @@
 #define HB_REPACKER_HH
 
 #include "hb-open-type.hh"
+#include "hb-map.hh"
 #include "hb-serialize.hh"
 #include "hb-vector.hh"
 
@@ -42,8 +43,30 @@ struct graph_t
    * serializer
    */
   graph_t (const hb_vector_t<hb_serialize_context_t::object_t *>& objects)
-      : objects_ (objects)
+  {
-  {}
+    bool removed_nil = false;
+    for (unsigned i = 0; i < objects.length; i++)
+    {
+      // If this graph came from a serialization buffer object 0 is the
+      // nil object. We don't need it for our purposes here so drop it.
+      if (i == 0 && !objects[i])
+      {
+        removed_nil = true;
+        continue;
+      }
+
+      auto* copy = objects_.push (*objects[i]);
+      if (!removed_nil) continue;
+      for (unsigned i = 0; i < copy->links.length; i++)
+        // Fix indices to account for removed nil object.
+        copy->links[i].objidx--;
+    }
+  }
+
+  ~graph_t ()
+  {
+    objects_.fini_deep ();
+  }
 
   /*
    * serialize graph into the provided serialization buffer.
@@ -52,17 +75,15 @@ struct graph_t
   {
     c->start_serialize<void> ();
     for (unsigned i = 0; i < objects_.length; i++) {
-      if (!objects_[i]) continue;
-
       c->push ();
 
-      size_t size = objects_[i]->tail - objects_[i]->head;
+      size_t size = objects_[i].tail - objects_[i].head;
       char* start = c->allocate_size <char> (size);
       if (!start) return;
 
-      memcpy (start, objects_[i]->head, size);
+      memcpy (start, objects_[i].head, size);
 
-      for (const auto& link : objects_[i]->links)
+      for (const auto& link : objects_[i].links)
         serialize_link (link, start, c);
 
       c->pop_pack (false);
@@ -75,36 +96,65 @@ struct graph_t
    */
   void sort_bfs ()
   {
-    hb_vector_t<int> queue;
+    // BFS doesn't always produce a topological sort so this is just
-    hb_vector_t<hb_serialize_context_t::object_t *> sorted_graph;
+    // for testing re-ordering capabilities for now.
+    // Will need to use a more advanced topological sorting algorithm
+
+    if (objects_.length <= 1) {
+      // Graph of 1 or less doesn't need sorting.
+      return;
+    }
+
+    hb_vector_t<unsigned> queue;
+    hb_vector_t<hb_serialize_context_t::object_t> sorted_graph;
+    hb_map_t id_map;
 
     // Object graphs are in reverse order, the first object is at the end
     // of the vector.
     queue.push (objects_.length - 1);
+    int new_id = objects_.length - 1;
 
     hb_set_t visited;
     while (queue.length)
     {
-      int next_id = queue[0];
+      unsigned next_id = queue[0];
       queue.remove(0);
       visited.add(next_id);
 
-      hb_serialize_context_t::object_t* next = objects_[next_id];
+      hb_serialize_context_t::object_t& next = objects_[next_id];
       sorted_graph.push (next);
+      id_map.set (next_id, new_id--);
 
-      for (const auto& link : next->links) {
+      for (const auto& link : next.links) {
         if (!visited.has (link.objidx))
           queue.push (link.objidx);
       }
     }
 
+    if (new_id != -1)
+    {
+      // Graph is not fully connected, there are unsorted objects.
+      // TODO(garretrieger): handle this.
+      assert (false);
+    }
+
+    // Apply objidx remapping.
+    // TODO(garretrieger): extract this to a helper.
+    for (unsigned i = 0; i < sorted_graph.length; i++)
+    {
+      for (unsigned j = 0; j < sorted_graph[i].links.length; j++)
+      {
+        auto& link = sorted_graph[i].links[j];
+        if (!id_map.has (link.objidx))
+          // TODO(garretrieger): handle this.
+          assert (false);
+        link.objidx = id_map.get (link.objidx);
+      }
+    }
+
     sorted_graph.as_array ().reverse ();
     objects_ = sorted_graph;
-    // TODO(garretrieger): remap object id's on the links.
+    sorted_graph.fini_deep ();
-    // TODO(garretrieger): what order should graphs be in (first object at the end? or the beginning)
-
-    // TODO(garretrieger): check that all objects made it over into the sorted copy
-    //                     (ie. all objects are connected in the original graph).
   }
 
   /*
@@ -113,6 +163,8 @@ struct graph_t
   bool will_overflow()
   {
     // TODO(garretrieger): implement me.
+    // Check for offsets that exceed their width or are negative if
+    // using a non-signed link.
     return false;
   }
 
@@ -126,7 +178,9 @@ struct graph_t
     OT::Offset<O>* offset = reinterpret_cast<OT::Offset<O>*> (head + link.position);
     *offset = 0;
     c->add_link (*offset,
-                 link.objidx,
+                 // serializer has an extra nil object at the start of the
+                 // object array. So all id's are +1 of what our id's are.
+                 link.objidx + 1,
                  (hb_serialize_context_t::whence_t) link.whence,
                  link.bias);
   }
@@ -153,7 +207,8 @@ struct graph_t
     }
   }
 
-  hb_vector_t<hb_serialize_context_t::object_t *> objects_;
+ public:
+  hb_vector_t<hb_serialize_context_t::object_t> objects_;
 };
 
 

src/test-repacker.cc

@@ -27,44 +27,102 @@
 #include "hb-repacker.hh"
 #include "hb-open-type.hh"
 
-static void
+static void start_object(const char* tag,
-populate_serializer (hb_serialize_context_t* c)
+                         unsigned len,
+                         hb_serialize_context_t* c)
 {
-  c->start_serialize<char> ();
   c->push ();
-  char* obj = c->allocate_size<char> (3);
+  char* obj = c->allocate_size<char> (len);
-  strncpy (obj, "ghi", 3);
+  strncpy (obj, tag, len);
-  unsigned obj_3 = c->pop_pack ();
+}
 
-  c->push ();
-  obj = c->allocate_size<char> (3);
-  strncpy (obj, "def", 3);
-  unsigned obj_2 = c->pop_pack ();
 
-  c->push ();
+static unsigned add_object(const char* tag,
-  obj = c->allocate_size<char> (3);
+                           unsigned len,
-  strncpy (obj, "abc", 3);
+                           hb_serialize_context_t* c)
+{
+  start_object (tag, len, c);
+  return c->pop_pack (false);
+}
 
+
+static void add_offset (unsigned id,
+                        hb_serialize_context_t* c)
+{
   OT::Offset16* offset = c->start_embed<OT::Offset16> ();
   c->extend_min (offset);
-  c->add_link (*offset, obj_2);
+  c->add_link (*offset, id);
+}
 
-  offset = c->start_embed<OT::Offset16> ();
+static void
-  c->extend_min (offset);
+populate_serializer_simple (hb_serialize_context_t* c)
-  c->add_link (*offset, obj_3);
+{
+  c->start_serialize<char> ();
 
+  unsigned obj_1 = add_object ("ghi", 3, c);
+  unsigned obj_2 = add_object ("def", 3, c);
+
+  start_object ("abc", 3, c);
+  add_offset (obj_2, c);
+  add_offset (obj_1, c);
   c->pop_pack ();
 
   c->end_serialize();
 }
 
+static void
+populate_serializer_complex (hb_serialize_context_t* c)
+{
+  c->start_serialize<char> ();
+
+  unsigned obj_4 = add_object ("jkl", 3, c);
+  unsigned obj_3 = add_object ("ghi", 3, c);
+
+  start_object ("def", 3, c);
+  add_offset (obj_3, c);
+  unsigned obj_2 = c->pop_pack (false);
+
+  start_object ("abc", 3, c);
+  add_offset (obj_2, c);
+  add_offset (obj_4, c);
+  c->pop_pack ();
+
+  c->end_serialize();
+}
+
+static void test_sort_bfs ()
+{
+  size_t buffer_size = 100;
+  void* buffer = malloc (buffer_size);
+  hb_serialize_context_t c (buffer, buffer_size);
+  populate_serializer_complex (&c);
+
+  graph_t graph (c.object_graph ());
+  graph.sort_bfs ();
+
+  assert(strncmp (graph.objects_[3].head, "abc", 3) == 0);
+  assert(graph.objects_[3].links.length == 2);
+  assert(graph.objects_[3].links[0].objidx == 2);
+  assert(graph.objects_[3].links[1].objidx == 1);
+
+  assert(strncmp (graph.objects_[2].head, "def", 3) == 0);
+  assert(graph.objects_[2].links.length == 1);
+  assert(graph.objects_[2].links[0].objidx == 0);
+
+  assert(strncmp (graph.objects_[1].head, "jkl", 3) == 0);
+  assert(graph.objects_[1].links.length == 0);
+
+  assert(strncmp (graph.objects_[0].head, "ghi", 3) == 0);
+  assert(graph.objects_[0].links.length == 0);
+}
+
 static void
 test_serialize ()
 {
   size_t buffer_size = 100;
   void* buffer_1 = malloc (buffer_size);
   hb_serialize_context_t c1 (buffer_1, buffer_size);
-  populate_serializer (&c1);
+  populate_serializer_simple (&c1);
   hb_bytes_t expected = c1.copy_bytes ();
 
   void* buffer_2 = malloc (buffer_size);
@@ -84,4 +142,5 @@ int
 main (int argc, char **argv)
 {
   test_serialize ();
+  test_sort_bfs ();
 }