[HB] When looking back/forward, skip marks only

src/TODO

@@ -5,7 +5,6 @@
 - Face index > 0 and dfont fonts
 - HB_OT_LAYOUT_GLYPH_CLASS_BASE_GLYPH vs LookupType::... mess
 - Rename LookupFlag::MarkAttachmentType to LookupFlag:IgnoreSpecialMarks
-- Skip forward/backward should only skip marks, not whatever lookupflags say!
 - layout vs font split
 - gpi again
 

src/hb-ot-layout-gpos-private.h

@@ -379,7 +379,7 @@ struct PairPosFormat1
       return false;
 
     unsigned int j = buffer->in_pos + 1;
-    while (!_hb_ot_layout_check_glyph_property (layout, IN_INFO (j), lookup_flag, &property))
+    while (_hb_ot_layout_skip_mark (layout, IN_INFO (j), lookup_flag, NULL))
     {
       if (HB_UNLIKELY (j == end))
 	return false;
@@ -444,7 +444,7 @@ struct PairPosFormat2
       return false;
 
     unsigned int j = buffer->in_pos + 1;
-    while (!_hb_ot_layout_check_glyph_property (layout, IN_INFO (j), lookup_flag, &property))
+    while (_hb_ot_layout_skip_mark (layout, IN_INFO (j), lookup_flag, NULL))
     {
       if (HB_UNLIKELY (j == end))
 	return false;
@@ -771,9 +771,6 @@ struct MarkBasePosFormat1
   private:
   inline bool apply (APPLY_ARG_DEF) const
   {
-    if  (lookup_flag & LookupFlag::IgnoreBaseGlyphs)
-      return false;
-
     unsigned int mark_index = (this+markCoverage) (IN_CURGLYPH ());
     if (HB_LIKELY (mark_index == NOT_COVERED))
       return false;
@@ -896,9 +893,6 @@ struct MarkLigPosFormat1
   private:
   inline bool apply (APPLY_ARG_DEF) const
   {
-    if  (lookup_flag & LookupFlag::IgnoreLigatures)
-      return false;
-
     unsigned int mark_index = (this+markCoverage) (IN_CURGLYPH ());
     if (HB_LIKELY (mark_index == NOT_COVERED))
       return false;
@@ -1034,9 +1028,6 @@ struct MarkMarkPosFormat1
   private:
   inline bool apply (APPLY_ARG_DEF) const
   {
-    if  (lookup_flag & LookupFlag::IgnoreMarks)
-      return false;
-
     unsigned int mark1_index = (this+mark1Coverage) (IN_CURGLYPH ());
     if (HB_LIKELY (mark1_index == NOT_COVERED))
       return false;

src/hb-ot-layout-gsub-private.h

@@ -45,11 +45,9 @@ struct SingleSubstFormat1
     glyph_id += deltaGlyphID;
     _hb_buffer_replace_glyph (buffer, glyph_id);
 
-    if ( _hb_ot_layout_has_new_glyph_classes (layout) )
-    {
-      /* we inherit the old glyph class to the substituted glyph */
+    /* We inherit the old glyph class to the substituted glyph */
+    if (_hb_ot_layout_has_new_glyph_classes (layout))
       _hb_ot_layout_set_glyph_property (layout, glyph_id, property);
-    }
 
     return true;
   }
@@ -82,11 +80,9 @@ struct SingleSubstFormat2
     glyph_id = substitute[index];
     _hb_buffer_replace_glyph (buffer, glyph_id);
 
-    if ( _hb_ot_layout_has_new_glyph_classes (layout) )
-    {
-      /* we inherit the old glyph class to the substituted glyph */
+    /* We inherit the old glyph class to the substituted glyph */
+    if (_hb_ot_layout_has_new_glyph_classes (layout))
       _hb_ot_layout_set_glyph_property (layout, glyph_id, property);
-    }
 
     return true;
   }
@@ -140,10 +136,10 @@ struct Sequence
 				  substitute.len, (const uint16_t *) substitute.array,
 				  0xFFFF, 0xFFFF);
 
-    if ( _hb_ot_layout_has_new_glyph_classes (layout) )
+    /* This is a guess only ... */
+    if (_hb_ot_layout_has_new_glyph_classes (layout))
     {
-      /* this is a guess only ... */
-      if ( property == HB_OT_LAYOUT_GLYPH_CLASS_LIGATURE )
+      if (property == HB_OT_LAYOUT_GLYPH_CLASS_LIGATURE)
         property = HB_OT_LAYOUT_GLYPH_CLASS_BASE_GLYPH;
 
       unsigned int count = substitute.len;
@@ -246,11 +242,9 @@ struct AlternateSubstFormat1
 
     _hb_buffer_replace_glyph (buffer, glyph_id);
 
-    if ( _hb_ot_layout_has_new_glyph_classes (layout) )
-    {
-      /* we inherit the old glyph class to the substituted glyph */
+    /* We inherit the old glyph class to the substituted glyph */
+    if (_hb_ot_layout_has_new_glyph_classes (layout))
       _hb_ot_layout_set_glyph_property (layout, glyph_id, property);
-    }
 
     return true;
   }
@@ -303,7 +297,7 @@ struct Ligature
 
     for (i = 1, j = buffer->in_pos + 1; i < count; i++, j++)
     {
-      while (!_hb_ot_layout_check_glyph_property (layout, IN_INFO (j), lookup_flag, &property))
+      while (_hb_ot_layout_skip_mark (layout, IN_INFO (j), lookup_flag, &property))
       {
 	if (HB_UNLIKELY (j + count - i == end))
 	  return false;
@@ -313,11 +307,11 @@ struct Ligature
       if (!(property & HB_OT_LAYOUT_GLYPH_CLASS_MARK))
 	is_mark = FALSE;
 
-      if (HB_LIKELY (IN_GLYPH(j) != component[i]))
+      if (HB_LIKELY (IN_GLYPH (j) != component[i]))
         return false;
     }
-    if ( _hb_ot_layout_has_new_glyph_classes (layout) )
-      /* this is just a guess ... */
+    /* This is just a guess ... */
+    if (_hb_ot_layout_has_new_glyph_classes (layout))
       hb_ot_layout_set_glyph_class (layout, ligGlyph,
 				    is_mark ? HB_OT_LAYOUT_GLYPH_CLASS_MARK
 					    : HB_OT_LAYOUT_GLYPH_CLASS_LIGATURE);
@@ -344,8 +338,8 @@ struct Ligature
 
       for ( i = 1; i < count; i++ )
       {
-	while (!_hb_ot_layout_check_glyph_property (layout, IN_CURINFO(), lookup_flag, &property))
-	  _hb_buffer_add_output_glyph (buffer, IN_CURGLYPH(), i - 1, lig_id);
+	while (_hb_ot_layout_skip_mark (layout, IN_CURINFO (), lookup_flag, NULL))
+	  _hb_buffer_add_output_glyph (buffer, IN_CURGLYPH (), i - 1, lig_id);
 
 	(buffer->in_pos)++;
       }
@@ -505,7 +499,7 @@ struct ReverseChainSingleSubstFormat1
 			 match_coverage, (char *) this,
 			 1))
     {
-      IN_CURGLYPH() = substitute[index];
+      IN_CURGLYPH () = substitute[index];
       buffer->in_pos--; /* Reverse! */
       return true;
     }

src/hb-ot-layout-gsubgpos-private.h

@@ -89,14 +89,14 @@ static inline bool match_input (APPLY_ARG_DEF,
 
   for (i = 1, j = buffer->in_pos + 1; i < count; i++, j++)
   {
-    while (!_hb_ot_layout_check_glyph_property (layout, IN_INFO (j), lookup_flag, &property))
+    while (_hb_ot_layout_skip_mark (layout, IN_INFO (j), lookup_flag, NULL))
     {
       if (HB_UNLIKELY (j + count - i == end))
 	return false;
       j++;
     }
 
-    if (HB_LIKELY (!match_func (IN_GLYPH(j), input[i - 1], match_data)))
+    if (HB_LIKELY (!match_func (IN_GLYPH (j), input[i - 1], match_data)))
       return false;
   }
 
@@ -116,14 +116,14 @@ static inline bool match_backtrack (APPLY_ARG_DEF,
 
   for (unsigned int i = 0, j = buffer->out_pos - 1; i < count; i++, j--)
   {
-    while (!_hb_ot_layout_check_glyph_property (layout, OUT_INFO (j), lookup_flag, &property))
+    while (_hb_ot_layout_skip_mark (layout, OUT_INFO (j), lookup_flag, NULL))
     {
       if (HB_UNLIKELY (j + 1 == count - i))
 	return false;
       j--;
     }
 
-    if (HB_LIKELY (!match_func (OUT_GLYPH(j), backtrack[i], match_data)))
+    if (HB_LIKELY (!match_func (OUT_GLYPH (j), backtrack[i], match_data)))
       return false;
   }
 
@@ -144,14 +144,14 @@ static inline bool match_lookahead (APPLY_ARG_DEF,
 
   for (i = 0, j = buffer->in_pos + offset; i < count; i++, j++)
   {
-    while (!_hb_ot_layout_check_glyph_property (layout, OUT_INFO (j), lookup_flag, &property))
+    while (_hb_ot_layout_skip_mark (layout, OUT_INFO (j), lookup_flag, NULL))
     {
       if (HB_UNLIKELY (j + count - i == end))
 	return false;
       j++;
     }
 
-    if (HB_LIKELY (!match_func (IN_GLYPH(j), lookahead[i], match_data)))
+    if (HB_LIKELY (!match_func (IN_GLYPH (j), lookahead[i], match_data)))
       return false;
   }
 
@@ -182,7 +182,7 @@ static inline bool apply_lookup (APPLY_ARG_DEF,
    *      Should be easy for in_place ones at least. */
   for (unsigned int i = 0; i < count; i++)
   {
-    while (!_hb_ot_layout_check_glyph_property (layout, IN_CURINFO (), lookup_flag, &property))
+    while (_hb_ot_layout_skip_mark (layout, IN_CURINFO (), lookup_flag, NULL))
     {
       if (HB_UNLIKELY (buffer->in_pos == end))
 	return true;

src/hb-ot-layout-private.h

@@ -88,6 +88,12 @@ _hb_ot_layout_check_glyph_property (hb_ot_layout_t  *layout,
 				    unsigned int     lookup_flags,
 				    unsigned int    *property);
 
+HB_INTERNAL hb_bool_t
+_hb_ot_layout_skip_mark (hb_ot_layout_t  *layout,
+			  hb_glyph_info_t *ginfo,
+			  unsigned int     lookup_flags,
+			  unsigned int    *property);
+
 HB_END_DECLS
 
 #endif /* HB_OT_LAYOUT_PRIVATE_H */

src/hb-ot-layout.cc

@@ -161,37 +161,26 @@ HB_INTERNAL hb_bool_t
 _hb_ot_layout_check_glyph_property (hb_ot_layout_t  *layout,
 				    hb_glyph_info_t *ginfo,
 				    unsigned int     lookup_flags,
-				    unsigned int    *property)
+				    unsigned int    *property_out)
 {
-  hb_ot_layout_glyph_class_t glyph_class;
-  unsigned int desired_attachment_class;
+  unsigned int property;
 
   if (ginfo->gproperty == HB_BUFFER_GLYPH_PROPERTIES_UNKNOWN)
-  {
-    ginfo->gproperty = *property = _hb_ot_layout_get_glyph_property (layout, ginfo->gindex);
-    if (ginfo->gproperty == HB_OT_LAYOUT_GLYPH_CLASS_UNCLASSIFIED)
-      return false;
-  }
-
-  *property = ginfo->gproperty;
-
-  /* If the glyph was found in the MarkAttachmentClass table,
-   * then that class value is stored in the high byte of the result.
-   * The low byte contains the basic type of the glyph as defined by
-   * the GlyphClassDef table.
-   */
-  glyph_class = (hb_ot_layout_glyph_class_t) *property;
+    ginfo->gproperty = _hb_ot_layout_get_glyph_property (layout, ginfo->gindex);
+  property = ginfo->gproperty;
+  if (property_out)
+    *property_out = property;
 
-  /* Not covered, if, for example, glyph_class is HB_GDEF_LIGATURE and
+  /* Not covered, if, for example, glyph class is ligature and
    * lookup_flags includes LookupFlags::IgnoreLigatures
    */
-  if (lookup_flags & glyph_class)
+  if (property & lookup_flags)
     return false;
 
-  if (glyph_class & HB_OT_LAYOUT_GLYPH_CLASS_MARK)
+  if (property & HB_OT_LAYOUT_GLYPH_CLASS_MARK)
   {
     /* If using mark filtering sets, the high short of
-     * lookup_flags hsa the set index.
+     * lookup_flags has the set index.
      */
     if (lookup_flags & LookupFlag::UseMarkFilteringSet)
       return layout->gdef->mark_set_covers (lookup_flags >> 16, ginfo->gindex);
@@ -200,14 +189,46 @@ _hb_ot_layout_check_glyph_property (hb_ot_layout_t  *layout,
      * "ignore marks of attachment type different than
      * the attachment type specified."
      */
-    if (lookup_flags & LookupFlag::MarkAttachmentType && *property & LookupFlag::MarkAttachmentType)
-      return (lookup_flags & LookupFlag::MarkAttachmentType) == (*property & LookupFlag::MarkAttachmentType);
+    if (lookup_flags & LookupFlag::MarkAttachmentType && property & LookupFlag::MarkAttachmentType)
+      return (lookup_flags & LookupFlag::MarkAttachmentType) == (property & LookupFlag::MarkAttachmentType);
   }
 
-
   return true;
 }
 
+HB_INTERNAL hb_bool_t
+_hb_ot_layout_skip_mark (hb_ot_layout_t  *layout,
+			 hb_glyph_info_t *ginfo,
+			 unsigned int     lookup_flags,
+			 unsigned int    *property_out)
+{
+  unsigned int property;
+
+  if (ginfo->gproperty == HB_BUFFER_GLYPH_PROPERTIES_UNKNOWN)
+    ginfo->gproperty = _hb_ot_layout_get_glyph_property (layout, ginfo->gindex);
+  property = ginfo->gproperty;
+  if (property_out)
+    *property_out = property;
+
+  if (property & HB_OT_LAYOUT_GLYPH_CLASS_MARK)
+  {
+    /* Skip mark if lookup_flags includes LookupFlags::IgnoreMarks */
+    if (lookup_flags & LookupFlag::IgnoreMarks)
+      return true;
+
+    /* If using mark filtering sets, the high short of lookup_flags has the set index. */
+    if (lookup_flags & LookupFlag::UseMarkFilteringSet)
+      return !layout->gdef->mark_set_covers (lookup_flags >> 16, ginfo->gindex);
+
+    /* The second byte of lookup_flags has the meaning "ignore marks of attachment type
+     * different than the attachment type specified." */
+    if (lookup_flags & LookupFlag::MarkAttachmentType && property & LookupFlag::MarkAttachmentType)
+      return (lookup_flags & LookupFlag::MarkAttachmentType) != (property & LookupFlag::MarkAttachmentType);
+  }
+
+  return false;
+}
+
 HB_INTERNAL void
 _hb_ot_layout_set_glyph_property (hb_ot_layout_t *layout,
 				  hb_codepoint_t  glyph,