[HarfBuzz] harfbuzz-ng: Branch 'master' - 6 commits

Behdad Esfahbod behdad at kemper.freedesktop.org
Sat Jul 28 16:16:05 PDT 2012


 src/hb-ot-layout-common-private.hh   |    8 -
 src/hb-ot-layout-gpos-table.hh       |  179 +++++++++++++++++++++++++++++----
 src/hb-ot-layout-gsub-table.hh       |  188 ++++++++++++++++++++++-------------
 src/hb-ot-layout-gsubgpos-private.hh |   52 ++++++---
 4 files changed, 313 insertions(+), 114 deletions(-)

New commits:
commit 338fe662b50f9309bf0050dd99becb644874195b
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sat Jul 28 18:53:01 2012 -0400

    [GSUB] Minor

diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh
index d95b691..03244b5 100644
--- a/src/hb-ot-layout-gsub-table.hh
+++ b/src/hb-ot-layout-gsub-table.hh
@@ -1106,22 +1106,6 @@ struct SubstLookup : Lookup
     if (!_hb_ot_layout_check_glyph_property (c->face, &c->buffer->cur(), c->lookup_props, &c->property))
       return false;
 
-    /* TODO: For the most common case this can move out of the main
-     * loop, but it's not a big deal for now. */
-    if (unlikely (lookup_type == SubstLookupSubTable::Extension))
-    {
-      /* The spec says all subtables should have the same type.
-       * This is specially important if one has a reverse type!
-       *
-       * This is rather slow to do this here for every glyph,
-       * but it's easiest, and who uses extension lookups anyway?!*/
-      unsigned int type = get_subtable(0).u.extension.get_type ();
-      unsigned int count = get_subtable_count ();
-      for (unsigned int i = 1; i < count; i++)
-        if (get_subtable(i).u.extension.get_type () != type)
-	  return false;
-    }
-
     unsigned int count = get_subtable_count ();
     for (unsigned int i = 0; i < count; i++)
       if (get_subtable (i).apply (c, lookup_type))
@@ -1191,7 +1175,22 @@ struct SubstLookup : Lookup
     TRACE_SANITIZE ();
     if (unlikely (!Lookup::sanitize (c))) return TRACE_RETURN (false);
     OffsetArrayOf<SubstLookupSubTable> &list = CastR<OffsetArrayOf<SubstLookupSubTable> > (subTable);
-    return TRACE_RETURN (list.sanitize (c, this, get_type ()));
+    if (unlikely (!list.sanitize (c, this, get_type ()))) return TRACE_RETURN (false);
+
+    if (unlikely (get_type () == SubstLookupSubTable::Extension))
+    {
+      /* The spec says all subtables of an Extension lookup should
+       * have the same type.  This is specially important if one has
+       * a reverse type!
+       *
+       * We just check that they are all either forward, or reverse. */
+      unsigned int type = get_subtable (0).u.extension.get_type ();
+      unsigned int count = get_subtable_count ();
+      for (unsigned int i = 1; i < count; i++)
+        if (get_subtable (i).u.extension.get_type () != type)
+	  return TRACE_RETURN (false);
+    }
+    return TRACE_RETURN (true);
   }
 };
 
commit e6f7479fe34fb4a7cada61d84c2ed70d1fd565c8
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sat Jul 28 18:34:58 2012 -0400

    [GSUB] Simplify would-apply

diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh
index 2cbab32..d95b691 100644
--- a/src/hb-ot-layout-gsub-table.hh
+++ b/src/hb-ot-layout-gsub-table.hh
@@ -55,11 +55,6 @@ struct SingleSubstFormat1
     return this+coverage;
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -112,11 +107,6 @@ struct SingleSubstFormat2
     return this+coverage;
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -174,15 +164,6 @@ struct SingleSubst
     }
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    switch (u.format) {
-    case 1: return u.format1.would_apply (c);
-    case 2: return u.format2.would_apply (c);
-    default:return false;
-    }
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -276,11 +257,6 @@ struct MultipleSubstFormat1
     return this+coverage;
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -331,14 +307,6 @@ struct MultipleSubst
     }
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    switch (u.format) {
-    case 1: return u.format1.would_apply (c);
-    default:return false;
-    }
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -393,11 +361,6 @@ struct AlternateSubstFormat1
     return this+coverage;
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -466,14 +429,6 @@ struct AlternateSubst
     }
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    switch (u.format) {
-    case 1: return u.format1.would_apply (c);
-    default:return false;
-    }
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -675,9 +630,7 @@ struct LigatureSubstFormat1
 
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
-    unsigned int index;
-    return (index = (this+coverage) (c->first)) != NOT_COVERED &&
-	   (this+ligatureSet[index]).would_apply (c);
+    return (this+ligatureSet[(this+coverage) (c->first)]).would_apply (c);
   }
 
   inline bool apply (hb_apply_context_t *c) const
@@ -871,11 +824,6 @@ struct ReverseChainSingleSubstFormat1
     return this+coverage;
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -958,14 +906,6 @@ struct ReverseChainSingleSubst
     }
   }
 
-  inline bool would_apply (hb_would_apply_context_t *c) const
-  {
-    switch (u.format) {
-    case 1: return u.format1.would_apply (c);
-    default:return false;
-    }
-  }
-
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -1048,15 +988,16 @@ struct SubstLookupSubTable
 			   unsigned int lookup_type) const
   {
     TRACE_WOULD_APPLY ();
+    if (get_coverage (lookup_type).get_coverage (c->first) == NOT_COVERED) return false;
+    if (c->len == 1) return true; /* Done! */
+
+    /* Only need to look further for lookups that support substitutions
+     * of input longer than 1. */
     switch (lookup_type) {
-    case Single:		return u.single.would_apply (c);
-    case Multiple:		return u.multiple.would_apply (c);
-    case Alternate:		return u.alternate.would_apply (c);
     case Ligature:		return u.ligature.would_apply (c);
     case Context:		return u.context.would_apply (c);
     case ChainContext:		return u.chainContext.would_apply (c);
     case Extension:		return u.extension.would_apply (c);
-    case ReverseChainSingle:	return u.reverseChainContextSingle.would_apply (c);
     default:			return false;
     }
   }
diff --git a/src/hb-ot-layout-gsubgpos-private.hh b/src/hb-ot-layout-gsubgpos-private.hh
index 870916f..2d6cafc 100644
--- a/src/hb-ot-layout-gsubgpos-private.hh
+++ b/src/hb-ot-layout-gsubgpos-private.hh
@@ -677,11 +677,8 @@ struct ContextFormat1
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     TRACE_WOULD_APPLY ();
-    unsigned int index = (this+coverage) (c->first);
-    if (likely (index == NOT_COVERED))
-      return TRACE_RETURN (false);
 
-    const RuleSet &rule_set = this+ruleSet[index];
+    const RuleSet &rule_set = this+ruleSet[(this+coverage) (c->first)];
     struct ContextApplyLookupContext lookup_context = {
       {match_glyph, NULL},
       NULL
@@ -752,11 +749,9 @@ struct ContextFormat2
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     TRACE_WOULD_APPLY ();
-    unsigned int index = (this+coverage) (c->first);
-    if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
 
     const ClassDef &class_def = this+classDef;
-    index = class_def (c->first);
+    unsigned int index = class_def (c->first);
     const RuleSet &rule_set = this+ruleSet[index];
     struct ContextApplyLookupContext lookup_context = {
       {match_class, NULL},
@@ -828,8 +823,6 @@ struct ContextFormat3
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     TRACE_WOULD_APPLY ();
-    unsigned int index = (this+coverage[0]) (c->first);
-    if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
 
     const LookupRecord *lookupRecord = &StructAtOffset<LookupRecord> (coverage, coverage[0].static_size * glyphCount);
     struct ContextApplyLookupContext lookup_context = {
@@ -1180,10 +1173,8 @@ struct ChainContextFormat1
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     TRACE_WOULD_APPLY ();
-    unsigned int index = (this+coverage) (c->first);
-    if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
 
-    const ChainRuleSet &rule_set = this+ruleSet[index];
+    const ChainRuleSet &rule_set = this+ruleSet[(this+coverage) (c->first)];
     struct ChainContextApplyLookupContext lookup_context = {
       {match_glyph, NULL},
       {NULL, NULL, NULL}
@@ -1256,12 +1247,10 @@ struct ChainContextFormat2
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     TRACE_WOULD_APPLY ();
-    unsigned int index = (this+coverage) (c->first);
-    if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
 
     const ClassDef &input_class_def = this+inputClassDef;
 
-    index = input_class_def (c->first);
+    unsigned int index = input_class_def (c->first);
     const ChainRuleSet &rule_set = this+ruleSet[index];
     struct ChainContextApplyLookupContext lookup_context = {
       {match_class, NULL},
@@ -1359,11 +1348,8 @@ struct ChainContextFormat3
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     TRACE_WOULD_APPLY ();
-    const OffsetArrayOf<Coverage> &input = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
-
-    unsigned int index = (this+input[0]) (c->first);
-    if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
 
+    const OffsetArrayOf<Coverage> &input = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
     const OffsetArrayOf<Coverage> &lookahead = StructAfter<OffsetArrayOf<Coverage> > (input);
     const ArrayOf<LookupRecord> &lookup = StructAfter<ArrayOf<LookupRecord> > (lookahead);
     struct ChainContextApplyLookupContext lookup_context = {
commit dadede012e4841f9fcb70d514fdc752f3ea4663d
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sat Jul 28 18:03:20 2012 -0400

    Minor

diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh
index fb7fd3b..db42894 100644
--- a/src/hb-ot-layout-common-private.hh
+++ b/src/hb-ot-layout-common-private.hh
@@ -34,7 +34,7 @@
 #include "hb-set-private.hh"
 
 
-#define NOT_COVERED		((unsigned int) 0x110000)
+#define NOT_COVERED		((unsigned int) -1)
 #define MAX_NESTING_LEVEL	8
 
 
@@ -348,9 +348,8 @@ struct CoverageFormat1
   inline unsigned int get_coverage (hb_codepoint_t glyph_id) const
   {
     int i = glyphArray.search (glyph_id);
-    if (i != -1)
-        return i;
-    return NOT_COVERED;
+    ASSERT_STATIC (((unsigned int) -1) == NOT_COVERED);
+    return i;
   }
 
   inline bool sanitize (hb_sanitize_context_t *c) {
diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh
index 6d05c99..3b1ae2a 100644
--- a/src/hb-ot-layout-gpos-table.hh
+++ b/src/hb-ot-layout-gpos-table.hh
@@ -1533,7 +1533,7 @@ struct PosLookup : Lookup
       while (c->buffer->idx < c->buffer->len)
       {
 	if ((c->buffer->cur().mask & c->lookup_mask) &&
-	    (*coverage) (c->buffer->cur().codepoint) != NOT_COVERED &&
+	    coverage->get_coverage (c->buffer->cur().codepoint) != NOT_COVERED &&
 	    apply_once (c))
 	  ret = true;
 	else
diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh
index e50ecd4..2cbab32 100644
--- a/src/hb-ot-layout-gsub-table.hh
+++ b/src/hb-ot-layout-gsub-table.hh
@@ -1210,7 +1210,7 @@ struct SubstLookup : Lookup
 	  while (c->buffer->idx < c->buffer->len)
 	  {
 	    if ((c->buffer->cur().mask & c->lookup_mask) &&
-		(*coverage) (c->buffer->cur().codepoint) != NOT_COVERED &&
+		coverage->get_coverage (c->buffer->cur().codepoint) != NOT_COVERED &&
 		apply_once (c))
 	      ret = true;
 	    else
commit 0b99429ead05ae32b3c210cb499af401b02770a9
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sat Jul 28 17:31:01 2012 -0400

    [GSUB/GPOS] Add get_coverage() and use it to speed up main loop
    
    And use it to speed up the hotspot by checking coverage directly in
    the main loop, not 10 functions deep in.
    
    Gives me a solid 20% boost with Indic test suite.  Less so for less
    lookup-intensive scenarios.
    
    Remove the "fast_path" hack from before.

diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh
index a35f633..6d05c99 100644
--- a/src/hb-ot-layout-gpos-table.hh
+++ b/src/hb-ot-layout-gpos-table.hh
@@ -429,6 +429,12 @@ struct SinglePosFormat1
   friend struct SinglePos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -466,6 +472,12 @@ struct SinglePosFormat2
   friend struct SinglePos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -506,6 +518,16 @@ struct SinglePos
   friend struct PosLookupSubTable;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    case 2: return u.format2.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -614,6 +636,12 @@ struct PairPosFormat1
   friend struct PairPos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -666,6 +694,12 @@ struct PairPosFormat2
   friend struct PairPos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -750,6 +784,16 @@ struct PairPos
   friend struct PosLookupSubTable;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    case 2: return u.format2.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -806,6 +850,12 @@ struct CursivePosFormat1
   friend struct CursivePos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -910,6 +960,15 @@ struct CursivePos
   friend struct PosLookupSubTable;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -946,6 +1005,12 @@ struct MarkBasePosFormat1
   friend struct MarkBasePos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+markCoverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -1001,6 +1066,15 @@ struct MarkBasePos
   friend struct PosLookupSubTable;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -1042,6 +1116,12 @@ struct MarkLigPosFormat1
   friend struct MarkLigPos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+markCoverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -1117,6 +1197,15 @@ struct MarkLigPos
   friend struct PosLookupSubTable;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -1153,6 +1242,12 @@ struct MarkMarkPosFormat1
   friend struct MarkMarkPos;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+mark1Coverage;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -1215,6 +1310,15 @@ struct MarkMarkPos
   friend struct PosLookupSubTable;
 
   private:
+
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -1280,6 +1384,8 @@ struct ExtensionPos : Extension
     return StructAtOffset<PosLookupSubTable> (this, offset);
   }
 
+  inline const Coverage &get_coverage (void) const;
+
   inline bool apply (hb_apply_context_t *c) const;
 
   inline bool sanitize (hb_sanitize_context_t *c);
@@ -1308,24 +1414,25 @@ struct PosLookupSubTable
     Extension		= 9
   };
 
-  inline bool can_use_fast_path (unsigned int lookup_type) const
+  inline const Coverage &get_coverage (unsigned int lookup_type) const
   {
-    /* Fast path, for those that have coverage in the same place. */
-    return likely (lookup_type && lookup_type < Context) ||
-	   (hb_in_range<unsigned int> (lookup_type, Context, ChainContext) &&
-	    hb_in_range<unsigned int> (u.header.sub_format, 1, 2));
+    switch (lookup_type) {
+    case Single:		return u.single.get_coverage ();
+    case Pair:			return u.pair.get_coverage ();
+    case Cursive:		return u.cursive.get_coverage ();
+    case MarkBase:		return u.markBase.get_coverage ();
+    case MarkLig:		return u.markLig.get_coverage ();
+    case MarkMark:		return u.markMark.get_coverage ();
+    case Context:		return u.context.get_coverage ();
+    case ChainContext:		return u.chainContext.get_coverage ();
+    case Extension:		return u.extension.get_coverage ();
+    default:			return Null(Coverage);
+    }
   }
 
   inline bool apply (hb_apply_context_t *c, unsigned int lookup_type) const
   {
     TRACE_APPLY ();
-    if (can_use_fast_path (lookup_type))
-    {
-      /* Fast path, for most that have coverage in the same place. */
-      hb_codepoint_t glyph_id = c->buffer->cur().codepoint;
-      unsigned int index = (this+u.header.coverage) (glyph_id);
-      if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
-    }
     switch (lookup_type) {
     case Single:		return TRACE_RETURN (u.single.apply (c));
     case Pair:			return TRACE_RETURN (u.pair.apply (c));
@@ -1342,8 +1449,7 @@ struct PosLookupSubTable
 
   inline bool sanitize (hb_sanitize_context_t *c, unsigned int lookup_type) {
     TRACE_SANITIZE ();
-    if (!u.header.sub_format.sanitize (c) ||
-	(can_use_fast_path (lookup_type) && !u.header.coverage.sanitize (c, this)))
+    if (!u.header.sub_format.sanitize (c))
       return TRACE_RETURN (false);
     switch (lookup_type) {
     case Single:		return TRACE_RETURN (u.single.sanitize (c));
@@ -1363,7 +1469,6 @@ struct PosLookupSubTable
   union {
   struct {
     USHORT			sub_format;
-    OffsetTo<Coverage>		coverage;
   } header;
   SinglePos		single;
   PairPos		pair;
@@ -1385,6 +1490,17 @@ struct PosLookup : Lookup
   inline const PosLookupSubTable& get_subtable (unsigned int i) const
   { return this+CastR<OffsetArrayOf<PosLookupSubTable> > (subTable)[i]; }
 
+  inline const Coverage *get_coverage (void) const
+  {
+    /* Only return non-NULL if there's just one Coverage table we care about. */
+    const Coverage *c = &get_subtable (0).get_coverage (get_type ());
+    unsigned int count = get_subtable_count ();
+    for (unsigned int i = 1; i < count; i++)
+      if (c != &get_subtable (i).get_coverage (get_type ()))
+        return NULL;
+    return c;
+  }
+
   inline bool apply_once (hb_apply_context_t *c) const
   {
     unsigned int lookup_type = get_type ();
@@ -1410,13 +1526,27 @@ struct PosLookup : Lookup
     c->set_lookup (*this);
 
     c->buffer->idx = 0;
-    while (c->buffer->idx < c->buffer->len)
-    {
-      if ((c->buffer->cur().mask & c->lookup_mask) && apply_once (c))
-	ret = true;
-      else
-	c->buffer->idx++;
-    }
+
+    /* Fast path for lookups with one coverage only (which is most). */
+    const Coverage *coverage = get_coverage ();
+    if (coverage)
+      while (c->buffer->idx < c->buffer->len)
+      {
+	if ((c->buffer->cur().mask & c->lookup_mask) &&
+	    (*coverage) (c->buffer->cur().codepoint) != NOT_COVERED &&
+	    apply_once (c))
+	  ret = true;
+	else
+	  c->buffer->idx++;
+      }
+    else
+      while (c->buffer->idx < c->buffer->len)
+      {
+	if ((c->buffer->cur().mask & c->lookup_mask) && apply_once (c))
+	  ret = true;
+	else
+	  c->buffer->idx++;
+      }
 
     return ret;
   }
@@ -1536,6 +1666,11 @@ GPOS::position_finish (hb_buffer_t *buffer)
 
 /* Out-of-class implementation for methods recursing */
 
+inline const Coverage & ExtensionPos::get_coverage (void) const
+{
+  return get_subtable ().get_coverage (get_type ());
+}
+
 inline bool ExtensionPos::apply (hb_apply_context_t *c) const
 {
   TRACE_APPLY ();
diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh
index 038cb93..e50ecd4 100644
--- a/src/hb-ot-layout-gsub-table.hh
+++ b/src/hb-ot-layout-gsub-table.hh
@@ -50,6 +50,11 @@ struct SingleSubstFormat1
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
@@ -102,6 +107,11 @@ struct SingleSubstFormat2
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
@@ -155,6 +165,15 @@ struct SingleSubst
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    case 2: return u.format2.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     switch (u.format) {
@@ -252,6 +271,11 @@ struct MultipleSubstFormat1
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
@@ -299,6 +323,14 @@ struct MultipleSubst
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     switch (u.format) {
@@ -356,6 +388,11 @@ struct AlternateSubstFormat1
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
@@ -421,6 +458,14 @@ struct AlternateSubst
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     switch (u.format) {
@@ -623,6 +668,11 @@ struct LigatureSubstFormat1
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     unsigned int index;
@@ -674,6 +724,14 @@ struct LigatureSubst
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     switch (u.format) {
@@ -764,6 +822,9 @@ struct ExtensionSubst : Extension
   }
 
   inline void closure (hb_closure_context_t *c) const;
+
+  inline const Coverage &get_coverage (void) const;
+
   inline bool would_apply (hb_would_apply_context_t *c) const;
 
   inline bool apply (hb_apply_context_t *c) const;
@@ -805,6 +866,16 @@ struct ReverseChainSingleSubstFormat1
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    return this+coverage;
+  }
+
+  inline bool would_apply (hb_would_apply_context_t *c) const
+  {
+    return c->len == 1 && (this+coverage) (c->first) != NOT_COVERED;
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -879,6 +950,22 @@ struct ReverseChainSingleSubst
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
+  inline bool would_apply (hb_would_apply_context_t *c) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.would_apply (c);
+    default:return false;
+    }
+  }
+
   inline bool apply (hb_apply_context_t *c) const
   {
     TRACE_APPLY ();
@@ -942,25 +1029,25 @@ struct SubstLookupSubTable
     }
   }
 
-  inline bool can_use_fast_path (unsigned int lookup_type) const
+  inline const Coverage &get_coverage (unsigned int lookup_type) const
   {
-    /* Fast path, for those that have coverage in the same place.
-     * Note that ReverseChainSingle can also go through this but
-     * it's not worth the effort. */
-    return likely (lookup_type && lookup_type < Context) ||
-	   (hb_in_range<unsigned int> (lookup_type, Context, ChainContext) &&
-	    hb_in_range<unsigned int> (u.header.sub_format, 1, 2));
+    switch (lookup_type) {
+    case Single:		return u.single.get_coverage ();
+    case Multiple:		return u.multiple.get_coverage ();
+    case Alternate:		return u.alternate.get_coverage ();
+    case Ligature:		return u.ligature.get_coverage ();
+    case Context:		return u.context.get_coverage ();
+    case ChainContext:		return u.chainContext.get_coverage ();
+    case Extension:		return u.extension.get_coverage ();
+    case ReverseChainSingle:	return u.reverseChainContextSingle.get_coverage ();
+    default:			return Null(Coverage);
+    }
   }
 
   inline bool would_apply (hb_would_apply_context_t *c,
 			   unsigned int lookup_type) const
   {
     TRACE_WOULD_APPLY ();
-    if (can_use_fast_path (lookup_type))
-    {
-      unsigned int index = (this+u.header.coverage) (c->first);
-      if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
-    }
     switch (lookup_type) {
     case Single:		return u.single.would_apply (c);
     case Multiple:		return u.multiple.would_apply (c);
@@ -969,6 +1056,7 @@ struct SubstLookupSubTable
     case Context:		return u.context.would_apply (c);
     case ChainContext:		return u.chainContext.would_apply (c);
     case Extension:		return u.extension.would_apply (c);
+    case ReverseChainSingle:	return u.reverseChainContextSingle.would_apply (c);
     default:			return false;
     }
   }
@@ -976,12 +1064,6 @@ struct SubstLookupSubTable
   inline bool apply (hb_apply_context_t *c, unsigned int lookup_type) const
   {
     TRACE_APPLY ();
-    if (can_use_fast_path (lookup_type))
-    {
-      hb_codepoint_t glyph_id = c->buffer->cur().codepoint;
-      unsigned int index = (this+u.header.coverage) (glyph_id);
-      if (likely (index == NOT_COVERED)) return TRACE_RETURN (false);
-    }
     switch (lookup_type) {
     case Single:		return TRACE_RETURN (u.single.apply (c));
     case Multiple:		return TRACE_RETURN (u.multiple.apply (c));
@@ -997,8 +1079,7 @@ struct SubstLookupSubTable
 
   inline bool sanitize (hb_sanitize_context_t *c, unsigned int lookup_type) {
     TRACE_SANITIZE ();
-    if (!u.header.sub_format.sanitize (c) ||
-	(can_use_fast_path (lookup_type) && !u.header.coverage.sanitize (c, this)))
+    if (!u.header.sub_format.sanitize (c))
       return TRACE_RETURN (false);
     switch (lookup_type) {
     case Single:		return TRACE_RETURN (u.single.sanitize (c));
@@ -1017,7 +1098,6 @@ struct SubstLookupSubTable
   union {
   struct {
     USHORT			sub_format;
-    OffsetTo<Coverage>		coverage;
   } header;
   SingleSubst			single;
   MultipleSubst			multiple;
@@ -1057,6 +1137,17 @@ struct SubstLookup : Lookup
       get_subtable (i).closure (c, lookup_type);
   }
 
+  inline const Coverage *get_coverage (void) const
+  {
+    /* Only return non-NULL if there's just one Coverage table we care about. */
+    const Coverage *c = &get_subtable (0).get_coverage (get_type ());
+    unsigned int count = get_subtable_count ();
+    for (unsigned int i = 1; i < count; i++)
+      if (c != &get_subtable (i).get_coverage (get_type ()))
+        return NULL;
+    return c;
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     unsigned int lookup_type = get_type ();
@@ -1112,14 +1203,28 @@ struct SubstLookup : Lookup
 	/* in/out forward substitution */
 	c->buffer->clear_output ();
 	c->buffer->idx = 0;
-	while (c->buffer->idx < c->buffer->len)
-	{
-	  if ((c->buffer->cur().mask & c->lookup_mask) && apply_once (c))
-	    ret = true;
-	  else
-	    c->buffer->next_glyph ();
 
-	}
+	/* Fast path for lookups with one coverage only (which is most). */
+	const Coverage *coverage = get_coverage ();
+	if (coverage)
+	  while (c->buffer->idx < c->buffer->len)
+	  {
+	    if ((c->buffer->cur().mask & c->lookup_mask) &&
+		(*coverage) (c->buffer->cur().codepoint) != NOT_COVERED &&
+		apply_once (c))
+	      ret = true;
+	    else
+	      c->buffer->next_glyph ();
+	  }
+	else
+	  while (c->buffer->idx < c->buffer->len)
+	  {
+	    if ((c->buffer->cur().mask & c->lookup_mask) && apply_once (c))
+	      ret = true;
+	    else
+	      c->buffer->next_glyph ();
+
+	  }
 	if (ret)
 	  c->buffer->swap_buffers ();
     }
@@ -1211,6 +1316,11 @@ inline void ExtensionSubst::closure (hb_closure_context_t *c) const
   get_subtable ().closure (c, get_type ());
 }
 
+inline const Coverage & ExtensionSubst::get_coverage (void) const
+{
+  return get_subtable ().get_coverage (get_type ());
+}
+
 inline bool ExtensionSubst::would_apply (hb_would_apply_context_t *c) const
 {
   return get_subtable ().would_apply (c, get_type ());
diff --git a/src/hb-ot-layout-gsubgpos-private.hh b/src/hb-ot-layout-gsubgpos-private.hh
index 48caf6a..870916f 100644
--- a/src/hb-ot-layout-gsubgpos-private.hh
+++ b/src/hb-ot-layout-gsubgpos-private.hh
@@ -894,6 +894,16 @@ struct Context
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return this + u.format1.coverage;
+    case 2: return this + u.format2.coverage;
+    case 3: return this + u.format3.coverage[0];
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     switch (u.format) {
@@ -1340,6 +1350,12 @@ struct ChainContextFormat3
 				  lookup_context);
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    const OffsetArrayOf<Coverage> &input = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
+    return this+input[0];
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     TRACE_WOULD_APPLY ();
@@ -1430,6 +1446,16 @@ struct ChainContext
     }
   }
 
+  inline const Coverage &get_coverage (void) const
+  {
+    switch (u.format) {
+    case 1: return this + u.format1.coverage;
+    case 2: return this + u.format2.coverage;
+    case 3: return u.format3.get_coverage ();
+    default:return Null(Coverage);
+    }
+  }
+
   inline bool would_apply (hb_would_apply_context_t *c) const
   {
     switch (u.format) {
commit 30ec9002d84e8b49290e782e6192069821ffa942
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sat Jul 28 17:25:20 2012 -0400

    Reject lookups with no subTable

diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh
index c7e458f..fb7fd3b 100644
--- a/src/hb-ot-layout-common-private.hh
+++ b/src/hb-ot-layout-common-private.hh
@@ -313,6 +313,7 @@ struct Lookup
     TRACE_SANITIZE ();
     /* Real sanitize of the subtables is done by GSUB/GPOS/... */
     if (!(c->check_struct (this) && subTable.sanitize (c))) return TRACE_RETURN (false);
+    if (!subTable.len) TRACE_RETURN (false);
     if (unlikely (lookupFlag & LookupFlag::UseMarkFilteringSet))
     {
       USHORT &markFilteringSet = StructAfter<USHORT> (subTable);
commit 0981068b75710397f08e0d2d776a0a2ea68d7117
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sat Jul 28 17:01:59 2012 -0400

    [GSUB/GPOS] Reject Context/ChainContext lookups with zero input

diff --git a/src/hb-ot-layout-gsubgpos-private.hh b/src/hb-ot-layout-gsubgpos-private.hh
index 14d36b0..48caf6a 100644
--- a/src/hb-ot-layout-gsubgpos-private.hh
+++ b/src/hb-ot-layout-gsubgpos-private.hh
@@ -857,6 +857,7 @@ struct ContextFormat3
     TRACE_SANITIZE ();
     if (!c->check_struct (this)) return TRACE_RETURN (false);
     unsigned int count = glyphCount;
+    if (unlikely (!glyphCount)) return TRACE_RETURN (false);
     if (!c->check_array (coverage, coverage[0].static_size, count)) return TRACE_RETURN (false);
     for (unsigned int i = 0; i < count; i++)
       if (!coverage[i].sanitize (c, this)) return TRACE_RETURN (false);
@@ -1386,6 +1387,7 @@ struct ChainContextFormat3
     if (!backtrack.sanitize (c, this)) return TRACE_RETURN (false);
     OffsetArrayOf<Coverage> &input = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
     if (!input.sanitize (c, this)) return TRACE_RETURN (false);
+    if (unlikely (!input.len)) return TRACE_RETURN (false);
     OffsetArrayOf<Coverage> &lookahead = StructAfter<OffsetArrayOf<Coverage> > (input);
     if (!lookahead.sanitize (c, this)) return TRACE_RETURN (false);
     ArrayOf<LookupRecord> &lookup = StructAfter<ArrayOf<LookupRecord> > (lookahead);



More information about the HarfBuzz mailing list