KF5::BreezeIcons shared library to have all breeze/breeze-dark icons and default to breeze icons set
Needs ReviewPublic

Authored by cullmann on Sat, Nov 2, 4:09 PM.

Details

Summary

At the moment, one big problem for non-unices is the deployment of icons
Close to all things in KDE stuff relies on some breeze like icon theme
At the moment, we help out with some hack in KIconThemes to load a binary resource
This makes it impossible to e.g. support dark mode properly, as only one theme is around

This patch provides an optional dynamic library containing both breeze icon sets (normal
and dark) and sets the fallback theme to breeze on load

Applications wanting to have breeze icons available, can just link
with this and be done

Diff Detail

Repository
R266 Breeze Icons
Branch
breeze-icon-shared-lib
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18445
Build 18463: arc lint + arc unit
cullmann created this revision.Sat, Nov 2, 4:09 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSat, Nov 2, 4:09 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
cullmann requested review of this revision.Sat, Nov 2, 4:09 PM

Some reviewers.

I am not sure this is the "perfect" solution, but it will make deploying applications a lot easier given just linking against this library provides the icons most of them will need.

And it will allow later to be able to switch to the "dark" variant for the dark mode most platforms provide today.

cullmann added a subscriber: rempt.

Perhaps @rempt has some input on this, for the cross-platform aspects.

At the moment this patch will only create the shared lib if we have binary resources configured.
One could always create the lib and only compile in the resources if wanted.
That would allow e.g. applications like Kate to always link against this on all platforms.

broulik added a subscriber: broulik.Sat, Nov 2, 6:00 PM

Can you expand on that dark theme issue? We just colorize the same SVGs as needed.

At the moment, the hack in kiconthemes only allows to load one theme.
This means you have just "breeze".
If you e.g. want to switch to "breeze-dark" this fails as the icons will not be found.

ndavis added a comment.EditedSat, Nov 2, 7:50 PM

I don't think my review is really necessary since I lack the technical expertise to know what the right solution is, but +1 to light and dark Breeze icon support on other platforms.

This patch by itself will not make the dark mode work, but e.g. Kate will be able to use it as the color theme chooser will set breeze-dark as theme and the icons will be "there".
Beside this, it avoids loading 20MB of data, as before the resource file got loaded into memory on Windows, with this the dll is just mapped.

rempt added a comment.Sun, Nov 3, 2:20 PM

It sounds like it would be something very useful and easy to use. Right now, we package all icons Krita uses in rc files that are compiled in as well. Also, our icon loading has kind of evolved and we've got a bit of code for switching between dark and light icons ourselves.

This patch will still need some work to make dark mode available.
Kate will be able to switch to it via the color theme switcher, as that will just switch over to breeze-dark.

Could you elaborate on

"Also, our icon loading has kind of evolved and we've got a bit of code for switching between dark and light icons ourselves."

If you have some hints where to look in the Krita code for ideas how that is done, I would be grateful ;=)

If you e.g. want to switch to "breeze-dark" this fails as the icons will not be found.

But why would you need this anyway? At least with KIconEngine we colorize the icons dynamically. I don't have to switch to breeze-dark when I use a dark color scheme on Plasma at least.

If you e.g. want to switch to "breeze-dark" this fails as the icons will not be found.

But why would you need this anyway? At least with KIconEngine we colorize the icons dynamically. I don't have to switch to breeze-dark when I use a dark color scheme on Plasma at least.

Hmm, where does that happen?

And does that work for icons you get via QIcon::fromTheme?

For me, on Windows, just switching to dark mode via the color theme chooser nicely changes the palette colors but my icons stay "normal".

Hmm, where does that happen?

https://cgit.kde.org/kiconthemes.git/tree/src/kiconloader.cpp#n888

And does that work for icons you get via QIcon::fromTheme?

Yes. As long as you pass a QIcon to the button or widget instead of an already rendered QPixmap.

On Windows you probably don't use KIconEngine but whatever Qt provides by itself.
There's some discussion on a fdo symbolic icon colorization (@davidedmundson should know more), which would be useful having in Qt then?

Hmm, where does that happen?

https://cgit.kde.org/kiconthemes.git/tree/src/kiconloader.cpp#n888

And does that work for icons you get via QIcon::fromTheme?

Yes. As long as you pass a QIcon to the button or widget instead of an already rendered QPixmap.

On Windows you probably don't use KIconEngine but whatever Qt provides by itself.
There's some discussion on a fdo symbolic icon colorization (@davidedmundson should know more), which would be useful having in Qt then?

Hmm, all KIconThemes related stuff should be in the application installer.
Need to investigate this more.

Ok, I traced this now a bit.
The KIconLoaderPrivate::createIconImage processing seems already not to work for resource based themes on Linux.

To try, e.g. use Kate, add the binary resource icon theme to the right location

cp /local/cullmann/kde/usr/share/icons/breeze/breeze-icons.rcc ~/.local/share/kate/icontheme.rcc

Switch in Kate to dark mode with Settings -> Color Theme -> Breeze Dark.

Without the rcc file, this works.
With the rcc file, the icons stay dark and are more or less invisible.

As soon as we use the rcc file for the icons, we set an icon theme name.

If you have an icon theme name set, QIcon::fromTheme will do:

bool hasUserTheme = QIconLoader::instance()->hasUserTheme();
QIconEngine * const engine = (platformTheme && !hasUserTheme) ? platformTheme->createIconEngine(name)
                                           : new QIconLoaderEngine(name);

This IMHO means no longer KIconLoader will be used at all.
(On Windows it will never be used, as the platform theme doesn't use it there anyways I assume)

But yes, packaging Breeze Dark icons in addition will not help to fix this :/