Review Request 128552: Distinguish symlinks from other files in folder view plasmoid

[Eike Hein]

> On July 31, 2016, 12:18 p.m., Eike Hein wrote:
> > containments/desktop/plugins/folder/foldermodel.cpp, line 952
> > <https://git.reviewboard.kde.org/r/128552/diff/2/?file=472577#file472577line952>
> >
> >     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.
> >     
> >     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.
> >     
> >     I also recommend looking at Dolphin code for reference/inspiration, to start with.
> 
> Chinmoy Ranjan Pradhan wrote:
>     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. 
>     
>     >I feel like there has to be a precedent for handling this in a smarter way somewhere
>     
>     And why it is not good to have this icon handling in Qt::DecorationRole?
>     
>     >I also recommend looking at Dolphin code for reference/inspiration, to start with.
>     
>     Actually thats where i got the KIconLoader part.
>     
>     And what about the italisicing of file name.

> And why it is not good to have this icon handling in Qt::DecorationRole?

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 *name*, 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.

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.

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.

One way to fix this is to properly add overlay support to IconItem, if it doesn't already have it.

> And what about the italisicing of file name.

That code is wrong, too; I forgot to comment on it so far. Will do in a sec.


- Eike


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128552/#review97950
-----------------------------------------------------------


On July 29, 2016, 3:40 p.m., Chinmoy Ranjan Pradhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128552/
> -----------------------------------------------------------
> 
> (Updated July 29, 2016, 3:40 p.m.)
> 
> 
> Review request for Plasma, Kai Uwe Broulik, Bhushan Shah, and Eike Hein.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   containments/desktop/package/contents/ui/FolderItemDelegate.qml e4fcd67 
>   containments/desktop/plugins/folder/CMakeLists.txt 1095f81 
>   containments/desktop/plugins/folder/foldermodel.h a6992bb 
>   containments/desktop/plugins/folder/foldermodel.cpp 2b9d41b 
> 
> Diff: https://git.reviewboard.kde.org/r/128552/diff/
> 
> 
> Testing
> -------
> 
> build
> 
> 
> File Attachments
> ----------------
> 
> symlinks and other files/folders look similar
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/07/29/7428b23d-03f9-4aed-8ca0-536d44e45e8c__beforepatch.png
> after patch , everything looks fine
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/07/29/040b2e3b-9ea0-4347-9e6c-ca3bdb73b36a__link.png
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160731/b7d535ae/attachment-0001.html>