cfeck added inline comments. INLINE COMMENTS
> desktopicon.cpp:522 > + //don't try for too big images > + if (img.width() > 256 || m_theme->supportsIconColoring()) { > + return false; Did you mean `>= 256`? > desktopicon.cpp:545 > + if (findIt != m_monochromeHeuristics.constEnd()) { > + return findIt.value(); > + } You are caching the result per size, but the initial decision depends on the actual icon image, right? Is it possible that the first icon examined is colorful, but the rest is not, or vice versa? If yes, would it make sense to examine a few icons (maybe three) before a decision is made? > desktopicon.cpp:551 > + int saturatedPixels = 0; > + for(int x=0; x<img.width(); x++) { > + for(int y=0; y<img.height(); y++) { Spaces > desktopicon.cpp:570 > + reverseDist.insertMulti(it.value(), it.key()); > + qreal probability = > (qreal)it.value()/(qreal)(img.size().width()*img.size().height() - > transparentPixels); > + entropy -= probability * log(probability)/log(255); Please add same spaces between binary operators. Also C++ casts are 'type(val)' instead of '(type)val'. > desktopicon.h:108 > QPointer<QNetworkReply> m_networkReply; > + QHash<int, bool> m_monochromeHeuristics; > QVariant m_source; `QHash<int, bool>` is just a `QSet<int>`. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D19392 To: mart, #kirigami Cc: cfeck, davidedmundson, plasma-devel, domson, dkardarakos, apol, mart, hein