From kde-mac Sat Dec 26 09:17:43 2015 From: "Jaime Torres Amate" Date: Sat, 26 Dec 2015 09:17:43 +0000 To: kde-mac Subject: Re: [KDE/Mac] Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only s Message-Id: <20151226091743.29845.34235 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-mac&m=145112149117584 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============6230936937700808315==" --===============6230936937700808315== Content-Type: multipart/alternative; boundary="===============6584605355956298157==" --===============6584605355956298157== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On Dec. 17, 2015, 4:16 p.m., Martin Gräßlin wrote: > > > > Jaime Torres Amate wrote: > Hello, > > This is just a warning to know if this patch has been tested in a two monitor environment in a laptop. > In a pyqt application I save and restore the size and position of the window (without additional checks), using: > settings.setValue("size", self.ui.size()) > settings.setValue("pos", self.ui.pos()) # save > ---- > self.ui.resize(settings.value("size", QSize(400, 400))) > self.ui.move(settings.value("pos", QPoint(200, 200))) # restore > > It works perfectly fine except in this case: > The window, when saved, was in a monitor that disappears in the next run. In the next run (without the monitor) the window is not shown, because it is still in the other monitor, but it is shown in the taskbar, I have to go to the taskbar or with an advanced task manager, tell the window to become maximized, then it is shown. > If this patch shows the window when the monitor disappears, then, please, ignore this comment. > > René J.V. Bertin wrote: > What OS/windowing environment is that? > > To the best of my knowledge, OS X will rearrange windows when a monitor is disconnected (regardless whether on a laptop or desktop host) and should do the same when reopening a window in an offscreen position if that window isn't meant to be offscreen. I haven't yet tested this because of the rearranging thing, but good point. I'll see if it can be simulated by storing an offscreen position (this is where the binary nature of the saved data is annoying indeed!) > > Jaime Torres Amate wrote: > Oh!, I'm sorry, I forgot to say. It is a windows 7. It rearranges other windows, but as it's position is restored, it goes to the non connected monitor. I hope it does not happen with this patch (In linux it does not happen to that pyqt program). > > René J.V. Bertin wrote: > Even if it doesn't on OS X, someone will have to test on MS Windows. > > I presume one can obtain the limits within which the position has to lie and constrain the position so that at least part of the window is visible (even if those limits reflect only the rectangle within which the available screens lie). > > René J.V. Bertin wrote: > I just checked this, doing `frameGeo.adjust()` with a selection of huge values in `GeometryData::applyToWindow()`. As I thought, OS X doesn't allow you to open a regular window completely offscreen. The requested position is altered such that there's always just enough of it that remains visible and available for grabbing and moving it to a better position. > > Jaime: do you know what `self.ui.pos()` resolves to, `QWindow::position()` or `QWindow::framePosition()`? I would not really surprise me if trying to set the former (through `setPosition()`) gives different results than using the latter (through `setFramePosition()`) in case of offscreen co-ordinates. Hi, the documentation here is lacking the description, but I guess it is position(), as there are other bindings for frameGeometry(). Thanks for checking it. I have no more objections. Best regards. - Jaime Torres ----------------------------------------------------------- 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 > > --===============6584605355956298157== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126324/

On December 25th, 2015, 12:36 p.m. UTC, Jaime Torres Amate wrote:

Hello,

This is just a warning to know if this patch has been tested in a two monitor environment in a laptop. In a pyqt application I save and restore the size and position of the window (without additional checks), using: settings.setValue("size", self.ui.size()) settings.setValue("pos", self.ui.pos()) # save ---- self.ui.resize(settings.value("size", QSize(400, 400))) self.ui.move(settings.value("pos", QPoint(200, 200))) # restore

It works perfectly fine except in this case: The window, when saved, was in a monitor that disappears in the next run. In the next run (without the monitor) the window is not shown, because it is still in the other monitor, but it is shown in the taskbar, I have to go to the taskbar or with an advanced task manager, tell the window to become maximized, then it is shown. If this patch shows the window when the monitor disappears, then, please, ignore this comment.

On December 25th, 2015, 1:33 p.m. UTC, René J.V. Bertin wrote:

What OS/windowing environment is that?

To the best of my knowledge, OS X will rearrange windows when a monitor is disconnected (regardless whether on a laptop or desktop host) and should do the same when reopening a window in an offscreen position if that window isn't meant to be offscreen. I haven't yet tested this because of the rearranging thing, but good point. I'll see if it can be simulated by storing an offscreen position (this is where the binary nature of the saved data is annoying indeed!)

On December 25th, 2015, 4:13 p.m. UTC, Jaime Torres Amate wrote:

Oh!, I'm sorry, I forgot to say. It is a windows 7. It rearranges other windows, but as it's position is restored, it goes to the non connected monitor. I hope it does not happen with this patch (In linux it does not happen to that pyqt program).

On December 25th, 2015, 4:45 p.m. UTC, René J.V. Bertin wrote:

Even if it doesn't on OS X, someone will have to test on MS Windows.

I presume one can obtain the limits within which the position has to lie and constrain the position so that at least part of the window is visible (even if those limits reflect only the rectangle within which the available screens lie).

On December 26th, 2015, 12:06 a.m. UTC, René J.V. Bertin wrote:

I just checked this, doing frameGeo.adjust() with a selection of huge values in GeometryData::applyToWindow(). As I thought, OS X doesn't allow you to open a regular window completely offscreen. The requested position is altered such that there's always just enough of it that remains visible and available for grabbing and moving it to a better position.

Jaime: do you know what self.ui.pos() resolves to, QWindow::position() or QWindow::framePosition()? I would not really surprise me if trying to set the former (through setPosition()) gives different results than using the latter (through setFramePosition()) in case of offscreen co-or dinates.

Hi, the documentation here is lacking the description, but I guess it is position(), as there are other bindings for frameGeometry(). Thanks for checking it. I have no more objections. Best regards.


- Jaime Torres


On December 14th, 2015, 4:04 p.m. UTC, René J.V. Bertin wrote:

Review request for KDE Software on Mac OS X and KDE Frameworks.
By René J.V. Bertin.

Updated Dec. 14, 2015, 4:04 p.m.

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?

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.

Diffs

  • src/gui/kwindowconfig.h (48a8f3c)
  • src/gui/kwindowconfig.cpp (d2f355c)

View Diff

--===============6584605355956298157==-- --===============6230936937700808315== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18Ka2RlLW1hY0Br ZGUub3JnCkxpc3QgSW5mb3JtYXRpb246IGh0dHBzOi8vbWFpbC5rZGUub3JnL21haWxtYW4vbGlz dGluZm8va2RlLW1hYwpLREUvTWFjIEluZm9ybWF0aW9uOiBodHRwOi8vY29tbXVuaXR5LmtkZS5v cmcvTWFj --===============6230936937700808315==--