<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126324/">https://git.reviewboard.kde.org/r/126324/</a>
</td>
</tr>
</table>
<br />
<p>On December 25th, 2015, 12:36 p.m. UTC, <b>Jaime Torres Amate</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hello,</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is just a warning to know if this patch has been tested in a two monitor environment in a laptop.
In a pyqt application I save and restore the size and position of the window (without additional checks), using:
settings.setValue("size", self.ui.size())
settings.setValue("pos", self.ui.pos()) # save
----
self.ui.resize(settings.value("size", QSize(400, 400)))
self.ui.move(settings.value("pos", QPoint(200, 200))) # restore</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It works perfectly fine except in this case:
The window, when saved, was in a monitor that disappears in the next run. In the next run (without the monitor) the window is not shown, because it is still in the other monitor, but it is shown in the taskbar, I have to go to the taskbar or with an advanced task manager, tell the window to become maximized, then it is shown.
If this patch shows the window when the monitor disappears, then, please, ignore this comment.</p></pre>
</blockquote>
<p>On December 25th, 2015, 1:33 p.m. UTC, <b>René J.V. Bertin</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What OS/windowing environment is that?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To the best of my knowledge, OS X will rearrange windows when a monitor is disconnected (regardless whether on a laptop or desktop host) and should do the same when reopening a window in an offscreen position if that window isn't meant to be offscreen. I haven't yet tested this because of the rearranging thing, but good point. I'll see if it can be simulated by storing an offscreen position (this is where the binary nature of the saved data is annoying indeed!)</p></pre>
</blockquote>
<p>On December 25th, 2015, 4:13 p.m. UTC, <b>Jaime Torres Amate</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Oh!, I'm sorry, I forgot to say. It is a windows 7. It rearranges other windows, but as it's position is restored, it goes to the non connected monitor. I hope it does not happen with this patch (In linux it does not happen to that pyqt program).</p></pre>
</blockquote>
<p>On December 25th, 2015, 4:45 p.m. UTC, <b>René J.V. Bertin</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Even if it doesn't on OS X, someone will have to test on MS Windows.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I presume one can obtain the limits within which the position has to lie and constrain the position so that at least part of the window is visible (even if those limits reflect only the rectangle within which the available screens lie).</p></pre>
</blockquote>
<p>On December 26th, 2015, 12:06 a.m. UTC, <b>René J.V. Bertin</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I just checked this, doing <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">frameGeo.adjust()</code> with a selection of huge values in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">GeometryData::applyToWindow()</code>. As I thought, OS X doesn't allow you to open a regular window completely offscreen. The requested position is altered such that there's always just enough of it that remains visible and available for grabbing and moving it to a better position.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Jaime: do you know what <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">self.ui.pos()</code> resolves to, <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QWindow::position()</code> or <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QWindow::framePosition()</code>? I would not really surprise me if trying to set the former (through <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">setPosition()</code>) gives different results than using the latter (through <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">setFramePosition()</code>) in case of offscreen co-ordinates.</p></pre>
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hi, the documentation here is lacking the description, but I guess it is position(), as there are other bindings for frameGeometry().
Thanks for checking it. I have no more objections.
Best regards.</p></pre>
<br />
<p>- Jaime Torres</p>
<br />
<p>On December 14th, 2015, 4:04 p.m. UTC, René J.V. Bertin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Software on Mac OS X and KDE Frameworks.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Dec. 14, 2015, 4:04 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kconfig
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In KDElibs4, the KMainWindow::saveWindowSize() and KMainWindow::restoreWindowSize() function saved and restored not only the size but also the position (i.e. the geometry) of windows, using QWidget::saveGeometry and QWidget::restoreGeometry.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2 main reasons for this (according to the comments):
- Under X11 restoring the position is tricky
- X11 has a window manager which might be considered responsible for that functionality (and I suppose most modern WMs have the feature enabled by default?)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Both arguments are moot on MS Windows and OS X, and on both platforms users expect to see window positions restored as well as window size. On OS X there is also little choice in the matter: most applications offer the geometry restore without asking (IIRC it is the same on MS Windows).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would thus like to propose to port the platform-specific code that existed for MS Windows (and for OS X as a MacPorts patch that apparently was never submitted upstreams). I realise that this violates the message conveyed by the function names but I would like to think that this is a case where function is more important.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You may also notice that the Mac version does not store resolution-specific settings. This happens to work best on OS X, where multi-screen support has been present since the early nineties, and where window geometry is restored regardless of the screen resolution (i.e. connect a different external screen with a different resolution, and windows will reopen as they were on that screen, not with some default geometry).
I required I can update the comments in the header to reflect this subtlety.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Note that for optimal functionality a companion patch to <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KMainWindow::event</code> is required:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"><span style="color: #A00000">--- a/src/kmainwindow.cpp</span>
<span style="color: #00A000">+++ b/src/kmainwindow.cpp</span>
<span style="color: #800080; font-weight: bold">@@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)</span>
{
K_D(KMainWindow);
switch (ev->type()) {
<span style="color: #A00000">-#ifdef Q_OS_WIN</span>
<span style="color: #00A000">+#if defined(Q_OS_WIN) || defined(Q_OS_OSX)</span>
case QEvent::Move:
#endif
case QEvent::Resize:
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This ensures that the window geometry save is performed also after a move (to update the position) without requiring a dummy resizing operation.
Do I need to create a separate RR for this change or is it small enough that I can push it if and when this RR is accepted?</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">On OS X 10.6 through 10.9 with various KDElibs4 versions and now with Qt 5.5.1 and frameworks 5.16.0 (and Kate as a test application).
I presume that the MS Windows code has been tested sufficiently in KDELibs4; I have only adapted it to Qt5 and tested if it builds.</p></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>src/gui/kwindowconfig.h <span style="color: grey">(48a8f3c)</span></li>
<li>src/gui/kwindowconfig.cpp <span style="color: grey">(d2f355c)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126324/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>