Hi,
Just an observation as a user this time. While using the Breeze widget style, focused areas are almost negated. This is nice and convenient. But also means that dark icons, normally contrasting with bright color theme (or white icons in dark theme, respectively) are barely visible. One can imagine a color theme when icons are completely invisible. That said, in properly composed color themes text is properly negated.

For example when Breeze light color theme is used, what we get now is:


Do you think it's good idea to have icons colorized based on the color of selected area's text, strictly speaking: from the palette's color, e.g. BrightText? Automatic solution could be to perform icon colorization. Picking dark/light theme (whatever fits best) would work so well. Imagine someone using lemon color for bright text. Icons sitting aside of the texts would still be just white, not lemon.

Or have I missed something (plans or ideas) in this regard?

Furthermore, given the logic (automatism) of colorization is exposed to the user, does it make sense in the future to even expose white and dark Breeze icon themes separately? Or one icon theme could be sufficient, and would handle any configuration of colors automatically.

*cough*
viewtopic.php?f=285&t=123132#p321010

luebking wrote:*cough*
viewtopic.php?f=285&t=123132#p321010

Ah Do you think there are any conclusions and if so, how they compare to what I proposed?

We have a mono icons here so any outline around icons makes the icons no longer mono.

BTW, Colorizing icons is way cheaper (not more expensive) compared to rendering, so technically speaking maybe we could colorize (replace the colour) in SVG dynamically and then re-render to the cache. We're rendering SVGs to cache anyway, IIRC.

I think the conclusion was "bad luck", but am not sure.

Technically:
-----------------
There're three obstacles:
- Can an icon be dumb-tinted at all?
The icon file needs a flag "monochrome"

- Different flavors.
QIcon knows several modes and states, eg. for disabled icons. Those are post-processed (grayed out, translucent, tinted blue, overlay, ...)
This implies that the tinting - if possible - has to occur *before* such postprocession (otherwise your just grayed out icon turns blue or sth.)

- Performance
What you do w/ the icon is not so much important (performancewise), as long as the result is cached.

The GUI style (in this case) knows the actually used palette *exclusively* (blue on hover, green on focus or whatever)
It usually demands QStyleOptionButton::icon.pixmap(btn->iconSize, mode, state);
That icon can come from multiple sources (eg. a .qrsc)
=> The style must be able to pass a QColor parameter to QIcon::pixmap(), which is the handed to KIconEngine and from there to KIconLoader which must detect whether the icon is monochrome and tint it accordingly (and include the color in the cache key) before postprocessing it and returning the result.

Problem (and I mean "PROBLEM!") of this approach: QIconEngine::pixmap() is (of course) virtual - and so would any overload/variant have to be. That's not ABI safe -> Qt6

As alternative, the style would operate on QIcon states:
static QIcon::setThemeColor(const QColor&);
QStyleOptionButton::icon.pixmap(btn->iconSize, mode, state);
static QIcon::unsetThemeColor();

The toolchain would be as above, just that no parameter is set, but a global variable set by the client code (style) and interpreted by the iconloader.
No need to mention that this is ugly and not thread safe
At best, this could be a member instead of a static

Better ideas?

This thread goes a bit technical, sorry everyone else

The dream is to stay away of API changes *and* support both Plasma 4 and 5.

Option 1. No alteration of APIs: alter the the fill colour in the SVG, e.g. fill attr here:

Code: Select all

 <path
       style="fill:#4d4d4d;fill-opacity:1;stroke:none"
       d="M 3.7070312 7 L 3 7.7070312 L 9.125 13.832031 L 11 15.707031 L 12.875 13.832031 L 19 7.7070312 L 18.292969 7 L 12.167969 13.125 L 11 14.292969 L 9.8320312 13.125 L 3.7070312 7 z "
       transform="translate(-5e-7,1030.3622)"
       id="rect4176" />

Cache the modified SVGs. Next, proceed as usual to generate PNGs but using the modified SVGs.

We know there are sometimes shapes filled in green or red colour. We know what exactly the color is, so we can keep it unchanged.


Option 2. (a sort of optimization) alter the the fill colour in the SVG during the loading of SVGs.

If you ask me, realistically, both approaches are rather cheap one time effort. ~700 svgz's currently, ~3MiB of processing. Can be smaller if we remove guidelines from the svgs before shipping.

"Normal" Users rarely change their highlight colour every hour to a custom one. In a typical case they would use a downloaded theme.

Technically, when QSvgRenderer is used in KPixmap cache, use QSvgRenderer( const QByteArray &data, ..) or QSvgRenderer(QXmlStreamReader *, ..) instead of QSvgRenderer( const QString & filename,).

The style doesn't load an icon, the icon is already set on the button, the style the paints it "somewhere".
So the problem to the API is less "how do we color this icon" (what's actually more a problem with the artists, we need to specify a key for the replacement color value and that probably has to be in every svg...) but "How do we obtain a tinted variant of the icon in the first place", ie. how does the IconEngine know what pixmap to return when asked for it.

The button on the other hand has no actual idea of what it looks like on screen, esp. cannot predict that for state changes, so it cannot reload the required icon variant either (let alone the overhead of that)

Since the QIcon (what the style sees) -> KIconLoader (what would provide the tinted pixmap) link is private (ie. the style cannot grab through to the loader to eg. cast it or alter a dynamic property etc.) I see little chance but an (ugly) API extension (to QIcon) for state control.
API adjustment would be nicer, but cannot even happen for Qt5.

=> ideas?

luebking wrote:The style doesn't load an icon, the icon is already set on the button, the style the paints it "somewhere".
So the problem to the API is less "how do we color this icon" (what's actually more a problem with the artists, we need to specify a key for the replacement color value and that probably has to be in every svg...) but "How do we obtain a tinted variant of the icon in the first place", ie. how does the IconEngine know what pixmap to return when asked for it.

The button on the other hand has no actual idea of what it looks like on screen, esp. cannot predict that for state changes, so it cannot reload the required icon variant either (let alone the overhead of that)

Since the QIcon (what the style sees) -> KIconLoader (what would provide the tinted pixmap) link is private (ie. the style cannot grab through to the loader to eg. cast it or alter a dynamic property etc.) I see little chance but an (ugly) API extension (to QIcon) for state control.
API adjustment would be nicer, but cannot even happen for Qt5.

=> ideas?

I think the recipe I provided is employed before the KIconLoader loads the icon. It's a pre-generation of icons based on current color theme. it's more like integration with the systemsettings color theme page than applications/QStyles. No change to Q/KIcon needed (kdelibs4/5). Do I miss anything?

it's more like integration with the systemsettings color theme page than applications/QStyles. No change to Q/KIcon needed (kdelibs4/5). Do I miss anything?

I thought you claimed a dark icon when the button is not focused and a bright one when it is?
An inverted icon theme already is available.

luebking wrote:

it's more like integration with the systemsettings color theme page than applications/QStyles. No change to Q/KIcon needed (kdelibs4/5). Do I miss anything?

I thought you claimed a dark icon when the button is not focused and a bright one when it is?
An inverted icon theme already is available.

I am sorry, I mean keeping one icon theme, assuming the only difference between them is the "fill colour" (red/green-filled parts stay the same). Given that I have not overlooked anything fundamental, the theme can have any "lightness" user-visible attribute removed, and single Breeze theme can act as a general purpose them where the shape is all the attribute that matters.

In this case the dark theme can be removed and user would be able to control the color not by changing the light/dark variant of theme but (at the higher level) by changing the Text/Highlighted Text/etc. color, which is also more fine-grained control. And probably it's also a bit easier to provide the icons for distribution.

SVGs seem to give quite a bit of flexibility/dynamics; the above alteration looks like a simple one.

You mean

it's good idea to have icons colorized based on the color of selected area's text, strictly speaking: from the palette's color, e.g. BrightText?

In this case rather HighlightedText, however:
You mean all (action) icons should in this case be monochromatically white?
Ie. more or less invisible in almost all cases but on focused buttons (because the "normal" background is very bright)??

I see what you mean. I am sorry for not seeing this before, I was busy with looking at the easy part with SVGs

I rather agree that we want universal support for Qt-onyl apps too, so the change is most welcome in Qt itself.

How about this: A binary-compatible change by adding more enumerators to QIcon::Mode.

QIcon::Highlighted = 4,
QIcon::DisabledHighlighted = 5,
QIcon::ActiveHighlighted = 6,
QIcon::SelectedHighlighted = 7

QIconEngineV2 (or equivalent) could set up icons for these extra enums and styles that know how to use them, would do so.

When icons are created from pixmaps I agree we're in trouble.

This topic is a candidate for discussion on the qt devel list, right?

jstaniek wrote:How about this: A binary-compatible change by adding more enumerators to QIcon::Mode.

QIcon::Highlighted = 4,
QIcon::DisabledHighlighted = 5,
QIcon::ActiveHighlighted = 6,
QIcon::SelectedHighlighted = 7

Unfortunately, this won't cut it, because every QWidget can have an individual palette (notably used on buttons to make them red or similar ...)
Passing a role *would* be sufficient, but we must get the related QPalette down with it

However, you've a trigger with sth. else:
One could of course overload "QIcon::pixmap(* size, QColor tint)" and that function would check whether dynamic_cast<QIconEngineV2>(d->engine) and in case invoke the tinting QIconEngineV2::pixmap(.) function (otherwise resort to legacy QIconEngine::pixmap(.))

The iconengine/iconloader would then be responsible to "somehow" get a tinted iconpixmap. (What's oc relatively simple for an svg icon, given the icon author hinted the replacement color)


I frankly don't know whether one would bring this to the Qt mailing list, file a bug or propose a patch directly on gerrit.

luebking wrote:I frankly don't know whether one would bring this to the Qt mailing list, file a bug or propose a patch directly on gerrit.

Thanks. Do you think we can call it a serious obstacle while implementing usable (as in UX) default Plasma 5 theme using Qt?

Once I started to think about it, I now see lots of the barely visible icons now, on buttons, lists...

jstaniek wrote:

luebking wrote:I frankly don't know whether one would bring this to the Qt mailing list, file a bug or propose a patch directly on gerrit.

Thanks. Do you think we can call it a serious obstacle while implementing usable (as in UX) default Plasma 5 theme using Qt?

Once I started to think about it, I now see lots of the barely visible icons now, on buttons, lists...

Speaking of default theme: an easy workaround is to never set the selection color too dark or light, which is the current setting

As suggested it the previous thread, I consider the status quo a problem, yes.

What would be required now, would be an "official" statement of the icon designers on th topic:

@Uri (and everyone else concerned
What's your position in this regard? Do you see (some) icon as "glyphs" which you'd like to meet the related foreground (text) color?

Would you be interested in even more palette invocation, eg. add elements in the (current) highlight or even background (for opaque inverted elements) color?