Add charstring offsets when needed in IftTable::AddToFont.

Author: garretrieger

ift/proto/format_2_patch_map_test.cc

@@ -18,30 +18,29 @@ class Format2PatchMapTest : public ::testing::Test {
   Format2PatchMapTest() {}
 };
 
-static std::string HeaderSimple(uint8_t entry_count = 1, uint8_t offset_delta = 0) {
-  std::string part1 {
-    0x02,  // format
-    0x00, 0x00, 0x00,
-    0x00,  // reserved
-    0x00, 0x00, 0x00,
-    0x01, 0x00, 0x00,
-    0x00, 0x02, 0x00,
-    0x00, 0x00, 0x03,
-    0x00, 0x00, 0x00,
-    0x04,                           // compat id
-    0x01,                           // default format = Table Keyed Full
-    0x00, 0x00, (char)entry_count,  // entry count
-    0x00, 0x00, 0x00};
-
-  std::string part2 {
-    0x00, 0x00, 0x00,
-    0x00,        // entry id string data offset
-    0x00, 0x06,  // uri template length
-    0x66, 0x6f, 0x6f,
-    0x2f, 0x24, 0x31,
+static std::string HeaderSimple(uint8_t entry_count = 1,
+                                uint8_t offset_delta = 0) {
+  std::string part1{0x02,  // format
+                    0x00, 0x00, 0x00,
+                    0x00,  // reserved
+                    0x00, 0x00, 0x00,
+                    0x01, 0x00, 0x00,
+                    0x00, 0x02, 0x00,
+                    0x00, 0x00, 0x03,
+                    0x00, 0x00, 0x00,
+                    0x04,  // compat id
+                    0x01,  // default format = Table Keyed Full
+                    0x00, 0x00, (char)entry_count,  // entry count
+                    0x00, 0x00, 0x00};
+
+  std::string part2{
+      0x00, 0x00, 0x00,
+      0x00,        // entry id string data offset
+      0x00, 0x06,  // uri template length
+      0x66, 0x6f, 0x6f, 0x2f, 0x24, 0x31,
   };
 
-  part1 += (char) ((uint8_t) 0x29 + offset_delta);
+  part1 += (char)((uint8_t)0x29 + offset_delta);
   part1 += part2;
   return part1;
 }
@@ -80,9 +79,7 @@ TEST_F(Format2PatchMapTest, Simple_WithCffOffset) {
   auto encoded = Format2PatchMap::Serialize(table, 0x7856, std::nullopt);
   ASSERT_TRUE(encoded.ok()) << encoded.status();
 
-  std::string cff_offset = {
-      0x00, 0x00, 0x78, 0x56
-  };
+  std::string cff_offset = {0x00, 0x00, 0x78, 0x56};
 
   std::string entry_0 = {
       // entry 0
@@ -105,9 +102,7 @@ TEST_F(Format2PatchMapTest, Simple_WithCff2Offset) {
   auto encoded = Format2PatchMap::Serialize(table, std::nullopt, 0x127856);
   ASSERT_TRUE(encoded.ok()) << encoded.status();
 
-  std::string cff2_offset = {
-      0x00, 0x12, 0x78, 0x56
-  };
+  std::string cff2_offset = {0x00, 0x12, 0x78, 0x56};
 
   std::string entry_0 = {
       // entry 0
@@ -131,8 +126,7 @@ TEST_F(Format2PatchMapTest, Simple_WithCffAndCff2Offset) {
   ASSERT_TRUE(encoded.ok()) << encoded.status();
 
   std::string offsets = {
-    0x00, 0x00, 0x01, 0x23,
-    0x00, 0x00, 0x04, 0x56,
+      0x00, 0x00, 0x01, 0x23, 0x00, 0x00, 0x04, 0x56,
   };
 
   std::string entry_0 = {

ift/proto/ift_table.cc

@@ -14,6 +14,7 @@
 #include "common/font_helper.h"
 #include "common/hb_set_unique_ptr.h"
 #include "common/sparse_bit_set.h"
+#include "common/try.h"
 #include "hb.h"
 #include "ift/proto/format_2_patch_map.h"
 
@@ -100,10 +101,11 @@ StatusOr<FontData> IFTTable::AddToFont(
 absl::StatusOr<common::FontData> IFTTable::AddToFont(
     hb_face_t* face, const IFTTable& main,
     std::optional<const IFTTable*> extension) {
-  // TODO XXXXX check for CFF/CFF2 and set offset as needed. Only set on the
-  // main table.
-  auto main_bytes =
-      Format2PatchMap::Serialize(main, std::nullopt, std::nullopt);
+  auto cff_charstrings_offset = TRY(FontHelper::CffCharStringsOffset(face));
+  auto cff2_charstrings_offset = TRY(FontHelper::Cff2CharStringsOffset(face));
+
+  auto main_bytes = Format2PatchMap::Serialize(main, cff_charstrings_offset,
+                                               cff2_charstrings_offset);
   if (!main_bytes.ok()) {
     return main_bytes.status();
   }

ift/proto/ift_table_test.cc

@@ -36,7 +36,8 @@ namespace ift::proto {
 
 class IFTTableTest : public ::testing::Test {
  protected:
-  IFTTableTest() : roboto_ab(make_hb_face(nullptr)) {
+  IFTTableTest()
+      : roboto_ab(make_hb_face(nullptr)), noto_sans_jp(make_hb_face(nullptr)) {
     sample.SetUrlTemplate("fonts/go/here");
     sample.SetId({1, 2, 3, 4});
     auto sc = sample.GetPatchMap().AddEntry({30, 32}, 1, TABLE_KEYED_PARTIAL);
@@ -63,9 +64,14 @@ class IFTTableTest : public ::testing::Test {
     hb_blob_unique_ptr blob = make_hb_blob(
         hb_blob_create_from_file("common/testdata/Roboto-Regular.ab.ttf"));
     roboto_ab = make_hb_face(hb_face_create(blob.get(), 0));
+
+    blob = make_hb_blob(
+        hb_blob_create_from_file("common/testdata/NotoSansJP-Regular.otf"));
+    noto_sans_jp = make_hb_face(hb_face_create(blob.get(), 0));
   }
 
   hb_face_unique_ptr roboto_ab;
+  hb_face_unique_ptr noto_sans_jp;
   IFTTable empty;
   IFTTable sample;
   IFTTable sample_with_extensions;
@@ -131,6 +137,25 @@ TEST_F(IFTTableTest, AddToFont_WithExtension) {
   EXPECT_EQ(original_tag_order, new_tag_order);
 }
 
+TEST_F(IFTTableTest, AddToFont_WithExtensionAndCharStringsOffset) {
+  auto font =
+      IFTTable::AddToFont(noto_sans_jp.get(), sample, &sample_with_extensions);
+  ASSERT_TRUE(font.ok()) << font.status();
+  hb_face_unique_ptr face = font->face();
+
+  FontData ift_table =
+      FontHelper::TableData(face.get(), HB_TAG('I', 'F', 'T', ' '));
+  FontData iftx_table =
+      FontHelper::TableData(face.get(), HB_TAG('I', 'F', 'T', 'X'));
+
+  FontData expected_ift(
+      *Format2PatchMap::Serialize(sample, 0xa7ed, std::nullopt));
+  FontData expected_iftx(*Format2PatchMap::Serialize(
+      sample_with_extensions, std::nullopt, std::nullopt));
+  ASSERT_EQ(ift_table, expected_ift);
+  ASSERT_EQ(iftx_table, expected_iftx);
+}
+
 TEST_F(IFTTableTest, GetId) { ASSERT_EQ(sample.GetId(), CompatId(1, 2, 3, 4)); }
 
 TEST_F(IFTTableTest, GetId_None) { ASSERT_EQ(empty.GetId(), CompatId()); }