From a733a9afa581ba2c8bac54ba5c0fe3daaddfc30c Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 00:32:47 +0000 Subject: [PATCH] [repacker] insert new subtables immediately after the subtable they split from in the lookup. --- src/graph/gsubgpos-graph.hh | 72 +++++++++++++++++++++++++++---------- src/hb-serialize.hh | 5 +++ src/test-repacker.cc | 6 ++-- 3 files changed, 60 insertions(+), 23 deletions(-) diff --git a/src/graph/gsubgpos-graph.hh b/src/graph/gsubgpos-graph.hh index afa1152c4..a42549655 100644 --- a/src/graph/gsubgpos-graph.hh +++ b/src/graph/gsubgpos-graph.hh @@ -124,7 +124,7 @@ struct Lookup : public OT::Lookup if (!is_ext && type != OT::Layout::GPOS_impl::PosLookupSubTable::Type::Pair) return true; - hb_vector_t all_new_subtables; + hb_vector_t>> all_new_subtables; for (unsigned i = 0; i < subTable.len; i++) { unsigned subtable_index = c.graph.index_for_offset (this_index, &subTable[i]); @@ -147,11 +147,14 @@ struct Lookup : public OT::Lookup hb_vector_t new_sub_tables = pairPos->split_subtables (c, subtable_index); if (new_sub_tables.in_error ()) return false; - + new_sub_tables.iter() | hb_sink (all_new_subtables); + hb_pair_t>* entry = all_new_subtables.push (); + entry->first = i; + entry->second = std::move (new_sub_tables); } - if (all_new_subtables) + if (all_new_subtables) { add_sub_tables (c, this_index, type, all_new_subtables); + } return true; } @@ -159,13 +162,18 @@ struct Lookup : public OT::Lookup void add_sub_tables (gsubgpos_graph_context_t& c, unsigned this_index, unsigned type, - hb_vector_t& subtable_indices) + hb_vector_t>>& subtable_ids) { bool is_ext = is_extension (c.table_tag); auto& v = c.graph.vertices_[this_index]; + fix_existing_subtable_links (c, this_index, subtable_ids); + + unsigned new_subtable_count = 0; + for (const auto& p : subtable_ids) + new_subtable_count += p.second.length; size_t new_size = v.table_size () - + subtable_indices.length * OT::Offset16::static_size; + + new_subtable_count * OT::Offset16::static_size; char* buffer = (char*) hb_calloc (1, new_size); c.add_buffer (buffer); memcpy (buffer, v.obj.head, v.table_size()); @@ -175,30 +183,56 @@ struct Lookup : public OT::Lookup Lookup* new_lookup = (Lookup*) buffer; - new_lookup->subTable.len = subTable.len + subtable_indices.length; - unsigned offset_index = subTable.len; - for (unsigned subtable_id : subtable_indices) + new_lookup->subTable.len = subTable.len + new_subtable_count; + for (const auto& p : subtable_ids) { - if (is_ext) + unsigned offset_index = p.first + 1; + for (unsigned subtable_id : p.second) { - unsigned ext_id = create_extension_subtable (c, subtable_id, type); - c.graph.vertices_[subtable_id].parents.push (ext_id); - subtable_id = ext_id; + if (is_ext) + { + unsigned ext_id = create_extension_subtable (c, subtable_id, type); + c.graph.vertices_[subtable_id].parents.push (ext_id); + subtable_id = ext_id; + } + + auto* link = v.obj.real_links.push (); + link->width = 2; + link->objidx = subtable_id; + link->position = (char*) &new_lookup->subTable[offset_index++] - + (char*) new_lookup; + c.graph.vertices_[subtable_id].parents.push (this_index); } - - auto* link = v.obj.real_links.push (); - link->width = 2; - link->objidx = subtable_id; - link->position = (char*) &new_lookup->subTable[offset_index++] - - (char*) new_lookup; - c.graph.vertices_[subtable_id].parents.push (this_index); } + // Repacker sort order depends on link order, which we've messed up so resort it. + v.obj.real_links.qsort (); + // The head location of the lookup has changed, invalidating the lookups map entry // in the context. Update the map. c.lookups.set (this_index, new_lookup); } + void fix_existing_subtable_links (gsubgpos_graph_context_t& c, + unsigned this_index, + hb_vector_t>>& subtable_ids) + { + auto& v = c.graph.vertices_[this_index]; + Lookup* lookup = (Lookup*) v.obj.head; + + for (const auto& p : subtable_ids) + { + unsigned insert_index = p.first; + unsigned pos_offset = p.second.length * OT::Offset16::static_size; + unsigned insert_offset = (char*) &lookup->subTable[insert_index] - (char*) lookup; + + for (auto& l : v.obj.all_links_writer ()) + { + if (l.position > insert_offset) l.position += pos_offset; + } + } + } + unsigned create_extension_subtable (gsubgpos_graph_context_t& c, unsigned subtable_index, unsigned type) diff --git a/src/hb-serialize.hh b/src/hb-serialize.hh index cecdcdeb7..f42257407 100644 --- a/src/hb-serialize.hh +++ b/src/hb-serialize.hh @@ -139,6 +139,11 @@ struct hb_serialize_context_t objidx = o.objidx; } #endif + + HB_INTERNAL static int cmp (const void* a, const void* b) + { + return ((const link_t*)a)->position - ((const link_t*)b)->position; + } }; char *head; diff --git a/src/test-repacker.cc b/src/test-repacker.cc index ae2a05a90..30fe44a4e 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -1183,18 +1183,16 @@ populate_serializer_with_large_pair_pos_1 (hb_serialize_context_t* c, unsigned pair_pos_2 = add_object (large_string.c_str(), 200, c); if (as_extension) { - + pair_pos_2 = add_extension (pair_pos_2, 2, c); for (int i = num_pair_pos_1 - 1; i >= 0; i--) pair_pos_1[i] = add_extension (pair_pos_1[i], 2, c); - pair_pos_2 = add_extension (pair_pos_2, 2, c); } start_lookup (as_extension ? 9 : 2, 1 + num_pair_pos_1, c); - add_offset (pair_pos_2, c); for (int i = 0; i < num_pair_pos_1; i++) add_offset (pair_pos_1[i], c); - + add_offset (pair_pos_2, c); unsigned lookup = finish_lookup (c);