> On Jan. 16, 2013, 10:01 a.m., Thomas Lübking wrote:
> > instead of maintoolbar (which could be 128px on even a netbook) maybe 
> > relate it to the font height? (what seems to be the idea behind 22px)
> 
> Kai Uwe Broulik wrote:
>     Main toolbar icon size is only settable from 16 to 48 pixels and defaults 
> to 22. And if you increase the size of toolbar icons, it will make the look 
> somewhat unified, if this icon grows as well. Don't know how I can properly 
> relate it to the font height, especially since I cannot just use an arbitrary 
> number but one of these: 16, 22, 32, 48, ..
> 
> Thomas Lübking wrote:
>     > Main toolbar icon size is only settable from 16 to 48 pixels and 
> defaults to 22.
>     Arbtrary GUI config limitation:
>     kwriteconfig --file kdeglobals --group MainToolbarIcons --key Size 128
>     
>     > And if you increase the size of toolbar icons, it will make the look 
> somewhat unified
>     As mentioned, i don't know about the idea behind this, just pointing that 
> a fixed size usually near the font height was initially picked.
>     If you raise dpi, thus font pixels ... you get what i mean.
>     
>     > Don't know how I can properly relate it to the font height
>     QFontMetrics(QWidget::font()).height()
>     
>     QPixmap QIcon::pixmap ( int w, int h, Mode mode = Normal, State state = 
> Off ) const
>     This is an overloaded function.
>     Returns a pixmap of size QSize(w, h). The pixmap might be smaller than 
> requested, but never larger.
>     
>     You /can/ use arbitrary sizes but may want to match them to the closest 
> size (simple for the 2^n sizes, but there's still stupid 22px - as it's 
> apparently not a hot path, just move up until qAbs(height-n) gets bigger 
> again)
> 
> Kai Uwe Broulik wrote:
>     > Arbtrary GUI config limitation:
>     But not my problem if a user bypasses this :P
>     
>     > QFontMetrics(QWidget::font()).height()
>     Thanks! Ah, this is how to use this. Already knew that there was this 
> QFontMetrics thing but didn't know how to use. Will make my work on other 
> parts easier as well.
>     
>     > You /can/ use arbitrary sizes but may want to match them to the closest 
> size
>     Yes, I know, but wanted to point out that I didn't want this to avoid 
> blurry icons ;)
>     
>     I don't understand this 22 thing anyway, why not just 24? Who knows..
> 
> Thomas Lübking wrote:
>     > But not my problem if a user bypasses this :P
>     This will fail as soon as the GUI is modified to reflect the requirements 
> of HiRes screens, isn't?
>     
>     > Yes, I know, but wanted to point out that I didn't want this to avoid 
> blurry icons ;)
>     I'm at hand not sure, but believe that QIcon will get you a correct 
> (unscaled) pixmap of the next lower size (so 21,21 will get you 16,16) - 
> yesno?
>     
>     > I don't understand this 22 thing anyway, why not just 24? Who knows..
>     because 24 is 2^4+2^3?
>     22 is 2^4+2^2+2^1 - equally good :-P
>     
>     Historical reasons, i think.
> 
> Kai Uwe Broulik wrote:
>     Tried it with QFontMetrics but doesn't look nice and is less predictable 
> than MainToolBar icon size.
> 
> Thomas Lübking wrote:
>     What do you mean "doesn't look nice"?
>     On a common setup this should result in a 22px icon - the same you use 
> now (for the configured toolbar size)
> 
> Christoph Feck wrote:
>     Regarding 22 vs. 24, the geometric mean of 16 and 32 is 22.63, which got 
> rounded down to 22 to get even sized icons. For the newer (larger) icon 
> sizes, a simple "multiple of 16" rule was used instead, because nobody knew 
> the old rule :)

@Thomas: But on my machine (125dpi) it also results in a 22px icon which looks 
weird and was the initial reason for this patch.

@Christoph: Thanks for the history lesson :-) So let's ditch 48px icons in 
favor of 45px ;)


- Kai Uwe


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


On Jan. 16, 2013, 1:34 a.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108434/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2013, 1:34 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> It took me hours (really, it's like. System Settings adds a KCModuleProxy 
> which calls KCModuleLoader to load a KCModule which is in some different 
> directory which uses KPageDialog (where I looked at first …) which uses 
> KPageWidget which is a private class inside which then uses KPageView bla.. 
> :D)
> 
> Finally got it and made it use the MainToolbar size which also defaults to 
> 22px, so you won't notice any difference, but users of MacBook Pro Retina and 
> similar devices will appreciate. Also makes it unified a bit because it is 
> always the toolbar icon size.
> 
> 
> Diffs
> -----
> 
>   kdeui/paged/kpageview.cpp 8863934 
> 
> Diff: http://git.reviewboard.kde.org/r/108434/diff/
> 
> 
> Testing
> -------
> 
> Yup, see screenshots.
> 
> Only minor problem is that it doesn't react to when the icon size changes 
> (like buttons and other elements do).
> 
> 
> File Attachments
> ----------------
> 
> KPageDialog icon adapted
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconding.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

Reply via email to