Review Request: Plasma: Folder View: Icon displays 'pressed' on non-activating clicks

[Aaron Seigo]

> On 2010-11-09 18:37:25, Aaron Seigo wrote:
> >
> 
> Lindsay Roberts wrote:
>     If there are no objections could I request that someone check this in?
> 
> Markus Slopianka wrote:
>     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.
> 
> Lindsay Roberts wrote:
>     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.

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.


- Aaron


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5805/#review8589
-----------------------------------------------------------


On 2010-11-09 11:50:55, Lindsay Roberts wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5805/
> -----------------------------------------------------------
> 
> (Updated 2010-11-09 11:50:55)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> This addresses bug 256297.
>     https://bugs.kde.org/show_bug.cgi?id=256297
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp 1193797 
> 
> Diff: http://svn.reviewboard.kde.org/r/5805/diff
> 
> 
> Testing
> -------
> 
> Tested with both single and double click settings.
> 
> 
> Thanks,
> 
> Lindsay
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101112/59ef9487/attachment.htm