The previous QIcon::name() check wasn't sufficient as it only works with theme icon names (e.g. kate) but not icons created from an absolute path, e.g. /some/special/icon/location/kate.png.
The latter is usually the case for containerized apps which have a proper application desktop file installed but their icon in some path within the application image or some container daemon location.
Since we already store the information for whether we had to fall back to using the actual window pixmap, check for this before trying to serialize icon pixmap data.
Details
Details
- Reviewers
hein - Group Reviewers
Plasma - Commits
- R120:b49d5cfde098: Only serialize window pixmap if we're using it
I noticed my Telegram Snap had its icon data serialized despite having found the correct desktop file/KService. With this patch it checks that it has a service which likely has a proper icon and doesn't bother serializing the window pixmap.
The service might not have a proper icon after all but then it also won't have one in the launcher to begin with, so I think this is a valid optimization, albeit only for master branch.
There is a KF6 task to address the QIcon issue: T12155
Diff Detail
Diff Detail
- Repository
- R120 Plasma Workspace
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
If the concern is that QIcon::name isn't good enough but we actually have an icon inside, why don't we just change the check to QIcon::isNull?
Comment Actions
why don't we just change the check to QIcon::isNull?
We still want to serialize custom pixmap data icons which won't be null.
What we want to do is tell between "icon on disk" and "icon from pixmap data" which we can't really at the moment.
Comment Actions
Ah right, I didn't look at the code context and forgot we don't just load the custom pixmap in the same function body but also in Private::icon. But we also do this there:
usingFallbackIcon.insert(window)
That means you can limit serializing to cases where the window is in usingFallbackIcon.
Comment Actions
That means you can limit serializing to cases where the window is in usingFallbackIcon.
Good idea, I'll give it a try.