ZmFont: Store character padding in font file

The size/variant specific character padding should be stored with the font data.
Modify the FontBitmapHeader accordingly and introduce a version field in the FontFileHeader
so we are able to check we have a font file with the right structure associated with that version.

The version field is set to 1 in this changeset.

src/zm_font.cpp

@@ -20,14 +20,16 @@
 #include <cstring>
 #include <fstream>
 
+constexpr uint8 kRequiredZmFntVersion = 1;
+
 constexpr uint8 FontVariant::kMaxNumCodePoints;
 constexpr uint8 FontVariant::kMaxCharHeight;
 constexpr uint8 FontVariant::kMaxCharWidth;
 
-FontVariant::FontVariant() : char_height_(0), char_width_(0), codepoint_count_(0) {}
+FontVariant::FontVariant() : char_height_(0), char_width_(0), char_padding_(0), codepoint_count_(0) {}
 
-FontVariant::FontVariant(uint16 char_height, uint16 char_width, std::vector<uint64> bitmap)
+FontVariant::FontVariant(uint16 char_height, uint16 char_width, uint8 char_padding, std::vector<uint64> bitmap)
-    : char_height_(char_height), char_width_(char_width), bitmap_(std::move(bitmap)) {
+    : char_height_(char_height), char_width_(char_width), char_padding_(char_padding), bitmap_(std::move(bitmap)) {
   if (char_height_ > kMaxCharHeight) {
     throw std::invalid_argument("char_height > kMaxCharHeight");
   }
@@ -61,6 +63,7 @@ std::ifstream &operator>>(std::ifstream &stream, FontBitmapHeader &bm_header) {
 
 std::ifstream &operator>>(std::ifstream &stream, FontFileHeader &header) {
   stream.read(header.magic, sizeof(header.magic));
+  stream.read(reinterpret_cast<char *>(&header.version), sizeof(header.version));
   stream.seekg(sizeof(header.pad), std::ifstream::cur);
 
   for (FontBitmapHeader &bm_header : header.bitmap_header)
@@ -84,7 +87,7 @@ FontLoadError ZmFont::LoadFontFile(const std::string &loc) {
     return FontLoadError::kInvalidFile;
   }
 
-  if (memcmp(file_header.magic, "ZMFNT", 5) != 0) {
+  if (memcmp(file_header.magic, "ZMFNT", 5) != 0 || file_header.version != kRequiredZmFntVersion) {
     return FontLoadError::kInvalidFile;
   }
 
@@ -104,7 +107,7 @@ FontLoadError ZmFont::LoadFontFile(const std::string &loc) {
     font_file.read(reinterpret_cast<char *>(bitmap.data()), static_cast<std::streamsize>(bitmap_bytes));
 
     variants_[i] =
-        {bitmap_header.char_height, bitmap_header.char_width, std::move(bitmap)};
+        {bitmap_header.char_height, bitmap_header.char_width, bitmap_header.char_padding, std::move(bitmap)};
   }
 
   if (font_file.fail()) {

src/zm_font.h

@@ -39,14 +39,16 @@ struct FontBitmapHeader {
   uint16 char_width; // width of every character
   uint32 number_of_code_points; // number of codepoints; max. 255 for now
   uint32 idx; // offset in data where data for the bitmap starts; not used
-  uint32 pad; // padding
+  uint8 char_padding; // padding around characters
+  uint8 pad[3]; // struct padding
 };
 #pragma pack(pop)
 
 #pragma pack(push, 1)
 struct FontFileHeader {
   char magic[6]; // "ZMFNT\0"
-  uint8 pad[2];
+  uint8 version;
+  uint8 pad;
   std::array<FontBitmapHeader, kNumFontSizes> bitmap_header;
 };
 #pragma pack(pop)
@@ -60,10 +62,11 @@ class FontVariant {
   static constexpr uint8 kMaxCharWidth = 64;
 
   FontVariant();
-  FontVariant(uint16 char_height, uint16 char_width, std::vector<uint64> bitmap);
+  FontVariant(uint16 char_height, uint16 char_width, uint8 char_padding, std::vector<uint64> bitmap);
 
   uint16 GetCharHeight() const { return char_height_; }
   uint16 GetCharWidth() const { return char_width_; }
+  uint8 GetCharPadding() const { return char_padding_; }
   uint8 GetCodepointsCount() const { return codepoint_count_; }
 
   // Returns the bitmap of the codepoint `idx`. If `idx` is greater than `GetCodepointsCount`
@@ -73,6 +76,7 @@ class FontVariant {
  private:
   uint16 char_height_;
   uint16 char_width_;
+  uint8 char_padding_;
   uint8 codepoint_count_;
   std::vector<uint64> bitmap_;
 };

src/zm_image.cpp

@@ -2033,7 +2033,7 @@ void Image::Annotate(
           }
 
           while (cp_row != 0) {
-            int column_idx = char_width - __builtin_ctzll(cp_row) + size;
+            int column_idx = char_width - __builtin_ctzll(cp_row) + font_variant.GetCharPadding();
             *(ptr + column_idx) = fg_colour & 0xff;
             cp_row = cp_row & (cp_row - 1);
           }
@@ -2061,7 +2061,7 @@ void Image::Annotate(
           }
 
           while (cp_row != 0) {
-            int column_idx = char_width - __builtin_ctzll(cp_row) + size;
+            int column_idx = char_width - __builtin_ctzll(cp_row) + font_variant.GetCharPadding();
             uint8 *colour_ptr = ptr + (column_idx * bytesPerPixel);
             RED_PTR_RGBA(colour_ptr) = RED_VAL_RGBA(fg_colour);
             GREEN_PTR_RGBA(colour_ptr) = GREEN_VAL_RGBA(fg_colour);
@@ -2087,7 +2087,7 @@ void Image::Annotate(
           }
 
           while (cp_row != 0) {
-            uint32 column_idx = char_width - __builtin_ctzll(cp_row) + size;
+            uint32 column_idx = char_width - __builtin_ctzll(cp_row) + font_variant.GetCharPadding();
             *(ptr + column_idx) = fg_rgb_col;
             cp_row = cp_row & (cp_row - 1);
           }

tests/data/fonts/generate_fonts.py

@@ -6,6 +6,8 @@ GOOD_MAGIC = b"ZMFNT\0"
 BAD_MAGIC = b"ABCDE\0"
 NUM_FONT_SIZES = 4
 
+ZMFNT_VERSION = 1
+
 
 class FontFile:
     def __init__(self, path):
@@ -14,11 +16,12 @@ class FontFile:
     def write_file_header(self, magic):
         with open(self.path, "wb") as f:
             f.write(magic)
-            f.write(struct.pack("BB", 0, 0))  # pad
+            f.write(struct.pack("B", ZMFNT_VERSION))
+            f.write(struct.pack("B", 0))  # pad
 
-    def write_bm_header(self, height, width, cp_count, idx):
+    def write_bm_header(self, height, width, cp_count, idx, padding):
         with open(self.path, "ab") as f:
-            f.write(struct.pack("HHIII", height, width, cp_count, idx, 0))
+            f.write(struct.pack("HHIIBBBB", height, width, cp_count, idx, padding, 0, 0, 0))
 
     def write_codepoints(self, value, height, count):
         with open(self.path, "ab") as f:
@@ -32,14 +35,14 @@ font.write_file_header(BAD_MAGIC)
 # height, width and number of codepoints out of bounds
 font = FontFile("02_variant_invalid.zmfnt")
 font.write_file_header(GOOD_MAGIC)
-font.write_bm_header(201, 65, 256, 0)
+font.write_bm_header(201, 65, 256, 0, 2)
 
 # mismatch between number of codepoints specified in header and actually stored ones
 font = FontFile("03_missing_cps.zmfnt")
 font.write_file_header(GOOD_MAGIC)
 offs = 0
 for _ in range(NUM_FONT_SIZES):
-    font.write_bm_header(10, 10, 10, offs)
+    font.write_bm_header(10, 10, 10, offs, 2)
     offs += 10 * 10
 for _ in range(NUM_FONT_SIZES):
     font.write_codepoints(1, 10, 9)
@@ -48,7 +51,7 @@ font = FontFile("04_valid.zmfnt")
 font.write_file_header(GOOD_MAGIC)
 offs = 0
 for i in range(NUM_FONT_SIZES):
-    font.write_bm_header(10 + i, 10 + i, 10, offs)
+    font.write_bm_header(10 + i, 10 + i, 10, offs, 2)
     offs += 10 * (10 + i)
 for i in range(NUM_FONT_SIZES):
     font.write_codepoints(i, 10 + i, 10)

tests/zm_font.cpp

@@ -35,9 +35,10 @@ TEST_CASE("FontVariant: construction") {
   SECTION("values in range") {
     constexpr uint8 height = 10;
     constexpr uint8 width = 10;
+    constexpr uint8 padding = 2;
     std::vector<uint64> bitmap(FontVariant::kMaxNumCodePoints * height);
 
-    REQUIRE_NOTHROW(variant = FontVariant(height, width, bitmap));
+    REQUIRE_NOTHROW(variant = FontVariant(height, width, padding, bitmap));
 
     REQUIRE(variant.GetCharHeight() == height);
     REQUIRE(variant.GetCharWidth() == width);
@@ -47,31 +48,35 @@ TEST_CASE("FontVariant: construction") {
   SECTION("height out of range") {
     constexpr uint8 height = FontVariant::kMaxCharHeight + 1;
     constexpr uint8 width = 10;
+    constexpr uint8 padding = 2;
     std::vector<uint64> bitmap(FontVariant::kMaxNumCodePoints * height);
 
-    REQUIRE_THROWS(variant = FontVariant(height, width, bitmap));
+    REQUIRE_THROWS(variant = FontVariant(height, width, padding, bitmap));
   }
 
   SECTION("width out of range") {
     constexpr uint8 height = 10;
     constexpr uint8 width = FontVariant::kMaxCharWidth + 1;
+    constexpr uint8 padding = 2;
     std::vector<uint64> bitmap(FontVariant::kMaxNumCodePoints * height);
 
-    REQUIRE_THROWS(variant = FontVariant(height, width, bitmap));
+    REQUIRE_THROWS(variant = FontVariant(height, width, padding, bitmap));
   }
 
   SECTION("bitmap of wrong size") {
     constexpr uint8 height = 10;
     constexpr uint8 width = 10;
+    constexpr uint8 padding = 2;
     std::vector<uint64> bitmap(FontVariant::kMaxNumCodePoints * height + 1);
 
-    REQUIRE_THROWS(variant = FontVariant(height, width, bitmap));
+    REQUIRE_THROWS(variant = FontVariant(height, width, padding, bitmap));
   }
 }
 
 TEST_CASE("FontVariant: GetCodepoint") {
   constexpr uint8 height = 10;
   constexpr uint8 width = 10;
+  constexpr uint8 padding = 2;
   std::vector<uint64> bitmap(FontVariant::kMaxNumCodePoints * height);
 
   // fill bitmap for each codepoint alternating with 1 and std::numeric_limits<uint64>::max()
@@ -89,7 +94,7 @@ TEST_CASE("FontVariant: GetCodepoint") {
                   }
                 });
 
-  FontVariant variant(height, width, bitmap);
+  FontVariant variant(height, width, padding, bitmap);
   nonstd::span<const uint64> cp;
 
   SECTION("in bounds") {