From 75f22702933bad4f0da2b63a94ea183021771e4c Mon Sep 17 00:00:00 2001 From: Yuhang Zhao <2546789017@qq.com> Date: Thu, 7 Apr 2022 10:32:52 +0800 Subject: [PATCH] Windows: Make QScreen::name() more user-friendly, take 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ø Reviewed-by: Morten Johan Sørvig Reviewed-by: Volker Hilsheimer --- .../platforms/windows/qwindowsscreen.cpp | 54 ++++++++++++++----- .../platforms/windows/qwindowsscreen.h | 4 +- 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(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 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 virtualSiblings() const override; QWindow *topLevelAt(const QPoint &point) const override;