improvement in Font::drawText: fix or only workaround?

[Simon Hausmann]

On Saturday 05 March 2005 09:29, Stefan Heimers wrote:
> Hi,
>
> I was reading the KDE CVS- Digest and found the following interesting
> optimization:
>
> Description:
> http://cvs-digest.org/?newissue=mar42005&type=Optimize
>
> Code:
> http://cvs-digest.org/?diff&file=kdelibs/khtml/rendering/render_text.cpp,v&
>version=1.267
>
>
> In the last section you can find this:
>
> // Due to some unfounded cause QPainter::drawText traverses the
> // *whole* string when painting, not only the specified
> // [pos, pos + len) segment. This makes painting *extremely* slow for
> // long render texts (in the order of several megabytes).
> // Hence, only hand over the piece of text of the actual inline text box
> QConstString cstr = QConstString(str + pos, len);
> p->drawText( x, y, cstr.string(), 0, len, d );
>
>
> It's a good thing that there was a problem discovered and speed
> improved. But I have three questions:
>
> 1) There is an additional QConstString element created.  Will this be
> automagically freed when not needed anymore? Or should it be deleted
> after the p->drawText?
> I think it's automatically deleted because it is local to the
> Font::drawText, but I am not sure because I don't know Qt and C++
> enough.

That's not a problem. QConstString doesn't create a copy of the data, it's 
cheap to construct and destruct.

> 2) Reading the comment it seams like a bug in Qt rather than khtml.
> Shouldn't it better be fixed in Qt or is there a good reason for Qt's
> behaviour? With a fix in Qt, you wouldn't need to create this
> unnecessary QConstString cstr and the fix would help everywhere, not
> only in Font::drawText.

I think the cause for the traversal is the following line in 
QPainter::drawText:

    bool simple = (dir == QPainter::Auto) && str.simpleText();

The call to checkSimpleText() through simpleText() is at least the only place 
I can find where the whole string is traversed.

What one could do instead is to utilize the fact that simpleText() caches the 
result of checkSimpleText().

Simon