summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohan Klokkhammer Helsing <johan.helsing@qt.io>2018-10-17 14:05:27 +0200
committerJohan Helsing <johan.helsing@qt.io>2018-10-18 12:11:27 +0000
commit4a328e3533f93ed65f9dc77f764b3cbe3c694266 (patch)
treec7e507cc991be9a01672972eff9ce88b56beade6
parent43d12496c684b5f2b08c6a8c0b994f06efc25712 (diff)
Compositor: Emit signals after applying pending surface statev5.12.0-beta3
[ChangeLog][Compositor] Fixed a bug where some signals on QWaylandSurface were emitted before all double buffered state had been applied. Restructures QWaylandSurface::commit so no signals are emitted until all state mutations are completed. Adds a test to confirm that pending state is applied at once. Also fixes opaqueRegion, which is documented to be double buffered as well, but the old implementation set it immediately. Change-Id: I1c4dfea7c83dd9ee84dc8c03e6d92e2924bf2fad Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
-rw-r--r--src/compositor/compositor_api/qwaylandsurface.cpp84
-rw-r--r--src/compositor/compositor_api/qwaylandsurface_p.h6
-rw-r--r--tests/auto/compositor/compositor/mockclient.cpp2
-rw-r--r--tests/auto/compositor/compositor/tst_compositor.cpp74
4 files changed, 113 insertions, 53 deletions
diff --git a/src/compositor/compositor_api/qwaylandsurface.cpp b/src/compositor/compositor_api/qwaylandsurface.cpp
index 2521b7fa..13ae2822 100644
--- a/src/compositor/compositor_api/qwaylandsurface.cpp
+++ b/src/compositor/compositor_api/qwaylandsurface.cpp
@@ -141,25 +141,6 @@ QWaylandSurfacePrivate::~QWaylandSurfacePrivate()
c->destroy();
}
-void QWaylandSurfacePrivate::setSize(const QSize &s)
-{
- Q_Q(QWaylandSurface);
- if (size != s) {
- opaqueRegion = QRegion();
- size = s;
- q->sizeChanged();
- }
-}
-
-void QWaylandSurfacePrivate::setBufferScale(int scale)
-{
- Q_Q(QWaylandSurface);
- if (scale == bufferScale)
- return;
- bufferScale = scale;
- emit q->bufferScaleChanged();
-}
-
void QWaylandSurfacePrivate::removeFrameCallback(QtWayland::FrameCallback *callback)
{
pendingFrameCallbacks.removeOne(callback);
@@ -235,7 +216,7 @@ void QWaylandSurfacePrivate::surface_frame(Resource *resource, uint32_t callback
void QWaylandSurfacePrivate::surface_set_opaque_region(Resource *, struct wl_resource *region)
{
- opaqueRegion = region ? QtWayland::Region::fromResource(region)->region() : QRegion();
+ pending.opaqueRegion = region ? QtWayland::Region::fromResource(region)->region() : QRegion();
}
void QWaylandSurfacePrivate::surface_set_input_region(Resource *, struct wl_resource *region)
@@ -251,42 +232,53 @@ void QWaylandSurfacePrivate::surface_commit(Resource *)
{
Q_Q(QWaylandSurface);
+ // Needed in order to know whether we want to emit signals later
+ QSize oldBufferSize = bufferSize;
+ bool oldHasContent = hasContent;
+ int oldBufferScale = bufferScale;
+
+ // Update all internal state
if (pending.buffer.hasBuffer() || pending.newlyAttached)
bufferRef = pending.buffer;
+ bufferSize = bufferRef.size();
+ damage = pending.damage.intersected(QRect(QPoint(), bufferSize));
+ hasContent = bufferRef.hasContent();
+ bufferScale = pending.bufferScale;
+ frameCallbacks << pendingFrameCallbacks;
+ inputRegion = pending.inputRegion.intersected(QRect(QPoint(), bufferSize));
+ opaqueRegion = pending.opaqueRegion.intersected(QRect(QPoint(), bufferSize));
+ QPoint offsetForNextFrame = pending.offset;
- auto buffer = bufferRef.buffer();
- if (buffer)
- buffer->setCommitted(pending.damage);
+ // Clear per-commit state
+ pending.buffer = QWaylandBufferRef();
+ pending.offset = QPoint();
+ pending.newlyAttached = false;
+ pending.damage = QRegion();
+ pendingFrameCallbacks.clear();
- setSize(bufferRef.size());
- damage = pending.damage.intersected(QRect(QPoint(), size));
+ // Notify buffers and views
+ if (auto *buffer = bufferRef.buffer())
+ buffer->setCommitted(damage);
+ for (auto *view : qAsConst(views))
+ view->bufferCommitted(bufferRef, damage);
- for (int i = 0; i < views.size(); i++) {
- views.at(i)->bufferCommitted(bufferRef, damage);
- }
+ // Now all double-buffered state has been applied so it's safe to emit general signals
+ // i.e. we won't have inconsistensies such as mismatched surface size and buffer scale in
+ // signal handlers.
emit q->damaged(damage);
- bool oldHasContent = hasContent;
- hasContent = bufferRef.hasContent();
- if (oldHasContent != hasContent)
- emit q->hasContentChanged();
-
- if (!pending.offset.isNull())
- emit q->offsetForNextFrame(pending.offset);
-
- setBufferScale(pending.bufferScale);
+ if (oldBufferSize != bufferSize)
+ emit q->sizeChanged();
+ if (oldBufferScale != bufferScale)
+ emit q->bufferScaleChanged();
- pending.buffer = QWaylandBufferRef();
- pending.offset = QPoint();
- pending.newlyAttached = false;
- pending.damage = QRegion();
-
- frameCallbacks << pendingFrameCallbacks;
- pendingFrameCallbacks.clear();
+ if (oldHasContent != hasContent)
+ emit q->hasContentChanged();
- inputRegion = pending.inputRegion.intersected(QRect(QPoint(), size));
+ if (!offsetForNextFrame.isNull())
+ emit q->offsetForNextFrame(offsetForNextFrame);
emit q->redraw();
}
@@ -469,7 +461,7 @@ bool QWaylandSurface::hasContent() const
QSize QWaylandSurface::size() const
{
Q_D(const QWaylandSurface);
- return d->size;
+ return d->bufferSize;
}
/*!
diff --git a/src/compositor/compositor_api/qwaylandsurface_p.h b/src/compositor/compositor_api/qwaylandsurface_p.h
index e0b624fc..df868de6 100644
--- a/src/compositor/compositor_api/qwaylandsurface_p.h
+++ b/src/compositor/compositor_api/qwaylandsurface_p.h
@@ -102,9 +102,6 @@ public:
using QtWaylandServer::wl_surface::resource;
- void setSize(const QSize &size);
- void setBufferScale(int bufferScale);
-
void removeFrameCallback(QtWayland::FrameCallback *callback);
void notifyViewsAboutDestruction();
@@ -155,6 +152,7 @@ public: //member variables
bool newlyAttached;
QRegion inputRegion;
int bufferScale;
+ QRegion opaqueRegion;
} pending;
QPoint lastLocalMousePos;
@@ -168,7 +166,7 @@ public: //member variables
QRegion inputRegion;
QRegion opaqueRegion;
- QSize size;
+ QSize bufferSize;
int bufferScale = 1;
bool isCursorSurface = false;
bool destroyed = false;
diff --git a/tests/auto/compositor/compositor/mockclient.cpp b/tests/auto/compositor/compositor/mockclient.cpp
index e6eac155..f7431440 100644
--- a/tests/auto/compositor/compositor/mockclient.cpp
+++ b/tests/auto/compositor/compositor/mockclient.cpp
@@ -166,7 +166,7 @@ void MockClient::handleGlobalRemove(void *data, wl_registry *wl_registry, uint32
void MockClient::handleGlobal(uint32_t id, const QByteArray &interface)
{
if (interface == "wl_compositor") {
- compositor = static_cast<wl_compositor *>(wl_registry_bind(registry, id, &wl_compositor_interface, 1));
+ compositor = static_cast<wl_compositor *>(wl_registry_bind(registry, id, &wl_compositor_interface, 3));
} else if (interface == "wl_output") {
auto output = static_cast<wl_output *>(wl_registry_bind(registry, id, &wl_output_interface, 2));
m_outputs.insert(id, output);
diff --git a/tests/auto/compositor/compositor/tst_compositor.cpp b/tests/auto/compositor/compositor/tst_compositor.cpp
index c7661e8d..2c0e46b2 100644
--- a/tests/auto/compositor/compositor/tst_compositor.cpp
+++ b/tests/auto/compositor/compositor/tst_compositor.cpp
@@ -74,6 +74,7 @@ private slots:
void modes();
void sizeFollowsWindow();
void mapSurface();
+ void mapSurfaceHiDpi();
void frameCallback();
void removeOutput();
@@ -421,9 +422,78 @@ void tst_WaylandCompositor::mapSurface()
wl_surface_damage(surface, 0, 0, size.width(), size.height());
wl_surface_commit(surface);
- QTRY_COMPARE(waylandSurface->size(), size);
- QTRY_COMPARE(waylandSurface->hasContent(), true);
QTRY_COMPARE(hasContentSpy.count(), 1);
+ QCOMPARE(waylandSurface->hasContent(), true);
+ QCOMPARE(waylandSurface->size(), size);
+
+ wl_surface_destroy(surface);
+}
+
+void tst_WaylandCompositor::mapSurfaceHiDpi()
+{
+ TestCompositor compositor;
+ compositor.create();
+
+ MockClient client;
+
+ wl_surface *surface = client.createSurface();
+ QTRY_COMPARE(compositor.surfaces.size(), 1);
+
+ QWaylandSurface *waylandSurface = compositor.surfaces.at(0);
+
+ constexpr int bufferScale = 2;
+ const QSize surfaceSize(128, 128);
+ const QSize bufferSize = surfaceSize * bufferScale;
+ const QPoint attachOffset(1, 2); // in surface-local coordinates
+
+ client.createShellSurface(surface);
+ ShmBuffer buffer(bufferSize, client.shm);
+ wl_surface_attach(surface, buffer.handle, attachOffset.x(), attachOffset.y());
+ wl_surface_set_buffer_scale(surface, bufferScale);
+ // wl_surface_damage is given in surface coordinates
+ wl_surface_damage(surface, 0, 0, surfaceSize.width(), surfaceSize.height());
+
+ auto verifyComittedState = [=]() {
+ QCOMPARE(waylandSurface->size(), bufferSize);
+ QCOMPARE(waylandSurface->bufferScale(), bufferScale);
+ QCOMPARE(waylandSurface->hasContent(), true);
+ };
+
+ QObject::connect(waylandSurface, &QWaylandSurface::damaged, [=] (const QRegion &damage) {
+ // Currently, QWaylandSurface::size returns the size in pixels.
+ // Should be fixed or removed for Qt 6.
+ QCOMPARE(damage, QRect(QPoint(), surfaceSize));
+ verifyComittedState();
+ });
+ QSignalSpy damagedSpy(waylandSurface, SIGNAL(damaged(const QRegion &)));
+
+ QObject::connect(waylandSurface, &QWaylandSurface::hasContentChanged, verifyComittedState);
+ QSignalSpy hasContentSpy(waylandSurface, SIGNAL(hasContentChanged()));
+
+ QObject::connect(waylandSurface, &QWaylandSurface::sizeChanged, verifyComittedState);
+ QSignalSpy sizeSpy(waylandSurface, SIGNAL(sizeChanged()));
+
+ QObject::connect(waylandSurface, &QWaylandSurface::bufferScaleChanged, verifyComittedState);
+ QSignalSpy bufferScaleSpy(waylandSurface, SIGNAL(bufferScaleChanged()));
+
+ QObject::connect(waylandSurface, &QWaylandSurface::offsetForNextFrame, [=](const QPoint &offset) {
+ QCOMPARE(offset, attachOffset);
+ verifyComittedState();
+ });
+ QSignalSpy offsetSpy(waylandSurface, SIGNAL(offsetForNextFrame(const QPoint &)));
+
+ // No state should be applied before the commit
+ QCOMPARE(waylandSurface->size(), QSize());
+ QCOMPARE(waylandSurface->hasContent(), false);
+ QCOMPARE(waylandSurface->bufferScale(), 1);
+ QCOMPARE(offsetSpy.count(), 0);
+
+ wl_surface_commit(surface);
+
+ QTRY_COMPARE(hasContentSpy.count(), 1);
+ QTRY_COMPARE(sizeSpy.count(), 1);
+ QTRY_COMPARE(bufferScaleSpy.count(), 1);
+ QTRY_COMPARE(offsetSpy.count(), 1);
wl_surface_destroy(surface);
}