I agree with David that this is a hack. Also I think it will fail for multi screen with different dpi.

Personally, I disagree it's hack. It's just defensively written: It makes sure users of the Svg API don't need to understand the order in which they need to call setScaleFactor and setImagePath for good performance. For something performance-critical I'd prefer not to have to check every six months whether everyone is using it correctly. Sure, cleaning up the call site is nice anyway, but it won't guard against future call sites being wrong. Self-tuning code is smart and practical.

It's also going to work fine with multiple screens. Svgs are usually instanciated in batches. If it's a screen with a different scale factor the first in the batch will hit the old slow path, all subsequent ones will be fast.

how is this one method different from setSize setColorGroup setContainsMultipleImages setUseSystemColors etc.?

In D9119#174887, @davidedmundson wrote:

how is this one method different from setSize setColorGroup setContainsMultipleImages setUseSystemColors etc.?

Most of those don't show up at 2% during my plasmashell startup. The scale factor penalty hits *every SVG* on a scaled system.

So I worked out why this happens. We have API like this:

SvgItem {

svg: Svg { imagePath: "..." }

}

Without my patch, Svg is instanciated with a scale factor of 1.0 always. Then setImagePath is called on it first. Then SvgItem::setSvg is called with the Svg instance and calls setScaleFactor on it.

In this pattern, SvgItem doesn't get a chance to set the scale factor before imagePath, because it's not the one setting the latter.

The only call site fix here would be to implement QQmlParserStatus in Svg and then set scaleFactor from QML everywhere by accessing Units. But this hygiene would be really hard to enforce, especially in third-party widgets.

I still consider my patch to be a fairly pragmatic solution to this.