From 46dbd22d987e5781ba7e6651f7e6be543009881f Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Tue, 18 Feb 2025 15:29:53 +0100 Subject: [PATCH 1/3] syncobj: use eventfd instead of stalling fd checks use eventfd and add it to the event loop and when it recieves a signal release the buffer, this means we dont stall entire compositor when waiting for materilisation of the fd. and change its related usage, this also means we need to store release points that can popup in a container and signal them all on buffer release. im not sure why directscanout previously manually tried to signal releasepoints. remove that and let the buffers releaser handle it. --- src/helpers/Monitor.cpp | 16 +----- src/helpers/sync/SyncReleaser.cpp | 4 -- src/helpers/sync/SyncReleaser.hpp | 3 -- src/helpers/sync/SyncTimeline.cpp | 17 ++++++ src/helpers/sync/SyncTimeline.hpp | 1 + src/protocols/DRMSyncobj.cpp | 75 +++++++++++++------------- src/protocols/DRMSyncobj.hpp | 22 ++++++-- src/protocols/core/Compositor.cpp | 16 +++--- src/protocols/core/Compositor.hpp | 4 +- src/protocols/types/Buffer.hpp | 4 +- src/render/Renderer.cpp | 5 +- src/render/pass/SurfacePassElement.cpp | 4 +- 12 files changed, 93 insertions(+), 78 deletions(-) diff --git a/src/helpers/Monitor.cpp b/src/helpers/Monitor.cpp index 24db3ea4b2d..6aa75ce9f95 100644 --- a/src/helpers/Monitor.cpp +++ b/src/helpers/Monitor.cpp @@ -1315,10 +1315,10 @@ bool CMonitor::attemptDirectScanout() { 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; + bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->syncobj->acquire.resource && PSURFACE->syncobj->acquire.resource->timeline && explicitOptions.explicitKMSEnabled; CFileDescriptor explicitWaitFD; if (DOEXPLICIT) { - explicitWaitFD = PSURFACE->syncobj->current.acquireTimeline->timeline->exportAsSyncFileFD(PSURFACE->syncobj->current.acquirePoint); + explicitWaitFD = PSURFACE->syncobj->acquire.resource->timeline->exportAsSyncFileFD(PSURFACE->syncobj->acquire.point); if (!explicitWaitFD.isValid()) Debug::log(TRACE, "attemptDirectScanout: failed to acquire an explicit wait fd"); } @@ -1352,18 +1352,6 @@ bool CMonitor::attemptDirectScanout() { 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 { Debug::log(TRACE, "attemptDirectScanout: failed to scanout surface"); lastScanout.reset(); diff --git a/src/helpers/sync/SyncReleaser.cpp b/src/helpers/sync/SyncReleaser.cpp index 198495ab6da..9fc6422b3cb 100644 --- a/src/helpers/sync/SyncReleaser.cpp +++ b/src/helpers/sync/SyncReleaser.cpp @@ -19,7 +19,3 @@ CSyncReleaser::~CSyncReleaser() { void CSyncReleaser::addReleaseSync(SP sync_) { sync = sync_; } - -void CSyncReleaser::drop() { - timeline.reset(); -} \ No newline at end of file diff --git a/src/helpers/sync/SyncReleaser.hpp b/src/helpers/sync/SyncReleaser.hpp index e21d2e34bd2..ca571c1631c 100644 --- a/src/helpers/sync/SyncReleaser.hpp +++ b/src/helpers/sync/SyncReleaser.hpp @@ -18,9 +18,6 @@ class CSyncReleaser { CSyncReleaser(WP timeline_, uint64_t point_); ~CSyncReleaser(); - // drops the releaser, will never signal anymore - void drop(); - // wait for this gpu job to finish before releasing void addReleaseSync(SP sync); diff --git a/src/helpers/sync/SyncTimeline.cpp b/src/helpers/sync/SyncTimeline.cpp index 46b617bc6d6..c2e8b001b6e 100644 --- a/src/helpers/sync/SyncTimeline.cpp +++ b/src/helpers/sync/SyncTimeline.cpp @@ -33,6 +33,12 @@ SP CSyncTimeline::create(int drmFD_, int drmSyncobjFD) { } CSyncTimeline::~CSyncTimeline() { + for (auto& w : waiters) { + if (w->source) { + wl_event_source_remove(w->source); + w->source = nullptr; + } + } if (handle == 0) return; @@ -124,6 +130,17 @@ void CSyncTimeline::removeWaiter(SWaiter* w) { 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; diff --git a/src/helpers/sync/SyncTimeline.hpp b/src/helpers/sync/SyncTimeline.hpp index ba65e004d0b..bfd70416106 100644 --- a/src/helpers/sync/SyncTimeline.hpp +++ b/src/helpers/sync/SyncTimeline.hpp @@ -34,6 +34,7 @@ class CSyncTimeline { bool addWaiter(const std::function& 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 from, uint64_t fromPoint, uint64_t toPoint); diff --git a/src/protocols/DRMSyncobj.cpp b/src/protocols/DRMSyncobj.cpp index 38aab305af3..63f0946b58c 100644 --- a/src/protocols/DRMSyncobj.cpp +++ b/src/protocols/DRMSyncobj.cpp @@ -24,8 +24,8 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SPsetSetReleasePoint([this](CWpLinuxDrmSyncobjSurfaceV1* r, wl_resource* timeline_, uint32_t hi, uint32_t lo) { @@ -35,12 +35,13 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP(pendingRelease.resource->timeline, pendingRelease.point)); }); listeners.surfacePrecommit = surface->events.precommit.registerListener([this](std::any d) { - if ((pending.acquireTimeline || pending.releaseTimeline) && !surface->pending.texture) { + if ((pendingAcquire.resource || pendingRelease.resource) && !surface->pending.texture) { resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_BUFFER, "Missing buffer"); surface->pending.rejected = true; return; @@ -49,55 +50,57 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SPpending.newBuffer) return; // this commit does not change the state here - if (!!pending.acquireTimeline != !!pending.releaseTimeline) { - resource->error(pending.acquireTimeline ? WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT : WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT, + if (!!pendingAcquire.resource != !!pendingRelease.resource) { + resource->error(pendingAcquire.resource ? WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT : WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT, "Missing timeline"); surface->pending.rejected = true; return; } - if (!pending.acquireTimeline) - return; - - if (pending.acquireTimeline && pending.releaseTimeline && pending.acquireTimeline == pending.releaseTimeline) { - if (pending.acquirePoint >= pending.releasePoint) { + if (pendingAcquire.resource && pendingRelease.resource && pendingAcquire.resource == pendingRelease.resource) { + if (pendingAcquire.point >= pendingRelease.point) { resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_CONFLICTING_POINTS, "Acquire and release points are on the same timeline, and acquire >= release"); surface->pending.rejected = true; return; } } - // wait for the acquire timeline to materialize - auto materialized = pending.acquireTimeline->timeline->check(pending.acquirePoint, DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE); - if (!materialized.has_value()) { - LOGM(ERR, "Failed to check the acquire timeline"); - resource->noMemory(); + if (!pendingAcquire.resource) return; + + if (acquireWaiting) + return; // wait for acquire waiter to signal. + + if (pendingAcquire.resource->timeline->addWaiter( + [this]() { + if (surface.expired()) + return; + + surface->unlockPendingState(); + surface->commitPendingState(); + acquireWaiting = false; + }, + pendingAcquire.point, 0u)) { + surface->lockPendingState(); + acquireWaiting = true; } + }); - if (materialized.value()) - return; + listeners.surfaceRoleCommit = surface->events.roleCommit.registerListener([this](std::any d) { + if (pendingAcquire.resource) + acquire = std::exchange(pendingAcquire, {}); - surface->lockPendingState(); - pending.acquireTimeline->timeline->addWaiter([this]() { surface->unlockPendingState(); }, pending.acquirePoint, DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE); + if (pendingRelease.resource) + release = std::exchange(pendingRelease, {}); }); +} - listeners.surfaceCommit = surface->events.roleCommit.registerListener([this](std::any d) { - // apply timelines if new ones have been attached, otherwise don't touch - // the current ones - if (pending.releaseTimeline) { - current.releaseTimeline = pending.releaseTimeline; - current.releasePoint = pending.releasePoint; - } +CDRMSyncobjSurfaceResource::~CDRMSyncobjSurfaceResource() { + if (acquire.resource) + acquire.resource->timeline->removeAllWaiters(); - if (pending.acquireTimeline) { - current.acquireTimeline = pending.acquireTimeline; - current.acquirePoint = pending.acquirePoint; - } - - pending.releaseTimeline.reset(); - pending.acquireTimeline.reset(); - }); + if (pendingAcquire.resource) + pendingAcquire.resource->timeline->removeAllWaiters(); } bool CDRMSyncobjSurfaceResource::good() { diff --git a/src/protocols/DRMSyncobj.hpp b/src/protocols/DRMSyncobj.hpp index 9895dff17e3..06a3def2565 100644 --- a/src/protocols/DRMSyncobj.hpp +++ b/src/protocols/DRMSyncobj.hpp @@ -2,6 +2,7 @@ #include #include "WaylandProtocol.hpp" +#include "helpers/sync/SyncReleaser.hpp" #include "linux-drm-syncobj-v1.hpp" #include "../helpers/signal/Signal.hpp" #include @@ -13,21 +14,32 @@ class CSyncTimeline; class CDRMSyncobjSurfaceResource { public: CDRMSyncobjSurfaceResource(SP resource_, SP surface_); + ~CDRMSyncobjSurfaceResource(); bool good(); WP surface; - struct { - WP acquireTimeline, releaseTimeline; - uint64_t acquirePoint = 0, releasePoint = 0; - } current, pending; + struct STimeLineState { + WP resource; + uint64_t point = 0; + + STimeLineState() = default; + STimeLineState(STimeLineState&&) noexcept = default; + STimeLineState& operator=(STimeLineState&&) noexcept = default; + STimeLineState(const STimeLineState&) = delete; + STimeLineState& operator=(const STimeLineState&) = delete; + ~STimeLineState() = default; + } acquire, release, pendingAcquire, pendingRelease; + + std::vector> releasePoints; private: SP resource; + bool acquireWaiting = false; struct { CHyprSignalListener surfacePrecommit; - CHyprSignalListener surfaceCommit; + CHyprSignalListener surfaceRoleCommit; } listeners; }; diff --git a/src/protocols/core/Compositor.cpp b/src/protocols/core/Compositor.cpp index 92c3c425d7c..87adaeae1be 100644 --- a/src/protocols/core/Compositor.cpp +++ b/src/protocols/core/Compositor.cpp @@ -113,8 +113,7 @@ CWLSurfaceResource::CWLSurfaceResource(SP resource_) : resource(reso return; } - if (stateLocks <= 0) - commitPendingState(); + commitPendingState(); }); resource->setDamage([this](CWlSurface* r, int32_t x, int32_t y, int32_t w, int32_t h) { pending.damage.add(CBox{x, y, w, h}); }); @@ -412,16 +411,17 @@ CRegion CWLSurfaceResource::accumulateCurrentBufferDamage() { } void CWLSurfaceResource::lockPendingState() { - stateLocks++; + stateLocked = true; } void CWLSurfaceResource::unlockPendingState() { - stateLocks--; - if (stateLocks <= 0) - commitPendingState(); + stateLocked = false; } void CWLSurfaceResource::commitPendingState() { + if (stateLocked && syncobj) + return; + static auto PDROP = CConfigValue("render:allow_early_buffer_release"); auto const previousBuffer = current.buffer; current = pending; @@ -433,8 +433,8 @@ void CWLSurfaceResource::commitPendingState() { events.roleCommit.emit(); - if (syncobj && syncobj->current.releaseTimeline && syncobj->current.releaseTimeline->timeline && current.buffer && current.buffer->buffer) - current.buffer->releaser = makeShared(syncobj->current.releaseTimeline->timeline, syncobj->current.releasePoint); + if (syncobj && syncobj->release.resource && syncobj->release.resource->timeline && current.buffer && current.buffer->buffer) + current.buffer->releaser = std::move(syncobj->releasePoints); if (current.texture) current.texture->m_eTransform = wlTransformToHyprutils(current.transform); diff --git a/src/protocols/core/Compositor.hpp b/src/protocols/core/Compositor.hpp index aaaf9b1a9cb..fdc0c5001d3 100644 --- a/src/protocols/core/Compositor.hpp +++ b/src/protocols/core/Compositor.hpp @@ -131,6 +131,7 @@ class CWLSurfaceResource { void presentFeedback(timespec* when, PHLMONITOR pMonitor, bool discarded = false); void lockPendingState(); void unlockPendingState(); + void commitPendingState(); // returns a pair: found surface (null if not found) and surface local coords. // localCoords param is relative to 0,0 of this surface @@ -144,13 +145,12 @@ class CWLSurfaceResource { // this stupid-ass hack is used WP lastBuffer; - int stateLocks = 0; + bool stateLocked = false; void destroy(); void releaseBuffers(bool onlyCurrent = true); void dropPendingBuffer(); void dropCurrentBuffer(); - void commitPendingState(); void bfHelper(std::vector> const& nodes, std::function, const Vector2D&, void*)> fn, void* data); void updateCursorShm(CRegion damage = CBox{0, 0, INT16_MAX, INT16_MAX}); diff --git a/src/protocols/types/Buffer.hpp b/src/protocols/types/Buffer.hpp index 7d84dce7390..03f4fe97d0e 100644 --- a/src/protocols/types/Buffer.hpp +++ b/src/protocols/types/Buffer.hpp @@ -43,8 +43,8 @@ class CHLBufferReference { CHLBufferReference(SP buffer, SP surface); ~CHLBufferReference(); - WP buffer; - SP releaser; + WP buffer; + std::vector> releaser; private: WP surface; diff --git a/src/render/Renderer.cpp b/src/render/Renderer.cpp index f01a0d1225b..09e3b04e779 100644 --- a/src/render/Renderer.cpp +++ b/src/render/Renderer.cpp @@ -1574,10 +1574,11 @@ bool CHyprRenderer::commitPendingAndDoExplicitSync(PHLMONITOR pMonitor) { Debug::log(TRACE, "Explicit: can't add sync, EGLSync failed"); else { for (auto const& e : explicitPresented) { - if (!e->current.buffer || !e->current.buffer->releaser) + if (!e->current.buffer || e->current.buffer->releaser.empty()) continue; - e->current.buffer->releaser->addReleaseSync(sync); + for (auto& r : e->current.buffer->releaser) + r->addReleaseSync(sync); } } diff --git a/src/render/pass/SurfacePassElement.cpp b/src/render/pass/SurfacePassElement.cpp index a5d3d7c001a..e6e03b99773 100644 --- a/src/render/pass/SurfacePassElement.cpp +++ b/src/render/pass/SurfacePassElement.cpp @@ -50,8 +50,8 @@ void CSurfacePassElement::draw(const CRegion& damage) { return; // explicit sync: wait for the timeline, if any - if (data.surface->syncobj && data.surface->syncobj->current.acquireTimeline) { - if (!g_pHyprOpenGL->waitForTimelinePoint(data.surface->syncobj->current.acquireTimeline->timeline, data.surface->syncobj->current.acquirePoint)) { + if (data.surface->syncobj && data.surface->syncobj->acquire.resource) { + if (!g_pHyprOpenGL->waitForTimelinePoint(data.surface->syncobj->acquire.resource->timeline, data.surface->syncobj->acquire.point)) { Debug::log(ERR, "Renderer: failed to wait for explicit timeline"); return; } From e156ace712a9dbe04b9a8759072c92f291507881 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Sat, 22 Feb 2025 19:52:49 +0100 Subject: [PATCH 2/3] syncobj: lets optimize it further lets use unique_pointers all over, add defaulted destructors and make fromResource directly check existing timelines instead of C style casting and return a WP from that. instead of moving the releasepoint vector lets swap it. --- src/protocols/DRMSyncobj.cpp | 31 ++++++++++++++++++------------- src/protocols/DRMSyncobj.hpp | 22 +++++++++++----------- src/protocols/core/Compositor.cpp | 2 +- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/protocols/DRMSyncobj.cpp b/src/protocols/DRMSyncobj.cpp index 63f0946b58c..af2fadfaa7d 100644 --- a/src/protocols/DRMSyncobj.cpp +++ b/src/protocols/DRMSyncobj.cpp @@ -8,7 +8,8 @@ #include using namespace Hyprutils::OS; -CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP resource_, SP surface_) : surface(surface_), resource(resource_) { +CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UP&& resource_, SP surface_) : + surface(surface_), resource(std::move(resource_)) { if UNLIKELY (!good()) return; @@ -107,7 +108,7 @@ bool CDRMSyncobjSurfaceResource::good() { return resource->resource(); } -CDRMSyncobjTimelineResource::CDRMSyncobjTimelineResource(SP resource_, CFileDescriptor&& fd_) : fd(std::move(fd_)), resource(resource_) { +CDRMSyncobjTimelineResource::CDRMSyncobjTimelineResource(UP&& resource_, CFileDescriptor&& fd_) : fd(std::move(fd_)), resource(std::move(resource_)) { if UNLIKELY (!good()) return; @@ -124,16 +125,20 @@ CDRMSyncobjTimelineResource::CDRMSyncobjTimelineResource(SP CDRMSyncobjTimelineResource::fromResource(wl_resource* res) { - auto data = (CDRMSyncobjTimelineResource*)(((CWpLinuxDrmSyncobjTimelineV1*)wl_resource_get_user_data(res))->data()); - return data ? data->self.lock() : nullptr; +WP CDRMSyncobjTimelineResource::fromResource(wl_resource* res) { + for (const auto& t : PROTO::sync->m_vTimelines) { + if (t && t->resource && t->resource->resource() == res) + return t; + } + + return {}; } bool CDRMSyncobjTimelineResource::good() { return resource->resource(); } -CDRMSyncobjManagerResource::CDRMSyncobjManagerResource(SP resource_) : resource(resource_) { +CDRMSyncobjManagerResource::CDRMSyncobjManagerResource(UP&& resource_) : resource(std::move(resource_)) { if UNLIKELY (!good()) return; @@ -157,28 +162,28 @@ CDRMSyncobjManagerResource::CDRMSyncobjManagerResource(SP(makeShared(resource->client(), resource->version(), id), SURF); + const auto& RESOURCE = PROTO::sync->m_vSurfaces.emplace_back( + makeUnique(makeUnique(resource->client(), resource->version(), id), SURF)); if UNLIKELY (!RESOURCE->good()) { resource->noMemory(); + PROTO::sync->m_vSurfaces.pop_back(); return; } - PROTO::sync->m_vSurfaces.emplace_back(RESOURCE); SURF->syncobj = RESOURCE; LOGM(LOG, "New linux_syncobj at {:x} for surface {:x}", (uintptr_t)RESOURCE.get(), (uintptr_t)SURF.get()); }); resource->setImportTimeline([this](CWpLinuxDrmSyncobjManagerV1* r, uint32_t id, int32_t fd) { - auto RESOURCE = makeShared(makeShared(resource->client(), resource->version(), id), CFileDescriptor{fd}); + const auto& RESOURCE = PROTO::sync->m_vTimelines.emplace_back( + makeUnique(makeUnique(resource->client(), resource->version(), id), CFileDescriptor{fd})); if UNLIKELY (!RESOURCE->good()) { resource->noMemory(); + PROTO::sync->m_vTimelines.pop_back(); return; } - PROTO::sync->m_vTimelines.emplace_back(RESOURCE); - RESOURCE->self = RESOURCE; - LOGM(LOG, "New linux_drm_timeline at {:x}", (uintptr_t)RESOURCE.get()); }); } @@ -192,7 +197,7 @@ CDRMSyncobjProtocol::CDRMSyncobjProtocol(const wl_interface* iface, const int& v } void CDRMSyncobjProtocol::bindManager(wl_client* client, void* data, uint32_t ver, uint32_t id) { - const auto RESOURCE = m_vManagers.emplace_back(makeShared(makeShared(client, ver, id))); + const auto& RESOURCE = m_vManagers.emplace_back(makeUnique(makeUnique(client, ver, id))); if UNLIKELY (!RESOURCE->good()) { wl_client_post_no_memory(client); diff --git a/src/protocols/DRMSyncobj.hpp b/src/protocols/DRMSyncobj.hpp index 06a3def2565..af4ee3567bd 100644 --- a/src/protocols/DRMSyncobj.hpp +++ b/src/protocols/DRMSyncobj.hpp @@ -13,7 +13,7 @@ class CSyncTimeline; class CDRMSyncobjSurfaceResource { public: - CDRMSyncobjSurfaceResource(SP resource_, SP surface_); + CDRMSyncobjSurfaceResource(UP&& resource_, SP surface_); ~CDRMSyncobjSurfaceResource(); bool good(); @@ -34,7 +34,7 @@ class CDRMSyncobjSurfaceResource { std::vector> releasePoints; private: - SP resource; + UP resource; bool acquireWaiting = false; struct { @@ -45,33 +45,33 @@ class CDRMSyncobjSurfaceResource { class CDRMSyncobjTimelineResource { public: - CDRMSyncobjTimelineResource(SP resource_, Hyprutils::OS::CFileDescriptor&& fd_); + CDRMSyncobjTimelineResource(UP&& resource_, Hyprutils::OS::CFileDescriptor&& fd_); ~CDRMSyncobjTimelineResource() = default; - static SP fromResource(wl_resource*); + static WP fromResource(wl_resource*); bool good(); - WP self; Hyprutils::OS::CFileDescriptor fd; SP timeline; private: - SP resource; + UP resource; }; class CDRMSyncobjManagerResource { public: - CDRMSyncobjManagerResource(SP resource_); + CDRMSyncobjManagerResource(UP&& resource_); bool good(); private: - SP resource; + UP resource; }; class CDRMSyncobjProtocol : public IWaylandProtocol { public: CDRMSyncobjProtocol(const wl_interface* iface, const int& ver, const std::string& name); + ~CDRMSyncobjProtocol() = default; virtual void bindManager(wl_client* client, void* data, uint32_t ver, uint32_t id); @@ -81,9 +81,9 @@ class CDRMSyncobjProtocol : public IWaylandProtocol { void destroyResource(CDRMSyncobjSurfaceResource* resource); // - std::vector> m_vManagers; - std::vector> m_vTimelines; - std::vector> m_vSurfaces; + std::vector> m_vManagers; + std::vector> m_vTimelines; + std::vector> m_vSurfaces; // int drmFD = -1; diff --git a/src/protocols/core/Compositor.cpp b/src/protocols/core/Compositor.cpp index 87adaeae1be..0dddf8b42c1 100644 --- a/src/protocols/core/Compositor.cpp +++ b/src/protocols/core/Compositor.cpp @@ -434,7 +434,7 @@ void CWLSurfaceResource::commitPendingState() { events.roleCommit.emit(); if (syncobj && syncobj->release.resource && syncobj->release.resource->timeline && current.buffer && current.buffer->buffer) - current.buffer->releaser = std::move(syncobj->releasePoints); + current.buffer->releaser.swap(syncobj->releasePoints); if (current.texture) current.texture->m_eTransform = wlTransformToHyprutils(current.transform); From 40df1f771c281d11c0b67569bd7a061a3c102994 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Sat, 22 Feb 2025 20:36:03 +0100 Subject: [PATCH 3/3] syncobj: remove early buffer release remove early buffer release config that previously tried to workaround flickers while it was more of an syncobj issue, it also cant be used since the buffer really is used for release point syncing in the renderer later. and use std::move instead of std::exchange to avoid a local temporar copy being created inside std::exchange. --- src/config/ConfigDescriptions.hpp | 6 ------ src/config/ConfigManager.cpp | 1 - src/protocols/DRMSyncobj.cpp | 12 ++++++++---- src/protocols/core/Compositor.cpp | 17 +++-------------- 4 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/config/ConfigDescriptions.hpp b/src/config/ConfigDescriptions.hpp index 4e591fa8107..f7e647b2b2f 100644 --- a/src/config/ConfigDescriptions.hpp +++ b/src/config/ConfigDescriptions.hpp @@ -1348,12 +1348,6 @@ inline static const std::vector CONFIG_OPTIONS = { .type = CONFIG_OPTION_INT, .data = SConfigOptionDescription::SRangeData{2, 0, 2}, }, - SConfigOptionDescription{ - .value = "render:allow_early_buffer_release", - .description = "Allow early buffer release event. Fixes stuttering and missing frames for some apps. May cause graphical glitches and memory leaks in others", - .type = CONFIG_OPTION_BOOL, - .data = SConfigOptionDescription::SBoolData{true}, - }, /* * cursor: diff --git a/src/config/ConfigManager.cpp b/src/config/ConfigManager.cpp index 9b547ffbb75..277098f5260 100644 --- a/src/config/ConfigManager.cpp +++ b/src/config/ConfigManager.cpp @@ -664,7 +664,6 @@ CConfigManager::CConfigManager() { m_pConfig->addConfigValue("render:expand_undersized_textures", Hyprlang::INT{1}); m_pConfig->addConfigValue("render:xp_mode", Hyprlang::INT{0}); m_pConfig->addConfigValue("render:ctm_animation", Hyprlang::INT{2}); - m_pConfig->addConfigValue("render:allow_early_buffer_release", Hyprlang::INT{1}); m_pConfig->addConfigValue("ecosystem:no_update_news", Hyprlang::INT{0}); m_pConfig->addConfigValue("ecosystem:no_donation_nag", Hyprlang::INT{0}); diff --git a/src/protocols/DRMSyncobj.cpp b/src/protocols/DRMSyncobj.cpp index af2fadfaa7d..0e8b4cb47fe 100644 --- a/src/protocols/DRMSyncobj.cpp +++ b/src/protocols/DRMSyncobj.cpp @@ -88,11 +88,15 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UPevents.roleCommit.registerListener([this](std::any d) { - if (pendingAcquire.resource) - acquire = std::exchange(pendingAcquire, {}); + if (pendingAcquire.resource) { + acquire = std::move(pendingAcquire); + pendingAcquire = {}; + } - if (pendingRelease.resource) - release = std::exchange(pendingRelease, {}); + if (pendingRelease.resource) { + release = std::move(pendingRelease); + pendingRelease = {}; + } }); } diff --git a/src/protocols/core/Compositor.cpp b/src/protocols/core/Compositor.cpp index 0dddf8b42c1..e3297da8a44 100644 --- a/src/protocols/core/Compositor.cpp +++ b/src/protocols/core/Compositor.cpp @@ -11,7 +11,6 @@ #include "../PresentationTime.hpp" #include "../DRMSyncobj.hpp" #include "../../render/Renderer.hpp" -#include "config/ConfigValue.hpp" #include class CDefaultSurfaceRole : public ISurfaceRole { @@ -422,14 +421,12 @@ void CWLSurfaceResource::commitPendingState() { if (stateLocked && syncobj) return; - static auto PDROP = CConfigValue("render:allow_early_buffer_release"); - auto const previousBuffer = current.buffer; - current = pending; + auto const previousBuffer = current.buffer; + current = pending; pending.damage.clear(); pending.bufferDamage.clear(); pending.newBuffer = false; - if (!*PDROP) - dropPendingBuffer(); // at this point current.buffer holds the same SP and we don't use pending anymore + dropPendingBuffer(); // at this point current.buffer holds the same SP and we don't use pending anymore events.roleCommit.emit(); @@ -447,14 +444,6 @@ void CWLSurfaceResource::commitPendingState() { // TODO: don't update the entire texture if (role->role() == SURFACE_ROLE_CURSOR && !DAMAGE.empty()) updateCursorShm(DAMAGE); - - // release the buffer if it's synchronous as update() has done everything thats needed - // so we can let the app know we're done. - // Some clients aren't ready to receive a release this early. Should be fine to release it on the next commitPendingState. - if (current.buffer->buffer->isSynchronous() && *PDROP) { - dropCurrentBuffer(); - dropPendingBuffer(); // at this point current.buffer holds the same SP and we don't use pending anymore - } } // TODO: we should _accumulate_ and not replace above if sync