From 99b222340672891d15e361298d7e4722a5ad0883 Mon Sep 17 00:00:00 2001 From: Louai Al-Khanji Date: Wed, 2 Mar 2016 09:54:07 -0800 Subject: xcb: Be smarter about how we flush For the remote X case the backing store previously always reuploaded image data for every expose event. Instead of doing that create a remote X pixmap and only flush repainted regions. For regular expose just copy from the pixmap. Additionally, atomically update the window by setting a clip mask and flushing the entire region at once instead of doing it rect by rect. Change-Id: I26bb1834b159e309c7ad93287dd297769f7e2633 Reviewed-by: Lars Knoll --- src/plugins/platforms/xcb/qxcbbackingstore.cpp | 204 ++++++++++++++++++------- 1 file changed, 145 insertions(+), 59 deletions(-) (limited to 'src/plugins/platforms/xcb/qxcbbackingstore.cpp') diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp index e57b089878..c4ee90e59f 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -74,13 +74,17 @@ public: QSize size() const { return m_qimage.size(); } bool hasAlpha() const { return m_hasAlpha; } + bool hasShm() const { return m_shm_info.shmaddr != nullptr; } - void put(xcb_window_t window, const QPoint &dst, const QRect &source); + void put(xcb_window_t window, const QRegion ®ion, const QPoint &offset); void preparePaint(const QRegion ®ion); private: void destroy(); + void flushPixmap(const QRegion ®ion); + void setClip(const QRegion ®ion); + xcb_shm_segment_info_t m_shm_info; xcb_image_t *m_xcb_image; @@ -91,7 +95,15 @@ private: xcb_gcontext_t m_gc; xcb_window_t m_gc_window; - QRegion m_dirty; + // When using shared memory this is the region currently shared with the server + QRegion m_dirtyShm; + + // When not using shared memory, we maintain a server-side pixmap with the backing + // store as well as repainted content not yet flushed to the pixmap. We only flush + // the regions we need and only when these are marked dirty. This way we can just + // do a server-side copy on expose instead of sending the pixels every time + xcb_pixmap_t m_xcb_pixmap; + QRegion m_pendingFlush; bool m_hasAlpha; }; @@ -131,6 +143,7 @@ QXcbShmImage::QXcbShmImage(QXcbScreen *screen, const QSize &size, uint depth, QI , m_graphics_buffer(Q_NULLPTR) , m_gc(0) , m_gc_window(0) + , m_xcb_pixmap(0) { Q_XCB_NOOP(connection()); @@ -189,6 +202,15 @@ QXcbShmImage::QXcbShmImage(QXcbScreen *screen, const QSize &size, uint depth, QI m_qimage = QImage( (uchar*) m_xcb_image->data, m_xcb_image->width, m_xcb_image->height, m_xcb_image->stride, format); m_graphics_buffer = new QXcbShmGraphicsBuffer(&m_qimage); + + if (!hasShm()) { + m_xcb_pixmap = xcb_generate_id(xcb_connection()); + Q_XCB_CALL(xcb_create_pixmap(xcb_connection(), + m_xcb_image->depth, + m_xcb_pixmap, + screen->screen()->root, + m_xcb_image->width, m_xcb_image->height)); + } } void QXcbShmImage::destroy() @@ -212,95 +234,163 @@ void QXcbShmImage::destroy() Q_XCB_CALL(xcb_free_gc(xcb_connection(), m_gc)); delete m_graphics_buffer; m_graphics_buffer = Q_NULLPTR; -} -void QXcbShmImage::put(xcb_window_t window, const QPoint &target, const QRect &source) -{ - Q_XCB_NOOP(connection()); - if (m_gc_window != window) { - if (m_gc) - Q_XCB_CALL(xcb_free_gc(xcb_connection(), m_gc)); - - m_gc = xcb_generate_id(xcb_connection()); - Q_XCB_CALL(xcb_create_gc(xcb_connection(), m_gc, window, 0, 0)); - - m_gc_window = window; + if (m_xcb_pixmap) { + Q_XCB_CALL(xcb_free_pixmap(xcb_connection(), m_xcb_pixmap)); + m_xcb_pixmap = 0; } +} - Q_XCB_NOOP(connection()); - if (m_shm_info.shmaddr) { - xcb_image_shm_put(xcb_connection(), - window, - m_gc, - m_xcb_image, - m_shm_info, - source.x(), - source.y(), - target.x(), - target.y(), - source.width(), - source.height(), - false); - } else { - // If we upload the whole image in a single chunk, the result might be - // larger than the server's maximum request size and stuff breaks. - // To work around that, we upload the image in chunks where each chunk - // is small enough for a single request. - int src_x = source.x(); - int src_y = source.y(); - int target_x = target.x(); - int target_y = target.y(); - int width = source.width(); - int height = source.height(); +void QXcbShmImage::flushPixmap(const QRegion ®ion) +{ + const QVector rects = m_pendingFlush.intersected(region).rects(); + m_pendingFlush -= region; + for (const QRect &rect : rects) { // We must make sure that each request is not larger than max_req_size. // Each request takes req_size + m_xcb_image->stride * height bytes. - uint32_t max_req_size = xcb_get_maximum_request_length(xcb_connection()); - uint32_t req_size = sizeof(xcb_put_image_request_t); - int rows_per_put = (max_req_size - req_size) / m_xcb_image->stride; + static const uint32_t req_size = sizeof(xcb_put_image_request_t); + const uint32_t max_req_size = xcb_get_maximum_request_length(xcb_connection()); + const int rows_per_put = (max_req_size - req_size) / m_xcb_image->stride; // This assert could trigger if a single row has more pixels than fit in // a single PutImage request. However, max_req_size is guaranteed to be // at least 16384 bytes. That should be enough for quite large images. Q_ASSERT(rows_per_put > 0); - // Convert the image to the native byte order. - xcb_image_t *converted_image = xcb_image_native(xcb_connection(), m_xcb_image, 1); + // If we upload the whole image in a single chunk, the result might be + // larger than the server's maximum request size and stuff breaks. + // To work around that, we upload the image in chunks where each chunk + // is small enough for a single request. + int src_x = rect.x(); + int src_y = rect.y(); + int target_x = rect.x(); + int target_y = rect.y(); + int width = rect.width(); + int height = rect.height(); while (height > 0) { int rows = std::min(height, rows_per_put); - xcb_image_t *subimage = xcb_image_subimage(converted_image, src_x, src_y, width, rows, + xcb_image_t *subimage = xcb_image_subimage(m_xcb_image, src_x, src_y, width, rows, 0, 0, 0); + + // Convert the image to the native byte order. + xcb_image_t *native_subimage = xcb_image_native(xcb_connection(), subimage, 1); + xcb_image_put(xcb_connection(), - window, + m_xcb_pixmap, m_gc, - subimage, + native_subimage, target_x, target_y, 0); + if (native_subimage != subimage) + xcb_image_destroy(native_subimage); + xcb_image_destroy(subimage); src_y += rows; target_y += rows; height -= rows; } + } +} + +void QXcbShmImage::setClip(const QRegion ®ion) +{ + if (region.isEmpty()) { + static const uint32_t mask = XCB_GC_CLIP_MASK; + static const uint32_t values[] = { XCB_NONE }; + Q_XCB_CALL(xcb_change_gc(xcb_connection(), + m_gc, + mask, + values)); + } else { + const QVector qrects = region.rects(); + QVector xcb_rects(qrects.size()); + + for (int i = 0; i < qrects.size(); i++) { + xcb_rects[i].x = qrects[i].x(); + xcb_rects[i].y = qrects[i].y(); + xcb_rects[i].width = qrects[i].width(); + xcb_rects[i].height = qrects[i].height(); + } - if (converted_image != m_xcb_image) - xcb_image_destroy(converted_image); + Q_XCB_CALL(xcb_set_clip_rectangles(xcb_connection(), + XCB_CLIP_ORDERING_YX_BANDED, + m_gc, + 0, 0, + xcb_rects.size(), xcb_rects.constData())); } +} + +void QXcbShmImage::put(xcb_window_t window, const QRegion ®ion, const QPoint &offset) +{ Q_XCB_NOOP(connection()); - m_dirty = m_dirty | source; + if (m_gc_window != window) { + if (m_gc) + Q_XCB_CALL(xcb_free_gc(xcb_connection(), m_gc)); + + static const uint32_t mask = XCB_GC_GRAPHICS_EXPOSURES; + static const uint32_t values[] = { 0 }; + + m_gc = xcb_generate_id(xcb_connection()); + Q_XCB_CALL(xcb_create_gc(xcb_connection(), m_gc, window, mask, values)); + + m_gc_window = window; + } + + Q_XCB_NOOP(connection()); + + setClip(region); + + const QRect bounds = region.boundingRect(); + const QPoint target = bounds.topLeft(); + const QRect source = bounds.translated(offset); + + if (hasShm()) { + Q_XCB_CALL(xcb_shm_put_image(xcb_connection(), + window, + m_gc, + m_xcb_image->width, + m_xcb_image->height, + source.x(), source.y(), + source.width(), source.height(), + target.x(), target.y(), + m_xcb_image->depth, + m_xcb_image->format, + 0, // send event? + m_shm_info.shmseg, + m_xcb_image->data - m_shm_info.shmaddr)); + m_dirtyShm |= region.translated(offset); + } else { + flushPixmap(region); + Q_XCB_CALL(xcb_copy_area(xcb_connection(), + m_xcb_pixmap, + window, + m_gc, + source.x(), source.y(), + target.x(), target.y(), + source.width(), source.height())); + } + + setClip(QRegion()); + Q_XCB_NOOP(connection()); } void QXcbShmImage::preparePaint(const QRegion ®ion) { - // to prevent X from reading from the image region while we're writing to it - if (m_dirty.intersects(region)) { - connection()->sync(); - m_dirty = QRegion(); + if (hasShm()) { + // to prevent X from reading from the image region while we're writing to it + if (m_dirtyShm.intersects(region)) { + connection()->sync(); + m_dirtyShm = QRegion(); + } + } else { + m_pendingFlush |= region; } } @@ -397,11 +487,7 @@ void QXcbBackingStore::flush(QWindow *window, const QRegion ®ion, const QPoin return; } - QVector rects = clipped.rects(); - for (int i = 0; i < rects.size(); ++i) { - QRect rect = QRect(rects.at(i).topLeft(), rects.at(i).size()); - m_image->put(platformWindow->xcb_window(), rect.topLeft(), rect.translated(offset)); - } + m_image->put(platformWindow->xcb_window(), clipped, offset); Q_XCB_NOOP(connection()); -- cgit v1.2.3 From 7b46cad5c944bf0b3fdde89e3cd736e0756569c6 Mon Sep 17 00:00:00 2001 From: Louai Al-Khanji Date: Wed, 2 Mar 2016 13:12:10 -0800 Subject: QXcbBackingStore: Minor code cleanup Change-Id: I5086e2031201b939b49603f17c373e414a91c32a Reviewed-by: Lars Knoll Reviewed-by: Laszlo Agocs --- src/plugins/platforms/xcb/qxcbbackingstore.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'src/plugins/platforms/xcb/qxcbbackingstore.cpp') diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp index c4ee90e59f..2e04799998 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -147,14 +147,8 @@ QXcbShmImage::QXcbShmImage(QXcbScreen *screen, const QSize &size, uint depth, QI { Q_XCB_NOOP(connection()); - const xcb_setup_t *setup = xcb_get_setup(xcb_connection()); - xcb_format_t *fmt = xcb_setup_pixmap_formats(setup); - xcb_format_t *fmtend = fmt + xcb_setup_pixmap_formats_length(setup); - for (; fmt != fmtend; ++fmt) - if (fmt->depth == depth) - break; - - Q_ASSERT(fmt != fmtend); + const xcb_format_t *fmt = connection()->formatForDepth(depth); + Q_ASSERT(fmt); m_xcb_image = xcb_image_create(size.width(), size.height(), XCB_IMAGE_FORMAT_Z_PIXMAP, -- cgit v1.2.3 From 6417bbde8565e0be2d049426b71e6fda538e4440 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sat, 17 Oct 2015 17:48:34 +0200 Subject: QtBase (remainder): use printf-style qWarning/qDebug where possible (I) The printf-style version of QDebug expands to a lot less code than the std::ostream-style version. Of course, you pay in type safety (but compilers warn about it these days), you cannot stream complex Qt types and streaming QStrings is awkward, but in many cases you actually improve on readability. But the main reason is that something that's not supposed to be executed under normal operation has no business bloating executable code size. This is not an attempt at converting all qWarnings() to printf-style, only the low-hanging fruit. In this first part, replace qWarning() << "" with qWarning("..."). Had to fix broken qImDebug() definition. Instead of defining it as a nullary macro in the QT_NO_DEBUG case and as a variadic macro in the other, define it in both cases, as is customary, as a non-function macro so that overload selection works without requiring variadic macro support of the compiler. Saves e.g. ~250b in text size in QtPrintSupport on optimized GCC 5.3 AMD64 builds. Change-Id: Ie30fe2f7942115d5dbf99fff1750ae0d477c379f Reviewed-by: Kai Koehne --- src/plugins/platforms/xcb/qxcbbackingstore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/plugins/platforms/xcb/qxcbbackingstore.cpp') diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp index 2e04799998..896eb61970 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -187,7 +187,7 @@ QXcbShmImage::QXcbShmImage(QXcbScreen *screen, const QSize &size, uint depth, QI m_xcb_image->data = (uint8_t *)malloc(segmentSize); } else { if (shmctl(m_shm_info.shmid, IPC_RMID, 0) == -1) - qWarning() << "QXcbBackingStore: Error while marking the shared memory segment to be destroyed"; + qWarning("QXcbBackingStore: Error while marking the shared memory segment to be destroyed"); } m_hasAlpha = QImage::toPixelFormat(format).alphaUsage() == QPixelFormat::UsesAlpha; -- cgit v1.2.3