Skip to content

Commit

Permalink
[GPOS] Fix interaction of mark attachments and cursive chaining
Browse files Browse the repository at this point in the history
Fixes #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.:

#211 (comment)

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.
  • Loading branch information
behdad committed Feb 16, 2016
1 parent 80c8855 commit 86c68c7
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 15 deletions.
31 changes: 16 additions & 15 deletions src/hb-ot-layout-gpos-table.hh
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,22 @@ struct MarkArray : ArrayOf<MarkRecord> /* 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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
}
}

Expand Down
Binary file not shown.
1 change: 1 addition & 0 deletions test/shaping/fonts/sha1sum/MANIFEST
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
5a5daf5eb5a4db77a2baa3ad9c7a6ed6e0655fa8.ttf
641bd9db850193064d17575053ae2bf8ec149ddc.ttf
6466d38c62e73a39202435a4f73bf5d6acbb73c0.ttf
706c5d7b625f207bc0d874c67237aad6f1e9cd6f.ttf
757ebd573617a24aa9dfbf0b885c54875c6fe06b.ttf
7a37dc4d5bf018456aea291cee06daf004c0221c.ttf
7e14e7883ed152baa158b80e207b66114c823a8b.ttf
Expand Down
1 change: 1 addition & 0 deletions test/shaping/tests/cursive-positioning.tests
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
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]
fonts/sha1sum/706c5d7b625f207bc0d874c67237aad6f1e9cd6f.ttf::U+0B1F,U+0B4D,U+0B1A,U+0B4D,U+0B1A:[ttaorya=0+1307|casubscriptorya=0@-242,104+-231|casubscriptnarroworya=0@20,104+507]

0 comments on commit 86c68c7

Please sign in to comment.