renderer: skip ds commits if buffer didn't change (#9556)

this fixes direct scanout glitches by ensuring that attemptDirectScanout doesn't try to recommit the same buffer to AQ
which would cause a pageflip event and the backendRelease to release the same buffer too early

5 files changed, 54 insertions, 61 deletions

diff --git a/src/helpers/Monitor.cpp b/src/helpers/Monitor.cpp
index da391e50..df672caf 100644
--- a/src/helpers/Monitor.cpp
+++ b/src/helpers/Monitor.cpp
@@ -1280,23 +1280,26 @@ bool CMonitor::attemptDirectScanout() {
 
     const auto PSURFACE = g_pXWaylandManager->getWindowSurface(PCANDIDATE);
 
-    if (!PSURFACE || !PSURFACE->current.buffer || PSURFACE->current.bufferSize != vecPixelSize || PSURFACE->current.transform != transform)
+    if (!PSURFACE || !PSURFACE->current.texture || !PSURFACE->current.buffer || PSURFACE->current.buffer->buffer.expired())
+        return false;
+
+    if (PSURFACE->current.bufferSize != vecPixelSize || PSURFACE->current.transform != transform)
         return false;
 
     // we can't scanout shm buffers.
-    if (!PSURFACE->current.buffer || !PSURFACE->current.buffer->buffer || !PSURFACE->current.texture || !PSURFACE->current.texture->m_pEglImage /* dmabuf */)
+    const auto params = PSURFACE->current.buffer->buffer->dmabuf();
+    if (!params.success || !PSURFACE->current.texture->m_pEglImage /* dmabuf */)
         return false;
 
-    Debug::log(TRACE, "attemptDirectScanout: surface {:x} passed, will attempt", (uintptr_t)PSURFACE.get());
+    Debug::log(TRACE, "attemptDirectScanout: surface {:x} passed, will attempt, buffer {}", (uintptr_t)PSURFACE.get(), (uintptr_t)PSURFACE->current.buffer->buffer.get());
+
+    auto PBUFFER = PSURFACE->current.buffer->buffer.lock();
+    if (PBUFFER == output->state->state().buffer)
+        return true;
 
     // FIXME: make sure the buffer actually follows the available scanout dmabuf formats
     // and comes from the appropriate device. This may implode on multi-gpu!!
 
-    const auto params = PSURFACE->current.buffer->buffer->dmabuf();
-    // scanout buffer isn't dmabuf, so no scanout
-    if (!params.success)
-        return false;
-
     // entering into scanout, so save monitor format
     if (lastScanout.expired())
         prevDrmFormat = drmFormat;
@@ -1306,7 +1309,7 @@ bool CMonitor::attemptDirectScanout() {
         drmFormat = params.format;
     }
 
-    output->state->setBuffer(PSURFACE->current.buffer->buffer.lock());
+    output->state->setBuffer(PBUFFER);
     output->state->setPresentationMode(tearingState.activelyTearing ? Aquamarine::eOutputPresentationMode::AQ_OUTPUT_PRESENTATION_IMMEDIATE :
                                                                       Aquamarine::eOutputPresentationMode::AQ_OUTPUT_PRESENTATION_VSYNC);
 
@@ -1315,20 +1318,6 @@ bool CMonitor::attemptDirectScanout() {
         return false;
     }
 
-    auto explicitOptions = g_pHyprRenderer->getExplicitSyncSettings(output);
-
-    // wait for the explicit fence if present, and if kms explicit is allowed
-    bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->syncobj->current.acquireTimeline && PSURFACE->syncobj->current.acquireTimeline->timeline && explicitOptions.explicitKMSEnabled;
-    CFileDescriptor explicitWaitFD;
-    if (DOEXPLICIT) {
-        explicitWaitFD = PSURFACE->syncobj->current.acquireTimeline->timeline->exportAsSyncFileFD(PSURFACE->syncobj->current.acquirePoint);
-        if (!explicitWaitFD.isValid())
-            Debug::log(TRACE, "attemptDirectScanout: failed to acquire an explicit wait fd");
-    }
-    DOEXPLICIT = DOEXPLICIT && explicitWaitFD.isValid();
-
-    auto     cleanup = CScopeGuard([this]() { output->state->resetExplicitFences(); });
-
     timespec now;
     clock_gettime(CLOCK_MONOTONIC, &now);
     PSURFACE->presentFeedback(&now, self.lock());
@@ -1336,11 +1325,24 @@ bool CMonitor::attemptDirectScanout() {
     output->state->addDamage(CBox{{}, vecPixelSize});
     output->state->resetExplicitFences();
 
+    auto cleanup = CScopeGuard([this]() { output->state->resetExplicitFences(); });
+
+    auto explicitOptions = g_pHyprRenderer->getExplicitSyncSettings(output);
+
+    bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->syncobj->current.acquireTimeline && PSURFACE->syncobj->current.acquireTimeline->timeline && explicitOptions.explicitKMSEnabled;
     if (DOEXPLICIT) {
-        Debug::log(TRACE, "attemptDirectScanout: setting IN_FENCE for aq to {}", explicitWaitFD.get());
-        output->state->setExplicitInFence(explicitWaitFD.get());
+        // wait for surface's explicit fence if present
+        CFileDescriptor fd = PSURFACE->syncobj->current.acquireTimeline->timeline->exportAsSyncFileFD(PSURFACE->syncobj->current.acquirePoint);
+        if (fd.isValid()) {
+            Debug::log(TRACE, "attemptDirectScanout: setting IN_FENCE for aq to {}", fd.get());
+            output->state->setExplicitInFence(fd.get());
+        } else
+            Debug::log(TRACE, "attemptDirectScanout: failed to acquire an sync file fd for aq IN_FENCE");
+        DOEXPLICIT = fd.isValid();
     }
 
+    commitSeq++;
+
     bool ok = output->commit();
 
     if (!ok && DOEXPLICIT) {
@@ -1350,29 +1352,25 @@ bool CMonitor::attemptDirectScanout() {
         ok = output->commit();
     }
 
-    if (ok) {
-        if (lastScanout.expired()) {
-            lastScanout = PCANDIDATE;
-            Debug::log(LOG, "Entered a direct scanout to {:x}: \"{}\"", (uintptr_t)PCANDIDATE.get(), PCANDIDATE->m_szTitle);
-        }
-
-        // delay explicit sync feedback until kms release of the buffer
-        if (DOEXPLICIT) {
-            Debug::log(TRACE, "attemptDirectScanout: Delaying explicit sync release feedback until kms release");
-            PSURFACE->current.buffer->releaser->drop();
-
-            PSURFACE->current.buffer->buffer->hlEvents.backendRelease2 = PSURFACE->current.buffer->buffer->events.backendRelease.registerListener([PSURFACE](std::any d) {
-                const bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->syncobj->current.releaseTimeline && PSURFACE->syncobj->current.releaseTimeline->timeline;
-                if (DOEXPLICIT)
-                    PSURFACE->syncobj->current.releaseTimeline->timeline->signal(PSURFACE->syncobj->current.releasePoint);
-            });
-        }
-    } else {
+    if (!ok) {
         Debug::log(TRACE, "attemptDirectScanout: failed to scanout surface");
         lastScanout.reset();
         return false;
     }
 
+    if (lastScanout.expired()) {
+        lastScanout = PCANDIDATE;
+        Debug::log(LOG, "Entered a direct scanout to {:x}: \"{}\"", (uintptr_t)PCANDIDATE.get(), PCANDIDATE->m_szTitle);
+    }
+
+    if (!PBUFFER->lockedByBackend)
+        return true;
+
+    // lock buffer while DRM/KMS is using it, then release it when page flip happens since DRM/KMS should be done by then
+    // btw buffer's syncReleaser will take care of signaling release point, so we don't do that here
+    PBUFFER->lock();
+    PBUFFER->onBackendRelease([PBUFFER]() { PBUFFER->unlock(); });
+
     return true;
 }
 
diff --git a/src/protocols/core/Compositor.cpp b/src/protocols/core/Compositor.cpp
index 06977338..f7af9504 100644
--- a/src/protocols/core/Compositor.cpp
+++ b/src/protocols/core/Compositor.cpp
@@ -439,9 +439,8 @@ void CWLSurfaceResource::unlockPendingState() {
 }
 
 void CWLSurfaceResource::commitPendingState() {
-    static auto PDROP          = CConfigValue<Hyprlang::INT>("render:allow_early_buffer_release");
-    auto const  previousBuffer = current.buffer;
-    current                    = pending;
+    static auto PDROP = CConfigValue<Hyprlang::INT>("render:allow_early_buffer_release");
+    current           = pending;
     pending.damage.clear();
     pending.bufferDamage.clear();
     pending.newBuffer = false;
@@ -495,15 +494,6 @@ void CWLSurfaceResource::commitPendingState() {
             nullptr);
     }
 
-    // for async buffers, we can only release the buffer once we are unrefing it from current.
-    // if the backend took it, ref it with the lambda. Otherwise, the end of this scope will release it.
-    if (previousBuffer && previousBuffer->buffer && !previousBuffer->buffer->isSynchronous()) {
-        if (previousBuffer->buffer->lockedByBackend && !previousBuffer->buffer->hlEvents.backendRelease) {
-            previousBuffer->buffer->lock();
-            previousBuffer->buffer->unlockOnBufferRelease(self);
-        }
-    }
-
     lastBuffer = current.buffer ? current.buffer->buffer : WP<IHLBuffer>{};
 }
 
diff --git a/src/protocols/types/Buffer.cpp b/src/protocols/types/Buffer.cpp
index ea12f017..9fb6294f 100644
--- a/src/protocols/types/Buffer.cpp
+++ b/src/protocols/types/Buffer.cpp
@@ -26,9 +26,14 @@ bool IHLBuffer::locked() {
     return nLocks > 0;
 }
 
-void IHLBuffer::unlockOnBufferRelease(WP<CWLSurfaceResource> surf) {
-    hlEvents.backendRelease = events.backendRelease.registerListener([this](std::any data) {
-        unlock();
+void IHLBuffer::onBackendRelease(const std::function<void()>& fn) {
+    if (hlEvents.backendRelease) {
+        hlEvents.backendRelease->emit(nullptr);
+        Debug::log(LOG, "backendRelease emitted early");
+    }
+
+    hlEvents.backendRelease = events.backendRelease.registerListener([this, fn](std::any) {
+        fn();
         hlEvents.backendRelease.reset();
     });
 }
diff --git a/src/protocols/types/Buffer.hpp b/src/protocols/types/Buffer.hpp
index 7d84dce7..bd02e2db 100644
--- a/src/protocols/types/Buffer.hpp
+++ b/src/protocols/types/Buffer.hpp
@@ -21,7 +21,7 @@ class IHLBuffer : public Aquamarine::IBuffer {
     virtual void                          unlock();
     virtual bool                          locked();
 
-    void                                  unlockOnBufferRelease(WP<CWLSurfaceResource> surf /* optional */);
+    void                                  onBackendRelease(const std::function<void()>& fn);
 
     SP<CTexture>                          texture;
     bool                                  opaque = false;
diff --git a/src/render/Renderer.cpp b/src/render/Renderer.cpp
index 2ebf5629..3175b915 100644
--- a/src/render/Renderer.cpp
+++ b/src/render/Renderer.cpp
@@ -1486,6 +1486,8 @@ static hdr_output_metadata createHDRMetadata(SImageDescription settings, Aquamar
 }
 
 bool CHyprRenderer::commitPendingAndDoExplicitSync(PHLMONITOR pMonitor) {
+    pMonitor->commitSeq++;
+
     // apply timelines for explicit sync
     // save inFD otherwise reset will reset it
     CFileDescriptor inFD{pMonitor->output->state->state().explicitInFence};
@@ -2278,8 +2280,6 @@ void CHyprRenderer::endRender() {
     const auto  PMONITOR           = g_pHyprOpenGL->m_RenderData.pMonitor;
     static auto PNVIDIAANTIFLICKER = CConfigValue<Hyprlang::INT>("opengl:nvidia_anti_flicker");
 
-    PMONITOR->commitSeq++;
-
     g_pHyprOpenGL->m_RenderData.damage = m_sRenderPass.render(g_pHyprOpenGL->m_RenderData.damage);
 
     auto cleanup = CScopeGuard([this]() {