From 86c68c7a2c971efe8e35b1f1bd99401dc8b688d2 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 16 Feb 2016 16:07:20 +0700 Subject: [PATCH] [GPOS] Fix interaction of mark attachments and cursive chaining Fixes https://github.com/behdad/harfbuzz/issues/211 What happens in that bug is that a mark is attached to base first, then a second mark is cursive-chained to the first mark. This only "works" because it's in the Indic shaper where mark advances are not zeroed. Before, we didn't allow cursive to run on marks at all. Fix that. We also where updating mark major offsets at the end of GPOS, such that changes in advance of base will not change the mark attachment position. That was superior to the alternative (which is what Uniscribe does BTW), but made it hard to apply cursive to the mark after it was positioned. We could track major-direction offset changes and apply that to cursive in the post process, but that's a much trickier thing to do than the fix here, which is to immediately apply the major-direction advance-width offsets... Ie.: https://github.com/behdad/harfbuzz/issues/211#issuecomment-183194739 If this breaks any fonts, the font should be fixed to do mark attachment after all the advances are set up first (kerning, etc). Finally, this, still doesn't make us match Uniscribe, for I explained in that bug. Looks like Uniscribe applies minor-direction cursive adjustment immediate as well. We don't, and we like it our way, at least for now. Eg. the sequence in the test case does this: - The first subscript attaches with mark-to-base, moving in x only, - The second subscript attaches with cursive attachment to first subscript moving in x only, - A final context rule moves the first subscript up by 104 units. The way we do, the final shift-up, also shifts up the second subscript mark because it's cursively-attached. Uniscribe doesn't. We get: [ttaorya=0+1307|casubscriptorya=0@-242,104+-231|casubscriptnarroworya=0@20,104+507] while Uniscribe gets: [ttaorya=0+1307|casubscriptorya=0@-242,104+-211|casubscriptnarroworya=0+487] note the different y-offset of the last glyph. In our view, after cursive, things move together, period. --- src/hb-ot-layout-gpos-table.hh | 31 +++++++++--------- ...6c5d7b625f207bc0d874c67237aad6f1e9cd6f.ttf | Bin 0 -> 3868 bytes test/shaping/fonts/sha1sum/MANIFEST | 1 + test/shaping/tests/cursive-positioning.tests | 1 + 4 files changed, 18 insertions(+), 15 deletions(-) create mode 100644 test/shaping/fonts/sha1sum/706c5d7b625f207bc0d874c67237aad6f1e9cd6f.ttf diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index 22eb499e..59dddcf7 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -437,6 +437,22 @@ struct MarkArray : ArrayOf /* Array of MarkRecords--in Coverage orde o.attach_type() = ATTACH_TYPE_MARK; o.attach_chain() = (int) glyph_pos - (int) buffer->idx; buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; + { + unsigned int i = buffer->idx; + unsigned int j = glyph_pos; + hb_glyph_position_t *pos = buffer->pos; + assert (j < i); + if (HB_DIRECTION_IS_FORWARD (c->direction)) + for (unsigned int k = j; k < i; k++) { + pos[i].x_offset -= pos[k].x_advance; + pos[i].y_offset -= pos[k].y_advance; + } + else + for (unsigned int k = j + 1; k < i + 1; k++) { + pos[i].x_offset += pos[k].x_advance; + pos[i].y_offset += pos[k].y_advance; + } + } buffer->idx++; return_trace (true); @@ -917,9 +933,6 @@ struct CursivePosFormat1 TRACE_APPLY (this); hb_buffer_t *buffer = c->buffer; - /* We don't handle mark glyphs here. */ - if (unlikely (_hb_glyph_info_is_mark (&buffer->cur()))) return_trace (false); - const EntryExitRecord &this_record = entryExitRecord[(this+coverage).get_coverage (buffer->cur().codepoint)]; if (!this_record.exitAnchor) return_trace (false); @@ -1580,18 +1593,6 @@ propagate_attachment_offsets (hb_glyph_position_t *pos, unsigned int i, hb_direc { pos[i].x_offset += pos[j].x_offset; pos[i].y_offset += pos[j].y_offset; - - assert (j < i); - if (HB_DIRECTION_IS_FORWARD (direction)) - for (unsigned int k = j; k < i; k++) { - pos[i].x_offset -= pos[k].x_advance; - pos[i].y_offset -= pos[k].y_advance; - } - else - for (unsigned int k = j + 1; k < i + 1; k++) { - pos[i].x_offset += pos[k].x_advance; - pos[i].y_offset += pos[k].y_advance; - } } } diff --git a/test/shaping/fonts/sha1sum/706c5d7b625f207bc0d874c67237aad6f1e9cd6f.ttf b/test/shaping/fonts/sha1sum/706c5d7b625f207bc0d874c67237aad6f1e9cd6f.ttf new file mode 100644 index 0000000000000000000000000000000000000000..eb5c50c664e07f6095f998292d56fad7b39d07d5 GIT binary patch literal 3868 zcmbVPdvKK16+id;_L1yv9`cA397sY85c1qd!Xs@lJR%TCNFam+V!k|_*0xivgWy=j0V-9dGo3nC>oD3*W;)Zx{!z2L{hjZAn}^t$ z==aTc_nvdlIrn$YJ@?$ZB#0=H`bnjR(hVConR`v25ZOKk_ZOuV*|UfLvPUuJ?n26~OZqe)Bn zby7d*e9)-@t+$g>Xbb3MrrWhZi|G*uY{ef1y`{4w)MXp|bO`hf;8n>?WzjLyyXI2VikxSXbx`SUE+WrZGh=F)VFW&XT73kzM&6y`2X&xFw3>0qX~oGhV= zG+}=AxG=wPL0AV%)*fjad;77s4jjlkvAh1cx{J>=j_wrId%Jt<>Us`kiuA}w;^f)r zQxi`dkB+pyv$f>d^Kbmqcd7T62k$AV8s7ip@2W={@<)63X+1sr8v4}S|6U{m>v>>Z zMlJ&_urgSNb>}kI9ZNl#vxI8ZY99~({MfZUVqjp$pN>3yxy@fNSbZDPrD93s6LGrk z`r)U(9IW$R2tOO`yWe}U_d@sg*2QqHJU0`k61f__P65PQB@KBE92Zk#n zE!!2Ok2!oM{2lU=gEA>1o&YbzaYMyR)*Lo+AmT!~fZLy#GSw@m*cJ^EPa%54m=GHW#dKt*`a9 z)+JaryO*zU6|Bs1igzM6uK?K)kZlLDG|Hh|$^*U_G9$!DhFC5bu-rvog=Ihd>giKW z%8?`9lQn(0i+1HnRN@-L$|>+XI=?LZ#eGBfzqq#|a^BzHwsnU)HA6zOSSlEjGi!xQ zLL!TvrMIy1`Lvcc(k3b;?5Ddq4H1!8y3UzQ*U2Q{yx`=JIMwvYbWM|8nYhZHbs^uu zguzq7@#5$oGjHS{ciCQ8ZW2_wm5m za7Sh2we9K$yc5d8*C*@Ds&89ku$5+AWNLZGeS;zsuHemSAdOWSy&8Ut3d2BWSz@|$ zoU<)421-+OY$8YbFgiS0KWHw)M0Oz8XIDK$TKCM_vEsXpdqb181DjL?!^vUWZN;P1 zwccMht#~-IvYbjeNlbqPru10dFfdKkjjtAtne|Mh9Y)!BQWIP@iZyr6UAyuaQC(24 zW{jvYyeJ%Be==4#T$TA1p41G6TvXB(w=$r7sAj6|>O<;~u1~MH zsv`9R_azm7LH0OZ^+wrNKEvt#zs0>D9-7E8aec$VEaizz*Xgo(9+jCFQ5jL>DpB|h zz9Dk~c*`U&HZDGkj*0Oo?hsOvZCI-((s9rfgoh#{rdu(qETKP%q?)dX6F61L{9`jt zA9_RnDvyv?c^JX5n-!DUq$nnmfDJ{W((>|BT4!Hq-zMj(mfX#sNlD72WhO&pKKpk-gd&1^AFrw;1E zmr7NnQ9FguE2ki}QXl9_YN7qqO@8!)x+S(pQ7_KhuX~aJR1vB$3A3cV>AvU4qAXV1Aeg-*9bFgb00RLWz}Wp zz{=DoqL1n`*vLl7c!heNXHjpV<@&(UfMvn$9As)NzB~$DR2sJIK)(ll5p731O%L&z z(n~;J*MqOaz~ZJgh(Q^ta7@mVq9aU)6yM%+bqU7Lp4fR7vO`luU*lFkEktxo?{k%o zGwdi-15f4wEyvSoM3q;A(eq4JwEQq7TNz_Hj!kdyd3QVTDSCEr6d4KY*bwKM+t!b* z@cKdyYv`qgXt9^E1=?wOXRsv%pLjlg*eYrjDExau%edwsA`NtjZqmPmQ*6?sAtSR; zAD6L~o|ttDtcR73MNnjcYlH7HPZ%%vtomG*$@ugmKG`%1F8-6Dl^G&4MqvFcu*vae zjBKBM=DFD}ztRWkX^dhU%;%V;LMP5xu^7(*UV_2noyd`#LbM5p7UG;`M47NMeYJQ- j<}-Kt{K!CKMv0ln