From 5b03dfe3249af0c5b634ae42c41a279f39ef1ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Fri, 28 Nov 2014 08:33:32 +0100 Subject: [PATCH] Improve attaching buffer and commiting in SurfaceInterface Now the code handles correctly the attaching of a null buffer (emits a signal unmapped) and if a commit doesn't have a pending buffer it won't be reset. Damage requests are ignored if no buffer has been attached yet. --- .../autotests/client/test_wayland_surface.cpp | 92 ++++++++++++++----- src/wayland/surface_interface.cpp | 35 +++++-- src/wayland/surface_interface.h | 4 + src/wayland/surface_interface_p.h | 1 + 4 files changed, 103 insertions(+), 29 deletions(-) diff --git a/src/wayland/autotests/client/test_wayland_surface.cpp b/src/wayland/autotests/client/test_wayland_surface.cpp index 58f2fae6ce..61bb6cb4c1 100644 --- a/src/wayland/autotests/client/test_wayland_surface.cpp +++ b/src/wayland/autotests/client/test_wayland_surface.cpp @@ -57,6 +57,7 @@ private: KWayland::Server::CompositorInterface *m_compositorInterface; KWayland::Client::ConnectionThread *m_connection; KWayland::Client::Compositor *m_compositor; + KWayland::Client::ShmPool *m_shm; QThread *m_thread; }; @@ -80,6 +81,7 @@ void TestWaylandSurface::init() m_display->setSocketName(s_socketName); m_display->start(); QVERIFY(m_display->isRunning()); + m_display->createShm(); m_compositorInterface = m_display->createCompositor(m_display); QVERIFY(m_compositorInterface); @@ -108,13 +110,21 @@ void TestWaylandSurface::init() KWayland::Client::Registry registry; QSignalSpy compositorSpy(®istry, SIGNAL(compositorAnnounced(quint32,quint32))); + QSignalSpy shmSpy(®istry, SIGNAL(shmAnnounced(quint32,quint32))); + QSignalSpy allAnnounced(®istry, SIGNAL(interfacesAnnounced())); + QVERIFY(allAnnounced.isValid()); + QVERIFY(shmSpy.isValid()); registry.create(m_connection->display()); QVERIFY(registry.isValid()); registry.setup(); - QVERIFY(compositorSpy.wait()); + QVERIFY(allAnnounced.wait()); + QVERIFY(!compositorSpy.isEmpty()); + QVERIFY(!shmSpy.isEmpty()); m_compositor = registry.createCompositor(compositorSpy.first().first().value(), compositorSpy.first().last().value(), this); QVERIFY(m_compositor->isValid()); + m_shm = registry.createShmPool(shmSpy.first().first().value(), shmSpy.first().last().value(), this); + QVERIFY(m_shm->isValid()); } void TestWaylandSurface::cleanup() @@ -123,6 +133,10 @@ void TestWaylandSurface::cleanup() delete m_compositor; m_compositor = nullptr; } + if (m_shm) { + delete m_shm; + m_shm = nullptr; + } if (m_thread) { m_thread->quit(); m_thread->wait(); @@ -198,7 +212,18 @@ void TestWaylandSurface::testDamage() QSignalSpy damageSpy(serverSurface, SIGNAL(damaged(QRegion))); QVERIFY(damageSpy.isValid()); - // TODO: actually we would need to attach a buffer first + // send damage without a buffer + s->damage(QRect(0, 0, 100, 100)); + s->commit(KWayland::Client::Surface::CommitFlag::None); + wl_display_flush(m_connection->display()); + QCoreApplication::processEvents(); + QCoreApplication::processEvents(); + QVERIFY(damageSpy.isEmpty()); + + QImage img(QSize(10, 10), QImage::Format_ARGB32); + img.fill(Qt::black); + auto b = m_shm->createBuffer(img); + s->attachBuffer(b); s->damage(QRect(0, 0, 10, 10)); s->commit(KWayland::Client::Surface::CommitFlag::None); QVERIFY(damageSpy.wait()); @@ -208,6 +233,10 @@ void TestWaylandSurface::testDamage() // damage multiple times QRegion testRegion(5, 8, 3, 6); testRegion = testRegion.united(QRect(10, 20, 30, 15)); + img = QImage(QSize(40, 35), QImage::Format_ARGB32); + img.fill(Qt::black); + b = m_shm->createBuffer(img); + s->attachBuffer(b); s->damage(testRegion); damageSpy.clear(); s->commit(KWayland::Client::Surface::CommitFlag::None); @@ -230,6 +259,10 @@ void TestWaylandSurface::testFrameCallback() QSignalSpy frameRenderedSpy(s, SIGNAL(frameRendered())); QVERIFY(frameRenderedSpy.isValid()); + QImage img(QSize(10, 10), QImage::Format_ARGB32); + img.fill(Qt::black); + auto b = m_shm->createBuffer(img); + s->attachBuffer(b); s->damage(QRect(0, 0, 10, 10)); s->commit(); QVERIFY(damageSpy.wait()); @@ -241,20 +274,6 @@ void TestWaylandSurface::testFrameCallback() void TestWaylandSurface::testAttachBuffer() { - // here we need a shm pool - m_display->createShm(); - - KWayland::Client::Registry registry; - QSignalSpy shmSpy(®istry, SIGNAL(shmAnnounced(quint32,quint32))); - registry.create(m_connection->display()); - QVERIFY(registry.isValid()); - registry.setup(); - QVERIFY(shmSpy.wait()); - - KWayland::Client::ShmPool pool; - pool.setup(registry.bindShm(shmSpy.first().first().value(), shmSpy.first().last().value())); - QVERIFY(pool.isValid()); - // create the surface QSignalSpy serverSurfaceCreated(m_compositorInterface, SIGNAL(surfaceCreated(KWayland::Server::SurfaceInterface*))); QVERIFY(serverSurfaceCreated.isValid()); @@ -271,9 +290,9 @@ void TestWaylandSurface::testAttachBuffer() QImage blue(24, 24, QImage::Format_ARGB32_Premultiplied); blue.fill(QColor(0, 0, 255, 128)); - wl_buffer *blackBuffer = *(pool.createBuffer(black).data()); - auto redBuffer = pool.createBuffer(red); - auto blueBuffer = pool.createBuffer(blue).toStrongRef(); + wl_buffer *blackBuffer = *(m_shm->createBuffer(black).data()); + auto redBuffer = m_shm->createBuffer(red); + auto blueBuffer = m_shm->createBuffer(blue).toStrongRef(); QCOMPARE(blueBuffer->format(), KWayland::Client::Buffer::Format::ARGB32); QCOMPARE(blueBuffer->size(), blue.size()); @@ -287,7 +306,10 @@ void TestWaylandSurface::testAttachBuffer() s->commit(KWayland::Client::Surface::CommitFlag::None); QSignalSpy damageSpy(serverSurface, SIGNAL(damaged(QRegion))); QVERIFY(damageSpy.isValid()); + QSignalSpy unmappedSpy(serverSurface, SIGNAL(unmapped())); + QVERIFY(unmappedSpy.isValid()); QVERIFY(damageSpy.wait()); + QVERIFY(unmappedSpy.isEmpty()); // now the ServerSurface should have the black image attached as a buffer KWayland::Server::BufferInterface *buffer = serverSurface->buffer(); @@ -302,6 +324,7 @@ void TestWaylandSurface::testAttachBuffer() s->commit(KWayland::Client::Surface::CommitFlag::None); damageSpy.clear(); QVERIFY(damageSpy.wait()); + QVERIFY(unmappedSpy.isEmpty()); KWayland::Server::BufferInterface *buffer2 = serverSurface->buffer(); buffer2->ref(); QVERIFY(buffer2->shmBuffer()); @@ -321,6 +344,7 @@ void TestWaylandSurface::testAttachBuffer() s->commit(); damageSpy.clear(); QVERIFY(damageSpy.wait()); + QVERIFY(unmappedSpy.isEmpty()); QVERIFY(!buffer2->isReferenced()); delete buffer2; // TODO: we should have a signal on when the Buffer gets released @@ -345,6 +369,32 @@ void TestWaylandSurface::testAttachBuffer() serverSurface->frameRendered(1); QVERIFY(frameRenderedSpy.wait()); + // commit a different value shouldn't change our buffer + QCOMPARE(serverSurface->buffer(), buffer3); + QVERIFY(serverSurface->input().isNull()); + damageSpy.clear(); + s->setInputRegion(m_compositor->createRegion(QRegion(0, 0, 24, 24)).get()); + s->commit(KWayland::Client::Surface::CommitFlag::None); + wl_display_flush(m_connection->display()); + QCoreApplication::processEvents(); + QCoreApplication::processEvents(); + QCOMPARE(serverSurface->input(), QRegion(0, 0, 24, 24)); + QCOMPARE(serverSurface->buffer(), buffer3); + QVERIFY(damageSpy.isEmpty()); + QVERIFY(unmappedSpy.isEmpty()); + + // clear the surface + s->attachBuffer(blackBuffer); + s->damage(QRect(0, 0, 1, 1)); + // TODO: better method + s->attachBuffer((wl_buffer*)nullptr); + s->damage(QRect(0, 0, 10, 10)); + s->commit(KWayland::Client::Surface::CommitFlag::None); + QVERIFY(unmappedSpy.wait()); + QVERIFY(!unmappedSpy.isEmpty()); + QCOMPARE(unmappedSpy.count(), 1); + QVERIFY(damageSpy.isEmpty()); + // TODO: add signal test on release buffer->unref(); } @@ -353,9 +403,6 @@ void TestWaylandSurface::testMultipleSurfaces() { using namespace KWayland::Client; using namespace KWayland::Server; - // here we need a shm pool - m_display->createShm(); - Registry registry; QSignalSpy shmSpy(®istry, SIGNAL(shmAnnounced(quint32,quint32))); registry.create(m_connection->display()); @@ -566,6 +613,7 @@ void TestWaylandSurface::testDestroy() connect(m_connection, &ConnectionThread::connectionDied, s, &Surface::destroy); connect(m_connection, &ConnectionThread::connectionDied, m_compositor, &Compositor::destroy); + connect(m_connection, &ConnectionThread::connectionDied, m_shm, &ShmPool::destroy); QVERIFY(s->isValid()); QSignalSpy connectionDiedSpy(m_connection, SIGNAL(connectionDied())); diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp index 11419b26c7..8cf56d9e6b 100644 --- a/src/wayland/surface_interface.cpp +++ b/src/wayland/surface_interface.cpp @@ -177,18 +177,24 @@ void SurfaceInterface::Private::commit() for (wl_resource *c : current.callbacks) { wl_resource_destroy(c); } + const bool bufferChanged = pending.bufferIsSet; const bool opaqueRegionChanged = pending.opaqueIsSet; const bool inputRegionChanged = pending.inputIsSet; const bool scaleFactorChanged = current.scale != pending.scale; const bool transformFactorChanged = current.transform != pending.transform; - if (current.buffer) { - current.buffer->unref(); - } - if (pending.buffer) { - pending.buffer->ref(); + auto buffer = current.buffer; + if (bufferChanged) { + if (current.buffer) { + current.buffer->unref(); + } + if (pending.buffer) { + pending.buffer->ref(); + } + buffer = pending.buffer; } // copy values current = pending; + current.buffer = buffer; pending = State{}; pending.children = current.children; pending.input = current.input; @@ -213,13 +219,21 @@ void SurfaceInterface::Private::commit() if (transformFactorChanged) { emit q->transformChanged(current.transform); } - if (!current.damage.isEmpty()) { - emit q->damaged(current.damage); + if (bufferChanged) { + if (!current.damage.isEmpty()) { + emit q->damaged(current.damage); + } else if (!current.buffer) { + emit q->unmapped(); + } } } void SurfaceInterface::Private::damage(const QRect &rect) { + if (!pending.bufferIsSet || (pending.bufferIsSet && !pending.buffer)) { + // TODO: should we send an error? + return; + } // TODO: documentation says we need to remove the parts outside of the surface pending.damage = pending.damage.united(rect); } @@ -247,10 +261,17 @@ void SurfaceInterface::Private::addFrameCallback(uint32_t callback) void SurfaceInterface::Private::attachBuffer(wl_resource *buffer, const QPoint &offset) { + pending.bufferIsSet = true; pending.offset = offset; if (pending.buffer) { delete pending.buffer; } + if (!buffer) { + // got a null buffer, deletes content in next frame + pending.buffer = nullptr; + pending.damage = QRegion(); + return; + } Q_Q(SurfaceInterface); pending.buffer = new BufferInterface(buffer, q); QObject::connect(pending.buffer, &BufferInterface::aboutToBeDestroyed, q, diff --git a/src/wayland/surface_interface.h b/src/wayland/surface_interface.h index 39aebdb345..6153c9c73a 100644 --- a/src/wayland/surface_interface.h +++ b/src/wayland/surface_interface.h @@ -76,6 +76,10 @@ Q_SIGNALS: void inputChanged(const QRegion&); void scaleChanged(qint32); void transformChanged(KWayland::Server::OutputInterface::Transform); + /** + * Emitted when the Surface removes its content + **/ + void unmapped(); private: friend class CompositorInterface; diff --git a/src/wayland/surface_interface_p.h b/src/wayland/surface_interface_p.h index 23b4473a33..02a2d7b4b9 100644 --- a/src/wayland/surface_interface_p.h +++ b/src/wayland/surface_interface_p.h @@ -39,6 +39,7 @@ public: QRegion input = QRegion(); bool inputIsSet = false; bool opaqueIsSet = false; + bool bufferIsSet = false; bool inputIsInfinite = true; qint32 scale = 1; OutputInterface::Transform transform = OutputInterface::Transform::Normal;