From ee2a8cba3901b7515a10f03e922b893d9361b3f5 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 30 Jul 2026 18:32:53 +0000 Subject: [PATCH 1/3] Update to harfbuzz head which now includes the dependency graph api. Updates to the new dependency graph api. --- MODULE.bazel | 6 +- ift/dep_graph/dependency_graph.cc | 147 +++++++++++++++--------------- ift/dep_graph/dependency_graph.h | 18 ++-- third_party/harfbuzz.BUILD | 3 +- 4 files changed, 91 insertions(+), 83 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 04c8f543..69f8333f 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -53,9 +53,9 @@ http_archive( http_archive( name = "harfbuzz", build_file = "//third_party:harfbuzz.BUILD", - integrity = "sha256-L3SwHBRR3Leerxu4zmnOP6nKaZ5rp8fHJSxZ6iiDrIQ=", - strip_prefix = "harfbuzz-d4d4c635217d7e769b66f7d2cb7d528b4a534555", - urls = ["https://github.com/harfbuzz/harfbuzz/archive/d4d4c635217d7e769b66f7d2cb7d528b4a534555.zip"], + strip_prefix = "harfbuzz-c5eda499b4bcc310095b19faec99c9749dabdb52", + integrity = "sha256-4yNdOLuwFK+A9zea2Ykvo1EZbfb+On36/7k7LaXFZYg=", + urls = ["https://github.com/harfbuzz/harfbuzz/archive/c5eda499b4bcc310095b19faec99c9749dabdb52.zip"], ) # Base 32 Hex diff --git a/ift/dep_graph/dependency_graph.cc b/ift/dep_graph/dependency_graph.cc index 26ab0db6..d87490bb 100644 --- a/ift/dep_graph/dependency_graph.cc +++ b/ift/dep_graph/dependency_graph.cc @@ -51,7 +51,7 @@ StatusOr DependencyGraph::Create( const RequestedSegmentationInformation* segmentation_info, hb_face_t* face, const DataFileResolver& resolver) { auto full_feature_set = TRY(FullFeatureSet(segmentation_info, face)); - hb_depend_t* depend = hb_depend_from_face_or_fail(face); + hb_subset_depend_t* depend = hb_subset_depend_from_face_or_fail(face); if (!depend) { return absl::InternalError("Call to hb_depend_from_face_or_fail() failed."); } @@ -65,23 +65,23 @@ StatusOr DependencyGraph::Create( DependencyGraph::DependencyGraph( const RequestedSegmentationInformation* segmentation_info, - hb_depend_t* depend, hb_face_t* face, + hb_subset_depend_t* depend, hb_face_t* face, flat_hash_set full_feature_set, UnicodeEdges unicode_edges) : segmentation_info_(segmentation_info), original_face_(ift::common::make_hb_face(hb_face_reference(face))), full_feature_set_(full_feature_set), - dependency_graph_(depend, &hb_depend_destroy), + dependency_graph_(depend, &hb_subset_depend_destroy), layout_feature_implied_edges_(ComputeFeatureEdges()), context_glyph_implied_edges_(ComputeContextGlyphEdges()), unicode_edges_(std::move(unicode_edges)) {} -StatusOr GetContextSet(hb_depend_t* depend, +StatusOr GetContextSet(hb_subset_depend_t* depend, const GlyphSet* full_closure, hb_codepoint_t context_set_id) { // the context set is actually a set of sets. hb_set_unique_ptr context_sets = make_hb_set(); - if (!hb_depend_get_set_from_index(depend, context_set_id, - context_sets.get())) { + if (!hb_subset_depend_lookup_set(depend, context_set_id, + context_sets.get())) { return absl::InternalError("Context set lookup failed."); } @@ -96,8 +96,8 @@ StatusOr GetContextSet(hb_depend_t* depend, hb_codepoint_t actual_set_id = set_id & 0x7FFFFFFF; hb_set_unique_ptr context_glyphs = make_hb_set(); - if (!hb_depend_get_set_from_index(depend, actual_set_id, - context_glyphs.get())) { + if (!hb_subset_depend_lookup_set(depend, actual_set_id, + context_glyphs.get())) { return absl::InternalError("Context sub set lookup failed."); } glyphs.union_from(context_glyphs.get()); @@ -115,7 +115,7 @@ StatusOr GetContextSet(hb_depend_t* depend, template class TraversalContext { public: - hb_depend_t* depend = nullptr; + hb_subset_depend_t* depend = nullptr; // Only edges from these tables will be followed. flat_hash_set table_filter = {FontHelper::kCmap, FontHelper::kGlyf, @@ -212,7 +212,7 @@ class TraversalContext { // This checks all pending edges and if any have their constraints satisfied // then they are traversed. Returns true if there are now more nodes in the // next queue. - StatusOr CheckPending(hb_depend_t* depend_graph); + StatusOr CheckPending(hb_subset_depend_t* depend_graph); // Returns true if one or more pending edges remains. // @@ -232,8 +232,10 @@ class TraversalContext { return absl::OkStatus(); } - TRYV(callback.Visit(*this, PendingEdge::Disjunctive( - source, dest, table_tag.value_or(HB_TAG(' ', ' ', ' ', ' '))))); + TRYV(callback.Visit( + *this, + PendingEdge::Disjunctive( + source, dest, table_tag.value_or(HB_TAG(' ', ' ', ' ', ' '))))); Reached(dest); return absl::OkStatus(); } @@ -332,7 +334,7 @@ class TraversalContext { private: StatusOr LigaSetSatisfied(hb_codepoint_t liga_set, const GlyphSet& reached) { - if (!hb_depend_get_set_from_index(depend, liga_set, scratch_set_.get())) { + if (!hb_subset_depend_lookup_set(depend, liga_set, scratch_set_.get())) { return absl::InternalError( "ConstraintsSatisfied(): Ligature set lookup failed."); } @@ -344,8 +346,8 @@ class TraversalContext { StatusOr ContextSetSatisfied(hb_codepoint_t context_set_index, const GlyphSet& reached) { // the context set is actually a set of sets. - if (!hb_depend_get_set_from_index(depend, context_set_index, - scratch_set_.get())) { + if (!hb_subset_depend_lookup_set(depend, context_set_index, + scratch_set_.get())) { return absl::InternalError( "ContextSetSatisfied(): Context set lookup failed."); } @@ -362,8 +364,8 @@ class TraversalContext { } hb_codepoint_t actual_set_id = set_id & 0x7FFFFFFF; - if (!hb_depend_get_set_from_index(depend, actual_set_id, - scratch_set_aux_.get())) { + if (!hb_subset_depend_lookup_set(depend, actual_set_id, + scratch_set_aux_.get())) { return absl::InternalError("Context sub set lookup failed."); } @@ -454,17 +456,18 @@ class TraversalContext { } }; -Status DependencyGraph::ClosureState::Visit(const TraversalContext& context, const PendingEdge& edge) { - +Status DependencyGraph::ClosureState::Visit( + const TraversalContext& context, const PendingEdge& edge) { if (edge.table_tag == HB_TAG(' ', ' ', ' ', ' ')) { traversal.Visit(edge.dest); } else if (edge.table_tag == FontHelper::kGSUB && - edge.required_feature.has_value()) { + edge.required_feature.has_value()) { if (edge.required_context_set_index.has_value() && collect_context) { GlyphSet context_glyphs = TRY(GetContextSet(context.depend, context.full_closure, *edge.required_context_set_index)); - traversal.VisitContextual(edge.dest, *edge.required_feature, context_glyphs); + traversal.VisitContextual(edge.dest, *edge.required_feature, + context_glyphs); } else { traversal.VisitGsub(edge.dest, *edge.required_feature); } @@ -511,7 +514,7 @@ static Status DoTraversal(const PendingEdge& edge, template StatusOr TraversalContext::CheckPending( - hb_depend_t* depend_graph) { + hb_subset_depend_t* depend_graph) { bool did_work = false; auto it = pending_edges_.begin(); while (it != pending_edges_.end()) { @@ -531,8 +534,9 @@ StatusOr TraversalContext::CheckPending( } template -Status TraversalContext::TraversePendingEdgeTo(Node dest, PendingEdge edge, - hb_tag_t table_tag) { +Status TraversalContext::TraversePendingEdgeTo(Node dest, + PendingEdge edge, + hb_tag_t table_tag) { if (!ShouldFollow(dest, table_tag, edge.required_feature)) { return absl::OkStatus(); } @@ -572,7 +576,8 @@ DependencyGraph::StronglyConnectedComponents( const absl::flat_hash_set* node_inclusion_filter) const { struct Callback { std::vector edges; - Status Visit(const TraversalContext& context, const PendingEdge& pe) { + Status Visit(const TraversalContext& context, + const PendingEdge& pe) { edges.push_back(pe.dest); return absl::OkStatus(); } @@ -903,7 +908,8 @@ Status DependencyGraph::HandleUnicodeOutgoingEdges( { auto it = unicode_edges_.unicode_to_gid.find(unicode); if (it != unicode_edges_.unicode_to_gid.end()) { - TRYV(context->TraverseEdgeTo(Node::Unicode(unicode), Node::Glyph(it->second))); + TRYV(context->TraverseEdgeTo(Node::Unicode(unicode), + Node::Glyph(it->second))); } } @@ -926,7 +932,7 @@ Status DependencyGraph::HandleUnicodeOutgoingEdges( if (decomp_edges != unicode_edges_.decomposition.end()) { for (hb_codepoint_t dest : decomp_edges->second) { TRYV(context->TraverseEdgeTo(Node::Unicode(unicode), Node::Unicode(dest), - FontHelper::kCmap)); + FontHelper::kCmap)); } } @@ -935,7 +941,8 @@ Status DependencyGraph::HandleUnicodeOutgoingEdges( auto unicode_funcs = hb_unicode_funcs_get_default(); hb_codepoint_t mirror = hb_unicode_mirroring(unicode_funcs, unicode); if (mirror != unicode) { - TRYV(context->TraverseEdgeTo(Node::Unicode(unicode), Node::Unicode(mirror))); + TRYV( + context->TraverseEdgeTo(Node::Unicode(unicode), Node::Unicode(mirror))); } return absl::OkStatus(); @@ -944,35 +951,34 @@ Status DependencyGraph::HandleUnicodeOutgoingEdges( template Status DependencyGraph::HandleGlyphOutgoingEdges( glyph_id_t gid, TraversalContext* context) const { - hb_codepoint_t index = 0; - hb_tag_t table_tag = HB_CODEPOINT_INVALID; - hb_codepoint_t dep_gid = HB_CODEPOINT_INVALID; - hb_tag_t layout_tag = HB_CODEPOINT_INVALID; - hb_codepoint_t ligature_set = HB_CODEPOINT_INVALID; - hb_codepoint_t context_set = HB_CODEPOINT_INVALID; - - while (hb_depend_get_glyph_entry( - dependency_graph_.get(), gid, index++, &table_tag, &dep_gid, &layout_tag, - &ligature_set, &context_set, nullptr /* flags */)) { + hb_codepoint_t entry_count = hb_subset_depend_lookup_glyph( + dependency_graph_.get(), gid, 0, nullptr, nullptr); + for (unsigned i = 0; i < entry_count; i++) { + unsigned count = 1; + hb_subset_depend_entry_t entry; + hb_subset_depend_lookup_glyph(dependency_graph_.get(), gid, i, &count, + &entry); + // Pre-filter to avoid doing extra work if not needed // TODO XXXX we're filtering twice once here, and once in TravserseEdgeTo // can we only filter once? - if (!context->table_filter.contains(table_tag)) { + if (!context->table_filter.contains(entry.table_tag)) { continue; } if (context->glyph_filter != nullptr && - !context->glyph_filter->contains(dep_gid)) { + !context->glyph_filter->contains(entry.dependent)) { continue; } - Node dest = Node::Glyph(dep_gid); - if (table_tag == HB_TAG('G', 'S', 'U', 'B')) { - TRYV(HandleGsubGlyphOutgoingEdges(gid, dep_gid, layout_tag, ligature_set, - context_set, context)); + Node dest = Node::Glyph(entry.dependent); + if (entry.table_tag == HB_TAG('G', 'S', 'U', 'B')) { + TRYV(HandleGsubGlyphOutgoingEdges(gid, entry.dependent, entry.layout_tag, + entry.ligature_set_index, + entry.context_set_index, context)); continue; } - TRYV(context->TraverseEdgeTo(Node::Glyph(gid), dest, table_tag)); + TRYV(context->TraverseEdgeTo(Node::Glyph(gid), dest, entry.table_tag)); } auto it = context_glyph_implied_edges_.find(gid); @@ -1047,7 +1053,7 @@ Status DependencyGraph::HandleSegmentOutgoingEdges( const Segment& s = segmentation_info_->Segments().at(id); TRYV(HandleSubsetDefinitionOutgoingEdges(Node::Segment(id), s.Definition(), - context)); + context)); return absl::OkStatus(); } @@ -1135,8 +1141,8 @@ StatusOr DependencyGraph::RequiredGlyphsFor( StatusOr DependencyGraph::GetLigaSet( hb_codepoint_t liga_set_id) const { - if (!hb_depend_get_set_from_index(dependency_graph_.get(), liga_set_id, - scratch_set_.get())) { + if (!hb_subset_depend_lookup_set(dependency_graph_.get(), liga_set_id, + scratch_set_.get())) { return absl::InternalError("Ligature set lookup failed."); } GlyphSet glyphs(scratch_set_.get()); @@ -1174,9 +1180,9 @@ StatusOr DependencyGraph::ExtractRequirements( // decide if the resulting conditions are exact or an over approximation. if (edge.required_context_set_index.has_value()) { // the context set is actually a set of sets. - if (!hb_depend_get_set_from_index(dependency_graph_.get(), - *edge.required_context_set_index, - scratch_set_.get())) { + if (!hb_subset_depend_lookup_set(dependency_graph_.get(), + *edge.required_context_set_index, + scratch_set_.get())) { return absl::InternalError("Context set lookup failed."); } @@ -1187,9 +1193,8 @@ StatusOr DependencyGraph::ExtractRequirements( req.push_back(Node::Glyph(set_id)); } else { hb_codepoint_t actual_set_id = set_id & 0x7FFFFFFF; - if (!hb_depend_get_set_from_index(dependency_graph_.get(), - actual_set_id, - scratch_set_aux_.get())) { + if (!hb_subset_depend_lookup_set(dependency_graph_.get(), actual_set_id, + scratch_set_aux_.get())) { return absl::InternalError("Context sub set lookup failed."); } @@ -1251,26 +1256,25 @@ DependencyGraph::ComputeFeatureEdges() const { for (glyph_id_t gid = 0; gid < hb_face_get_glyph_count(original_face_.get()); gid++) { - hb_codepoint_t index = 0; - hb_tag_t table_tag = HB_CODEPOINT_INVALID; - hb_codepoint_t dest_gid = HB_CODEPOINT_INVALID; - hb_tag_t layout_tag = HB_CODEPOINT_INVALID; - hb_codepoint_t ligature_set = HB_CODEPOINT_INVALID; - hb_codepoint_t context_set = HB_CODEPOINT_INVALID; - while (hb_depend_get_glyph_entry( - dependency_graph_.get(), gid, index++, &table_tag, &dest_gid, - &layout_tag, &ligature_set, &context_set, nullptr /* flags */)) { - if (table_tag != FontHelper::kGSUB || - layout_tag == HB_CODEPOINT_INVALID) { + hb_codepoint_t entry_count = hb_subset_depend_lookup_glyph( + dependency_graph_.get(), gid, 0, nullptr, nullptr); + for (unsigned i = 0; i < entry_count; i++) { + unsigned count = 1; + hb_subset_depend_entry_t entry; + hb_subset_depend_lookup_glyph(dependency_graph_.get(), gid, i, &count, + &entry); + + if (entry.table_tag != FontHelper::kGSUB || + entry.layout_tag == HB_CODEPOINT_INVALID) { continue; } - edges[layout_tag].insert(LayoutFeatureEdge{ - .layout_tag = layout_tag, + edges[entry.layout_tag].insert(LayoutFeatureEdge{ + .layout_tag = entry.layout_tag, .source_gid = gid, - .dest_gid = dest_gid, - .ligature_set = ligature_set, - .context_set = context_set, + .dest_gid = entry.dependent, + .ligature_set = entry.ligature_set_index, + .context_set = entry.context_set_index, }); } } @@ -1296,7 +1300,8 @@ DependencyGraph::CollectIncomingEdges( nullptr; const DependencyGraph* graph = nullptr; - Status Visit(const TraversalContext& context, const PendingEdge& pe) { + Status Visit(const TraversalContext& context, + const PendingEdge& pe) { auto reqs = TRY(graph->ExtractRequirements(pe)); (*incoming_edges)[pe.dest].insert(std::move(reqs)); return absl::OkStatus(); diff --git a/ift/dep_graph/dependency_graph.h b/ift/dep_graph/dependency_graph.h index 7f7fe096..f4c23edc 100644 --- a/ift/dep_graph/dependency_graph.h +++ b/ift/dep_graph/dependency_graph.h @@ -91,13 +91,13 @@ class DependencyGraph { // If filter is null then the filter defaults to the set of non init font // glyphs in segmentation info. absl::StatusOr ClosureTraversal( - const ift::common::SegmentSet& start, TraversalMode mode = ENFORCE_CONTEXT) const; + const ift::common::SegmentSet& start, + TraversalMode mode = ENFORCE_CONTEXT) const; absl::StatusOr ClosureTraversal( const absl::btree_set& nodes, const ift::common::GlyphSet* glyph_filter_ptr = nullptr, const ift::common::CodepointSet* unicode_filter_ptr = nullptr, - TraversalMode mode = ENFORCE_CONTEXT - ) const; + TraversalMode mode = ENFORCE_CONTEXT) const; const absl::flat_hash_set& FullFeatureSet() const { return full_feature_set_; @@ -136,7 +136,7 @@ class DependencyGraph { private: DependencyGraph( const ift::encoder::RequestedSegmentationInformation* segmentation_info, - hb_depend_t* depend, hb_face_t* face, + hb_subset_depend_t* depend, hb_face_t* face, absl::flat_hash_set full_feature_set, UnicodeEdges unicode_edges); @@ -146,7 +146,8 @@ class DependencyGraph { Traversal traversal; bool collect_context = true; - absl::Status Visit(const TraversalContext& context, const PendingEdge& edge); + absl::Status Visit(const TraversalContext& context, + const PendingEdge& edge); std::optional GetNext(); bool Reached(Node node); @@ -183,8 +184,8 @@ class DependencyGraph { hb_tag_t feature_tag, TraversalContext* context) const; template - absl::Status HandleSegmentOutgoingEdges(encoder::segment_index_t id, - TraversalContext* context) const; + absl::Status HandleSegmentOutgoingEdges( + encoder::segment_index_t id, TraversalContext* context) const; template absl::Status HandleSubsetDefinitionOutgoingEdges( @@ -213,7 +214,8 @@ class DependencyGraph { ift::common::hb_face_unique_ptr original_face_; absl::flat_hash_set full_feature_set_; - std::unique_ptr dependency_graph_; + std::unique_ptr + dependency_graph_; struct LayoutFeatureEdge { hb_tag_t layout_tag; diff --git a/third_party/harfbuzz.BUILD b/third_party/harfbuzz.BUILD index ebfaae6f..a17ac966 100644 --- a/third_party/harfbuzz.BUILD +++ b/third_party/harfbuzz.BUILD @@ -74,13 +74,14 @@ cc_library( ), hdrs = [ "src/hb.h", + "src/hb-gpu.h", "src/hb-aat.h", "src/hb-aat-layout.h", "src/hb-blob.h", "src/hb-buffer.h", "src/hb-cairo.h", "src/hb-common.h", - "src/hb-depend.h", + "src/hb-subset-depend.h", "src/hb-deprecated.h", "src/hb-draw.h", "src/hb-face.h", From 9add9e6be3e0e47d7e4774090397b40993d26390 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 30 Jul 2026 19:28:19 +0000 Subject: [PATCH 2/3] Remove build option to disable dependency graph support. Dependency graph API is now in mainline harfbuzz, so there's no longer a need to allow for the encoder to be built without dependency graph support. --- .github/workflows/ci.yml | 4 -- BUILD | 13 ---- README.md | 13 ---- ift/config/auto_segmenter_config.cc | 4 -- ift/config/auto_segmenter_config_test.cc | 4 -- ift/config/segmenter_config.proto | 10 +-- ift/dep_graph/BUILD | 33 ++++------ ift/encoder/BUILD | 19 ++---- ift/encoder/candidate_merge_test.cc | 2 - ift/encoder/closure_glyph_segmenter_test.cc | 71 --------------------- ift/encoder/dependency_closure.h | 17 ----- ift/encoder/dependency_closure_disabled.cc | 47 -------------- ift/encoder/glyph_groupings_test.cc | 2 - ift/encoder/segmentation_context.cc | 17 ----- third_party/harfbuzz.BUILD | 7 +- 15 files changed, 24 insertions(+), 239 deletions(-) delete mode 100644 ift/encoder/dependency_closure_disabled.cc diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 108f1fc6..1c9bfe6e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,10 +22,6 @@ jobs: run: | "${GITHUB_WORKSPACE}/bin/bazel" test -c opt ... --test_output=all - - name: Test (Default Build, No Dep Graph) - run: | - "${GITHUB_WORKSPACE}/bin/bazel" test --//:harfbuzz_dep_graph=False ... --test_output=all - check_tests_osx: name: Check Tests (Mac OS) runs-on: macos-latest diff --git a/BUILD b/BUILD index 6a7ce3b2..8adb25b3 100644 --- a/BUILD +++ b/BUILD @@ -2,16 +2,3 @@ load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") package(features = ["layering_check"]) -bool_flag( - name = "harfbuzz_dep_graph", - build_setting_default = True, - visibility = ["//visibility:public"], -) - -config_setting( - name = "use_harfbuzz_dep_graph", - flag_values = { - ":harfbuzz_dep_graph": "True", - }, - visibility = ["//visibility:public"], -) diff --git a/README.md b/README.md index 87eb9cda..a5f4dafd 100644 --- a/README.md +++ b/README.md @@ -33,19 +33,6 @@ and run all of the tests: bazel test ... ``` -### Building without Dependency Graph Support - -By default this depends on the experimental harfbuzz dependency graph API which isn't yet in mainline harfbuzz. -The dependency graph functionality can be disabled at compile time using the `harfbuzz_dep_graph` build flag. -For example: - -```sh -bazel build --//:harfbuzz_dep_graph=False ... -bazel test --//:harfbuzz_dep_graph=False ... -``` - -Disabling the harfbuzz dependency graph API will cause segmenter runs using the `CLOSURE_AND_DEP_GRAPH` and `CLOSURE_AND_VALIDATE_DEP_GRAPH` condition analysis modes to fail. - ## Producing IFT Encoded Fonts This project provides command line utilities and a C++ API which can be used to produce IFT encoded diff --git a/ift/config/auto_segmenter_config.cc b/ift/config/auto_segmenter_config.cc index b22c8149..1cfaaea8 100644 --- a/ift/config/auto_segmenter_config.cc +++ b/ift/config/auto_segmenter_config.cc @@ -602,15 +602,11 @@ static void ApplyQualityLevelTo(Quality quality, SegmenterConfig& config) { config.set_unmapped_glyph_handling(MOVE_TO_INIT_FONT); -#ifdef HB_DEPEND_API if (quality == ONE) { config.set_condition_analysis_mode(DEP_GRAPH_ONLY_WITH_SIMPLIFICATION); } else { config.set_condition_analysis_mode(DEP_GRAPH_ONLY); } -#else - config.set_condition_analysis_mode(CLOSURE_ONLY); -#endif switch (quality) { case ONE: diff --git a/ift/config/auto_segmenter_config_test.cc b/ift/config/auto_segmenter_config_test.cc index 8913b0e5..ebe6eb5c 100644 --- a/ift/config/auto_segmenter_config_test.cc +++ b/ift/config/auto_segmenter_config_test.cc @@ -160,11 +160,7 @@ base_segmentation_plan { } generate_feature_segments: true )" -#ifdef HB_DEPEND_API "condition_analysis_mode: DEP_GRAPH_ONLY\n" -#else - "condition_analysis_mode: CLOSURE_ONLY\n" -#endif ); } diff --git a/ift/config/segmenter_config.proto b/ift/config/segmenter_config.proto index ae8c826f..31eb2c13 100644 --- a/ift/config/segmenter_config.proto +++ b/ift/config/segmenter_config.proto @@ -6,15 +6,16 @@ import "ift/config/segmentation_plan.proto"; import "ift/config/common.proto"; enum ConditionAnalysisMode { + // [DEPRECATED] // Analyzes glyph conditions using closure analysis only. + // This is a legacy mode, it's much slower and generally produces + // worse results then the dependency graph approach. CLOSURE_ONLY = 0; // Attempts to analyze glyph conditions using a dependency // graph where possible. When dep graph is expected not to be accurate // it falls back to closure analysis. This is for many cases much faster - // then pure closure based analysis. However, the dependency - // graph approach is still experimental and may (rarely) produce - // different results than the pure closure analysis approach. + // then pure closure based analysis. CLOSURE_AND_DEP_GRAPH = 1; // Always runs both closure and dependency graph analysis, the @@ -23,6 +24,7 @@ enum ConditionAnalysisMode { // graph approach. CLOSURE_AND_VALIDATE_DEP_GRAPH = 2; + // [RECOMMENDED] // Analyze glyph conditions using dep graph only. Closure is // only used at the very end to validate that founds conditions // meet the glyph closure requirement. @@ -148,7 +150,7 @@ message SegmenterConfig { // Configures the method used to find glyph activation conditions. See the enum // for more informmation. - ConditionAnalysisMode condition_analysis_mode = 15 [default = CLOSURE_ONLY]; + ConditionAnalysisMode condition_analysis_mode = 15 [default = DEP_GRAPH_ONLY]; // next = 16 } diff --git a/ift/dep_graph/BUILD b/ift/dep_graph/BUILD index 9e36ecf6..88b21e3a 100644 --- a/ift/dep_graph/BUILD +++ b/ift/dep_graph/BUILD @@ -26,21 +26,15 @@ cc_library( cc_library( name = "dep_graph", - srcs = select({ - "//:use_harfbuzz_dep_graph": [ - "dependency_graph.cc", - ], - "//conditions:default": [], - }), - hdrs = select({ - "//:use_harfbuzz_dep_graph": [ - "dependency_graph.h", - "node.h", - "pending_edge.h", - "traversal.h", - ], - "//conditions:default": [], - }), + srcs = [ + "dependency_graph.cc", + ], + hdrs = [ + "dependency_graph.h", + "node.h", + "pending_edge.h", + "traversal.h", + ], visibility = [ "//ift/encoder:__pkg__", ], @@ -65,12 +59,9 @@ cc_library( cc_test( name = "dependency_graph_test", size = "small", - srcs = select({ - "//:use_harfbuzz_dep_graph": [ - "dependency_graph_test.cc", - ], - "//conditions:default": [], - }), + srcs = [ + "dependency_graph_test.cc", + ], data = [ "//ift/common:testdata", ], diff --git a/ift/encoder/BUILD b/ift/encoder/BUILD index f23c17e7..c500f478 100644 --- a/ift/encoder/BUILD +++ b/ift/encoder/BUILD @@ -72,19 +72,13 @@ cc_library( srcs = [ "candidate_merge.cc", "complex_condition_finder.cc", + "dependency_closure.cc", "estimated_patch_size_cache.cc", "glyph_condition_set.cc", "glyph_groupings.cc", "merger.cc", "segmentation_context.cc", - ] + select({ - "//:use_harfbuzz_dep_graph": [ - "dependency_closure.cc", - ], - "//conditions:default": [ - "dependency_closure_disabled.cc", - ], - }), + ], hdrs = [ "candidate_merge.h", "complex_condition_finder.h", @@ -465,12 +459,9 @@ cc_test( cc_test( name = "dependency_closure_test", size = "small", - srcs = select({ - "//:use_harfbuzz_dep_graph": [ - "dependency_closure_test.cc", - ], - "//conditions:default": [], - }), + srcs = [ + "dependency_closure_test.cc", + ], data = [ "//ift/common:testdata", ], diff --git a/ift/encoder/candidate_merge_test.cc b/ift/encoder/candidate_merge_test.cc index defcf89d..ce676ff5 100644 --- a/ift/encoder/candidate_merge_test.cc +++ b/ift/encoder/candidate_merge_test.cc @@ -556,7 +556,6 @@ TEST_F(CandidateMergeTest, AssessPatchMerge_NonDisjunctive) { ASSERT_EQ(merge.SegmentsToMerge(), SegmentSet({0, 1, 2})); } -#ifdef HB_DEPEND_API TEST_F(CandidateMergeTest, AssessPatchMerge_NonDisjunctive_WithSimplification) { std::vector segments = { {{'A'}, ProbabilityBound{0.95, 0.95}}, @@ -634,7 +633,6 @@ TEST_F(CandidateMergeTest, AssessPatchMerge_NonDisjunctive_WithSimplification) { ASSERT_EQ(*context->glyph_groupings.CombinedPatches().Find(gid_A), *context->glyph_groupings.CombinedPatches().Find(gid_B)); } -#endif TEST_F(CandidateMergeTest, ComputeInitFontCostDelta) { std::vector segments = { diff --git a/ift/encoder/closure_glyph_segmenter_test.cc b/ift/encoder/closure_glyph_segmenter_test.cc index 9708feec..79377226 100644 --- a/ift/encoder/closure_glyph_segmenter_test.cc +++ b/ift/encoder/closure_glyph_segmenter_test.cc @@ -56,7 +56,6 @@ class ClosureGlyphSegmenterTest : public ::testing::Test { resolver), segmenter_move_to_init_font(8, 8, MOVE_TO_INIT_FONT, CLOSURE_ONLY, resolver), -#ifdef HB_DEPEND_API segmenter_dep_graph(8, 8, PATCH, CLOSURE_AND_VALIDATE_DEP_GRAPH, resolver), segmenter_dep_graph_only(8, 8, PATCH, DEP_GRAPH_ONLY, resolver), @@ -66,16 +65,6 @@ class ClosureGlyphSegmenterTest : public ::testing::Test { 8, 8, FIND_CONDITIONS, CLOSURE_AND_VALIDATE_DEP_GRAPH, resolver), segmenter_move_to_init_font_dep_graph( 8, 8, MOVE_TO_INIT_FONT, CLOSURE_AND_VALIDATE_DEP_GRAPH, resolver) -#else - segmenter_dep_graph(8, 8, PATCH, CLOSURE_ONLY, resolver), - segmenter_dep_graph_only(8, 8, PATCH, CLOSURE_ONLY, resolver), - segmenter_dep_graph_only_with_simplification(8, 8, PATCH, CLOSURE_ONLY, - resolver), - segmenter_find_conditions_dep_graph(8, 8, FIND_CONDITIONS, CLOSURE_ONLY, - resolver), - segmenter_move_to_init_font_dep_graph(8, 8, MOVE_TO_INIT_FONT, - CLOSURE_ONLY, resolver) -#endif { roboto = from_file("ift/common/testdata/Roboto-Regular.ttf"); noto_nastaliq_urdu = @@ -365,7 +354,6 @@ if ((s0 OR s1 OR s2)) then p2 )"); ASSERT_EQ(segmentation->ToString(), dep_graph_segmentation->ToString()); -#ifdef HB_DEPEND_API ASSERT_EQ(dep_graph_only_segmentation->ToString(), R"(initial font: { gid0, gid69 } p0: { gid74 } @@ -375,7 +363,6 @@ if ((s0 OR s2)) then p0 if ((s1 OR s2)) then p1 if ((s0 OR s2) AND (s1 OR s2)) then p2 )"); -#endif } TEST_F(ClosureGlyphSegmenterTest, SegmentationWithFeatures) { @@ -627,7 +614,6 @@ if ((s0 OR s1 OR s2 OR s3)) then p6 )"); ASSERT_EQ(segmentation->ToString(), dep_graph_segmentation->ToString()); -#ifdef HB_DEPEND_API ASSERT_EQ(dep_graph_only_segmentation->ToString(), R"(initial font: { gid0 } p0: { gid3, gid9, gid155 } @@ -649,7 +635,6 @@ if ((s0 OR s1 OR s2 OR s3)) then p6 if ((s0 OR s1) AND s2) then p7 if ((s0 OR s1) AND (s2 OR s3)) then p8 )"); -#endif } TEST_F(ClosureGlyphSegmenterTest, UnmappedGlyphs_FindConditions) { @@ -682,7 +667,6 @@ if ((s1 OR s2 OR s3 OR s4)) then p7 )"); } -#ifdef HB_DEPEND_API TEST_F(ClosureGlyphSegmenterTest, DepGraphOnly_FindConditions) { auto segmentation = segmenter_dep_graph_only.CodepointToGlyphSegments( noto_nastaliq_urdu.get(), {}, @@ -747,7 +731,6 @@ if ((s3 OR s4)) then p6 if ((s1 OR s2 OR s3 OR s4)) then p7 )"); } -#endif TEST_F(ClosureGlyphSegmenterTest, UnmappedGlyphs_FindConditions_IsFallback) { // Here the found conditions are equal to the fallback segment, this ensures @@ -1013,7 +996,6 @@ TEST_F(ClosureGlyphSegmenterTest, SimpleSegmentation_PatchMerge) { }; ASSERT_EQ(segmentation->Segments(), expected_segments); -#ifdef HB_DEPEND_API ASSERT_EQ(segmentation->ToString(), R"(initial font: { gid0 } p0: { gid117, gid169, gid640, gid700 } @@ -1021,15 +1003,6 @@ p1: { gid37, gid39 } if ((s2 OR s3)) then p0 if ((s0 OR s1 OR s2 OR s3)) then p1 )"); -#else - ASSERT_EQ(segmentation->ToString(), - R"(initial font: { gid0 } -p0: { gid117, gid169 } -p1: { gid37, gid39, gid640, gid700 } -if ((s2 OR s3)) then p0 -if ((s0 OR s1 OR s2 OR s3)) then p1 -)"); -#endif } TEST_F(ClosureGlyphSegmenterTest, SimpleSegmentation_NoPatchMerge) { @@ -1064,7 +1037,6 @@ TEST_F(ClosureGlyphSegmenterTest, SimpleSegmentation_NoPatchMerge) { }; ASSERT_EQ(segmentation->Segments(), expected_segments); -#ifdef HB_DEPEND_API ASSERT_EQ(segmentation->ToString(), R"(initial font: { gid0 } p0: { gid37 } @@ -1078,19 +1050,6 @@ if ((s2 OR s3)) then p2 if ((s0 OR s2) AND (s2 OR s3)) then p3 if ((s1 OR s3) AND (s2 OR s3)) then p4 )"); -#else - ASSERT_EQ(segmentation->ToString(), - R"(initial font: { gid0 } -p0: { gid37 } -p1: { gid39 } -p2: { gid117, gid169 } -p3: { gid640, gid700 } -if ((s0 OR s2)) then p0 -if ((s1 OR s3)) then p1 -if ((s2 OR s3)) then p2 -if ((s0 OR s1 OR s2 OR s3)) then p3 -)"); -#endif } TEST_F(ClosureGlyphSegmenterTest, SimpleSegmentation_PatchMerge_MinGroupSize) { @@ -1126,7 +1085,6 @@ TEST_F(ClosureGlyphSegmenterTest, SimpleSegmentation_PatchMerge_MinGroupSize) { // When min group size is 2, there's no merging done since merges are // unfavourable and the minimum is met. -#ifdef HB_DEPEND_API ASSERT_EQ(segmentation->ToString(), R"(initial font: { gid0 } p0: { gid37 } @@ -1140,19 +1098,6 @@ if ((s2 OR s3)) then p2 if ((s0 OR s2) AND (s2 OR s3)) then p3 if ((s1 OR s3) AND (s2 OR s3)) then p4 )"); -#else - ASSERT_EQ(segmentation->ToString(), - R"(initial font: { gid0 } -p0: { gid37 } -p1: { gid39 } -p2: { gid117, gid169 } -p3: { gid640, gid700 } -if ((s0 OR s2)) then p0 -if ((s1 OR s3)) then p1 -if ((s2 OR s3)) then p2 -if ((s0 OR s1 OR s2 OR s3)) then p3 -)"); -#endif // Now with min group size larger merges will be done to reach min group size strategy.SetMinimumGroupSize(3); @@ -1169,7 +1114,6 @@ if ((s0 OR s1 OR s2 OR s3)) then p3 ASSERT_EQ(segmentation->Segments(), expected_segments); // Group size minimum is met for everything other than the last condition // which has no other candidates to merge with. -#ifdef HB_DEPEND_API ASSERT_EQ(segmentation->ToString(), R"(initial font: { gid0 } p0: { gid37, gid117, gid169 } @@ -1179,20 +1123,8 @@ if ((s0 OR s2 OR s3)) then p0 if ((s1 OR s2 OR s3)) then p1 if ((s1 OR s3) AND (s2 OR s3)) then p2 )"); -#else - ASSERT_EQ(segmentation->ToString(), - R"(initial font: { gid0 } -p0: { gid39 } -p1: { gid37, gid117, gid169 } -p2: { gid640, gid700 } -if ((s1 OR s3)) then p0 -if ((s0 OR s2 OR s3)) then p1 -if ((s0 OR s1 OR s2 OR s3)) then p2 -)"); -#endif } -#ifdef HB_DEPEND_API TEST_F(ClosureGlyphSegmenterTest, PatchMerge_WithSimplification) { UnicodeFrequencies frequencies{ {{0x62a, 0x62a}, 100}, @@ -1250,7 +1182,6 @@ if ((s2 OR s3)) then p4 if ((s0 OR s1 OR s2 OR s3)) then p5 )"); } -#endif TEST_F(ClosureGlyphSegmenterTest, SimpleSegmentation_NoCostCutoff) { UnicodeFrequencies frequencies{ @@ -2034,7 +1965,6 @@ if (s1) then p1 if (s2) then p2 )"); -#ifdef HB_DEPEND_API ASSERT_EQ(dep_graph_only_segmentation->ToString(), R"(initial font: { gid0, gid69, gid106, gid670 } p0: { gid51, gid660 } @@ -2046,7 +1976,6 @@ if (s1) then p1 if (s2) then p2 if ((s0 OR s1) AND s2) then p3 )"); -#endif // Rerun segmentation with merging of Ö and ö allowed, should now get // the true condition. diff --git a/ift/encoder/dependency_closure.h b/ift/encoder/dependency_closure.h index 79e7e5f5..d55b1bb2 100644 --- a/ift/encoder/dependency_closure.h +++ b/ift/encoder/dependency_closure.h @@ -15,10 +15,8 @@ #include "ift/encoder/subset_definition.h" #include "ift/encoder/types.h" -#ifdef HB_DEPEND_API #include "ift/dep_graph/dependency_graph.h" #include "ift/dep_graph/node.h" -#endif namespace ift::encoder { @@ -38,9 +36,6 @@ class DependencyClosure { const RequestedSegmentationInformation* segmentation_info, hb_face_t* face, const ift::common::DataFileResolver& resolver, bool allow_context_glyph_analysis = false) { -#ifndef HB_DEPEND_API - return std::unique_ptr(new DependencyClosure()); -#else dep_graph::DependencyGraph graph = TRY( dep_graph::DependencyGraph::Create(segmentation_info, face, resolver)); auto result = std::unique_ptr( @@ -53,7 +48,6 @@ class DependencyClosure { TRYV(result->InitFontChanged(ift::common::SegmentSet::all())); return result; -#endif } enum AnalysisAccuracy { @@ -94,7 +88,6 @@ class DependencyClosure { const ift::common::SegmentSet& segments, ift::common::GlyphSet& and_gids, ift::common::GlyphSet& or_gids, ift::common::GlyphSet& exclusive_gids); -#ifdef HB_DEPEND_API // Extracts the full activations conditions (as specified by the dependency // graph) for all glyphs. In some cases may overestimate activation conditions // versus real subsetting closure due to reliance on the dependency graph. @@ -112,7 +105,6 @@ class DependencyClosure { const ift::common::SegmentSet& InertSegments() const { return inert_segments_; } -#endif // This structure caches information derived from the segmentation info // segments. These two function signal that segmentation info segments have @@ -138,16 +130,13 @@ class DependencyClosure { const common::GlyphSet& glyphs) const; absl::StatusOr SegmentsThatInteractWith( const SubsetDefinition& def) const; -#ifdef HB_DEPEND_API absl::StatusOr SegmentsThatInteractWith( const absl::flat_hash_set nodes) const; -#endif uint64_t AccurateResults() const { return accurate_results_; } uint64_t InaccurateResults() const { return inaccurate_results_; } private: -#ifdef HB_DEPEND_API // Extracts the full activations conditions (as specified by the dependency // graph) for all graph nodes. In some cases may overestimate activation @@ -193,11 +182,7 @@ class DependencyClosure { const std::vector>& sccs, absl::flat_hash_map& conditions, absl::flat_hash_set& modified) const; -#endif -#ifndef HB_DEPEND_API - DependencyClosure() {} -#else DependencyClosure(dep_graph::DependencyGraph&& graph, const RequestedSegmentationInformation* segmentation_info, hb_face_t* face) @@ -258,8 +243,6 @@ class DependencyClosure { // init items. absl::flat_hash_set init_font_nodes_; -#endif - uint64_t accurate_results_ = 0; uint64_t inaccurate_results_ = 0; }; diff --git a/ift/encoder/dependency_closure_disabled.cc b/ift/encoder/dependency_closure_disabled.cc deleted file mode 100644 index afd6164a..00000000 --- a/ift/encoder/dependency_closure_disabled.cc +++ /dev/null @@ -1,47 +0,0 @@ -#include "ift/common/int_set.h" -#include "ift/encoder/dependency_closure.h" - -using absl::Status; -using absl::StatusOr; -using ift::common::GlyphSet; -using ift::common::IntSet; -using ift::common::SegmentSet; - -namespace ift::encoder { - -Status DependencyClosure::InitFontChanged(const SegmentSet& segments) { - return absl::UnimplementedError( - "Dependency graph functionality was disabled during compilation and is " - "unvailable"); -} - -Status DependencyClosure::SegmentsMerged(segment_index_t base_segment, - const SegmentSet& segments) { - return absl::UnimplementedError( - "Dependency graph functionality was disabled during compilation and is " - "unavailable"); -} - -StatusOr DependencyClosure::AnalyzeSegment( - const SegmentSet& segments, GlyphSet& and_gids, GlyphSet& or_gids, - GlyphSet& exclusive_gids) { - return absl::UnimplementedError( - "Dependency graph functionality was disabled during compilation and is " - "unavailable"); -} - -StatusOr DependencyClosure::SegmentsThatInteractWith( - const GlyphSet& glyphs) const { - return absl::UnimplementedError( - "Dependency graph functionality was disabled during compilation and is " - "unavailable"); -} - -StatusOr DependencyClosure::SegmentsThatInteractWith( - const SubsetDefinition& def) const { - return absl::UnimplementedError( - "Dependency graph functionality was disabled during compilation and is " - "unavailable"); -} - -} // namespace ift::encoder \ No newline at end of file diff --git a/ift/encoder/glyph_groupings_test.cc b/ift/encoder/glyph_groupings_test.cc index 5bc72bfa..e98b145d 100644 --- a/ift/encoder/glyph_groupings_test.cc +++ b/ift/encoder/glyph_groupings_test.cc @@ -729,7 +729,6 @@ TEST_F(GlyphGroupingsTest, ComplexConditionFinding_Basic) { ASSERT_TRUE(glyph_groupings_complex_.UnmappedGlyphs().empty()); } -#ifdef HB_DEPEND_API TEST_F(GlyphGroupingsTest, ComplexConditionFinding_Basic_WithDependencyGraph) { std::unique_ptr dep_closure = *DependencyClosure::Create( requested_segmentation_info_complex_.get(), roboto_.get(), *resolver); @@ -765,7 +764,6 @@ TEST_F(GlyphGroupingsTest, ComplexConditionFinding_Basic_WithDependencyGraph) { ASSERT_EQ(expected, glyph_groupings_complex_.ConditionsAndGlyphs()); ASSERT_TRUE(glyph_groupings_complex_.UnmappedGlyphs().empty()); } -#endif TEST_F(GlyphGroupingsTest, ComplexConditionFinding_IncrementalUnchanged) { auto sc = glyph_groupings_complex_.GroupGlyphs( diff --git a/ift/encoder/segmentation_context.cc b/ift/encoder/segmentation_context.cc index d5380eb8..c3f4936c 100644 --- a/ift/encoder/segmentation_context.cc +++ b/ift/encoder/segmentation_context.cc @@ -91,15 +91,10 @@ Status SegmentationContext::ReprocessChanged(InvalidationSet modified) { modified.glyphs.union_set(analysis_modified_gids); } } else { -#ifndef HB_DEPEND_API - return absl::InternalError( - "DEP_GRAPH_ONLY mode requires dependency graph support."); -#else modified.glyphs.union_set( (*dependency_closure_) ->SegmentsToAffectedGlyphs({modified.base_segment})); TransferDependencyGraphGlyphConditions(modified.glyphs); -#endif } return GroupGlyphs(modified.glyphs, modified.segments); @@ -113,15 +108,10 @@ Status SegmentationContext::ReprocessAll() { TRY(ReprocessSegment(segment_index)); } } else { -#ifndef HB_DEPEND_API - return absl::InternalError( - "DEP_GRAPH_ONLY mode requires dependency graph support."); -#else // Pull conditions directly out of the dep graph instead of running closure // processing. TransferDependencyGraphGlyphConditions( segmentation_info_->NonInitFontGlyphs()); -#endif } return GroupGlyphs(SegmentationInfo().NonInitFontGlyphs(), {}); @@ -231,16 +221,11 @@ Status SegmentationContext::ReassignInitSubset(SubsetDefinition new_def) { TRY(ReprocessSegment(segment_index)); } } else { -#ifndef HB_DEPEND_API - return absl::InternalError( - "DEP_GRAPH_ONLY mode requires dependency graph support."); -#else GlyphSet gids; for (segment_index_t s : segments_to_reprocess) { gids.union_set((*dependency_closure_)->SegmentsToAffectedGlyphs({s})); } TransferDependencyGraphGlyphConditions(gids); -#endif } // the groupings can be incrementally recomputed by looking at what conditions @@ -363,7 +348,6 @@ SegmentationContext::InitializeSegmentationContext( void SegmentationContext::TransferDependencyGraphGlyphConditions( const GlyphSet& gids) { -#ifdef HB_DEPEND_API const auto& conditions = (*dependency_closure_)->AllGlyphConditions(); for (glyph_id_t g : gids) { ActivationCondition condition = conditions.at(g); @@ -384,7 +368,6 @@ void SegmentationContext::TransferDependencyGraphGlyphConditions( glyph_condition_set.SetCondition(g, condition); } inert_segments_ = (*dependency_closure_)->InertSegments(); -#endif } } // namespace ift::encoder \ No newline at end of file diff --git a/third_party/harfbuzz.BUILD b/third_party/harfbuzz.BUILD index a17ac966..6b6c2908 100644 --- a/third_party/harfbuzz.BUILD +++ b/third_party/harfbuzz.BUILD @@ -120,12 +120,7 @@ cc_library( }), defines = [ "HB_EXPERIMENTAL_API", - ] + select({ - "@ift_encoder//:use_harfbuzz_dep_graph": [ - "HB_DEPEND_API", - ], - "//conditions:default": [], - }), + ], includes = [ "src", "src/hb-ucdn", From 0d53154e2399ac6a0e41936b0adea1b8617e08b5 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 30 Jul 2026 19:41:35 +0000 Subject: [PATCH 3/3] Update closure phases specification. MATH closure comes before GSUB now. Matches changes to harfbuzz in: https://github.com/harfbuzz/harfbuzz/pull/6032 --- ift/dep_graph/dependency_graph.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ift/dep_graph/dependency_graph.h b/ift/dep_graph/dependency_graph.h index f4c23edc..1db2ff9a 100644 --- a/ift/dep_graph/dependency_graph.h +++ b/ift/dep_graph/dependency_graph.h @@ -46,15 +46,18 @@ class DependencyGraph { // _populate_gids_to_retain() from // https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-subset-plan.cc#L439 static constexpr hb_tag_t kClosurePhaseTable[] = { - common::FontHelper::kCmap, common::FontHelper::kGSUB, - common::FontHelper::kMATH, common::FontHelper::kCOLR, - common::FontHelper::kGlyf, common::FontHelper::kCFF, + /* 0 */ common::FontHelper::kCmap, + /* 1 */ common::FontHelper::kMATH, + /* 2 */ common::FontHelper::kGSUB, + /* 3 */ common::FontHelper::kCOLR, + /* 4 */ common::FontHelper::kGlyf, + /* 5 */ common::FontHelper::kCFF, }; static constexpr hb_tag_t kClosurePhaseNodeFilter[] = { /* cmap */ 0xFFFFFFFF, - /* GSUB */ Node::NodeType::GLYPH, /* MATH */ Node::NodeType::GLYPH, + /* GSUB */ Node::NodeType::GLYPH, /* COLR */ Node::NodeType::GLYPH, /* glyf */ Node::NodeType::GLYPH, /* CFF */ Node::NodeType::GLYPH, @@ -62,10 +65,10 @@ class DependencyGraph { static constexpr hb_tag_t kClosurePhaseStartNodes[] = { /* cmap */ Node::NodeType::SEGMENT, + /* MATH */ Node::NodeType::GLYPH, // For GSUB we also need to consider reached features as starting nodes // since those have outgoing GSUB edges. /* GSUB */ Node::NodeType::GLYPH | Node::NodeType::FEATURE, - /* MATH */ Node::NodeType::GLYPH, /* COLR */ Node::NodeType::GLYPH, /* glyf */ Node::NodeType::GLYPH, /* CFF */ Node::NodeType::GLYPH,