Fix preview zoom problems on windows

Apparently the window is moved to random places on multi-monitor setups
on windows.

Fixes: QDS-263
Change-Id: I21082c7031fefff3057074c147e82df7a88f4f78
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

5 files changed, 189 insertions, 74 deletions

diff --git a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.cpp b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.cpp
index 5bd96af582..5d2684b510 100644
--- a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.cpp
+++ b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.cpp
@@ -106,16 +106,9 @@ static void closeAllWindows()
 
 bool QQmlPreviewHandler::eventFilter(QObject *obj, QEvent *event)
 {
-    if (event->type() == QEvent::Show) {
-        if (QWindow *window = qobject_cast<QQuickWindow*>(obj)) {
-            m_lastPosition.initLastSavedWindowPosition(window);
-        }
-    }
-    if (m_currentWindow && (event->type() == QEvent::Move || event->type() == QEvent::Resize) &&
+    if (m_currentWindow && (event->type() == QEvent::Move) &&
             qobject_cast<QQuickWindow*>(obj) == m_currentWindow) {
-        // we always start with factor 1 so calculate and save the origin as it would be not scaled
-        m_lastPosition.setPosition(m_currentWindow->framePosition() *
-                                    QHighDpiScaling::factor(m_currentWindow));
+        m_lastPosition.takePosition(m_currentWindow);
     }
 
     return QObject::eventFilter(obj, event);
@@ -196,49 +189,38 @@ void QQmlPreviewHandler::rerun()
 
 void QQmlPreviewHandler::zoom(qreal newFactor)
 {
+    m_zoomFactor = newFactor;
+    QTimer::singleShot(0, this, &QQmlPreviewHandler::doZoom);
+}
+
+void QQmlPreviewHandler::doZoom()
+{
     if (!m_currentWindow)
         return;
-    if (qFuzzyIsNull(newFactor)) {
+    if (qFuzzyIsNull(m_zoomFactor)) {
         emit error(QString::fromLatin1("Zooming with factor: %1 will result in nothing " \
-                                       "so it will be ignored.").arg(newFactor));
+                                       "so it will be ignored.").arg(m_zoomFactor));
         return;
     }
-    QString errorMessage;
-    bool resetZoom = false;
 
-    if (newFactor < 0) {
+    bool resetZoom = false;
+    if (m_zoomFactor < 0) {
         resetZoom = true;
-        newFactor = 1.0;
+        m_zoomFactor = 1.0;
     }
 
-    // On single-window devices we allow any scale factor as the window will adapt to the screen.
-    if (m_supportsMultipleWindows) {
-        const QSize newAvailableScreenSize = QQmlPreviewPosition::currentScreenSize(m_currentWindow)
-                * QHighDpiScaling::factor(m_currentWindow) / newFactor;
-        if (m_currentWindow->size().width() > newAvailableScreenSize.width()) {
-            errorMessage = QString::fromLatin1(
-                        "Zooming with factor: "
-                        "%1 will result in a too wide preview.").arg(newFactor);
-        }
-        if (m_currentWindow->size().height() > newAvailableScreenSize.height()) {
-            errorMessage = QString::fromLatin1(
-                        "Zooming with factor: "
-                        "%1 will result in a too heigh preview.").arg(newFactor);
-        }
-    }
+    m_currentWindow->setGeometry(m_currentWindow->geometry());
 
-    if (errorMessage.isEmpty()) {
-        const QPoint newToOriginMappedPosition = m_currentWindow->position() *
-                QHighDpiScaling::factor(m_currentWindow) / newFactor;
-        m_currentWindow->destroy();
-        QHighDpiScaling::setScreenFactor(m_currentWindow->screen(), newFactor);
-        if (resetZoom)
-            QHighDpiScaling::updateHighDpiScaling();
-        m_currentWindow->setPosition(newToOriginMappedPosition);
-        m_currentWindow->show();
-    } else {
-        emit error(errorMessage);
-    }
+    m_lastPosition.takePosition(m_currentWindow, QQmlPreviewPosition::InitializePosition);
+    m_currentWindow->destroy();
+
+    for (QScreen *screen : QGuiApplication::screens())
+        QHighDpiScaling::setScreenFactor(screen, m_zoomFactor);
+    if (resetZoom)
+        QHighDpiScaling::updateHighDpiScaling();
+
+    m_currentWindow->show();
+    m_lastPosition.initLastSavedWindowPosition(m_currentWindow);
 }
 
 void QQmlPreviewHandler::removeTranslators()
diff --git a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.h b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.h
index 21ea672580..47491b9d8f 100644
--- a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.h
+++ b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewhandler.h
@@ -104,6 +104,7 @@ signals:
 protected:
     bool eventFilter(QObject *obj, QEvent *event);
 private:
+    void doZoom();
     void tryCreateObject();
     void showObject(QObject *object);
     void setCurrentWindow(QQuickWindow *window);
@@ -121,6 +122,7 @@ private:
     QVector<QPointer<QObject>> m_createdObjects;
     QScopedPointer<QQmlComponent> m_component;
     QPointer<QQuickWindow> m_currentWindow;
+    qreal m_zoomFactor = 1.0;
     bool m_supportsMultipleWindows;
     QQmlPreviewPosition m_lastPosition;
 
diff --git a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.cpp b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.cpp
index 3edcbac0a9..d4acd24da5 100644
--- a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.cpp
+++ b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.cpp
@@ -42,14 +42,47 @@
 #include <QtGui/qwindow.h>
 #include <QtGui/qscreen.h>
 #include <QtGui/qguiapplication.h>
+#include <private/qhighdpiscaling_p.h>
 
 QT_BEGIN_NAMESPACE
 
-static const QSize availableScreenSize(const QPoint &point)
+static QVector<QQmlPreviewPosition::ScreenData> initScreensData()
 {
-    if (const QScreen *screen = QGuiApplication::screenAt(point))
-        return screen->availableGeometry().size();
-    return QSize();
+    QVector<QQmlPreviewPosition::ScreenData> screensData;
+
+    for (QScreen *screen : QGuiApplication::screens()) {
+        QQmlPreviewPosition::ScreenData sd{screen->name(), screen->size()};
+        screensData.append(sd);
+    }
+    return screensData;
+}
+
+static QScreen *findScreen(const QString &nameOfScreen)
+{
+    for (QScreen *screen : QGuiApplication::screens()) {
+        if (screen->name() == nameOfScreen)
+            return screen;
+    }
+    return nullptr;
+}
+
+static QDataStream &operator<<(QDataStream &out, const QQmlPreviewPosition::ScreenData &screenData)
+{
+    out << screenData.name;
+    out << screenData.size;
+    return out;
+}
+
+static QDataStream &operator>>(QDataStream &in, QQmlPreviewPosition::ScreenData &screenData)
+{
+    in >> screenData.name;
+    in >> screenData.size;
+    return in;
+}
+
+bool QQmlPreviewPosition::ScreenData::operator==(const QQmlPreviewPosition::ScreenData &other) const
+{
+    return other.size == size && other.name == name;
 }
 
 QQmlPreviewPosition::QQmlPreviewPosition()
@@ -62,20 +95,36 @@ QQmlPreviewPosition::QQmlPreviewPosition()
     });
 }
 
-void QQmlPreviewPosition::setPosition(const QPoint &point)
+QQmlPreviewPosition::~QQmlPreviewPosition()
+{
+    saveWindowPosition();
+}
+
+void QQmlPreviewPosition::takePosition(QWindow *window, InitializeState state)
 {
-    m_hasPosition = true;
-    m_lastWindowPosition = point;
-    m_savePositionTimer.start();
+    Q_ASSERT(window);
+    // only save the position if we already tried to get the last saved position
+    if (m_initializeState == PositionInitialized) {
+        m_hasPosition = true;
+        auto screen = window->screen();
+        auto nativePosition = QHighDpiScaling::mapPositionToNative(window->framePosition(),
+                                                                   screen->handle());
+        m_lastWindowPosition = {screen->name(), nativePosition};
+
+        m_savePositionTimer.start();
+    }
+    if (state == InitializePosition)
+        m_initializeState = InitializePosition;
 }
 
 void QQmlPreviewPosition::saveWindowPosition()
 {
     if (m_hasPosition) {
+        const QByteArray positionAsByteArray = fromPositionToByteArray(m_lastWindowPosition);
         if (!m_settingsKey.isNull())
-            m_settings.setValue(m_settingsKey, m_lastWindowPosition);
+            m_settings.setValue(m_settingsKey, positionAsByteArray);
 
-        m_settings.setValue(QLatin1String("global_lastpostion"), m_lastWindowPosition);
+        m_settings.setValue(QLatin1String("global_lastpostion"), positionAsByteArray);
     }
 }
 
@@ -85,29 +134,86 @@ void QQmlPreviewPosition::loadWindowPositionSettings(const QUrl &url)
 
     if (m_settings.contains(m_settingsKey)) {
         m_hasPosition = true;
-        m_lastWindowPosition = m_settings.value(m_settingsKey).toPoint();
+        readLastPositionFromByteArray(m_settings.value(m_settingsKey).toByteArray());
     }
 }
 
 void QQmlPreviewPosition::initLastSavedWindowPosition(QWindow *window)
 {
-    if (m_positionedWindows.contains(window))
-        return;
+    Q_ASSERT(window);
+    m_initializeState = PositionInitialized;
+    if (m_currentInitScreensData.isEmpty())
+        m_currentInitScreensData = initScreensData();
+    // if it is the first time we just use the fall back from a last shown qml file
     if (!m_hasPosition) {
-        // in case there was nothing saved, we do not want to set anything
         if (!m_settings.contains(QLatin1String("global_lastpostion")))
             return;
-        m_lastWindowPosition = m_settings.value(QLatin1String("global_lastpostion")).toPoint();
+        readLastPositionFromByteArray(m_settings.value(QLatin1String("global_lastpostion"))
+                                              .toByteArray());
     }
-    if (QGuiApplication::screenAt(m_lastWindowPosition))
-        window->setFramePosition(m_lastWindowPosition);
+    setPosition(m_lastWindowPosition, window);
+}
+
+QByteArray QQmlPreviewPosition::fromPositionToByteArray(
+        const QQmlPreviewPosition::Position &position)
+{
+    QByteArray array;
+    QDataStream stream(&array, QIODevice::WriteOnly);
+    stream.setVersion(QDataStream::Qt_5_12);
+
+    const quint16 majorVersion = 1;
+    const quint16 minorVersion = 0;
+
+    stream << majorVersion
+           << minorVersion
+           << m_currentInitScreensData
+           << position.screenName
+           << position.nativePosition;
+    return array;
+}
+
+void QQmlPreviewPosition::readLastPositionFromByteArray(const QByteArray &array)
+{
+    QDataStream stream(array);
+    stream.setVersion(QDataStream::Qt_5_12);
+
+    // no version check for 1.0
+    //const quint16 currentMajorVersion = 1;
+    quint16 majorVersion = 0;
+    quint16 minorVersion = 0;
 
-    m_positionedWindows.append(window);
+    stream >> majorVersion >> minorVersion;
+
+    QVector<ScreenData> initScreensData;
+    stream >> initScreensData;
+
+    if (m_currentInitScreensData != initScreensData)
+        return;
+
+    QString nameOfScreen;
+    stream >> nameOfScreen;
+
+    QScreen *screen = findScreen(nameOfScreen);
+    if (!screen)
+        return;
+
+    QPoint nativePosition;
+    stream >> nativePosition;
+    if (nativePosition.isNull())
+        return;
+    m_lastWindowPosition = {nameOfScreen, nativePosition};
 }
 
-const QSize QQmlPreviewPosition::currentScreenSize(QWindow *window)
+void QQmlPreviewPosition::setPosition(const QQmlPreviewPosition::Position &position,
+                                      QWindow *window)
 {
-    return availableScreenSize(window->position());
+    if (position.nativePosition.isNull())
+        return;
+    if (QScreen *screen = findScreen(position.screenName)) {
+        window->setScreen(screen);
+        window->setFramePosition(QHighDpiScaling::mapPositionFromNative(position.nativePosition,
+                                                                        screen->handle()));
+    }
 }
 
 QT_END_NAMESPACE
diff --git a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.h b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.h
index 3d4ca9dc67..f403917f8c 100644
--- a/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.h
+++ b/src/plugins/qmltooling/qmldbg_preview/qqmlpreviewposition.h
@@ -56,6 +56,8 @@
 #include <QtCore/qurl.h>
 #include <QtCore/qtimer.h>
 #include <QtCore/qsettings.h>
+#include <QtCore/qsize.h>
+#include <QtCore/qdatastream.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -64,23 +66,46 @@ class QWindow;
 class QQmlPreviewPosition
 {
 public:
+    class ScreenData {
+    public:
+        bool operator==(const QQmlPreviewPosition::ScreenData &other) const;
+        QString name;
+        QSize size;
+    };
+    class Position {
+    public:
+        QString screenName;
+        QPoint nativePosition;
+    };
+    enum InitializeState {
+        InitializePosition,
+        PositionInitialized
+    };
+
     QQmlPreviewPosition();
+    ~QQmlPreviewPosition();
 
-    void setPosition(const QPoint &point);
-    void saveWindowPosition();
-    void loadWindowPositionSettings(const QUrl &url);
+
+    void takePosition(QWindow *window, InitializeState state = PositionInitialized);
     void initLastSavedWindowPosition(QWindow *window);
-    static const QSize currentScreenSize(QWindow *window);
+    void loadWindowPositionSettings(const QUrl &url);
 
 private:
+    void setPosition(const QQmlPreviewPosition::Position &position, QWindow *window);
+    QByteArray fromPositionToByteArray(const Position &position);
+    void readLastPositionFromByteArray(const QByteArray &array);
+    void saveWindowPosition();
+
     bool m_hasPosition = false;
-    QPoint m_lastWindowPosition;
+    InitializeState m_initializeState = InitializePosition;
     QSettings m_settings;
     QString m_settingsKey;
     QTimer m_savePositionTimer;
+    Position m_lastWindowPosition;
     QVector<QWindow *> m_positionedWindows;
-};
 
+    QVector<ScreenData> m_currentInitScreensData;
+};
 
 QT_END_NAMESPACE
 
diff --git a/tests/auto/qml/debugger/qqmlpreview/tst_qqmlpreview.cpp b/tests/auto/qml/debugger/qqmlpreview/tst_qqmlpreview.cpp
index f713aa76c3..c7f8ec1118 100644
--- a/tests/auto/qml/debugger/qqmlpreview/tst_qqmlpreview.cpp
+++ b/tests/auto/qml/debugger/qqmlpreview/tst_qqmlpreview.cpp
@@ -319,12 +319,12 @@ void tst_QQmlPreview::zoom()
     QTRY_VERIFY(m_files.contains(testFile(file)));
     float baseZoomFactor = -1;
     QTRY_VERIFY_WITH_TIMEOUT((baseZoomFactor = parseZoomFactor(m_process->output())) > 0, 30000);
-    m_client->triggerZoom(2.0f);
-    verifyZoomFactor(m_process, baseZoomFactor * 2.0f);
-    m_client->triggerZoom(1.5f);
-    verifyZoomFactor(m_process, baseZoomFactor * 1.5f);
-    m_client->triggerZoom(0.5f);
-    verifyZoomFactor(m_process, baseZoomFactor * 0.5f);
+
+    for (auto testZoomFactor : {2.0f, 1.5f, 0.5f}) {
+        m_client->triggerZoom(testZoomFactor);
+        verifyZoomFactor(m_process, baseZoomFactor * testZoomFactor);
+    }
+
     m_client->triggerZoom(-1.0f);
     verifyZoomFactor(m_process, baseZoomFactor);
     m_process->stop();