> On Dec. 14, 2015, 12:23 p.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.
> 
> René J.V. Bertin wrote:
>     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".
> 
> Boudhayan Gupta wrote:
>     In Qt5, a dependency on QtWidgets is the difference between having to use 
> a QGuiApplication (without) and QApplication (with). QtWidgets is a 20MB or 
> so dependency, so in terms of library load times at runtime the difference is 
> somewhat significant.
> 
> René J.V. Bertin wrote:
>     I'm sensitive to that kind of argument. Interesting btw; my OS X 
> QtWidgets library is 8.6Mb (which should include debug info), but on Linux 
> using the same script for installing Qt it is 97Mb (!). So there is indeed 
> some reason to introduce the extra dependency only on platforms where it's 
> needed, if it's needed.
>     
>     But (something I cannot NOT ask in this context) : just how many 
> applications are there that present a user interface without any need for 
> QtWidgets in any of their dependencies?

The idea is that you can write OpenGL apps without introducing the QtWidgets 
dependency, by using Qt's event loop and drawing on a QWindow. The KDE 
Frameworks follow this philosophy - because they're supposed to be used 
independently of KDE, the frameworks strive to minimise their dependency 
footprint.


- Boudhayan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126324/#review89447
-----------------------------------------------------------


On Dec. 13, 2015, 7:24 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, 7:24 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

Reply via email to