D19392: shannon entropy to guess monochrome icon

[Christoph Feck]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190227/e12758d3/attachment-0001.html>