Summary: | Light theme icons randomly get installed in breeze-dark theme | ||
---|---|---|---|
Product: | [Plasma] Breeze | Reporter: | Andreas Ferber <af+kde> |
Component: | Icons | Assignee: | Rodney Dawes <dobey.pwns> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | dobey.pwns, kainz.a, KDE, med.medin.2014, mk47blast, mo78, nate, RaitaroHikami |
Priority: | NOR | Keywords: | regression |
Version: | master | ||
Target Milestone: | --- | ||
Platform: | Compiled Sources | ||
OS: | Linux | ||
See Also: |
https://bugs.kde.org/show_bug.cgi?id=444095 https://bugs.kde.org/show_bug.cgi?id=445518 |
||
Latest Commit: | https://invent.kde.org/frameworks/breeze-icons/commit/250a7d0bbe6dd8e7d2e7cba4cc82a2d8cce37cf1 | Version Fixed In: | 5.89 |
Description
Andreas Ferber
2021-11-14 14:51:27 UTC
It took me a while to realize what the mentioned commits were actually trying to achieve. The underlying issue of bug It took me a while to realize what the mentioned commits actually were trying to achieve. The underlying issue of bug 444095 is that according to https://specifications.freedesktop.org/icon-theme-spec/latest/ar01s05.html if an icon doesn't exist in the requested size in a theme then falling back to a different size from the same theme takes precedence over falling back to a parent theme. According to the KDE HIG for the mimetypes and places categories monochrome icons should be used for sizes 16 and 22 while colorful icons are used from 32 upwards. The breeze-dark theme only contains the monochrome sizes of those icons, and because of the lookup rules those were then used for all sizes. So what Rodney Dawes was trying to do is installing those "colorful" sizes from the breeze theme into the breeze-dark theme as well, but his solution copies a lot more than just that. After further investigation I think https://invent.kde.org/frameworks/breeze-icons/-/commit/1b92cfc450f6ab6b72ed9ef69c052e4624e5a040 needs to be reverted too, as that appears to be the root cause of bug 444095 and the ill-fated attempt to fix it. Because of the icon lookup rules mentioned in my previous comment the way duplicates between icons and icons-dark were removed actually broke *all* icons where only certain sizes have tweaked dark-mode versions. I thiink the only clean way to achieve the goal of not having duplicate files between the two themes is to first rename the `icons` and `icons-dark` folders in the source tree to `breeze` and `breeze-dark` respectively in order to be able to make symlinks between the two themes that don't break when getting installed (I don't think cmake can adjust symlink targets during install), and then replacing duplicate files with symlinks. Only in cases where *all* sizes are identical between breeze and breeze-dark can the icon safely be removed from breeze-dark completely. The alternatives would be to either make a separate list of all breeze icons that need to be installed into breeze-dark in order to make all sizes work properly, or make a complex heuristic that determines which icons need to be copied at install time. Both are IMO inferior to the symlink approach outlined above. A list would have to be maintained and is easy to miss when adding new tweaked icons to the dark theme. A heuristic OTOH would most likely be fragile and prone to future breakage. And both approaches would only remove the duplicates in the source tree, the installed versions of the themes would still have duplicated files while the symlink approach would remove duplicates there as well. A possibly relevant merge request was started @ https://invent.kde.org/frameworks/breeze-icons/-/merge_requests/171 A quick fix for this issue is to use the `CODE` option of CMake's `install` command, so that we instead do the copy ourselves, and ignore the timestamps when copying. I've opened a PR which does this. Renaming the directories in the repository and creating a symlink farm would be better for the installed themes, as it would take up much less disk space, but maintaining it in repo would be nearly as daunting as having to ensure duplicate icons are kept updated in the repo as well. *** Bug 446253 has been marked as a duplicate of this bug. *** *** Bug 446293 has been marked as a duplicate of this bug. *** *** Bug 446383 has been marked as a duplicate of this bug. *** Git commit 250a7d0bbe6dd8e7d2e7cba4cc82a2d8cce37cf1 by Rodney Dawes. Committed on 03/12/2021 at 17:49. Pushed by ngraham into branch 'master'. Install dark icons via execute_process to use copy command In order to ensure the dark icons are copied over the light icons which are installed into the dark theme for BUG:444095, we need to execute the copy command ourselves rather than using CMake's internal copy mechanism used by the normal install command, so we use CODE mode of install to execute_process and print an appropriate status message. M +28 -2 icons-dark/CMakeLists.txt https://invent.kde.org/frameworks/breeze-icons/commit/250a7d0bbe6dd8e7d2e7cba4cc82a2d8cce37cf1 |