properly release buffers in renderer

Author: ikalco

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"

src/helpers/Monitor.hpp

@@ -140,8 +140,7 @@ class CMonitor {
 
     // explicit sync
     SP<CSyncTimeline>              inTimeline;
-    Hyprutils::OS::CFileDescriptor inFence;
-    SP<CEGLSync>                   eglSync;
+    Hyprutils::OS::CFileDescriptor inFence; // TODO: remove when aq uses CFileDescriptor
     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_sync = sync;
+        m_fd = syncFd.duplicate();
 }
 
 void CSyncReleaser::drop() {

src/helpers/sync/SyncReleaser.hpp

@@ -22,9 +22,8 @@ class CSyncReleaser {
     // drops the releaser, will never signal anymore
     void drop();
 
-    // wait for this gpu job to finish before releasing
-    Hyprutils::OS::CFileDescriptor mergeSyncFds(const Hyprutils::OS::CFileDescriptor& fd1, const Hyprutils::OS::CFileDescriptor& fd2);
-    void                           addReleaseSync(SP<CEGLSync> sync);
+    // wait for this sync_fd to signal before releasing
+    void addSyncFileFd(const Hyprutils::OS::CFileDescriptor& syncFd);
 
   private:
     SP<CSyncTimeline>              m_timeline;

src/protocols/DRMSyncobj.cpp

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

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
-    // so we can let the app know we're done.
+    // release the buffer if it's synchronous (SHM) as updateSynchronousTexture() has copied the buffer data to a GPU tex
     // 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();
 }
@@ -64,8 +69,7 @@ CHLBufferReference& CHLBufferReference::operator=(const CHLBufferReference& othe
         other.buffer->lock();
     if (buffer)
         buffer->unlock();
-    buffer  = other.buffer;
-    release = other.release;
+    buffer = other.buffer;
     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;
@@ -57,8 +58,7 @@ class CHLBufferReference {
     operator bool() const;
 
     // unlock and drop the buffer without sending release
-    void               drop();
+    void          drop();
 
-    CDRMSyncPointState release;
-    SP<IHLBuffer>      buffer;
+    SP<IHLBuffer> buffer;
 };

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>
@@ -1553,25 +1552,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;
 }
 
@@ -2278,42 +2258,42 @@ 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);
-
-    if (PMONITOR->inTimeline && explicitOptions.explicitEnabled) {
-        PMONITOR->eglSync = makeShared<CEGLSync>();
-        if (!PMONITOR->eglSync->isValid()) {
-            Debug::log(ERR, "renderer: couldn't create an EGLSync for out in endRender");
-            return;
+    CEGLSync eglSync = CEGLSync();
+    if (eglSync.isValid()) {
+        for (auto const& buf : usedAsyncBuffers) {
+            for (const auto& releaser : buf->syncReleasers) {
+                releaser->addSyncFileFd(eglSync.fd());
+            }
         }
 
-        PMONITOR->inTimelinePoint++;
-        bool ok = PMONITOR->inTimeline->importFromSyncFileFD(PMONITOR->inTimelinePoint, PMONITOR->eglSync->fd());
-        if (!ok) {
-            Debug::log(ERR, "renderer: couldn't import from sync file fd in endRender");
-            return;
-        }
+        // release buffer refs with release points now, since syncReleaser handles actual buffer release based on EGLSync
+        std::erase_if(usedAsyncBuffers, [](const auto& buf) { return !buf->syncReleasers.empty(); });
 
-        if (m_eRenderMode == RENDER_MODE_NORMAL && explicitOptions.explicitKMSEnabled) {
-            PMONITOR->inFence = CFileDescriptor{PMONITOR->inTimeline->exportAsSyncFileFD(PMONITOR->inTimelinePoint)};
-            if (!PMONITOR->inFence.isValid()) {
-                Debug::log(ERR, "renderer: couldn't export from sync timeline in endRender");
-                return;
-            }
+        // release buffer refs without release points when EGLSync sync_file/fence is signalled
+        g_pEventLoopManager->doOnReadable(eglSync.fd().duplicate(), [prevbfs = std::move(usedAsyncBuffers)]() mutable { prevbfs.clear(); });
+        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
     }
 }
 

src/render/Renderer.hpp

@@ -91,20 +91,20 @@ class CHyprRenderer {
 
     bool m_bBlockSurfaceFeedback = false;
     bool m_bRenderingSnapshot    = false;
-    PHLMONITORREF                       m_pMostHzMonitor;
-    bool                                m_bDirectScanoutBlocked = false;
+    PHLMONITORREF                   m_pMostHzMonitor;
+    bool                            m_bDirectScanoutBlocked = false;
 
-    void                                setSurfaceScanoutMode(SP<CWLSurfaceResource> surface, PHLMONITOR monitor); // nullptr monitor resets
-    void                                initiateManualCrash();
+    void                            setSurfaceScanoutMode(SP<CWLSurfaceResource> surface, PHLMONITOR monitor); // nullptr monitor resets
+    void                            initiateManualCrash();
 
-    bool                                m_bCrashingInProgress = false;
-    float                               m_fCrashingDistort    = 0.5f;
-    wl_event_source*                    m_pCrashingLoop       = nullptr;
-    wl_event_source*                    m_pCursorTicker       = nullptr;
+    bool                            m_bCrashingInProgress = false;
+    float                           m_fCrashingDistort    = 0.5f;
+    wl_event_source*                m_pCrashingLoop       = nullptr;
+    wl_event_source*                m_pCursorTicker       = nullptr;
 
-    CTimer                              m_tRenderTimer;
+    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);
 }