[cbdt] Clean up some more

Almost there..

src/hb-ot-cbdt-table.hh

@@ -39,6 +39,14 @@ struct SmallGlyphMetrics
     return_trace (c->check_struct (this));
   }
 
+  inline void get_extents (hb_glyph_extents_t *extents) const
+  {
+    extents->x_bearing = bearingX;
+    extents->y_bearing = bearingY;
+    extents->width = width;
+    extents->height = height;
+  }
+
   BYTE height;
   BYTE width;
   CHAR bearingX;
@@ -75,17 +83,14 @@ struct SBitLineMetrics
 /*
  * Index Subtables.
  */
-struct IndexSubtable
+struct IndexSubtableHeader
 {
-  USHORT firstGlyphIndex;
+  inline bool sanitize (hb_sanitize_context_t *c) const
-  USHORT lastGlyphIndex;
+  {
-  ULONG offsetToSubtable;
+    TRACE_SANITIZE (this);
+    return_trace (c->check_struct (this));
+  }
 
-  DEFINE_SIZE_STATIC(8);
-};
-
-struct IndexSubHeader
-{
   USHORT indexFormat;
   USHORT imageFormat;
   ULONG imageDataOffset;
@@ -95,12 +100,108 @@ struct IndexSubHeader
 
 struct IndexSubtableFormat1
 {
-  IndexSubHeader header;
+  inline bool sanitize (hb_sanitize_context_t *c, unsigned int glyph_count) const
-  ULONG offsetArrayZ[VAR];
+  {
+    TRACE_SANITIZE (this);
+    return_trace (c->check_struct (this) &&
+		  c->check_array (offsetArrayZ, offsetArrayZ[0].static_size, glyph_count + 1));
+  }
+
+  bool get_image_data (unsigned int idx,
+		       unsigned int *offset,
+		       unsigned int *length) const
+  {
+    if (unlikely (offsetArrayZ[idx + 1] <= offsetArrayZ[idx]))
+      return false;
+
+    *offset = header.imageDataOffset + offsetArrayZ[idx];
+    *length = offsetArrayZ[idx + 1] - offsetArrayZ[idx];
+    return true;
+  }
+
+  IndexSubtableHeader header;
+  Offset<ULONG> offsetArrayZ[VAR];
 
   DEFINE_SIZE_ARRAY(8, offsetArrayZ);
 };
 
+struct IndexSubtable
+{
+  inline bool sanitize (hb_sanitize_context_t *c, unsigned int glyph_count) const
+  {
+    TRACE_SANITIZE (this);
+    if (!u.header.sanitize (c)) return_trace (false);
+    switch (u.header.indexFormat) {
+    case 1: return_trace (u.format1.sanitize (c, glyph_count));
+    default:return_trace (true);
+    }
+  }
+
+  inline bool get_extents (hb_glyph_extents_t *extents) const
+  {
+    switch (u.header.indexFormat) {
+    case 2: case 5: /* TODO */
+    case 1: case 3: case 4: /* Variable-metrics formats do not have metrics here. */
+    default:return (false);
+    }
+  }
+
+  bool get_image_data (unsigned int idx,
+		       unsigned int *offset,
+		       unsigned int *length,
+		       unsigned int *format) const
+  {
+    *format = u.header.imageFormat;
+    switch (u.header.indexFormat) {
+    case 1: return u.format1.get_image_data (idx, offset, length);
+    default: return false;
+    }
+  }
+
+  protected:
+  union {
+  IndexSubtableHeader	header;
+  IndexSubtableFormat1	format1;
+  } u;
+  public:
+  DEFINE_SIZE_UNION (8, header);
+};
+
+struct IndexSubtableRecord
+{
+  inline bool sanitize (hb_sanitize_context_t *c, const void *base) const
+  {
+    TRACE_SANITIZE (this);
+    return_trace (c->check_struct (this) &&
+		  firstGlyphIndex <= lastGlyphIndex &&
+		  offsetToSubtable.sanitize (c, this, lastGlyphIndex - firstGlyphIndex + 1));
+  }
+
+  inline bool get_extents (hb_glyph_extents_t *extents) const
+  {
+    return (this+offsetToSubtable).get_extents (extents);
+  }
+
+  bool get_image_data (unsigned int gid,
+		       unsigned int *offset,
+		       unsigned int *length,
+		       unsigned int *format) const
+  {
+    if (gid < firstGlyphIndex || gid > lastGlyphIndex)
+    {
+      return false;
+    }
+    return (this+offsetToSubtable).get_image_data (gid - firstGlyphIndex,
+						   offset, length, format);
+  }
+
+  USHORT firstGlyphIndex;
+  USHORT lastGlyphIndex;
+  OffsetTo<IndexSubtable, ULONG> offsetToSubtable;
+
+  DEFINE_SIZE_STATIC(8);
+};
+
 /*
  * Glyph Bitmap Data Formats.
  */
@@ -119,11 +220,16 @@ struct IndexSubtableArray
   inline bool sanitize (hb_sanitize_context_t *c, unsigned int count) const
   {
     TRACE_SANITIZE (this);
-    return_trace (c->check_struct (this)); // XXX
+    if (unlikely (!c->check_array (&indexSubtablesZ, indexSubtablesZ[0].static_size, sizeof (count))))
+      return_trace (false);
+    for (unsigned int i = 0; i < count; i++)
+      if (unlikely (!indexSubtablesZ[i].sanitize (c, this)))
+	return_trace (false);
+    return_trace (true);
   }
 
   public:
-  const IndexSubtable* find_table (hb_codepoint_t glyph, unsigned int numTables) const
+  const IndexSubtableRecord* find_table (hb_codepoint_t glyph, unsigned int numTables) const
   {
     for (unsigned int i = 0; i < numTables; ++i)
     {
@@ -137,7 +243,7 @@ struct IndexSubtableArray
   }
 
   protected:
-  IndexSubtable indexSubtablesZ[VAR];
+  IndexSubtableRecord indexSubtablesZ[VAR];
 
   public:
   DEFINE_SIZE_ARRAY(0, indexSubtablesZ);

src/hb-ot-font.cc

@@ -250,41 +250,25 @@ struct hb_ot_face_cbdt_accelerator_t
       return false;
     }
 
-    const OT::IndexSubtableArray& subtables =
+    const OT::IndexSubtableArray& subtables = this->cblc + sizeTable->indexSubtableArrayOffset;
-            OT::StructAtOffset<OT::IndexSubtableArray> (this->cblc, sizeTable->indexSubtableArrayOffset);
+    const OT::IndexSubtableRecord *subtable_record = subtables.find_table (glyph, sizeTable->numberOfIndexSubtables);
-    const OT::IndexSubtable* subtable = subtables.find_table(glyph, sizeTable->numberOfIndexSubtables);
+    if (subtable_record == NULL) {
-    if (subtable == NULL) {
       return false;
     }
 
-    unsigned int offsetToSubtable = sizeTable->indexSubtableArrayOffset + subtable->offsetToSubtable;
+    if (subtable_record->get_extents (extents))
-    const OT::IndexSubHeader& header =
+      return true;
-            OT::StructAtOffset<OT::IndexSubHeader> (this->cblc, offsetToSubtable);
 
-    unsigned int imageDataOffset = header.imageDataOffset;
+    unsigned int image_offset = 0, image_length = 0, image_format = 0;
-    switch (header.indexFormat)
+    if (!subtable_record->get_image_data (glyph, &image_offset, &image_length, &image_format))
-    {
+      return false;
-      case 1:
-	{
-	  const OT::IndexSubtableFormat1& format1 =
-	      OT::StructAtOffset<OT::IndexSubtableFormat1> (this->cblc, offsetToSubtable);
-	  imageDataOffset += format1.offsetArrayZ[glyph - subtable->firstGlyphIndex];
-	}
-	break;
-      default:
-	// TODO: Support other index subtable format.
-	return false;
-    }
 
-    switch (header.imageFormat)
+    switch (image_format)
     {
       case 17: {
 	const OT::GlyphBitmapDataFormat17& glyphFormat17 =
-	    OT::StructAtOffset<OT::GlyphBitmapDataFormat17> (this->cbdt, imageDataOffset);
+	    OT::StructAtOffset<OT::GlyphBitmapDataFormat17> (this->cbdt, image_offset);
-	extents->x_bearing = glyphFormat17.glyphMetrics.bearingX;
+	glyphFormat17.glyphMetrics.get_extents (extents);
-	extents->y_bearing = glyphFormat17.glyphMetrics.bearingY;
-	extents->width = glyphFormat17.glyphMetrics.width;
-	extents->height = -glyphFormat17.glyphMetrics.height;
       }
       break;
       default: