<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://reviewboard.kde.org/r/4623/">http://reviewboard.kde.org/r/4623/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 14th, 2010, 5:39 p.m., <b>Aaron Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="/r/4623/diff/3/?file=31168#file31168line322" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/ui/applet.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;">void Applet::checkSizes()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">321</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"> <span class="n">normalRect</span><span class="p">.</span><span class="n">setHeight</span><span class="p">(</span><span class="n">normalRect</span><span class="p">.</span><span class="n">height</span><span class="p">()</span> <span class="o">-</span> <span class="n">rightEasement</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">322</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">323</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"> <span class="n">lastRect</span><span class="p">.</span><span class="n">setY</span><span class="p">(</span><span class="n">normalRect</span><span class="p">.</span><span class="n">bottom</span><span class="p">()</span> <span class="o">+</span> <span class="mi">1</span> <span class="o">-</span> <span class="n">margin</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">324</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"> <span class="n">lastRect</span><span class="p">.</span><span class="n">setHeight</span><span class="p">(</span><span class="n">rightEasement</span> <span class="o">-</span> <span class="n">margin</span> <span class="o">*</span><span class="mi">2</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
</tr>
</tbody>
</table>
<pre>these kinds of changes are really annoying. yes, rightEasement isn't used right now. but what if we do use it in the future? then we'll have to add this code back in.
is the code that was removed a performance problem? no.
was it unreadable? no.
so why was it removed? just because.
at one point, it ONLY supported rightEasement. when we had need for leftEasement, and switched over to that, both left and right easement calculation was kept so that we could easily change the right/left layout decisions later on down the road. now we're back to having an assumption in the code that we only have some special space on the left.
please put the rightEasement code back in ASAP.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em">I discussed this with notmart already. The code isn't used at the moment and notmart said it will not be used in the future. So removed it as the calucaltions where wrong and why correct a not used code?. For the planed rewrite for 4.6 I can reintroduce this in a clean way. No Problem.
But as we are on this, one question: Should there be a seperator in case right easment is used? I would do it, as the background would change to (if the theme has one...)</pre>
<br />
<p>- Manuel</p>
<br />
<p>On July 13th, 2010, 7:09 p.m., Manuel Mommertz wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 Manuel Mommertz.</div>
<p style="color: grey;"><i>Updated 2010-07-13 19:09:06</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;">Correct drawing code in systemtray.
don't use rect() anymore as this rect doesn't contain borders in planar more and leads to drawing errors there. setting to same size as contentsRect.
Right easment is currently not used and therefore removed from calculation.
Qt::IntersectClip doesn't make sense here as this clips away the background for firstelements.
For 4.6 I would completly rewrite the drawing code for better reading. But to not introduce new bugs in 4.5 in this late phase I want to get this patch for now.</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/kdebase/workspace/plasma/generic/applets/systemtray/ui/applet.cpp <span style="color: grey">(1148491)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4623/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://reviewboard.kde.org/r/4623/s/458/"><img src="http://reviewboard.kde.org/media/uploaded/images/2010/07/13/systray_400x100.png" style="border: 1px black solid;" alt="systray in floating layout. bottom with patch applied" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>