<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/128552/">https://git.reviewboard.kde.org/r/128552/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 31st, 2016, 12:18 p.m. UTC, <b>Eike Hein</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/128552/diff/2/?file=472577#file472577line952" style="color: black; font-weight: bold; text-decoration: underline;">containments/desktop/plugins/folder/foldermodel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">952</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QPixmap</span> <span class="n">pixmap</span> <span class="o">=</span> <span class="n">icon</span><span class="p">.</span><span class="n">pixmap</span><span class="p">(</span><span class="n">icon</span><span class="p">.</span><span class="n">availableSizes</span><span class="p">().</span><span class="n">last</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I still don't like this because we're going from a 'smart' icon with multiple available sizes from a pixmap with one size. Using a pixmap and scaling it will lead to inconsistent visual results in the view.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I feel like there has to be a precedent for handling this in a smarter way somewhere, or if not, we should create one. For example IconItem could have an 'overlay' prop for setting an overlay name similar to the icon name, and doing the painting there.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I also recommend looking at Dolphin code for reference/inspiration, to start with.</p></pre>
</blockquote>
<p>On July 31st, 2016, 12:28 p.m. UTC, <b>Chinmoy Ranjan Pradhan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well...as of now i didnt got any inconsistent result(maybe i've not tested enough). The resize of icons works the way it should. </p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I feel like there has to be a precedent for handling this in a smarter way somewhere</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And why it is not good to have this icon handling in Qt::DecorationRole?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I also recommend looking at Dolphin code for reference/inspiration, to start with.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Actually thats where i got the KIconLoader part.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And what about the italisicing of file name.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And why it is not good to have this icon handling in Qt::DecorationRole?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Icon theming works this way: Icon themes usually ship pre-rendered icon versions optimized for different sizes (16px, 32px, 64px, etc.) in PNG format. When the icon rendering stack is asked to render a 70px icon, for example, it will pick the closest version and resample it for the target size. If it's asked to render a 64px icon it will just use the already-existing version. For that reason QIcon supports storing multiple pixmaps of different sizes. The best way to make sure that smart rendering happens in a Plasma UI is to have a model return an icon <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">name</em>, so that the icon theme lookup can happen based on it, and the icon of the right size is used. If a theme ships scalale SVG icons, smart rendering means rasterizing the SVG to the target size.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Your code skips both ways of smart rendering: For a theme with pre-rendered PNGs, it will disregard the smaller versions because you're rasterizing to a particular pixmap size before handing the pixmap to the view. For a theme with SVGs, again you're rasterizing to a specific size and then the view ends up resampling.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This will cause inconsistent visual results in some cases because only the icons with the overlay undergo this extra indirection step through a particular rasterized size, but the others don't.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">One way to fix this is to properly add overlay support to IconItem, if it doesn't already have it.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And what about the italisicing of file name.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That code is wrong, too; I forgot to comment on it so far. Will do in a sec.</p></pre>
<br />
<p>- Eike</p>
<br />
<p>On July 29th, 2016, 3:40 p.m. UTC, Chinmoy Ranjan Pradhan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma, Kai Uwe Broulik, Bhushan Shah, and Eike Hein.</div>
<div>By Chinmoy Ranjan Pradhan.</div>
<p style="color: grey;"><i>Updated July 29, 2016, 3:40 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In folder view plasmoid symlinks and ordinary files look similar. This patch makes the symlink look different by italicising its name and adding an icon overlay.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">build</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>containments/desktop/package/contents/ui/FolderItemDelegate.qml <span style="color: grey">(e4fcd67)</span></li>
<li>containments/desktop/plugins/folder/CMakeLists.txt <span style="color: grey">(1095f81)</span></li>
<li>containments/desktop/plugins/folder/foldermodel.h <span style="color: grey">(a6992bb)</span></li>
<li>containments/desktop/plugins/folder/foldermodel.cpp <span style="color: grey">(2b9d41b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128552/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/07/29/7428b23d-03f9-4aed-8ca0-536d44e45e8c__beforepatch.png">symlinks and other files/folders look similar</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/07/29/040b2e3b-9ea0-4347-9e6c-ca3bdb73b36a__link.png">after patch , everything looks fine</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>