<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 12th, 2010, 8:19 p.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;">just looking at the code seems sensible so far, did you tested it extensively? did you find any misbehaviour?
(will do it tomorrow anyways)
if it appear to behave well i think it could be tried to make it go in</pre>
</blockquote>
<p>On October 12th, 2010, 9:33 p.m., <b>Ingomar Wesp</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;">> did you tested it extensively? did you find any misbehaviour?
I've played around with a default plasma-desktop and tested the constrained size hint with a local version of the quicklaunch applet that makes use of it. Unfortunately I haven't found the time for a more systematic and thorough testing …
> if it appear to behave well i think it could be tried to make it go in
Ready when you are :)</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;">I've done some more testing and noticed that effectiveSizeHint apparently changes the returned sizeHint so that the dimension which has been constrained matches the given constraint. I find that a bit surprising, since it doesn't allow the item to indicate that the constraint doesn't match it's preferred size in that direction, but well...
I don't mean to nag, but since I'd like to depend on the support of constrained sizeHints in the IconWidget: Have you noticed any regressions yet? Otherwise, can the patch go in?</pre>
<br />
<p>- Ingomar</p>
<br />
<p>On October 11th, 2010, 1:06 p.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 13:06:24</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've also changed the signatures of elidedText(...) and layoutText(...) by
removing the need to pass a QStyleOptionGraphicsItem*, since it wasn't used
there anyway. If there's some reason why this could be a bad idea, I'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'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'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'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'm setting any unconstrained dimension to QWIDGETSIZE_MAX for
the purpose of determining the icon size that fits within.
- I'm not very fond of having to initialize and pass around
QStyleOptionGraphicsItem objects, especially since they are apparently only
used for determining whether we'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>