src/hb-ot-layout-gpos-table.hh | 70 ++++++++-- test/shaping/Makefile.am | 1 test/shaping/fonts/sha1sum/298c9e1d955f10f6f72c6915c3c6ff9bf9695cec.ttf |binary test/shaping/fonts/sha1sum/MANIFEST | 2 test/shaping/fonts/sha1sum/c4e48b0886ef460f532fb49f00047ec92c432ec0.ttf |binary test/shaping/tests/MANIFEST | 1 test/shaping/tests/cursive-positioning.tests | 2 7 files changed, 64 insertions(+), 12 deletions(-)
New commits: commit 6578575cc8aeb05341f2053039acfcd735707674 Author: Behdad Esfahbod <[email protected]> Date: Tue Aug 25 20:24:59 2015 +0100 [GPOS] Fix cursive connection with mix of RTL and non-RTL lookups See thread "Issue with cursive attachment" started by Khaled. Turned out fixing this wasn't as bad as I had assumed. I like the new code better; we now have a theoretical model of cursive connections that is easier to reason about. diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index 6dcada6..996f8b5 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -883,6 +883,9 @@ struct EntryExitRecord DEFINE_SIZE_STATIC (4); }; +static void +reverse_cursive_minor_offset (hb_glyph_position_t *pos, unsigned int i, hb_direction_t direction, unsigned int new_parent); + struct CursivePosFormat1 { inline void collect_glyphs (hb_collect_glyphs_context_t *c) const @@ -980,6 +983,13 @@ struct CursivePosFormat1 y_offset = -y_offset; } + /* If child was already connected to someone else, walk through its old + * chain and reverse the link direction, such that the whole tree of its + * previous connection now attaches to new parent. Watch out for case + * where new parent is on the path from old chain... + */ + reverse_cursive_minor_offset (pos, child, c->direction, parent); + pos[child].cursive_chain() = parent - child; if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction))) pos[child].y_offset = y_offset; @@ -1495,6 +1505,30 @@ struct GPOS : GSUBGPOS static void +reverse_cursive_minor_offset (hb_glyph_position_t *pos, unsigned int i, hb_direction_t direction, unsigned int new_parent) +{ + unsigned int j = pos[i].cursive_chain(); + if (likely (!j)) + return; + + j += i; + + pos[i].cursive_chain() = 0; + + /* Stop if we see new parent in the chain. */ + if (j == new_parent) + return; + + reverse_cursive_minor_offset (pos, j, direction, new_parent); + + if (HB_DIRECTION_IS_HORIZONTAL (direction)) + pos[j].y_offset = -pos[i].y_offset; + else + pos[j].x_offset = -pos[i].x_offset; + + pos[j].cursive_chain() = i - j; +} +static void fix_cursive_minor_offset (hb_glyph_position_t *pos, unsigned int i, hb_direction_t direction) { unsigned int j = pos[i].cursive_chain(); commit 7368da67244ea53195cd9b95a5c57485df695732 Author: Behdad Esfahbod <[email protected]> Date: Tue Aug 25 20:28:39 2015 +0100 [test] Add test for cursive-positioning with mixed directions Fails now. Fix coming. See thread "Issue with cursive attachment" started by Khaled. Test fonts were made by modifying test font from Khaled to add more anchors. diff --git a/test/shaping/Makefile.am b/test/shaping/Makefile.am index bab3b47..69779b1 100644 --- a/test/shaping/Makefile.am +++ b/test/shaping/Makefile.am @@ -40,6 +40,7 @@ TESTS = \ tests/arabic-feature-order.tests \ tests/cluster.tests \ tests/context-matching.tests \ + tests/cursive-positioning.tests \ tests/default-ignorables.tests \ tests/hangul-jamo.tests \ tests/indic-joiner-candrabindu.tests \ diff --git a/test/shaping/fonts/sha1sum/298c9e1d955f10f6f72c6915c3c6ff9bf9695cec.ttf b/test/shaping/fonts/sha1sum/298c9e1d955f10f6f72c6915c3c6ff9bf9695cec.ttf new file mode 100644 index 0000000..0d677a8 Binary files /dev/null and b/test/shaping/fonts/sha1sum/298c9e1d955f10f6f72c6915c3c6ff9bf9695cec.ttf differ diff --git a/test/shaping/fonts/sha1sum/MANIFEST b/test/shaping/fonts/sha1sum/MANIFEST index 072911f..1e78f0a 100644 --- a/test/shaping/fonts/sha1sum/MANIFEST +++ b/test/shaping/fonts/sha1sum/MANIFEST @@ -2,6 +2,7 @@ 191826b9643e3f124d865d617ae609db6a2ce203.ttf 226bc2deab3846f1a682085f70c67d0421014144.ttf 270b89df543a7e48e206a2d830c0e10e5265c630.ttf +298c9e1d955f10f6f72c6915c3c6ff9bf9695cec.ttf 37033cc5cf37bb223d7355153016b6ccece93b28.ttf 4cce528e99f600ed9c25a2b69e32eb94a03b4ae8.ttf 5028afb650b1bb718ed2131e872fbcce57828fff.ttf @@ -15,6 +16,7 @@ a919b33197965846f21074b24e30250d67277bce.ttf bb29ce50df2bdba2d10726427c6b7609bf460e04.ttf bb9473d2403488714043bcfb946c9f78b86ad627.ttf +c4e48b0886ef460f532fb49f00047ec92c432ec0.ttf d629e7fedc0b350222d7987345fe61613fa3929a.ttf df768b9c257e0c9c35786c47cae15c46571d56be.ttf e207635780b42f898d58654b65098763e340f5c7.ttf diff --git a/test/shaping/fonts/sha1sum/c4e48b0886ef460f532fb49f00047ec92c432ec0.ttf b/test/shaping/fonts/sha1sum/c4e48b0886ef460f532fb49f00047ec92c432ec0.ttf new file mode 100644 index 0000000..99cda16 Binary files /dev/null and b/test/shaping/fonts/sha1sum/c4e48b0886ef460f532fb49f00047ec92c432ec0.ttf differ diff --git a/test/shaping/tests/MANIFEST b/test/shaping/tests/MANIFEST index 0d97806..b4ee18e 100644 --- a/test/shaping/tests/MANIFEST +++ b/test/shaping/tests/MANIFEST @@ -2,6 +2,7 @@ arabic-fallback-shaping.tests arabic-feature-order.tests cluster.tests context-matching.tests +cursive-positioning.tests default-ignorables.tests hangul-jamo.tests indic-joiner-candrabindu.tests diff --git a/test/shaping/tests/cursive-positioning.tests b/test/shaping/tests/cursive-positioning.tests new file mode 100644 index 0000000..b61d0c1 --- /dev/null +++ b/test/shaping/tests/cursive-positioning.tests @@ -0,0 +1,2 @@ +fonts/sha1sum/c4e48b0886ef460f532fb49f00047ec92c432ec0.ttf::U+0643,U+0645,U+0645,U+062B,U+0644:[gid8=4+738|gid5=3@441,1197+0|gid6=3@0,432+405|gid9=2@0,477+452|gid9=1@0,977+452|gid10=0@20,1577+207] +fonts/sha1sum/298c9e1d955f10f6f72c6915c3c6ff9bf9695cec.ttf::U+0643,U+0645,U+0645,U+062B,U+0644:[gid8=4+738|gid5=3@441,1197+0|gid6=3@0,432+405|gid9=2@0,477+500|gid9=1@0,577+452|gid10=0@20,1177+207] commit 58f2a73fb95af42e264a91cdef7bb5a89e965601 Author: Behdad Esfahbod <[email protected]> Date: Tue Aug 25 18:55:34 2015 +0100 [GPOS] Rewrite cursive attachment slightly differently In anticipation for upcoming fix for bug reported by Khaled in thread "Issue with cursive attachment". diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index da9506c..6dcada6 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -960,20 +960,32 @@ struct CursivePosFormat1 } /* Cross-direction adjustment */ - if (c->lookup_props & LookupFlag::RightToLeft) { - pos[i].cursive_chain() = j - i; - if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction))) - pos[i].y_offset = entry_y - exit_y; - else - pos[i].x_offset = entry_x - exit_x; - } else { - pos[j].cursive_chain() = i - j; - if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction))) - pos[j].y_offset = exit_y - entry_y; - else - pos[j].x_offset = exit_x - entry_x; + + /* We attach child to parent (think graph theory and rooted trees whereas + * the root stays on baseline and each node aligns itself against its + * parent. + * + * Optimize things for the case of RightToLeft, as that's most common in + * Arabinc. */ + unsigned int child = i; + unsigned int parent = j; + hb_position_t x_offset = entry_x - exit_x; + hb_position_t y_offset = entry_y - exit_y; + if (!(c->lookup_props & LookupFlag::RightToLeft)) + { + unsigned int k = child; + child = parent; + parent = k; + x_offset = -x_offset; + y_offset = -y_offset; } + pos[child].cursive_chain() = parent - child; + if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction))) + pos[child].y_offset = y_offset; + else + pos[child].x_offset = x_offset; + buffer->idx = j; return TRACE_RETURN (true); } _______________________________________________ HarfBuzz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/harfbuzz
