> On Dec. 17, 2015, 5: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.
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. - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126324/#review89665 ----------------------------------------------------------- On Dec. 14, 2015, 5: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, 5: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 Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
