[webkit-reviews] review denied: [Bug 100050]=?UTF-8?Q?=20Font=E2=80=99s=20fast=20code=20path=20doesn=E2=80=99t=20handle=20partial=20runs=20correctly=20when=20kerning=20or=20ligatures=20are=20enabled=20?=: [Attachment 199667] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org <mitz at webkit.org> has denied Allan Sandfeld Jensen
<allan.jensen at digia.com>'s request for review:
Bug 100050: Font’s fast code path doesn’t handle partial runs correctly when
kerning or ligatures are enabled
https://bugs.webkit.org/show_bug.cgi?id=100050

Attachment 199667: Patch
https://bugs.webkit.org/attachment.cgi?id=199667&action=review

------- Additional Comments from mitz at webkit.org <mitz at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=199667&action=review


> Source/WebCore/platform/graphics/WidthIterator.cpp:115
> +    // Transforms may also change the advance of the preceding character if
there is one.
> +    int transformStart = lastGlyphCount > 0 ? lastGlyphCount - 1 : 0;

I don’t understand this. It is not necessarily true that
glyphBuffer->fontDataAt(lastGlyphCount - 1) == fontData, but it makes no sense
to pass glyphs from a different font to SimpleFontData::applyTransforms(). I
also don’t understand why this consideration applies to the glyph just before
lastGlyphCount but not to the glyph before that.