From de2cabb8970a84cad312e1a0edc0249078978ceb Mon Sep 17 00:00:00 2001
From: Khaled Hosny <khaledhosny@eglug.org>
Date: Fri, 10 Apr 2015 22:05:04 +0200
Subject: [PATCH] Fix simple glyph bounding box handling

Per the spec[1], the bounding box should be calculated and if it differs
from the one stored on the font, the original should be written
otherwise it should be omitted. On the other hand, the bounding box of
composite glyphs should be always written.

Fixes #12.

1. http://dev.w3.org/webfonts/WOFF2/spec/#conform-mustCalculateOmitBBoxValues
---
 src/normalize.cc | 23 -----------------------
 src/transform.cc | 41 +++++++++++++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/src/normalize.cc b/src/normalize.cc
index c633b3f..a16f7b6 100644
--- a/src/normalize.cc
+++ b/src/normalize.cc
@@ -40,28 +40,6 @@ void StoreLoca(int index_fmt, uint32_t value, size_t* offset, uint8_t* dst) {
   }
 }
 
-void NormalizeSimpleGlyphBoundingBox(Glyph* glyph) {
-  if (glyph->contours.empty() || glyph->contours[0].empty()) {
-    return;
-  }
-  int16_t x_min = glyph->contours[0][0].x;
-  int16_t y_min = glyph->contours[0][0].y;
-  int16_t x_max = x_min;
-  int16_t y_max = y_min;
-  for (const auto& contour : glyph->contours) {
-    for (const auto& point : contour) {
-      if (point.x < x_min) x_min = point.x;
-      if (point.x > x_max) x_max = point.x;
-      if (point.y < y_min) y_min = point.y;
-      if (point.y > y_max) y_max = point.y;
-    }
-  }
-  glyph->x_min = x_min;
-  glyph->y_min = y_min;
-  glyph->x_max = x_max;
-  glyph->y_max = y_max;
-}
-
 }  // namespace
 
 namespace {
@@ -88,7 +66,6 @@ bool WriteNormalizedLoca(int index_fmt, int num_glyphs, Font* font) {
         (glyph_size > 0 && !ReadGlyph(glyph_data, glyph_size, &glyph))) {
       return FONT_COMPRESSION_FAILURE();
     }
-    NormalizeSimpleGlyphBoundingBox(&glyph);
     size_t glyf_dst_size = glyf_table->buffer.size() - glyf_offset;
     if (!StoreGlyph(glyph, glyf_dst + glyf_offset, &glyf_dst_size)) {
       return FONT_COMPRESSION_FAILURE();
diff --git a/src/transform.cc b/src/transform.cc
index 9eb0d1c..8b3731b 100644
--- a/src/transform.cc
+++ b/src/transform.cc
@@ -58,11 +58,10 @@ void WriteLong(std::vector<uint8_t>* out, int value) {
 
 // Glyf table preprocessing, based on
 // GlyfEncoder.java
-// but only the "sbbox" and "cbbox" options are supported.
 class GlyfEncoder {
  public:
   explicit GlyfEncoder(int num_glyphs)
-      : sbbox_(false), cbbox_(true), n_glyphs_(num_glyphs) {
+      : n_glyphs_(num_glyphs) {
     bbox_bitmap_.resize(((num_glyphs + 31) >> 5) << 2);
   }
 
@@ -105,10 +104,40 @@ class GlyfEncoder {
                glyph.instructions_data, glyph.instructions_size);
   }
 
+  bool ShouldWriteSimpleGlyphBbox(const Glyph& glyph) {
+    if (glyph.contours.empty() || glyph.contours[0].empty()) {
+      return glyph.x_min || glyph.y_min || glyph.x_max || glyph.y_max;
+    }
+
+    int16_t x_min = glyph.contours[0][0].x;
+    int16_t y_min = glyph.contours[0][0].y;
+    int16_t x_max = x_min;
+    int16_t y_max = y_min;
+    for (const auto& contour : glyph.contours) {
+      for (const auto& point : contour) {
+        if (point.x < x_min) x_min = point.x;
+        if (point.x > x_max) x_max = point.x;
+        if (point.y < y_min) y_min = point.y;
+        if (point.y > y_max) y_max = point.y;
+      }
+    }
+
+    if (glyph.x_min != x_min)
+      return true;
+    if (glyph.y_min != y_min)
+      return true;
+    if (glyph.x_max != x_max)
+      return true;
+    if (glyph.y_max != y_max)
+      return true;
+
+    return false;
+  }
+
   void WriteSimpleGlyph(int glyph_id, const Glyph& glyph) {
     int num_contours = glyph.contours.size();
     WriteUShort(&n_contour_stream_, num_contours);
-    if (sbbox_) {
+    if (ShouldWriteSimpleGlyphBbox(glyph)) {
       WriteBbox(glyph_id, glyph);
     }
     // TODO: check that bbox matches, write bbox if not
@@ -136,9 +165,7 @@ class GlyfEncoder {
 
   void WriteCompositeGlyph(int glyph_id, const Glyph& glyph) {
     WriteUShort(&n_contour_stream_, -1);
-    if (cbbox_) {
-      WriteBbox(glyph_id, glyph);
-    }
+    WriteBbox(glyph_id, glyph);
     WriteBytes(&composite_stream_,
                glyph.composite_data,
                glyph.composite_data_size);
@@ -204,8 +231,6 @@ class GlyfEncoder {
   std::vector<uint8_t> bbox_stream_;
   std::vector<uint8_t> glyph_stream_;
   std::vector<uint8_t> instruction_stream_;
-  bool sbbox_;
-  bool cbbox_;
   int n_glyphs_;
 };