protocols/presentation: associate feedback with committed surface state, attempt 2 (#15459)

* Revert "presentation: revert "associate feedback with committed surface state (#15243)""

This reverts commit 4a753ea.

* presentation: fix WP locks and discard failed draws

the reason it stopped working is last change that PR
did was moving from raw ptrs to UP's with WP's but tried to .lock() the
WP, we cant lock a WP that points to a UP.

also ensure all draws that isnt visible or simply failed creating
texture or invalid sizes discard in elementrenderer. and all others
present.

Author: gulafaran

src/protocols/PresentationTime.cpp

@@ -6,7 +6,8 @@
 #include "core/Output.hpp"
 #include <aquamarine/output/Output.hpp>
 
-CQueuedPresentationData::CQueuedPresentationData(SP surf) : m_surface(surf) {
+CQueuedPresentationData::CQueuedPresentationData(SP surf, std::vector<WP> feedbacks) :
+    m_surface(surf), m_feedbacks(std::move(feedbacks)) {
     ;
 }
 
@@ -76,9 +77,23 @@ void CPresentationFeedback::sendQueued(WP data, const t
     m_done = true;
 }
 
+void CPresentationFeedback::sendDiscarded() {
+    if (m_done)
+        return;
+
+    m_resource->sendDiscarded();
+    m_done = true;
+}
+
 CPresentationProtocol::CPresentationProtocol(const wl_interface* iface, const int& ver, const std::string& name) : IWaylandProtocol(iface, ver, name) {
-    static auto P = Event::bus()->m_events.monitor.removed.listen(
-        [this](PHLMONITOR mon) { std::erase_if(m_queue, [mon](const auto& other) { return !other->m_surface || other->m_monitor == mon; }); });
+    static auto P = Event::bus()->m_events.monitor.removed.listen([this](PHLMONITOR mon) {
+        for (auto& data : m_queue) {
+            if (!data->m_surface || data->m_monitor == mon)
+                discardFeedbacks(data->m_feedbacks);
+        }
+
+        std::erase_if(m_queue, [mon](const auto& other) { return !other->m_surface || other->m_monitor == mon; });
+    });
 }
 
 void CPresentationProtocol::bindManager(wl_client* client, void* data, uint32_t ver, uint32_t id) {
@@ -95,33 +110,40 @@ void CPresentationProtocol::onManagerResourceDestroy(wl_resource* res) {
 }
 
 void CPresentationProtocol::destroyResource(CPresentationFeedback* feedback) {
+    feedback->m_done = true;
     std::erase_if(m_feedbacks, [&](const auto& other) { return other.get() == feedback; });
 }
 
 void CPresentationProtocol::onGetFeedback(CWpPresentation* pMgr, wl_resource* surf, uint32_t id) {
     const auto  CLIENT = pMgr->client();
     const auto& RESOURCE =
-        m_feedbacks.emplace_back(makeUnique(makeUnique(CLIENT, pMgr->version(), id), CWLSurfaceResource::fromResource(surf))).get();
+        m_feedbacks.emplace_back(makeUnique(makeUnique(CLIENT, pMgr->version(), id), CWLSurfaceResource::fromResource(surf)));
 
     if UNLIKELY (!RESOURCE->good()) {
         pMgr->noMemory();
         m_feedbacks.pop_back();
         return;
     }
+
+    if (const auto SURFACE = CWLSurfaceResource::fromResource(surf); SURFACE)
+        SURFACE->m_pending.presentationFeedbacks.emplace_back(RESOURCE);
 }
 
 void CPresentationProtocol::onPresented(PHLMONITOR pMonitor, const timespec& when, uint32_t untilRefreshNs, uint64_t seq, uint32_t reportedFlags) {
-    for (auto const& feedback : m_feedbacks) {
-        if (!feedback->m_surface)
+    for (auto const& data : m_queue) {
+        if (!data->m_surface || !data->m_monitor) {
+            discardFeedbacks(data->m_feedbacks);
             continue;
+        }
 
-        for (auto const& data : m_queue) {
-            if (!data->m_surface || data->m_surface != feedback->m_surface || (data->m_monitor && data->m_monitor != pMonitor))
+        if (data->m_monitor != pMonitor)
+            continue;
+
+        for (auto const& feedback : data->m_feedbacks) {
+            if (!feedback || feedback->m_done)
                 continue;
 
             feedback->sendQueued(data, when, untilRefreshNs, seq, reportedFlags);
-            feedback->m_done = true;
-            break;
         }
     }
 
@@ -147,6 +169,33 @@ void CPresentationProtocol::queueData(UP&& data) {
     m_queue.emplace_back(std::move(data));
 }
 
+void CPresentationProtocol::discardFeedbacks(std::vector<WP>& feedbacks) {
+    for (auto const& feedback : feedbacks) {
+        if (!feedback || feedback->m_done)
+            continue;
+
+        feedback->sendDiscarded();
+    }
+
+    feedbacks.clear();
+    std::erase_if(m_feedbacks, [](const auto& other) { return !other->m_surface || other->m_done; });
+}
+
+void CPresentationProtocol::discardFeedbacksForSurface(WP surface) {
+    if (!surface)
+        return;
+
+    for (auto const& feedback : m_feedbacks) {
+        if (feedback->m_surface != surface)
+            continue;
+
+        feedback->sendDiscarded();
+    }
+
+    std::erase_if(m_queue, [surface](const auto& other) { return !other->m_surface || other->m_surface == surface; });
+    std::erase_if(m_feedbacks, [](const auto& other) { return !other->m_surface || other->m_done; });
+}
+
 bool CPresentationProtocol::hasPendingFeedbacks() const {
     return !m_feedbacks.empty();
 }

src/protocols/PresentationTime.hpp

@@ -8,10 +8,11 @@
 #include "../helpers/time/Time.hpp"
 
 class CWLSurfaceResource;
+class CPresentationFeedback;
 
 class CQueuedPresentationData {
   public:
-    CQueuedPresentationData(SP surf);
+    CQueuedPresentationData(SP surf, std::vector<WP> feedbacks);
 
     void setPresentationType(bool zeroCopy);
     void attachMonitor(PHLMONITOR pMonitor);
@@ -22,10 +23,11 @@ class CQueuedPresentationData {
     bool m_done = false;
 
   private:
-    bool                   m_wasPresented = false;
-    bool                   m_zeroCopy     = false;
-    PHLMONITORREF          m_monitor;
-    WP m_surface;
+    bool                                   m_wasPresented = false;
+    bool                                   m_zeroCopy     = false;
+    PHLMONITORREF                          m_monitor;
+    WP                 m_surface;
+    std::vector<WP> m_feedbacks;
 
     friend class CPresentationFeedback;
     friend class CPresentationProtocol;
@@ -38,6 +40,7 @@ class CPresentationFeedback {
     bool good();
 
     void sendQueued(WP data, const timespec& when, uint32_t untilRefreshNs, uint64_t seq, uint32_t reportedFlags);
+    void sendDiscarded();
 
   private:
     UP m_resource;
@@ -55,6 +58,8 @@ class CPresentationProtocol : public IWaylandProtocol {
 
     void         onPresented(PHLMONITOR pMonitor, const timespec& when, uint32_t untilRefreshNs, uint64_t seq, uint32_t reportedFlags);
     void         queueData(UP&& data);
+    void         discardFeedbacks(std::vector<WP>& feedbacks);
+    void         discardFeedbacksForSurface(WP surface);
     bool         hasPendingFeedbacks() const;
 
   private:

src/protocols/core/Compositor.cpp

@@ -146,6 +146,7 @@ CWLSurfaceResource::CWLSurfaceResource(SP resource_) : m_resource(re
         m_events.precommit.emit();
         if (m_pending.rejected) {
             m_pending.rejected = false;
+            PROTO::presentation->discardFeedbacks(m_pending.presentationFeedbacks);
             dropPendingBuffer();
             return;
         }
@@ -253,10 +254,19 @@ CWLSurfaceResource::CWLSurfaceResource(SP resource_) : m_resource(re
 }
 
 CWLSurfaceResource::~CWLSurfaceResource() {
+    discardPresentationFeedbacks();
     m_events.destroy.emit();
 }
 
+void CWLSurfaceResource::discardPresentationFeedbacks() {
+    PROTO::presentation->discardFeedbacks(m_pending.presentationFeedbacks);
+    PROTO::presentation->discardFeedbacks(m_current.presentationFeedbacks);
+    PROTO::presentation->discardFeedbacksForSurface(m_self);
+}
+
 void CWLSurfaceResource::destroy() {
+    discardPresentationFeedbacks();
+
     if (m_mapped) {
         m_events.unmap.emit();
         unmap();
@@ -514,6 +524,10 @@ void CWLSurfaceResource::unmap() {
     if UNLIKELY (!m_mapped)
         return;
 
+    // unmapped content will never be displayed: terminate outstanding feedbacks,
+    // or clients blocking on them (present_wait) stall forever.
+    discardPresentationFeedbacks();
+
     m_mapped        = false;
     m_lastTransform = std::nullopt;
     m_lastScale     = std::nullopt;
@@ -607,6 +621,9 @@ void CWLSurfaceResource::commitState(SSurfaceState& state) {
     if (!state.updated.all && m_mapped && state.fifoScheduled)
         return;
 
+    if (state.updated.all)
+        PROTO::presentation->discardFeedbacks(m_current.presentationFeedbacks);
+
     auto lastTexture = m_current.texture;
     m_current.updateFrom(state);
 
@@ -777,7 +794,7 @@ void CWLSurfaceResource::updateCursorShm(CRegion damage) {
 void CWLSurfaceResource::presentFeedback(const Time::steady_tp& when, PHLMONITOR pMonitor, bool discarded) {
     frame(when);
 
-    auto FEEDBACK = makeUnique(m_self.lock());
+    auto FEEDBACK = makeUnique(m_self.lock(), std::move(m_current.presentationFeedbacks));
     FEEDBACK->attachMonitor(pMonitor);
     if (discarded)
         FEEDBACK->discarded();

src/protocols/core/Compositor.hpp

@@ -145,6 +145,7 @@ class CWLSurfaceResource {
     void                               releaseBuffers(bool onlyCurrent = true);
     void                               dropPendingBuffer();
     void                               dropCurrentBuffer();
+    void                               discardPresentationFeedbacks();
     void                               bfHelper(std::span<const SP> nodes, std::function<void(SP, const Vector2D&, void*)> fn, void* data);
     SP             findFirstPreorderHelper(SP root, std::function<bool(SP)> fn);
     void                               updateCursorShm(CRegion damage = CBox{0, 0, INT16_MAX, INT16_MAX});

src/protocols/types/SurfaceState.cpp

@@ -73,6 +73,7 @@ void SSurfaceState::reset() {
     bufferDamage.clear();
 
     callbacks.clear();
+    presentationFeedbacks.clear();
     lockMask = LOCK_REASON_NONE;
 
     barrierSet    = false;
@@ -134,6 +135,12 @@ void SSurfaceState::updateFrom(SSurfaceState& ref) {
         ref.callbacks.clear();
     }
 
+    if (!ref.presentationFeedbacks.empty()) {
+        presentationFeedbacks.insert(presentationFeedbacks.end(), std::make_move_iterator(ref.presentationFeedbacks.begin()),
+                                     std::make_move_iterator(ref.presentationFeedbacks.end()));
+        ref.presentationFeedbacks.clear();
+    }
+
     if (ref.barrierSet)
         barrierSet = ref.barrierSet;
 }

src/protocols/types/SurfaceState.hpp

@@ -11,6 +11,7 @@ namespace Render {
 }
 class CDRMSyncPointState;
 class CWLCallbackResource;
+class CPresentationFeedback;
 
 enum eLockReason : uint8_t {
     LOCK_REASON_NONE  = 0,
@@ -77,6 +78,9 @@ struct SSurfaceState {
     // for wl_surface::frame callbacks.
     std::vector<SP> callbacks;
 
+    // for wp_presentation feedbacks, tied to this commit.
+    std::vector<WP> presentationFeedbacks;
+
     // viewporter protocol surface state
     struct {
         bool     hasDestination = false;

src/protocols/types/SurfaceStateQueue.cpp

@@ -1,10 +1,14 @@
 #include "SurfaceStateQueue.hpp"
 #include "../core/Compositor.hpp"
+#include "../PresentationTime.hpp"
 #include "SurfaceState.hpp"
 
 CSurfaceStateQueue::CSurfaceStateQueue(WP surf) : m_surface(std::move(surf)) {}
 
 void CSurfaceStateQueue::clear() {
+    for (auto& state : m_queue)
+        PROTO::presentation->discardFeedbacks(state->presentationFeedbacks);
+
     m_queue.clear();
 }
 
@@ -17,6 +21,8 @@ void CSurfaceStateQueue::dropState(const WP& state) {
     if (it == m_queue.end())
         return;
 
+    PROTO::presentation->discardFeedbacks((*it)->presentationFeedbacks);
+
     m_queue.erase(it);
 }
 

src/render/ElementRenderer.cpp

@@ -225,15 +225,19 @@ void IElementRenderer::drawSurface(WP element, const CRegio
         g_pHyprRenderer->m_renderData.primarySurfaceUVBottomRight = Vector2D(-1, -1);
     }};
 
-    if (!m_data.texture)
+    if (!m_data.texture) {
+        element->discard();
         return;
+    }
 
     const auto& TEXTURE = m_data.texture;
 
     // this is bad, probably has been logged elsewhere. Means the texture failed
     // uploading to the GPU.
-    if (!TEXTURE->ok())
+    if (!TEXTURE->ok()) {
+        element->discard();
         return;
+    }
 
     const auto INTERACTIVERESIZEINPROGRESS = m_data.pWindow && g_layoutManager->dragController()->target() && g_layoutManager->dragController()->mode() == MBIND_RESIZE;
     TRACY_GPU_ZONE("RenderSurface");
@@ -266,8 +270,10 @@ void IElementRenderer::drawSurface(WP element, const CRegio
 
     auto cancelRender = false;
     auto clipRegion   = element->visibleRegion(cancelRender);
-    if (cancelRender)
+    if (cancelRender) {
+        element->discard();
         return;
+    }
 
     // check for fractional scale surfaces misaligning the buffer size
     // in those cases it's better to just force nearest neighbor
@@ -380,6 +386,9 @@ void IElementRenderer::drawSurface(WP element, const CRegio
     }
 
     g_pHyprRenderer->blend(true);
+
+    if (!g_pHyprRenderer->m_bBlockSurfaceFeedback)
+        element->m_data.surface->presentFeedback(element->m_data.when, element->m_data.pMonitor->m_self.lock());
 };
 
 void IElementRenderer::preDrawSurface(WP element, const CRegion& damage) {
@@ -391,9 +400,6 @@ void IElementRenderer::preDrawSurface(WP element, const CRe
 
     drawSurface(element, damage);
 
-    if (!g_pHyprRenderer->m_bBlockSurfaceFeedback)
-        element->m_data.surface->presentFeedback(element->m_data.when, element->m_data.pMonitor->m_self.lock());
-
     // add async (dmabuf) buffers to usedBuffers so we can handle release later
     // sync (shm) buffers will be released in commitState, so no need to track them here
     if (element->m_data.surface->m_current.buffer && !element->m_data.surface->m_current.buffer->isSynchronous())