[KStandardItemListWidget] Request the pixmap size we want and let the icon loader scale it
ClosedPublic

Authored by broulik on Jun 27 2019, 10:28 AM.

Details

Summary

I noticed that depending on the configured icon size it would spend a significant amount of time in KPixmapModifier::scale. I don't see a point in requesting a fixed icon size and then scale it down manually as opposed to having the KIconLoader do the scaling for us. Especially for SVGs it could then even serve us a properly rendered SVG for this size rather than a scaled down pixmap version.

Test Plan

I dug in git history to see where this came from but this has been in there forever as far as I can tell.
The only reason I can see is to explicitly prefer downsampling the icon pixmap over upsampling?

Tried with Breeze and Oxygen themes, didn't notice any difference. Small icons still switch to symbolic in Breeze.

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Jun 27 2019, 10:28 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptJun 27 2019, 10:28 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
broulik requested review of this revision.Jun 27 2019, 10:28 AM
broulik added inline comments.
src/kitemviews/kstandarditemlistwidget.cpp
1475

I also never hit this codepath now... perhaps that's a behavioral change from using KIconLoader which might pad the icons to QIcon which will just bluntly scale?

elvisangelaccio accepted this revision.Sun, Sep 15, 6:51 PM

Sorry for the delay.

If this change is still relevant, feel free to push it.

This revision is now accepted and ready to land.Sun, Sep 15, 6:51 PM
This revision was automatically updated to reflect the committed changes.