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 760b2929a3, 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>
bb10
Gatis Paeglis 2019-01-16 15:12:16 +08:00 committed by Liang Qi
parent edc455c69b
commit c0ebec51e3
6 changed files with 39 additions and 26 deletions

View File

@ -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());

View File

@ -240,8 +240,8 @@ QXcbClipboard::QXcbClipboard(QXcbConnection *c)
xcb_xfixes_select_selection_input_checked(xcb_connection(), m_owner, atom(QXcbAtom::CLIPBOARD), mask);
}
// change property protocol request is 24 bytes
m_increment = (xcb_get_maximum_request_length(xcb_connection()) * 4) - 24;
// xcb_change_property_request_t and xcb_get_property_request_t are the same size
m_maxPropertyRequestDataBytes = connection()->maxRequestDataBytes(sizeof(xcb_change_property_request_t));
}
QXcbClipboard::~QXcbClipboard()
@ -486,7 +486,7 @@ xcb_atom_t QXcbClipboard::sendSelection(QMimeData *d, xcb_atom_t target, xcb_win
if (m_clipboard_closing)
allow_incr = false;
if (data.size() > m_increment && allow_incr) {
if (data.size() > m_maxPropertyRequestDataBytes && allow_incr) {
long bytes = data.size();
xcb_change_property(xcb_connection(), XCB_PROP_MODE_REPLACE, window, property,
atom(QXcbAtom::INCR), 32, 1, (const void *)&bytes);
@ -496,7 +496,7 @@ xcb_atom_t QXcbClipboard::sendSelection(QMimeData *d, xcb_atom_t target, xcb_win
}
// make sure we can perform the XChangeProperty in a single request
if (data.size() > m_increment)
if (data.size() > m_maxPropertyRequestDataBytes)
return XCB_NONE; // ### perhaps use several XChangeProperty calls w/ PropModeAppend?
int dataSize = data.size() / (dataFormat / 8);
// use a single request to transfer data
@ -678,17 +678,8 @@ void QXcbClipboard::handleXFixesSelectionRequest(xcb_xfixes_selection_notify_eve
emitChanged(mode);
}
static inline int maxSelectionIncr(xcb_connection_t *c)
{
int l = xcb_get_maximum_request_length(c);
return (l > 65536 ? 65536*4 : l*4) - 100;
}
bool QXcbClipboard::clipboardReadProperty(xcb_window_t win, xcb_atom_t property, bool deleteProperty, QByteArray *buffer, int *size, xcb_atom_t *type, int *format)
{
int maxsize = maxSelectionIncr(xcb_connection());
ulong bytes_left; // bytes_after
xcb_atom_t dummy_type;
int dummy_format;
@ -705,7 +696,8 @@ bool QXcbClipboard::clipboardReadProperty(xcb_window_t win, xcb_atom_t property,
}
*type = reply->type;
*format = reply->format;
bytes_left = reply->bytes_after;
auto bytes_left = reply->bytes_after;
int offset = 0, buffer_offset = 0;
@ -720,7 +712,8 @@ bool QXcbClipboard::clipboardReadProperty(xcb_window_t win, xcb_atom_t property,
while (bytes_left) {
// more to read...
reply = Q_XCB_REPLY(xcb_get_property, xcb_connection(), false, win, property, XCB_GET_PROPERTY_TYPE_ANY, offset, maxsize/4);
reply = Q_XCB_REPLY(xcb_get_property, xcb_connection(), false, win, property,
XCB_GET_PROPERTY_TYPE_ANY, offset, m_maxPropertyRequestDataBytes / 4);
if (!reply || reply->type == XCB_NONE)
break;

View File

@ -113,7 +113,7 @@ public:
xcb_window_t getSelectionOwner(xcb_atom_t atom) const;
QByteArray getSelection(xcb_atom_t selection, xcb_atom_t target, xcb_atom_t property, xcb_timestamp_t t = 0);
int increment() const { return m_increment; }
int increment() const { return m_maxPropertyRequestDataBytes; }
int clipboardTimeout() const { return clipboard_timeout; }
void removeTransaction(xcb_window_t window) { m_transactions.remove(window); }
@ -137,7 +137,7 @@ private:
static const int clipboard_timeout;
int m_increment = 0;
int m_maxPropertyRequestDataBytes = 0;
bool m_clipboard_closing = false;
xcb_timestamp_t m_incr_receive_time = 0;

View File

@ -245,7 +245,7 @@ public:
void flush() { xcb_flush(xcb_connection()); }
void processXcbEvents(QEventLoop::ProcessEventsFlags flags);
QTimer &focusInTimer() { return m_focusInTimer; };
QTimer &focusInTimer() { return m_focusInTimer; }
protected:
bool event(QEvent *e) override;

View File

@ -134,6 +134,7 @@ QXcbBasicConnection::QXcbBasicConnection(const char *displayName)
m_setup = xcb_get_setup(m_xcbConnection);
m_xcbAtom.initialize(m_xcbConnection);
m_maximumRequestLength = xcb_get_maximum_request_length(m_xcbConnection);
xcb_extension_t *extensions[] = {
&xcb_shm_id, &xcb_xfixes_id, &xcb_randr_id, &xcb_shape_id, &xcb_sync_id,
@ -178,6 +179,14 @@ QXcbBasicConnection::~QXcbBasicConnection()
}
}
size_t QXcbBasicConnection::maxRequestDataBytes(size_t requestSize) const
{
if (hasBigRequest())
requestSize += 4; // big-request encoding adds 4 bytes
return m_maximumRequestLength * 4 - requestSize;
}
xcb_atom_t QXcbBasicConnection::internAtom(const char *name)
{
if (!name || *name == 0)
@ -199,6 +208,11 @@ QByteArray QXcbBasicConnection::atomName(xcb_atom_t atom)
return QByteArray();
}
bool QXcbBasicConnection::hasBigRequest() const
{
return m_maximumRequestLength > m_setup->maximum_request_length;
}
#if QT_CONFIG(xcb_xinput)
// Starting from the xcb version 1.9.3 struct xcb_ge_event_t has changed:
// - "pad0" became "extension"

View File

@ -73,6 +73,8 @@ public:
}
const xcb_setup_t *setup() const { return m_setup; }
size_t maxRequestDataBytes(size_t requestSize) const;
inline xcb_atom_t atom(QXcbAtom::Atom qatom) const { return m_xcbAtom.atom(qatom); }
QXcbAtom::Atom qatom(xcb_atom_t atom) const { return m_xcbAtom.qatom(atom); }
xcb_atom_t internAtom(const char *name);
@ -94,6 +96,7 @@ public:
bool hasShmFd() const { return m_hasShmFd; }
bool hasXSync() const { return m_hasXSync; }
bool hasXinerama() const { return m_hasXinerama; }
bool hasBigRequest() const;
#if QT_CONFIG(xcb_xinput)
bool isAtLeastXI21() const { return m_xi2Enabled && m_xi2Minor >= 1; }
@ -152,6 +155,8 @@ private:
uint32_t m_xfixesFirstEvent = 0;
uint32_t m_xrandrFirstEvent = 0;
uint32_t m_xkbFirstEvent = 0;
uint32_t m_maximumRequestLength = 0;
};
#define Q_XCB_REPLY_CONNECTION_ARG(connection, ...) connection