Review Request: make the DeviceNotifier Plasmoid scale with high-res screens

[Marco Martin]

> On Nov. 26, 2012, 9:35 a.m., Marco Martin wrote:
> > This is really an issue that needs addressing, thanks for taking time for it.
> > 
> > i'm very in favor of removing all those hardcoded sizes from the code.
> > 
> > however i see two problems with that approach
> > 
> > 1) icons are cleanly painted only when they have a size that is the "right" one (16,22,32,48,64,128,512 exported in qml with theme.smallIconSize, smallMediumIconSize etc)  for big sizes doesn't matter if the pixmap ends up being scaled, but small icons really look horrible when scaled (and in the new code there is no check the size ends up being one of those).
> > 
> > 2) it's anyways quite arbitrary, so i can easily see each plasmoid using more or less its own logic, with the end result looking a bit like a patchwork.
> > 
> > the"proper" solution i think is to export in Theme the configurable KIconLoader sizes, (Desktop, Toolbar, SmallIcons etc)   and those can be controlled by a kcm, so will be properly set on higher resolution displays
> 
> Marco Martin wrote:
>     just added the bindings:
>     use
>     theme.iconSizes.dialog for most icons
>     theme.iconSizes.toolbar for unmount
>     theme.iconSizes.small for all that is normally 16x16
>     
>     
>     so the icon settings in systemsettings will affect those icons
> 
> Michael Zanetti wrote:
>     Hey Marco,
>     
>     I'm not sure if this is really a good idea. Using KIconLoader::IconSize() is a good idea for toolbars and such. However, in a plasma layout I don't think its a good idea to let the user change the size of the icons. It can and will mess up the layout. For example I use the same size for "Toolbar" and "Dialog" because I like big toolbars. I certainly don't want to the unmount icon to be that huge in the devicemanager plasmoid because if that...
>     
>     regarding the scaling: I was thinking that Plasma uses SVG icons... Reading throught the QIconItem code I see now that it doesn't. In that case its obviously not good to freely scale it around. Maybe it should be replaced by PlasmaCore.SvgItem?

what is important is that the sizes of the icons are something that is
a) consistent everywhere (not having each applet decide with its criteria)
b) be something that can depend from dpi, those being settable are fine, since will be the same as things around application

now, indeed can be not a good idea to have the unmount icon as toolbar, since the size can be messed up (so keeping a size relative to the size used to the main icon, that i think dialog is ok)

This i think makes the case for exposing a currently internal component that i'm using in buttons and toolbuttons called IconLoader
the behavior is:
* use an svg if it does exist in the theme: requiring to have a complete svg theme (and with enough quality) is neither feasible nor desiderable
* fallback to a qicon if the svg is not found
* always paint in the nearest (smaller) kiconloader standard size (even for svg, for consistency in layouts and because they aren't as scalable as they seem, if their shape edges are not grid aligned they look horrible)
* will probably also have to directly support the on mouse over highlight animation or people will keep using the horrible PlasmaWidgets.IconWidget


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107428/#review22548
-----------------------------------------------------------


On Nov. 23, 2012, 10:11 p.m., Michael Zanetti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107428/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2012, 10:11 p.m.)
> 
> 
> Review request for Plasma and Kai Uwe Broulik.
> 
> 
> Description
> -------
> 
> This adjustst the DeviceNotifier Plasmoid to scale nicely with high resolution screens.
> 
> The patch changes also one label from wordWrapping to clipping behavior because otherwise it messes up the layout.
> 
> 
> Diffs
> -----
> 
>   plasma/generic/applets/devicenotifier/package/contents/ui/ActionItem.qml 3087a07 
>   plasma/generic/applets/devicenotifier/package/contents/ui/DeviceItem.qml 1fab0ef 
>   plasma/generic/applets/devicenotifier/package/contents/ui/devicenotifier.qml f8728d0 
> 
> Diff: http://git.reviewboard.kde.org/r/107428/diff/
> 
> 
> Testing
> -------
> 
> Tested on High res screen at full DPI and at regular DPI values
> 
> 
> Thanks,
> 
> Michael Zanetti
> 
>

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