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?
> 
> Marco Martin wrote:
>     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
>
> 
> Jacopo De Simoi wrote:
>     Hi Michael,
>       
>       indeed I can well see your point. Using a fraction of the size of the dialog icon might be so-so (in fact the ratio 22/32 is quite unique in our icons, it would basically be good just for the current setting.).The best looking (although horrible code-wise) thing to do would probably be to cook up a map from the size of a device icon to the size of an action icon (without worrying about rounding issues); for instance we would have: 32 -> 22; 48 -> 32; 64 -> 48. Same thing for the emblems, we would need 32 -> 16; 48 -> 22; 64 -> 32.
>     What do you think?  Marco? 
>
> 
> Michael Zanetti wrote:
>     What IMHO would be really needed is to extend KIconloader:
>     
>     a) deprecate the enum KIconLoader::StdSizes asap as it shouln't be used ever again
>     b) create a replacement for it in the form of: "int KIconLoader::standardSize(Small | Medium | Large | whatnot)" that returns a pixelsizes that is calculated using the DPIs
>     
>     Then this could be used whereever we need non-user-configurable standardized icon sizes. What do you guys think?

a) is something that i would probably like, but it's a frameworks5 thing i think.
it can be done in the plasma theme bindings tough

aaanyways, I've now finished a new component called PlasmaCore.IconItem that will among other things render icons always of a "proper" size.
so icons in the device notifier can be replaced with that.

then i would say still use the "dialog" size for icons in the devicenotifier.
like in the patch, the unmount icon can become a ratio of the main icon, like 80%, and will be rendered to the largest possible standard size.


- 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/0d2c0dff/attachment.html>