Bug 487235 - Some Breeze (light) icons not recolored for the Breeze Dark package
Summary: Some Breeze (light) icons not recolored for the Breeze Dark package
Status: RESOLVED FIXED
Alias: None
Product: Breeze
Classification: Plasma
Component: Icons (show other bugs)
Version: 6.0.4
Platform: unspecified All
: NOR normal
Target Milestone: ---
Assignee: visual-design
URL:
Keywords:
: 487234 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-05-19 14:05 UTC by Corbin
Modified: 2024-05-20 18:29 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
breeze 6 left vs breeze 5 right (from the fedora rpms, identical to arch packages) (766.61 KB, image/png)
2024-05-19 14:11 UTC, Corbin
Details
breeze 6 left vs breeze 5 right (dark) (from the fedora rpms, identical to arch packages) (768.03 KB, image/png)
2024-05-19 14:18 UTC, Corbin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Corbin 2024-05-19 14:05:12 UTC
I'm developing an application with PySide6  (Qt for Python) and use xdg icon theme names to let my app be themeable.
This means it pulls icons from the system's icon theme, in my case, Breeze Dark.
Unfortunately, Qt doesn't natively recolor icons, so with Breeze6 I noticed that some of my icons were now from the breeze
instead of the breeze dark set.
Looking at where the icons are stored: /usr/share/icons/breeze-dark I saw that many of the icons, especially in the 
status/16 directory, weren't actually recolored, but instead identical to their breeze (light) counterpart.
Looking at the Gitlab, it's clear that there is an automated process that generates the dark icons from the light icons,
but apparently it isn't working so well, as the number of un-recolored icons has drastically increased since breeze5.

Checking the pacman cache, to look at the contents of the tar.gz that was installed, I saw much the same. 
Then checking another distro, I looked at the contents of the breeze-icon-theme-6.2.0 Fedora rpm 
(https://src.fedoraproject.org/rpms/breeze-icon-theme), which was identical. Attached is a split for the breeze 5
and breeze 6 dark icons.

Using the Icon Explorer, the icons do load correctly, showing no signs of wrong coloring, so it's likely the runtime
icon loader is fixing all these issues, but the static recoloring script isn't doing as good of a job.

OBSERVED RESULT
Many icons in breeze dark aren't fit for a dark background.

EXPECTED RESULT
All icons are properly recolored.
Comment 1 Corbin 2024-05-19 14:07:15 UTC
*** Bug 487234 has been marked as a duplicate of this bug. ***
Comment 2 Corbin 2024-05-19 14:11:27 UTC
Created attachment 169616 [details]
breeze 6 left vs breeze 5 right (from the fedora rpms, identical to arch packages)

Not sure why this didn't attach to the main comment.
Comment 3 Corbin 2024-05-19 14:16:03 UTC
Comment on attachment 169616 [details]
breeze 6 left vs breeze 5 right (from the fedora rpms, identical to arch packages)

Wait, wrong picture.
Comment 4 Corbin 2024-05-19 14:18:05 UTC
Created attachment 169617 [details]
breeze 6 left vs breeze 5 right (dark) (from the fedora rpms, identical to arch packages)

All right, sorry for the abject incompetence on display on my part, I hope I got this report properly assembled now.
Comment 5 Corbin 2024-05-19 21:01:14 UTC
Looking into this more, I found the regular expression that find the style color: "\\.ColorScheme-(\\S+){color:(#[0-9a-fA-F]+);}" (https://invent.kde.org/frameworks/breeze-icons/-/blob/master/src/tools/generate-symbolic-dark.cpp?ref_type=heads)

Which works, until some of the svg files are formatted like this: (actions/16/blur.svg)
    <style>.ColorScheme-Text { 
    color:#232629;
}</style>

There you'd need "\\.ColorScheme-(\\S+)\\s*{\\s*color:(#[0-9a-fA-F]+);\\s*}"

But it also fails for files like actions/16/bookmark-remove:
    <style type="text/css" id="current-color-scheme">.ColorScheme-Text{color:#232629;}.ColorScheme-NegativeText { color: #da4453; } </style>

Matter of fact, the pattern also doesn't match the latter negative text color scheme, due to the additional space between color: and the #,
needing even more \s* patterns to properly detect all of these edge cases. Though for this one, it doesn't matter, as only Text and Background
are actually handled in the code anyway. Though it'd probably be simpler to eliminate all whitespace before trying to use regex to parse this.

Interestingly, it did work for a different file, where that looked like this:
    <style type="text/css" id="current-color-scheme">.ColorScheme-Text { color: #fcfcfc; } </style>
with spaces in various positions.

And another one where it failed was well formatted, with no whitespace:
    <style type="text/css" id="current-color-scheme">.ColorScheme-Text{color:#232629;}.ColorScheme-NeutralText{color:#f67400;}.ColorScheme-NegativeText { color: #da4453; } </style>


The only consistent throughline here is that all the ones that failed, had more than one ColorScheme set. Which shouldn't be the issue,
since a regex iterator is used, but that's just what must be happening.
Comment 6 Corbin 2024-05-19 22:44:15 UTC
I shall attempt to fix the issue, but success isn't guaranteed...
Comment 7 Corbin 2024-05-20 01:29:29 UTC
I've found the issue. It's the greedy match on \S+ which WILL match across the other tags, creating one giant abomination
of a match when there are multiple tags present. Classic regex. Merge Request incoming, hopefully I remember how to do that correctly.
Comment 8 Bug Janitor Service 2024-05-20 02:28:35 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/breeze-icons/-/merge_requests/372
Comment 9 Corbin 2024-05-20 18:29:26 UTC
Ah, turns out I used the wrong email for opening this bug report, so it didn't auto close.
Anyway, my merge request was accepted, this problem will be solved with the next release.