[subset] Improve sharing of Ligature subtables.

Ligature subtables use virtual links to enforce an ordering constraint between the subtables and the coverage table. Unfortunately this has the sideeffect of prevent the subtables from being shared by another Ligature with a different coverage table since object equality compares all links real and virtual. This change makes virtual links stored separately from real links and updates the equality check to only check real links. If an object is de-duped any virtual links it has are merged into the object that replaces it.
This commit is contained in:
Garret Rieger 2021-11-30 13:45:22 -08:00 committed by Behdad Esfahbod
parent ca22741110
commit 9121ed0cec
19 changed files with 181 additions and 103 deletions

View File

@ -100,7 +100,7 @@ struct graph_t
bool is_leaf () const
{
return !obj.links.length;
return !obj.real_links.length && !obj.virtual_links.length;
}
void raise_priority ()
@ -164,9 +164,11 @@ struct graph_t
if (check_success (!vertices_.in_error ()))
v->obj = *objects[i];
if (!removed_nil) continue;
for (unsigned i = 0; i < v->obj.links.length; i++)
// Fix indices to account for removed nil object.
v->obj.links[i].objidx--;
// Fix indices to account for removed nil object.
for (auto& l : hb_concat (v->obj.real_links.writer (),
v->obj.virtual_links.writer ())) {
l.objidx--;
}
}
}
@ -215,7 +217,8 @@ struct graph_t
memcpy (start, vertices_[i].obj.head, size);
for (const auto& link : vertices_[i].obj.links)
// Only real links needs to be serialized.
for (const auto& link : vertices_[i].obj.real_links)
serialize_link (link, start, c);
// All duplications are already encoded in the graph, so don't
@ -260,7 +263,7 @@ struct graph_t
sorted_graph[new_id] = next;
id_map[next_id] = new_id--;
for (const auto& link : next.obj.links) {
for (const auto& link : hb_concat (next.obj.real_links, next.obj.virtual_links)) {
removed_edges[link.objidx]++;
if (!(vertices_[link.objidx].incoming_edges () - removed_edges[link.objidx]))
queue.push (link.objidx);
@ -314,7 +317,7 @@ struct graph_t
sorted_graph[new_id] = next;
id_map[next_id] = new_id--;
for (const auto& link : next.obj.links) {
for (const auto& link : hb_concat (next.obj.real_links, next.obj.virtual_links)) {
removed_edges[link.objidx]++;
if (!(vertices_[link.objidx].incoming_edges () - removed_edges[link.objidx]))
// Add the order that the links were encountered to the priority.
@ -348,7 +351,8 @@ struct graph_t
hb_set_t roots;
for (unsigned i = 0; i <= root_index; i++)
{
for (auto& l : vertices_[i].obj.links)
// Only real links can form 32 bit spaces
for (auto& l : vertices_[i].obj.real_links)
{
if (l.width == 4 && !l.is_signed)
{
@ -466,7 +470,8 @@ struct graph_t
void find_subgraph (unsigned node_idx, hb_hashmap_t<unsigned, unsigned>& subgraph)
{
for (const auto& link : vertices_[node_idx].obj.links)
for (const auto& link : hb_concat (vertices_[node_idx].obj.real_links,
vertices_[node_idx].obj.virtual_links))
{
if (subgraph.has (link.objidx))
{
@ -482,7 +487,8 @@ struct graph_t
{
if (subgraph.has (node_idx)) return;
subgraph.add (node_idx);
for (const auto& link : vertices_[node_idx].obj.links)
for (const auto& link : hb_concat (vertices_[node_idx].obj.real_links,
vertices_[node_idx].obj.virtual_links))
find_subgraph (link.objidx, subgraph);
}
@ -497,7 +503,8 @@ struct graph_t
return;
index_map.set (node_idx, duplicate (node_idx));
for (const auto& l : object (node_idx).links) {
for (const auto& l : hb_concat (object (node_idx).real_links,
object (node_idx).virtual_links)) {
duplicate_subgraph (l.objidx, index_map);
}
}
@ -523,13 +530,19 @@ struct graph_t
clone->parents.reset ();
unsigned clone_idx = vertices_.length - 2;
for (const auto& l : child.obj.links)
for (const auto& l : child.obj.real_links)
{
clone->obj.links.push (l);
clone->obj.real_links.push (l);
vertices_[l.objidx].parents.push (clone_idx);
}
for (const auto& l : child.obj.virtual_links)
{
clone->obj.virtual_links.push (l);
vertices_[l.objidx].parents.push (clone_idx);
}
check_success (!clone->obj.links.in_error ());
check_success (!clone->obj.real_links.in_error ());
check_success (!clone->obj.virtual_links.in_error ());
// The last object is the root of the graph, so swap back the root to the end.
// The root's obj idx does change, however since it's root nothing else refers to it.
@ -539,7 +552,8 @@ struct graph_t
vertices_[vertices_.length - 1] = root;
// Since the root moved, update the parents arrays of all children on the root.
for (const auto& l : root.obj.links)
for (const auto& l : hb_concat (root.obj.real_links,
root.obj.virtual_links))
vertices_[l.objidx].remap_parent (root_idx () - 1, root_idx ());
return clone_idx;
@ -555,7 +569,8 @@ struct graph_t
update_parents ();
unsigned links_to_child = 0;
for (const auto& l : vertices_[parent_idx].obj.links)
for (const auto& l : hb_concat (vertices_[parent_idx].obj.real_links,
vertices_[parent_idx].obj.virtual_links))
{
if (l.objidx == child_idx) links_to_child++;
}
@ -578,9 +593,9 @@ struct graph_t
if (parent_idx == clone_idx) parent_idx++;
auto& parent = vertices_[parent_idx];
for (unsigned i = 0; i < parent.obj.links.length; i++)
for (auto& l : hb_concat (parent.obj.real_links.writer (),
parent.obj.virtual_links.writer ()))
{
auto& l = parent.obj.links[i];
if (l.objidx != child_idx)
continue;
@ -601,8 +616,9 @@ struct graph_t
// to invalidate positions. It does not change graph structure so no need
// to update distances or edge counts.
auto& parent = vertices_[parent_idx].obj;
for (unsigned i = 0; i < parent.links.length; i++)
vertices_[parent.links[i].objidx].raise_priority ();
for (auto& l : hb_concat (parent.real_links.writer (),
parent.virtual_links.writer ()))
vertices_[l.objidx].raise_priority ();
}
/*
@ -615,7 +631,8 @@ struct graph_t
for (int parent_idx = vertices_.length - 1; parent_idx >= 0; parent_idx--)
{
for (const auto& link : vertices_[parent_idx].obj.links)
// Don't need to check virtual links for overflow
for (const auto& link : vertices_[parent_idx].obj.real_links)
{
int64_t offset = compute_offset (parent_idx, link);
if (is_valid_offset (offset, link))
@ -665,11 +682,11 @@ struct graph_t
"%4d (%4d in, %4d out, space %2d)",
o.parent,
parent.incoming_edges (),
parent.obj.links.length,
parent.obj.real_links.length + parent.obj.virtual_links.length,
space_for (o.parent),
o.child,
child.incoming_edges (),
child.obj.links.length,
child.obj.real_links.length + child.obj.virtual_links.length,
space_for (o.child));
}
}
@ -728,7 +745,8 @@ struct graph_t
if (visited.has (p)) continue;
visited.add (p);
for (const auto& l : vertices_[p].obj.links)
// Only real links can be wide
for (const auto& l : vertices_[p].obj.real_links)
{
if (l.objidx == node_idx && l.width == 4 && !l.is_signed)
{
@ -755,7 +773,8 @@ struct graph_t
for (unsigned p = 0; p < vertices_.length; p++)
{
for (auto& l : vertices_[p].obj.links)
for (auto& l : hb_concat (vertices_[p].obj.real_links,
vertices_[p].obj.virtual_links))
{
vertices_[l.objidx].parents.push (p);
}
@ -823,7 +842,8 @@ struct graph_t
int64_t next_distance = vertices_[next_idx].distance;
visited[next_idx] = true;
for (const auto& link : next.obj.links)
for (const auto& link : hb_concat (next.obj.real_links,
next.obj.virtual_links))
{
if (visited[link.objidx]) continue;
@ -922,9 +942,9 @@ struct graph_t
if (!id_map) return;
for (unsigned i : subgraph)
{
for (unsigned j = 0; j < vertices_[i].obj.links.length; j++)
for (auto& link : hb_concat (vertices_[i].obj.real_links.writer (),
vertices_[i].obj.virtual_links.writer ()))
{
auto& link = vertices_[i].obj.links[j];
if (!id_map.has (link.objidx)) continue;
if (only_wide && !(link.width == 4 && !link.is_signed)) continue;
@ -942,9 +962,10 @@ struct graph_t
for (unsigned i = 0; i < sorted_graph->length; i++)
{
(*sorted_graph)[i].remap_parents (id_map);
for (unsigned j = 0; j < (*sorted_graph)[i].obj.links.length; j++)
for (auto& link : hb_concat ((*sorted_graph)[i].obj.real_links.writer (),
(*sorted_graph)[i].obj.virtual_links.writer ()))
{
auto& link = (*sorted_graph)[i].obj.links[j];
link.objidx = id_map[link.objidx];
}
}
@ -1023,7 +1044,8 @@ struct graph_t
const auto& v = vertices_[start_idx];
// Graph is treated as undirected so search children and parents of start_idx
for (const auto& l : v.obj.links)
for (const auto& l : hb_concat (v.obj.real_links,
v.obj.virtual_links))
find_connected_nodes (l.objidx, targets, visited, connected);
for (unsigned p : v.parents)

View File

@ -65,19 +65,26 @@ struct hb_serialize_context_t
struct object_t
{
void fini () { links.fini (); }
void fini () {
real_links.fini ();
virtual_links.fini ();
}
bool operator == (const object_t &o) const
{
// Virtual links aren't considered for equality since they don't affect the functionality
// of the object.
return (tail - head == o.tail - o.head)
&& (links.length == o.links.length)
&& (real_links.length == o.real_links.length)
&& 0 == hb_memcmp (head, o.head, tail - head)
&& links.as_bytes () == o.links.as_bytes ();
&& real_links.as_bytes () == o.real_links.as_bytes ();
}
uint32_t hash () const
{
// Virtual links aren't considered for equality since they don't affect the functionality
// of the object.
return hb_bytes_t (head, tail - head).hash () ^
links.as_bytes ().hash ();
real_links.as_bytes ().hash ();
}
struct link_t
@ -92,7 +99,8 @@ struct hb_serialize_context_t
char *head;
char *tail;
hb_vector_t<link_t> links;
hb_vector_t<link_t> real_links;
hb_vector_t<link_t> virtual_links;
object_t *next;
};
@ -101,12 +109,14 @@ struct hb_serialize_context_t
char *head;
char *tail;
object_t *current; // Just for sanity check
unsigned num_links;
unsigned num_real_links;
unsigned num_virtual_links;
hb_serialize_error_t errors;
};
snapshot_t snapshot ()
{ return snapshot_t { head, tail, current, current->links.length, errors }; }
{ return snapshot_t {
head, tail, current, current->real_links.length, current->virtual_links.length, errors }; }
hb_serialize_context_t (void *start_, unsigned int size) :
start ((char *) start_),
@ -282,7 +292,8 @@ struct hb_serialize_context_t
if (!len)
{
assert (!obj->links.length);
assert (!obj->real_links.length);
assert (!obj->virtual_links.length);
return 0;
}
@ -292,6 +303,7 @@ struct hb_serialize_context_t
objidx = packed_map.get (obj);
if (objidx)
{
merge_virtual_links (obj, objidx);
obj->fini ();
return objidx;
}
@ -327,7 +339,8 @@ struct hb_serialize_context_t
// Overflows that happened after the snapshot will be erased by the revert.
if (unlikely (in_error () && !only_overflow ())) return;
assert (snap.current == current);
current->links.shrink (snap.num_links);
current->real_links.shrink (snap.num_real_links);
current->virtual_links.shrink (snap.num_virtual_links);
errors = snap.errors;
revert (snap.head, snap.tail);
}
@ -375,8 +388,8 @@ struct hb_serialize_context_t
assert (current);
auto& link = *current->links.push ();
if (current->links.in_error ())
auto& link = *current->virtual_links.push ();
if (current->virtual_links.in_error ())
err (HB_SERIALIZE_ERROR_OTHER);
link.width = 0;
@ -400,8 +413,8 @@ struct hb_serialize_context_t
assert (current);
assert (current->head <= (const char *) &ofs);
auto& link = *current->links.push ();
if (current->links.in_error ())
auto& link = *current->real_links.push ();
if (current->real_links.in_error ())
err (HB_SERIALIZE_ERROR_OTHER);
link.width = sizeof (T);
@ -440,10 +453,8 @@ struct hb_serialize_context_t
assert (packed.length > 1);
for (const object_t* parent : ++hb_iter (packed))
for (const object_t::link_t &link : parent->links)
for (const object_t::link_t &link : parent->real_links)
{
if (unlikely (!link.width)) continue; // Don't need to resolve virtual offsets
const object_t* child = packed[link.objidx];
if (unlikely (!child)) { err (HB_SERIALIZE_ERROR_OTHER); return; }
unsigned offset = 0;
@ -642,6 +653,13 @@ struct hb_serialize_context_t
private:
void merge_virtual_links (const object_t* from, objidx_t to_idx) {
object_t* to = packed[to_idx];
for (const auto& l : from->virtual_links) {
to->virtual_links.push (l);
}
}
/* Object memory pool. */
hb_pool_t<object_t> object_pool;

View File

@ -763,20 +763,20 @@ populate_serializer_virtual_link (hb_serialize_context_t* c)
start_object ("b", 1, c);
add_offset (obj_d, c);
unsigned obj_b = c->pop_pack ();
unsigned obj_b = c->pop_pack (false);
start_object ("e", 1, c);
add_virtual_offset (obj_b, c);
unsigned obj_e = c->pop_pack();
unsigned obj_e = c->pop_pack (false);
start_object ("c", 1, c);
add_offset (obj_e, c);
unsigned obj_c = c->pop_pack ();
unsigned obj_c = c->pop_pack (false);
start_object ("a", 1, c);
add_offset (obj_b, c);
add_offset (obj_c, c);
c->pop_pack ();
c->pop_pack (false);
c->end_serialize();
}
@ -792,19 +792,19 @@ static void test_sort_kahn_1 ()
graph.sort_kahn ();
assert(strncmp (graph.object (3).head, "abc", 3) == 0);
assert(graph.object (3).links.length == 2);
assert(graph.object (3).links[0].objidx == 2);
assert(graph.object (3).links[1].objidx == 1);
assert(graph.object (3).real_links.length == 2);
assert(graph.object (3).real_links[0].objidx == 2);
assert(graph.object (3).real_links[1].objidx == 1);
assert(strncmp (graph.object (2).head, "def", 3) == 0);
assert(graph.object (2).links.length == 1);
assert(graph.object (2).links[0].objidx == 0);
assert(graph.object (2).real_links.length == 1);
assert(graph.object (2).real_links[0].objidx == 0);
assert(strncmp (graph.object (1).head, "jkl", 3) == 0);
assert(graph.object (1).links.length == 0);
assert(graph.object (1).real_links.length == 0);
assert(strncmp (graph.object (0).head, "ghi", 3) == 0);
assert(graph.object (0).links.length == 0);
assert(graph.object (0).real_links.length == 0);
free (buffer);
}
@ -821,24 +821,24 @@ static void test_sort_kahn_2 ()
assert(strncmp (graph.object (4).head, "abc", 3) == 0);
assert(graph.object (4).links.length == 3);
assert(graph.object (4).links[0].objidx == 3);
assert(graph.object (4).links[1].objidx == 0);
assert(graph.object (4).links[2].objidx == 2);
assert(graph.object (4).real_links.length == 3);
assert(graph.object (4).real_links[0].objidx == 3);
assert(graph.object (4).real_links[1].objidx == 0);
assert(graph.object (4).real_links[2].objidx == 2);
assert(strncmp (graph.object (3).head, "def", 3) == 0);
assert(graph.object (3).links.length == 1);
assert(graph.object (3).links[0].objidx == 1);
assert(graph.object (3).real_links.length == 1);
assert(graph.object (3).real_links[0].objidx == 1);
assert(strncmp (graph.object (2).head, "mn", 2) == 0);
assert(graph.object (2).links.length == 0);
assert(graph.object (2).real_links.length == 0);
assert(strncmp (graph.object (1).head, "ghi", 3) == 0);
assert(graph.object (1).links.length == 1);
assert(graph.object (1).links[0].objidx == 0);
assert(graph.object (1).real_links.length == 1);
assert(graph.object (1).real_links[0].objidx == 0);
assert(strncmp (graph.object (0).head, "jkl", 3) == 0);
assert(graph.object (0).links.length == 0);
assert(graph.object (0).real_links.length == 0);
free (buffer);
}
@ -854,24 +854,24 @@ static void test_sort_shortest ()
graph.sort_shortest_distance ();
assert(strncmp (graph.object (4).head, "abc", 3) == 0);
assert(graph.object (4).links.length == 3);
assert(graph.object (4).links[0].objidx == 2);
assert(graph.object (4).links[1].objidx == 0);
assert(graph.object (4).links[2].objidx == 3);
assert(graph.object (4).real_links.length == 3);
assert(graph.object (4).real_links[0].objidx == 2);
assert(graph.object (4).real_links[1].objidx == 0);
assert(graph.object (4).real_links[2].objidx == 3);
assert(strncmp (graph.object (3).head, "mn", 2) == 0);
assert(graph.object (3).links.length == 0);
assert(graph.object (3).real_links.length == 0);
assert(strncmp (graph.object (2).head, "def", 3) == 0);
assert(graph.object (2).links.length == 1);
assert(graph.object (2).links[0].objidx == 1);
assert(graph.object (2).real_links.length == 1);
assert(graph.object (2).real_links[0].objidx == 1);
assert(strncmp (graph.object (1).head, "ghi", 3) == 0);
assert(graph.object (1).links.length == 1);
assert(graph.object (1).links[0].objidx == 0);
assert(graph.object (1).real_links.length == 1);
assert(graph.object (1).real_links[0].objidx == 0);
assert(strncmp (graph.object (0).head, "jkl", 3) == 0);
assert(graph.object (0).links.length == 0);
assert(graph.object (0).real_links.length == 0);
free (buffer);
}
@ -887,27 +887,27 @@ static void test_duplicate_leaf ()
graph.duplicate (4, 1);
assert(strncmp (graph.object (5).head, "abc", 3) == 0);
assert(graph.object (5).links.length == 3);
assert(graph.object (5).links[0].objidx == 3);
assert(graph.object (5).links[1].objidx == 4);
assert(graph.object (5).links[2].objidx == 0);
assert(graph.object (5).real_links.length == 3);
assert(graph.object (5).real_links[0].objidx == 3);
assert(graph.object (5).real_links[1].objidx == 4);
assert(graph.object (5).real_links[2].objidx == 0);
assert(strncmp (graph.object (4).head, "jkl", 3) == 0);
assert(graph.object (4).links.length == 0);
assert(graph.object (4).real_links.length == 0);
assert(strncmp (graph.object (3).head, "def", 3) == 0);
assert(graph.object (3).links.length == 1);
assert(graph.object (3).links[0].objidx == 2);
assert(graph.object (3).real_links.length == 1);
assert(graph.object (3).real_links[0].objidx == 2);
assert(strncmp (graph.object (2).head, "ghi", 3) == 0);
assert(graph.object (2).links.length == 1);
assert(graph.object (2).links[0].objidx == 1);
assert(graph.object (2).real_links.length == 1);
assert(graph.object (2).real_links[0].objidx == 1);
assert(strncmp (graph.object (1).head, "jkl", 3) == 0);
assert(graph.object (1).links.length == 0);
assert(graph.object (1).real_links.length == 0);
assert(strncmp (graph.object (0).head, "mn", 2) == 0);
assert(graph.object (0).links.length == 0);
assert(graph.object (0).real_links.length == 0);
free (buffer);
}
@ -923,32 +923,32 @@ static void test_duplicate_interior ()
graph.duplicate (3, 2);
assert(strncmp (graph.object (6).head, "abc", 3) == 0);
assert(graph.object (6).links.length == 3);
assert(graph.object (6).links[0].objidx == 4);
assert(graph.object (6).links[1].objidx == 2);
assert(graph.object (6).links[2].objidx == 1);
assert(graph.object (6).real_links.length == 3);
assert(graph.object (6).real_links[0].objidx == 4);
assert(graph.object (6).real_links[1].objidx == 2);
assert(graph.object (6).real_links[2].objidx == 1);
assert(strncmp (graph.object (5).head, "jkl", 3) == 0);
assert(graph.object (5).links.length == 1);
assert(graph.object (5).links[0].objidx == 0);
assert(graph.object (5).real_links.length == 1);
assert(graph.object (5).real_links[0].objidx == 0);
assert(strncmp (graph.object (4).head, "def", 3) == 0);
assert(graph.object (4).links.length == 1);
assert(graph.object (4).links[0].objidx == 3);
assert(graph.object (4).real_links.length == 1);
assert(graph.object (4).real_links[0].objidx == 3);
assert(strncmp (graph.object (3).head, "ghi", 3) == 0);
assert(graph.object (3).links.length == 1);
assert(graph.object (3).links[0].objidx == 5);
assert(graph.object (3).real_links.length == 1);
assert(graph.object (3).real_links[0].objidx == 5);
assert(strncmp (graph.object (2).head, "jkl", 3) == 0);
assert(graph.object (2).links.length == 1);
assert(graph.object (2).links[0].objidx == 0);
assert(graph.object (2).real_links.length == 1);
assert(graph.object (2).real_links[0].objidx == 0);
assert(strncmp (graph.object (1).head, "mn", 2) == 0);
assert(graph.object (1).links.length == 0);
assert(graph.object (1).real_links.length == 0);
assert(strncmp (graph.object (0).head, "opqrst", 6) == 0);
assert(graph.object (0).links.length == 0);
assert(graph.object (0).real_links.length == 0);
free (buffer);
}
@ -1247,6 +1247,43 @@ static void test_virtual_link ()
free (out_buffer);
}
static void
test_shared_node_with_virtual_links ()
{
size_t buffer_size = 100;
void* buffer = malloc (buffer_size);
hb_serialize_context_t c (buffer, buffer_size);
c.start_serialize<char> ();
unsigned obj_b = add_object ("b", 1, &c);
unsigned obj_c = add_object ("c", 1, &c);
start_object ("d", 1, &c);
add_virtual_offset (obj_b, &c);
unsigned obj_d_1 = c.pop_pack ();
start_object ("d", 1, &c);
add_virtual_offset (obj_c, &c);
unsigned obj_d_2 = c.pop_pack ();
assert (obj_d_1 == obj_d_2);
start_object ("a", 1, &c);
add_offset (obj_b, &c);
add_offset (obj_c, &c);
add_offset (obj_d_1, &c);
add_offset (obj_d_2, &c);
c.pop_pack ();
c.end_serialize ();
assert(c.object_graph() [obj_d_1]->virtual_links.length == 2);
assert(c.object_graph() [obj_d_1]->virtual_links[0].objidx == obj_b);
assert(c.object_graph() [obj_d_1]->virtual_links[1].objidx == obj_c);
free(buffer);
}
// TODO(garretrieger): update will_overflow tests to check the overflows array.
// TODO(garretrieger): add tests for priority raising.
@ -1273,4 +1310,5 @@ main (int argc, char **argv)
test_duplicate_leaf ();
test_duplicate_interior ();
test_virtual_link ();
test_shared_node_with_virtual_links ();
}