<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/103756/">http://git.reviewboard.kde.org/r/103756/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 22nd, 2012, 7:54 p.m., <b>Thomas Lübking</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="http://git.reviewboard.kde.org/r/103756/diff/1/?file=47685#file47685line250" style="color: black; font-weight: bold; text-decoration: underline;">kwin/geometry.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; ">QRect Workspace::clientArea(clientAreaOption opt, int screen, int desktop) const</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="k">if</span> <span class="p">(</span><span class="n">screen</span> <span class="o">==</span> <span class="o">-</span><span class="mi">1</span><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">250</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">screen</span> <span class="o">==</span> <span class="o">-</span><span class="mi">1</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">should possibly be
if (is_multihead)
screen = screen_number;
else if (screen == -1)
....??
is screen_number part of the "make kwin work on real multihead" thing? looks like it's statically assigned to DefaultScreen(dpy) in main.cpp what does not cover the case when one head has multiple (randr) screens?!</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not sure why it is that way. Last commit which changed it: "Replace -1 with active screen in clientArea() if needed."
So no idea whether this is related to multi head at all. -1 means active screen, I think.
> does not cover the case when one head has multiple (randr) screens?!
I think it is a save assumption to say that someone using multi head won't use xrandr as well. Even if not I think we can make that a limitation. If someone wants multi head he has to live with it.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 22nd, 2012, 7:54 p.m., <b>Thomas Lübking</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="http://git.reviewboard.kde.org/r/103756/diff/1/?file=47685#file47685line255" style="color: black; font-weight: bold; text-decoration: underline;">kwin/geometry.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; ">QRect Workspace::clientArea(clientAreaOption opt, int screen, int desktop) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">255</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">is_multihead</span><span class="p">)</span> <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">255</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">is_multihead</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">not your change, but looks like senseless code duplication.
esp: why does the is_multihead block test "screen < screenarea[desktop].size()" but uses screenarea[desktop][screen_number]?
Otherwise the blocks are equal but for screen/screen_number</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">will keep that for now as well and probably have a more close look on why we do all these calculations in the first place. Seems all a bit redundant.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 22nd, 2012, 7:54 p.m., <b>Thomas Lübking</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="http://git.reviewboard.kde.org/r/103756/diff/1/?file=47685#file47685line283" style="color: black; font-weight: bold; text-decoration: underline;">kwin/geometry.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; ">QRect Workspace::clientArea(clientAreaOption opt, int screen, int desktop) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">283</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">Kephal</span><span class="o">::</span><span class="n">ScreenUtils</span><span class="o">::</span><span class="n">screenGeometry</span><span class="p">(</span><span class="n">screen_number</span><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">281</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">Kephal</span><span class="o">::</span><span class="n">ScreenUtils</span><span class="o">::</span><span class="n">screenGeometry</span><span class="p">(</span><span class="n">screen_number</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">since we acquire sarea/warea above (including some sanity check), can't we just return it here?
Otherwise it maybe shouldn't be calculated / fetched against screenarea[desktop] unconditionally but just for MaximizeArea & resp. WorkArea</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">will keep it for now as well.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 22nd, 2012, 7:54 p.m., <b>Thomas Lübking</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="http://git.reviewboard.kde.org/r/103756/diff/1/?file=47686#file47686line229" style="color: black; font-weight: bold; text-decoration: underline;">kwin/manage.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; ">bool Client::manage(Window w, bool isMapped)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">229</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">screen</span> <span class="o">=</span> <span class="n">options</span><span class="o">-></span><span class="n">xineramaPlacementScreen</span><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">229</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">screen</span> <span class="o">=</span> <span class="n">options</span><span class="o">-></span><span class="n">xineramaPlacementScreen</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">afaics this option is dead.
I've that key nowhere in no config and it wasn't provided by the xinerama kcm either</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">yes, you are right, it's nowhere set and I dropped it.</pre>
<br />
<p>- Martin</p>
<br />
<p>On January 22nd, 2012, 10:21 a.m., Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 kwin and Plasma.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Jan. 22, 2012, 10:21 a.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;">As discussed on the mailinglist: drop of the xinerama related options and the kcm. Given that the show unmanaged windows on option is in fact not used, I dropped the complete KCM.</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;">Fullscreen: works
Maximize: works
Movment: works
Placement: works</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>kcontrol/CMakeLists.txt <span style="color: grey">(8cd9a46)</span></li>
<li>kcontrol/xinerama/CMakeLists.txt <span style="color: grey">(fe332e5)</span></li>
<li>kcontrol/xinerama/Messages.sh <span style="color: grey">(f4aa134)</span></li>
<li>kcontrol/xinerama/interface_util.h <span style="color: grey">(8a4fcd1)</span></li>
<li>kcontrol/xinerama/kcmxinerama.h <span style="color: grey">(18b9241)</span></li>
<li>kcontrol/xinerama/kcmxinerama.cpp <span style="color: grey">(a456b2c)</span></li>
<li>kcontrol/xinerama/test_kcm_xinerama.cpp <span style="color: grey">(a358a2c)</span></li>
<li>kcontrol/xinerama/xinerama.desktop <span style="color: grey">(e8ce525)</span></li>
<li>kcontrol/xinerama/xineramawidget.h <span style="color: grey">(2c446a2)</span></li>
<li>kcontrol/xinerama/xineramawidget.cpp <span style="color: grey">(df9cb2f)</span></li>
<li>kcontrol/xinerama/xineramawidget.ui <span style="color: grey">(90ec4d4)</span></li>
<li>kwin/geometry.cpp <span style="color: grey">(a414e26)</span></li>
<li>kwin/manage.cpp <span style="color: grey">(c551eac)</span></li>
<li>kwin/options.h <span style="color: grey">(9dc29cf)</span></li>
<li>kwin/options.cpp <span style="color: grey">(d496569)</span></li>
<li>kwin/tabbox/tabbox.cpp <span style="color: grey">(3051316)</span></li>
<li>kwin/toplevel.cpp <span style="color: grey">(ffe7f0c)</span></li>
<li>kwin/workspace.cpp <span style="color: grey">(69b4ecb)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103756/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>