Bug 106013 – [Qt] Support kerning in fast path font rendering

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Allan Sandfeld Jensen]

The fast path font rendering has support for kerning, but to do so requires the SimpleFontData::applyTransforms method. Fortunately the method is available in QRawFont under the name advancesForGlyphIndexes. By tweaking the GlyphBufferGlyph and GlyphBufferAdvance types similar to what Safari does, this method can be called directly.

[Allan Sandfeld Jensen]

Created attachment 181169 [details]
Patch

[Allan Sandfeld Jensen]

Comment on attachment 181169 [details]
Patch

This patch is missing a change to ensure kerning is set on QRawFont.

[Allan Sandfeld Jensen]

Actually it seems I have tested this incorrectly last time, or there have been a regression in Qt. QRawFont::advancesForGlyphIndexes does not currently perform kerning.

[Allan Sandfeld Jensen]

Created attachment 193128 [details]
Patch

Updated to new proposed Qt 5.1 API

[Allan Sandfeld Jensen]

Created attachment 193769 [details]
Patch

[EFL EWS Bot]

Comment on attachment 193769 [details]
Patch

Attachment 193769 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17238256

[Early Warning System Bot]

Comment on attachment 193769 [details]
Patch

Attachment 193769 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17116699

[Early Warning System Bot]

Comment on attachment 193769 [details]
Patch

Attachment 193769 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17247024

[WebKit Review Bot]

Comment on attachment 193769 [details]
Patch

Attachment 193769 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17194461

[WebKit Review Bot]

Comment on attachment 193769 [details]
Patch

Attachment 193769 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17194463

[Allan Sandfeld Jensen]

Created attachment 193772 [details]
Patch

[EFL EWS Bot]

Comment on attachment 193772 [details]
Patch

Attachment 193772 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17160655

[Allan Sandfeld Jensen]

Created attachment 193774 [details]
Patch

Do not use QT_VERSION_CHECK() in parts that other ports needs to parse

[Pierre Rossi]

Comment on attachment 193774 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193774&action=review

LGTM

> Source/WebCore/platform/graphics/GlyphBuffer.h:95
> +struct GlyphBufferAdvance : public QPointF {

Just out of curiosity, why not use QSizeF here ? Wouldn't a simple typedef do the trick even since the function signatures match already ?

[Allan Sandfeld Jensen]

(In reply to comment #14)
> (From update of attachment 193774 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193774&action=review
> 
> LGTM
> 
> > Source/WebCore/platform/graphics/GlyphBuffer.h:95
> > +struct GlyphBufferAdvance : public QPointF {
> 
> Just out of curiosity, why not use QSizeF here ? Wouldn't a simple typedef do the trick even since the function signatures match already ?

The problem is that QRawFont::advancesForGlyphIndexes takes a pointer to an QPointF array as an argument. So I want a GlyphBufferAdvance class that is also makes for a valid QPointF pointer.

[Pierre Rossi]

Comment on attachment 193774 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193774&action=review

>>> Source/WebCore/platform/graphics/GlyphBuffer.h:95
>>> +struct GlyphBufferAdvance : public QPointF {
>> 
>> Just out of curiosity, why not use QSizeF here ? Wouldn't a simple typedef do the trick even since the function signatures match already ?
> 
> The problem is that QRawFont::advancesForGlyphIndexes takes a pointer to an QPointF array as an argument. So I want a GlyphBufferAdvance class that is also makes for a valid QPointF pointer.

Right. So there is no way around it indeed. so now we could also save a line in Font::drawGlyphs in FontQt.cpp ;)

[Allan Sandfeld Jensen]

Created attachment 193818 [details]
Patch

Moving enabling kerning by default to a separate patch

[Jocelyn Turcotte]

Comment on attachment 193818 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193818&action=review

> Source/WebCore/platform/graphics/SimpleFontData.h:209
> +#elif PLATFORM(QT) && QT_VERSION >= 0x050100

QT_VERSION_CHECK?

> Source/WebCore/platform/graphics/WidthIterator.h:69
> +#elif PLATFORM(QT) && QT_VERSION >= 0x050100

ditto

LGTM too for the rest, r=me.

[Allan Sandfeld Jensen]

Committed r146209: <http://trac.webkit.org/changeset/146209>