<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/108434/">http://git.reviewboard.kde.org/r/108434/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 16th, 2013, 10:01 a.m. UTC, <b>Thomas Lübking</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)</pre>
 </blockquote>




 <p>On January 16th, 2013, 1:31 p.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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, ..</pre>
 </blockquote>





 <p>On January 16th, 2013, 1:59 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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)</pre>
 </blockquote>





 <p>On January 16th, 2013, 2:04 p.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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..</pre>
 </blockquote>





 <p>On January 16th, 2013, 4:15 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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.</pre>
 </blockquote>





 <p>On February 9th, 2013, 10:14 a.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tried it with QFontMetrics but doesn't look nice and is less predictable than MainToolBar icon size.</pre>
 </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)</pre>
<br />










<p>- Thomas</p>


<br />
<p>On January 16th, 2013, 1:34 a.m. UTC, Kai Uwe Broulik wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for kdelibs.</div>
<div>By Kai Uwe Broulik.</div>


<p style="color: grey;"><i>Updated Jan. 16, 2013, 1:34 a.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yup, see screenshots.

Only minor problem is that it doesn't react to when the icon size changes (like buttons and other elements do).</pre>
  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kdeui/paged/kpageview.cpp <span style="color: grey">(8863934)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/108434/diff/" style="margin-left: 3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>

<ul>

 <li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconding.png">KPageDialog icon adapted</a></li>

</ul>





  </td>
 </tr>
</table>








  </div>
 </body>
</html>