xcb: respect big-request encoding in max request size

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 <zccrs@live.com>
Done-with: Mikhail Svetkin <mikhail.svetkin@qt.io>
Change-Id: I06beb6082da3e8bc78225a87603914e796fe5878
Reviewed-by: Błażej Szczygieł <spaz16@wp.pl>
Reviewed-by: JiDe Zhang <zccrs@live.com>
Reviewed-by: Mikhail Svetkin <mikhail.svetkin@qt.io>
Reviewed-by: Uli Schlachter <psychon@znc.in>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

1 files changed, 9 insertions, 8 deletions

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 &region, 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 &region, 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<size_t>(subImage.sizeInBytes()) <= maxPutImageRequestDataBytes);
+
             xcb_subimage.width = width;
             xcb_subimage.height = rows;
             xcb_subimage.data = const_cast<uint8_t *>(subImage.constBits());