[HarfBuzz] Placement of arabic diacritics over 3 component ligature is incorrent
Going through old Pango bugs to see if there is anything HarfBuzz related, I found this one which is still reproducible with HarfBuzz (marks over 2nd component of لله are placed over the 3rd one, marks over the other two components are placed correctly): https://bugzilla.gnome.org/show_bug.cgi?id=437633 Regards, Khaled ___ HarfBuzz mailing list HarfBuzz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/harfbuzz
[HarfBuzz] harfbuzz-ng: Branch 'master'
src/hb-ot-layout-gpos-table.hh
| 27 +++---
src/hb-ot-layout-private.hh
|5 +
test/shaping/texts/in-tree/shaper-indic/MANIFEST
|1
test/shaping/texts/in-tree/shaper-indic/indic/script-sinhala/misc/MANIFEST
|1
test/shaping/texts/in-tree/shaper-indic/south-asian/MANIFEST
|1
test/shaping/texts/in-tree/shaper-indic/south-asian/script-tibetan/MANIFEST
|1
test/shaping/texts/in-tree/shaper-indic/south-asian/script-tibetan/misc/MANIFEST
|1
test/shaping/texts/in-tree/shaper-indic/south-asian/script-tibetan/misc/misc.txt
|1
8 files changed, 29 insertions(+), 9 deletions(-)
New commits:
commit 5d874d566fe5d2cc4cfaf02c79b663d8a626ca1e
Author: Behdad Esfahbod
Date: Sat Jul 28 21:05:25 2012 -0400
[GPOS] Fix mark-to-mark positioning when one of the marks is a ligature
This commit: a3313e54008167e415b72c780ca7b9cda958d07e broke MarkMarkPos
when one of the marks itself is a ligature. That regressed 26 Tibetan
tests (up from zero!). Fix that. Tibetan back to zero.
diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh
index 3b1ae2a..83252c1 100644
--- a/src/hb-ot-layout-gpos-table.hh
+++ b/src/hb-ot-layout-gpos-table.hh
@@ -1263,14 +1263,27 @@ struct MarkMarkPosFormat1
unsigned int j = skippy_iter.idx;
-/* Two marks match only if they belong to the same base, or same component
- * of the same ligature. That is, the lig_id numbers must match, and
- * if those are non-zero, the lig_comp number should also match. */
-if ((get_lig_id (c->buffer->info[j]) != get_lig_id (c->buffer->cur())) ||
- (get_lig_id (c->buffer->info[j]) > 0 &&
-get_lig_comp (c->buffer->info[j]) != get_lig_comp (c->buffer->cur(
- return TRACE_RETURN (false);
+unsigned int id1 = get_lig_id (c->buffer->cur());
+unsigned int id2 = get_lig_id (c->buffer->info[j]);
+unsigned int comp1 = get_lig_comp (c->buffer->cur());
+unsigned int comp2 = get_lig_comp (c->buffer->info[j]);
+
+if (likely (id1 == id2)) {
+ if (id1 == 0) /* Marks belonging to the same base. */
+ goto good;
+ else if (comp1 == comp2) /* Marks belonging to the same ligature
component. */
+goto good;
+} else {
+ /* If ligature ids don't match, it may be the case that one of the marks
+ * itself is a ligature. In which case match. */
+ if ((id1 > 0 && !comp1) || (id2 > 0 && !comp2))
+ goto good;
+}
+
+/* Didn't match. */
+return TRACE_RETURN (false);
+good:
unsigned int mark2_index = (this+mark2Coverage)
(c->buffer->info[j].codepoint);
if (mark2_index == NOT_COVERED) return TRACE_RETURN (false);
diff --git a/src/hb-ot-layout-private.hh b/src/hb-ot-layout-private.hh
index 294c359..1c108e0 100644
--- a/src/hb-ot-layout-private.hh
+++ b/src/hb-ot-layout-private.hh
@@ -73,7 +73,8 @@ _hb_ot_layout_skip_mark (hb_face_t*face,
*
* When a ligature is formed:
*
- * - The ligature glyph and any marks in between all get a unique lig_id,
+ * - The ligature glyph and any marks in between all the same newly allocated
+ * lig_id,
* - The ligature glyph will get lig_comp = 0
* - The marks get lig_comp > 0, reflecting which component of the ligature
* they were applied to.
@@ -84,7 +85,7 @@ _hb_ot_layout_skip_mark (hb_face_t*face,
*
* - All resulting glyphs will have lig_id = 0,
* - The resulting glyphs will have lig_comp = 0, 1, 2, ... respectively.
- * - This is used in GPOS to attack marks to the first component of a
+ * - This is used in GPOS to attach marks to the first component of a
* multiple substitution in MarkBasePos.
*
* The numbers are also used in GPOS to do mark-to-mark positioning only
diff --git a/test/shaping/texts/in-tree/shaper-indic/MANIFEST
b/test/shaping/texts/in-tree/shaper-indic/MANIFEST
index 3f2011f..5e0651b 100644
--- a/test/shaping/texts/in-tree/shaper-indic/MANIFEST
+++ b/test/shaping/texts/in-tree/shaper-indic/MANIFEST
@@ -1,2 +1,3 @@
indic
+south-asian
south-east-asian
diff --git
a/test/shaping/texts/in-tree/shaper-indic/indic/script-sinhala/misc/MANIFEST
b/test/shaping/texts/in-tree/shaper-indic/indic/script-sinhala/misc/MANIFEST
index 3c2a4fb..7eff9e1 100644
--- a/test/shaping/texts/in-tree/shaper-indic/indic/script-sinhala/misc/MANIFEST
+++ b/test/shaping/texts/in-tree/shaper-indic/indic/script-sinhala/misc/MANIFEST
@@ -1,2 +1,3 @@
+extensive.txt
misc.txt
reph.txt
diff --git a/test/shaping/texts/in-tree/shaper-indic/south-asian/MANIFEST
b/test/shaping/texts/in-tree/shaper-indic/south-asian/MANIFEST
new file mode 100644
index 000..3ed6c85
--- /dev/null
+++ b/test/shaping/texts/in-tree/shaper-indic/south-asian/MANIFEST
@@ -0,0 +1 @@
+script-tibetan
diff --git
a/test/shaping/texts/in-tree/shaper-i
[HarfBuzz] And more optimizations
I pushed another set of optimizations. This gives me over 20% improvement on Indic. To recap, with my Indic wordlist test case (which is, admittedly, not representative of most usecases), since last week, HarfBuzz has become twice faster. Before it was taking 45 seconds, now it does 23 seconds. That's faster than the hb-old backend (25s). behdad ___ HarfBuzz mailing list HarfBuzz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/harfbuzz
[HarfBuzz] harfbuzz-ng: Branch 'master' - 6 commits
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
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 &list =
CastR > (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
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
