Change ArrayOf.bsearch() return semantics
authorBehdad Esfahbod <behdad@behdad.org>
Sat, 24 Nov 2018 05:35:31 +0000 (00:35 -0500)
committerBehdad Esfahbod <behdad@behdad.org>
Sat, 24 Nov 2018 05:48:26 +0000 (00:48 -0500)
Towards consolidating all array bsearch/...

src/hb-aat-layout-kerx-table.hh
src/hb-open-file.hh
src/hb-open-type.hh
src/hb-ot-cmap-table.hh
src/hb-ot-color-svg-table.hh
src/hb-ot-layout-common.hh
src/hb-ot-vorg-table.hh
src/hb-vector.hh

index c60f29a..521c4c7 100644 (file)
@@ -49,7 +49,7 @@ kerxTupleKern (int value,
               const void *base,
               hb_aat_apply_context_t *c)
 {
-  if (likely (!tupleCount)) return value;
+  if (likely (!tupleCount || !c)) return value;
 
   unsigned int offset = value;
   const FWORD *pv = &StructAtOffset<FWORD> (base, offset);
@@ -93,21 +93,11 @@ struct KernPair
 template <typename KernSubTableHeader>
 struct KerxSubTableFormat0
 {
-  inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const
-  {
-    hb_glyph_pair_t pair = {left, right};
-    int i = pairs.bsearch (pair);
-    if (i == -1) return 0;
-    return pairs[i].get_kerning ();
-  }
-
   inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right,
-                         hb_aat_apply_context_t *c) const
+                         hb_aat_apply_context_t *c = nullptr) const
   {
     hb_glyph_pair_t pair = {left, right};
-    int i = pairs.bsearch (pair);
-    if (i == -1) return 0;
-    int v = pairs[i].get_kerning ();
+    int v = pairs.bsearch (pair).get_kerning ();
     return kerxTupleKern (v, header.tuple_count (), this, c);
   }
 
index cd5f9db..d3ecc60 100644 (file)
@@ -111,10 +111,14 @@ typedef struct OffsetTable
   {
     Tag t;
     t.set (tag);
-    int i = tables.bsearch (t);
-    if (table_index)
-      *table_index = i == -1 ? (unsigned) Index::NOT_FOUND_INDEX : (unsigned) i;
-    return i != -1;
+    unsigned int i;
+    if (tables.bfind (t, &i))
+    {
+      if (table_index) *table_index = i;
+      return true;
+    }
+    if (table_index) *table_index = (unsigned) Index::NOT_FOUND_INDEX;
+    return false;
   }
   inline const TableRecord& get_table_by_tag (hb_tag_t tag) const
   {
index 1c7b738..f255ea1 100644 (file)
@@ -740,21 +740,45 @@ struct ArrayOfM1
 template <typename Type, typename LenType=HBUINT16>
 struct SortedArrayOf : ArrayOf<Type, LenType>
 {
-  template <typename SearchType>
-  inline int bsearch (const SearchType &x) const
+  template <typename T>
+  inline Type &bsearch (const T &x)
+  {
+    unsigned int i;
+    return bfind (x, &i) ? this->arrayZ[i] : Crap(Type);
+  }
+  template <typename T>
+  inline const Type &bsearch (const T &x) const
+  {
+    unsigned int i;
+    return bfind (x, &i) ? this->arrayZ[i] : Null(Type);
+  }
+  template <typename T>
+  inline bool bfind (const T &x, unsigned int *i = nullptr) const
   {
-    /* Hand-coded bsearch here since this is in the hot inner loop. */
-    const Type *arr = this->arrayZ;
     int min = 0, max = (int) this->len - 1;
+    const Type *array = this->arrayZ;
     while (min <= max)
     {
       int mid = ((unsigned int) min + (unsigned int) max) / 2;
-      int c = arr[mid].cmp (x);
-      if (c < 0) max = mid - 1;
-      else if (c > 0) min = mid + 1;
-      else return mid;
+      int c = array[mid].cmp (x);
+      if (c < 0)
+        max = mid - 1;
+      else if (c > 0)
+        min = mid + 1;
+      else
+      {
+       if (i)
+         *i = mid;
+       return true;
+      }
     }
-    return -1;
+    if (i)
+    {
+      if (max < 0 || (max < (int) this->len && array[max].cmp (x) > 0))
+       max++;
+      *i = max;
+    }
+    return false;
   }
 };
 
index 70373ee..cdc610b 100644 (file)
@@ -468,10 +468,7 @@ struct CmapSubtableLongSegmented
 
   inline bool get_glyph (hb_codepoint_t codepoint, hb_codepoint_t *glyph) const
   {
-    int i = groups.bsearch (codepoint);
-    if (i == -1)
-      return false;
-    hb_codepoint_t gid = T::group_get_glyph (groups[i], codepoint);
+    hb_codepoint_t gid = T::group_get_glyph (groups.bsearch (codepoint), codepoint);
     if (!gid)
       return false;
     *glyph = gid;
@@ -518,7 +515,8 @@ struct CmapSubtableFormat12 : CmapSubtableLongSegmented<CmapSubtableFormat12>
 {
   static inline hb_codepoint_t group_get_glyph (const CmapSubtableLongGroup &group,
                                                hb_codepoint_t u)
-  { return group.glyphID + (u - group.startCharCode); }
+  { return likely (group.startCharCode <= group.endCharCode) ?
+          group.glyphID + (u - group.startCharCode) : 0; }
 
 
   bool serialize (hb_serialize_context_t *c,
@@ -674,16 +672,12 @@ struct VariationSelectorRecord
                                    hb_codepoint_t *glyph,
                                    const void *base) const
   {
-    int i;
-    const DefaultUVS &defaults = base+defaultUVS;
-    i = defaults.bsearch (codepoint);
-    if (i != -1)
+    if ((base+defaultUVS).bfind (codepoint))
       return GLYPH_VARIANT_USE_DEFAULT;
-    const NonDefaultUVS &nonDefaults = base+nonDefaultUVS;
-    i = nonDefaults.bsearch (codepoint);
-    if (i != -1 && nonDefaults[i].glyphID)
+    const UVSMapping &nonDefault = (base+nonDefaultUVS).bsearch (codepoint);
+    if (nonDefault.glyphID)
     {
-      *glyph = nonDefaults[i].glyphID;
+      *glyph = nonDefault.glyphID;
        return GLYPH_VARIANT_FOUND;
     }
     return GLYPH_VARIANT_NOT_FOUND;
@@ -723,7 +717,7 @@ struct CmapSubtableFormat14
                                            hb_codepoint_t variation_selector,
                                            hb_codepoint_t *glyph) const
   {
-    return record[record.bsearch (variation_selector)].get_glyph (codepoint, glyph, this);
+    return record.bsearch (variation_selector).get_glyph (codepoint, glyph, this);
   }
 
   inline void collect_variation_selectors (hb_set_t *out) const
@@ -735,7 +729,7 @@ struct CmapSubtableFormat14
   inline void collect_variation_unicodes (hb_codepoint_t variation_selector,
                                          hb_set_t *out) const
   {
-    record[record.bsearch (variation_selector)].collect_unicodes (out, this);
+    record.bsearch (variation_selector).collect_unicodes (out, this);
   }
 
   inline bool sanitize (hb_sanitize_context_t *c) const
@@ -1157,11 +1151,11 @@ struct cmap
     key.platformID.set (platform_id);
     key.encodingID.set (encoding_id);
 
-    int result = encodingRecord.bsearch (key);
-    if (result == -1 || !encodingRecord[result].subtable)
+    const EncodingRecord &result = encodingRecord.bsearch (key);
+    if (!result.subtable)
       return nullptr;
 
-    return &(this+encodingRecord[result].subtable);
+    return &(this+result.subtable);
   }
 
   public:
index bad8ef5..ad35510 100644 (file)
@@ -103,8 +103,7 @@ struct SVG
 
   inline const SVGDocumentIndexEntry &get_glyph_entry (hb_codepoint_t glyph_id) const
   {
-    const SortedArrayOf<SVGDocumentIndexEntry> &docs = this+svgDocEntries;
-    return docs[docs.bsearch (glyph_id)];
+    return (this+svgDocEntries).bsearch (glyph_id);
   }
 
   inline bool sanitize (hb_sanitize_context_t *c) const
index d7c3178..6c6bcf5 100644 (file)
@@ -128,15 +128,12 @@ struct RecordArrayOf : SortedArrayOf<Record<Type> >
   }
   inline bool find_index (hb_tag_t tag, unsigned int *index) const
   {
-    /* If we want to allow non-sorted data, we can lsearch(). */
-    int i = this->/*lsearch*/bsearch (tag);
-    if (i != -1) {
-      if (index) *index = i;
-      return true;
-    } else {
+    if (!this->bfind (tag, index))
+    {
       if (index) *index = Index::NOT_FOUND_INDEX;
       return false;
     }
+    return true;
   }
 };
 
@@ -823,8 +820,9 @@ struct CoverageFormat1
   private:
   inline unsigned int get_coverage (hb_codepoint_t glyph_id) const
   {
-    int i = glyphArray.bsearch (glyph_id);
-    static_assert ((((unsigned int) -1) == NOT_COVERED), "");
+    unsigned int i;
+    if (!glyphArray.bfind (glyph_id, &i))
+      return NOT_COVERED;
     return i;
   }
 
@@ -896,12 +894,10 @@ struct CoverageFormat2
   private:
   inline unsigned int get_coverage (hb_codepoint_t glyph_id) const
   {
-    int i = rangeRecord.bsearch (glyph_id);
-    if (i != -1) {
-      const RangeRecord &range = rangeRecord[i];
-      return (unsigned int) range.value + (glyph_id - range.start);
-    }
-    return NOT_COVERED;
+    const RangeRecord &range = rangeRecord.bsearch (glyph_id);
+    return likely (range.start <= range.end) ?
+          (unsigned int) range.value + (glyph_id - range.start) :
+          NOT_COVERED;
   }
 
   inline bool serialize (hb_serialize_context_t *c,
@@ -1277,10 +1273,7 @@ struct ClassDefFormat2
   private:
   inline unsigned int get_class (hb_codepoint_t glyph_id) const
   {
-    int i = rangeRecord.bsearch (glyph_id);
-    if (unlikely (i != -1))
-      return rangeRecord[i].value;
-    return 0;
+    return rangeRecord.bsearch (glyph_id).value;
   }
 
   inline bool sanitize (hb_sanitize_context_t *c) const
index 98af00e..a0480cb 100644 (file)
@@ -63,11 +63,10 @@ struct VORG
 
   inline int get_y_origin (hb_codepoint_t glyph) const
   {
-    int i = vertYOrigins.bsearch (glyph);
-    if (i != -1)
-      return vertYOrigins[i].vertOriginY;
-
-    return defaultVertOriginY;
+    unsigned int i;
+    if (!vertYOrigins.bfind (glyph, &i))
+      return defaultVertOriginY;
+    return vertYOrigins[i].vertOriginY;
   }
 
   inline bool _subset (const hb_subset_plan_t *plan HB_UNUSED,
index c1d7f94..b30511d 100644 (file)
@@ -260,7 +260,7 @@ struct hb_vector_t
     return bfind (x, &i) ? &arrayZ()[i] : nullptr;
   }
   template <typename T>
-  inline bool bfind (const T &x, unsigned int *i) const
+  inline bool bfind (const T &x, unsigned int *i = nullptr) const
   {
     int min = 0, max = (int) this->len - 1;
     const Type *array = this->arrayZ();
@@ -274,13 +274,17 @@ struct hb_vector_t
         min = mid + 1;
       else
       {
-        *i = mid;
+       if (i)
+         *i = mid;
        return true;
       }
     }
-    if (max < 0 || (max < (int) this->len && array[max].cmp (&x) > 0))
-      max++;
-    *i = max;
+    if (i)
+    {
+      if (max < 0 || (max < (int) this->len && array[max].cmp (&x) > 0))
+       max++;
+      *i = max;
+    }
     return false;
   }
 };