I see you're trying to adjust the size of the pixmap, but if you are calculating the size anyway, why don't you have it just create a correctly-sized pixmap on the first try?

@alvinhochun Not sure I understand your question. The first try in the first run is the calculation of the correct size, as it's not known beforehand. After that, i.e. in later invocations of KisFpsDecoration::drawDecoration, there will be no two tries unless the pixmap size changed.

In D8701#166647, @poke1024 wrote:

@alvinhochun Not sure I understand your question. The first try in the first run is the calculation of the correct size, as it's not known beforehand. After that, i.e. in later invocations of KisFpsDecoration::drawDecoration, there will be no two tries unless the pixmap size changed.

I mean you can calculate the size, resize the pixmap if needed and then paint once., instead of painting an extra time if a resize is needed.

I mean you can calculate the size, resize the pixmap if needed and then paint once., instead of painting an extra time if a resize is needed.

Ok, I could add additional checks for not drawing on the first run, but it's really only the very first run, and it complicates the code, because there either needs to be code duplication for computing the size separately (not nice) or an additional if for each draw call for not drawing (not nice either). I prefer it this way.

Hi, @poke1024!

Can you just refactor the code, so that some function would return QStringList and another function just calculated the size of the and painted it in one go? Calculation and painting at the same time is not that hard:

As far as I can tell, the size of the pixmap can be calculated very easily:

QRect boundingRect = metrics.boundingRect(allStrings.join('\n'));

So, speaking truly, I don't see why it is called twice :)

In D8701#170018, @dkazakov wrote:

Hi, @poke1024!

Can you just refactor the code, so that some function would return QStringList and another function just calculated the size of the and painted it in one go? Calculation and painting at the same time is not that hard:

As far as I can tell, the size of the pixmap can be calculated very easily:

QRect boundingRect = metrics.boundingRect(allStrings.join('\n'));

So, speaking truly, I don't see why it is called twice :)

Concatenating with += is even less work than using join.
And couldn't the whole string be painted in one go instead of doing it line-by-line?

I'll have another look. But you guys forget that the drawing code also has two different offsets that need also be duplicated in the size code and that the size can only work when there's a painter instantiated that knows the current font. It all leads to code duplication in some form or to more complex code that needs to externalize those constants. More, it also makes future changes like adding color to single lines completely impossible. It's premature optimization, it removes all leeway without any gain. Having one additional draw once at the startup of Krita is really not an issue.

EDIT Maybe I should stress this again, as this does not seem to be clear: the loop runs two iterations only at the very first drawing of the canvas. After that, unless there is some drastic change in text lengths, for all time and until the sun becomes a red giant, the loop will have one iteration.

In D8701#170042, @poke1024 wrote:

I'll have another look. But you guys forget that the drawing code also has two different offsets that need also be duplicated in the size code and that the size can only work when there's a painter instantiated that knows the current font. It all leads to code duplication in some form or to more complex code that needs to externalize those constants.

I believe you can store a QFont object in KisFpsDecoration for use in metrics calculation and painting (with QPainter::setFont), which has to be obtained in the first place (probably with QFontDatabase::systemFont), but you can customize it which is a plus. This way you don't need a QPainter to calculate the metrics, do you?

More, it also makes future changes like adding color to single lines completely impossible. It's premature optimization, it removes all leeway without any gain. Having one additional draw once at the startup of Krita is really not an issue.

I guess you can keep using a QStringList and forget about the string concatenation that I mentioned :)

EDIT Maybe I should stress this again, as this does not seem to be clear: the loop runs two iterations only at the very first drawing of the canvas. After that, unless there is some drastic change in text lengths, for all time and until the sun becomes a red giant, the loop will have one iteration.

I guess it's kind of true, but I don't really like that for-loop when the code can be written in another clearer way.

*Gasp* the text is so big... IMO even 9/10pt would be enough. The new "shadow" effect doesn't really look like any drop shadow at all... perhaps even white with a normal pure black border would do the job better...