[webkit-reviews] review granted: [Bug 51106] [Qt] Implement the fast font path for Qt : [Attachment 92225] Proposed patch v4

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Andreas Kling
<kling at webkit.org>'s request for review:
Bug 51106: [Qt] Implement the fast font path for Qt
https://bugs.webkit.org/show_bug.cgi?id=51106

Attachment 92225: Proposed patch v4
https://bugs.webkit.org/attachment.cgi?id=92225&action=review

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

r=me

Follow-up stuff:

    1) Implement stroking and shadowing for drawGlyphs (maybe in Qt)
    2) Extend QRawFont APIs so that we can pass const QChar* str, int len and
get glyphs/advances in out-arrays, so that we can use QVarLengthArray on the
WebKit side
    3) Other stuff I forgot ;)

>> Source/WebCore/platform/graphics/qt/FontCacheQt.cpp:101
>> +	//return getCachedFontData(fontDescription, fallbackFamily);
> 
> Should have a space between // and comment  [whitespace/comments] [4]

Remove this before landing :)

> Source/WebCore/platform/graphics/qt/FontQt.cpp:337
> +    QVector<quint32> glyphIndexes;
> +    QVector<QPointF> positions;

These should be QVarLengthArrays. Fix before landing I'd say.

> Source/WebCore/platform/graphics/qt/FontQt.cpp:441
> +bool Font::primaryFontHasGlyphForCharacter(UChar32) const
> +{
> +    notImplemented();
> +    return true;
> +}

This should be pretty easy to implement now, no?

> Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp:138
> +    QVector<quint32> indexes = rawFont.glyphIndexesForString(QLatin1String("
"));

Urgh, I think we really need QRawFont::glyphIndexForCharacter, or better: an
overload that takes a const QChar* and int len.