[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.

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. */
+    switch (lookup_type) {
-    return likely (lookup_type && lookup_type < Context) ||
+    case Single:		return u.single.get_coverage ();
-	   (hb_in_range<unsigned int> (lookup_type, Context, ChainContext) &&
+    case Pair:			return u.pair.get_coverage ();
-	    hb_in_range<unsigned int> (u.header.sub_format, 1, 2));
+    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) ||
+    if (!u.header.sub_format.sanitize (c))
-	(can_use_fast_path (lookup_type) && !u.header.coverage.sanitize (c, this)))
       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)
+
-    {
+    /* Fast path for lookups with one coverage only (which is most). */
-      if ((c->buffer->cur().mask & c->lookup_mask) && apply_once (c))
+    const Coverage *coverage = get_coverage ();
-	ret = true;
+    if (coverage)
-      else
+      while (c->buffer->idx < c->buffer->len)
-	c->buffer->idx++;
+      {
-    }
+	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 ();

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.
+    switch (lookup_type) {
-     * Note that ReverseChainSingle can also go through this but
+    case Single:		return u.single.get_coverage ();
-     * it's not worth the effort. */
+    case Multiple:		return u.multiple.get_coverage ();
-    return likely (lookup_type && lookup_type < Context) ||
+    case Alternate:		return u.alternate.get_coverage ();
-	   (hb_in_range<unsigned int> (lookup_type, Context, ChainContext) &&
+    case Ligature:		return u.ligature.get_coverage ();
-	    hb_in_range<unsigned int> (u.header.sub_format, 1, 2));
+    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) ||
+    if (!u.header.sub_format.sanitize (c))
-	(can_use_fast_path (lookup_type) && !u.header.coverage.sanitize (c, this)))
       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 ());

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) {