davidedmundson added a comment.
  > so, we colorize monochrome icons treating them as just masks, and that 
works ok, tough there isn't any spec to know with 100% certainty if an icon is 
monochrome or colored (all icon names that end up with -symbolys usually are 
monochrome, but in breeze and other themes monochrome icons are all over the 
place and often it depends from the icon size)
  
  For breeze there's an alternate approach. We "just" add "-symbolic" symlinks 
in everywhere. We could even use this code to automate doing that. That would 
cover the android case.
  
  > Or technically one can do a kirigami app with zero dependencies and not 
have qqc2-desktop-style installed and run it on gnome.
  
  It seems gnome themes also do the -symbolic suffix thing and do SVG 
replacement like we do. With very similar keys too!
  Even ignoring this Kirigami issue we have an issue of Breeze icons on gnome 
and vice versa that's worth fixing.
  
  If we need to do this as a temporary measure, then fine.
  
  However,  I want to hear a plan on what a "correct" cross-desktop KF6 + Qt6 
solution would be - and maybe start some ML threads together before accepting. 
  Otherwise we'll carry on with hacks that only half work forever and ever.

INLINE COMMENTS

> desktopicon.cpp:494
>  
>              const QColor tintColor = !m_color.isValid() || m_color == 
> Qt::transparent ? (m_selected ? m_theme->highlightedTextColor() : 
> m_theme->textColor()) : m_color;
>  

Will this code ever get called if we are using the KIconLoader?  We want to 
avoid that. (Even if it's just looking at QIcon.engine()->metaObject)

> desktopicon.cpp:496
>  
> -            if (m_isMask ||
> -                //this is an heuristic to decide when to tint and when to 
> just draw
> -                //(fullcolor icons) in reality on basic styles the only 
> colored icons should be -symbolic, this heuristic is the most compatible 
> middle ground
> -                icon.isMask() ||
> -                //if symbolic color based on tintColor
> -                (iconSource.endsWith(QLatin1String("-symbolic")) && 
> tintColor.isValid() && tintColor != Qt::transparent) ||
> -                //if path color based on m_color
> -                (isPath && m_color.isValid() && m_color != Qt::transparent)) 
> {
> +            if (m_isMask || icon.isMask() || 
> iconSource.endsWith(QStringLiteral("-symbolic")) || guessMonochrome(img)) {
>                  QPainter p(&img);

Do we need to test:

-symbolic
-symbolic-ltr
-symbolic-rtl

(I saw this in some gnome code)

> platformtheme.cpp:695
>  
>  QIcon PlatformTheme::iconFromTheme(const QString &name, const QColor 
> &customColor)
>  {

Is this useful if DesktopIcon does this anyway?

(or to turn the question around, can we do it all here and have DesktopIcon 
only check isMask)

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D19392

To: mart, #kirigami
Cc: cfeck, davidedmundson, plasma-devel, domson, dkardarakos, apol, mart, hein

Reply via email to