Skip to content

Commit

Permalink
refactor(client): handle subsurfaces without QPointer
Browse files Browse the repository at this point in the history 
It is not needed. The objects keep their lifetimes according to the Wayland
protocol.
  • Loading branch information
@romangg
romangg committed Mar 3, 2024
1 parent 082e99c commit c2cb5ef
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 105 deletions.
86 changes: 35 additions & 51 deletions autotests/client/subsurface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,7 @@ void TestSubsurface::testCreate()

// create subsurface for surface of parent
std::unique_ptr<Wrapland::Client::SubSurface> subsurface(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface, *parent));

QVERIFY(subsurfaceCreatedSpy.wait());

Expand Down Expand Up @@ -267,8 +266,7 @@ void TestSubsurface::testMode()

// create the Subsurface for surface of parent
std::unique_ptr<Wrapland::Client::SubSurface> subsurface(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());
auto serverSubsurface
= subsurfaceCreatedSpy.first().first().value<Wrapland::Server::Subsurface*>();
Expand Down Expand Up @@ -321,8 +319,7 @@ void TestSubsurface::testPosition()

// create the Subsurface for surface of parent
std::unique_ptr<Wrapland::Client::SubSurface> subsurface(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());
auto serverSubsurface
= subsurfaceCreatedSpy.first().first().value<Wrapland::Server::Subsurface*>();
Expand Down Expand Up @@ -379,8 +376,7 @@ void TestSubsurface::testPlaceAbove()

// Create the Subsurfaces for surface of parent.
std::unique_ptr<Wrapland::Client::SubSurface> subsurface1(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface1.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface1, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());

auto serverSubsurface1
Expand All @@ -389,8 +385,7 @@ void TestSubsurface::testPlaceAbove()

subsurfaceCreatedSpy.clear();
std::unique_ptr<Wrapland::Client::SubSurface> subsurface2(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface2.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface2, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());

auto serverSubsurface2
Expand All @@ -399,8 +394,7 @@ void TestSubsurface::testPlaceAbove()

subsurfaceCreatedSpy.clear();
std::unique_ptr<Wrapland::Client::SubSurface> subsurface3(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface3.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface3, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());

auto serverSubsurface3
Expand Down Expand Up @@ -443,7 +437,7 @@ void TestSubsurface::testPlaceAbove()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface1);

// Try placing 3 above 1, should result in 2, 1, 3.
subsurface3->placeAbove(QPointer<Wrapland::Client::SubSurface>(subsurface1.get()));
subsurface3->placeAbove(*subsurface1);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -453,7 +447,7 @@ void TestSubsurface::testPlaceAbove()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface3);

// try placing 3 above 2, should result in 2, 3, 1
subsurface3->placeAbove(QPointer<Wrapland::Client::SubSurface>(subsurface2.get()));
subsurface3->placeAbove(*subsurface2);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -463,7 +457,7 @@ void TestSubsurface::testPlaceAbove()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface1);

// try placing 1 above 3 - shouldn't change
subsurface1->placeAbove(QPointer<Wrapland::Client::SubSurface>(subsurface3.get()));
subsurface1->placeAbove(*subsurface3);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -473,7 +467,7 @@ void TestSubsurface::testPlaceAbove()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface1);

// and 2 above 3 - > 3, 2, 1
subsurface2->placeAbove(QPointer<Wrapland::Client::SubSurface>(subsurface3.get()));
subsurface2->placeAbove(*subsurface3);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -498,24 +492,21 @@ void TestSubsurface::testPlaceBelow()

// create the Subsurfaces for surface of parent
std::unique_ptr<Wrapland::Client::SubSurface> subsurface1(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface1.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface1, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());
auto serverSubsurface1
= subsurfaceCreatedSpy.first().first().value<Wrapland::Server::Subsurface*>();
QVERIFY(serverSubsurface1);
subsurfaceCreatedSpy.clear();
std::unique_ptr<Wrapland::Client::SubSurface> subsurface2(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface2.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface2, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());
auto serverSubsurface2
= subsurfaceCreatedSpy.first().first().value<Wrapland::Server::Subsurface*>();
QVERIFY(serverSubsurface2);
subsurfaceCreatedSpy.clear();
std::unique_ptr<Wrapland::Client::SubSurface> subsurface3(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface3.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface3, *parent));
QVERIFY(subsurfaceCreatedSpy.wait());
auto serverSubsurface3
= subsurfaceCreatedSpy.first().first().value<Wrapland::Server::Subsurface*>();
Expand Down Expand Up @@ -552,7 +543,7 @@ void TestSubsurface::testPlaceBelow()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface2);

// place 1 below 3 -> 1, 3, 2
subsurface1->placeBelow(QPointer<Wrapland::Client::SubSurface>(subsurface3.get()));
subsurface1->placeBelow(*subsurface3);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -562,7 +553,7 @@ void TestSubsurface::testPlaceBelow()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface2);

// 2 below 3 -> 1, 2, 3
subsurface2->placeBelow(QPointer<Wrapland::Client::SubSurface>(subsurface3.get()));
subsurface2->placeBelow(*subsurface3);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -572,7 +563,7 @@ void TestSubsurface::testPlaceBelow()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface3);

// 1 below 2 -> shouldn't change
subsurface1->placeBelow(QPointer<Wrapland::Client::SubSurface>(subsurface2.get()));
subsurface1->placeBelow(*subsurface2);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -582,7 +573,7 @@ void TestSubsurface::testPlaceBelow()
QCOMPARE(serverSubsurface1->parentSurface()->state().children.at(2), serverSubsurface3);

// and 3 below 1 -> 3, 1, 2
subsurface3->placeBelow(QPointer<Wrapland::Client::SubSurface>(subsurface1.get()));
subsurface3->placeBelow(*subsurface1);
parent->commit(Wrapland::Client::Surface::CommitFlag::None);
wl_display_flush(m_connection->display());
QCoreApplication::processEvents();
Expand All @@ -599,8 +590,7 @@ void TestSubsurface::testDestroy()
std::unique_ptr<Wrapland::Client::Surface> parent(m_compositor->createSurface());
// create subsurface for surface of parent
std::unique_ptr<Wrapland::Client::SubSurface> subsurface(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface, *parent));

connect(m_connection,
&Wrapland::Client::ConnectionThread::establishedChanged,
Expand Down Expand Up @@ -650,8 +640,7 @@ void TestSubsurface::testCast()

// Create subsurface for surface with parent.
std::unique_ptr<Wrapland::Client::SubSurface> subsurface(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface, *parent));

QCOMPARE(Wrapland::Client::SubSurface::get(*(subsurface.get())),
QPointer<Wrapland::Client::SubSurface>(subsurface.get()));
Expand Down Expand Up @@ -684,8 +673,7 @@ void TestSubsurface::testSyncMode()

// create subsurface for surface of parent
std::unique_ptr<Wrapland::Client::SubSurface> subsurface(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface, *parent));
QVERIFY(!subsurfaceTreeChangedSpy.wait(100));

parent->commit(Wrapland::Client::Surface::CommitFlag::None);
Expand Down Expand Up @@ -774,8 +762,7 @@ void TestSubsurface::testDeSyncMode()

// create subsurface for surface of parent
std::unique_ptr<Wrapland::Client::SubSurface> subsurface(
m_subCompositor->createSubSurface(QPointer<Wrapland::Client::Surface>(surface.get()),
QPointer<Wrapland::Client::Surface>(parent.get())));
m_subCompositor->createSubSurface(*surface, *parent));
QVERIFY(!subsurfaceTreeChangedSpy.wait(100));

parent->commit(Wrapland::Client::Surface::CommitFlag::None);
Expand Down Expand Up @@ -865,11 +852,9 @@ void TestSubsurface::testMainSurfaceFromTree()
&Wrapland::Server::Surface::subsurfaceTreeChanged);
QVERIFY(subsurfaceTreeChangedSpy.isValid());

auto* sub1 = m_subCompositor->createSubSurface(childLevel1Surface.get(), parentSurface.get());
auto* sub2
= m_subCompositor->createSubSurface(childLevel2Surface.get(), childLevel1Surface.get());
auto* sub3
= m_subCompositor->createSubSurface(childLevel3Surface.get(), childLevel2Surface.get());
auto* sub1 = m_subCompositor->createSubSurface(*childLevel1Surface, *parentSurface);
auto* sub2 = m_subCompositor->createSubSurface(*childLevel2Surface, *childLevel1Surface);
auto* sub3 = m_subCompositor->createSubSurface(*childLevel3Surface, *childLevel2Surface);

childLevel2Surface->commit(Wrapland::Client::Surface::CommitFlag::None);
childLevel1Surface->commit(Wrapland::Client::Surface::CommitFlag::None);
Expand Down Expand Up @@ -927,7 +912,7 @@ void TestSubsurface::testRemoveSurface()
QVERIFY(subsurfaceTreeChangedSpy.isValid());

std::unique_ptr<Wrapland::Client::SubSurface> sub(
m_subCompositor->createSubSurface(childSurface.get(), parentSurface.get()));
m_subCompositor->createSubSurface(*childSurface, *parentSurface));

parentSurface->commit(Wrapland::Client::Surface::CommitFlag::None);
QVERIFY(parent_commit_spy.wait());
Expand Down Expand Up @@ -976,12 +961,11 @@ void TestSubsurface::testMappingOfSurfaceTree()
&Wrapland::Server::Surface::subsurfaceTreeChanged);
QVERIFY(subsurfaceTreeChangedSpy.isValid());

auto subsurfaceLevel1
= m_subCompositor->createSubSurface(childLevel1Surface.get(), parentSurface.get());
auto subsurfaceLevel1 = m_subCompositor->createSubSurface(*childLevel1Surface, *parentSurface);
auto subsurfaceLevel2
= m_subCompositor->createSubSurface(childLevel2Surface.get(), childLevel1Surface.get());
= m_subCompositor->createSubSurface(*childLevel2Surface, *childLevel1Surface);
auto subsurfaceLevel3
= m_subCompositor->createSubSurface(childLevel3Surface.get(), childLevel2Surface.get());
= m_subCompositor->createSubSurface(*childLevel3Surface, *childLevel2Surface);

childLevel3Surface->commit(Wrapland::Client::Surface::CommitFlag::None);
childLevel2Surface->commit(Wrapland::Client::Surface::CommitFlag::None);
Expand Down Expand Up @@ -1150,7 +1134,7 @@ void TestSubsurface::testSurfaceAt()

// Create the subsurfaces for the children.
std::unique_ptr<Wrapland::Client::SubSurface> child1Sub(
m_subCompositor->createSubSurface(child1.get(), parent.get()));
m_subCompositor->createSubSurface(*child1, *parent));
child1Sub->setMode(Wrapland::Client::SubSurface::Mode::Desynchronized);

child1->attachBuffer(m_shm->createBuffer(childImage));
Expand All @@ -1159,7 +1143,7 @@ void TestSubsurface::testSurfaceAt()

// Note: child2Sub is initially above child1Sub since it is added after.
std::unique_ptr<Wrapland::Client::SubSurface> child2Sub(
m_subCompositor->createSubSurface(child2.get(), parent.get()));
m_subCompositor->createSubSurface(*child2, *parent));
child2Sub->setMode(Wrapland::Client::SubSurface::Mode::Desynchronized);
child2Sub->setPosition(QPoint(50, 50));

Expand All @@ -1180,12 +1164,12 @@ void TestSubsurface::testSurfaceAt()

// Create subsurfaces for the grandchildren.
std::unique_ptr<Wrapland::Client::SubSurface> grandchild1Sub(
m_subCompositor->createSubSurface(grandchild1.get(), child1.get()));
m_subCompositor->createSubSurface(*grandchild1, *child1));
grandchild1Sub->setMode(Wrapland::Client::SubSurface::Mode::Desynchronized);
grandchild1Sub->setPosition(QPoint(-25, -25));

std::unique_ptr<Wrapland::Client::SubSurface> grandchild2Sub(
m_subCompositor->createSubSurface(grandchild2.get(), child2.get()));
m_subCompositor->createSubSurface(*grandchild2, *child2));
grandchild2Sub->setMode(Wrapland::Client::SubSurface::Mode::Desynchronized);
grandchild2Sub->setPosition(QPoint(25, 25));

Expand Down Expand Up @@ -1314,7 +1298,7 @@ void TestSubsurface::testDestroyAttachedBuffer()
auto serverChildSurface
= serverSurfaceCreated.last().first().value<Wrapland::Server::Surface*>();
// create sub-surface
auto* sub = m_subCompositor->createSubSurface(child.get(), parent.get());
auto sub = m_subCompositor->createSubSurface(*child, *parent);

// let's damage this surface, will be in sub-surface pending state
QImage image(QSize(100, 100), QImage::Format_ARGB32_Premultiplied);
Expand Down Expand Up @@ -1358,9 +1342,9 @@ void TestSubsurface::testDestroyParentSurface()
auto serverGrandChildSurface
= serverSurfaceCreated.last().first().value<Wrapland::Server::Surface*>();
// create sub-surface in desynchronized mode as Qt uses them
auto sub1 = m_subCompositor->createSubSurface(child.get(), parent.get());
auto sub1 = m_subCompositor->createSubSurface(*child, *parent);
sub1->setMode(Wrapland::Client::SubSurface::Mode::Desynchronized);
auto sub2 = m_subCompositor->createSubSurface(grandChild.get(), child.get());
auto sub2 = m_subCompositor->createSubSurface(*grandChild, *child);
sub2->setMode(Wrapland::Client::SubSurface::Mode::Desynchronized);

// let's damage this surface
Expand Down
6 changes: 3 additions & 3 deletions src/client/subcompositor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ void SubCompositor::setup(wl_subcompositor* subcompositor)
d->subCompositor.setup(subcompositor);
}

SubSurface* SubCompositor::createSubSurface(QPointer<Surface> surface,
QPointer<Surface> parentSurface,
SubSurface* SubCompositor::createSubSurface(Surface const& surface,
Surface const& parentSurface,
QObject* parent)
{
Q_ASSERT(isValid());
SubSurface* s = new SubSurface(surface, parentSurface, parent);
auto w = wl_subcompositor_get_subsurface(d->subCompositor, *surface, *parentSurface);
auto w = wl_subcompositor_get_subsurface(d->subCompositor, surface, parentSurface);
if (d->queue) {
d->queue->addProxy(w);
}
Expand Down
5 changes: 2 additions & 3 deletions src/client/subcompositor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ License along with this library. If not, see <http://www.gnu.org/licenses/>.
#define WAYLAND_SUBCOMPOSITOR_H

#include <QObject>
#include <QPointer>
// STD
#include <Wrapland/Client/wraplandclient_export.h>
#include <memory>
Expand Down Expand Up @@ -85,8 +84,8 @@ class WRAPLANDCLIENT_EXPORT SubCompositor : public QObject
* @param parent The parent to pass to the Surface.
* @returns The new created Surface
**/
SubSurface* createSubSurface(QPointer<Surface> surface,
QPointer<Surface> parentSurface,
SubSurface* createSubSurface(Surface const& surface,
Surface const& parentSurface,
QObject* parent = nullptr);

operator wl_subcompositor*();
Expand Down
Loading

0 comments on commit c2cb5ef

Please sign in to comment.