From 980795dc557772d117ec03d16646557665eb895e Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Mon, 15 Jun 2020 14:07:48 +0200 Subject: Let QScreen::grabWindow's winId parameter default to 0 and add test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform plugins are implemented to grab the entire screen if no window ID is provided. They do not grab the entire virtual screen, just the screen the method is called on. On macOS, the implementation ignored the window parameter, and always grabbed the entire virtual screen. This change fixes the cocoa implementation. The test passes in local tests (with two displays with different dpr). Since grabbing a screen returns an image with managed colors, we need to convert it to sRGB color spec first, otherwise displaying a grabbed image will produce different results. This will need to be changed once Qt supports a fully color managed flow. The test does not cover the case where a window spans multiple displays, since this is generally not supported at least on macOS. The code that exists in QCocoaScreen to handle that case is untested, but with the exception of the optimization it is also unchanged. Done-with: Morten Sørvig Change-Id: I8ac1233e56d559230ff9e10111abfb6227431e8c Fixes: QTBUG-84876 Reviewed-by: Tor Arne Vestbø --- src/gui/kernel/qscreen.cpp | 5 +- src/gui/kernel/qscreen.h | 2 +- src/plugins/platforms/cocoa/qcocoascreen.h | 1 + src/plugins/platforms/cocoa/qcocoascreen.mm | 119 ++++++++++++++-------- tests/auto/gui/kernel/qscreen/tst_qscreen.cpp | 138 ++++++++++++++++++++++++++ 5 files changed, 219 insertions(+), 46 deletions(-) diff --git a/src/gui/kernel/qscreen.cpp b/src/gui/kernel/qscreen.cpp index 3fd1548d04..1d11358b3d 100644 --- a/src/gui/kernel/qscreen.cpp +++ b/src/gui/kernel/qscreen.cpp @@ -703,10 +703,11 @@ QScreen *QScreen::virtualSiblingAt(QPoint point) /*! Creates and returns a pixmap constructed by grabbing the contents of the given \a window restricted by QRect(\a x, \a y, \a width, - \a height). + \a height). If \a window is 0, then the entire screen will be + grabbed. The arguments (\a{x}, \a{y}) specify the offset in the window, - whereas (\a{width}, \a{height}) specify the area to be copied. If + whereas (\a{width}, \a{height}) specify the area to be copied. If \a width is negative, the function copies everything to the right border of the window. If \a height is negative, the function copies everything to the bottom of the window. diff --git a/src/gui/kernel/qscreen.h b/src/gui/kernel/qscreen.h index 039940f196..1bff3f4ec1 100644 --- a/src/gui/kernel/qscreen.h +++ b/src/gui/kernel/qscreen.h @@ -144,7 +144,7 @@ public: bool isPortrait(Qt::ScreenOrientation orientation) const; bool isLandscape(Qt::ScreenOrientation orientation) const; - QPixmap grabWindow(WId window, int x = 0, int y = 0, int w = -1, int h = -1); + QPixmap grabWindow(WId window = 0, int x = 0, int y = 0, int w = -1, int h = -1); qreal refreshRate() const; diff --git a/src/plugins/platforms/cocoa/qcocoascreen.h b/src/plugins/platforms/cocoa/qcocoascreen.h index 5a5d7fc735..ffbb67b677 100644 --- a/src/plugins/platforms/cocoa/qcocoascreen.h +++ b/src/plugins/platforms/cocoa/qcocoascreen.h @@ -79,6 +79,7 @@ public: // ---------------------------------------------------- + static NSScreen *nativeScreenForDisplayId(CGDirectDisplayID displayId); NSScreen *nativeScreen() const; void requestUpdate(); diff --git a/src/plugins/platforms/cocoa/qcocoascreen.mm b/src/plugins/platforms/cocoa/qcocoascreen.mm index ebf1afb57d..70d02d1a0d 100644 --- a/src/plugins/platforms/cocoa/qcocoascreen.mm +++ b/src/plugins/platforms/cocoa/qcocoascreen.mm @@ -592,70 +592,99 @@ QWindow *QCocoaScreen::topLevelAt(const QPoint &point) const return window; } +/*! + \internal + + Coordinates are in screen coordinates if \a view is 0, otherwise they are in view + coordiantes. +*/ QPixmap QCocoaScreen::grabWindow(WId view, int x, int y, int width, int height) const { - // Determine the grab rect. FIXME: The rect should be bounded by the view's - // geometry, but note that for the pixeltool use case that window will be the - // desktop widget's view, which currently gets resized to fit one screen - // only, since its NSWindow has the NSWindowStyleMaskTitled flag set. - Q_UNUSED(view); + /* + Grab the grabRect section of the specified display into a pixmap that has + sRGB color spec. Once Qt supports a fully color-managed flow and conversions + that don't lose the colorspec information, we would want the image to maintain + the color spec of the display from which it was grabbed. Ultimately, rendering + the returned pixmap on the same display from which it was grabbed should produce + identical visual results. + */ + auto grabFromDisplay = [](CGDirectDisplayID displayId, const QRect &grabRect) -> QPixmap { + QCFType image = CGDisplayCreateImageForRect(displayId, grabRect.toCGRect()); + const QCFType sRGBcolorSpace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB); + if (CGImageGetColorSpace(image) != sRGBcolorSpace) { + qCDebug(lcQpaScreen) << "applying color correction for display" << displayId; + image = CGImageCreateCopyWithColorSpace(image, sRGBcolorSpace); + } + QPixmap pixmap = QPixmap::fromImage(qt_mac_toQImage(image)); + pixmap.setDevicePixelRatio(nativeScreenForDisplayId(displayId).backingScaleFactor); + return pixmap; + }; + QRect grabRect = QRect(x, y, width, height); qCDebug(lcQpaScreen) << "input grab rect" << grabRect; - // Find which displays to grab from, or all of them if the grab size is unspecified + if (!view) { + // coordinates are relative to the screen + if (!grabRect.isValid()) // entire screen + grabRect = QRect(QPoint(0, 0), geometry().size()); + else + grabRect.translate(-geometry().topLeft()); + return grabFromDisplay(displayId(), grabRect); + } + + // grab the window; grab rect in window coordinates might span multiple screens + NSView *nsView = reinterpret_cast(view); + NSPoint windowPoint = [nsView convertPoint:NSMakePoint(0, 0) toView:nil]; + NSRect screenRect = [nsView.window convertRectToScreen:NSMakeRect(windowPoint.x, windowPoint.y, 1, 1)]; + QPoint position = mapFromNative(screenRect.origin).toPoint(); + QSize size = QRectF::fromCGRect(NSRectToCGRect(nsView.bounds)).toRect().size(); + QRect windowRect = QRect(position, size); + if (!grabRect.isValid()) + grabRect = windowRect; + else + grabRect.translate(windowRect.topLeft()); + + // Find which displays to grab from const int maxDisplays = 128; CGDirectDisplayID displays[maxDisplays]; CGDisplayCount displayCount; - CGRect cgRect = (width < 0 || height < 0) ? CGRectInfinite : grabRect.toCGRect(); + CGRect cgRect = grabRect.isValid() ? grabRect.toCGRect() : CGRectInfinite; const CGDisplayErr err = CGGetDisplaysWithRect(cgRect, maxDisplays, displays, &displayCount); if (err || displayCount == 0) return QPixmap(); - // If the grab size is not specified, set it to be the bounding box of all screens, - if (width < 0 || height < 0) { - QRect windowRect; - for (uint i = 0; i < displayCount; ++i) { - QRect displayBounds = QRectF::fromCGRect(CGDisplayBounds(displays[i])).toRect(); - // Only include the screen if it is positioned past the x/y position - if ((displayBounds.x() >= x || displayBounds.right() > x) && - (displayBounds.y() >= y || displayBounds.bottom() > y)) { - windowRect = windowRect.united(displayBounds); - } - } - if (grabRect.width() < 0) - grabRect.setWidth(windowRect.width()); - if (grabRect.height() < 0) - grabRect.setHeight(windowRect.height()); - } - qCDebug(lcQpaScreen) << "final grab rect" << grabRect << "from" << displayCount << "displays"; // Grab images from each display - QVector images; + QVector pixmaps; QVector destinations; for (uint i = 0; i < displayCount; ++i) { auto display = displays[i]; - QRect displayBounds = QRectF::fromCGRect(CGDisplayBounds(display)).toRect(); - QRect grabBounds = displayBounds.intersected(grabRect); + const QRect displayBounds = QRectF::fromCGRect(CGDisplayBounds(display)).toRect(); + const QRect grabBounds = displayBounds.intersected(grabRect); if (grabBounds.isNull()) { destinations.append(QRect()); - images.append(QImage()); + pixmaps.append(QPixmap()); continue; } - QRect displayLocalGrabBounds = QRect(QPoint(grabBounds.topLeft() - displayBounds.topLeft()), grabBounds.size()); - QImage displayImage = qt_mac_toQImage(QCFType(CGDisplayCreateImageForRect(display, displayLocalGrabBounds.toCGRect()))); - displayImage.setDevicePixelRatio(displayImage.size().width() / displayLocalGrabBounds.size().width()); - images.append(displayImage); - QRect destBounds = QRect(QPoint(grabBounds.topLeft() - grabRect.topLeft()), grabBounds.size()); + const QRect displayLocalGrabBounds = QRect(QPoint(grabBounds.topLeft() - displayBounds.topLeft()), grabBounds.size()); + + qCDebug(lcQpaScreen) << "grab display" << i << "global" << grabBounds << "local" << displayLocalGrabBounds; + QPixmap displayPixmap = grabFromDisplay(display, displayLocalGrabBounds); + // Fast path for when grabbing from a single screen only + if (displayCount == 1) + return displayPixmap; + + qCDebug(lcQpaScreen) << "grab sub-image size" << displayPixmap.size() << "devicePixelRatio" << displayPixmap.devicePixelRatio(); + pixmaps.append(displayPixmap); + const QRect destBounds = QRect(QPoint(grabBounds.topLeft() - grabRect.topLeft()), grabBounds.size()); destinations.append(destBounds); - qCDebug(lcQpaScreen) << "grab display" << i << "global" << grabBounds << "local" << displayLocalGrabBounds - << "grab image size" << displayImage.size() << "devicePixelRatio" << displayImage.devicePixelRatio(); } // Determine the highest dpr, which becomes the dpr for the returned pixmap. qreal dpr = 1.0; for (uint i = 0; i < displayCount; ++i) - dpr = qMax(dpr, images.at(i).devicePixelRatio()); + dpr = qMax(dpr, pixmaps.at(i).devicePixelRatio()); // Allocate target pixmap and draw each screen's content qCDebug(lcQpaScreen) << "Create grap pixmap" << grabRect.size() << "at devicePixelRatio" << dpr; @@ -664,7 +693,7 @@ QPixmap QCocoaScreen::grabWindow(WId view, int x, int y, int width, int height) windowPixmap.fill(Qt::transparent); QPainter painter(&windowPixmap); for (uint i = 0; i < displayCount; ++i) - painter.drawImage(destinations.at(i), images.at(i)); + painter.drawPixmap(destinations.at(i), pixmaps.at(i)); return windowPixmap; } @@ -754,19 +783,23 @@ QCocoaScreen *QCocoaScreen::get(CFUUIDRef uuid) return nullptr; } -NSScreen *QCocoaScreen::nativeScreen() const +NSScreen *QCocoaScreen::nativeScreenForDisplayId(CGDirectDisplayID displayId) { - if (!m_displayId) - return nil; // The display has been disconnected - for (NSScreen *screen in NSScreen.screens) { - if (screen.qt_displayId == m_displayId) + if (screen.qt_displayId == displayId) return screen; } - return nil; } +NSScreen *QCocoaScreen::nativeScreen() const +{ + if (!m_displayId) + return nil; // The display has been disconnected + + return nativeScreenForDisplayId(m_displayId); +} + CGPoint QCocoaScreen::mapToNative(const QPointF &pos, QCocoaScreen *screen) { Q_ASSERT(screen); diff --git a/tests/auto/gui/kernel/qscreen/tst_qscreen.cpp b/tests/auto/gui/kernel/qscreen/tst_qscreen.cpp index 9f2a767d60..baa36f2c4e 100644 --- a/tests/auto/gui/kernel/qscreen/tst_qscreen.cpp +++ b/tests/auto/gui/kernel/qscreen/tst_qscreen.cpp @@ -26,6 +26,8 @@ ** ****************************************************************************/ +#include +#include #include #include @@ -41,6 +43,9 @@ private slots: void transformBetween_data(); void transformBetween(); void orientationChange(); + + void grabWindow_data(); + void grabWindow(); }; void tst_QScreen::angleBetween_data() @@ -197,5 +202,138 @@ void tst_QScreen::orientationChange() QCOMPARE(spy.count(), ++expectedSignalCount); } +void tst_QScreen::grabWindow_data() +{ + if (QGuiApplication::platformName().startsWith(QLatin1String("offscreen"), Qt::CaseInsensitive)) + QSKIP("Offscreen: Screen grabbing not implemented."); + + QTest::addColumn("screenIndex"); + QTest::addColumn("screenName"); + QTest::addColumn("grabWindow"); + QTest::addColumn("windowRect"); + QTest::addColumn("grabRect"); + + int screenIndex = 0; + for (const auto screen : QGuiApplication::screens()) { + const QByteArray screenName = screen->name().toUtf8(); + const QRect availableGeometry = screen->availableGeometry(); + const QPoint topLeft = availableGeometry.topLeft() + QPoint(20, 20); + QTest::addRow("%s - Window", screenName.data()) + << screenIndex << screenName << true << QRect(topLeft, QSize(200, 200)) << QRect(0, 0, -1, -1); + QTest::addRow("%s - Window Section", screenName.data()) + << screenIndex << screenName << true << QRect(topLeft, QSize(200, 200)) << QRect(50, 50, 100, 100); + QTest::addRow("%s - Screen", screenName.data()) + << screenIndex << screenName << false << QRect(topLeft, QSize(200, 200)) << QRect(0, 0, -1, -1); + QTest::addRow("%s - Screen Section", screenName.data()) + << screenIndex << screenName << false << QRect(topLeft, QSize(200, 200)) << QRect(topLeft, QSize(200, 200)); + + ++screenIndex; + } +} + +void tst_QScreen::grabWindow() +{ + QFETCH(int, screenIndex); + QFETCH(QByteArray, screenName); + QFETCH(bool, grabWindow); + QFETCH(QRect, windowRect); + QFETCH(QRect, grabRect); + + class Window : public QRasterWindow + { + public: + Window(QScreen *scr) + : image(scr->size(), QImage::Format_ARGB32_Premultiplied) + { + setFlags(Qt::CustomizeWindowHint|Qt::FramelessWindowHint|Qt::WindowStaysOnTopHint); + setScreen(scr); + image.setDevicePixelRatio(scr->devicePixelRatio()); + } + QImage image; + + protected: + void resizeEvent(QResizeEvent *e) override + { + const QSize sz = e->size(); + image = image.scaled(sz * image.devicePixelRatioF()); + QPainter painter(&image); + painter.fillRect(0, 0, sz.width(), sz.height(), Qt::black); + painter.setPen(QPen(Qt::red, 2)); + painter.drawLine(0, 0, sz.width(), sz.height()); + painter.drawLine(0, sz.height(), sz.width(), 0); + painter.drawRect(0, 0, sz.width(), sz.height()); + } + void paintEvent(QPaintEvent *) override + { + QPainter painter(this); + painter.drawImage(0, 0, image); + } + }; + const auto screens = QGuiApplication::screens(); + double highestDpr = 0; + for (auto screen : screens) + highestDpr = qMax(highestDpr, screen->devicePixelRatio()); + + QScreen *screen = screens.at(screenIndex); + QCOMPARE(screen->name().toUtf8(), screenName); + const double screenDpr = screen->devicePixelRatio(); + + Window window(screen); + window.setGeometry(windowRect); + window.show(); + + if (!QTest::qWaitForWindowExposed(&window)) + QSKIP("Failed to expose window - aborting"); + + if (QGuiApplication::platformName().startsWith(QLatin1String("xcb"), Qt::CaseInsensitive)) + QTest::qWait(1500); // this is ridiculously necessary because of effects combined with slowness of VMs +#ifdef Q_OS_MACOS // wait for desktop on screen to scroll into place + QTest::qWait(1000); +#endif + + QSize expectedGrabSize = grabRect.isValid() ? grabRect.size() : (grabWindow ? windowRect.size() : screen->size()); + // we ask for pixel coordinates, but will get a pixmap with device-specific DPR + expectedGrabSize *= screen->devicePixelRatio(); + + // the painted image will always be in the screen's DPR + QImage paintedImage = window.image; + QCOMPARE(paintedImage.devicePixelRatio(), screenDpr); + + const QPixmap pixmap = screen->grabWindow(grabWindow ? window.winId() : 0, grabRect.x(), grabRect.y(), grabRect.width(), grabRect.height()); + + QImage grabbedImage = pixmap.toImage(); + const QSize grabbedSize = grabbedImage.size(); + QCOMPARE(grabbedSize, expectedGrabSize); + + QPoint pixelOffset = QPoint(0, 0); + if (!grabRect.isValid()) { + if (grabWindow) { + // if we grab the entire window, then the grabbed image should be as large as the window + QCOMPARE(grabbedImage.size(), paintedImage.size()); + } else { + // if we grab the entire screen, then the grabbed image should be as large as the screen + QCOMPARE(grabbedImage.size(), screen->size() * screenDpr); + pixelOffset = window.geometry().topLeft() - screen->geometry().topLeft(); + grabbedImage = grabbedImage.copy(QRect(pixelOffset * screenDpr, window.geometry().size() * screenDpr)); + } + } else if (grabWindow) { + // if we grab the section, compare with the corresponding section from the painted image + const QRect sectionRect = QRect(grabRect.topLeft() * screenDpr, + grabRect.size() * screenDpr); + paintedImage = paintedImage.copy(sectionRect); + } + QCOMPARE(grabbedImage.size(), paintedImage.size()); + + // the two images might differ in format, or DPR, so instead of comparing them, sample a few pixels + for (auto point : { + QPoint(0, 0), + QPoint(5, 15), + QPoint(paintedImage.width() - 1, paintedImage.height() - 1), + QPoint(paintedImage.width() - 5, paintedImage.height() - 10) + }) { + QCOMPARE(grabbedImage.pixelColor(point), paintedImage.pixelColor(point)); + } +} + #include QTEST_MAIN(tst_QScreen); -- cgit v1.2.3