<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://svn.reviewboard.kde.org/r/5580/">http://svn.reviewboard.kde.org/r/5580/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 11th, 2010, 8:34 a.m., <b>Marco Martin</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;">&gt; The reported preferredSize is not bounded by the maximumSize. 

you don&#39;t have to call maximumSize(), it&#39;s the hardcoded overriding size hint that you can set from the outside ith setMaximumSize.

you just have to obtain it in hte same way you use in sizeHint:
sizeFromIconSize(qMax(d-&gt;maximumIconSize.height(), d-&gt;maximumIconSize.width()));

and yes, size hints are cached, that&#39;s (besides the existence of setMaximum/MinimumSize) why sizeHint() is protected and effectiveSizeHint exists</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;">&gt;&gt; The reported preferredSize is not bounded by the maximumSize.
&gt; 
&gt; you don&#39;t have to call maximumSize(), it&#39;s the hardcoded overriding size
&gt; hint that you can set from the outside ith setMaximumSize.

Err... but maximumSize() delegates to sizeHint() if no maximum size has been explicitly set, no?

&gt; you just have to obtain it in hte same way you use in sizeHint:
&gt; sizeFromIconSize(qMax(d-&gt;maximumIconSize.height(),
&gt; d-&gt;maximumIconSize.width()));

This has crossed my mind as well. It works if there is a valid maximumIconSize set. Otherwise, falling back to QGraphicsLayoutItem::sizeHint(Qt::MaximumSize) would probably be the way to go. Unfortunately, if the logic for the maximumSize in sizeHint ever changes, we&#39;ll need to duplicate this here as well.

I will change the patch accordingly.

&gt; and yes, size hints are cached, that&#39;s (besides the existence of
&gt; setMaximum/MinimumSize) why sizeHint() is protected and effectiveSizeHint
&gt; exists

I know, I just found it surprising that effectiveSizeHint tries to fetch all size hints and that it apparently even uses the previously given constraint for that...</pre>
<br />








<p>- Ingomar</p>


<br />
<p>On October 11th, 2010, 1:40 a.m., Ingomar Wesp wrote:</p>






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

<div>Review request for Plasma.</div>
<div>By Ingomar Wesp.</div>


<p style="color: grey;"><i>Updated 2010-10-11 01:40:48</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;">I poked around the IconWidget class and came up with the following patch that
implements handling of constraints in sizeHint(...). It basically pulls the 
code that determines the icon size for a given widget size out of 
layoutIcons(...) into an the a new iconSizeForWidgetSize(...) method so that 
it can be reused by sizeHint(...).

I&#39;ve also changed the signatures of elidedText(...) and layoutText(...) by 
removing the need to pass a QStyleOptionGraphicsItem*, since it wasn&#39;t used 
there anyway. If there&#39;s some reason why this could be a bad idea, I&#39;ll put it 
back in.

The patch changes the behavior of sizeHint(...) in the following way:

- If there is no constraint, the behavior remains unchanged.

- Likweise, the minimum and maximum sizes are unaffected by constraints. I 
  don&#39;t see how limiting the available width/height would affect either one?
  
- The preferred size computation now uses the constraint to compute the 
  expected size of the icon and adjusts the returned value accordingly.

I must admit that I&#39;m not entirely happy with the solution as it has (at
least ;) the following issues:

- The reported preferredSize is not bounded by the maximumSize. Unfortunately 
  I can&#39;t call maximumSize() and its derivatives from within sizeHint, since
  this might cause QGraphicsLayoutItem to update all sizeHints, which might in
  turn lead to unbound recursion, since it apparently also caches the 
  previously passed constraints :/
  
  This is why I&#39;m setting any unconstrained dimension to QWIDGETSIZE_MAX for 
  the purpose of determining the icon size that fits within. 

- I&#39;m not very fond of having to initialize and pass around
  QStyleOptionGraphicsItem objects, especially since they are apparently only 
  used for determining whether we&#39;ve got an RTL layout. 

Any testing / feedback would be greatly appreciated.</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>/trunk/KDE/kdelibs/plasma/widgets/iconwidget.cpp <span style="color: grey">(1182894)</span></li>

 <li>/trunk/KDE/kdelibs/plasma/widgets/iconwidget_p.h <span style="color: grey">(1182894)</span></li>

</ul>

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




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








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