render: properly release rendered buffers (#9807)

* cleanup eglSync

* properly release buffers in renderer

* add renderingDoneCallback and use it in screencopy

* use static constructor for CEGLSync

src/helpers/Monitor.cpp

@@ -3,7 +3,6 @@
 #include "../macros.hpp"
 #include "math/Math.hpp"
 #include "../protocols/ColorManagement.hpp"
-#include "sync/SyncReleaser.hpp"
 #include "../Compositor.hpp"
 #include "../config/ConfigValue.hpp"
 #include "../config/ConfigManager.hpp"
@@ -61,10 +60,6 @@ void CMonitor::onConnect(bool noRule) {
 
     g_pEventLoopManager->doLater([] { g_pConfigManager->ensurePersistentWorkspacesPresent(); });
 
-    if (output->supportsExplicit) {
-        inTimeline = CSyncTimeline::create(output->getBackend()->drmFD());
-    }
-
     listeners.frame  = output->events.frame.registerListener([this](std::any d) { onMonitorFrame(); });
     listeners.commit = output->events.commit.registerListener([this](std::any d) {
         if (true) { // FIXME: E->state->committed & WLR_OUTPUT_STATE_BUFFER

src/helpers/Monitor.hpp

@@ -139,10 +139,7 @@ class CMonitor {
     SMonitorRule                activeMonitorRule;
 
     // explicit sync
-    SP<CSyncTimeline>              inTimeline;
+    Hyprutils::OS::CFileDescriptor inFence; // TODO: remove when aq uses CFileDescriptor
-    Hyprutils::OS::CFileDescriptor inFence;
-    SP<CEGLSync>                   eglSync;
-    uint64_t                       inTimelinePoint = 0;
 
     PHLMONITORREF                  self;
 

src/helpers/sync/SyncReleaser.cpp

@@ -35,7 +35,10 @@ CSyncReleaser::~CSyncReleaser() {
         m_timeline->signal(m_point);
 }
 
-CFileDescriptor CSyncReleaser::mergeSyncFds(const CFileDescriptor& fd1, const CFileDescriptor& fd2) {
+static CFileDescriptor mergeSyncFds(const CFileDescriptor& fd1, const CFileDescriptor& fd2) {
+    // combines the fences of both sync_fds into a dma_fence_array (https://www.kernel.org/doc/html/latest/driver-api/dma-buf.html#c.dma_fence_array_create)
+    // with the signal_on_any param set to false, so the new sync_fd will "signal when all fences in the array signal."
+
     struct sync_merge_data data{
         .name  = "merged release fence",
         .fd2   = fd2.get(),
@@ -51,13 +54,11 @@ CFileDescriptor CSyncReleaser::mergeSyncFds(const CFileDescriptor& fd1, const CF
         return CFileDescriptor(data.fence);
 }
 
-void CSyncReleaser::addReleaseSync(SP<CEGLSync> sync) {
+void CSyncReleaser::addSyncFileFd(const Hyprutils::OS::CFileDescriptor& syncFd) {
     if (m_fd.isValid())
-        m_fd = mergeSyncFds(m_fd, sync->takeFD());
+        m_fd = mergeSyncFds(m_fd, syncFd);
     else
-        m_fd = sync->fd().duplicate();
+        m_fd = syncFd.duplicate();
-
-    m_sync = sync;
 }
 
 void CSyncReleaser::drop() {

src/helpers/sync/SyncReleaser.hpp

@@ -12,7 +12,6 @@
 */
 
 class CSyncTimeline;
-class CEGLSync;
 
 class CSyncReleaser {
   public:
@@ -22,13 +21,11 @@ class CSyncReleaser {
     // drops the releaser, will never signal anymore
     void drop();
 
-    // wait for this gpu job to finish before releasing
+    // wait for this sync_fd to signal before releasing
-    Hyprutils::OS::CFileDescriptor mergeSyncFds(const Hyprutils::OS::CFileDescriptor& fd1, const Hyprutils::OS::CFileDescriptor& fd2);
+    void addSyncFileFd(const Hyprutils::OS::CFileDescriptor& syncFd);
-    void                           addReleaseSync(SP<CEGLSync> sync);
 
   private:
     SP<CSyncTimeline>              m_timeline;
     uint64_t                       m_point = 0;
     Hyprutils::OS::CFileDescriptor m_fd;
-    SP<CEGLSync>                   m_sync;
 };

src/protocols/DRMSyncobj.cpp

@@ -105,10 +105,8 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UP<CWpLinuxDrmSyncobjSurf
         surface->pending.acquire         = pendingAcquire;
         pendingAcquire                   = {};
 
-        surface->pending.buffer.release = pendingRelease;
+        surface->pending.buffer->addReleasePoint(pendingRelease);
         pendingRelease = {};
-
-        surface->pending.buffer->syncReleaser = surface->pending.buffer.release.createSyncRelease();
     });
 }
 

src/protocols/Screencopy.cpp

@@ -221,21 +221,11 @@ void CScreencopyFrame::copyDmabuf(std::function<void(bool)> callback) {
     }
 
     g_pHyprOpenGL->m_RenderData.blockScreenShader = true;
-    g_pHyprRenderer->endRender();
 
-    auto explicitOptions = g_pHyprRenderer->getExplicitSyncSettings(pMonitor->output);
+    g_pHyprRenderer->endRender([callback]() {
-    if (pMonitor->inTimeline && explicitOptions.explicitEnabled) {
-        if (pMonitor->inTimeline->addWaiter(
-                [callback, sync = pMonitor->eglSync]() {
-                    LOGM(TRACE, "Copied frame via dma with explicit sync");
-                    callback(true);
-                },
-                pMonitor->inTimelinePoint, 0))
-            return;
-        // on explicit sync failure, fallthrough to immediate callback
-    }
         LOGM(TRACE, "Copied frame via dma");
         callback(true);
+    });
 }
 
 bool CScreencopyFrame::copyShm() {

src/protocols/core/Compositor.cpp

@@ -7,7 +7,6 @@
 #include "Subcompositor.hpp"
 #include "../Viewporter.hpp"
 #include "../../helpers/Monitor.hpp"
-#include "../../helpers/sync/SyncReleaser.hpp"
 #include "../PresentationTime.hpp"
 #include "../DRMSyncobj.hpp"
 #include "../types/DMABuffer.hpp"
@@ -518,8 +517,7 @@ void CWLSurfaceResource::commitState(SSurfaceState& state) {
             nullptr);
     }
 
-    // release the buffer if it's synchronous (SHM) as update() has done everything thats needed
+    // release the buffer if it's synchronous (SHM) as updateSynchronousTexture() has copied the buffer data to a GPU tex
-    // so we can let the app know we're done.
     // if it doesn't have a role, we can't release it yet, in case it gets turned into a cursor.
     if (current.buffer && current.buffer->isSynchronous() && role->role() != SURFACE_ROLE_UNASSIGNED)
         dropCurrentBuffer();
@@ -572,12 +570,6 @@ void CWLSurfaceResource::presentFeedback(const Time::steady_tp& when, PHLMONITOR
     else
         FEEDBACK->presented();
     PROTO::presentation->queueData(FEEDBACK);
-
-    if (!pMonitor || !pMonitor->inTimeline || !syncobj)
-        return;
-
-    // attach explicit sync
-    g_pHyprRenderer->explicitPresented.emplace_back(self.lock());
 }
 
 CWLCompositorResource::CWLCompositorResource(SP<CWlCompositor> resource_) : resource(resource_) {

src/protocols/types/Buffer.cpp

@@ -7,6 +7,7 @@ IHLBuffer::~IHLBuffer() {
 
 void IHLBuffer::sendRelease() {
     resource->sendRelease();
+    syncReleasers.clear();
 }
 
 void IHLBuffer::lock() {
@@ -18,10 +19,8 @@ void IHLBuffer::unlock() {
 
     ASSERT(nLocks >= 0);
 
-    if (nLocks == 0) {
+    if (nLocks == 0)
         sendRelease();
-        syncReleaser.reset();
-    }
 }
 
 bool IHLBuffer::locked() {
@@ -40,11 +39,17 @@ void IHLBuffer::onBackendRelease(const std::function<void()>& fn) {
     });
 }
 
+void IHLBuffer::addReleasePoint(CDRMSyncPointState& point) {
+    ASSERT(locked());
+    if (point)
+        syncReleasers.emplace_back(point.createSyncRelease());
+}
+
 CHLBufferReference::CHLBufferReference() : buffer(nullptr) {
     ;
 }
 
-CHLBufferReference::CHLBufferReference(const CHLBufferReference& other) : release(other.release), buffer(other.buffer) {
+CHLBufferReference::CHLBufferReference(const CHLBufferReference& other) : buffer(other.buffer) {
     if (buffer)
         buffer->lock();
 }
@@ -65,7 +70,6 @@ CHLBufferReference& CHLBufferReference::operator=(const CHLBufferReference& othe
     if (buffer)
         buffer->unlock();
     buffer = other.buffer;
-    release = other.release;
     return *this;
 }
 

src/protocols/types/Buffer.hpp

@@ -24,11 +24,12 @@ class IHLBuffer : public Aquamarine::IBuffer {
     virtual bool                          locked();
 
     void                                  onBackendRelease(const std::function<void()>& fn);
+    void                                  addReleasePoint(CDRMSyncPointState& point);
 
     SP<CTexture>                          texture;
     bool                                  opaque = false;
     SP<CWLBufferResource>                 resource;
-    UP<CSyncReleaser>                     syncReleaser;
+    std::vector<UP<CSyncReleaser>>        syncReleasers;
 
     struct {
         CHyprSignalListener backendRelease;
@@ -59,6 +60,5 @@ class CHLBufferReference {
     // unlock and drop the buffer without sending release
     void          drop();
 
-    CDRMSyncPointState release;
     SP<IHLBuffer> buffer;
 };

src/render/OpenGL.cpp

@@ -3090,44 +3090,6 @@ std::vector<SDRMFormat> CHyprOpenGLImpl::getDRMFormats() {
     return drmFormats;
 }
 
-SP<CEGLSync> CHyprOpenGLImpl::createEGLSync(int fenceFD) {
-    std::vector<EGLint> attribs;
-    CFileDescriptor     dupFd;
-    if (fenceFD >= 0) {
-        dupFd = CFileDescriptor{fcntl(fenceFD, F_DUPFD_CLOEXEC, 0)};
-        if (!dupFd.isValid()) {
-            Debug::log(ERR, "createEGLSync: dup failed");
-            return nullptr;
-        }
-        // reserve number of elements to avoid reallocations
-        attribs.reserve(3);
-        attribs.push_back(EGL_SYNC_NATIVE_FENCE_FD_ANDROID);
-        attribs.push_back(dupFd.get());
-        attribs.push_back(EGL_NONE);
-    }
-
-    EGLSyncKHR sync = m_sProc.eglCreateSyncKHR(m_pEglDisplay, EGL_SYNC_NATIVE_FENCE_ANDROID, attribs.data());
-    if (sync == EGL_NO_SYNC_KHR) {
-        Debug::log(ERR, "eglCreateSyncKHR failed");
-        return nullptr;
-    } else
-        dupFd.take(); // eglCreateSyncKHR only takes ownership on success
-
-    // we need to flush otherwise we might not get a valid fd
-    glFlush();
-
-    int fd = g_pHyprOpenGL->m_sProc.eglDupNativeFenceFDANDROID(g_pHyprOpenGL->m_pEglDisplay, sync);
-    if (fd == EGL_NO_NATIVE_FENCE_FD_ANDROID) {
-        Debug::log(ERR, "eglDupNativeFenceFDANDROID failed");
-        return nullptr;
-    }
-
-    auto eglsync   = SP<CEGLSync>(new CEGLSync);
-    eglsync->sync  = sync;
-    eglsync->m_iFd = CFileDescriptor{fd};
-    return eglsync;
-}
-
 void SRenderModifData::applyToBox(CBox& box) {
     if (!enabled)
         return;
@@ -3189,18 +3151,47 @@ float SRenderModifData::combinedScale() {
     return scale;
 }
 
+UP<CEGLSync> CEGLSync::create() {
+    EGLSyncKHR sync = g_pHyprOpenGL->m_sProc.eglCreateSyncKHR(g_pHyprOpenGL->m_pEglDisplay, EGL_SYNC_NATIVE_FENCE_ANDROID, nullptr);
+
+    if (sync == EGL_NO_SYNC_KHR) {
+        Debug::log(ERR, "eglCreateSyncKHR failed");
+        return nullptr;
+    }
+
+    // we need to flush otherwise we might not get a valid fd
+    glFlush();
+
+    int fd = g_pHyprOpenGL->m_sProc.eglDupNativeFenceFDANDROID(g_pHyprOpenGL->m_pEglDisplay, sync);
+    if (fd == EGL_NO_NATIVE_FENCE_FD_ANDROID) {
+        Debug::log(ERR, "eglDupNativeFenceFDANDROID failed");
+        return nullptr;
+    }
+
+    UP<CEGLSync> eglSync(new CEGLSync);
+    eglSync->m_fd    = CFileDescriptor(fd);
+    eglSync->m_sync  = sync;
+    eglSync->m_valid = true;
+
+    return eglSync;
+}
+
 CEGLSync::~CEGLSync() {
-    if (sync == EGL_NO_SYNC_KHR)
+    if (m_sync == EGL_NO_SYNC_KHR)
         return;
 
-    if (g_pHyprOpenGL && g_pHyprOpenGL->m_sProc.eglDestroySyncKHR(g_pHyprOpenGL->m_pEglDisplay, sync) != EGL_TRUE)
+    if (g_pHyprOpenGL && g_pHyprOpenGL->m_sProc.eglDestroySyncKHR(g_pHyprOpenGL->m_pEglDisplay, m_sync) != EGL_TRUE)
         Debug::log(ERR, "eglDestroySyncKHR failed");
 }
 
-CFileDescriptor&& CEGLSync::takeFD() {
+CFileDescriptor& CEGLSync::fd() {
-    return std::move(m_iFd);
+    return m_fd;
 }
 
-CFileDescriptor& CEGLSync::fd() {
+CFileDescriptor&& CEGLSync::takeFd() {
-    return m_iFd;
+    return std::move(m_fd);
+}
+
+bool CEGLSync::isValid() {
+    return m_valid && m_sync != EGL_NO_SYNC_KHR && m_fd.isValid();
 }

src/render/OpenGL.hpp

@@ -150,17 +150,20 @@ struct SCurrentRenderData {
 
 class CEGLSync {
   public:
+    static UP<CEGLSync> create();
+
     ~CEGLSync();
 
-    EGLSyncKHR                       sync = nullptr;
-
-    Hyprutils::OS::CFileDescriptor&& takeFD();
     Hyprutils::OS::CFileDescriptor&  fd();
+    Hyprutils::OS::CFileDescriptor&& takeFd();
+    bool                             isValid();
 
   private:
     CEGLSync() = default;
 
-    Hyprutils::OS::CFileDescriptor m_iFd;
+    Hyprutils::OS::CFileDescriptor m_fd;
+    EGLSyncKHR                     m_sync  = EGL_NO_SYNC_KHR;
+    bool                           m_valid = false;
 
     friend class CHyprOpenGLImpl;
 };
@@ -234,7 +237,6 @@ class CHyprOpenGLImpl {
     uint32_t     getPreferredReadFormat(PHLMONITOR pMonitor);
     std::vector<SDRMFormat>                     getDRMFormats();
     EGLImageKHR                                 createEGLImage(const Aquamarine::SDMABUFAttrs& attrs);
-    SP<CEGLSync>                                createEGLSync(int fence = -1);
 
     bool                                        initShaders();
     bool                                        m_bShadersInitialized = false;

src/render/Renderer.cpp

@@ -1,7 +1,6 @@
 #include "Renderer.hpp"
 #include "../Compositor.hpp"
 #include "../helpers/math/Math.hpp"
-#include "../helpers/sync/SyncReleaser.hpp"
 #include <algorithm>
 #include <aquamarine/output/Output.hpp>
 #include <filesystem>
@@ -1571,25 +1570,6 @@ bool CHyprRenderer::commitPendingAndDoExplicitSync(PHLMONITOR pMonitor) {
         }
     }
 
-    auto explicitOptions = getExplicitSyncSettings(pMonitor->output);
-    if (!explicitOptions.explicitEnabled)
-        return ok;
-
-    Debug::log(TRACE, "Explicit: {} presented", explicitPresented.size());
-
-    if (!pMonitor->eglSync)
-        Debug::log(TRACE, "Explicit: can't add sync, monitor has no EGLSync");
-    else {
-        for (auto const& e : explicitPresented) {
-            if (!e->current.buffer || !e->current.buffer->syncReleaser)
-                continue;
-
-            e->current.buffer->syncReleaser->addReleaseSync(pMonitor->eglSync);
-        }
-    }
-
-    explicitPresented.clear();
-
     return ok;
 }
 
@@ -2275,7 +2255,7 @@ bool CHyprRenderer::beginRender(PHLMONITOR pMonitor, CRegion& damage, eRenderMod
     return true;
 }
 
-void CHyprRenderer::endRender() {
+void CHyprRenderer::endRender(const std::function<void()>& renderingDoneCallback) {
     const auto  PMONITOR           = g_pHyprOpenGL->m_RenderData.pMonitor;
     static auto PNVIDIAANTIFLICKER = CConfigValue<Hyprlang::INT>("opengl:nvidia_anti_flicker");
 
@@ -2296,42 +2276,48 @@ void CHyprRenderer::endRender() {
         g_pHyprOpenGL->m_RenderData.mouseZoomUseMouse = true;
     }
 
+    // send all queued opengl commands so rendering starts happening immediately
+    glFlush();
+
     if (m_eRenderMode == RENDER_MODE_FULL_FAKE)
         return;
 
     if (m_eRenderMode == RENDER_MODE_NORMAL)
         PMONITOR->output->state->setBuffer(m_pCurrentBuffer);
 
-    auto explicitOptions = getExplicitSyncSettings(PMONITOR->output);
+    UP<CEGLSync> eglSync = CEGLSync::create();
-
+    if (eglSync && eglSync->isValid()) {
-    if (PMONITOR->inTimeline && explicitOptions.explicitEnabled) {
+        for (auto const& buf : usedAsyncBuffers) {
-        PMONITOR->eglSync = g_pHyprOpenGL->createEGLSync();
+            for (const auto& releaser : buf->syncReleasers) {
-        if (!PMONITOR->eglSync) {
+                releaser->addSyncFileFd(eglSync->fd());
-            Debug::log(ERR, "renderer: couldn't create an EGLSync for out in endRender");
+            }
-            return;
         }
 
-        PMONITOR->inTimelinePoint++;
+        // release buffer refs with release points now, since syncReleaser handles actual buffer release based on EGLSync
-        bool ok = PMONITOR->inTimeline->importFromSyncFileFD(PMONITOR->inTimelinePoint, PMONITOR->eglSync->fd());
+        std::erase_if(usedAsyncBuffers, [](const auto& buf) { return !buf->syncReleasers.empty(); });
-        if (!ok) {
-            Debug::log(ERR, "renderer: couldn't import from sync file fd in endRender");
-            return;
-        }
 
-        if (m_eRenderMode == RENDER_MODE_NORMAL && explicitOptions.explicitKMSEnabled) {
+        // release buffer refs without release points when EGLSync sync_file/fence is signalled
-            PMONITOR->inFence = CFileDescriptor{PMONITOR->inTimeline->exportAsSyncFileFD(PMONITOR->inTimelinePoint)};
+        g_pEventLoopManager->doOnReadable(eglSync->fd().duplicate(), [renderingDoneCallback, prevbfs = std::move(usedAsyncBuffers)]() mutable {
-            if (!PMONITOR->inFence.isValid()) {
+            prevbfs.clear();
-                Debug::log(ERR, "renderer: couldn't export from sync timeline in endRender");
+            if (renderingDoneCallback)
-                return;
+                renderingDoneCallback();
-            }
+        });
+        usedAsyncBuffers.clear();
 
+        if (m_eRenderMode == RENDER_MODE_NORMAL) {
+            PMONITOR->inFence = eglSync->takeFd();
             PMONITOR->output->state->setExplicitInFence(PMONITOR->inFence.get());
         }
     } else {
+        Debug::log(ERR, "renderer: couldn't use EGLSync for explicit gpu synchronization");
+
+        // nvidia doesn't have implicit sync, so we have to explicitly wait here
         if (isNvidia() && *PNVIDIAANTIFLICKER)
             glFinish();
-        else
+
-            glFlush();
+        usedAsyncBuffers.clear(); // release all buffer refs and hope implicit sync works
+        if (renderingDoneCallback)
+            renderingDoneCallback();
     }
 }
 

src/render/Renderer.hpp

@@ -87,7 +87,7 @@ class CHyprRenderer {
     // if RENDER_MODE_NORMAL, provided damage will be written to.
     // otherwise, it will be the one used.
     bool beginRender(PHLMONITOR pMonitor, CRegion& damage, eRenderMode mode = RENDER_MODE_NORMAL, SP<IHLBuffer> buffer = {}, CFramebuffer* fb = nullptr, bool simple = false);
-    void endRender();
+    void endRender(const std::function<void()>& renderingDoneCallback = {});
 
     bool m_bBlockSurfaceFeedback = false;
     bool m_bRenderingSnapshot    = false;
@@ -104,7 +104,7 @@ class CHyprRenderer {
 
     CTimer                          m_tRenderTimer;
 
-    std::vector<SP<CWLSurfaceResource>> explicitPresented;
+    std::vector<CHLBufferReference> usedAsyncBuffers;
 
     struct {
         int                           hotspotX = 0;

src/render/pass/SurfacePassElement.cpp

@@ -129,6 +129,11 @@ void CSurfacePassElement::draw(const CRegion& damage) {
     if (!g_pHyprRenderer->m_bBlockSurfaceFeedback)
         data.surface->presentFeedback(data.when, data.pMonitor->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 (data.surface->current.buffer && !data.surface->current.buffer->isSynchronous())
+        g_pHyprRenderer->usedAsyncBuffers.emplace_back(data.surface->current.buffer);
+
     g_pHyprOpenGL->blend(true);
 }