Consolidate usages of hb_set_unique_ptr, flat_hash_set, and btree_set to IntSet.

brotli/brotli_font_diff.cc

@@ -6,14 +6,14 @@
 #include "brotli/hmtx_differ.h"
 #include "brotli/loca_differ.h"
 #include "brotli/table_range.h"
-#include "common/hb_set_unique_ptr.h"
+#include "common/int_set.h"
 
 namespace brotli {
 
 using absl::Span;
 using absl::Status;
 using common::FontData;
-using common::hb_set_unique_ptr;
+using common::IntSet;
 
 static bool HasTable(hb_face_t* face, hb_tag_t tag) {
   hb_blob_t* table = hb_face_reference_table(face, tag);
@@ -49,7 +49,7 @@ class DiffDriver {
  public:
   DiffDriver(hb_subset_plan_t* base_plan, hb_face_t* base_face,
              hb_subset_plan_t* derived_plan, hb_face_t* derived_face,
-             const hb_set_t* custom_diff_tables, BrotliStream& stream)
+             const IntSet& custom_diff_tables, BrotliStream& stream)
       : out(stream),
         base_new_to_old(hb_subset_plan_new_to_old_glyph_mapping(base_plan)),
         derived_old_to_new(
@@ -77,8 +77,7 @@ class DiffDriver {
     constexpr hb_tag_t LOCA = HB_TAG('l', 'o', 'c', 'a');
     constexpr hb_tag_t GLYF = HB_TAG('g', 'l', 'y', 'f');
 
-    hb_tag_t tag = HB_SET_VALUE_INVALID;
-    while (hb_set_next(custom_diff_tables, &tag)) {
+    for (hb_tag_t tag : custom_diff_tables) {
       switch (tag) {
         case HMTX:
           if (HasTable(base_face, derived_face, HMTX) &&
@@ -229,8 +228,8 @@ class DiffDriver {
   }
 };
 
-void BrotliFontDiff::SortForDiff(const hb_set_t* immutable_tables,
-                                 const hb_set_t* custom_diff_tables,
+void BrotliFontDiff::SortForDiff(const IntSet& immutable_tables,
+                                 const IntSet& custom_diff_tables,
                                  const hb_face_t* original_face,
                                  hb_face_t* face_builder) {
   // Place generic diff tables,
@@ -244,21 +243,19 @@ void BrotliFontDiff::SortForDiff(const hb_set_t* immutable_tables,
           num_tables)) {
     for (unsigned i = 0; i < num_tables; ++i) {
       hb_tag_t tag = table_tags[i];
-      if (!hb_set_has(immutable_tables, tag) &&
-          !hb_set_has(custom_diff_tables, tag)) {
+      if (!immutable_tables.contains(tag) &&
+          !custom_diff_tables.contains(tag)) {
         table_order.push_back(tag);
       }
     }
     offset += num_tables;
   }
 
-  hb_codepoint_t tag = HB_SET_VALUE_INVALID;
-  while (hb_set_next(immutable_tables, &tag)) {
+  for (hb_codepoint_t tag : immutable_tables) {
     table_order.push_back(tag);
   }
 
-  tag = HB_SET_VALUE_INVALID;
-  while (hb_set_next(custom_diff_tables, &tag)) {
+  for (hb_codepoint_t tag : custom_diff_tables) {
     table_order.push_back(tag);
   }
 
@@ -287,15 +284,13 @@ Status BrotliFontDiff::Diff(hb_subset_plan_t* base_plan, hb_blob_t* base,
   unsigned base_end_offset = 0;
 
   DiffDriver diff_driver(base_plan, base_face, derived_plan, derived_face,
-                         custom_diff_tables_.get(), out);
+                         custom_diff_tables_, out);
 
-  const hb_set_t* tag_sets[] = {immutable_tables_.get(),
-                                custom_diff_tables_.get()};
+  const IntSet* tag_sets[] = {&immutable_tables_, &custom_diff_tables_};
   unsigned base_region_sizes[] = {0, 0};
   unsigned i = 0;
-  for (const hb_set_t* set : tag_sets) {
-    hb_tag_t tag = HB_SET_VALUE_INVALID;
-    while (hb_set_next(set, &tag)) {
+  for (const IntSet* set : tag_sets) {
+    for (hb_tag_t tag : *set) {
       if (!HasTable(derived_face, tag)) {
         continue;
       }

brotli/brotli_font_diff.h

@@ -3,7 +3,7 @@
 
 #include "absl/status/status.h"
 #include "common/font_data.h"
-#include "common/hb_set_unique_ptr.h"
+#include "common/int_set.h"
 #include "hb-subset.h"
 
 namespace brotli {
@@ -16,23 +16,23 @@ class BrotliFontDiff {
  public:
   // Sorts the tables in face_builder into the order expected by the font
   // differ.
-  static void SortForDiff(const hb_set_t* immutable_tables,
-                          const hb_set_t* custom_diff_tables,
+  static void SortForDiff(const common::IntSet& immutable_tables,
+                          const common::IntSet& custom_diff_tables,
                           const hb_face_t* original_face,
                           hb_face_t* face_builder /* IN/OUT */);
 
-  BrotliFontDiff(const hb_set_t* immutable_tables,
-                 const hb_set_t* custom_diff_tables)
-      : immutable_tables_(hb_set_copy(immutable_tables), &hb_set_destroy),
-        custom_diff_tables_(hb_set_copy(custom_diff_tables), &hb_set_destroy) {}
+  BrotliFontDiff(const common::IntSet& immutable_tables,
+                 const common::IntSet& custom_diff_tables)
+      : immutable_tables_(immutable_tables),
+        custom_diff_tables_(custom_diff_tables) {}
 
   absl::Status Diff(hb_subset_plan_t* base_plan, hb_blob_t* base,
                     hb_subset_plan_t* derived_plan, hb_blob_t* derived,
                     common::FontData* patch) const;
 
  private:
-  common::hb_set_unique_ptr immutable_tables_;
-  common::hb_set_unique_ptr custom_diff_tables_;
+  common::IntSet immutable_tables_;
+  common::IntSet custom_diff_tables_;
 };
 
 }  // namespace brotli

brotli/brotli_font_diff_test.cc

@@ -2,7 +2,7 @@
 
 #include "absl/types/span.h"
 #include "common/brotli_binary_patch.h"
-#include "common/hb_set_unique_ptr.h"
+#include "common/int_set.h"
 #include "gtest/gtest.h"
 #include "hb-subset.h"
 
@@ -12,8 +12,7 @@ using absl::Span;
 using absl::Status;
 using common::BrotliBinaryPatch;
 using common::FontData;
-using common::hb_set_unique_ptr;
-using common::make_hb_set;
+using common::IntSet;
 
 const std::string kTestDataDir = "common/testdata/";
 
@@ -47,10 +46,10 @@ class BrotliFontDiffTest : public ::testing::Test {
 
     input = hb_subset_input_create_or_fail();
 
-    immutable_tables = make_hb_set();
+    immutable_tables = IntSet{};
     custom_tables =
-        make_hb_set(4, HB_TAG('g', 'l', 'y', 'f'), HB_TAG('l', 'o', 'c', 'a'),
-                    HB_TAG('h', 'm', 't', 'x'), HB_TAG('v', 'm', 't', 'x'));
+        IntSet{HB_TAG('g', 'l', 'y', 'f'), HB_TAG('l', 'o', 'c', 'a'),
+               HB_TAG('h', 'm', 't', 'x'), HB_TAG('v', 'm', 't', 'x')};
   }
 
   void TearDown() override {
@@ -72,12 +71,11 @@ class BrotliFontDiffTest : public ::testing::Test {
   }
 
   void SortTables(hb_face_t* face, hb_face_t* subset) {
-    BrotliFontDiff::SortForDiff(immutable_tables.get(), custom_tables.get(),
-                                face, subset);
+    BrotliFontDiff::SortForDiff(immutable_tables, custom_tables, face, subset);
   }
 
-  hb_set_unique_ptr immutable_tables = make_hb_set();
-  hb_set_unique_ptr custom_tables = make_hb_set();
+  IntSet immutable_tables;
+  IntSet custom_tables;
 
   hb_face_t* roboto;
   hb_face_t* noto_sans_jp;
@@ -101,7 +99,7 @@ TEST_F(BrotliFontDiffTest, Diff) {
   FontData derived(derived_face);
   ASSERT_TRUE(derived_plan);
 
-  BrotliFontDiff differ(immutable_tables.get(), custom_tables.get());
+  BrotliFontDiff differ(immutable_tables, custom_tables);
   FontData patch;
   ASSERT_EQ(
       differ.Diff(base_plan, base_blob, derived_plan, derived_blob, &patch),
@@ -136,7 +134,7 @@ TEST_F(BrotliFontDiffTest, DiffRetainGids) {
   FontData derived(derived_face);
   ASSERT_TRUE(derived_plan);
 
-  BrotliFontDiff differ(immutable_tables.get(), custom_tables.get());
+  BrotliFontDiff differ(immutable_tables, custom_tables);
   FontData patch;
   ASSERT_EQ(
       differ.Diff(base_plan, base_blob, derived_plan, derived_blob, &patch),
@@ -175,7 +173,7 @@ TEST_F(BrotliFontDiffTest, LongLoca) {
   FontData derived(derived_face);
   ASSERT_TRUE(derived_plan);
 
-  BrotliFontDiff differ(immutable_tables.get(), custom_tables.get());
+  BrotliFontDiff differ(immutable_tables, custom_tables);
   FontData patch;
   ASSERT_EQ(
       differ.Diff(base_plan, base_blob, derived_plan, derived_blob, &patch),
@@ -213,7 +211,7 @@ TEST_F(BrotliFontDiffTest, ShortToLongLoca) {
   FontData derived(derived_face);
   ASSERT_TRUE(derived_plan);
 
-  BrotliFontDiff differ(immutable_tables.get(), custom_tables.get());
+  BrotliFontDiff differ(immutable_tables, custom_tables);
   FontData patch;
   ASSERT_EQ(
       differ.Diff(base_plan, base_blob, derived_plan, derived_blob, &patch),
@@ -235,8 +233,9 @@ TEST_F(BrotliFontDiffTest, WithImmutableTables) {
              HB_TAG('G', 'S', 'U', 'B'));
   hb_set_add(hb_subset_input_set(input, HB_SUBSET_SETS_NO_SUBSET_TABLE_TAG),
              HB_TAG('G', 'P', 'O', 'S'));
-  hb_set_add(immutable_tables.get(), HB_TAG('G', 'S', 'U', 'B'));
-  hb_set_add(immutable_tables.get(), HB_TAG('G', 'P', 'O', 'S'));
+
+  immutable_tables.insert(HB_TAG('G', 'S', 'U', 'B'));
+  immutable_tables.insert(HB_TAG('G', 'P', 'O', 'S'));
 
   hb_set_add_range(hb_subset_input_unicode_set(input), 0x41, 0x5A);
   hb_subset_plan_t* base_plan = hb_subset_plan_create_or_fail(roboto, input);
@@ -254,7 +253,7 @@ TEST_F(BrotliFontDiffTest, WithImmutableTables) {
   FontData derived(derived_face);
   ASSERT_TRUE(derived_plan);
 
-  BrotliFontDiff differ(immutable_tables.get(), custom_tables.get());
+  BrotliFontDiff differ(immutable_tables, custom_tables);
   FontData patch;
   ASSERT_EQ(
       differ.Diff(base_plan, base_blob, derived_plan, derived_blob, &patch),

common/font_helper.cc

@@ -9,6 +9,7 @@
 #include "common/font_data.h"
 #include "common/hb_set_unique_ptr.h"
 #include "common/indexed_data_reader.h"
+#include "common/int_set.h"
 #include "hb-ot.h"
 #include "hb-subset.h"
 #include "hb.h"
@@ -215,10 +216,9 @@ StatusOr<uint32_t> FontHelper::GvarSharedTupleCount(const hb_face_t* face) {
   return *count;
 }
 
-btree_set<uint32_t> FontHelper::GidsToUnicodes(
-    hb_face_t* face, const btree_set<uint32_t>& gids) {
+CodepointSet FontHelper::GidsToUnicodes(hb_face_t* face, const GlyphSet& gids) {
   auto gid_to_unicode = FontHelper::GidToUnicodeMap(face);
-  btree_set<uint32_t> result;
+  CodepointSet result;
   for (uint32_t gid : gids) {
     auto unicode = gid_to_unicode.find(gid);
     if (unicode != gid_to_unicode.end()) {
@@ -244,17 +244,10 @@ flat_hash_map<uint32_t, uint32_t> FontHelper::GidToUnicodeMap(hb_face_t* face) {
   return gid_to_unicode;
 }
 
-btree_set<uint32_t> FontHelper::ToCodepointsSet(hb_face_t* face) {
+CodepointSet FontHelper::ToCodepointsSet(hb_face_t* face) {
   hb_set_unique_ptr codepoints = make_hb_set();
   hb_face_collect_unicodes(face, codepoints.get());
-
-  btree_set<uint32_t> result;
-  hb_codepoint_t cp = HB_SET_VALUE_INVALID;
-  while (hb_set_next(codepoints.get(), &cp)) {
-    result.insert(cp);
-  }
-
-  return result;
+  return CodepointSet(codepoints);
 }
 
 absl::flat_hash_set<hb_tag_t> FontHelper::GetTags(hb_face_t* face) {

common/font_helper.h

@@ -12,6 +12,7 @@
 #include "absl/strings/string_view.h"
 #include "common/axis_range.h"
 #include "common/font_data.h"
+#include "common/int_set.h"
 #include "hb.h"
 
 namespace common {
@@ -192,10 +193,9 @@ class FontHelper {
   static absl::flat_hash_map<uint32_t, uint32_t> GidToUnicodeMap(
       hb_face_t* face);
 
-  static absl::btree_set<uint32_t> GidsToUnicodes(
-      hb_face_t* face, const absl::btree_set<uint32_t>& gids);
+  static CodepointSet GidsToUnicodes(hb_face_t* face, const GlyphSet& gids);
 
-  static absl::btree_set<uint32_t> ToCodepointsSet(hb_face_t* face);
+  static CodepointSet ToCodepointsSet(hb_face_t* face);
 
   static absl::flat_hash_set<hb_tag_t> GetTags(hb_face_t* face);
   static std::vector<hb_tag_t> GetOrderedTags(hb_face_t* face);

common/hb_set_unique_ptr.cc

@@ -1,13 +1,9 @@
 #include "common/hb_set_unique_ptr.h"
 
 #include <cstdarg>
-#include <memory>
 
 #include "hb.h"
 
-using absl::btree_set;
-using absl::flat_hash_set;
-
 namespace common {
 
 hb_set_unique_ptr make_hb_set() {
@@ -18,14 +14,6 @@ hb_set_unique_ptr make_hb_set(hb_set_t* set) {
   return hb_set_unique_ptr(set, &hb_set_destroy);
 }
 
-hb_set_unique_ptr make_hb_set(const absl::flat_hash_set<uint32_t>& int_set) {
-  hb_set_unique_ptr out = make_hb_set();
-  for (uint32_t v : int_set) {
-    hb_set_add(out.get(), v);
-  }
-  return out;
-}
-
 hb_set_unique_ptr make_hb_set(int length, ...) {
   hb_set_unique_ptr result = make_hb_set();
   va_list values;
@@ -54,22 +42,4 @@ hb_set_unique_ptr make_hb_set_from_ranges(int number_of_ranges, ...) {
   return result;
 }
 
-flat_hash_set<uint32_t> to_hash_set(const hb_set_t* set) {
-  flat_hash_set<uint32_t> out;
-  hb_codepoint_t v = HB_SET_VALUE_INVALID;
-  while (hb_set_next(set, &v)) {
-    out.insert(v);
-  }
-  return out;
-}
-
-btree_set<uint32_t> to_btree_set(const hb_set_t* set) {
-  btree_set<uint32_t> out;
-  hb_codepoint_t v = HB_SET_VALUE_INVALID;
-  while (hb_set_next(set, &v)) {
-    out.insert(v);
-  }
-  return out;
-}
-
 }  // namespace common

common/hb_set_unique_ptr.h

@@ -3,8 +3,6 @@
 
 #include <memory>
 
-#include "absl/container/btree_set.h"
-#include "absl/container/flat_hash_set.h"
 #include "hb.h"
 
 namespace common {
@@ -16,18 +14,12 @@ hb_set_unique_ptr make_hb_set();
 // Takes ownership of set
 hb_set_unique_ptr make_hb_set(hb_set_t* set);
 
-hb_set_unique_ptr make_hb_set(const absl::flat_hash_set<uint32_t>& int_set);
-
 hb_set_unique_ptr make_hb_set(int length, ...);
 
 hb_set_unique_ptr make_hb_set_from_ranges(int number_of_ranges, ...);
 
 hb_set_unique_ptr make_hb_set(int length, ...);
 
-absl::flat_hash_set<uint32_t> to_hash_set(const hb_set_t* set);
-
-absl::btree_set<uint32_t> to_btree_set(const hb_set_t* set);
-
 }  // namespace common
 
 #endif  // COMMON_HB_SET_UNIQUE_PTR_H_