Re-write SegmentsThatInteractWith(...) to use extracted node conditions.

Additionally drops a bunch of now uneeded code from DependencyClosure.

Author: garretrieger

ift/encoder/BUILD

@@ -67,8 +67,6 @@ cc_library(
         "merger.cc",
         "merger.h",
         "patch_size_cache.h",
-        "reachability_index.cc",
-        "reachability_index.h",
         "segmentation_context.cc",
         "segmentation_context.h",
     ] + select({
@@ -369,16 +367,3 @@ cc_test(
         "@googletest//:gtest_main",
     ],
 )
-
-cc_test(
-    name = "reachability_index_test",
-    srcs = ["reachability_index_test.cc"],
-    data = [
-        "//ift/common:testdata",
-    ],
-    deps = [
-        ":segmentation_context",
-        "//ift/common",
-        "@googletest//:gtest_main",
-    ],
-)

ift/encoder/dependency_closure.cc

@@ -58,7 +58,6 @@ Status DependencyClosure::InitFontChanged(const SegmentSet& segments) {
   // closure, we need to record the context glyphs from these as they are
   // potential interaction points.
   init_font_context_glyphs_.clear();
-  init_font_features_ = TRY(graph_.InitFontFeatureSet());
   Traversal init_font_traversal = TRY(graph_.ClosureTraversal(
       {Node::InitFont()}, &segmentation_info_->FullClosure(),
       &segmentation_info_->FullDefinition().codepoints, false));
@@ -69,13 +68,6 @@ Status DependencyClosure::InitFontChanged(const SegmentSet& segments) {
     }
   }
 
-  init_font_reachable_glyphs_ = init_font_traversal.ReachedGlyphs();
-  init_font_reachable_glyphs_.subtract(segmentation_info_->InitFontGlyphs());
-
-  // Note: reachability index must be updated after context_glyphs_, init_font_*
-  // sets are populated. since it utilizes those to assess traversal accuracy.
-  TRYV(UpdateReachabilityIndex(segments));
-
   return absl::OkStatus();
 }
 
@@ -87,11 +79,10 @@ Status DependencyClosure::SegmentsMerged(segment_index_t base_segment,
     return InitFontChanged(segments);
   }
 
+  VLOG(1) << "DependencyClosure::SegmentsMerged()";
+
   // Manually update the glyph condition cache.
   UpdateNodeConditionsCache(base_segment, segments);
-
-  VLOG(1) << "DependencyClosure::SegmentsMerged()";
-  TRYV(UpdateReachabilityIndex(segments));
   return absl::OkStatus();
 }
 
@@ -327,13 +318,6 @@ StatusOr<SegmentSet> DependencyClosure::SegmentsThatInteractWith(
 
 StatusOr<SegmentSet> DependencyClosure::SegmentsThatInteractWith(
     const absl::flat_hash_set<dep_graph::Node> nodes) const {
-  // TODO(garretrieger): we can narrow the set by considering context glyphs per
-  // activated glyph instead of just the whole set of context glyphs.
-
-  flat_hash_set<Node> visited;
-  SegmentSet visited_segments;
-  GlyphSet visited_glyphs;
-
   btree_set<Node> to_check;
   to_check.insert(nodes.begin(), nodes.end());
 
@@ -351,127 +335,15 @@ StatusOr<SegmentSet> DependencyClosure::SegmentsThatInteractWith(
     to_check.insert(Node::Feature(f));
   }
 
-  bool init_font_context_added = false;
-
-  while (!to_check.empty()) {
-    Node next = *to_check.begin();
-    to_check.erase(next);
-    visited.insert(next);
-    if (next.IsGlyph()) {
-      visited_glyphs.insert(next.Id());
-    }
-
-    // node may be reachable from the init font.
-    if (!init_font_context_added && next.IsGlyph() &&
-        init_font_reachable_glyphs_.contains(next.Id())) {
-      ReachabilityInitFontAddToCheck(visited_glyphs, to_check);
-      init_font_context_added = true;
-    }
-
-    // now check if any segments can reach it.
-    SegmentSet segments;
-    if (next.IsGlyph()) {
-      segments = unconstrained_reachability_.SegmentsForGlyph(next.Id());
-    } else if (next.IsUnicode()) {
-      segments = unconstrained_reachability_.SegmentsForCodepoint(next.Id());
-    } else if (next.IsFeature()) {
-      segments = unconstrained_reachability_.SegmentsForFeature(next.Id());
-    } else {
-      // ignored node type.
-      continue;
-    }
-
-    if (segments.empty()) {
+  SegmentSet out;
+  for (Node node : to_check) {
+    auto it = node_condition_cache_.find(node);
+    if (it == node_condition_cache_.end()) {
       continue;
     }
-
-    TRYV(ReachabilitySegmentsAddToCheck(segments, visited_glyphs,
-                                        visited_segments, to_check));
-  }
-
-  return visited_segments;
-}
-
-void DependencyClosure::ReachabilityInitFontAddToCheck(
-    const GlyphSet& visited_glyphs, btree_set<Node>& to_check) const {
-  GlyphSet additional = init_font_context_glyphs_;
-  additional.subtract(visited_glyphs);
-  for (glyph_id_t gid : additional) {
-    to_check.insert(Node::Glyph(gid));
+    out.union_set(it->second.TriggeringSegments());
   }
-}
-
-Status DependencyClosure::ReachabilitySegmentsAddToCheck(
-    const SegmentSet& segments, const GlyphSet& visited_glyphs,
-    SegmentSet& visited_segments, btree_set<Node>& to_check) const {
-  for (segment_index_t s : segments) {
-    if (visited_segments.contains(s)) {
-      continue;
-    }
-
-    visited_segments.insert(s);
-
-    GlyphSet additional = context_reachability_.GlyphsForSegment(s);
-
-    additional.subtract(visited_glyphs);
-
-    for (glyph_id_t gid : additional) {
-      to_check.insert(Node::Glyph(gid));
-    }
-  }
-  return absl::OkStatus();
-}
-
-Status DependencyClosure::UpdateReachabilityIndex(SegmentSet segments) {
-  if (reachability_index_valid_) {
-    // If indices have existing data, then we need to ensure prior entries for
-    // the segments to be updated are cleared out.
-    for (segment_index_t s : segments) {
-      if (s >= segmentation_info_->Segments().size()) {
-        break;
-      }
-      ClearReachabilityIndex(s);
-    }
-  } else {
-    // If the index isn't built yet then all segments need to be updated.
-    segments = SegmentSet::all();
-  }
-
-  for (segment_index_t s : segments) {
-    if (s >= segmentation_info_->Segments().size()) {
-      break;
-    }
-    TRYV(UpdateReachabilityIndex(s));
-  }
-
-  reachability_index_valid_ = true;
-  return absl::OkStatus();
-}
-
-Status DependencyClosure::UpdateReachabilityIndex(segment_index_t s) {
-  auto traversal = TRY(graph_.ClosureTraversal({s}, false));
-  for (glyph_id_t g : traversal.ReachedGlyphs()) {
-    unconstrained_reachability_.AddGlyph(s, g);
-  }
-
-  for (hb_codepoint_t cp : traversal.ReachedCodepoints()) {
-    unconstrained_reachability_.AddCodepoint(s, cp);
-  }
-
-  for (hb_tag_t f : traversal.ReachedLayoutFeatures()) {
-    unconstrained_reachability_.AddFeature(s, f);
-  }
-
-  for (glyph_id_t g : traversal.ContextGlyphs()) {
-    context_reachability_.AddGlyph(s, g);
-  }
-
-  return absl::OkStatus();
-}
-
-void DependencyClosure::ClearReachabilityIndex(segment_index_t segment) {
-  unconstrained_reachability_.ClearSegment(segment);
-  context_reachability_.ClearSegment(segment);
+  return out;
 }
 
 flat_hash_map<Node, ActivationCondition>

ift/encoder/dependency_closure.h

@@ -11,7 +11,6 @@
 #include "ift/common/int_set.h"
 #include "ift/common/try.h"
 #include "ift/encoder/activation_condition.h"
-#include "ift/encoder/reachability_index.h"
 #include "ift/encoder/subset_definition.h"
 #include "ift/encoder/types.h"
 
@@ -112,6 +111,8 @@ class DependencyClosure {
   //
   // Utilizes the dependency graph to make the determination, so it's possible
   // that the result may be overestimated.
+  //
+  // TODO XXXX can drop StatusOr<> from these.
   absl::StatusOr<ift::common::SegmentSet> SegmentsThatInteractWith(
       const common::GlyphSet& glyphs) const;
   absl::StatusOr<ift::common::SegmentSet> SegmentsThatInteractWith(
@@ -175,52 +176,25 @@ class DependencyClosure {
   absl::StatusOr<common::SegmentSet> FilterSegments(
       const common::SegmentSet& segments) const;
 
-  void ReachabilityInitFontAddToCheck(
-      const common::GlyphSet& visited_glyphs,
-      absl::btree_set<dep_graph::Node>& to_check) const;
-
-  absl::Status ReachabilitySegmentsAddToCheck(
-      const ift::common::SegmentSet& segments,
-      const common::GlyphSet& visited_glyphs,
-      ift::common::SegmentSet& visited_segments,
-      absl::btree_set<dep_graph::Node>& to_check) const;
-
   absl::Status InitNodeConditionsCache();
   void UpdateNodeConditionsCache(segment_index_t base_segment,
                                  const common::SegmentSet& segments);
   absl::flat_hash_set<dep_graph::Node> SegmentsToAffectedNodeConditions(
       const common::SegmentSet& segments) const;
 
-  absl::Status UpdateReachabilityIndex(ift::common::SegmentSet segments);
-  absl::Status UpdateReachabilityIndex(segment_index_t segment);
-  void ClearReachabilityIndex(segment_index_t segment);
-
   const RequestedSegmentationInformation* segmentation_info_;
   ift::common::hb_face_unique_ptr original_face_;
   dep_graph::DependencyGraph graph_;
 
   ift::common::GlyphSet context_glyphs_;
-  ift::common::GlyphSet init_font_reachable_glyphs_;
   ift::common::GlyphSet init_font_context_glyphs_;
-  absl::flat_hash_set<hb_tag_t> init_font_features_;
 
   absl::flat_hash_map<glyph_id_t, ActivationCondition> glyph_condition_cache_;
   absl::flat_hash_map<dep_graph::Node, ActivationCondition>
       node_condition_cache_;
   absl::flat_hash_map<segment_index_t, absl::flat_hash_set<dep_graph::Node>>
       node_conditions_with_segment_;
 
-  // Reachability indexes: these indexes are used to quickly locate segments
-  // reachable from glyph and features (and in reverse as well).
-  bool reachability_index_valid_ = false;
-
-  // Unconstrained reachability, these are the glyphs/features that can be
-  // reached by a segment if context constraints are not enforced.
-  ReachabilityIndex unconstrained_reachability_;
-
-  // Tracks which context glyphs (for contextual gsub substitutions) can be
-  // reached from a segment.
-  ReachabilityIndex context_reachability_;
 #endif
 
   uint64_t accurate_results_ = 0;