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>

2 files changed, 40 insertions, 50 deletions

diff --git a/src/compositor/compositor_api/qwaylandsurface.cpp b/src/compositor/compositor_api/qwaylandsurface.cpp
index 2521b7fa1..13ae28220 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 e0b624fce..df868de63 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;