client: Use separate queue for SHM buffers

With the current setup, clients can try and submit buffers without
waiting for frame callbacks. This can lead to buffers being exhausted.
When this happens we go into a blocking path waiting for buffers to be
freed. The current implementation dispatches all events on the main
queue, which means we can be processing input events in the middle of
our drawing code, leading to obscure crashes.

This code uses a separate queue for shm buffers. This queue is processed
on the main thread manually before trying to re-use an old buffer this
queue is dispatched. If we need to block, we poll, but only dispatch the
one queue.

In addition explicitly dispatching solves a separate race condition.
Frame callbacks are always handled in the frame event thread. Depending
on how threads are woken this can be processed before main event thread
notifies of new events. This can lead the Qt main thread to be
processing drawing before we process the buffer release and consider the
buffer still busy. The end result is clients using one more buffer than
necessary. The explicit dispatching of shm events solves that problem.

Co-authored-by: Vlad Zahorodnii<vlad.zahorodnii@kde.org>
Pick-to: 6.9
Task-number: QTBUG-134234
Change-Id: Id8a84c72f669f187e368fd5ef32e8e4c150f8849
Reviewed-by: David Edmundson <davidedmundson@kde.org>
This commit is contained in:
David Edmundson 2025-04-16 14:03:37 +01:00
parent d8f07f43e5
commit a0161a2b78
4 changed files with 23 additions and 20 deletions

View File

@ -556,18 +556,6 @@ void QWaylandDisplay::checkWaylandError()
_exit(-1);
}
void QWaylandDisplay::blockingReadEvents()
{
if (wl_display_dispatch(mDisplay) < 0) {
int ecode = wl_display_get_error(mDisplay);
if ((ecode == EPIPE || ecode == ECONNRESET))
qWarning("The Wayland connection broke during blocking read event. Did the Wayland compositor die?");
else
qWarning("The Wayland connection experienced a fatal error during blocking read event: %s", strerror(ecode));
_exit(-1);
}
}
void QWaylandDisplay::checkTextInputProtocol()
{
QStringList tips, timps; // for text input protocols and text input manager protocols

View File

@ -273,7 +273,6 @@ public:
void initEventThread();
public Q_SLOTS:
void blockingReadEvents();
void flushRequests();
Q_SIGNALS:

View File

@ -50,7 +50,7 @@ extern void qt_scrollRectInImage(QImage &, const QRect &, const QPoint &);
namespace QtWaylandClient {
QWaylandShmBuffer::QWaylandShmBuffer(QWaylandDisplay *display,
const QSize &size, QImage::Format format, qreal scale)
const QSize &size, QImage::Format format, qreal scale, wl_event_queue *customEventQueue)
: mDirtyRegion(QRect(QPoint(0, 0), size / scale))
{
int stride = size.width() * 4;
@ -101,6 +101,8 @@ QWaylandShmBuffer::QWaylandShmBuffer(QWaylandDisplay *display,
mShmPool = wl_shm_create_pool(shm->object(), fd, alloc);
init(wl_shm_pool_create_buffer(mShmPool,0, size.width(), size.height(),
stride, wl_format));
if (customEventQueue)
wl_proxy_set_queue(reinterpret_cast<struct wl_proxy *>(buffer()), customEventQueue);
}
QWaylandShmBuffer::~QWaylandShmBuffer(void)
@ -144,7 +146,10 @@ QWaylandShmBackingStore::QWaylandShmBackingStore(QWindow *window, QWaylandDispla
: QPlatformBackingStore(window)
, mDisplay(display)
{
mEventQueue = wl_display_create_queue(mDisplay->wl_display());
QObject::connect(mDisplay, &QWaylandDisplay::connected, window, [this]() {
auto oldEventQueue = mEventQueue;
mEventQueue = wl_display_create_queue(mDisplay->wl_display());
auto copy = mBuffers;
// clear available buffers so we create new ones
// actual deletion is deferred till after resize call so we can copy
@ -155,6 +160,7 @@ QWaylandShmBackingStore::QWaylandShmBackingStore(QWindow *window, QWaylandDispla
if (mRequestedSize.isValid() && waylandWindow())
recreateBackBufferIfNeeded();
qDeleteAll(copy);
wl_event_queue_destroy(oldEventQueue);
});
}
@ -167,6 +173,7 @@ QWaylandShmBackingStore::~QWaylandShmBackingStore()
// waylandWindow()->attach(0);
qDeleteAll(mBuffers);
wl_event_queue_destroy(mEventQueue);
}
QPaintDevice *QWaylandShmBackingStore::paintDevice()
@ -257,7 +264,7 @@ void QWaylandShmBackingStore::flush(QWindow *window, const QRegion &region, cons
// however so no need to reimplement that.
if (window != this->window()) {
auto waylandWindow = static_cast<QWaylandWindow *>(window->handle());
auto newBuffer = new QWaylandShmBuffer(mDisplay, window->size(), mBackBuffer->image()->format(), mBackBuffer->scale());
auto newBuffer = new QWaylandShmBuffer(mDisplay, window->size(), mBackBuffer->image()->format(), mBackBuffer->scale(), mEventQueue);
newBuffer->setDeleteOnRelease(true);
QRect sourceRect(window->position(), window->size());
QPainter painter(newBuffer->image());
@ -322,7 +329,7 @@ QWaylandShmBuffer *QWaylandShmBackingStore::getBuffer(const QSize &size, bool &b
QImage::Format format = QImage::Format_ARGB32_Premultiplied;
if (!waylandWindow()->format().hasAlpha())
format = QImage::Format_RGB32;
QWaylandShmBuffer *b = new QWaylandShmBuffer(mDisplay, size, format, waylandWindow()->scale());
QWaylandShmBuffer *b = new QWaylandShmBuffer(mDisplay, size, format, waylandWindow()->scale(), mEventQueue);
bufferWasRecreated = true;
mBuffers.push_front(b);
return b;
@ -332,6 +339,8 @@ QWaylandShmBuffer *QWaylandShmBackingStore::getBuffer(const QSize &size, bool &b
bool QWaylandShmBackingStore::recreateBackBufferIfNeeded()
{
wl_display_dispatch_queue_pending(mDisplay->wl_display(), mEventQueue);
bool bufferWasRecreated = false;
QMargins margins = windowDecorationMargins();
qreal scale = waylandWindow()->scale();
@ -347,9 +356,15 @@ bool QWaylandShmBackingStore::recreateBackBufferIfNeeded()
// run single buffered, while with the pixman renderer we have to use two.
QWaylandShmBuffer *buffer = getBuffer(sizeWithMargins, bufferWasRecreated);
while (!buffer) {
qCDebug(lcWaylandBackingstore, "QWaylandShmBackingStore: stalling waiting for a buffer to be released from the compositor...");
mDisplay->blockingReadEvents();
struct ::wl_display *display = mDisplay->wl_display();
if (wl_display_dispatch_queue(display, mEventQueue) < 0) {
int ecode = wl_display_get_error(display);
if ((ecode == EPIPE || ecode == ECONNRESET))
qWarning("The Wayland connection broke during blocking read event. Did the Wayland compositor die?");
else
qWarning("The Wayland connection experienced a fatal error during blocking read event: %s", strerror(ecode));
_exit(-1);
}
buffer = getBuffer(sizeWithMargins, bufferWasRecreated);
}

View File

@ -33,7 +33,7 @@ class QWaylandWindow;
class Q_WAYLANDCLIENT_EXPORT QWaylandShmBuffer : public QWaylandBuffer {
public:
QWaylandShmBuffer(QWaylandDisplay *display,
const QSize &size, QImage::Format format, qreal scale = 1);
const QSize &size, QImage::Format format, qreal scale = 1, wl_event_queue *customEventQueue = nullptr);
~QWaylandShmBuffer() override;
QSize size() const override { return mImage.size(); }
int scale() const override { return int(mImage.devicePixelRatio()); }
@ -98,6 +98,7 @@ private:
QSize mRequestedSize;
Qt::WindowFlags mCurrentWindowFlags;
struct wl_event_queue *mEventQueue = nullptr;
};
}