From ccbdba7ee2ccb835306de89a6023134fa6b8006f Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Fri, 21 Mar 2025 20:19:53 +0100 Subject: [PATCH] syncobj: refactor point timelines (#9689) no need to store the resource, just store the csynctimeline as a shared pointer and make the timeline own the syncobj fd. --- src/helpers/sync/SyncTimeline.cpp | 11 +++--- src/helpers/sync/SyncTimeline.hpp | 5 +-- src/protocols/DRMSyncobj.cpp | 59 +++++++------------------------ src/protocols/DRMSyncobj.hpp | 13 +++---- 4 files changed, 27 insertions(+), 61 deletions(-) diff --git a/src/helpers/sync/SyncTimeline.cpp b/src/helpers/sync/SyncTimeline.cpp index d11205bc..224b2c0f 100644 --- a/src/helpers/sync/SyncTimeline.cpp +++ b/src/helpers/sync/SyncTimeline.cpp @@ -19,12 +19,13 @@ SP CSyncTimeline::create(int drmFD_) { return timeline; } -SP CSyncTimeline::create(int drmFD_, int drmSyncobjFD) { - auto timeline = SP(new CSyncTimeline); - timeline->drmFD = drmFD_; - timeline->self = timeline; +SP CSyncTimeline::create(int drmFD_, CFileDescriptor&& drmSyncobjFD) { + auto timeline = SP(new CSyncTimeline); + timeline->drmFD = drmFD_; + timeline->syncobjFd = std::move(drmSyncobjFD); + timeline->self = timeline; - if (drmSyncobjFDToHandle(drmFD_, drmSyncobjFD, &timeline->handle)) { + if (drmSyncobjFDToHandle(drmFD_, timeline->syncobjFd.get(), &timeline->handle)) { Debug::log(ERR, "CSyncTimeline: failed to create a drm syncobj from fd??"); return nullptr; } diff --git a/src/helpers/sync/SyncTimeline.hpp b/src/helpers/sync/SyncTimeline.hpp index bfd70416..a2422149 100644 --- a/src/helpers/sync/SyncTimeline.hpp +++ b/src/helpers/sync/SyncTimeline.hpp @@ -17,7 +17,7 @@ struct wl_event_source; class CSyncTimeline { public: static SP create(int drmFD_); - static SP create(int drmFD_, int drmSyncobjFD); + static SP create(int drmFD_, Hyprutils::OS::CFileDescriptor&& drmSyncobjFD); ~CSyncTimeline(); struct SWaiter { @@ -40,7 +40,8 @@ class CSyncTimeline { bool transfer(SP from, uint64_t fromPoint, uint64_t toPoint); void signal(uint64_t point); - int drmFD = -1; + int drmFD = -1; + Hyprutils::OS::CFileDescriptor syncobjFd; uint32_t handle = 0; WP self; diff --git a/src/protocols/DRMSyncobj.cpp b/src/protocols/DRMSyncobj.cpp index 11ae02c1..943aad4d 100644 --- a/src/protocols/DRMSyncobj.cpp +++ b/src/protocols/DRMSyncobj.cpp @@ -9,50 +9,27 @@ #include using namespace Hyprutils::OS; -CDRMSyncPointState::CDRMSyncPointState(WP resource_, uint64_t point_) : m_resource(resource_), m_point(point_) {} +CDRMSyncPointState::CDRMSyncPointState(SP timeline_, uint64_t point_) : m_timeline(timeline_), m_point(point_) {} const uint64_t& CDRMSyncPointState::point() { return m_point; } -WP CDRMSyncPointState::resource() { - return m_resource; -} - WP CDRMSyncPointState::timeline() { - if (expired()) { - Debug::log(ERR, "CDRMSyncPointState: getting a timeline on a expired point"); - return {}; - } - - return m_resource->timeline; -} - -bool CDRMSyncPointState::expired() { - return m_resource.expired() || !m_resource->timeline; + return m_timeline; } UP CDRMSyncPointState::createSyncRelease() { - if (expired()) { - Debug::log(ERR, "CDRMSyncPointState: creating a sync releaser on an expired point"); - return nullptr; - } - if (m_releaseTaken) Debug::log(ERR, "CDRMSyncPointState: creating a sync releaser on an already created SyncRelease"); m_releaseTaken = true; - return makeUnique(m_resource->timeline, m_point); + return makeUnique(m_timeline, m_point); } bool CDRMSyncPointState::addWaiter(const std::function& waiter) { - if (expired()) { - Debug::log(ERR, "CDRMSyncPointState: adding a waiter on an expired point"); - return false; - } - m_acquireCommitted = true; - return m_resource->timeline->addWaiter(waiter, m_point, 0u); + return m_timeline->addWaiter(waiter, m_point, 0u); } bool CDRMSyncPointState::comitted() { @@ -60,21 +37,11 @@ bool CDRMSyncPointState::comitted() { } CFileDescriptor CDRMSyncPointState::exportAsFD() { - if (expired()) { - Debug::log(ERR, "CDRMSyncPointState: exporting a FD on an expired point"); - return {}; - } - - return m_resource->timeline->exportAsSyncFileFD(m_point); + return m_timeline->exportAsSyncFileFD(m_point); } void CDRMSyncPointState::signal() { - if (expired()) { - Debug::log(ERR, "CDRMSyncPointState: signaling on an expired point"); - return; - } - - m_resource->timeline->signal(m_point); + m_timeline->signal(m_point); } CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UP&& resource_, SP surface_) : @@ -94,7 +61,7 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UPtimeline, ((uint64_t)hi << 32) | (uint64_t)lo}; }); resource->setSetReleasePoint([this](CWpLinuxDrmSyncobjSurfaceV1* r, wl_resource* timeline_, uint32_t hi, uint32_t lo) { @@ -104,7 +71,7 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UPtimeline, ((uint64_t)hi << 32) | (uint64_t)lo}; }); listeners.surfacePrecommit = surface->events.precommit.registerListener([this](std::any d) { @@ -126,12 +93,12 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UPpending.buffer->acquire = makeUnique(std::move(pendingAcquire)); pendingAcquire = {}; } - if (!pendingRelease.expired()) { + if (pendingRelease.timeline()) { surface->pending.buffer->release = makeUnique(std::move(pendingRelease)); pendingRelease = {}; } @@ -158,8 +125,8 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UPbuffer && s->buffer->acquire && !s->buffer->acquire->expired()) - s->buffer->acquire->resource()->timeline->removeAllWaiters(); + if (s && s->buffer && s->buffer->acquire) + s->buffer->acquire->timeline()->removeAllWaiters(); } pendingStates.clear(); @@ -212,7 +179,7 @@ CDRMSyncobjTimelineResource::CDRMSyncobjTimelineResource(UPsetOnDestroy([this](CWpLinuxDrmSyncobjTimelineV1* r) { PROTO::sync->destroyResource(this); }); resource->setDestroy([this](CWpLinuxDrmSyncobjTimelineV1* r) { PROTO::sync->destroyResource(this); }); - timeline = CSyncTimeline::create(PROTO::sync->drmFD, fd.get()); + timeline = CSyncTimeline::create(PROTO::sync->drmFD, std::move(fd)); if (!timeline) { resource->error(WP_LINUX_DRM_SYNCOBJ_MANAGER_V1_ERROR_INVALID_TIMELINE, "Timeline failed importing"); diff --git a/src/protocols/DRMSyncobj.hpp b/src/protocols/DRMSyncobj.hpp index 4d9c2ffc..d2259a34 100644 --- a/src/protocols/DRMSyncobj.hpp +++ b/src/protocols/DRMSyncobj.hpp @@ -17,13 +17,11 @@ struct SSurfaceState; class CDRMSyncPointState { public: CDRMSyncPointState() = default; - CDRMSyncPointState(WP resource_, uint64_t point_); + CDRMSyncPointState(SP timeline_, uint64_t point_); ~CDRMSyncPointState() = default; const uint64_t& point(); - WP resource(); WP timeline(); - bool expired(); Hyprutils::Memory::CUniquePointer createSyncRelease(); bool addWaiter(const std::function& waiter); bool comitted(); @@ -31,11 +29,10 @@ class CDRMSyncPointState { void signal(); private: - WP m_resource = {}; - uint64_t m_point = 0; - WP m_timeline = {}; - bool m_acquireCommitted = false; - bool m_releaseTaken = false; + SP m_timeline = {}; + uint64_t m_point = 0; + bool m_acquireCommitted = false; + bool m_releaseTaken = false; }; class CDRMSyncobjSurfaceResource {