> On Dec. 17, 2015, 4:16 p.m., Martin Gräßlin wrote:
> > src/gui/kwindowconfig.h, lines 38-39
> > <https://git.reviewboard.kde.org/r/126324/diff/4/?file=422749#file422749line38>
> >
> > That doesn't match the method name. It's saveWindowSize, not
> > saveWindowGeometry. It's highly unexpected that saveWindowSize saves the
> > position.
> >
> > If you want that: please introduce a new saveWindowGeometry method.
>
> René J.V. Bertin wrote:
> 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 *negative* way that window size AND position are restored on
> a platform where this is the default behaviour?
>
> 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).
>
> 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).
>
> René J.V. Bertin wrote:
> 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).
>
> But please consider this: that KConfig key has been called `geometry` 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...
>
> Martin Gräßlin wrote:
> I leave it to the maintainers. On API I maintain I would say no to
> something changing the semantics like that.
>
> René J.V. Bertin wrote:
> 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 *after* restoring window size
> (because that operation *can* affect window position), but in the end that's
> (indeed) up to the code's developers to decide.
>
> 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.
>
> 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.
>
> Side-observation: OS X (Cocoa) provides a `[NSWindow
> setFrameAutosaveName:]` method, i.e. it avoids reference to specifics like
> size or geometry completely.
>
> 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 `[NSWindow
> setFrameautosaveName:[window representedFile]]`, where `[NSWindow
> representedFile]` corresponds to `QWindow::filePath` (but AFAICS those are
> not coupled in Qt5).
>
> 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.
>
> David Faure wrote:
> 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?)
>
> 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.
>
> René J.V. Bertin wrote:
> I just had a look: KMainWindow:setAutoSaveSettings() indeed leads to
> `KMainWindow::saveMainWindowSettings()`, 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.
>
>
> How many apps do *not* use KMainWindow, and how many libraries
> (frameworks) use KWindowConfig directly to keep dependencies down.
>
> 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 *is* restored when
> restoring the session state at login, at least on my KDE4 desktop.)
>
> David Faure wrote:
> I propose to add a saveWindowPosition next to saveWindowSize, and to call
> both from KMainWindow.
>
> To find out who uses KWindowConfig directly, use http://lxr.kde.org
>
> 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.
>
> René J.V. Bertin wrote:
> X11 also allows providing hints to the WM, which is how position restore
> could have been made optional IIRC.
>
> Is this really a question of X11 vs. the rest of the world, what about
> Wayland?
>
> 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
>
> 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.
>
> 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.
>
> I presume that absence of such an option is why
> save/restoreMainWindowSettings don't call QMainWindow::save/restoreState?
>
> PS: should I read `restoreMainWindowSettings` as "restore the main window
> settings" as opposed to "restore the mainwindow settings"
> (`restoreMainwindowSettings`)?
>
> David Faure wrote:
> 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....
>
> 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.
>
> 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.
>
> 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.
>
> 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.
>
> 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....)
>
> A good fallback solution is a
> KConfigGui::saveWindowGeometry/restoreWindowGeometry.
>
> 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 *changing* semantics of existing
> methods, but this is now about the semantics of a new method.
>
> René J.V. Bertin wrote:
> > 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....
>
> 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.
>
> 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.
>
> > If you had used LXR as I suggested you would have a much stronger
> argument against me ;)
>
> 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?
>
> > I assume you would want dialog positions to be stored also, on non-X11?
>
> 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.
>
> > [In that case] a KConfigGui::saveWindowGeometry would indeed be better
> API to avoid having to call two methods
>
> 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?
>
> QWidget::save/restoreGeometry:
> > 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?
>
> > 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.
>
> > If not then yeah, the best solution is to not save position, and
> document that in saveWindowGeometry.
>
> 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).
>
> Martin Gräßlin wrote:
> > is there actually a problem with saving/restoring the position on X11?
>
> 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.
>
> > 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?
>
> 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.
>
> As long as we cannot have the position relative to the connected screens
> it doesn't make sense.
>
> Concerning Wayland: on Wayland windows don't know there absolute position.
>
> David Faure wrote:
> Isn't this just an argument for being careful when restoring position, to
> make sure it fits within the available geometry?
> I thought there were stronger reasons against storing position, not one
> that can be fixed with a few qMin/qMax calls.
>
> René J.V. Bertin wrote:
> The argument is moot in any case on OS X: windows are restored in such a
> way that you can reposition them if their saved position cannot be restored
> exactly (And again, there is no window manager nor a set of rules related to
> such a thing, but if anything, position restore is the rule.)
> I'd also consider it a WM bug if it interprets the WMHints position
> without taking the actual screen estate into account; the whole idea with WM
> hints is that the WM can know better.
>
> David: you did take the fact into consideration that not all
> multi-monitor set-ups use multiple identical monitors, right? IOW, checking
> against the rectangle defined by two diagonally opposite corners of the
> spanned desktop doesn't necessarily catch all opportunities to map a window
> off-screen.
>
> Martin Gräßlin wrote:
> > not one that can be fixed with a few qMin/qMax calls
>
> This is not fixable with qMin/qMax calls. If the window is not on the
> screen where it was before it shouldn't have any position, so that it can be
> placed by the window manager. If we only sanitize the position through
> qMin/qMax we end up with a window positioning at a worse position than what
> the WM would have done. Now how do we know on which screen the window was on?
> We don't, because X11 lacks that info. A window is not bound to an XRandR
> screen. We would have to store quite some information in addition to the
> position. Like the complete setup at that time, all modes, etc. etc. Whenever
> anything of that doesn't match, we would have to fall back to not setting a
> position.
>
> Now that's quite some complex code to hack and impossible to test -
> XRandR based setups just cannot be tested.
OK, good points. But then my suggestion for a simple restore algorithm becomes:
if (saved pos fits within current screens geometry)
use saved pos
else
let WM do placement
It still helps quite a lot for everyone with a consistent screen setup, doesn't
it? [at least at saving time; e.g. I never shutdown my laptop with the office
monitor still attached to it, I use suspend-to-ram when leaving the office].
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126324/#review89665
-----------------------------------------------------------
On Dec. 14, 2015, 4:04 p.m., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126324/
> -----------------------------------------------------------
>
> (Updated Dec. 14, 2015, 4:04 p.m.)
>
>
> Review request for KDE Software on Mac OS X and KDE Frameworks.
>
>
> Repository: kconfig
>
>
> Description
> -------
>
> 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.
>
> 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?)
>
> 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).
>
> 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.
>
> 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.
>
> Note that for optimal functionality a companion patch to `KMainWindow::event`
> is required:
> ```
> --- a/src/kmainwindow.cpp
> +++ b/src/kmainwindow.cpp
> @@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)
> {
> K_D(KMainWindow);
> switch (ev->type()) {
> -#ifdef Q_OS_WIN
> +#if defined(Q_OS_WIN) || defined(Q_OS_OSX)
> case QEvent::Move:
> #endif
> case QEvent::Resize:
> ```
>
> 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?
>
>
> Diffs
> -----
>
> src/gui/kwindowconfig.h 48a8f3c
> src/gui/kwindowconfig.cpp d2f355c
>
> Diff: https://git.reviewboard.kde.org/r/126324/diff/
>
>
> Testing
> -------
>
> 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.
>
>
> Thanks,
>
> René J.V. Bertin
>
>
_______________________________________________
Kde-frameworks-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
