syncobj: use eventfd instead of stalling fd checks (#9437)

* syncobj: cleanup and use uniqueptrs

cleanup a bit missing removals if resource not good, erasing from
containers etc. make use of unique ptrs instead. and add default
destructors.

* syncobj: rework syncobj entirerly

remove early buffer release that was breaking explicit sync, the buffer
needs to exist until the surface commit event has been emitted and draw
calls added egl sync points, move to eventfd signaling instead of
stalling sync point checks, and recommit pending commits if waiting on a
signal. add a CDRMSyncPointState helper class. move a few weak pointers
to shared pointers so they dont destruct before we need to use them.

* syncobj: queue pending states for eventfd

eventfd requires us to queue pending stats until ready and then apply to
current, and also when no ready state exist commit the client commit on
the current existing buffer, if there is one.

* syncobj: clear current buffer damage

clear current buffer damage on current buffer commits.

* syncobj: cleanup code and fix hyprlock

remove unused code, and ensure we dont commit a empty texture causing
locksession protocol and gtk4-layer-shell misbehaving.

* syncobj: ensure buffers are cleaned up

ensure the containers having the various buffers actually gets cleaned
up from their containers, incase the CSignal isnt signaled because of
expired smart pointers or just wrong order destruction because mishaps.
also move the acquire/point setting to buffer attaching. instead of on
precommit.

* syncobj: remove unused code, optimize

remove unused code and merge sync fds if fence is valid, remove manual
directscanout buffer dropping that signals release point on pageflip, it
can cause us to signal the release point while still keeping the current
buffer and rendering it yet again causing wrong things.

* syncobj: delay buffer release on non syncobj

delay buffer releases on non syncobj surfaces until next commit, and
check on async buffers if syncobj and drop and signal the release point
on backend buffer release.

* syncobj: ensure we follow protocol

ensure we follow protocol by replacing acquire/release points if they
arrive late and replace already existing ones. also remove unneded
brackets, and dont try to manual lock/release buffers when it comes to
explicit protocol. it doesnt care about buffer releases only about
acquire and release points and signaling them.

* syncobj: lets not complicate things

set points in precommit, before checking protocol errors and we catch
any pending acquire/release points arriving late.

* syncobj: move SSurfaceState to types

remove destructor resource destroying, let resources destroys them on
their events, and move SSurfaceStates to types/SurfaceState.hpp

* syncobj: actually store the merged fd

have to actually store the mergedfd to use it.

* syncobj: cleanup a bit around fences

ensure the current asynchronous buffer is actually released on pageflip
not the previous. cleanup a bit FD handling in
commitPendingAndDoExplicitSync, and reuse the in fence when syncing
surfaces.

* syncobjs: ensure fence FD doesnt leak

calling resetexplicitfence without properly ensuring the FD is closed
before will leak it, store it per monitor and let it close itself with
the CFileDescriptor class.

* syncobj: ensure buffers are actually released

buffers were never being sent released properly.

* types: Defer buffer sync releaser until unlock

* syncobj: store directscanout fence in monitor

ensure the infence fd survives the scope of attemptdirectscanout so it
doesnt close before it should have.

* syncobj: check if if acquire is expired

we might hit a race to finish on exit where the timeline just has
destructed but the buffer waiter is still pending. and such we
removeAllWaiters null dereferences.

* syncobj: code style changes

remove quack comment, change to m_foo and use a std::vector and
weakpointer in the waiter for removal instead of a std::list.

* syncobj: remove unused async buffer drop

remove unused async buffer drop, only related to directscanout and is
handled elsewhere.

---------

Co-authored-by: Lee Bousfield <ljbousfield@gmail.com>

1 files changed, 160 insertions, 63 deletions

diff --git a/src/protocols/DRMSyncobj.cpp b/src/protocols/DRMSyncobj.cpp
index 38aab305..406b55e0 100644
--- a/src/protocols/DRMSyncobj.cpp
+++ b/src/protocols/DRMSyncobj.cpp
@@ -4,11 +4,82 @@
 #include "core/Compositor.hpp"
 #include "../helpers/sync/SyncTimeline.hpp"
 #include "../Compositor.hpp"
+#include "render/OpenGL.hpp"
 
 #include <fcntl.h>
 using namespace Hyprutils::OS;
 
-CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP<CWpLinuxDrmSyncobjSurfaceV1> resource_, SP<CWLSurfaceResource> surface_) : surface(surface_), resource(resource_) {
+CDRMSyncPointState::CDRMSyncPointState(WP<CDRMSyncobjTimelineResource> resource_, uint64_t point_, bool acquirePoint) :
+    m_resource(resource_), m_point(point_), m_acquirePoint(acquirePoint) {}
+
+const uint64_t& CDRMSyncPointState::point() {
+    return m_point;
+}
+
+WP<CDRMSyncobjTimelineResource> CDRMSyncPointState::resource() {
+    return m_resource;
+}
+
+WP<CSyncTimeline> CDRMSyncPointState::timeline() {
+    if (expired()) {
+        Debug::log(ERR, "CDRMSyncPointState: getting a timeline on a expired point");
+        return {};
+    }
+
+    return m_resource->timeline;
+}
+
+bool CDRMSyncPointState::expired() {
+    return !m_resource || !m_resource->timeline;
+}
+
+UP<CSyncReleaser> CDRMSyncPointState::createSyncRelease() {
+    if (expired()) {
+        Debug::log(ERR, "CDRMSyncPointState: creating a sync releaser on an expired point");
+        return nullptr;
+    }
+
+    if (m_releaseTaken)
+        Debug::log(ERR, "CDRMSyncPointState: creating a sync releaser on an already created SyncRelease");
+
+    m_releaseTaken = true;
+    return makeUnique<CSyncReleaser>(m_resource->timeline, m_point);
+}
+
+bool CDRMSyncPointState::addWaiter(const std::function<void()>& waiter) {
+    if (expired()) {
+        Debug::log(ERR, "CDRMSyncPointState: adding a waiter on an expired point");
+        return false;
+    }
+
+    m_acquireCommitted = true;
+    return m_resource->timeline->addWaiter(waiter, m_point, 0u);
+}
+
+bool CDRMSyncPointState::comitted() {
+    return m_acquireCommitted;
+}
+
+CFileDescriptor CDRMSyncPointState::exportAsFD() {
+    if (expired()) {
+        Debug::log(ERR, "CDRMSyncPointState: exporting a FD on an expired point");
+        return {};
+    }
+
+    return m_resource->timeline->exportAsSyncFileFD(m_point);
+}
+
+void CDRMSyncPointState::signal() {
+    if (expired()) {
+        Debug::log(ERR, "CDRMSyncPointState: signaling on an expired point");
+        return;
+    }
+
+    m_resource->timeline->signal(m_point);
+}
+
+CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UP<CWpLinuxDrmSyncobjSurfaceV1>&& resource_, SP<CWLSurfaceResource> surface_) :
+    surface(surface_), resource(std::move(resource_)) {
     if UNLIKELY (!good())
         return;
 
@@ -23,9 +94,8 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP<CWpLinuxDrmSyncobjSurf
             return;
         }
 
-        auto timeline           = CDRMSyncobjTimelineResource::fromResource(timeline_);
-        pending.acquireTimeline = timeline;
-        pending.acquirePoint    = ((uint64_t)hi << 32) | (uint64_t)lo;
+        auto timeline  = CDRMSyncobjTimelineResource::fromResource(timeline_);
+        pendingAcquire = {timeline, ((uint64_t)hi << 32) | (uint64_t)lo, true};
     });
 
     resource->setSetReleasePoint([this](CWpLinuxDrmSyncobjSurfaceV1* r, wl_resource* timeline_, uint32_t hi, uint32_t lo) {
@@ -34,77 +104,102 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP<CWpLinuxDrmSyncobjSurf
             return;
         }
 
-        auto timeline           = CDRMSyncobjTimelineResource::fromResource(timeline_);
-        pending.releaseTimeline = timeline;
-        pending.releasePoint    = ((uint64_t)hi << 32) | (uint64_t)lo;
+        auto timeline  = CDRMSyncobjTimelineResource::fromResource(timeline_);
+        pendingRelease = {timeline, ((uint64_t)hi << 32) | (uint64_t)lo, false};
     });
 
     listeners.surfacePrecommit = surface->events.precommit.registerListener([this](std::any d) {
-        if ((pending.acquireTimeline || pending.releaseTimeline) && !surface->pending.texture) {
-            resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_BUFFER, "Missing buffer");
-            surface->pending.rejected = true;
-            return;
+        if (!surface->pending.buffer && surface->pending.newBuffer && !surface->pending.texture) {
+            removeAllWaiters();
+            surface->commitPendingState(surface->pending);
+            return; // null buffer attached.
         }
 
-        if (!surface->pending.newBuffer)
-            return; // this commit does not change the state here
-
-        if (!!pending.acquireTimeline != !!pending.releaseTimeline) {
-            resource->error(pending.acquireTimeline ? WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT : WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT,
-                            "Missing timeline");
-            surface->pending.rejected = true;
-            return;
+        if (!surface->pending.buffer && !surface->pending.newBuffer && surface->current.buffer) {
+            surface->current.bufferDamage.clear();
+            surface->current.damage.clear();
+            surface->commitPendingState(surface->current);
+            return; // no new buffer, but we still have current around and a commit happend, commit current again.
         }
 
-        if (!pending.acquireTimeline)
+        if (!surface->pending.buffer && !surface->pending.newBuffer && !surface->current.buffer) {
+            surface->commitPendingState(surface->pending); // no pending buffer, no current buffer. probably first commit
             return;
+        }
 
-        if (pending.acquireTimeline && pending.releaseTimeline && pending.acquireTimeline == pending.releaseTimeline) {
-            if (pending.acquirePoint >= pending.releasePoint) {
-                resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_CONFLICTING_POINTS, "Acquire and release points are on the same timeline, and acquire >= release");
-                surface->pending.rejected = true;
-                return;
-            }
+        if (!pendingAcquire.expired()) {
+            surface->pending.buffer->acquire = makeUnique<CDRMSyncPointState>(std::move(pendingAcquire));
+            pendingAcquire                   = {};
         }
 
-        // wait for the acquire timeline to materialize
-        auto materialized = pending.acquireTimeline->timeline->check(pending.acquirePoint, DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE);
-        if (!materialized.has_value()) {
-            LOGM(ERR, "Failed to check the acquire timeline");
-            resource->noMemory();
-            return;
+        if (!pendingRelease.expired()) {
+            surface->pending.buffer->release = makeUnique<CDRMSyncPointState>(std::move(pendingRelease));
+            pendingRelease                   = {};
         }
 
-        if (materialized.value())
+        if (protocolError())
             return;
 
-        surface->lockPendingState();
-        pending.acquireTimeline->timeline->addWaiter([this]() { surface->unlockPendingState(); }, pending.acquirePoint, DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE);
+        const auto& state = pendingStates.emplace_back(makeShared<SSurfaceState>(surface->pending));
+        surface->pending.damage.clear();
+        surface->pending.bufferDamage.clear();
+        surface->pending.newBuffer = false;
+        surface->pending.buffer.reset();
+
+        state->buffer->buffer->syncReleaser = state->buffer->release->createSyncRelease();
+        state->buffer->acquire->addWaiter([this, surf = surface, wp = CWeakPointer<SSurfaceState>(*std::prev(pendingStates.end()))] {
+            if (!surf)
+                return;
+
+            surf->commitPendingState(*wp.lock());
+            std::erase(pendingStates, wp);
+        });
     });
+}
 
-    listeners.surfaceCommit = surface->events.roleCommit.registerListener([this](std::any d) {
-        // apply timelines if new ones have been attached, otherwise don't touch
-        // the current ones
-        if (pending.releaseTimeline) {
-            current.releaseTimeline = pending.releaseTimeline;
-            current.releasePoint    = pending.releasePoint;
-        }
+void CDRMSyncobjSurfaceResource::removeAllWaiters() {
+    for (auto& s : pendingStates) {
+        if (s && s->buffer && s->buffer->acquire && !s->buffer->acquire->expired())
+            s->buffer->acquire->resource()->timeline->removeAllWaiters();
+    }
 
-        if (pending.acquireTimeline) {
-            current.acquireTimeline = pending.acquireTimeline;
-            current.acquirePoint    = pending.acquirePoint;
+    pendingStates.clear();
+}
+
+CDRMSyncobjSurfaceResource::~CDRMSyncobjSurfaceResource() {
+    removeAllWaiters();
+}
+
+bool CDRMSyncobjSurfaceResource::protocolError() {
+    if (!surface->pending.texture) {
+        resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_BUFFER, "Missing buffer");
+        surface->pending.rejected = true;
+        return true;
+    }
+
+    if (!!surface->pending.buffer->acquire != !!surface->pending.buffer->release) {
+        resource->error(surface->pending.buffer->acquire ? WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT : WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT,
+                        "Missing timeline");
+        surface->pending.rejected = true;
+        return true;
+    }
+
+    if (surface->pending.buffer->acquire->timeline() == surface->pending.buffer->release->timeline()) {
+        if (surface->pending.buffer->acquire->point() >= surface->pending.buffer->release->point()) {
+            resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_CONFLICTING_POINTS, "Acquire and release points are on the same timeline, and acquire >= release");
+            surface->pending.rejected = true;
+            return true;
         }
+    }
 
-        pending.releaseTimeline.reset();
-        pending.acquireTimeline.reset();
-    });
+    return false;
 }
 
 bool CDRMSyncobjSurfaceResource::good() {
     return resource->resource();
 }
 
-CDRMSyncobjTimelineResource::CDRMSyncobjTimelineResource(SP<CWpLinuxDrmSyncobjTimelineV1> resource_, CFileDescriptor&& fd_) : fd(std::move(fd_)), resource(resource_) {
+CDRMSyncobjTimelineResource::CDRMSyncobjTimelineResource(UP<CWpLinuxDrmSyncobjTimelineV1>&& resource_, CFileDescriptor&& fd_) : fd(std::move(fd_)), resource(std::move(resource_)) {
     if UNLIKELY (!good())
         return;
 
@@ -121,16 +216,20 @@ CDRMSyncobjTimelineResource::CDRMSyncobjTimelineResource(SP<CWpLinuxDrmSyncobjTi
     }
 }
 
-SP<CDRMSyncobjTimelineResource> CDRMSyncobjTimelineResource::fromResource(wl_resource* res) {
-    auto data = (CDRMSyncobjTimelineResource*)(((CWpLinuxDrmSyncobjTimelineV1*)wl_resource_get_user_data(res))->data());
-    return data ? data->self.lock() : nullptr;
+WP<CDRMSyncobjTimelineResource> CDRMSyncobjTimelineResource::fromResource(wl_resource* res) {
+    for (const auto& r : PROTO::sync->m_vTimelines) {
+        if (r && r->resource && r->resource->resource() == res)
+            return r;
+    }
+
+    return {};
 }
 
 bool CDRMSyncobjTimelineResource::good() {
     return resource->resource();
 }
 
-CDRMSyncobjManagerResource::CDRMSyncobjManagerResource(SP<CWpLinuxDrmSyncobjManagerV1> resource_) : resource(resource_) {
+CDRMSyncobjManagerResource::CDRMSyncobjManagerResource(UP<CWpLinuxDrmSyncobjManagerV1>&& resource_) : resource(std::move(resource_)) {
     if UNLIKELY (!good())
         return;
 
@@ -154,28 +253,28 @@ CDRMSyncobjManagerResource::CDRMSyncobjManagerResource(SP<CWpLinuxDrmSyncobjMana
             return;
         }
 
-        auto RESOURCE = makeShared<CDRMSyncobjSurfaceResource>(makeShared<CWpLinuxDrmSyncobjSurfaceV1>(resource->client(), resource->version(), id), SURF);
+        const auto& RESOURCE = PROTO::sync->m_vSurfaces.emplace_back(
+            makeUnique<CDRMSyncobjSurfaceResource>(makeUnique<CWpLinuxDrmSyncobjSurfaceV1>(resource->client(), resource->version(), id), SURF));
         if UNLIKELY (!RESOURCE->good()) {
             resource->noMemory();
+            PROTO::sync->m_vSurfaces.pop_back();
             return;
         }
 
-        PROTO::sync->m_vSurfaces.emplace_back(RESOURCE);
         SURF->syncobj = RESOURCE;
 
         LOGM(LOG, "New linux_syncobj at {:x} for surface {:x}", (uintptr_t)RESOURCE.get(), (uintptr_t)SURF.get());
     });
 
     resource->setImportTimeline([this](CWpLinuxDrmSyncobjManagerV1* r, uint32_t id, int32_t fd) {
-        auto RESOURCE = makeShared<CDRMSyncobjTimelineResource>(makeShared<CWpLinuxDrmSyncobjTimelineV1>(resource->client(), resource->version(), id), CFileDescriptor{fd});
+        const auto& RESOURCE = PROTO::sync->m_vTimelines.emplace_back(
+            makeUnique<CDRMSyncobjTimelineResource>(makeUnique<CWpLinuxDrmSyncobjTimelineV1>(resource->client(), resource->version(), id), CFileDescriptor{fd}));
         if UNLIKELY (!RESOURCE->good()) {
             resource->noMemory();
+            PROTO::sync->m_vTimelines.pop_back();
             return;
         }
 
-        PROTO::sync->m_vTimelines.emplace_back(RESOURCE);
-        RESOURCE->self = RESOURCE;
-
         LOGM(LOG, "New linux_drm_timeline at {:x}", (uintptr_t)RESOURCE.get());
     });
 }
@@ -184,12 +283,10 @@ bool CDRMSyncobjManagerResource::good() {
     return resource->resource();
 }
 
-CDRMSyncobjProtocol::CDRMSyncobjProtocol(const wl_interface* iface, const int& ver, const std::string& name) : IWaylandProtocol(iface, ver, name) {
-    drmFD = g_pCompositor->m_iDRMFD;
-}
+CDRMSyncobjProtocol::CDRMSyncobjProtocol(const wl_interface* iface, const int& ver, const std::string& name) : IWaylandProtocol(iface, ver, name), drmFD(g_pCompositor->m_iDRMFD) {}
 
 void CDRMSyncobjProtocol::bindManager(wl_client* client, void* data, uint32_t ver, uint32_t id) {
-    const auto RESOURCE = m_vManagers.emplace_back(makeShared<CDRMSyncobjManagerResource>(makeShared<CWpLinuxDrmSyncobjManagerV1>(client, ver, id)));
+    const auto& RESOURCE = m_vManagers.emplace_back(makeUnique<CDRMSyncobjManagerResource>(makeUnique<CWpLinuxDrmSyncobjManagerV1>(client, ver, id)));
 
     if UNLIKELY (!RESOURCE->good()) {
         wl_client_post_no_memory(client);