<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/110824/">http://git.reviewboard.kde.org/r/110824/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 4th, 2013, 3:22 p.m. UTC, <b>Martin Gräßlin</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;">I think the fix is applied at the wrong place. The actual problem is that window not on the current activity are included in the layouting and so on which results in the empty spots where a window is. So the fix should be to not include such windows in the first place, then it doesn't matter where the user clicks.</pre>
</blockquote>
<p>On June 4th, 2013, 4:16 p.m. UTC, <b>Stefanos Harhalakis</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;">This would mean changing one of the following:
* Workspace::xStackingOrder()
* EffectsHandlerImpl::stackingOrder()
I assume you don't mean Workspace::xStackingOrder() so that would be EffectsHandlerImpl::stackingOrder(). Right?
In any case, if isOnCurrentActivity() is expensive as Thomas said, would it be ok to proactively run this against all available windows instead of doing it at the very last moment?
Changing EffectsHandlerImpl for example, would mean this:
foreach (Toplevel *w, list)
if (!w->isOnCurrentActivity())
ret.append(effectWindow(w));
</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;">Correction:
foreach (Toplevel *w, list)
if (w->isOnCurrentActivity())
ret.append(effectWindow(w));</pre>
<br />
<p>- Stefanos</p>
<br />
<p>On June 4th, 2013, 3:02 p.m. UTC, Stefanos Harhalakis 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 kde-workspace.</div>
<div>By Stefanos Harhalakis.</div>
<p style="color: grey;"><i>Updated June 4, 2013, 3:02 p.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;">Fix windowAt function to only return windows from current activity.
</pre>
</td>
</tr>
</table>
<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=301447">301447</a>
</div>
<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/desktopgrid/desktopgrid.cpp <span style="color: grey">(dc3d82b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110824/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>