diff --git a/common/font_helper_test.cc b/common/font_helper_test.cc index cc7c5889..90c578e1 100644 --- a/common/font_helper_test.cc +++ b/common/font_helper_test.cc @@ -356,9 +356,9 @@ TEST_F(FontHelperTest, GvarData) { data = FontHelper::GvarData(roboto_vf.get(), 5); ASSERT_TRUE(data.ok()) << data.status(); - ASSERT_EQ(data->size(), 250); + ASSERT_EQ(data->size(), 252); const uint8_t expected[11] = {0x80, 0x06, 0x00, 0x2c, 0x00, 0x2a, - 0x00, 0x02, 0x00, 0x26, 0x00}; + 0x00, 0x04, 0x00, 0x26, 0x00}; string_view expected_str((const char*)expected, 11); ASSERT_EQ(data->substr(0, 11), expected_str); } @@ -448,7 +448,7 @@ TEST_F(FontHelperTest, GvarSharedTupleCount) { } TEST_F(FontHelperTest, GvarData_NotFound) { - auto data = FontHelper::GvarData(roboto_vf.get(), 1300); + auto data = FontHelper::GvarData(roboto_vf.get(), 1400); ASSERT_TRUE(absl::IsNotFound(data.status())) << data.status(); } diff --git a/common/testdata/Roboto[wdth,wght].ttf b/common/testdata/Roboto[wdth,wght].ttf index 175c7b7e..1d3b2903 100644 Binary files a/common/testdata/Roboto[wdth,wght].ttf and b/common/testdata/Roboto[wdth,wght].ttf differ diff --git a/ift/dep_graph/dependency_graph.cc b/ift/dep_graph/dependency_graph.cc index 15305790..dce296b9 100644 --- a/ift/dep_graph/dependency_graph.cc +++ b/ift/dep_graph/dependency_graph.cc @@ -113,21 +113,24 @@ class PendingEdge { return edge; } - static PendingEdge Gsub(hb_tag_t feature, glyph_id_t gid) { - PendingEdge edge(Node::Glyph(gid), GSUB); + static PendingEdge Gsub(glyph_id_t source_gid, hb_tag_t feature, glyph_id_t dest_gid) { + PendingEdge edge(Node::Glyph(dest_gid), GSUB); + edge.required_glyph_ = source_gid; edge.required_feature_ = feature; return edge; } - static PendingEdge Ligature(hb_tag_t feature, glyph_id_t gid, hb_codepoint_t liga_set_index) { - PendingEdge edge(Node::Glyph(gid), GSUB); + static PendingEdge Ligature(glyph_id_t source_gid, hb_tag_t feature, glyph_id_t dest_gid, hb_codepoint_t liga_set_index) { + PendingEdge edge(Node::Glyph(dest_gid), GSUB); + edge.required_glyph_ = source_gid; edge.required_feature_ = feature; edge.required_liga_set_index_ = liga_set_index; return edge; } - static PendingEdge Context(hb_tag_t feature, glyph_id_t gid, hb_codepoint_t context_set_index) { - PendingEdge edge(Node::Glyph(gid), GSUB); + static PendingEdge Context(glyph_id_t source_gid, hb_tag_t feature, glyph_id_t dest_gid, hb_codepoint_t context_set_index) { + PendingEdge edge(Node::Glyph(dest_gid), GSUB); + edge.required_glyph_ = source_gid; edge.required_feature_ = feature; edge.required_context_set_index_ = context_set_index; return edge; @@ -146,6 +149,10 @@ class PendingEdge { const flat_hash_set& reached_features ) const { + if (required_glyph_.has_value() && !reached_glyphs.contains(*required_glyph_)) { + return false; + } + if (required_codepoints_.has_value() && (!reached_unicodes.contains(required_codepoints_->first) || !reached_unicodes.contains(required_codepoints_->second))) { @@ -176,6 +183,7 @@ class PendingEdge { Node dest_; hb_tag_t table_tag_; + std::optional required_glyph_ = std::nullopt; std::optional required_feature_ = std::nullopt; std::optional required_liga_set_index_ = std::nullopt; std::optional required_context_set_index_ = std::nullopt; @@ -330,31 +338,31 @@ class TraversalContext { return TraverseEdgeTo(dest, edge, cmap); } - Status TraverseGsubEdgeTo(glyph_id_t gid, hb_tag_t feature) { - PendingEdge edge = PendingEdge::Gsub(feature, gid); - Node dest = Node::Glyph(gid); + Status TraverseGsubEdgeTo(glyph_id_t source_gid, glyph_id_t dest_gid, hb_tag_t feature) { + PendingEdge edge = PendingEdge::Gsub(source_gid, feature, dest_gid); + Node dest = Node::Glyph(dest_gid); return TraverseEdgeTo(dest, edge, GSUB); } - Status TraverseContextualEdgeTo(glyph_id_t gid, hb_tag_t feature, hb_codepoint_t context_set) { + Status TraverseContextualEdgeTo(glyph_id_t source_gid, glyph_id_t dest_gid, hb_tag_t feature, hb_codepoint_t context_set) { if (!TRY(ContextSetSatisfied(depend, context_set, *full_closure))) { // Not possible for this edge to be activated so it can be ignored. return absl::OkStatus(); } - PendingEdge edge = PendingEdge::Context(feature, gid, context_set); - Node dest = Node::Glyph(gid); + PendingEdge edge = PendingEdge::Context(source_gid, feature, dest_gid, context_set); + Node dest = Node::Glyph(dest_gid); return TraverseEdgeTo(dest, edge, GSUB); } - Status TraverseLigatureEdgeTo(glyph_id_t gid, hb_tag_t feature, hb_codepoint_t liga_set_index) { + Status TraverseLigatureEdgeTo(glyph_id_t source_gid, glyph_id_t dest_gid, hb_tag_t feature, hb_codepoint_t liga_set_index) { if (!TRY(LigaSetSatisfied(depend, liga_set_index, *full_closure))) { // Not possible for this edge to be activated so it can be ignored. return absl::OkStatus(); } - PendingEdge edge = PendingEdge::Ligature(feature, gid, liga_set_index); - Node dest = Node::Glyph(gid); + PendingEdge edge = PendingEdge::Ligature(source_gid, feature, dest_gid, liga_set_index); + Node dest = Node::Glyph(dest_gid); return TraverseEdgeTo(dest, edge, GSUB); } @@ -501,11 +509,13 @@ StatusOr DependencyGraph::TraverseGraph(TraversalContext* context) co HandleSegmentOutgoingEdges(next->Id(), context); } + if (next->IsFeature()) { + TRYV(HandleFeatureOutgoingEdges(next->Id(), context)); + } + if (next->IsInitFont()) { HandleSubsetDefinitionOutgoingEdges(segmentation_info_->InitFontSegment(), context); } - - // Features don't have any outgoing edges } if (context->HasPendingEdges()) { @@ -627,6 +637,14 @@ Status DependencyGraph::ClosureSubTraversal( start_nodes.insert(Node::Glyph(gid)); } + if (table == GSUB) { + // For GSUB we also need to consider reached features as starting nodes + // since those have outgoing GSUB edges. + for (hb_tag_t feature : traversal_full.ReachedLayoutFeatures()) { + start_nodes.insert(Node::Feature(feature)); + } + } + TraversalContext context = *base_context; context.SetStartNodes(start_nodes); context.table_filter = {table}; @@ -680,13 +698,7 @@ Status DependencyGraph::HandleGlyphOutgoingEdges( &dep_gid, &layout_tag, &ligature_set, &context_set, nullptr /* flags */)) { Node dest = Node::Glyph(dep_gid); if (table_tag == HB_TAG('G', 'S', 'U', 'B')) { - if (context_set != HB_CODEPOINT_INVALID) { - TRYV(context->TraverseContextualEdgeTo(dep_gid, layout_tag, context_set)); - } else if (ligature_set != HB_CODEPOINT_INVALID) { - TRYV(context->TraverseLigatureEdgeTo(dep_gid, layout_tag, ligature_set)); - } else { - TRYV(context->TraverseGsubEdgeTo(dep_gid, layout_tag)); - } + TRYV(HandleGsubGlyphOutgoingEdges(gid, dep_gid, layout_tag, ligature_set, context_set, context)); continue; } @@ -702,6 +714,44 @@ Status DependencyGraph::HandleGlyphOutgoingEdges( return absl::OkStatus(); } +Status DependencyGraph::HandleGsubGlyphOutgoingEdges( + glyph_id_t source_gid, + glyph_id_t dest_gid, + hb_tag_t layout_tag, + hb_codepoint_t ligature_set, + hb_codepoint_t context_set, + TraversalContext* context +) const { + if (context_set != HB_CODEPOINT_INVALID) { + return context->TraverseContextualEdgeTo(source_gid, dest_gid, layout_tag, context_set); + } else if (ligature_set != HB_CODEPOINT_INVALID) { + return context->TraverseLigatureEdgeTo(source_gid, dest_gid, layout_tag, ligature_set); + } else { + return context->TraverseGsubEdgeTo(source_gid, dest_gid, layout_tag); + } +} + +Status DependencyGraph::HandleFeatureOutgoingEdges( + hb_tag_t feature_tag, + TraversalContext* context +) const { + if (!context->table_filter.contains(GSUB)) { + // All feature edges are GSUB edges so we can skip + // this if GSUB is being filtered out. + return absl::OkStatus(); + } + + auto edges = layout_fature_implied_edges_.find(feature_tag); + if (edges == layout_fature_implied_edges_.end()) { + // No outgoing edges + return absl::OkStatus(); + } + for (const auto& edge : edges->second) { + TRYV(HandleGsubGlyphOutgoingEdges(edge.source_gid, edge.dest_gid, feature_tag, edge.ligature_set, edge.context_set, context)); + } + return absl::OkStatus(); +} + void DependencyGraph::HandleSegmentOutgoingEdges( segment_index_t id, TraversalContext* context @@ -827,4 +877,32 @@ flat_hash_map> DependencyGraph::ComputeFeatureEdges() const { + flat_hash_map> edges; + + 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 != GSUB || layout_tag == HB_CODEPOINT_INVALID) { + continue; + } + + edges[layout_tag].insert(LayoutFeatureEdge { + .source_gid = gid, + .dest_gid = dest_gid, + .ligature_set = ligature_set, + .context_set = context_set, + }); + } + } + + return edges; +} + } // namespace ift::dep_graph \ No newline at end of file diff --git a/ift/dep_graph/dependency_graph.h b/ift/dep_graph/dependency_graph.h index 717d18d8..003626ed 100644 --- a/ift/dep_graph/dependency_graph.h +++ b/ift/dep_graph/dependency_graph.h @@ -79,7 +79,9 @@ class DependencyGraph { full_feature_set_(full_feature_set), unicode_to_gid_(UnicodeToGid(face)), dependency_graph_(depend, &hb_depend_destroy), - variation_selector_implied_edges_(ComputeUVSEdges()) {} + variation_selector_implied_edges_(ComputeUVSEdges()), + layout_fature_implied_edges_(ComputeFeatureEdges()) + {} absl::StatusOr TraverseGraph( TraversalContext* context @@ -100,6 +102,20 @@ class DependencyGraph { TraversalContext* context ) const; + absl::Status HandleGsubGlyphOutgoingEdges( + encoder::glyph_id_t source_gid, + encoder::glyph_id_t dest_gid, + hb_tag_t layout_tag, + hb_codepoint_t ligature_set, + hb_codepoint_t context_set, + TraversalContext* context + ) const; + + absl::Status HandleFeatureOutgoingEdges( + hb_tag_t feature_tag, + TraversalContext* context + ) const; + void HandleSegmentOutgoingEdges( encoder::segment_index_t id, TraversalContext* context @@ -136,9 +152,38 @@ class DependencyGraph { hb_codepoint_t gid; }; + struct LayoutFeatureEdge { + hb_codepoint_t source_gid; + hb_codepoint_t dest_gid; + hb_codepoint_t ligature_set; + hb_codepoint_t context_set; + + bool operator==(const LayoutFeatureEdge& other) const { + return source_gid == other.dest_gid && + dest_gid == other.dest_gid && + ligature_set == other.ligature_set && + context_set == other.context_set; + } + + bool operator<(const LayoutFeatureEdge& other) const { + if (source_gid != other.source_gid) { + return source_gid < other.source_gid; + } + if (dest_gid != other.dest_gid) { + return dest_gid < other.dest_gid; + } + if (ligature_set != other.ligature_set) { + return ligature_set < other.ligature_set; + } + return context_set < other.context_set; + } + }; + absl::flat_hash_map> ComputeUVSEdges() const; + absl::flat_hash_map> ComputeFeatureEdges() const; absl::flat_hash_map> variation_selector_implied_edges_; + absl::flat_hash_map> layout_fature_implied_edges_; }; } // namespace ift::dep_graph diff --git a/ift/dep_graph/dependency_graph_test.cc b/ift/dep_graph/dependency_graph_test.cc index 5426eaee..d7c09d50 100644 --- a/ift/dep_graph/dependency_graph_test.cc +++ b/ift/dep_graph/dependency_graph_test.cc @@ -235,6 +235,86 @@ TEST_F(DependencyGraphTest, IgnoreUnreachable_Liga) { ASSERT_FALSE(traversal.HasPendingEdges()); } +TEST_F(DependencyGraphTest, ImpliedFeatureEdge) { + SubsetDefinition c2sc; + c2sc.feature_tags = {HB_TAG('c', '2', 's', 'c')}; + Reconfigure(WithDefaultFeatures({'A'}), { + {{'B'}, ProbabilityBound::Zero()}, + {c2sc, ProbabilityBound::Zero()}, + }); + + /* ### s0 ### */ + auto traversal = graph.ClosureTraversal({ + Node::Segment(0), + }); + ASSERT_TRUE(traversal.ok()) << traversal.status(); + + ASSERT_EQ(traversal->ReachedGlyphs(), (GlyphSet { + 38, /* B */ + })); + ASSERT_TRUE(traversal->HasPendingEdges()); + + /* ### s1 ### */ + traversal = graph.ClosureTraversal({ + Node::Segment(1), + }); + ASSERT_TRUE(traversal.ok()) << traversal.status(); + + ASSERT_EQ(traversal->ReachedGlyphs(), (GlyphSet { + 563, /* smcap A */ + })); + ASSERT_TRUE(traversal->HasPendingEdges()); + + /* ### s0 + s1 ### */ + traversal = graph.ClosureTraversal({ + Node::Segment(0), + Node::Segment(1), + }); + ASSERT_TRUE(traversal.ok()) << traversal.status(); + + ASSERT_EQ(traversal->ReachedGlyphs(), (GlyphSet { + 38, /* B */ + 562, /* smcap B */ + 563, /* smcap A */ + })); + ASSERT_FALSE(traversal->HasPendingEdges()); +} + +TEST_F(DependencyGraphTest, ImpliedFeatureEdge_Liga) { + SubsetDefinition liga; + liga.feature_tags = {HB_TAG('l', 'i', 'g', 'a')}; + Reconfigure({'f'}, { + {{'i'}, ProbabilityBound::Zero()}, + {liga, ProbabilityBound::Zero()}, + }); + + /* s0 constraints not satisfied */ + auto traversal = graph.ClosureTraversal({ + Node::Segment(0), + }); + ASSERT_TRUE(traversal.ok()) << traversal.status(); + + ASSERT_EQ(traversal->ReachedGlyphs(), (GlyphSet { + 77, /* i */ + })); + ASSERT_TRUE(traversal->HasPendingEdges()); + + /* s0 + s1 all constraints satisfied */ + traversal = graph.ClosureTraversal({ + Node::Segment(0), + Node::Segment(1), + }); + ASSERT_TRUE(traversal.ok()) << traversal.status(); + + ASSERT_EQ(traversal->ReachedGlyphs(), (GlyphSet { + /* f is in init */ + 77, /* i */ + 444, /* fi */ + 446, /* ffi */ + })); + ASSERT_FALSE(traversal->HasPendingEdges()); +} + // TODO(garretrieger): // - basic math, CFF, and COLR tests. diff --git a/ift/encoder/dependency_closure.cc b/ift/encoder/dependency_closure.cc index dad47fc5..aa46c0a9 100644 --- a/ift/encoder/dependency_closure.cc +++ b/ift/encoder/dependency_closure.cc @@ -44,20 +44,6 @@ DependencyClosure::AnalysisAccuracy DependencyClosure::TraversalAccuracy(const T return AnalysisAccuracy::INACCURATE; } - for (hb_tag_t tag : traversal.ContextLayoutFeatures()) { - // non init font context features implies conjunction. - if (!init_font_features_.contains(tag)) { - return AnalysisAccuracy::INACCURATE; - } - } - - for (hb_tag_t tag : traversal.ReachedLayoutFeatures()) { - // non init font context features implies conjunction. - if (!init_font_features_.contains(tag)) { - return AnalysisAccuracy::INACCURATE; - } - } - if (traversal.ReachedGlyphs().intersects(context_glyphs_)) { // A glyph which appears in a context may have complicated interactions with other segments // that aren't captured by the direct traversal. diff --git a/ift/encoder/dependency_closure_test.cc b/ift/encoder/dependency_closure_test.cc index 6a55e818..1157f4f6 100644 --- a/ift/encoder/dependency_closure_test.cc +++ b/ift/encoder/dependency_closure_test.cc @@ -2,6 +2,7 @@ #include #include +#include "common/font_helper.h" #include "common/int_set.h" #include "ift/encoder/glyph_closure_cache.h" #include "ift/encoder/init_subset_defaults.h" @@ -30,6 +31,7 @@ class DependencyClosureTest : public ::testing::Test { face(from_file("common/testdata/Roboto-Regular.ttf")), double_nested_face(from_file("common/testdata/double-nested-components.ttf")), noto_sans_jp(from_file("common/testdata/NotoSansJP-Regular.ttf")), + roboto_vf(from_file("common/testdata/Roboto[wdth,wght].ttf")), closure_cache(face.get()), segmentation_info(*RequestedSegmentationInformation::Create(segments, WithDefaultFeatures(), closure_cache, PATCH)), dependency_closure(*DependencyClosure::Create(segmentation_info.get(), face.get())) @@ -138,6 +140,7 @@ class DependencyClosureTest : public ::testing::Test { hb_face_unique_ptr face; hb_face_unique_ptr double_nested_face; hb_face_unique_ptr noto_sans_jp; + hb_face_unique_ptr roboto_vf; GlyphClosureCache closure_cache; std::unique_ptr segmentation_info; std::unique_ptr dependency_closure; @@ -355,7 +358,7 @@ TEST_F(DependencyClosureTest, Rejected_InitFontContext) { ASSERT_TRUE(s.ok()) << s; } -TEST_F(DependencyClosureTest, Rejected_Features) { +TEST_F(DependencyClosureTest, Noop_Features) { SubsetDefinition features; features.feature_tags.insert(HB_TAG('a', 'b', 'c', 'd')); Reconfigure({}, { @@ -366,7 +369,7 @@ TEST_F(DependencyClosureTest, Rejected_Features) { Status s = CompareAnalysis({0}); ASSERT_TRUE(s.ok()) << s; - s = RejectedAnalysis(1); + s = CompareAnalysis({1}); ASSERT_TRUE(s.ok()) << s; } @@ -608,6 +611,30 @@ TEST_F(DependencyClosureTest, SegmentInteractionGroup_WithInitFont) { (SegmentSet {0, 3, 4})); } +TEST_F(DependencyClosureTest, InitFontFeatureConjunction) { + SubsetDefinition init; + init.codepoints = {0x30}; /* zero */ + init.gids = {20}; + + SubsetDefinition s0 {8320}; + SubsetDefinition s1 {}; + s1.feature_tags.insert(HB_TAG('s', 'u', 'b', 's')); + + Reconfigure(roboto_vf.get(), init, { + /* 0 */ {s0, ProbabilityBound::Zero()}, + /* 1 */ {s1, ProbabilityBound::Zero()}, + }); + + Status s = CompareAnalysis({0}); + ASSERT_TRUE(s.ok()) << s; + + s = CompareAnalysis({1}); + ASSERT_TRUE(s.ok()) << s; + + s = CompareAnalysis({0, 1}); + ASSERT_TRUE(s.ok()) << s; +} + } // namespace ift::encoder // TODO(garretrieger): missing tests