Hi, Currently the QtWebKit class FontQt.cpp is using a very expensive method to effectively inform Qt's text engine that it should substitute glyphs for particular unicode characters with another unicode character. Right now FontQt.cpp is reconstructing a QString based on a TextRun and then detaching and memcpy'ing whenever it encounters a character that should be treated differently by Qt's text engine according to WebCore::Font::treatAsSpace and WebCore::Font::treatAsZeroWidthSpace. Profiling shows that this is one of the top ten most expensive methods in all of QtWebKit right now.
... The patches attached improve this situation by *never* detaching or recreating a new QString for these operations on a TextRun. The first patch is a patch to Qt itself that exposes two new methods in the QFont class which allow an application to pass a substitution map to Qt's underlying text engine. I've only implemented the patch for the Freetype engine in Qt, but other engines could/should be made to honor these substitutions. The second patch is a patch to FontQt.cpp in WebCore that utilizes the new API in Qfont. Together these two patches effectively eliminate the cost of the current method of doing things in FontQt.cpp. Cheers, Adam
*** Bug 24469 has been marked as a duplicate of this bug. ***
Created attachment 28419 [details] Patch for Qt that adds API to QFont
Created attachment 28420 [details] Patch to QtWebKit that takes advantage of new API
Note: for the QtWebKit patch to apply cleanly, you're webkit branch will need to have applied the change here first: http://trac.webkit.org/changeset/41527
Cool. Is that somehow possible to share the table with Font.cpp without runtime costs? In RenderThemeQt we have a style painter which is setting and clearing the QStyle. Could something like this be used for the substitution hints. Something like this http://doc.trolltech.com/4.4/qmutexlocker.html.
Instead of the API I think it would be cleaner to make Qt behave the way WebKit expects it to, without the need to modify the string at all. If necessary we could make this configurable through QTextOption or the text flags in the Qt namespace (like TextJustificationForced). I think it should be possible to implement this in a platform independent manner in qtextengine.cpp, see for example QTextEngine::itemize().
Comment on attachment 28420 [details] Patch to QtWebKit that takes advantage of new API The patch looks fine to me, but apparently Qt Software wants this done differently. I guess it could be reworked to only apply on older Qt versions up until the one that addresses the issue internally. Also a changelog is missing.
Does this have a clear path forward in Qt now? Any updates?
What is up with this Simon? Is there an internal Qt bug report for tracking this? or who is responsible for the Font code in Qt?
Is this patch still relevant?
(In reply to comment #11) > Is this patch still relevant? Another Triage and question remains... is this still relevant?
Added to QtWebKit QFont API wishlist at http://bugreports.qt.nokia.com/browse/QTBUG-15627
We'll be taking another route with the FontFastPath in QtWebKit. See bug 51106 for those developments.