From c0ebec51e3dd1b52767878fe7fd56ce6e8f95461 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Wed, 16 Jan 2019 15:12:16 +0800 Subject: xcb: respect big-request encoding in max request size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From big-request specification: "This extension defines a mechanism for extending the length field beyond 16 bits. If the normal 16-bit length field of the protocol request is zero, then an additional 32-bit field containing the actual length (in 4-byte units) is inserted into the request, immediately following the 16-bit length field." Meaning that the request requires 4 additional bytes. This patch provides a convenience API for calculating maximum request data size. Besides fixing QTBUG-73044, it was also discovered that calculations for xcb_image_put (in QXcbBackingStoreImage::flushPixmap) were wrong. The code assumed that xcb_get_maximum_request_length() returns bytes, but what it actually returns is length which is measured in four-byte units. This means that we were sending 4x less bytes than allowed by the protocol. Furthermore, use the actual 'stride' (bytes per line) value when calculating rows_per_put. The new stride value was introduced by 760b2929a3b268e2edf14a561329bdb78fbdc26e, but was not updated in rows_per_put calculations. Fixes: QTBUG-73044 Done-with: JiDe Zhang Done-with: Mikhail Svetkin Change-Id: I06beb6082da3e8bc78225a87603914e796fe5878 Reviewed-by: Błażej Szczygieł Reviewed-by: JiDe Zhang Reviewed-by: Mikhail Svetkin Reviewed-by: Uli Schlachter Reviewed-by: Allan Sandfeld Jensen --- src/plugins/platforms/xcb/qxcbbackingstore.cpp | 17 +++++++++-------- 1 file changed, 9 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 f9240a45cc..741317d766 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -641,17 +641,17 @@ void QXcbBackingStoreImage::flushPixmap(const QRegion ®ion, bool fullRegion) xcb_subimage.bit_order = m_xcb_image->bit_order; const bool needsByteSwap = xcb_subimage.byte_order != m_xcb_image->byte_order; + // Ensure that we don't send more than maxPutImageRequestDataBytes per request. + const auto maxPutImageRequestDataBytes = connection()->maxRequestDataBytes(sizeof(xcb_put_image_request_t)); for (const QRect &rect : region) { - // 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. - 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; + const quint32 stride = round_up_scanline(rect.width() * m_qimage.depth(), xcb_subimage.scanline_pad) >> 3; + const int rows_per_put = maxPutImageRequestDataBytes / 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. + // a single PutImage request. In the absence of the BIG-REQUESTS extension + // the theoretical maximum lengths of maxPutImageRequestDataBytes can be + // roughly 256kB. Q_ASSERT(rows_per_put > 0); // If we upload the whole image in a single chunk, the result might be @@ -666,9 +666,10 @@ void QXcbBackingStoreImage::flushPixmap(const QRegion ®ion, bool fullRegion) while (height > 0) { const int rows = std::min(height, rows_per_put); const QRect subRect(x, y, width, rows); - const quint32 stride = round_up_scanline(width * m_qimage.depth(), xcb_subimage.scanline_pad) >> 3; const QImage subImage = native_sub_image(&m_flushBuffer, stride, m_qimage, subRect, needsByteSwap); + Q_ASSERT(static_cast(subImage.sizeInBytes()) <= maxPutImageRequestDataBytes); + xcb_subimage.width = width; xcb_subimage.height = rows; xcb_subimage.data = const_cast(subImage.constBits()); -- cgit v1.2.3