diff --git a/ift/encoder/activation_condition_test.cc b/ift/encoder/activation_condition_test.cc index 9659768c..54a61eb9 100644 --- a/ift/encoder/activation_condition_test.cc +++ b/ift/encoder/activation_condition_test.cc @@ -34,35 +34,35 @@ TEST(ActivationConditionTest, ActivationConditionsToEncoderConditions) { }; // Entry generation optimizes the entries, by combining segments - // where possible. + // where possible. Entry order tries to match input order where possible std::vector expected; - // Entry 0: s1 OR s3 -> 5 + // Entry 0: s2 -> 2 { - PatchMap::Entry entry({'a', 'b', 'd', 'e', 'f'}, 5, - PatchEncoding::GLYPH_KEYED); + PatchMap::Entry entry({'c'}, 2, PatchEncoding::GLYPH_KEYED); expected.push_back(entry); } - // Entry 1: (s1 OR s3) AND (s2 OR s4) -> 6 + // Entry 1: s3 -> 4 { - PatchMap::Entry entry; - entry.coverage.codepoints = {'c', 'g'}; - entry.coverage.child_indices = {0}; - entry.coverage.conjunctive = true; - entry.patch_indices = {6}; + PatchMap::Entry entry({'d', 'e', 'f'}, 4, PatchEncoding::GLYPH_KEYED); expected.push_back(entry); } - // Entry 2: s3 -> 4 + // Entry 2: s1 OR s3 -> 5 { - PatchMap::Entry entry({'d', 'e', 'f'}, 4, PatchEncoding::GLYPH_KEYED); + PatchMap::Entry entry({'a', 'b', 'd', 'e', 'f'}, 5, + PatchEncoding::GLYPH_KEYED); expected.push_back(entry); } - // Entry 3: s2 -> 2 + // Entry 3: (s1 OR s3) AND (s2 OR s4) -> 6 { - PatchMap::Entry entry({'c'}, 2, PatchEncoding::GLYPH_KEYED); + PatchMap::Entry entry; + entry.coverage.codepoints = {'c', 'g'}; + entry.coverage.child_indices = {2}; + entry.coverage.conjunctive = true; + entry.patch_indices = {6}; expected.push_back(entry); } @@ -98,33 +98,33 @@ TEST(ActivationConditionTest, std::vector expected; - // Entry 0: s1 OR s3 -> 5 + // Entry 0: s2 -> 2 { - PatchMap::Entry entry({'a', 'b', 'd', 'e', 'f'}, 5, - PatchEncoding::GLYPH_KEYED); + PatchMap::Entry entry({'c'}, 2, PatchEncoding::GLYPH_KEYED); expected.push_back(entry); } - // Entry 1: (s1 OR s3) AND (s2 OR s4) -> 6 + // Entry 1: s3 -> 4 { - PatchMap::Entry entry; - entry.coverage.codepoints = {'c', 'g'}; - entry.coverage.child_indices = {0}; - entry.coverage.conjunctive = true; - entry.patch_indices = {6}; + PatchMap::Entry entry({'d', 'e', 'f'}, {4, 13, 12}, + proto::PatchEncoding::TABLE_KEYED_FULL); expected.push_back(entry); } - // Entry 2: s3 -> 4 + // Entry 2: s1 OR s3 -> 5 { - PatchMap::Entry entry({'d', 'e', 'f'}, {4, 13, 12}, - proto::PatchEncoding::TABLE_KEYED_FULL); + PatchMap::Entry entry({'a', 'b', 'd', 'e', 'f'}, 5, + PatchEncoding::GLYPH_KEYED); expected.push_back(entry); } - // Entry 3: s2 -> 2 + // Entry 1: (s1 OR s3) AND (s2 OR s4) -> 6 { - PatchMap::Entry entry({'c'}, 2, PatchEncoding::GLYPH_KEYED); + PatchMap::Entry entry; + entry.coverage.codepoints = {'c', 'g'}; + entry.coverage.child_indices = {2}; + entry.coverage.conjunctive = true; + entry.patch_indices = {6}; expected.push_back(entry); } @@ -153,20 +153,22 @@ TEST(ActivationConditionTest, std::vector expected; - // Entry 0: dlig (ignored) + + + // Entry 0: {d, e, f} (ignored) { PatchMap::Entry condition; - condition.coverage.features = {HB_TAG('d', 'l', 'i', 'g')}; + condition.coverage.codepoints = {'d', 'e', 'f'}; condition.ignored = true; condition.patch_indices = {1}; condition.encoding = PatchEncoding::GLYPH_KEYED; expected.push_back(condition); } - // Entry 1: {d, e, f} (ignored) + // Entry 1: dlig (ignored) { PatchMap::Entry condition; - condition.coverage.codepoints = {'d', 'e', 'f'}; + condition.coverage.features = {HB_TAG('d', 'l', 'i', 'g')}; condition.ignored = true; condition.patch_indices = {2}; condition.encoding = PatchEncoding::GLYPH_KEYED; diff --git a/ift/encoder/compiler.cc b/ift/encoder/compiler.cc index 765b1901..cbff3087 100644 --- a/ift/encoder/compiler.cc +++ b/ift/encoder/compiler.cc @@ -1,6 +1,7 @@ #include "ift/encoder/compiler.h" #include +#include #include #include #include @@ -238,13 +239,10 @@ StatusOr Compiler::Compile() const { // This will optimize for cases that don't include the entire original // font. context.force_long_loca_and_gvar_ = false; - auto expanded = FullyExpandedSubset(context); - if (!expanded.ok()) { - return expanded.status(); - } + auto expanded = TRY(FullyExpandedSubset(context)); - context.fully_expanded_subset_.shallow_copy(*expanded); - auto expanded_face = expanded->face(); + context.fully_expanded_subset_.shallow_copy(expanded); + auto expanded_face = expanded.face(); // TODO(garretrieger): we don't need to force long gvar anymore. The client is // now capable of upgrading the offset size as needed. Forcing long loca @@ -253,10 +251,7 @@ StatusOr Compiler::Compile() const { TRY(FontHelper::HasLongLoca(expanded_face.get())) || FontHelper::HasWideGvar(expanded_face.get()); - auto init_font = Compile(context, context.init_subset_, true); - if (!init_font.ok()) { - return init_font.status(); - } + auto init_compile_result = TRY(Compile(context, context.init_subset_, true)); Encoding result; @@ -264,14 +259,14 @@ StatusOr Compiler::Compile() const { // Glyph transforms in woff2 encoding aren't safe if we are patching glyf // with a table keyed patch otherwise they are safe to use. See: // https://w3c.github.io/IFT/Overview.html#ift-and-compression - hb_face_unique_ptr face = init_font->face(); + hb_face_unique_ptr face = init_compile_result.font_data.face(); auto tags = FontHelper::GetTags(face.get()); bool has_glyf = tags.contains(FontHelper::kGlyf) || tags.contains(FontHelper::kLoca); - result.init_font = - TRY(Woff2::EncodeWoff2(init_font->str(), IsMixedMode() || !has_glyf)); + result.init_font = TRY(Woff2::EncodeWoff2( + init_compile_result.font_data.str(), IsMixedMode() || !has_glyf)); } else { - result.init_font.shallow_copy(*init_font); + result.init_font.shallow_copy(init_compile_result.font_data); } result.patches = std::move(context.patches_); return result; @@ -327,11 +322,8 @@ Status Compiler::EnsureGlyphKeyedPatchesPopulated( if (!design_space.empty()) { // If a design space is provided, apply it. - auto result = Instance(context, full_face.get(), design_space); - if (!result.ok()) { - return result.status(); - } - instance.shallow_copy(*result); + auto result = TRY(Instance(context, full_face.get(), design_space)); + instance.shallow_copy(result); } GlyphKeyedDiff differ(instance, compat_id, @@ -348,12 +340,8 @@ Status Compiler::EnsureGlyphKeyedPatchesPopulated( std::string url = TRY(URLTemplate::PatchToUrl(url_template, index)); const auto& gids = e->second; - auto patch = differ.CreatePatch(gids); - if (!patch.ok()) { - return patch.status(); - } - - context.patches_[url].shallow_copy(*patch); + auto patch = TRY(differ.CreatePatch(gids)); + context.patches_[url].shallow_copy(patch); } return absl::OkStatus(); @@ -434,32 +422,112 @@ std::vector Compiler::EdgesToActivationConditions( return result; } +StatusOr>> Compiler::EstimateEdgeSizes( + ProcessingContext& context, const SubsetDefinition& base_subset, + const FontData& base_font_data, const CompatId& table_keyed_compat_id, + const std::vector& glyph_keyed_url_template, + absl::Span edges) const { + std::vector> sizes; + sizes.reserve(edges.size()); + + size_t index = 0; + for (const auto& edge : edges) { + uint64_t total_estimated_size = 0; + + SubsetDefinition current_subset = base_subset; + CompileResult current{ + .table_keyed_compat_id = table_keyed_compat_id, + .glyph_keyed_url_template = glyph_keyed_url_template, + }; + current.font_data.shallow_copy(base_font_data); + + for (const auto& j : edge.Jumps(base_subset, this->use_prefetch_lists_)) { + if (j.start != current_subset) { + return absl::InternalError( + "PopulateTableKeyedPatchMap: Base mismatch with the current jump."); + } + + auto next_res = TRY(Compile(context, j.end, false)); + + uint64_t patch_size = 0; + auto cache_it = context.estimated_patch_sizes_.find(j); + if (cache_it != context.estimated_patch_sizes_.end()) { + patch_size = cache_it->second; + } else { + bool replace_url_template = + IsMixedMode() && (next_res.glyph_keyed_url_template != + current.glyph_keyed_url_template); + + auto tentative_differ = TRY(GetTentativeDifferFor( + current.table_keyed_compat_id, replace_url_template)); + + FontData tentative_patch; + TRYV(tentative_differ->Diff(current.font_data, next_res.font_data, + &tentative_patch)); + patch_size = tentative_patch.size(); + context.estimated_patch_sizes_[j] = patch_size; + } + + total_estimated_size += patch_size; + + current = std::move(next_res); + current_subset = j.end; + } + + sizes.push_back(std::make_pair(index++, total_estimated_size)); + } + + return sizes; +} + Status Compiler::PopulateTableKeyedPatchMap( ProcessingContext& context, const SubsetDefinition& node_subset, + const FontData& base_font_data, const CompatId& table_keyed_compat_id, + const std::vector& glyph_keyed_url_template, const std::vector& edges, PatchEncoding encoding, PatchMap& table_keyed_patch_map) const { // To create the table keyed patch mappings we use the activation condition // compiler. The outgoing edges for this node are converted into an activation // condition list and then compiled into mapping entries. + + // The invalidating selection criteria breaks ties to entries that occur + // earlier in the patch map. To optimize client behaviour we want to ensure + // that patch map entries are ordered from smallest to largest number of + // loaded bytes. So ties will break towards the smaller patch set. + std::vector> edge_sizes = TRY(EstimateEdgeSizes( + context, node_subset, base_font_data, table_keyed_compat_id, + glyph_keyed_url_template, edges)); + + std::stable_sort( + edge_sizes.begin(), edge_sizes.end(), + [](const std::pair& a, + const std::pair& b) { return a.second < b.second; }); + std::vector sorted_edges; + sorted_edges.reserve(edge_sizes.size()); + for (const auto& [edge_index, _] : edge_sizes) { + sorted_edges.push_back(edges.at(edge_index)); + } + flat_hash_map segments; - auto conditions = EdgesToActivationConditions(context, node_subset, edges, - encoding, segments); + auto final_conditions = EdgesToActivationConditions( + context, node_subset, sorted_edges, encoding, segments); + auto entries = TRY(ActivationCondition::ActivationConditionsToPatchMapEntries( - conditions, segments)); + final_conditions, segments)); for (auto e : entries) { TRYV(table_keyed_patch_map.AddEntry(e)); } return absl::OkStatus(); } -StatusOr Compiler::Compile(ProcessingContext& context, - const SubsetDefinition& node_subset, - bool is_root) const { +StatusOr Compiler::Compile( + ProcessingContext& context, const SubsetDefinition& node_subset, + bool is_root) const { // See ../../docs/experimental/compiler.md for a detailed discussion of // how this implementation works. auto it = context.built_subsets_.find(node_subset); if (it != context.built_subsets_.end()) { - FontData copy; + CompileResult copy; copy.shallow_copy(it->second); return copy; } @@ -482,8 +550,12 @@ StatusOr Compiler::Compile(ProcessingContext& context, if (edges.empty() && !IsMixedMode()) { // This is a leaf node, a IFT table isn't needed. - context.built_subsets_[node_subset].shallow_copy(node_data); - return node_data; + CompileResult result = {.font_data = std::move(node_data), + .table_keyed_compat_id = {}, + .glyph_keyed_compat_id = {}, + .glyph_keyed_url_template = {}}; + context.built_subsets_[node_subset].shallow_copy(result); + return std::move(result); } IFTTable table_keyed; @@ -499,8 +571,9 @@ StatusOr Compiler::Compile(ProcessingContext& context, PatchMap& table_keyed_patch_map = table_keyed.GetPatchMap(); PatchEncoding encoding = IsMixedMode() ? TABLE_KEYED_PARTIAL : TABLE_KEYED_FULL; - TRYV(PopulateTableKeyedPatchMap(context, node_subset, edges, encoding, - table_keyed_patch_map)); + TRYV(PopulateTableKeyedPatchMap( + context, node_subset, node_data, table_keyed_compat_id, + glyph_keyed_url_template, edges, encoding, table_keyed_patch_map)); auto face = node_data.face(); std::optional ext = @@ -515,72 +588,109 @@ StatusOr Compiler::Compile(ProcessingContext& context, node_data.shallow_copy(new_node_data); } - context.built_subsets_[node_subset].shallow_copy(node_data); + CompileResult result = {.font_data = std::move(node_data), + .table_keyed_compat_id = table_keyed_compat_id, + .glyph_keyed_compat_id = glyph_keyed_compat_id, + .glyph_keyed_url_template = glyph_keyed_url_template}; + context.built_subsets_[node_subset].shallow_copy(result); for (const auto& edge : edges) { SubsetDefinition current_node_subset = node_subset; FontData current_node_data; - current_node_data.shallow_copy(node_data); + current_node_data.shallow_copy(result.font_data); + CompatId current_table_keyed_compat_id = table_keyed_compat_id; + std::vector current_glyph_keyed_url_template = + glyph_keyed_url_template; for (const auto& j : edge.Jumps(node_subset, use_prefetch_lists_)) { uint32_t id = context.table_keyed_patch_id_map_[j]; if (j.start != current_node_subset) { - return absl::InternalError("Base mismatch with the current jump."); + return absl::InternalError( + "Compile: Base mismatch with the current jump."); } auto next = TRY(Compile(context, j.end, false)); if (context.built_table_keyed_patches_.contains(id)) { current_node_subset = j.end; - current_node_data = std::move(next); + current_node_data = std::move(next.font_data); + current_table_keyed_compat_id = next.table_keyed_compat_id; + current_glyph_keyed_url_template = next.glyph_keyed_url_template; continue; } - // Check if the main table URL will change with this subset - std::vector next_glyph_keyed_url_template; - CompatId next_glyph_keyed_compat_id; - TRYV(EnsureGlyphKeyedPatchesPopulated(context, j.end.design_space, - next_glyph_keyed_url_template, - next_glyph_keyed_compat_id)); - bool replace_url_template = IsMixedMode() && - (next_glyph_keyed_url_template != glyph_keyed_url_template); + (next.glyph_keyed_url_template != current_glyph_keyed_url_template); FontData patch; - auto differ = - TRY(GetDifferFor(next, table_keyed_compat_id, replace_url_template)); + auto differ = TRY( + GetDifferFor(current_table_keyed_compat_id, replace_url_template)); - TRYV((*differ).Diff(current_node_data, next, &patch)); + TRYV((*differ).Diff(current_node_data, next.font_data, &patch)); std::string url = TRY(URLTemplate::PatchToUrl(table_keyed_url_template, id)); context.patches_[url].shallow_copy(patch); context.built_table_keyed_patches_.insert(id); - current_node_data = std::move(next); + current_node_data = std::move(next.font_data); + current_table_keyed_compat_id = next.table_keyed_compat_id; + current_glyph_keyed_url_template = next.glyph_keyed_url_template; current_node_subset = j.end; } } - return node_data; + return std::move(result); } -StatusOr> Compiler::GetDifferFor( - const FontData& font_data, CompatId compat_id, - bool replace_url_template) const { +TableKeyedDiff* Compiler::GetTableKeyedDifferFor(CompatId compat_id, + bool replace_url_template, + bool exclude_ift) const { if (!IsMixedMode()) { - return std::unique_ptr( - Compiler::FullFontTableKeyedDiff(compat_id)); + // If only table keyed patches are used we can diff the whole font. + if (exclude_ift) { + return new TableKeyedDiff(compat_id, {"IFT ", "IFTX"}); + } else { + return new TableKeyedDiff(compat_id); + } } if (replace_url_template) { - return std::unique_ptr( - Compiler::ReplaceIftMapTableKeyedDiff(compat_id)); + // Glyph keyed patches are in use, but we are changing design space. So + // tables with variations will need to be replaced (gvar/CFF2), all other + // glyph data tables are excluded. + if (exclude_ift) { + return new TableKeyedDiff(compat_id, + {"IFT ", "IFTX", "glyf", "loca", "CFF "}, + {"gvar", "CFF2"}); + } else { + return new TableKeyedDiff(compat_id, {"glyf", "loca", "CFF "}, + {"IFTX", "gvar", "CFF2"}); + } + } + + // Glyph keyed patches are in use, all glyph data tables are excluded as these + // are handled by glyph keyed patches + if (exclude_ift) { + return new TableKeyedDiff( + compat_id, {"IFT ", "IFTX", "glyf", "loca", "gvar", "CFF ", "CFF2"}); + } else { + return new TableKeyedDiff(compat_id, + {"IFTX", "glyf", "loca", "gvar", "CFF ", "CFF2"}); } +} - return std::unique_ptr( - Compiler::MixedModeTableKeyedDiff(compat_id)); +StatusOr> Compiler::GetTentativeDifferFor( + CompatId compat_id, bool replace_url_template) const { + return std::unique_ptr(GetTableKeyedDifferFor( + compat_id, replace_url_template, /*exclude_ift=*/true)); +} + +StatusOr> Compiler::GetDifferFor( + CompatId compat_id, bool replace_url_template) const { + return std::unique_ptr(GetTableKeyedDifferFor( + compat_id, replace_url_template, /*exclude_ift=*/false)); } StatusOr Compiler::CreateSubsetPlan( @@ -639,10 +749,7 @@ StatusOr Compiler::GenerateBaseGvar( // not modify shared tuples. // Step 1: Instancing - auto instance = Instance(context, font, design_space); - if (!instance.ok()) { - return instance.status(); - } + auto instance = TRY(Instance(context, font, design_space)); // Step 2: glyph subsetting SubsetDefinition subset = context.init_subset_; @@ -650,16 +757,13 @@ StatusOr Compiler::GenerateBaseGvar( // so clear out the design space. subset.design_space = {}; - hb_face_unique_ptr instanced_face = instance->face(); + hb_face_unique_ptr instanced_face = instance.face(); auto face_builder = - CutSubsetFaceBuilder(context, instanced_face.get(), subset); - if (!face_builder.ok()) { - return face_builder.status(); - } + TRY(CutSubsetFaceBuilder(context, instanced_face.get(), subset)); // Step 3: extract gvar table. hb_blob_unique_ptr gvar_blob = make_hb_blob( - hb_face_reference_table(face_builder->get(), FontHelper::kGvar)); + hb_face_reference_table(face_builder.get(), FontHelper::kGvar)); FontData result(gvar_blob.get()); return result; } @@ -745,11 +849,8 @@ StatusOr Compiler::GenerateBaseCff2( // our own. // Step 1: Instancing - auto instance = Instance(context, font, design_space); - if (!instance.ok()) { - return instance.status(); - } - auto instance_face = instance->face(); + auto instance = TRY(Instance(context, font, design_space)); + auto instance_face = instance.face(); // Step 2: find the glyph closure for the base subset. SubsetDefinition subset = context.init_subset_; @@ -830,10 +931,7 @@ StatusOr Compiler::CutSubset(const ProcessingContext& context, hb_face_t* font, const SubsetDefinition& def, bool generate_glyph_keyed_bases) const { - auto result = CutSubsetFaceBuilder(context, font, def); - if (!result.ok()) { - return result.status(); - } + auto result = TRY(CutSubsetFaceBuilder(context, font, def)); auto tags = FontHelper::GetTags(font); if (generate_glyph_keyed_bases && TRY(def.IsVariableFor(font)) && @@ -848,8 +946,7 @@ StatusOr Compiler::CutSubset(const ProcessingContext& context, // handle including a replacement gvar patch when needed. auto base_gvar = TRY(GenerateBaseGvar(context, font, def.design_space)); hb_blob_unique_ptr gvar_blob = base_gvar.blob(); - hb_face_builder_add_table(result->get(), FontHelper::kGvar, - gvar_blob.get()); + hb_face_builder_add_table(result.get(), FontHelper::kGvar, gvar_blob.get()); } if (generate_glyph_keyed_bases && tags.contains(FontHelper::kCFF2)) { @@ -859,11 +956,10 @@ StatusOr Compiler::CutSubset(const ProcessingContext& context, // variation data will match whatever the glyph keyed patches were cut from. auto base_cff2 = TRY(GenerateBaseCff2(context, font, def.design_space)); hb_blob_unique_ptr cff2_blob = base_cff2.blob(); - hb_face_builder_add_table(result->get(), FontHelper::kCFF2, - cff2_blob.get()); + hb_face_builder_add_table(result.get(), FontHelper::kCFF2, cff2_blob.get()); } - hb_blob_unique_ptr blob = make_hb_blob(hb_face_reference_blob(result->get())); + hb_blob_unique_ptr blob = make_hb_blob(hb_face_reference_blob(result.get())); FontData subset(blob.get()); return subset; @@ -898,12 +994,8 @@ StatusOr Compiler::Instance( StatusOr Compiler::RoundTripWoff2(string_view font, bool glyf_transform) { - auto r = Woff2::EncodeWoff2(font, glyf_transform); - if (!r.ok()) { - return r.status(); - } - - return Woff2::DecodeWoff2(r->str()); + auto r = TRY(Woff2::EncodeWoff2(font, glyf_transform)); + return Woff2::DecodeWoff2(r.str()); } CompatId Compiler::ProcessingContext::GenerateCompatId() { diff --git a/ift/encoder/compiler.h b/ift/encoder/compiler.h index 7ad50056..6f02ce54 100644 --- a/ift/encoder/compiler.h +++ b/ift/encoder/compiler.h @@ -145,7 +145,6 @@ class Compiler { static absl::StatusOr RoundTripWoff2( absl::string_view font, bool glyf_transform = true); - public: absl::Status SetInitSubsetFromDef(const SubsetDefinition& init_subset) { if (!init_subset_.Empty()) { return absl::FailedPreconditionError("Base subset has already been set."); @@ -239,6 +238,19 @@ class Compiler { private: struct ProcessingContext; + struct CompileResult { + ift::common::FontData font_data; + ift::common::CompatId table_keyed_compat_id; + ift::common::CompatId glyph_keyed_compat_id; + std::vector glyph_keyed_url_template; + + void shallow_copy(const CompileResult& other) { + font_data.shallow_copy(other.font_data); + table_keyed_compat_id = other.table_keyed_compat_id; + glyph_keyed_compat_id = other.glyph_keyed_compat_id; + glyph_keyed_url_template = other.glyph_keyed_url_template; + } + }; // Returns the font subset which would be reach if all segments where added to // the font. @@ -280,9 +292,9 @@ class Compiler { * Returns: the IFT encoded initial font. Patches() will be populated with the * set of associated patch files. */ - absl::StatusOr Compile( - ProcessingContext& context, const SubsetDefinition& base_subset, - bool is_root = true) const; + absl::StatusOr Compile(ProcessingContext& context, + const SubsetDefinition& base_subset, + bool is_root = true) const; /* * Returns true if this encoding will contain both glyph keyed and table keyed @@ -303,8 +315,18 @@ class Compiler { absl::Span edges, proto::PatchEncoding encoding, absl::flat_hash_map& segments) const; + absl::StatusOr>> EstimateEdgeSizes( + ProcessingContext& context, const SubsetDefinition& base_subset, + const ift::common::FontData& base_font_data, + const ift::common::CompatId& table_keyed_compat_id, + const std::vector& glyph_keyed_url_template, + absl::Span edges) const; + absl::Status PopulateTableKeyedPatchMap( ProcessingContext& context, const SubsetDefinition& base_subset, + const ift::common::FontData& base_font_data, + const ift::common::CompatId& table_keyed_compat_id, + const std::vector& glyph_keyed_url_template, const std::vector& edges, proto::PatchEncoding encoding, proto::PatchMap& table_keyed_patch_map) const; @@ -335,30 +357,16 @@ class Compiler { const ProcessingContext& context, hb_face_t* font, const design_space_t& design_space) const; - absl::StatusOr> GetDifferFor( - const ift::common::FontData& font_data, ift::common::CompatId compat_id, - bool replace_url_template) const; - - static ift::TableKeyedDiff* FullFontTableKeyedDiff( - ift::common::CompatId base_compat_id) { - return new TableKeyedDiff(base_compat_id); - } + absl::StatusOr> + GetTentativeDifferFor(ift::common::CompatId compat_id, + bool replace_url_template) const; - static ift::TableKeyedDiff* MixedModeTableKeyedDiff( - ift::common::CompatId base_compat_id) { - return new TableKeyedDiff(base_compat_id, - {"IFTX", "glyf", "loca", "gvar", "CFF ", "CFF2"}); - } + absl::StatusOr> GetDifferFor( + ift::common::CompatId compat_id, bool replace_url_template) const; - static ift::TableKeyedDiff* ReplaceIftMapTableKeyedDiff( - ift::common::CompatId base_compat_id) { - // the replacement differ is used during design space expansions, both - // gvar and "IFT " are overwritten to be compatible with the new design - // space. Glyph segment patches for all prev loaded glyphs will be - // downloaded to repopulate variation data for any already loaded glyphs. - return new TableKeyedDiff(base_compat_id, {"glyf", "loca", "CFF "}, - {"IFTX", "gvar", "CFF2"}); - } + ift::TableKeyedDiff* GetTableKeyedDifferFor(ift::common::CompatId compat_id, + bool replace_url_template, + bool exclude_ift) const; bool AllocatePatchSet(ProcessingContext& context, const design_space_t& design_space, @@ -397,9 +405,10 @@ class Compiler { absl::flat_hash_map glyph_keyed_compat_ids_; - absl::flat_hash_map built_subsets_; + absl::flat_hash_map built_subsets_; absl::flat_hash_map patches_; absl::flat_hash_map table_keyed_patch_id_map_; + absl::flat_hash_map estimated_patch_sizes_; ift::common::IntSet built_table_keyed_patches_; SubsetDefinition init_subset_; diff --git a/ift/encoder/compiler_test.cc b/ift/encoder/compiler_test.cc index 7b8af440..214756d6 100644 --- a/ift/encoder/compiler_test.cc +++ b/ift/encoder/compiler_test.cc @@ -629,24 +629,28 @@ TEST_F(CompilerTest, Encode_ThreeSubsets_Mixed_VF) { ASSERT_TRUE(sc.ok()) << sc; graph expected{ - {"", {"ABCDEFGH|08.ift_tk", "|wght[200..700]|0C.ift_tk"}}, - {"ABCDEFGH", {"ABCDEFGH|wght[200..700]|0G.ift_tk"}}, + {"", {"ABCDEFGH|0G.ift_tk", "|wght[200..700]|0K.ift_tk"}}, + {"ABCDEFGH", {"ABCDEFGH|wght[200..700]|08.ift_tk"}}, {"ABCDEFGH|wght[200..700]", {}}, - {"|wght[200..700]", {"ABCDEFGH|wght[200..700]|0K.ift_tk"}}, + {"|wght[200..700]", {"ABCDEFGH|wght[200..700]|0C.ift_tk"}}, }; ASSERT_EQ(g, expected); // Patches that don't modify variation space should not modify gvar: - auto has_gvar = PatchHasGvar(encoding->patches, "08.ift_tk"); + auto has_gvar = PatchHasGvar(encoding->patches, "0G.ift_tk"); ASSERT_TRUE(has_gvar.ok()) << has_gvar.status(); ASSERT_FALSE(*has_gvar); - has_gvar = PatchHasGvar(encoding->patches, "0K.ift_tk"); + has_gvar = PatchHasGvar(encoding->patches, "0C.ift_tk"); ASSERT_TRUE(has_gvar.ok()) << has_gvar.status(); ASSERT_FALSE(*has_gvar); // Patches that modify variation space should replace gvar: - has_gvar = PatchHasGvar(encoding->patches, "0G.ift_tk"); + has_gvar = PatchHasGvar(encoding->patches, "08.ift_tk"); + ASSERT_TRUE(has_gvar.ok()) << has_gvar.status(); + ASSERT_TRUE(*has_gvar); + + has_gvar = PatchHasGvar(encoding->patches, "0K.ift_tk"); ASSERT_TRUE(has_gvar.ok()) << has_gvar.status(); ASSERT_TRUE(*has_gvar); } @@ -1028,4 +1032,38 @@ TEST_F(CompilerTest, UrlTemplateOverride) { ASSERT_TRUE(ift_table.str().find(expected_template) != std::string::npos); } +TEST_F(CompilerTest, PopulateTableKeyedPatchMap_OrdersByPatchSize) { + Compiler compiler; + auto face = noto_sans_jp.face(); + compiler.SetFace(face.get()); + + auto s = compiler.SetInitSubset(IntSet{'a'}); + ASSERT_TRUE(s.ok()) << s; + + // Codepoint 'b' will be shared between both segments. + IntSet small_segment = {'b'}; + IntSet large_segment = segment_4_cps; + large_segment.insert('b'); + + // Add the large segment first, and the small segment second. + compiler.AddNonGlyphDataSegment(large_segment); + compiler.AddNonGlyphDataSegment(small_segment); + + auto encoding = compiler.Compile(); + ASSERT_TRUE(encoding.ok()) << encoding.status(); + + // Request codepoint 'b', which matches both segments. + btree_set applied_uris; + auto extended = ift::client::ExtendWithDesignSpace(*encoding, IntSet{'b'}, {}, + {}, &applied_uris); + ASSERT_TRUE(extended.ok()) << extended.status(); + + // Since small_segment produces a smaller patch its entry is sorted first in + // the PatchMap. The client will match it first and fetch its patch + // ("04.ift_tk") instead of large_segment's patch ("03.ift_tk"). + EXPECT_EQ(applied_uris.size(), 1); + EXPECT_TRUE(applied_uris.contains("04.ift_tk")); + EXPECT_FALSE(applied_uris.contains("03.ift_tk")); +} + } // namespace ift::encoder diff --git a/ift/encoder/entry_graph.cc b/ift/encoder/entry_graph.cc index 28fb9148..57892913 100644 --- a/ift/encoder/entry_graph.cc +++ b/ift/encoder/entry_graph.cc @@ -2,6 +2,7 @@ #include #include +#include #include "absl/strings/str_cat.h" #include "ift/common/axis_range.h" @@ -38,8 +39,9 @@ StatusOr EntryGraph::Create( Span conditions, const flat_hash_map& segments) { EntryGraph graph; - for (const auto& condition : conditions) { - uint32_t _ = TRY(graph.AddMapping(condition, segments)); + for (uint32_t i = 0; i < conditions.size(); ++i) { + const auto& condition = conditions[i]; + uint32_t _ = TRY(graph.AddMapping(condition, segments, i, true)); ActivationCondition normalized = Normalize(condition); std::vector patches = {condition.activated()}; @@ -142,27 +144,41 @@ StatusOr> EntryGraph::TopologicalSort() const { edge_count[it->second]--; } - std::vector sorted; - sorted.reserve(nodes.size()); + struct NodeComparator { + const std::vector& nodes; + bool operator()(uint32_t a, uint32_t b) const { + const auto& node_a = nodes[a], node_b = nodes[b]; + if (node_a.condition_index == node_b.condition_index) { + return a < b; + } + return node_a.condition_index < node_b.condition_index; + } + }; + + std::priority_queue, NodeComparator> queue(NodeComparator{nodes}); + for (uint32_t i = 0; i < edge_count.size(); i++) { if (edge_count[i] == 0) { - sorted.push_back(i); + queue.push(i); } } - std::sort(sorted.begin(), sorted.end()); + std::vector sorted; + sorted.reserve(nodes.size()); // Kahn's algorithm (ref: https://en.wikipedia.org/wiki/Topological_sorting) - size_t head = 0; - while (head < sorted.size()) { - uint32_t id = sorted[head++]; + while (!queue.empty()) { + uint32_t id = queue.top(); + queue.pop(); + sorted.push_back(id); + for (uint32_t child : nodes[id].children_ids) { if (edge_count[child] == 0) { return absl::InternalError("Edge count underflow."); } edge_count[child]--; if (edge_count[child] == 0) { - sorted.push_back(child); + queue.push(child); } } } @@ -176,11 +192,18 @@ StatusOr> EntryGraph::TopologicalSort() const { StatusOr EntryGraph::AddMapping( const ActivationCondition& condition, - const flat_hash_map& segments) { + const flat_hash_map& segments, + uint32_t condition_index, + bool root + ) { ActivationCondition normalized = Normalize(condition); auto existing = condition_to_node_id.find(normalized); if (existing != condition_to_node_id.end()) { + // Top level condition indices take precedent. + if (root) { + nodes[existing->second].condition_index = condition_index; + } incoming_edge_count[existing->second]++; return existing->second; } @@ -189,23 +212,26 @@ StatusOr EntryGraph::AddMapping( const auto& segment = segments.at(*normalized.TriggeringSegments().begin()); if (!segment.codepoints.empty() && segment.feature_tags.empty() && segment.design_space.empty()) { - uint32_t node_id = TRY(AddCodepointsOnly(segment.codepoints)); + uint32_t node_id = + TRY(AddCodepointsOnly(segment.codepoints, condition_index)); condition_to_node_id[normalized] = node_id; return node_id; } else if (segment.codepoints.empty() && !segment.feature_tags.empty() && segment.design_space.empty()) { - uint32_t node_id = TRY(AddFeaturesOnly(segment.feature_tags)); + uint32_t node_id = + TRY(AddFeaturesOnly(segment.feature_tags, condition_index)); condition_to_node_id[normalized] = node_id; return node_id; } else if (segment.codepoints.empty() && segment.feature_tags.empty() && !segment.design_space.empty()) { - uint32_t node_id = TRY(AddDesignSpaceOnly(segment.design_space)); + uint32_t node_id = + TRY(AddDesignSpaceOnly(segment.design_space, condition_index)); condition_to_node_id[normalized] = node_id; return node_id; } } - uint32_t node_id = TRY(CreateNode()); + uint32_t node_id = TRY(CreateNode(condition_index)); incoming_edge_count[node_id]++; condition_to_node_id[normalized] = node_id; @@ -215,15 +241,18 @@ StatusOr EntryGraph::AddMapping( // features and one for the codepoints. const auto& segment = segments.at(*normalized.TriggeringSegments().begin()); if (!segment.codepoints.empty()) { - uint32_t codepoints_id = TRY(AddCodepointsOnly(segment.codepoints)); + uint32_t codepoints_id = + TRY(AddCodepointsOnly(segment.codepoints, condition_index)); nodes[node_id].children_ids.insert(codepoints_id); } if (!segment.feature_tags.empty()) { - uint32_t features_id = TRY(AddFeaturesOnly(segment.feature_tags)); + uint32_t features_id = + TRY(AddFeaturesOnly(segment.feature_tags, condition_index)); nodes[node_id].children_ids.insert(features_id); } if (!segment.design_space.empty()) { - uint32_t design_space_id = TRY(AddDesignSpaceOnly(segment.design_space)); + uint32_t design_space_id = + TRY(AddDesignSpaceOnly(segment.design_space, condition_index)); nodes[node_id].children_ids.insert(design_space_id); } @@ -236,8 +265,8 @@ StatusOr EntryGraph::AddMapping( nodes[node_id].child_mode = OR; std::vector children; for (const auto& s : or_segments) { - children.push_back(TRY( - AddMapping(ActivationCondition::exclusive_segment(s, 0), segments))); + children.push_back(TRY(AddMapping( + ActivationCondition::exclusive_segment(s, 0), segments, condition_index, false))); } TRYV(AddChildrenToNode(node_id, OR, children)); return node_id; @@ -247,7 +276,7 @@ StatusOr EntryGraph::AddMapping( std::vector children; for (const auto& or_segments : normalized.conditions()) { children.push_back(TRY(AddMapping( - ActivationCondition::or_segments(or_segments, 0), segments))); + ActivationCondition::or_segments(or_segments, 0), segments, condition_index, false))); } TRYV(AddChildrenToNode(node_id, AND, children)); @@ -260,7 +289,7 @@ absl::Status EntryGraph::AddChildrenToNode(uint32_t node_id, ChildMode mode, for (size_t i = 0; i < 126; ++i) { nodes[node_id].children_ids.insert(children_ids[i]); } - uint32_t next_node_id = TRY(CreateNode()); + uint32_t next_node_id = TRY(CreateNode(nodes[node_id].condition_index)); nodes[next_node_id].child_mode = mode; incoming_edge_count[next_node_id]++; nodes[node_id].children_ids.insert(next_node_id); @@ -276,14 +305,14 @@ absl::Status EntryGraph::AddChildrenToNode(uint32_t node_id, ChildMode mode, } StatusOr EntryGraph::AddCodepointsOnly( - const common::CodepointSet& codepoints) { + const common::CodepointSet& codepoints, uint32_t condition_index) { auto existing = codepoints_to_node_id.find(codepoints); if (existing != codepoints_to_node_id.end()) { incoming_edge_count[existing->second]++; return existing->second; } - uint32_t node_id = TRY(CreateNode()); + uint32_t node_id = TRY(CreateNode(condition_index)); incoming_edge_count[node_id]++; codepoints_to_node_id[codepoints] = node_id; nodes[node_id].and_codepoints = codepoints; @@ -291,7 +320,7 @@ StatusOr EntryGraph::AddCodepointsOnly( } StatusOr EntryGraph::AddFeaturesOnly( - const btree_set& features) { + const btree_set& features, uint32_t condition_index) { flat_hash_set features_set; features_set.insert(features.begin(), features.end()); @@ -301,7 +330,7 @@ StatusOr EntryGraph::AddFeaturesOnly( return existing->second; } - uint32_t node_id = TRY(CreateNode()); + uint32_t node_id = TRY(CreateNode(condition_index)); incoming_edge_count[node_id]++; features_to_node_id[features_set] = node_id; nodes[node_id].and_features = features_set; @@ -310,21 +339,22 @@ StatusOr EntryGraph::AddFeaturesOnly( // Returns node id. De-dups if possible. StatusOr EntryGraph::AddDesignSpaceOnly( - const flat_hash_map& design_space) { + const flat_hash_map& design_space, + uint32_t condition_index) { auto existing = design_space_to_node_id.find(design_space); if (existing != design_space_to_node_id.end()) { incoming_edge_count[existing->second]++; return existing->second; } - uint32_t node_id = TRY(CreateNode()); + uint32_t node_id = TRY(CreateNode(condition_index)); incoming_edge_count[node_id]++; design_space_to_node_id[design_space] = node_id; nodes[node_id].and_design_space = design_space; return node_id; } -StatusOr EntryGraph::CreateNode() { +StatusOr EntryGraph::CreateNode(uint32_t condition_index) { if (nodes.size() > UINT32_MAX) { return absl::InternalError("Node ID integer overflow."); } @@ -335,6 +365,7 @@ StatusOr EntryGraph::CreateNode() { .and_design_space = {}, .child_mode = NONE, .children_ids = {}, + .condition_index = condition_index, }); incoming_edge_count.push_back(0); return node_id; @@ -726,19 +757,19 @@ Status EntryGraph::ActuateSubsumption(uint32_t node_id, node.child_mode = OR; if (result.codepoints) { - uint32_t id = TRY(CreateNode()); + uint32_t id = TRY(CreateNode(node.condition_index)); nodes[id].and_codepoints = *result.codepoints; incoming_edge_count[id]++; nodes[node_id].children_ids.insert(id); } if (result.features) { - uint32_t id = TRY(CreateNode()); + uint32_t id = TRY(CreateNode(node.condition_index)); nodes[id].and_features = *result.features; incoming_edge_count[id]++; nodes[node_id].children_ids.insert(id); } if (result.design_space) { - uint32_t id = TRY(CreateNode()); + uint32_t id = TRY(CreateNode(node.condition_index)); nodes[id].and_design_space = *result.design_space; incoming_edge_count[id]++; nodes[node_id].children_ids.insert(id); diff --git a/ift/encoder/entry_graph.h b/ift/encoder/entry_graph.h index a8c56272..a52969f5 100644 --- a/ift/encoder/entry_graph.h +++ b/ift/encoder/entry_graph.h @@ -16,9 +16,6 @@ enum ChildMode { OR, }; -// TODO(garretrieger): better optimize patch id encodings by ordering entries by -// patch ids where possible. - struct EntryNode { common::CodepointSet and_codepoints; absl::flat_hash_set and_features; @@ -26,6 +23,7 @@ struct EntryNode { ChildMode child_mode = NONE; common::IntSet children_ids; + uint32_t condition_index = 0; // Generate an estimated encoding cost, ignores the impact of last patch index // and the default format selection on final encoding size. @@ -84,22 +82,26 @@ class EntryGraph { // Returns node id. De-dups if possible. absl::StatusOr AddMapping( const ActivationCondition& condition, - const absl::flat_hash_map& segments); + const absl::flat_hash_map& segments, + uint32_t condition_index, + bool root + ); // Returns node id. De-dups if possible. absl::StatusOr AddCodepointsOnly( - const common::CodepointSet& codepoints); + const common::CodepointSet& codepoints, uint32_t condition_index); // Returns node id. De-dups if possible. absl::StatusOr AddFeaturesOnly( - const absl::btree_set& features); + const absl::btree_set& features, uint32_t condition_index); // Returns node id. De-dups if possible. absl::StatusOr AddDesignSpaceOnly( const absl::flat_hash_map& - design_space); + design_space, + uint32_t condition_index); - absl::StatusOr CreateNode(); + absl::StatusOr CreateNode(uint32_t condition_index); common::IntSet ReachableNodes() const; diff --git a/ift/encoder/entry_graph_test.cc b/ift/encoder/entry_graph_test.cc index f5a626dc..be6a6bd9 100644 --- a/ift/encoder/entry_graph_test.cc +++ b/ift/encoder/entry_graph_test.cc @@ -681,4 +681,55 @@ TEST(EntryGraphTest, Optimize_Conjunctive) { EXPECT_EQ(root_entry.coverage.child_indices.size(), 1); } +TEST(EntryGraphTest, TopologicalSort_ActivationConditionOrdering) { + flat_hash_map segments = { + {1, {'a'}}, + {2, {'b'}}, + {3, {'c'}}, + }; + + ActivationCondition cond_a = ActivationCondition::exclusive_segment(1, 10); + ActivationCondition cond_b = ActivationCondition::exclusive_segment(2, 20); + ActivationCondition cond_c = ActivationCondition::exclusive_segment(3, 30); + + // Case 1: Order [a, b, c] + { + std::vector conditions = {cond_a, cond_b, cond_c}; + auto graph = EntryGraph::Create(conditions, segments); + ASSERT_TRUE(graph.ok()) << graph.status(); + auto entries = graph->ToPatchMapEntries(proto::GLYPH_KEYED); + ASSERT_TRUE(entries.ok()) << entries.status(); + ASSERT_EQ(entries->size(), 3); + EXPECT_EQ(entries->at(0).patch_indices[0], 10); + EXPECT_EQ(entries->at(1).patch_indices[0], 20); + EXPECT_EQ(entries->at(2).patch_indices[0], 30); + } + + // Case 2: Order [c, b, a] + { + std::vector conditions = {cond_c, cond_b, cond_a}; + auto graph = EntryGraph::Create(conditions, segments); + ASSERT_TRUE(graph.ok()) << graph.status(); + auto entries = graph->ToPatchMapEntries(proto::GLYPH_KEYED); + ASSERT_TRUE(entries.ok()) << entries.status(); + ASSERT_EQ(entries->size(), 3); + EXPECT_EQ(entries->at(0).patch_indices[0], 30); + EXPECT_EQ(entries->at(1).patch_indices[0], 20); + EXPECT_EQ(entries->at(2).patch_indices[0], 10); + } + + // Case 3: Order [condB, condA, condC] + { + std::vector conditions = {cond_b, cond_a, cond_c}; + auto graph = EntryGraph::Create(conditions, segments); + ASSERT_TRUE(graph.ok()) << graph.status(); + auto entries = graph->ToPatchMapEntries(proto::GLYPH_KEYED); + ASSERT_TRUE(entries.ok()) << entries.status(); + ASSERT_EQ(entries->size(), 3); + EXPECT_EQ(entries->at(0).patch_indices[0], 20); + EXPECT_EQ(entries->at(1).patch_indices[0], 10); + EXPECT_EQ(entries->at(2).patch_indices[0], 30); + } +} + } // namespace ift::encoder diff --git a/ift/integration_test.cc b/ift/integration_test.cc index e57da29a..3e3dddfa 100644 --- a/ift/integration_test.cc +++ b/ift/integration_test.cc @@ -1441,7 +1441,7 @@ TEST_F(IntegrationTest, ASSERT_TRUE(extended.ok()) << extended.status(); auto extended_face = extended->face(); - auto expected_uris = btree_set{"18.ift_tk", "0C.2.ift_gk"}; + auto expected_uris = btree_set{"2K.ift_tk", "0C.2.ift_gk"}; ASSERT_EQ(fetched_uris, expected_uris); ASSERT_TRUE(GvarHasLongOffsets(*extended)); @@ -1511,7 +1511,7 @@ TEST_F(IntegrationTest, auto extended_face = extended->face(); auto expected_uris = - btree_set{"0O.ift_tk", "18.ift_tk", "0C.2.ift_gk"}; + btree_set{"1C.ift_tk", "1O.ift_tk", "0C.2.ift_gk"}; ASSERT_EQ(fetched_uris, expected_uris); ASSERT_TRUE(GvarHasLongOffsets(*extended)); @@ -1571,7 +1571,7 @@ TEST_F(IntegrationTest, MixedMode_DesignSpaceAugmentation_DropsUnusedPatches) { &fetched_uris); // correspond to ids 3, 4, 6, d - btree_set expected_uris{"0S.ift_tk", "20.ift_tk", "0C.2.ift_gk", + btree_set expected_uris{"1G.ift_tk", "20.ift_tk", "0C.2.ift_gk", "0G.2.ift_gk"}; ASSERT_EQ(fetched_uris, expected_uris);