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
