ervin added a comment.

  In D28194#632629 <https://phabricator.kde.org/D28194#632629>, @apol wrote:
  
  > Fixing QIcon would make sense but I'd say getting this in is not the worst 
thing either.
  
  
  I don't think it's QIcon's fault to be honest. The pattern used in several 
places of `(icon.name || icon.source)` doesn't help, it makes the lower layer 
loose the information of an icon specified by name (thus going through theme 
loading) or by URL (thus going through direct loading). Due to that the lower 
layer ends up having code trying to guess in which situation it is.

INLINE COMMENTS

> kquickstyleitem.cpp:182
>  
>          const QVariant icon = m_properties[QStringLiteral("icon")];
>          if (icon.canConvert<QIcon>()) {

Note that this construct is duplicated three times in that file. It really 
calls for having a findIcon like that:

  QIcon KQuickStyleItem::findIcon(const QVariant &iconProp) const
  {
      if (iconProp.canConvert<QIcon>()) {
          return iconProp.value<QIcon>();
      } else if (iconProp.canConvert<QString>()) {
          const auto iconId = iconProp.toString();
          if (iconId.startsWith(QLatin1Char('/')) || 
iconId.startsWith(QStringLiteral(":/"))) {
              return QIcon(iconId);
          } else if (iconId.startsWith(QStringLiteral("file:"))) {
              return QIcon(QUrl(iconId).toLocalFile());
          } else if (iconId.startsWith(QStringLiteral("qrc:"))) {
              return QIcon(QLatin1Char(':') + QUrl(iconId).path());
          } else {
              return m_theme->iconFromTheme(iconId, 
m_properties[QStringLiteral("iconColor")].value<QColor>());
          }
      } else {
          return QIcon();
      }
  }

And then using it for setting opt->icon and opt->currentIcon (combobox case). 
Yes, I did that locally and that definitely helped.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: nicolasfella, #plasma, mart, ervin
Cc: apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

Reply via email to