<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 17th, 2015, 5:16 p.m. CET, <b>Martin Gräßlin</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="https://git.reviewboard.kde.org/r/126324/diff/4/?file=422749#file422749line38" style="color: black; font-weight: bold; text-decoration: underline;">src/gui/kwindowconfig.h</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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; ">class QWindow;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">37</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * global or application config file.</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">38</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * global or application config file.<span class="hl"> On MS Windows and Mac OS X this also</span></span></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">39</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * saves the window position.</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That doesn't match the method name. It's saveWindowSize, not saveWindowGeometry. It's highly unexpected that saveWindowSize saves the position.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If you want that: please introduce a new saveWindowGeometry method.</p></pre>
</blockquote>
<p>On December 17th, 2015, 6:21 p.m. CET, <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 was afraid someone was going to say that, which is why I tried to argue that it's highly unexpected from a user viewpoint that only window size is saved and not position. How often would it happen that a developer is "highly surprised" in a <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">negative</em> way that window size AND position are restored on a platform where this is the default behaviour?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have nothing against introducing a pair of new methods, but how is that supposed to be done in transparent fashion? I do have a lot against a need to change all dependent software to call those methods (maintenance burden and all that).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Counter proposal: replace save/restoreWindowSize with save/restoreWindowGeometry everywhere, with a platform-specific interpretation of what exactly geometry encompasses. Much less surprise there, just a bit more need to read the documentation. Are these functions ever called intentionally outside of what I suppose is a more or less automatic feature that takes care of restoring window, erm, layout (saving is clearly automatic).</p></pre>
</blockquote>
<p>On December 17th, 2015, 6:36 p.m. CET, <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;">Just to be clear: if I am going to introduce restore/saveWindowGeometry methods they'll replace the WindowSize variants on OS X or at least those will then use a different KConfig key to avoid conflicts.
I'd also be dropping the MS Windows part of the patch (as this is not a decision I want to make for a platform I don't use).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But please consider this: that KConfig key has been called <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">geometry</code> for a long time. Where exactly is the surprise, that restore/saveWindowSize never did what the key they operate with suggests, or that they have always been using an inaptly named key? For me the answer is straightforward and based on what users will expect...</p></pre>
</blockquote>
<p>On December 18th, 2015, 8:08 a.m. CET, <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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I leave it to the maintainers. On API I maintain I would say no to something changing the semantics like that.</p></pre>
</blockquote>
<p>On December 18th, 2015, 1:02 p.m. CET, <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;">As I just wrote in reply to a message from Valorie, I have absolutely no issue with maintaining methods for saving and restoring only window size, for code that somehow requires that. I'd guess that such code would probably enforce the intended window position itself <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">after</em> restoring window size (because that operation <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">can</em> affect window position), but in the end that's (indeed) up to the code's developers to decide.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">IOW, I'm perfectly willing to discuss a better solution in which the burden to ensure that window save/restore works as "natively" as possible on each platform is shared. The best way to do that is of course to have a single pair of methods that have platform-specific implementations.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As far as I'm concerned such a solution might even be prepared completely in KConfig/gui before changes are made everywhere else to deploy that new solution. In that case I would for instance run temporary local (MacPorts) patches that replace saveWindowSize/restoreWindowSize with wrappers for saveWindowGeometry/restoreWindowGeometry.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Side-observation: OS X (Cocoa) provides a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">[NSWindow setFrameAutosaveName:]</code> method, i.e. it avoids reference to specifics like size or geometry completely.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That method also provides another thought that could be taken into consideration if it is decided to evolve this part of the frameworks, something I'd be interested in collaborating on. Currently, there is no support for saving and restoring multiple windows per application. That may be more or less sufficient when applications always follow a MDI approach, but even if they do that still doesn't make them applications that are active only in a single instance. Example: KDevelop. One might expect that opening a given, pre-existing session (collection of open projects) restores the main window geometry (size and/or position) that used previously for that session, rather than the geometry used by whatever KDevelop session was run last. On OS X that would be done with something like <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">[NSWindow setFrameautosaveName:[window representedFile]]</code>, where <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">[NSWindow representedFile]</code> corresponds to <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QWindow::filePath</code> (but AFAICS those are not coupled in Qt5).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I already had a quick look, but realised I don't know if the KConfig mechanism has facilities to handle cleanup of stale/obsolete key/value entries.</p></pre>
</blockquote>
<p>On December 19th, 2015, 11:13 a.m. CET, <b>David Faure</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;">Note that most apps use this via the higher-level KMainWindow::setAutoSaveSettings() anyway, which is supposed to 'do the right thing'.
So my suggestion is to fix things one level higher - let saveWindowSize save only the window size, but update KMainWindow::saveMainWindowSettings/restoreMainWindowSettings to also store geometry on platforms (windowing systems, more precisely) where it makes sense to also store the position (i.e. non-X11, as I understand it?)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">René: you are wrong about "no support for saving and restoring multiple windows per application", that is definitely there, see the "groupName" argument to setAutoSaveSettings or the KConfigGroup argument to KWindowConfig::saveWindowSize(). Different (types of) mainwindows in the same application can use different config groups.</p></pre>
</blockquote>
<p>On December 19th, 2015, 10:16 p.m. CET, <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 had a look: KMainWindow:setAutoSaveSettings() indeed leads to <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KMainWindow::saveMainWindowSettings()</code>, which still uses KWindowConfig::saveWindowSize(). So you're proposing what, to add a call to save position too where appropriate, or to replace saveWindowSize in those cases?
It's a solution, but I don't really like the idea of fixing things above the level where the actual work is being done. In my experience it's a great way to get deja-vu kind of situations where you wonder why that fix you applied isn't working anymore, only to find out that some bit of code you hadn't encountered before uses the lower level directly.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">How many apps do <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> use KMainWindow, and how many libraries (frameworks) use KWindowConfig directly to keep dependencies down.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have been wondering why in fact position isn't saved on X11 desktops too, as far as that is not in fact the case? (position <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">is</em> restored when restoring the session state at login, at least on my KDE4 desktop.)</p></pre>
</blockquote>
<p>On December 20th, 2015, 12:08 a.m. CET, <b>David Faure</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 propose to add a saveWindowPosition next to saveWindowSize, and to call both from KMainWindow.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To find out who uses KWindowConfig directly, use http://lxr.kde.org</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Position is restored on X11 because ksmserver+kwin takes care of it, which fits with "the window manager takes care of position on X11". Both during session management and when launching apps interactively.</p></pre>
</blockquote>
<p>On December 20th, 2015, 11:01 a.m. CET, <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;">X11 also allows providing hints to the WM, which is how position restore could have been made optional IIRC.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is this really a question of X11 vs. the rest of the world, what about Wayland?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Anyway, I don't like the idea of having to call several functions (and introduce a set of new functions) if there is no reason those new functions will ever be used outside of saveMainWindowSettings/restoreMainWindowSettings</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KXmlGui already links to QtWidgets, so there is no extra cost in allowing saveMainWindowSettings/restoreMainWindowSettings to let QWidget::saveGeometry/restoreGeometry handle all settings related to window size and position. Those are the functions designed to work as properly as possible on all supported platforms.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's a pity that QWidget::restoreGeometry doesn't have an optional filter to select the aspects to restore: that would be the most elegant option. Use a single function to save the relevant information, and another single function with a platform-specific filter argument to restore it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I presume that absence of such an option is why save/restoreMainWindowSettings don't call QMainWindow::save/restoreState?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">PS: should I read <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">restoreMainWindowSettings</code> as "restore the main window settings" as opposed to "restore the mainwindow settings" (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">restoreMainwindowSettings</code>)?</p></pre>
</blockquote>
<p>On December 25th, 2015, 10:11 a.m. CET, <b>David Faure</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;">No clue about whether WMs on wayland handle window positioning. Well, in a way all windowing systems including OSX and Windows do handle positioning of new windows, don't they? It's not like all your windows and dialogs appear at (0,0) on OSX or Windows.
I'm wondering if there's really a difference here....</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If you had used LXR as I suggested you would have a much stronger argument against me ;) http://lxr.kde.org/ident?_i=saveWindowSize&_remember=1 actually shows a huge list of code that uses KConfigGui::saveWindowSize directly: for dialogs.
I assume you would want dialog positions to be stored also, on non-X11? In that case a KConfigGui::saveWindowGeometry would indeed be better API to avoid having to call two methods in all these places.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I didn't know about QByteArray QWidget::saveGeometry() (when I worked on this kmainwindow code Qt 4.2 didn't exist yet). It has three problems though: 1) it's an ugly blob of binary data, 2) it's most probably broken on OSX (look at the ifdef in the implementation), 3) it's in QWidget rather than QWindow, so it's not the right solution for QML based windows.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please forget saveState/restoreState, that's an even bigger hammer (which includes the position of toolbars and dockwidgets etc.), and also widget-only, even worse, mainwindows-only.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">PS: it's called KMainWindow, hence the name restoreMainWindowSettings. It's the settings for that instance of KMainWindow, there can be many instances. Don't read "main" as "the one and only primary", that's not what the main in [QK]MainWindow means, it just means it's a window with toolbars and statusbars.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">IMHO a good solution would be to contribute to Qt a QWindow::saveGeometry/restoreGeometry, similar to the QWidget one but at the QWindow level (it makes more sense there, not only for QML... who wants to save/restore the geometry of a checkbox....)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">A good fallback solution is a KConfigGui::saveWindowGeometry/restoreWindowGeometry.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Martin: is there actually a problem with saving/restoring the position on X11? The WM does clever auto-positioning for new windows, but if as a user I position some dialog on the right side of the screen, and I find it there again next time, it's fine, right? If not then yeah, the best solution is to not save position, and document that in saveWindowGeometry.
I think your objection was about <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">changing</em> semantics of existing methods, but this is now about the semantics of a new method.</p></pre>
</blockquote>
<p>On December 25th, 2015, 12:57 p.m. CET, <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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's not like all your windows and dialogs appear at (0,0) on OSX or Windows.
I’m wondering if there's really a difference here....</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I’ve asked myself the same thing. The difference is probably in how windows are positioned initially (I don’t know any way to configure it on OS X or MS Windows), and what happens when a window is reopened. Another difference is how the window server/manager handles positioning instructions. The lack of a default positioning choice is probably what makes it obey the instructions on OS X/MS Windows, whereas an X11 window manager has to find a compromise between its user setting and what an application requests.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Note that OS X does have a cascading option in which windows are opened with a slight offset w.r.t. each other, but that’s an application, not a system-wide user choice.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If you had used LXR as I suggested you would have a much stronger argument against me ;)</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Actually I did and saw what you saw (or maybe I searched for restoreWindowSize). I suppose didn't mention it because I didn't want to be accused of arguing too much?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I assume you would want dialog positions to be stored also, on non-X11?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd say that for dialogs it's more important that they reopen on the screen they were last used, but restoring position is probably the best way to achieve that without complexifying the code unnecessarily.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[In that case] a KConfigGui::saveWindowGeometry would indeed be better API to avoid having to call two methods</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I’d argue that’s always the case and that the most elegant solution would be using a saveWindowGeometry() method combined with a restoreGeometry() that takes additional flags that control what saved data are to be restored (with a platform-dependent default or a platform-dependent "RestoreWhatsUsualHere" constant). The flags could also instruct if position is to be restored "hard" or through a WMHint - I take it KWin supports those?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QWidget::save/restoreGeometry:</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1) it's an ugly blob of binary data
That’d be saved as base64 to avoid issues with binary. In a reimplementation we could easily use a different method to generate a human-readable QByteArray. Parsing that might not be so easy though?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2) it's most probably broken on OSX (look at the ifdef in the implementation)
I wondered about that, but in fact it works just fine as far as I’ve been able to check.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If not then yeah, the best solution is to not save position, and document that in saveWindowGeometry.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Did I mention I think the choice should be at the moment of restoring the information? :)
If anything that would have the advantage that information doesn’t get lost, and can be restored when the user changes a global preference (or changes from X11 to Wayland, presuming Wayland restores position by default).</p></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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">is there actually a problem with saving/restoring the position on X11?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">of course! That's why it's not implemented. I consider it as a stupid idea to save the position. And the reasoning probably also applies to OSX.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The WM does clever auto-positioning for new windows, but if as a user I position some dialog on the right side of the screen, and I find it there again next time, it's fine, right?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">On X11 the window specified position is used, if provided. ICCCM explicitly says that a WM has to honor the position, so that's fine. The problem is with multiple screen. If I close a window on external screen, then disconnect and open again, the window will be positioned outside the viewable area. It's a window which cannot be interacted with. So no, please don't store the position, bad idea! The same argument might also be relevant on OSX.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As long as we cannot have the position relative to the connected screens it doesn't make sense.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Concerning Wayland: on Wayland windows don't know there absolute position.</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On December 14th, 2015, 5:04 p.m. CET, 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, 5: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>