From 8e2a509eb6711afe20f2a5426ca5b111add82373 Mon Sep 17 00:00:00 2001 From: netborg <137700136+netborg-afps@users.noreply.github.com> Date: Wed, 19 Feb 2025 15:47:45 +0100 Subject: [PATCH] Revert "[dxvk] Fix lack of forward progress guarantee in presenter" This reverts commit efeb15edbd6dc913030f0846cbc1b587f6fb7c5d, because ordering guarantees were broken, that notifyGpuPresentEnd should happen after notifyGpuPresentBegin, which in turn lead to wrong latency measurements in case vkWaitForPresent was skipped. --- src/dxvk/dxvk_presenter.cpp | 61 ++++++++++++++----------------------- src/dxvk/dxvk_presenter.h | 1 - 2 files changed, 23 insertions(+), 39 deletions(-) diff --git a/src/dxvk/dxvk_presenter.cpp b/src/dxvk/dxvk_presenter.cpp index 79e10ad66..3297d14a0 100644 --- a/src/dxvk/dxvk_presenter.cpp +++ b/src/dxvk/dxvk_presenter.cpp @@ -259,16 +259,9 @@ namespace dxvk { return; if (m_device->features().khrPresentWait.presentWait) { - bool canSignal = false; - - { std::unique_lock lock(m_frameMutex); - - m_lastSignaled = frameId; - canSignal = m_lastCompleted >= frameId; - } - - if (canSignal) - m_signal->signal(frameId); + std::lock_guard lock(m_frameMutex); + m_lastSignaled = frameId; + m_frameCond.notify_one(); } else { m_fpsLimiter.delay(tracker); m_signal->signal(frameId); @@ -1210,26 +1203,25 @@ namespace dxvk { void Presenter::runFrameThread() { env::setThreadName("dxvk-frame"); - while (true) { - PresenterFrame frame = { }; + std::unique_lock lock(m_frameMutex); + while (true) { // Wait for all GPU work for this frame to complete in order to maintain // ordering guarantees of the frame signal w.r.t. objects being released - { std::unique_lock lock(m_frameMutex); + m_frameCond.wait(lock, [this] { + return !m_frameQueue.empty() && m_frameQueue.front().frameId <= m_lastSignaled; + }); - m_frameCond.wait(lock, [this] { - return !m_frameQueue.empty(); - }); + // Use a frame ID of 0 as an exit condition + PresenterFrame frame = m_frameQueue.front(); - // Use a frame ID of 0 as an exit condition - frame = m_frameQueue.front(); - - if (!frame.frameId) { - m_frameQueue.pop(); - return; - } + if (!frame.frameId) { + m_frameQueue.pop(); + return; } + lock.unlock(); + // If the present operation has succeeded, actually wait for it to complete. // Don't bother with it on MAILBOX / IMMEDIATE modes since doing so would // restrict us to the display refresh rate on some platforms (XWayland). @@ -1246,28 +1238,21 @@ namespace dxvk { if (frame.tracker) frame.tracker->notifyGpuPresentEnd(frame.frameId); - // Apply FPS limiter here to align it as closely with scanout as we can, + // Apply FPS limtier here to align it as closely with scanout as we can, // and delay signaling the frame latency event to emulate behaviour of a // low refresh rate display as closely as we can. m_fpsLimiter.delay(frame.tracker); frame.tracker = nullptr; - // Wake up any thread that may be waiting for the queue to become empty - bool canSignal = false; - - { std::unique_lock lock(m_frameMutex); - - m_frameQueue.pop(); - m_frameDrain.notify_one(); - - m_lastCompleted = frame.frameId; - canSignal = m_lastSignaled >= frame.frameId; - } - // Always signal even on error, since failures here // are transparent to the front-end. - if (canSignal) - m_signal->signal(frame.frameId); + m_signal->signal(frame.frameId); + + // Wake up any thread that may be waiting for the queue to become empty + lock.lock(); + + m_frameQueue.pop(); + m_frameDrain.notify_one(); } } diff --git a/src/dxvk/dxvk_presenter.h b/src/dxvk/dxvk_presenter.h index 8e403b244..afbe465c3 100644 --- a/src/dxvk/dxvk_presenter.h +++ b/src/dxvk/dxvk_presenter.h @@ -315,7 +315,6 @@ namespace dxvk { std::queue m_frameQueue; uint64_t m_lastSignaled = 0u; - uint64_t m_lastCompleted = 0u; alignas(CACHE_LINE_SIZE) FpsLimiter m_fpsLimiter;