[KFileItemModelRolesUpdater] Check icon validity before using it
Review Request #125998 - Created Nov. 8, 2015 and discarded
|Kai Uwe Broulik|
For some exotic file types where I don't have an icon (such as Audacity project, or Android package because the icon name and mimetype don't match up) I got a blank icon rather than the unknown filetype icon.
I now at least get an "unknown" icon. Not sure if the approach is worth it for such corner cases or okay, I cache the results because QIcon::fromTheme apparently causes loads of IO. (I just saw there's a KIconLoader::hasIcon() ... probably better to use that one?)
I would recommend not adding any cache here, the performance issues of QIcon::fromTheme have already been addressed in Qt, which now uses the gtk icon cache (which our ECM macros now call when installing icons).
This was done in e68d06714fa29f986d0fc7de324b79ea94493dfc in qtbase, which apparently will be for Qt 5.7, though.
My attempts at making KIconLoader::hasIcon faster worked, but didn't help with QIcon::fromTheme itself.
However if you can use hasIcon() here and https://git.reviewboard.kde.org/r/124811/ helps, that would be reason to commit it ;)
Only insert the key-value pair when the icon is available. Inserting empty strings only wastes memory, because the return value of a non existing key will be an empty string anyway ;)
Can you please add the same check to KFileItemModel::retrieveData()? We set the icon name directly when the mime type is already known.