> 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.
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. - 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
