<table><tr><td style="">cfeck added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D19392">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D19392#inline-108243">View Inline</a><span style="color: #4b4d51; font-weight: bold;">desktopicon.cpp:522</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #74777d">//don't try for too big images</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">img</span><span class="p">.</span><span class="n">width</span><span class="p">()</span> <span style="color: #aa2211">></span> <span style="color: #601200">256</span> <span style="color: #aa2211">||</span> <span class="n">m_theme</span><span style="color: #aa2211">-></span><span class="n">supportsIconColoring</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span style="color: #aa4000">return</span> <span style="color: #304a96">false</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Did you mean <tt style="background: #ebebeb; font-size: 13px;">>= 256</tt>?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D19392#inline-108247">View Inline</a><span style="color: #4b4d51; font-weight: bold;">desktopicon.cpp:545</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">findIt</span> <span style="color: #aa2211">!=</span> <span class="n">m_monochromeHeuristics</span><span class="p">.</span><span class="n">constEnd</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span style="color: #aa4000">return</span> <span class="n">findIt</span><span class="p">.</span><span class="n">value</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D19392#inline-108248">View Inline</a><span style="color: #4b4d51; font-weight: bold;">desktopicon.cpp:551</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">int</span> <span class="n">saturatedPixels</span> <span style="color: #aa2211">=</span> <span style="color: #601200">0</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">for</span><span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">x</span><span style="color: #aa2211">=</span><span style="color: #601200">0</span><span class="p">;</span> <span class="n">x</span><span style="color: #aa2211"><</span><span class="n">img</span><span class="p">.</span><span class="n">width</span><span class="p">();</span> <span class="n">x</span><span style="color: #aa2211">++</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span style="color: #aa4000">for</span><span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">y</span><span style="color: #aa2211">=</span><span style="color: #601200">0</span><span class="p">;</span> <span class="n">y</span><span style="color: #aa2211"><</span><span class="n">img</span><span class="p">.</span><span class="n">height</span><span class="p">();</span> <span class="n">y</span><span style="color: #aa2211">++</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Spaces</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D19392#inline-108244">View Inline</a><span style="color: #4b4d51; font-weight: bold;">desktopicon.cpp:570</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span class="n">reverseDist</span><span class="p">.</span><span class="n">insertMulti</span><span class="p">(</span><span class="n">it</span><span class="p">.</span><span class="n">value</span><span class="p">(),</span> <span class="n">it</span><span class="p">.</span><span class="n">key</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span class="n">qreal</span> <span class="n">probability</span> <span style="color: #aa2211">=</span> <span class="p">(</span><span class="n">qreal</span><span class="p">)</span><span class="n">it</span><span class="p">.</span><span class="n">value</span><span class="p">()</span><span style="color: #aa2211">/</span><span class="p">(</span><span class="n">qreal</span><span class="p">)(</span><span class="n">img</span><span class="p">.</span><span class="n">size</span><span class="p">().</span><span class="n">width</span><span class="p">()</span><span style="color: #aa2211">*</span><span class="n">img</span><span class="p">.</span><span class="n">size</span><span class="p">().</span><span class="n">height</span><span class="p">()</span> <span style="color: #aa2211">-</span> <span class="n">transparentPixels</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span class="n">entropy</span> <span style="color: #aa2211">-=</span> <span class="n">probability</span> <span style="color: #aa2211">*</span> <span class="n">log</span><span class="p">(</span><span class="n">probability</span><span class="p">)</span><span style="color: #aa2211">/</span><span class="n">log</span><span class="p">(</span><span style="color: #601200">255</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please add same spaces between binary operators. Also C++ casts are 'type(val)' instead of '(type)val'.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D19392#inline-108249">View Inline</a><span style="color: #4b4d51; font-weight: bold;">desktopicon.h:108</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; ">    <span class="n">QPointer</span><span style="color: #aa2211"><</span><span class="n">QNetworkReply</span><span style="color: #aa2211">></span> <span class="n">m_networkReply</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">QHash</span><span style="color: #aa2211"><</span><span style="color: #aa4000">int</span><span class="p">,</span> <span style="color: #aa4000">bool</span><span style="color: #aa2211">></span> <span class="n">m_monochromeHeuristics</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">    <span class="n">QVariant</span> <span class="n">m_source</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">QHash<int, bool></tt> is just a <tt style="background: #ebebeb; font-size: 13px;">QSet<int></tt>.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R169 Kirigami</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D19392">https://phabricator.kde.org/D19392</a></div></div><br /><div><strong>To: </strong>mart, Kirigami<br /><strong>Cc: </strong>cfeck, davidedmundson, plasma-devel, domson, dkardarakos, apol, mart, hein<br /></div>