From b0bdffe08fe2ca85290d159d020919a7c8479305 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Tue, 2 May 2023 18:18:42 +0300 Subject: [PATCH] backends/x11: Avoid rendering to buffers held by Xorg Otherwise it's possible to see visual artifacts. --- .../x11/windowed/x11_windowed_backend.cpp | 7 + .../x11/windowed/x11_windowed_egl_backend.cpp | 85 ++++------ .../x11/windowed/x11_windowed_egl_backend.h | 13 +- .../x11/windowed/x11_windowed_output.cpp | 148 +++++++++++++++++- .../x11/windowed/x11_windowed_output.h | 37 +++++ .../x11_windowed_qpainter_backend.cpp | 31 +--- .../windowed/x11_windowed_qpainter_backend.h | 5 - 7 files changed, 235 insertions(+), 91 deletions(-) diff --git a/src/backends/x11/windowed/x11_windowed_backend.cpp b/src/backends/x11/windowed/x11_windowed_backend.cpp index 674eb93048..08d03030fa 100644 --- a/src/backends/x11/windowed/x11_windowed_backend.cpp +++ b/src/backends/x11/windowed/x11_windowed_backend.cpp @@ -669,6 +669,13 @@ void X11WindowedBackend::handleXinputEvent(xcb_ge_generic_event_t *ge) void X11WindowedBackend::handlePresentEvent(xcb_ge_generic_event_t *ge) { switch (ge->event_type) { + case XCB_PRESENT_EVENT_IDLE_NOTIFY: { + xcb_present_idle_notify_event_t *idleNotify = reinterpret_cast(ge); + if (X11WindowedOutput *output = findOutput(idleNotify->window)) { + output->handlePresentIdleNotify(idleNotify); + } + break; + } case XCB_PRESENT_EVENT_COMPLETE_NOTIFY: { xcb_present_complete_notify_event_t *completeNotify = reinterpret_cast(ge); if (X11WindowedOutput *output = findOutput(completeNotify->window)) { diff --git a/src/backends/x11/windowed/x11_windowed_egl_backend.cpp b/src/backends/x11/windowed/x11_windowed_egl_backend.cpp index 624bff020b..9a7d16ee42 100644 --- a/src/backends/x11/windowed/x11_windowed_egl_backend.cpp +++ b/src/backends/x11/windowed/x11_windowed_egl_backend.cpp @@ -15,42 +15,14 @@ #include "x11_windowed_output.h" #include -#include namespace KWin { -X11WindowedEglLayerBuffer::X11WindowedEglLayerBuffer(GbmGraphicsBuffer *graphicsBuffer, uint32_t depth, uint32_t bpp, xcb_drawable_t drawable, X11WindowedEglBackend *backend) - : m_backend(backend) - , m_graphicsBuffer(graphicsBuffer) +X11WindowedEglLayerBuffer::X11WindowedEglLayerBuffer(GbmGraphicsBuffer *graphicsBuffer, X11WindowedEglBackend *backend) + : m_graphicsBuffer(graphicsBuffer) { - X11WindowedBackend *x11Backend = backend->backend(); - const DmaBufAttributes *attributes = graphicsBuffer->dmabufAttributes(); - - m_pixmap = xcb_generate_id(x11Backend->connection()); - if (x11Backend->driMajorVersion() >= 1 || x11Backend->driMinorVersion() >= 2) { - // xcb_dri3_pixmap_from_buffers() takes the ownership of the file descriptors. - int fds[4] = { - attributes->fd[0].duplicate().take(), - attributes->fd[1].duplicate().take(), - attributes->fd[2].duplicate().take(), - attributes->fd[3].duplicate().take(), - }; - xcb_dri3_pixmap_from_buffers(x11Backend->connection(), m_pixmap, drawable, attributes->planeCount, - attributes->width, attributes->height, - attributes->pitch[0], attributes->offset[0], - attributes->pitch[1], attributes->offset[1], - attributes->pitch[2], attributes->offset[2], - attributes->pitch[3], attributes->offset[3], - depth, bpp, attributes->modifier, fds); - } else { - // xcb_dri3_pixmap_from_buffer() takes the ownership of the file descriptor. - xcb_dri3_pixmap_from_buffer(x11Backend->connection(), m_pixmap, drawable, - attributes->height * attributes->pitch[0], attributes->width, attributes->height, - attributes->pitch[0], depth, bpp, attributes->fd[0].duplicate().take()); - } - - m_texture = backend->importDmaBufAsTexture(*attributes); + m_texture = backend->importDmaBufAsTexture(*graphicsBuffer->dmabufAttributes()); m_framebuffer = std::make_unique(m_texture.get()); } @@ -58,14 +30,12 @@ X11WindowedEglLayerBuffer::~X11WindowedEglLayerBuffer() { m_texture.reset(); m_framebuffer.reset(); - - xcb_free_pixmap(m_backend->backend()->connection(), m_pixmap); m_graphicsBuffer->drop(); } -xcb_pixmap_t X11WindowedEglLayerBuffer::pixmap() const +GraphicsBuffer *X11WindowedEglLayerBuffer::graphicsBuffer() const { - return m_pixmap; + return m_graphicsBuffer; } std::shared_ptr X11WindowedEglLayerBuffer::texture() const @@ -83,19 +53,13 @@ int X11WindowedEglLayerBuffer::age() const return m_age; } -X11WindowedEglLayerSwapchain::X11WindowedEglLayerSwapchain(const QSize &size, uint32_t format, uint32_t depth, uint32_t bpp, const QVector &modifiers, xcb_drawable_t drawable, X11WindowedEglBackend *backend) - : m_size(size) +X11WindowedEglLayerSwapchain::X11WindowedEglLayerSwapchain(const QSize &size, uint32_t format, const QVector &modifiers, X11WindowedEglBackend *backend) + : m_backend(backend) + , m_allocator(new GbmGraphicsBufferAllocator(backend->backend()->gbmDevice())) + , m_size(size) + , m_format(format) + , m_modifiers(modifiers) { - GbmGraphicsBufferAllocator allocator(backend->backend()->gbmDevice()); - - for (int i = 0; i < 2; ++i) { - GbmGraphicsBuffer *graphicsBuffer = allocator.allocate(size, format, modifiers); - if (!graphicsBuffer) { - qCCritical(KWIN_X11WINDOWED) << "Failed to allocate a buffer for an output layer"; - continue; - } - m_buffers.append(std::make_shared(graphicsBuffer, depth, bpp, drawable, backend)); - } } X11WindowedEglLayerSwapchain::~X11WindowedEglLayerSwapchain() @@ -109,14 +73,26 @@ QSize X11WindowedEglLayerSwapchain::size() const std::shared_ptr X11WindowedEglLayerSwapchain::acquire() { - m_index = (m_index + 1) % m_buffers.count(); - return m_buffers[m_index]; + for (const auto &buffer : std::as_const(m_buffers)) { + if (!buffer->graphicsBuffer()->isReferenced()) { + return buffer; + } + } + + GbmGraphicsBuffer *graphicsBuffer = m_allocator->allocate(m_size, m_format, m_modifiers); + if (!graphicsBuffer) { + qCWarning(KWIN_X11WINDOWED) << "Failed to allocate layer swapchain buffer"; + return nullptr; + } + + auto buffer = std::make_shared(graphicsBuffer, m_backend); + m_buffers.append(buffer); + + return buffer; } void X11WindowedEglLayerSwapchain::release(std::shared_ptr buffer) { - Q_ASSERT(m_buffers[m_index] == buffer); - for (qsizetype i = 0; i < m_buffers.count(); ++i) { if (m_buffers[i] == buffer) { m_buffers[i]->m_age = 1; @@ -143,7 +119,7 @@ std::optional X11WindowedEglPrimaryLayer::beginFrame( if (!formatTable.contains(format)) { return std::nullopt; } - m_swapchain = std::make_unique(bufferSize, format, 24, 32, formatTable[format], m_output->window(), m_backend); + m_swapchain = std::make_unique(bufferSize, format, formatTable[format], m_backend); } m_buffer = m_swapchain->acquire(); @@ -168,6 +144,9 @@ bool X11WindowedEglPrimaryLayer::endFrame(const QRegion &renderedRegion, const Q void X11WindowedEglPrimaryLayer::present() { + xcb_pixmap_t pixmap = m_output->importBuffer(m_buffer->graphicsBuffer()); + Q_ASSERT(pixmap != XCB_PIXMAP_NONE); + xcb_xfixes_region_t valid = 0; xcb_xfixes_region_t update = 0; uint32_t serial = 0; @@ -176,7 +155,7 @@ void X11WindowedEglPrimaryLayer::present() xcb_present_pixmap(m_output->backend()->connection(), m_output->window(), - m_buffer->pixmap(), + pixmap, serial, valid, update, diff --git a/src/backends/x11/windowed/x11_windowed_egl_backend.h b/src/backends/x11/windowed/x11_windowed_egl_backend.h index 015fb4add8..4fa1e27608 100644 --- a/src/backends/x11/windowed/x11_windowed_egl_backend.h +++ b/src/backends/x11/windowed/x11_windowed_egl_backend.h @@ -24,17 +24,15 @@ class X11WindowedEglBackend; class X11WindowedEglLayerBuffer { public: - X11WindowedEglLayerBuffer(GbmGraphicsBuffer *graphicsBuffers, uint32_t depth, uint32_t bpp, xcb_drawable_t drawable, X11WindowedEglBackend *backend); + X11WindowedEglLayerBuffer(GbmGraphicsBuffer *graphicsBuffers, X11WindowedEglBackend *backend); ~X11WindowedEglLayerBuffer(); - xcb_pixmap_t pixmap() const; + GraphicsBuffer *graphicsBuffer() const; std::shared_ptr texture() const; GLFramebuffer *framebuffer() const; int age() const; private: - X11WindowedEglBackend *m_backend; - xcb_pixmap_t m_pixmap = XCB_PIXMAP_NONE; GbmGraphicsBuffer *m_graphicsBuffer; std::unique_ptr m_framebuffer; std::shared_ptr m_texture; @@ -45,7 +43,7 @@ private: class X11WindowedEglLayerSwapchain { public: - X11WindowedEglLayerSwapchain(const QSize &size, uint32_t format, uint32_t depth, uint32_t bpp, const QVector &modifiers, xcb_drawable_t drawable, X11WindowedEglBackend *backend); + X11WindowedEglLayerSwapchain(const QSize &size, uint32_t format, const QVector &modifiers, X11WindowedEglBackend *backend); ~X11WindowedEglLayerSwapchain(); QSize size() const; @@ -54,9 +52,12 @@ public: void release(std::shared_ptr buffer); private: + X11WindowedEglBackend *m_backend; + std::unique_ptr m_allocator; QSize m_size; + uint32_t m_format; + QVector m_modifiers; QVector> m_buffers; - int m_index = 0; }; class X11WindowedEglPrimaryLayer : public OutputLayer diff --git a/src/backends/x11/windowed/x11_windowed_output.cpp b/src/backends/x11/windowed/x11_windowed_output.cpp index b0c493fb1b..9a6991261b 100644 --- a/src/backends/x11/windowed/x11_windowed_output.cpp +++ b/src/backends/x11/windowed/x11_windowed_output.cpp @@ -9,10 +9,12 @@ #include "x11_windowed_output.h" #include "../common/kwinxrenderutils.h" #include "x11_windowed_backend.h" +#include "x11_windowed_logging.h" #include #include "composite.h" +#include "core/graphicsbuffer.h" #include "core/renderbackend.h" #include "core/renderlayer.h" #include "core/renderloop_p.h" @@ -29,9 +31,54 @@ #include #include +#include +#include +#include + namespace KWin { +X11WindowedBuffer::X11WindowedBuffer(X11WindowedOutput *output, xcb_pixmap_t pixmap, GraphicsBuffer *graphicsBuffer) + : m_output(output) + , m_buffer(graphicsBuffer) + , m_pixmap(pixmap) +{ + connect(graphicsBuffer, &GraphicsBuffer::destroyed, this, &X11WindowedBuffer::defunct); +} + +X11WindowedBuffer::~X11WindowedBuffer() +{ + m_buffer->disconnect(this); + xcb_free_pixmap(m_output->backend()->connection(), m_pixmap); + unlock(); +} + +GraphicsBuffer *X11WindowedBuffer::buffer() const +{ + return m_buffer; +} + +xcb_pixmap_t X11WindowedBuffer::pixmap() const +{ + return m_pixmap; +} + +void X11WindowedBuffer::lock() +{ + if (!m_locked) { + m_locked = true; + m_buffer->ref(); + } +} + +void X11WindowedBuffer::unlock() +{ + if (m_locked) { + m_locked = false; + m_buffer->unref(); + } +} + X11WindowedCursor::X11WindowedCursor(X11WindowedOutput *output) : m_output(output) { @@ -105,6 +152,8 @@ X11WindowedOutput::X11WindowedOutput(X11WindowedBackend *backend) X11WindowedOutput::~X11WindowedOutput() { + m_buffers.clear(); + xcb_present_select_input(m_backend->connection(), m_presentEvent, m_window, 0); xcb_unmap_window(m_backend->connection(), m_window); xcb_destroy_window(m_backend->connection(), m_window); @@ -198,7 +247,7 @@ void X11WindowedOutput::init(const QSize &pixelSize, qreal scale) // select xinput 2 events initXInputForWindow(); - const uint32_t presentEventMask = XCB_PRESENT_EVENT_MASK_COMPLETE_NOTIFY; + const uint32_t presentEventMask = XCB_PRESENT_EVENT_MASK_IDLE_NOTIFY | XCB_PRESENT_EVENT_MASK_COMPLETE_NOTIFY; m_presentEvent = xcb_generate_id(m_backend->connection()); xcb_present_select_input(m_backend->connection(), m_presentEvent, m_window, presentEventMask); @@ -265,6 +314,16 @@ void X11WindowedOutput::handlePresentCompleteNotify(xcb_present_complete_notify_ RenderLoopPrivate::get(m_renderLoop.get())->notifyFrameCompleted(timestamp); } +void X11WindowedOutput::handlePresentIdleNotify(xcb_present_idle_notify_event_t *event) +{ + for (auto &[graphicsBuffer, x11Buffer] : m_buffers) { + if (x11Buffer->pixmap() == event->pixmap) { + x11Buffer->unlock(); + return; + } + } +} + void X11WindowedOutput::setWindowTitle(const QString &title) { m_winInfo->setName(title.toUtf8().constData()); @@ -327,4 +386,91 @@ void X11WindowedOutput::updateEnabled(bool enabled) setState(next); } +xcb_pixmap_t X11WindowedOutput::importDmaBufBuffer(const DmaBufAttributes *attributes) +{ + uint8_t depth; + uint8_t bpp; + switch (attributes->format) { + case DRM_FORMAT_ARGB8888: + depth = 32; + bpp = 32; + break; + case DRM_FORMAT_XRGB8888: + depth = 24; + bpp = 32; + break; + default: + qCWarning(KWIN_X11WINDOWED) << "Cannot import a buffer with unsupported format"; + return XCB_PIXMAP_NONE; + } + + xcb_pixmap_t pixmap = xcb_generate_id(m_backend->connection()); + if (m_backend->driMajorVersion() >= 1 || m_backend->driMinorVersion() >= 2) { + // xcb_dri3_pixmap_from_buffers() takes the ownership of the file descriptors. + int fds[4] = { + attributes->fd[0].duplicate().take(), + attributes->fd[1].duplicate().take(), + attributes->fd[2].duplicate().take(), + attributes->fd[3].duplicate().take(), + }; + xcb_dri3_pixmap_from_buffers(m_backend->connection(), pixmap, m_window, attributes->planeCount, + attributes->width, attributes->height, + attributes->pitch[0], attributes->offset[0], + attributes->pitch[1], attributes->offset[1], + attributes->pitch[2], attributes->offset[2], + attributes->pitch[3], attributes->offset[3], + depth, bpp, attributes->modifier, fds); + } else { + // xcb_dri3_pixmap_from_buffer() takes the ownership of the file descriptor. + xcb_dri3_pixmap_from_buffer(m_backend->connection(), pixmap, m_window, + attributes->height * attributes->pitch[0], attributes->width, attributes->height, + attributes->pitch[0], depth, bpp, attributes->fd[0].duplicate().take()); + } + + return pixmap; +} + +xcb_pixmap_t X11WindowedOutput::importShmBuffer(const ShmAttributes *attributes) +{ + // xcb_shm_attach_fd() takes the ownership of the passed shm file descriptor. + FileDescriptor poolFileDescriptor = attributes->fd.duplicate(); + if (!poolFileDescriptor.isValid()) { + qCWarning(KWIN_X11WINDOWED) << "Failed to duplicate shm file descriptor"; + return XCB_PIXMAP_NONE; + } + + xcb_shm_seg_t segment = xcb_generate_id(m_backend->connection()); + xcb_shm_attach_fd(m_backend->connection(), segment, poolFileDescriptor.take(), 0); + + xcb_pixmap_t pixmap = xcb_generate_id(m_backend->connection()); + xcb_shm_create_pixmap(m_backend->connection(), pixmap, m_window, attributes->size.width(), attributes->size.height(), depth(), segment, 0); + xcb_shm_detach(m_backend->connection(), segment); + + return pixmap; +} + +xcb_pixmap_t X11WindowedOutput::importBuffer(GraphicsBuffer *graphicsBuffer) +{ + std::unique_ptr &x11Buffer = m_buffers[graphicsBuffer]; + if (!x11Buffer) { + xcb_pixmap_t pixmap = XCB_PIXMAP_NONE; + if (const DmaBufAttributes *attributes = graphicsBuffer->dmabufAttributes()) { + pixmap = importDmaBufBuffer(attributes); + } else if (const ShmAttributes *attributes = graphicsBuffer->shmAttributes()) { + pixmap = importShmBuffer(attributes); + } + if (pixmap == XCB_PIXMAP_NONE) { + return XCB_PIXMAP_NONE; + } + + x11Buffer = std::make_unique(this, pixmap, graphicsBuffer); + connect(x11Buffer.get(), &X11WindowedBuffer::defunct, this, [this, graphicsBuffer]() { + m_buffers.erase(graphicsBuffer); + }); + } + + x11Buffer->lock(); + return x11Buffer->pixmap(); +} + } // namespace KWin diff --git a/src/backends/x11/windowed/x11_windowed_output.h b/src/backends/x11/windowed/x11_windowed_output.h index 263d66978a..bda4cd1357 100644 --- a/src/backends/x11/windowed/x11_windowed_output.h +++ b/src/backends/x11/windowed/x11_windowed_output.h @@ -15,6 +15,8 @@ #include #include +#include + #include #include @@ -23,9 +25,37 @@ class NETWinInfo; namespace KWin { +class GraphicsBuffer; class X11WindowedBackend; class X11WindowedOutput; +struct DmaBufAttributes; +struct ShmAttributes; + +class X11WindowedBuffer : public QObject +{ + Q_OBJECT + +public: + X11WindowedBuffer(X11WindowedOutput *output, xcb_pixmap_t pixmap, GraphicsBuffer *buffer); + ~X11WindowedBuffer() override; + + GraphicsBuffer *buffer() const; + xcb_pixmap_t pixmap() const; + + void lock(); + void unlock(); + +Q_SIGNALS: + void defunct(); + +private: + X11WindowedOutput *m_output; + GraphicsBuffer *m_buffer; + xcb_pixmap_t m_pixmap; + bool m_locked = false; +}; + class X11WindowedCursor { public: @@ -59,6 +89,8 @@ public: xcb_window_t window() const; int depth() const; + xcb_pixmap_t importBuffer(GraphicsBuffer *buffer); + QPoint internalPosition() const; QPoint hostPosition() const; void setHostPosition(const QPoint &pos); @@ -80,15 +112,20 @@ public: void updateEnabled(bool enabled); void handlePresentCompleteNotify(xcb_present_complete_notify_event_t *event); + void handlePresentIdleNotify(xcb_present_idle_notify_event_t *event); private: void initXInputForWindow(); + xcb_pixmap_t importDmaBufBuffer(const DmaBufAttributes *attributes); + xcb_pixmap_t importShmBuffer(const ShmAttributes *attributes); + xcb_window_t m_window = XCB_WINDOW_NONE; xcb_present_event_t m_presentEvent = XCB_NONE; std::unique_ptr m_winInfo; std::unique_ptr m_renderLoop; std::unique_ptr m_cursor; + std::unordered_map> m_buffers; QPoint m_hostPosition; QRegion m_exposedArea; diff --git a/src/backends/x11/windowed/x11_windowed_qpainter_backend.cpp b/src/backends/x11/windowed/x11_windowed_qpainter_backend.cpp index 3c6b3715a1..059d951293 100644 --- a/src/backends/x11/windowed/x11_windowed_qpainter_backend.cpp +++ b/src/backends/x11/windowed/x11_windowed_qpainter_backend.cpp @@ -18,7 +18,6 @@ #include #include #include -#include namespace KWin { @@ -36,25 +35,10 @@ static QImage::Format drmFormatToQImageFormat(uint32_t drmFormat) } X11WindowedQPainterLayerBuffer::X11WindowedQPainterLayerBuffer(ShmGraphicsBuffer *buffer, X11WindowedOutput *output) - : m_connection(output->backend()->connection()) - , m_graphicsBuffer(buffer) + : m_graphicsBuffer(buffer) { const ShmAttributes *attributes = buffer->shmAttributes(); - // xcb_shm_attach_fd() takes the ownership of the passed shm file descriptor. - FileDescriptor poolFileDescriptor = attributes->fd.duplicate(); - if (!poolFileDescriptor.isValid()) { - qCWarning(KWIN_X11WINDOWED) << "Failed to duplicate shm file descriptor"; - return; - } - - xcb_shm_seg_t segment = xcb_generate_id(m_connection); - xcb_shm_attach_fd(m_connection, segment, poolFileDescriptor.take(), 0); - - m_pixmap = xcb_generate_id(m_connection); - xcb_shm_create_pixmap(m_connection, m_pixmap, output->window(), attributes->size.width(), attributes->size.height(), output->depth(), segment, 0); - xcb_shm_detach(m_connection, segment); - m_size = attributes->size.height() * attributes->stride; m_data = mmap(nullptr, m_size, PROT_READ | PROT_WRITE, MAP_SHARED, attributes->fd.get(), 0); if (m_data == MAP_FAILED) { @@ -67,9 +51,6 @@ X11WindowedQPainterLayerBuffer::X11WindowedQPainterLayerBuffer(ShmGraphicsBuffer X11WindowedQPainterLayerBuffer::~X11WindowedQPainterLayerBuffer() { - if (m_pixmap != XCB_PIXMAP_NONE) { - xcb_free_pixmap(m_connection, m_pixmap); - } if (m_data) { munmap(m_data, m_size); } @@ -82,11 +63,6 @@ ShmGraphicsBuffer *X11WindowedQPainterLayerBuffer::graphicsBuffer() const return m_graphicsBuffer; } -xcb_pixmap_t X11WindowedQPainterLayerBuffer::pixmap() const -{ - return m_pixmap; -} - QImage *X11WindowedQPainterLayerBuffer::view() const { return m_view.get(); @@ -158,6 +134,9 @@ bool X11WindowedQPainterPrimaryLayer::endFrame(const QRegion &renderedRegion, co void X11WindowedQPainterPrimaryLayer::present() { + xcb_pixmap_t pixmap = m_output->importBuffer(m_current->graphicsBuffer()); + Q_ASSERT(pixmap != XCB_PIXMAP_NONE); + xcb_xfixes_region_t valid = 0; xcb_xfixes_region_t update = 0; uint32_t serial = 0; @@ -166,7 +145,7 @@ void X11WindowedQPainterPrimaryLayer::present() xcb_present_pixmap(m_output->backend()->connection(), m_output->window(), - m_current->pixmap(), + pixmap, serial, valid, update, diff --git a/src/backends/x11/windowed/x11_windowed_qpainter_backend.h b/src/backends/x11/windowed/x11_windowed_qpainter_backend.h index 53727da4ad..32b87fd91c 100644 --- a/src/backends/x11/windowed/x11_windowed_qpainter_backend.h +++ b/src/backends/x11/windowed/x11_windowed_qpainter_backend.h @@ -17,8 +17,6 @@ #include -#include - namespace KWin { @@ -34,16 +32,13 @@ public: ~X11WindowedQPainterLayerBuffer(); ShmGraphicsBuffer *graphicsBuffer() const; - xcb_pixmap_t pixmap() const; QImage *view() const; private: - xcb_connection_t *m_connection; ShmGraphicsBuffer *m_graphicsBuffer; void *m_data = nullptr; int m_size = 0; std::unique_ptr m_view; - xcb_pixmap_t m_pixmap = XCB_PIXMAP_NONE; }; class X11WindowedQPainterLayerSwapchain