From 00f393dc3fdd40a761df4fe988745ecb0e62df4b Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 29 Oct 2020 14:58:34 -0700 Subject: [PATCH] [subset] finish up BFS sort implementation. --- src/hb-repacker.hh | 93 ++++++++++++++++++++++++++++++++++--------- src/test-repacker.cc | 95 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 151 insertions(+), 37 deletions(-) diff --git a/src/hb-repacker.hh b/src/hb-repacker.hh index 4bcc2393a..2be1433f6 100644 --- a/src/hb-repacker.hh +++ b/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& 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 (); 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 (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 queue; - hb_vector_t sorted_graph; + // BFS doesn't always produce a topological sort so this is just + // 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 queue; + hb_vector_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. - // 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). + sorted_graph.fini_deep (); } /* @@ -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* offset = reinterpret_cast*> (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 objects_; + public: + hb_vector_t objects_; }; diff --git a/src/test-repacker.cc b/src/test-repacker.cc index c94804054..c181eb02e 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -27,44 +27,102 @@ #include "hb-repacker.hh" #include "hb-open-type.hh" -static void -populate_serializer (hb_serialize_context_t* c) +static void start_object(const char* tag, + unsigned len, + hb_serialize_context_t* c) { - c->start_serialize (); c->push (); - char* obj = c->allocate_size (3); - strncpy (obj, "ghi", 3); - unsigned obj_3 = c->pop_pack (); + char* obj = c->allocate_size (len); + strncpy (obj, tag, len); +} - c->push (); - obj = c->allocate_size (3); - strncpy (obj, "def", 3); - unsigned obj_2 = c->pop_pack (); - c->push (); - obj = c->allocate_size (3); - strncpy (obj, "abc", 3); +static unsigned add_object(const char* tag, + unsigned len, + 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 (); c->extend_min (offset); - c->add_link (*offset, obj_2); + c->add_link (*offset, id); +} - offset = c->start_embed (); - c->extend_min (offset); - c->add_link (*offset, obj_3); +static void +populate_serializer_simple (hb_serialize_context_t* c) +{ + c->start_serialize (); + 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 (); + + 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 (); }