> 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 :)
>
> Kai Uwe Broulik wrote:
> @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 ;)
>
> Thomas Lübking wrote:
> Depends on the implementation.
> 22/72*125 = 38.1944 - what is indeed closer to 32 than 48px - but not 22px
> even if one assumed the 22px were chosen for 96 dpi you'd be at 28.6458px
> (thus closer to 32 than 22)
>
> The "problem" of my original suggestion is that the targetsize was
> unlikely ever QFontMetrics(QWidget::font()).height() - because even on a
> 96dpi display a relatively huge 12pt font would be only 16px high... so the
> idea was "font height + "some padding" - or maybe just "*1.618"
>
> Kai Uwe Broulik wrote:
> Btw it doesn't matter which size is closer, docs say:
> "Returns a pixmap with the requested size […]. The pixmap might be
> smaller than requested, but never larger."
>
> Kai Uwe Broulik wrote:
> (Sorry for the noise)
> > even if one assumed the 22px were chosen for 96 dpi
> which is the case here
> > you'd be at 28.6458px (thus closer to 32 than 22)
> see above which is why I get 22 px icon :)
Why this is what your algorithm should cover :)
Naive approach (and untested ;-)
static const int[10] iconSize = {16,22,32,48,64,72,96,128,256,512};
const float idealSize = QFontMetrics(QWidget::font()).height() * 1.618;
float diff = qAbs(idealSize-iconSize[0]);
for (int i = 1; i < 10; ++i) {
const float diff = qAbs(idealSize-iconSize[i]);
if (diff > lastDiff)
return iconSize[i-1];
lastDiff = diff;
}
- Thomas
-----------------------------------------------------------
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
>
>
