> On Dec. 14, 2015, 7:53 a.m., Martin Gräßlin wrote: > > src/gui/CMakeLists.txt, line 2 > > <https://git.reviewboard.kde.org/r/126324/diff/3/?file=421934#file421934line2> > > > > this introduces a QtWidgets dependency and thus changes the integration > > level of the framework. I highly recommend to not go this way. > > > > Looking at the code there is no reason for this. The usage of > > QDesktopWidget is not needed as QScreen provides the same. Similar one > > doesn't need the QWidget usage as QWindow is already there.
I'm all for reducing the number of dependencies, but haven't found another way to get at QWidget::saveGeometry and QWidget::restoreGeometry. You're probably much more familiar with what those functions really save and restore, and thus to what extent they're essentially convenience functions here for something I could just as well access via QWindow::geometry or QWindow::frameGeometry. I'd have to figure out on my end which of the two I'd need to use because that'd be specific to OS X (knowing there is no QWindow::setFrameGeometry). I won't be able to test that on MS Windows though. What integration level are you invoking? This dependency doesn't make kconfig a Tier 2 framework, does it? Is it so bad to add a dependency on Qt5Widgets to something that already depends on Qt5Gui? A more fundamental question would be why this is in KConfig. One could argue that window size (and position) are not application configuration parameters when they're saved automatically; they're a reflection of application interface state (@). Maybe a subtle difference (and maybe a debate that was already held a long time ago), but doesn't this rather (or better) belong in KWindowSystem? @) Off-topic, but like other state variables saved automatically it might even be wise to save them in a separate file so it's easier to reset state without doing a full "factory reset". - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126324/#review89447 ----------------------------------------------------------- On Dec. 13, 2015, 2:54 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. 13, 2015, 2:54 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/CMakeLists.txt 9663e09 > 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
