<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="https://git.reviewboard.kde.org/r/115903/">https://git.reviewboard.kde.org/r/115903/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">moving the GrabRole seems fine to me, but it has two small problems:
* if the window removes the property the grabrole needs to be removed
* if a window adds the property the grabrole needs to be added
This probably needs adjustment in slotPropertyNotify</pre>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/115903/diff/1/?file=244991#file244991line250" style="color: black; font-weight: bold; text-decoration: underline;">kwin/effects/slidingpopups/slidingpopups.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">void SlidingPopupsEffect::paintWindow(EffectWindow* w, int mask, QRegion region, WindowPaintData& data)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">250</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="n">m_backgroundContrastForced</span><span class="p">.</span><span class="n">contains</span><span class="p">(</span><span class="n">w</span><span class="p">)</span> <span class="o">&&</span> <span class="n">w</span><span class="o">-></span><span class="n">hasAlpha</span><span class="p">()</span> <span class="o">&&</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">251</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">w</span><span class="o">-></span><span class="n">data</span><span class="p">(</span><span class="n">WindowForceBackgroundContrastRole</span><span class="p">).</span><span class="n">toBool</span><span class="p">())</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">252</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">w</span><span class="o">-></span><span class="n">setData</span><span class="p">(</span><span class="n">WindowForceBackgroundContrastRole</span><span class="p">,</span> <span class="n">QVariant</span><span class="p">());</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">253</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_backgroundContrastForced</span><span class="p">.</span><span class="n">removeAll</span><span class="p">(</span><span class="n">w</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">250</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">254</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think there should be a better place for this than in postPaintWindow. We only want to do this once per window and not always.</pre>
</div>
<br />
<p>- Martin Gräßlin</p>
<br />
<p>On February 20th, 2014, 4:27 a.m. CET, Sebastian Kügler wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 kwin and Plasma.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated Feb. 20, 2014, 4:27 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=329991">329991</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</div>
<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;">slidingpopups claim windowClosedGrabRole earlier
This seems like a more proper fix for the flickering issue in the
sliding popups effect. The problem is that slidingpopups grabs the
window in windowClosed, the fade effect checks it there, which makes
it racy.
In my tests, I've not seen this problem with the WindowAddedGrab, but
as far as I understand, the problem may well be present there as well.
(And my proposed trick doesn't work.) I've not seen this happening in my
debugging, however. The problem there is also less visible since the
transparency curves go into the same direction, and are more "in line
with each other".
So, proposed fix: Move the setData(WindowClosedGrabRole, ...) call from
windowClosed into windowAdded, which makes sure it's set whenever the
window goes away.
constness for list
Force backgroundcontrast during slidingpopup animations
This fixes the sliding popups losing their contrast effect when
animating, less flicker.
In this patch, we temporarily force the contrast effect on, but only if
it hasn't been explicitely disabled. As soon as the animation stops, the
force flag is disabled again. For disappearing windows, we just set the
flag in the same way, but skip over the bookkeeping, since the window is
going to be deleted, anyway.</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;">No flickering observed anymore when closing the window. No side-effects visible. With print-debugging, I verified that the WindowClosedGrabRole flag is now picked up.</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>kwin/effects/slidingpopups/slidingpopups.h <span style="color: grey">(f66e42ec4205c6a9d0375a89d0f5cc07ab163365)</span></li>
<li>kwin/effects/slidingpopups/slidingpopups.cpp <span style="color: grey">(4f0a9ea11db858b1d4233330662c3a35c5a956f6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115903/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>