Windows: Make QScreen::name() more user-friendly, take 2

The first attempt introduced in e9fd1c6aab28f had an unwanted
side-effect which makes the screen name non-unique and made the
screen manager think multiple different monitors with the
same name is one same monitor. This behavior is of course wrong
and unwanted but it was missed due to there's no unit test
for this property and I was not familiar enough with QPA
stuff. As a result the change was reverted in 851dc581cf9a76.

However, the original change is still a good improvement,
so we bring it back, but fixed the side-effect mentioned above,
by letting the screen manager compare the device name instead
of monitor name.

If multiple monitors have the same name, a numeric suffix
will be appended to the monitor name to let people still be
able to distinguish between them.

Change-Id: Ic03105d0825abbb3d21cb51d1da3ab2cbb4ca913
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 45 insertions, 13 deletions

diff --git a/src/plugins/platforms/windows/qwindowsscreen.cpp b/src/plugins/platforms/windows/qwindowsscreen.cpp
index 2c46ff14ee..6b60083759 100644
--- a/src/plugins/platforms/windows/qwindowsscreen.cpp
+++ b/src/plugins/platforms/windows/qwindowsscreen.cpp
@@ -117,8 +117,21 @@ static bool monitorData(HMONITOR hMonitor, QWindowsScreenData *data)
     data->hMonitor = hMonitor;
     data->geometry = QRect(QPoint(info.rcMonitor.left, info.rcMonitor.top), QPoint(info.rcMonitor.right - 1, info.rcMonitor.bottom - 1));
     data->availableGeometry = QRect(QPoint(info.rcWork.left, info.rcWork.top), QPoint(info.rcWork.right - 1, info.rcWork.bottom - 1));
-    data->name = QString::fromWCharArray(info.szDevice);
-    if (data->name == u"WinDisc") {
+    data->deviceName = QString::fromWCharArray(info.szDevice);
+    DISPLAYCONFIG_PATH_INFO pathInfo = {};
+    const bool hasPathInfo = getPathInfo(info, &pathInfo);
+    if (hasPathInfo) {
+        DISPLAYCONFIG_TARGET_DEVICE_NAME deviceName = {};
+        deviceName.header.type = DISPLAYCONFIG_DEVICE_INFO_GET_TARGET_NAME;
+        deviceName.header.size = sizeof(DISPLAYCONFIG_TARGET_DEVICE_NAME);
+        deviceName.header.adapterId = pathInfo.targetInfo.adapterId;
+        deviceName.header.id = pathInfo.targetInfo.id;
+        if (DisplayConfigGetDeviceInfo(&deviceName.header) == ERROR_SUCCESS)
+            data->name = QString::fromWCharArray(deviceName.monitorFriendlyDeviceName);
+    }
+    if (data->name.isEmpty())
+        data->name = data->deviceName;
+    if (data->deviceName == u"WinDisc") {
         data->flags |= QWindowsScreenData::LockScreen;
     } else {
         if (const HDC hdc = CreateDC(info.szDevice, nullptr, nullptr, nullptr)) {
@@ -133,15 +146,14 @@ static bool monitorData(HMONITOR hMonitor, QWindowsScreenData *data)
             DeleteDC(hdc);
         } else {
             qWarning("%s: Unable to obtain handle for monitor '%s', defaulting to %g DPI.",
-                     __FUNCTION__, qPrintable(QString::fromWCharArray(info.szDevice)),
+                     __FUNCTION__, qPrintable(data->deviceName),
                      data->dpi.first);
         } // CreateDC() failed
     } // not lock screen
 
     // ### We might want to consider storing adapterId/id from DISPLAYCONFIG_PATH_TARGET_INFO,
     // if we are going to use DISPLAYCONFIG lookups more.
-    DISPLAYCONFIG_PATH_INFO pathInfo = {};
-    if (getPathInfo(info, &pathInfo)) {
+    if (hasPathInfo) {
         switch (pathInfo.targetInfo.rotation) {
         case DISPLAYCONFIG_ROTATION_IDENTITY:
             data->orientation = Qt::LandscapeOrientation;
@@ -184,6 +196,16 @@ BOOL QT_WIN_CALLBACK monitorEnumCallback(HMONITOR hMonitor, HDC, LPRECT, LPARAM
     QWindowsScreenData data;
     if (monitorData(hMonitor, &data)) {
         auto *result = reinterpret_cast<WindowsScreenDataList *>(p);
+        auto it = std::find_if(result->rbegin(), result->rend(),
+            [&data](QWindowsScreenData i){ return i.name == data.name; });
+        if (it != result->rend()) {
+            int previousIndex = 1;
+            if (it->deviceIndex.has_value())
+                previousIndex = it->deviceIndex.value();
+            else
+                (*it).deviceIndex = 1;
+            data.deviceIndex = previousIndex + 1;
+        }
         // QWindowSystemInterface::handleScreenAdded() documentation specifies that first
         // added screen will be the primary screen, so order accordingly.
         // Note that the side effect of this policy is that there is no way to change primary
@@ -217,7 +239,8 @@ static QDebug operator<<(QDebug dbg, const QWindowsScreenData &d)
         << " physical: " << d.physicalSizeMM.width() << 'x' << d.physicalSizeMM.height()
         << " DPI: " << d.dpi.first << 'x' << d.dpi.second << " Depth: " << d.depth
         << " Format: " << d.format
-        << " hMonitor: " << d.hMonitor;
+        << " hMonitor: " << d.hMonitor
+        << " device name: " << d.deviceName;
     if (d.flags & QWindowsScreenData::PrimaryScreen)
         dbg << " primary";
     if (d.flags & QWindowsScreenData::VirtualDesktop)
@@ -243,6 +266,13 @@ QWindowsScreen::QWindowsScreen(const QWindowsScreenData &data) :
 {
 }
 
+QString QWindowsScreen::name() const
+{
+    return m_data.deviceIndex.has_value()
+               ? (u"%1 (%2)"_s).arg(m_data.name, QString::number(m_data.deviceIndex.value()))
+               : m_data.name;
+}
+
 Q_GUI_EXPORT QPixmap qt_pixmapFromWinHBITMAP(HBITMAP bitmap, int hbitmapFormat = 0);
 
 QPixmap QWindowsScreen::grabWindow(WId window, int xIn, int yIn, int width, int height) const
@@ -492,19 +522,19 @@ bool QWindowsScreenManager::isSingleScreen()
 }
 
 static inline int indexOfMonitor(const QWindowsScreenManager::WindowsScreenList &screens,
-                                 const QString &monitorName)
+                                 const QString &deviceName)
 {
     for (int i= 0; i < screens.size(); ++i)
-        if (screens.at(i)->data().name == monitorName)
+        if (screens.at(i)->data().deviceName == deviceName)
             return i;
     return -1;
 }
 
 static inline int indexOfMonitor(const WindowsScreenDataList &screenData,
-                                 const QString &monitorName)
+                                 const QString &deviceName)
 {
     for (int i = 0; i < screenData.size(); ++i)
-        if (screenData.at(i).name == monitorName)
+        if (screenData.at(i).deviceName == deviceName)
             return i;
     return -1;
 }
@@ -570,7 +600,7 @@ bool QWindowsScreenManager::handleScreenChanges()
     const bool lockScreen = newDataList.size() == 1 && (newDataList.front().flags & QWindowsScreenData::LockScreen);
     bool primaryScreenChanged = false;
     for (const QWindowsScreenData &newData : newDataList) {
-        const int existingIndex = indexOfMonitor(m_screens, newData.name);
+        const int existingIndex = indexOfMonitor(m_screens, newData.deviceName);
         if (existingIndex != -1) {
             m_screens.at(existingIndex)->handleChanges(newData);
             if (existingIndex == 0)
@@ -587,7 +617,7 @@ bool QWindowsScreenManager::handleScreenChanges()
     // temporary lock screen to avoid window recreation (QTBUG-33062).
     if (!lockScreen) {
         for (int i = m_screens.size() - 1; i >= 0; --i) {
-            if (indexOfMonitor(newDataList, m_screens.at(i)->data().name) == -1)
+            if (indexOfMonitor(newDataList, m_screens.at(i)->data().deviceName) == -1)
                 removeScreen(i);
         }     // for existing screens
     }     // not lock screen
diff --git a/src/plugins/platforms/windows/qwindowsscreen.h b/src/plugins/platforms/windows/qwindowsscreen.h
index 563b02f110..babb9de758 100644
--- a/src/plugins/platforms/windows/qwindowsscreen.h
+++ b/src/plugins/platforms/windows/qwindowsscreen.h
@@ -33,6 +33,8 @@ struct QWindowsScreenData
     Qt::ScreenOrientation orientation = Qt::LandscapeOrientation;
     qreal refreshRateHz = 60;
     HMONITOR hMonitor = nullptr;
+    QString deviceName = {};
+    std::optional<int> deviceIndex = std::nullopt;
 };
 
 class QWindowsScreen : public QPlatformScreen
@@ -53,7 +55,7 @@ public:
     QDpi logicalBaseDpi() const override { return QDpi(baseDpi, baseDpi); }
     qreal devicePixelRatio() const override { return 1.0; }
     qreal refreshRate() const override { return m_data.refreshRateHz; }
-    QString name() const override { return m_data.name; }
+    QString name() const override;
     Qt::ScreenOrientation orientation() const override { return m_data.orientation; }
     QList<QPlatformScreen *> virtualSiblings() const override;
     QWindow *topLevelAt(const QPoint &point) const override;