<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5805/">http://svn.reviewboard.kde.org/r/5805/</a>
</td>
</tr>
</table>
<br />
<p>On November 10th, 2010, 12:54 a.m., <b>Lindsay Roberts</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;">If there are no objections could I request that someone check this in?</pre>
</blockquote>
<p>On November 10th, 2010, 11:02 a.m., <b>Markus Slopianka</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;">Does it change the behavior described in https://bugs.kde.org/show_bug.cgi?id=256465 (always using the large 256x256 icon for the scaling effect)?
If no, I suggest to tweak the patch because ever since Nuno changed the hi-res icons to look completely different from the smaller ones, the scaling effect looks really weird.</pre>
</blockquote>
<p>On November 11th, 2010, 11:40 p.m., <b>Lindsay Roberts</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;">I've looked a bit into this issue: This is at a much lower level than the FolderView itself, it is traceable to KIcon/KIconLoader/KTheme and the strategy used to generate icon's for a given size. This is not a part of, or unique to, Plasma. To visualise this: open Dolphin and navigate to a folder full of movies that have a generic icon, turn previews off, and scale through the available zoom levels. You will see the icon alternating between the smaller icon (on sizes exactly matching the /share/icons/style/SIZExSIZE) and the 256x256 icon (on all other sizes).
Having said that, its questionable whether FolderView should be requesting an Icon in this way; what we really want in this instance is (existing_icon * 0.9) not (most_suitable_icon_for(size * 0.9)). But. The behaviour of the icon sizer seems quite clearly incorrect. If we mean to use the largest icon for scaling in all scaled cases we may as well not ship anything but 256x icons, we are essentially not allowing the content disparity the system pertains to provide. Admittedly it is utilised far less than it perhaps could be, but that is no reason not to support it.
We could fix the issue by scaling the pixmap from the icon in FolderView itself. There are some issues with regards to whether we cache those scaled pixmaps. Another question: is it wise to cache pixmaps for actions that only happen once on activate and then possibly not for a stretch of time? Perhaps it would be better to leave the cache for icon content that will be regularly repainted. In that case we can just scale on paint() when pressed and delete the pixmap on unpressed or similar.
If we don't want to fix it in that way, then there are issues to be dealt with at the KDELibs level, and that would be firmly out of the scope of this patch/review. Even the paint fix might be out of scope of this particular patch, I'll defer to the list on that.</pre>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">i completely agree with this: "what we really want in this instance is (existing_icon * 0.9) not (most_suitable_icon_for(size * 0.9))" and that's easy enough to do just by changing the icon requested in the code and then doing the scaling in folderview. in any case, i'll commit this patch as it fixes something regardless of that other behaviour.</pre>
<br />
<p>- Aaron</p>
<br />
<p>On November 9th, 2010, 11:50 a.m., Lindsay Roberts wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Lindsay Roberts.</div>
<p style="color: grey;"><i>Updated 2010-11-09 11:50:55</i></p>
<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;">The folder view employs a 'pressed' effect to show when an icon is activated.
This works as expected when global settings are set to single click: the icon
is scaled to 90% on press, and rescales back up on activate.
Under double click the icon scales on the first click, and weirdly stays scaled
until the mouse leaves the hover area.
Further, the same issue applies when modifier keys are used to change
selection.</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;">Tested with both single and double click settings.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=256297">256297</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp <span style="color: grey">(1193797)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5805/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>