core: wait for dmabuf readiness (hyprwm#9806)

* add doOnReadable to event loop manager

* move syncTimeline addWaiter to doOnReadable

* wait on dmabuf buffers to be readable

* don't over synchronize in scanout, also give present feedback on same buffer commit

Author: ikalco

src/helpers/Monitor.cpp

@@ -1377,6 +1377,10 @@ bool CMonitor::attemptDirectScanout() {
     auto PBUFFER = PSURFACE->current.buffer.buffer;
 
     if (PBUFFER == output->state->state().buffer) {
+        timespec now;
+        clock_gettime(CLOCK_MONOTONIC, &now);
+        PSURFACE->presentFeedback(&now, self.lock());
+
         if (scanoutNeedsCursorUpdate) {
             if (!state.test()) {
                 Debug::log(TRACE, "attemptDirectScanout: failed basic test");
@@ -1423,32 +1427,10 @@ bool CMonitor::attemptDirectScanout() {
     output->state->addDamage(PSURFACE->current.accumulateBufferDamage());
     output->state->resetExplicitFences();
 
-    auto cleanup = CScopeGuard([this]() { output->state->resetExplicitFences(); });
-
-    auto explicitOptions = g_pHyprRenderer->getExplicitSyncSettings(output);
-
-    bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->current.buffer && PSURFACE->current.acquire && explicitOptions.explicitKMSEnabled;
-    if (DOEXPLICIT) {
-        // wait for surface's explicit fence if present
-        inFence = PSURFACE->current.acquire.exportAsFD();
-        if (inFence.isValid()) {
-            Debug::log(TRACE, "attemptDirectScanout: setting IN_FENCE for aq to {}", inFence.get());
-            output->state->setExplicitInFence(inFence.get());
-        } else {
-            Debug::log(TRACE, "attemptDirectScanout: failed to acquire an sync file fd for aq IN_FENCE");
-            DOEXPLICIT = false;
-        }
-    }
+    // no need to do explicit sync here as surface current can only ever be ready to read
 
     bool ok = output->commit();
 
-    if (!ok && DOEXPLICIT) {
-        Debug::log(TRACE, "attemptDirectScanout: EXPLICIT SYNC FAILED: commit() returned false. Resetting fences and retrying, might result in glitches.");
-        output->state->resetExplicitFences();
-
-        ok = output->commit();
-    }
-
     if (!ok) {
         Debug::log(TRACE, "attemptDirectScanout: failed to scanout surface");
         lastScanout.reset();

src/helpers/sync/SyncTimeline.cpp

@@ -34,13 +34,6 @@ SP<CSyncTimeline> CSyncTimeline::create(int drmFD_, CFileDescriptor&& drmSyncobj
 }
 
 CSyncTimeline::~CSyncTimeline() {
-    for (auto& w : waiters) {
-        if (w->source) {
-            wl_event_source_remove(w->source);
-            w->source = nullptr;
-        }
-    }
-
     if (handle == 0)
         return;
 
@@ -64,34 +57,8 @@ std::optional<bool> CSyncTimeline::check(uint64_t point, uint32_t flags) {
     return ret == 0;
 }
 
-static int handleWaiterFD(int fd, uint32_t mask, void* data) {
-    auto waiter = (CSyncTimeline::SWaiter*)data;
-
-    if (mask & (WL_EVENT_HANGUP | WL_EVENT_ERROR)) {
-        Debug::log(ERR, "handleWaiterFD: eventfd error");
-        return 0;
-    }
-
-    if (mask & WL_EVENT_READABLE) {
-        uint64_t value = 0;
-        if (read(fd, &value, sizeof(value)) <= 0)
-            Debug::log(ERR, "handleWaiterFD: failed to read from eventfd");
-    }
-
-    wl_event_source_remove(waiter->source);
-    waiter->source = nullptr;
-
-    if (waiter->fn)
-        waiter->fn();
-
-    if (waiter->timeline)
-        waiter->timeline->removeWaiter(waiter);
-
-    return 0;
-}
-
 bool CSyncTimeline::addWaiter(const std::function<void()>& waiter, uint64_t point, uint32_t flags) {
-    CFileDescriptor eventFd = CFileDescriptor{eventfd(0, EFD_CLOEXEC)};
+    auto eventFd = CFileDescriptor(eventfd(0, EFD_CLOEXEC));
 
     if (!eventFd.isValid()) {
         Debug::log(ERR, "CSyncTimeline::addWaiter: failed to acquire an eventfd");
@@ -103,46 +70,11 @@ bool CSyncTimeline::addWaiter(const std::function<void()>& waiter, uint64_t poin
         return false;
     }
 
-    if (eventFd.isReadable()) {
-        waiter();
-        return true;
-    }
-
-    auto w      = makeShared<SWaiter>();
-    w->fn       = waiter;
-    w->timeline = self;
-    w->eventFd  = std::move(eventFd);
-
-    w->source = wl_event_loop_add_fd(g_pEventLoopManager->m_sWayland.loop, w->eventFd.get(), WL_EVENT_READABLE, ::handleWaiterFD, w.get());
-    if (!w->source) {
-        Debug::log(ERR, "CSyncTimeline::addWaiter: wl_event_loop_add_fd failed");
-        return false;
-    }
-
-    waiters.emplace_back(w);
+    g_pEventLoopManager->doOnReadable(std::move(eventFd), waiter);
 
     return true;
 }
 
-void CSyncTimeline::removeWaiter(SWaiter* w) {
-    if (w->source) {
-        wl_event_source_remove(w->source);
-        w->source = nullptr;
-    }
-    std::erase_if(waiters, [w](const auto& e) { return e.get() == w; });
-}
-
-void CSyncTimeline::removeAllWaiters() {
-    for (auto& w : waiters) {
-        if (w->source) {
-            wl_event_source_remove(w->source);
-            w->source = nullptr;
-        }
-    }
-
-    waiters.clear();
-}
-
 CFileDescriptor CSyncTimeline::exportAsSyncFileFD(uint64_t src) {
     int      sync = -1;
 

src/helpers/sync/SyncTimeline.hpp

@@ -20,21 +20,12 @@ class CSyncTimeline {
     static SP<CSyncTimeline> create(int drmFD_, Hyprutils::OS::CFileDescriptor&& drmSyncobjFD);
     ~CSyncTimeline();
 
-    struct SWaiter {
-        std::function<void()>          fn;
-        wl_event_source*               source = nullptr;
-        WP<CSyncTimeline>              timeline;
-        Hyprutils::OS::CFileDescriptor eventFd;
-    };
-
     // check if the timeline point has been signaled
     // flags: DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT or DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
     // std::nullopt on fail
     std::optional<bool>            check(uint64_t point, uint32_t flags);
 
     bool                           addWaiter(const std::function<void()>& waiter, uint64_t point, uint32_t flags);
-    void                           removeWaiter(SWaiter*);
-    void                           removeAllWaiters();
     Hyprutils::OS::CFileDescriptor exportAsSyncFileFD(uint64_t src);
     bool                           importFromSyncFileFD(uint64_t dst, Hyprutils::OS::CFileDescriptor& fd);
     bool                           transfer(SP<CSyncTimeline> from, uint64_t fromPoint, uint64_t toPoint);
@@ -47,6 +38,4 @@ class CSyncTimeline {
 
   private:
     CSyncTimeline() = default;
-
-    std::vector<SP<SWaiter>> waiters;
 };

src/managers/eventLoop/EventLoopManager.cpp

@@ -26,6 +26,11 @@ CEventLoopManager::~CEventLoopManager() {
         wl_event_source_remove(eventSourceData.eventSource);
     }
 
+    for (auto const& w : m_vReadableWaiters) {
+        if (w->source != nullptr)
+            wl_event_source_remove(w->source);
+    }
+
     if (m_sWayland.eventSource)
         wl_event_source_remove(m_sWayland.eventSource);
     if (m_sIdle.eventSource)
@@ -50,6 +55,33 @@ static int configWatcherWrite(int fd, uint32_t mask, void* data) {
     return 0;
 }
 
+static int handleWaiterFD(int fd, uint32_t mask, void* data) {
+    if (mask & (WL_EVENT_HANGUP | WL_EVENT_ERROR)) {
+        Debug::log(ERR, "handleWaiterFD: readable waiter error");
+        return 0;
+    }
+
+    if (mask & WL_EVENT_READABLE)
+        g_pEventLoopManager->onFdReadable((CEventLoopManager::SReadableWaiter*)data);
+
+    return 0;
+}
+
+void CEventLoopManager::onFdReadable(SReadableWaiter* waiter) {
+    auto it = std::ranges::find_if(m_vReadableWaiters, [waiter](const UP<SReadableWaiter>& w) { return waiter == w.get() && w->fd == waiter->fd && w->source == waiter->source; });
+
+    if (waiter->source) {
+        wl_event_source_remove(waiter->source);
+        waiter->source = nullptr;
+    }
+
+    if (waiter->fn)
+        waiter->fn();
+
+    if (it != m_vReadableWaiters.end())
+        m_vReadableWaiters.erase(it);
+}
+
 void CEventLoopManager::enterLoop() {
     m_sWayland.eventSource = wl_event_loop_add_fd(m_sWayland.loop, m_sTimers.timerfd.get(), WL_EVENT_READABLE, timerWrite, nullptr);
 
@@ -143,6 +175,16 @@ void CEventLoopManager::doLater(const std::function<void()>& fn) {
         &m_sIdle);
 }
 
+void CEventLoopManager::doOnReadable(CFileDescriptor fd, const std::function<void()>& fn) {
+    if (!fd.isValid() || fd.isReadable()) {
+        fn();
+        return;
+    }
+
+    auto& waiter   = m_vReadableWaiters.emplace_back(makeUnique<SReadableWaiter>(nullptr, std::move(fd), fn));
+    waiter->source = wl_event_loop_add_fd(g_pEventLoopManager->m_sWayland.loop, waiter->fd.get(), WL_EVENT_READABLE, ::handleWaiterFD, waiter.get());
+}
+
 void CEventLoopManager::syncPollFDs() {
     auto aqPollFDs = g_pCompositor->m_pAqBackend->getPollFDs();
 

src/managers/eventLoop/EventLoopManager.hpp

@@ -38,6 +38,17 @@ class CEventLoopManager {
         std::vector<std::function<void()>> fns;
     };
 
+    struct SReadableWaiter {
+        wl_event_source*               source;
+        Hyprutils::OS::CFileDescriptor fd;
+        std::function<void()>          fn;
+    };
+
+    // schedule function to when fd is readable (WL_EVENT_READABLE / POLLIN),
+    // takes ownership of fd
+    void doOnReadable(Hyprutils::OS::CFileDescriptor fd, const std::function<void()>& fn);
+    void onFdReadable(SReadableWaiter* waiter);
+
   private:
     // Manages the event sources after AQ pollFDs change.
     void syncPollFDs();
@@ -58,16 +69,16 @@ class CEventLoopManager {
         Hyprutils::OS::CFileDescriptor   timerfd;
     } m_sTimers;
 
-    SIdleData                       m_sIdle;
-    std::map<int, SEventSourceData> aqEventSources;
+    SIdleData                        m_sIdle;
+    std::map<int, SEventSourceData>  aqEventSources;
+    std::vector<UP<SReadableWaiter>> m_vReadableWaiters;
 
     struct {
         CHyprSignalListener pollFDsChanged;
     } m_sListeners;
 
     wl_event_source* m_configWatcherInotifySource = nullptr;
 
-    friend class CSyncTimeline;
     friend class CAsyncDialogBox;
 };
 

src/protocols/core/Compositor.cpp

@@ -10,8 +10,10 @@
 #include "../../helpers/sync/SyncReleaser.hpp"
 #include "../PresentationTime.hpp"
 #include "../DRMSyncobj.hpp"
+#include "../types/DMABuffer.hpp"
 #include "../../render/Renderer.hpp"
 #include "config/ConfigValue.hpp"
+#include "../../managers/eventLoop/EventLoopManager.hpp"
 #include "protocols/types/SurfaceRole.hpp"
 #include "render/Texture.hpp"
 #include <cstring>
@@ -123,16 +125,15 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : resource(reso
             return;
         }
 
-        if ((!pending.updated.buffer) ||                                  // no new buffer attached
-            (!pending.buffer && !pending.texture) ||                      // null buffer attached
-            (!pending.updated.acquire && pending.buffer->isSynchronous()) // synchronous buffers (ex. shm) can be read immediately
+        if ((!pending.updated.buffer) ||          // no new buffer attached
+            (!pending.buffer && !pending.texture) // null buffer attached
         ) {
             commitState(pending);
             pending.reset();
             return;
         }
 
-        // save state while we wait for buffer to become ready
+        // save state while we wait for buffer to become ready to read
         const auto& state = pendingStates.emplace(makeUnique<SSurfaceState>(pending));
         pending.reset();
 
@@ -152,13 +153,19 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : resource(reso
         if (state->updated.acquire) {
             // wait on acquire point for this surface, from explicit sync protocol
             state->acquire.addWaiter(whenReadable);
-        } else if (state->buffer->dmabuf().success) {
-            // https://www.kernel.org/doc/html/latest/driver-api/dma-buf.html#implicit-fence-poll-support
-            // TODO: wait for the dma-buf fd's to become readable
+        } 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 = dynamic_cast<CDMABuffer*>(state->buffer.buffer.get())->exportSyncFile();
+
+            if (syncFd.isValid())
+                g_pEventLoopManager->doOnReadable(std::move(syncFd), whenReadable);
+            else
+                whenReadable();
         } else {
-            // huh??? only buffers with acquire or dmabuf should get through here...
-            Debug::log(ERR, "BUG THIS: wl_surface.commit: non-acquire non-dmabuf buffers needs wait...");
+            Debug::log(ERR, "BUG THIS: wl_surface.commit: no acquire, non-dmabuf, async buffer, needs wait... this shouldn't happen");
             whenReadable();
         }
     });