compositor: make wl_surface::frame follow pending states (#11953)

* compositor: make pending states store frame callbacks

move frame callbacks to pending states so they are only committed in the
order they came depending on the buffer wait for readyness.

* buffer: damage is relative to current commit

ensure the damage is only used once, or we are constantly redrawing
things on state commits that isnt a buffer.

thanks PlasmaPower.

* compositor: move callbacks back to compositor

move SSurfaceStateFrameCB back to compositor in the class
CWLCallbackResource as per request, but still keep the state as owning.

* compositor: ensure commits come in order

if a buffer is waiting any commits after it might be non buffer commits
from the "future" and applying directly. and when the old buffer that
was waiting becomes ready it applies its states and overwrites the
future changes.

move things to scheduleState and add a m_pendingWaiting guard. and
schedule the next pending state from the old buffer commit when done.
and as such it loops itself and keeps thing orderly.

5 files changed, 97 insertions, 49 deletions

diff --git a/src/protocols/core/Compositor.cpp b/src/protocols/core/Compositor.cpp
index 484df21f..955ba5ca 100644
--- a/src/protocols/core/Compositor.cpp
+++ b/src/protocols/core/Compositor.cpp
@@ -27,16 +27,20 @@ class CDefaultSurfaceRole : public ISurfaceRole {
     }
 };
 
-CWLCallbackResource::CWLCallbackResource(UP<CWlCallback>&& resource_) : m_resource(std::move(resource_)) {
+CWLCallbackResource::CWLCallbackResource(SP<CWlCallback>&& resource_) : m_resource(std::move(resource_)) {
     ;
 }
 
 bool CWLCallbackResource::good() {
-    return m_resource->resource();
+    return m_resource && m_resource->resource();
 }
 
 void CWLCallbackResource::send(const Time::steady_tp& now) {
+    if (!good())
+        return;
+
     m_resource->sendDone(Time::millis(now));
+    m_resource.reset();
 }
 
 CWLRegionResource::CWLRegionResource(SP<CWlRegion> resource_) : m_resource(resource_) {
@@ -127,17 +131,16 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : m_resource(re
             return;
         }
 
-        if ((!m_pending.updated.bits.buffer) ||       // no new buffer attached
-            (!m_pending.buffer && !m_pending.texture) // null buffer attached
-        ) {
+        // null buffer attached
+        if (!m_pending.buffer && !m_pending.texture && m_pending.updated.bits.buffer) {
             commitState(m_pending);
 
-            if (!m_pending.buffer && !m_pending.texture) {
-                // null buffer attached, remove any pending states.
-                while (!m_pendingStates.empty()) {
-                    m_pendingStates.pop();
-                }
+            // remove any pending states.
+            while (!m_pendingStates.empty()) {
+                m_pendingStates.pop();
             }
+
+            m_pendingWaiting = false;
             m_pending.reset();
             return;
         }
@@ -146,36 +149,9 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : m_resource(re
         const auto& state = m_pendingStates.emplace(makeUnique<SSurfaceState>(m_pending));
         m_pending.reset();
 
-        auto whenReadable = [this, surf = m_self, state = WP<SSurfaceState>(m_pendingStates.back())] {
-            if (!surf || state.expired())
-                return;
-
-            while (!m_pendingStates.empty() && m_pendingStates.front() != state) {
-                commitState(*m_pendingStates.front());
-                m_pendingStates.pop();
-            }
-
-            commitState(*m_pendingStates.front());
-            m_pendingStates.pop();
-        };
-
-        if (state->updated.bits.acquire) {
-            // wait on acquire point for this surface, from explicit sync protocol
-            state->acquire.addWaiter(std::move(whenReadable));
-        } else if (state->buffer->isSynchronous()) {
-            // synchronous (shm) buffers can be read immediately
-            whenReadable();
-        } else if (state->buffer->type() == Aquamarine::BUFFER_TYPE_DMABUF && state->buffer->dmabuf().success) {
-            // async buffer and is dmabuf, then we can wait on implicit fences
-            auto syncFd = dc<CDMABuffer*>(state->buffer.m_buffer.get())->exportSyncFile();
-
-            if (syncFd.isValid())
-                g_pEventLoopManager->doOnReadable(std::move(syncFd), std::move(whenReadable));
-            else
-                whenReadable();
-        } else {
-            Debug::log(ERR, "BUG THIS: wl_surface.commit: no acquire, non-dmabuf, async buffer, needs wait... this shouldn't happen");
-            whenReadable();
+        if (!m_pendingWaiting) {
+            m_pendingWaiting = true;
+            scheduleState(state);
         }
     });
 
@@ -239,7 +215,10 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : m_resource(re
         m_pending.opaque = RG->m_region;
     });
 
-    m_resource->setFrame([this](CWlSurface* r, uint32_t id) { m_callbacks.emplace_back(makeUnique<CWLCallbackResource>(makeUnique<CWlCallback>(m_client, 1, id))); });
+    m_resource->setFrame([this](CWlSurface* r, uint32_t id) {
+        m_pending.updated.bits.frame = true;
+        m_pending.callbacks.emplace_back(makeShared<CWLCallbackResource>(makeShared<CWlCallback>(m_client, 1, id)));
+    });
 
     m_resource->setOffset([this](CWlSurface* r, int32_t x, int32_t y) {
         m_pending.updated.bits.offset = true;
@@ -340,14 +319,14 @@ void CWLSurfaceResource::sendPreferredScale(int32_t scale) {
 }
 
 void CWLSurfaceResource::frame(const Time::steady_tp& now) {
-    if (m_callbacks.empty())
+    if (m_current.callbacks.empty())
         return;
 
-    for (auto const& c : m_callbacks) {
+    for (auto const& c : m_current.callbacks) {
         c->send(now);
     }
 
-    m_callbacks.clear();
+    m_current.callbacks.clear();
 }
 
 void CWLSurfaceResource::resetRole() {
@@ -501,6 +480,47 @@ CBox CWLSurfaceResource::extends() {
     return full.getExtents();
 }
 
+void CWLSurfaceResource::scheduleState(WP<SSurfaceState> state) {
+    auto whenReadable = [this, surf = m_self, state] {
+        if (!surf || state.expired() || m_pendingStates.empty())
+            return;
+
+        while (!m_pendingStates.empty() && m_pendingStates.front() != state) {
+            commitState(*m_pendingStates.front());
+            m_pendingStates.pop();
+        }
+
+        commitState(*m_pendingStates.front());
+        m_pendingStates.pop();
+
+        // If more states are queued, schedule next state
+        if (!m_pendingStates.empty()) {
+            scheduleState(m_pendingStates.front());
+        } else {
+            m_pendingWaiting = false;
+        }
+    };
+
+    if (state->updated.bits.acquire) {
+        // wait on acquire point for this surface, from explicit sync protocol
+        state->acquire.addWaiter(std::move(whenReadable));
+    } else if (state->buffer && state->buffer->isSynchronous()) {
+        // synchronous (shm) buffers can be read immediately
+        whenReadable();
+    } else if (state->buffer && state->buffer->type() == Aquamarine::BUFFER_TYPE_DMABUF && state->buffer->dmabuf().success) {
+        // async buffer and is dmabuf, then we can wait on implicit fences
+        auto syncFd = dc<CDMABuffer*>(state->buffer.m_buffer.get())->exportSyncFile();
+
+        if (syncFd.isValid())
+            g_pEventLoopManager->doOnReadable(std::move(syncFd), std::move(whenReadable));
+        else
+            whenReadable();
+    } else {
+        // state commit without a buffer.
+        whenReadable();
+    }
+}
+
 void CWLSurfaceResource::commitState(SSurfaceState& state) {
     auto lastTexture = m_current.texture;
     m_current.updateFrom(state);
diff --git a/src/protocols/core/Compositor.hpp b/src/protocols/core/Compositor.hpp
index 0eff3f08..dc61917d 100644
--- a/src/protocols/core/Compositor.hpp
+++ b/src/protocols/core/Compositor.hpp
@@ -35,13 +35,21 @@ class CContentType;
 
 class CWLCallbackResource {
   public:
-    CWLCallbackResource(UP<CWlCallback>&& resource_);
+    CWLCallbackResource(SP<CWlCallback>&& resource_);
+    ~CWLCallbackResource() noexcept = default;
+    // disable copy
+    CWLCallbackResource(const CWLCallbackResource&)            = delete;
+    CWLCallbackResource& operator=(const CWLCallbackResource&) = delete;
 
-    bool good();
-    void send(const Time::steady_tp& now);
+    // allow move
+    CWLCallbackResource(CWLCallbackResource&&) noexcept            = default;
+    CWLCallbackResource& operator=(CWLCallbackResource&&) noexcept = default;
+
+    bool                 good();
+    void                 send(const Time::steady_tp& now);
 
   private:
-    UP<CWlCallback> m_resource;
+    SP<CWlCallback> m_resource;
 };
 
 class CWLRegionResource {
@@ -94,8 +102,8 @@ class CWLSurfaceResource {
     SSurfaceState                          m_current;
     SSurfaceState                          m_pending;
     std::queue<UP<SSurfaceState>>          m_pendingStates;
+    bool                                   m_pendingWaiting = false;
 
-    std::vector<UP<CWLCallbackResource>>   m_callbacks;
     WP<CWLSurfaceResource>                 m_self;
     WP<CWLSurface>                         m_hlSurface;
     std::vector<PHLMONITORREF>             m_enteredOutputs;
@@ -110,6 +118,7 @@ class CWLSurfaceResource {
     SP<CWLSurfaceResource>                 findFirstPreorder(std::function<bool(SP<CWLSurfaceResource>)> fn);
     SP<CWLSurfaceResource>                 findWithCM();
     void                                   presentFeedback(const Time::steady_tp& when, PHLMONITOR pMonitor, bool discarded = false);
+    void                                   scheduleState(WP<SSurfaceState> state);
     void                                   commitState(SSurfaceState& state);
     NColorManagement::SImageDescription    getPreferredImageDescription();
     void                                   sortSubsurfaces();
diff --git a/src/protocols/types/SurfaceState.cpp b/src/protocols/types/SurfaceState.cpp
index a8838170..6be8e173 100644
--- a/src/protocols/types/SurfaceState.cpp
+++ b/src/protocols/types/SurfaceState.cpp
@@ -60,6 +60,8 @@ void SSurfaceState::reset() {
     // wl_surface.commit assigns pending ... and clears pending damage.
     damage.clear();
     bufferDamage.clear();
+
+    callbacks.clear();
 }
 
 void SSurfaceState::updateFrom(SSurfaceState& ref) {
@@ -75,6 +77,10 @@ void SSurfaceState::updateFrom(SSurfaceState& ref) {
     if (ref.updated.bits.damage) {
         damage       = ref.damage;
         bufferDamage = ref.bufferDamage;
+    } else {
+        // damage is always relative to the current commit
+        damage.clear();
+        bufferDamage.clear();
     }
 
     if (ref.updated.bits.input)
@@ -100,4 +106,9 @@ void SSurfaceState::updateFrom(SSurfaceState& ref) {
 
     if (ref.updated.bits.acked)
         ackedSize = ref.ackedSize;
+
+    if (ref.updated.bits.frame) {
+        callbacks.insert(callbacks.end(), std::make_move_iterator(ref.callbacks.begin()), std::make_move_iterator(ref.callbacks.end()));
+        ref.callbacks.clear();
+    }
 }
diff --git a/src/protocols/types/SurfaceState.hpp b/src/protocols/types/SurfaceState.hpp
index eb50a988..e6764eda 100644
--- a/src/protocols/types/SurfaceState.hpp
+++ b/src/protocols/types/SurfaceState.hpp
@@ -6,6 +6,7 @@
 
 class CTexture;
 class CDRMSyncPointState;
+class CWLCallbackResource;
 
 struct SSurfaceState {
     union {
@@ -21,6 +22,7 @@ struct SSurfaceState {
             bool viewport : 1;
             bool acquire : 1;
             bool acked : 1;
+            bool frame : 1;
         } bits;
     } updated;
 
@@ -41,6 +43,9 @@ struct SSurfaceState {
     // for xdg_shell resizing
     Vector2D ackedSize;
 
+    // for wl_surface::frame callbacks.
+    std::vector<SP<CWLCallbackResource>> callbacks;
+
     // viewporter protocol surface state
     struct {
         bool     hasDestination = false;
diff --git a/src/render/Texture.cpp b/src/render/Texture.cpp
index b9797751..17c416c7 100644
--- a/src/render/Texture.cpp
+++ b/src/render/Texture.cpp
@@ -111,6 +111,9 @@ void CTexture::createFromDma(const Aquamarine::SDMABUFAttrs& attrs, void* image)
 }
 
 void CTexture::update(uint32_t drmFormat, uint8_t* pixels, uint32_t stride, const CRegion& damage) {
+    if (damage.empty())
+        return;
+
     g_pHyprRenderer->makeEGLCurrent();
 
     const auto format = NFormatUtils::getPixelFormatFromDRM(drmFormat);