Share logic for QScreen property update emitting in helper class

With high-DPI scaling in place QScreen properties like the geometry
can be affected both by screen resolution changes, as well as logical
DPI changes. We want to ensure similar behavior in both cases when it
comes to which change-signals we emit, so centralizing this code makes
sense.

As the update of the cached primary orientation is trivial we do it
unconditionally.

Change-Id: I712005075a4b758180906fb88b2ac187b3dbe1ff
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

4 files changed, 92 insertions, 59 deletions

diff --git a/src/gui/kernel/qguiapplication.cpp b/src/gui/kernel/qguiapplication.cpp
index cc532401b4..7887db8534 100644
--- a/src/gui/kernel/qguiapplication.cpp
+++ b/src/gui/kernel/qguiapplication.cpp
@@ -3065,25 +3065,16 @@ void QGuiApplicationPrivate::processScreenGeometryChange(QWindowSystemInterfaceP
     if (!e->screen)
         return;
 
-    QScreen *s = e->screen.data();
-
-    bool geometryChanged = e->geometry != s->d_func()->geometry;
-    s->d_func()->geometry = e->geometry;
+    {
+        QScreen *s = e->screen.data();
+        QScreenPrivate::UpdateEmitter updateEmitter(s);
 
-    bool availableGeometryChanged = e->availableGeometry != s->d_func()->availableGeometry;
-    s->d_func()->availableGeometry = e->availableGeometry;
+        // Note: The incoming geometries have already been scaled by QHighDpi
+        // in the QWSI layer, so we don't need to call updateGeometry() here.
+        s->d_func()->geometry = e->geometry;
+        s->d_func()->availableGeometry = e->availableGeometry;
 
-    const Qt::ScreenOrientation primaryOrientation = s->primaryOrientation();
-    if (geometryChanged)
         s->d_func()->updatePrimaryOrientation();
-
-    s->d_func()->emitGeometryChangeSignals(geometryChanged, availableGeometryChanged);
-
-    if (geometryChanged) {
-        emit s->physicalSizeChanged(s->physicalSize());
-
-        if (s->primaryOrientation() != primaryOrientation)
-            emit s->primaryOrientationChanged(s->primaryOrientation());
     }
 
     resetCachedDevicePixelRatio();
@@ -3100,11 +3091,12 @@ void QGuiApplicationPrivate::processScreenLogicalDotsPerInchChange(QWindowSystem
     if (!e->screen)
         return;
 
-    QScreen *s = e->screen.data();
-    s->d_func()->logicalDpi = QDpi(e->dpiX, e->dpiY);
-
-    emit s->logicalDotsPerInchChanged(s->logicalDotsPerInch());
-    s->d_func()->updateGeometriesWithSignals();
+    {
+        QScreen *s = e->screen.data();
+        QScreenPrivate::UpdateEmitter updateEmitter(s);
+        s->d_func()->logicalDpi = QDpi(e->dpiX, e->dpiY);
+        s->d_func()->updateGeometry();
+    }
 
     resetCachedDevicePixelRatio();
 }
diff --git a/src/gui/kernel/qhighdpiscaling.cpp b/src/gui/kernel/qhighdpiscaling.cpp
index 342ee39b8a..666a79463f 100644
--- a/src/gui/kernel/qhighdpiscaling.cpp
+++ b/src/gui/kernel/qhighdpiscaling.cpp
@@ -545,7 +545,10 @@ void QHighDpiScaling::setGlobalFactor(qreal factor)
     m_active = m_globalScalingActive || m_screenFactorSet || m_platformPluginDpiScalingActive ;
     const auto screens = QGuiApplication::screens();
     for (QScreen *screen : screens)
-         screen->d_func()->updateGeometry();
+        screen->d_func()->updateGeometry();
+
+    // FIXME: The geometry has been updated based on the new scale factor,
+    // but we don't emit any geometry change signals for the screens.
 }
 
 static const char scaleFactorProperty[] = "_q_scaleFactor";
diff --git a/src/gui/kernel/qscreen.cpp b/src/gui/kernel/qscreen.cpp
index 32990de3d9..189f08b6ac 100644
--- a/src/gui/kernel/qscreen.cpp
+++ b/src/gui/kernel/qscreen.cpp
@@ -44,33 +44,6 @@ QScreen::QScreen(QPlatformScreen *screen)
     d->setPlatformScreen(screen);
 }
 
-void QScreenPrivate::updateGeometriesWithSignals()
-{
-    const QRect oldGeometry = geometry;
-    const QRect oldAvailableGeometry = availableGeometry;
-    updateGeometry();
-    emitGeometryChangeSignals(oldGeometry != geometry, oldAvailableGeometry != availableGeometry);
-}
-
-void QScreenPrivate::emitGeometryChangeSignals(bool geometryChanged, bool availableGeometryChanged)
-{
-    Q_Q(QScreen);
-    if (geometryChanged)
-        emit q->geometryChanged(geometry);
-
-    if (availableGeometryChanged)
-        emit q->availableGeometryChanged(availableGeometry);
-
-    if (geometryChanged || availableGeometryChanged) {
-        const auto siblings = q->virtualSiblings();
-        for (QScreen* sibling : siblings)
-            emit sibling->virtualGeometryChanged(sibling->virtualGeometry());
-    }
-
-    if (geometryChanged)
-        emit q->physicalDotsPerInchChanged(q->physicalDotsPerInch());
-}
-
 void QScreenPrivate::setPlatformScreen(QPlatformScreen *screen)
 {
     Q_Q(QScreen);
@@ -817,6 +790,60 @@ Q_GUI_EXPORT QDebug operator<<(QDebug debug, const QScreen *screen)
 }
 #endif // !QT_NO_DEBUG_STREAM
 
+QScreenPrivate::UpdateEmitter::UpdateEmitter(QScreen *screen)
+{
+    initialState.platformScreen = screen->handle();
+
+    // Use public APIs to read out current state, rather
+    // than accessing the QScreenPrivate members, so that
+    // we detect any changes to the high-DPI scale factors
+    // that may be applied in the getters.
+
+    initialState.logicalDpi = QDpi{
+        screen->logicalDotsPerInchX(),
+        screen->logicalDotsPerInchY()
+    };
+    initialState.geometry = screen->geometry();
+    initialState.availableGeometry = screen->availableGeometry();
+    initialState.primaryOrientation = screen->primaryOrientation();
+}
+
+QScreenPrivate::UpdateEmitter::~UpdateEmitter()
+{
+    QScreen *screen = initialState.platformScreen->screen();
+
+    const auto logicalDotsPerInch = QDpi{
+        screen->logicalDotsPerInchX(),
+        screen->logicalDotsPerInchY()
+    };
+    if (logicalDotsPerInch != initialState.logicalDpi)
+        emit screen->logicalDotsPerInchChanged(screen->logicalDotsPerInch());
+
+    const auto geometry = screen->geometry();
+    const auto geometryChanged = geometry != initialState.geometry;
+    if (geometryChanged)
+        emit screen->geometryChanged(geometry);
+
+    const auto availableGeometry = screen->availableGeometry();
+    const auto availableGeometryChanged = availableGeometry != initialState.availableGeometry;
+    if (availableGeometryChanged)
+        emit screen->availableGeometryChanged(availableGeometry);
+
+    if (geometryChanged || availableGeometryChanged) {
+        const auto siblings = screen->virtualSiblings();
+        for (QScreen* sibling : siblings)
+            emit sibling->virtualGeometryChanged(sibling->virtualGeometry());
+    }
+
+    if (geometryChanged) {
+        emit screen->physicalDotsPerInchChanged(screen->physicalDotsPerInch());
+
+        const auto primaryOrientation = screen->primaryOrientation();
+        if (primaryOrientation != initialState.primaryOrientation)
+            emit screen->primaryOrientationChanged(primaryOrientation);
+    }
+}
+
 QT_END_NAMESPACE
 
 #include "moc_qscreen.cpp"
diff --git a/src/gui/kernel/qscreen_p.h b/src/gui/kernel/qscreen_p.h
index fa7e1ec06a..b9e726a9dc 100644
--- a/src/gui/kernel/qscreen_p.h
+++ b/src/gui/kernel/qscreen_p.h
@@ -24,17 +24,8 @@
 
 QT_BEGIN_NAMESPACE
 
-class QScreenPrivate : public QObjectPrivate
+struct QScreenData
 {
-    Q_DECLARE_PUBLIC(QScreen)
-public:
-    void setPlatformScreen(QPlatformScreen *screen);
-    void updateGeometry();
-
-    void updatePrimaryOrientation();
-    void updateGeometriesWithSignals();
-    void emitGeometryChangeSignals(bool geometryChanged, bool availableGeometryChanged);
-
     QPlatformScreen *platformScreen = nullptr;
 
     Qt::ScreenOrientation orientation = Qt::PrimaryOrientation;
@@ -45,6 +36,26 @@ public:
     qreal refreshRate = 60;
 };
 
+class QScreenPrivate : public QObjectPrivate, public QScreenData
+{
+    Q_DECLARE_PUBLIC(QScreen)
+public:
+    void setPlatformScreen(QPlatformScreen *screen);
+    void updateGeometry();
+    void updatePrimaryOrientation();
+
+    class UpdateEmitter
+    {
+    public:
+        explicit UpdateEmitter(QScreen *screen);
+        ~UpdateEmitter();
+        UpdateEmitter(UpdateEmitter&&) noexcept = default;
+    private:
+        Q_DISABLE_COPY(UpdateEmitter)
+        QScreenData initialState;
+    };
+};
+
 QT_END_NAMESPACE
 
 #endif // QSCREEN_P_H