Skip to content

Commit da86db4

Browse files
ikalcoikalco
authored
core: refactor and improve surface commit (#9805)
* make CHLBufferReference not a SP anymore * copy over release and acquire points in CHLBufferReference * use CHLBufferReference in screencopy and toplevel export TODO: use CHLBufferReference in direct scanout properly the only problem is the scanout buffer release timing, specifically the onBackendRelease mechanism * cleanup SSurfaceState and surface pending commit tracking * move surface code from DRMSyncobj, and move acquire to SSurfaceState * use queue for comitted pending surface states like proto says "The content update is placed in a queue until it becomes active." - wl_surface::commit * drop, not release, prev buffer if 2nd buffer wl_surface.attach is sent "A wl_buffer that has been attached and then replaced by another attach instead of committed will not receive a release event, and is not used by the compositor." - wl_surface::attach
1 parent 70ae99f commit da86db4

15 files changed

+277
-215
lines changed

‎src/helpers/Monitor.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,13 +1348,13 @@ bool CMonitor::attemptDirectScanout() {
13481348
return false;
13491349

13501350
// we can't scanout shm buffers.
1351-
const auto params = PSURFACE->current.buffer->buffer->dmabuf();
1351+
const auto params = PSURFACE->current.buffer->dmabuf();
13521352
if (!params.success || !PSURFACE->current.texture->m_pEglImage /* dmabuf */)
13531353
return false;
13541354

1355-
Debug::log(TRACE, "attemptDirectScanout: surface {:x} passed, will attempt, buffer {}", (uintptr_t)PSURFACE.get(), (uintptr_t)PSURFACE->current.buffer->buffer.get());
1355+
Debug::log(TRACE, "attemptDirectScanout: surface {:x} passed, will attempt, buffer {}", (uintptr_t)PSURFACE.get(), (uintptr_t)PSURFACE->current.buffer.buffer.get());
13561356

1357-
auto PBUFFER = PSURFACE->current.buffer->buffer;
1357+
auto PBUFFER = PSURFACE->current.buffer.buffer;
13581358

13591359
if (PBUFFER == output->state->state().buffer) {
13601360
if (scanoutNeedsCursorUpdate) {
@@ -1407,10 +1407,10 @@ bool CMonitor::attemptDirectScanout() {
14071407

14081408
auto explicitOptions = g_pHyprRenderer->getExplicitSyncSettings(output);
14091409

1410-
bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->current.buffer && PSURFACE->current.buffer->acquire && explicitOptions.explicitKMSEnabled;
1410+
bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->current.buffer && PSURFACE->current.acquire && explicitOptions.explicitKMSEnabled;
14111411
if (DOEXPLICIT) {
14121412
// wait for surface's explicit fence if present
1413-
inFence = PSURFACE->current.buffer->acquire->exportAsFD();
1413+
inFence = PSURFACE->current.acquire.exportAsFD();
14141414
if (inFence.isValid()) {
14151415
Debug::log(TRACE, "attemptDirectScanout: setting IN_FENCE for aq to {}", inFence.get());
14161416
output->state->setExplicitInFence(inFence.get());

‎src/protocols/DRMSyncobj.cpp

Lines changed: 23 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -75,100 +75,43 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(UP<CWpLinuxDrmSyncobjSurf
7575
});
7676

7777
listeners.surfacePrecommit = surface->events.precommit.registerListener([this](std::any d) {
78-
const bool PENDING_HAS_NEW_BUFFER = surface->pending.updated & SSurfaceState::eUpdatedProperties::SURFACE_UPDATED_BUFFER;
79-
80-
if (!surface->pending.buffer && PENDING_HAS_NEW_BUFFER && !surface->pending.texture) {
81-
removeAllWaiters();
82-
surface->commitPendingState(surface->pending);
83-
return; // null buffer attached.
78+
if (!surface->pending.updated.buffer || !surface->pending.buffer) {
79+
if (pendingAcquire.timeline() || pendingRelease.timeline()) {
80+
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_BUFFER, "Missing buffer");
81+
surface->pending.rejected = true;
82+
}
83+
return;
8484
}
8585

86-
if (!surface->pending.buffer && !PENDING_HAS_NEW_BUFFER && surface->current.buffer) {
87-
surface->current.bufferDamage.clear();
88-
surface->current.damage.clear();
89-
surface->commitPendingState(surface->current);
90-
return; // no new buffer, but we still have current around and a commit happend, commit current again.
86+
if (!pendingAcquire.timeline()) {
87+
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT, "Missing acquire timeline");
88+
surface->pending.rejected = true;
89+
return;
9190
}
9291

93-
if (!surface->pending.buffer && !PENDING_HAS_NEW_BUFFER && !surface->current.buffer) {
94-
surface->commitPendingState(surface->pending); // no pending buffer, no current buffer. probably first commit
92+
if (!pendingRelease.timeline()) {
93+
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT, "Missing release timeline");
94+
surface->pending.rejected = true;
9595
return;
9696
}
9797

98-
if (pendingAcquire.timeline()) {
99-
surface->pending.buffer->acquire = makeUnique<CDRMSyncPointState>(std::move(pendingAcquire));
100-
pendingAcquire = {};
98+
if (pendingAcquire.timeline() == pendingRelease.timeline() && pendingAcquire.point() >= pendingRelease.point()) {
99+
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_CONFLICTING_POINTS, "Acquire and release points are on the same timeline, and acquire >= release");
100+
surface->pending.rejected = true;
101+
return;
101102
}
102103

103-
if (pendingRelease.timeline()) {
104-
surface->pending.buffer->release = makeUnique<CDRMSyncPointState>(std::move(pendingRelease));
105-
pendingRelease = {};
106-
}
104+
surface->pending.updated.acquire = true;
105+
surface->pending.acquire = pendingAcquire;
106+
pendingAcquire = {};
107107

108-
if (protocolError())
109-
return;
108+
surface->pending.buffer.release = pendingRelease;
109+
pendingRelease = {};
110110

111-
const auto& state = pendingStates.emplace_back(makeShared<SSurfaceState>(surface->pending));
112-
surface->pending.damage.clear();
113-
surface->pending.bufferDamage.clear();
114-
surface->pending.updated &= ~SSurfaceState::eUpdatedProperties::SURFACE_UPDATED_BUFFER;
115-
surface->pending.updated &= ~SSurfaceState::eUpdatedProperties::SURFACE_UPDATED_DAMAGE;
116-
surface->pending.buffer.reset();
117-
118-
state->buffer->buffer->syncReleaser = state->buffer->release->createSyncRelease();
119-
state->buffer->acquire->addWaiter([this, surf = surface, wp = CWeakPointer<SSurfaceState>(*std::prev(pendingStates.end()))] {
120-
if (!surf)
121-
return;
122-
123-
surf->commitPendingState(*wp.lock());
124-
std::erase(pendingStates, wp);
125-
});
111+
surface->pending.buffer->syncReleaser = surface->pending.buffer.release.createSyncRelease();
126112
});
127113
}
128114

129-
void CDRMSyncobjSurfaceResource::removeAllWaiters() {
130-
for (auto& s : pendingStates) {
131-
if (s && s->buffer && s->buffer->acquire)
132-
s->buffer->acquire->timeline()->removeAllWaiters();
133-
}
134-
135-
pendingStates.clear();
136-
}
137-
138-
CDRMSyncobjSurfaceResource::~CDRMSyncobjSurfaceResource() {
139-
removeAllWaiters();
140-
}
141-
142-
bool CDRMSyncobjSurfaceResource::protocolError() {
143-
if (!surface->pending.buffer) {
144-
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_BUFFER, "Missing buffer");
145-
surface->pending.rejected = true;
146-
return true;
147-
}
148-
149-
if (!surface->pending.buffer->acquire || !surface->pending.buffer->acquire->timeline()) {
150-
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT, "Missing acquire timeline");
151-
surface->pending.rejected = true;
152-
return true;
153-
}
154-
155-
if (!surface->pending.buffer->release || !surface->pending.buffer->release->timeline()) {
156-
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT, "Missing release timeline");
157-
surface->pending.rejected = true;
158-
return true;
159-
}
160-
161-
if (surface->pending.buffer->acquire->timeline() == surface->pending.buffer->release->timeline()) {
162-
if (surface->pending.buffer->acquire->point() >= surface->pending.buffer->release->point()) {
163-
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_CONFLICTING_POINTS, "Acquire and release points are on the same timeline, and acquire >= release");
164-
surface->pending.rejected = true;
165-
return true;
166-
}
167-
}
168-
169-
return false;
170-
}
171-
172115
bool CDRMSyncobjSurfaceResource::good() {
173116
return resource->resource();
174117
}

‎src/protocols/DRMSyncobj.hpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,11 @@
55
#include "../helpers/sync/SyncReleaser.hpp"
66
#include "linux-drm-syncobj-v1.hpp"
77
#include "../helpers/signal/Signal.hpp"
8-
#include "types/SurfaceState.hpp"
98
#include <hyprutils/os/FileDescriptor.hpp>
10-
#include <list>
119

1210
class CWLSurfaceResource;
1311
class CDRMSyncobjTimelineResource;
1412
class CSyncTimeline;
15-
struct SSurfaceState;
1613

1714
class CDRMSyncPointState {
1815
public:
@@ -28,6 +25,10 @@ class CDRMSyncPointState {
2825
Hyprutils::OS::CFileDescriptor exportAsFD();
2926
void signal();
3027

28+
operator bool() const {
29+
return m_timeline;
30+
}
31+
3132
private:
3233
SP<CSyncTimeline> m_timeline = {};
3334
uint64_t m_point = 0;
@@ -38,19 +39,15 @@ class CDRMSyncPointState {
3839
class CDRMSyncobjSurfaceResource {
3940
public:
4041
CDRMSyncobjSurfaceResource(UP<CWpLinuxDrmSyncobjSurfaceV1>&& resource_, SP<CWLSurfaceResource> surface_);
41-
~CDRMSyncobjSurfaceResource();
4242

43-
bool protocolError();
4443
bool good();
4544

4645
private:
47-
void removeAllWaiters();
4846
WP<CWLSurfaceResource> surface;
4947
UP<CWpLinuxDrmSyncobjSurfaceV1> resource;
5048

5149
CDRMSyncPointState pendingAcquire;
5250
CDRMSyncPointState pendingRelease;
53-
std::vector<SP<SSurfaceState>> pendingStates;
5451

5552
struct {
5653
CHyprSignalListener surfacePrecommit;

‎src/protocols/Screencopy.cpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,6 @@
1414
#include <algorithm>
1515
#include <functional>
1616

17-
CScreencopyFrame::~CScreencopyFrame() {
18-
if (buffer && buffer->locked())
19-
buffer->unlock();
20-
}
21-
2217
CScreencopyFrame::CScreencopyFrame(SP<CZwlrScreencopyFrameV1> resource_, int32_t overlay_cursor, wl_resource* output, CBox box_) : resource(resource_) {
2318
if UNLIKELY (!good())
2419
return;
@@ -102,8 +97,6 @@ void CScreencopyFrame::copy(CZwlrScreencopyFrameV1* pFrame, wl_resource* buffer_
10297
return;
10398
}
10499

105-
PBUFFER->buffer->lock();
106-
107100
if UNLIKELY (PBUFFER->buffer->size != box.size()) {
108101
LOGM(ERR, "Invalid dimensions in {:x}", (uintptr_t)this);
109102
resource->error(ZWLR_SCREENCOPY_FRAME_V1_ERROR_INVALID_BUFFER, "invalid buffer dimensions");
@@ -146,7 +139,7 @@ void CScreencopyFrame::copy(CZwlrScreencopyFrameV1* pFrame, wl_resource* buffer_
146139
return;
147140
}
148141

149-
buffer = PBUFFER->buffer;
142+
buffer = CHLBufferReference(PBUFFER->buffer.lock());
150143

151144
PROTO::screencopy->m_vFramesAwaitingWrite.emplace_back(self);
152145

@@ -207,7 +200,7 @@ void CScreencopyFrame::copyDmabuf(std::function<void(bool)> callback) {
207200

208201
CRegion fakeDamage = {0, 0, INT16_MAX, INT16_MAX};
209202

210-
if (!g_pHyprRenderer->beginRender(pMonitor.lock(), fakeDamage, RENDER_MODE_TO_BUFFER, buffer.lock(), nullptr, true)) {
203+
if (!g_pHyprRenderer->beginRender(pMonitor.lock(), fakeDamage, RENDER_MODE_TO_BUFFER, buffer.buffer, nullptr, true)) {
211204
LOGM(ERR, "Can't copy: failed to begin rendering to dma frame");
212205
callback(false);
213206
return;

‎src/protocols/Screencopy.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#include "../defines.hpp"
4+
#include "./types/Buffer.hpp"
45
#include "wlr-screencopy-unstable-v1.hpp"
56
#include "WaylandProtocol.hpp"
67

@@ -50,7 +51,6 @@ class CScreencopyClient {
5051
class CScreencopyFrame {
5152
public:
5253
CScreencopyFrame(SP<CZwlrScreencopyFrameV1> resource, int32_t overlay_cursor, wl_resource* output, CBox box);
53-
~CScreencopyFrame();
5454

5555
bool good();
5656

@@ -65,7 +65,7 @@ class CScreencopyFrame {
6565
bool withDamage = false;
6666
bool lockedSWCursors = false;
6767

68-
WP<IHLBuffer> buffer;
68+
CHLBufferReference buffer;
6969
bool bufferDMA = false;
7070
uint32_t shmFormat = 0;
7171
uint32_t dmabufFormat = 0;

‎src/protocols/ToplevelExport.cpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,6 @@ bool CToplevelExportClient::good() {
7373
return resource->resource();
7474
}
7575

76-
CToplevelExportFrame::~CToplevelExportFrame() {
77-
if (buffer && buffer->locked())
78-
buffer->unlock();
79-
}
80-
8176
CToplevelExportFrame::CToplevelExportFrame(SP<CHyprlandToplevelExportFrameV1> resource_, int32_t overlayCursor_, PHLWINDOW pWindow_) : resource(resource_), pWindow(pWindow_) {
8277
if UNLIKELY (!good())
8378
return;
@@ -159,8 +154,6 @@ void CToplevelExportFrame::copy(CHyprlandToplevelExportFrameV1* pFrame, wl_resou
159154
return;
160155
}
161156

162-
PBUFFER->buffer->lock();
163-
164157
if UNLIKELY (PBUFFER->buffer->size != box.size()) {
165158
resource->error(HYPRLAND_TOPLEVEL_EXPORT_FRAME_V1_ERROR_INVALID_BUFFER, "invalid buffer dimensions");
166159
PROTO::toplevelExport->destroyResource(this);
@@ -197,7 +190,7 @@ void CToplevelExportFrame::copy(CHyprlandToplevelExportFrameV1* pFrame, wl_resou
197190
return;
198191
}
199192

200-
buffer = PBUFFER->buffer;
193+
buffer = CHLBufferReference(PBUFFER->buffer.lock());
201194

202195
m_ignoreDamage = ignoreDamage;
203196

@@ -340,7 +333,7 @@ bool CToplevelExportFrame::copyDmabuf(timespec* now) {
340333
g_pPointerManager->damageCursor(PMONITOR->self.lock());
341334
}
342335

343-
if (!g_pHyprRenderer->beginRender(PMONITOR, fakeDamage, RENDER_MODE_TO_BUFFER, buffer.lock()))
336+
if (!g_pHyprRenderer->beginRender(PMONITOR, fakeDamage, RENDER_MODE_TO_BUFFER, buffer.buffer))
344337
return false;
345338

346339
g_pHyprOpenGL->clear(CHyprColor(0, 0, 0, 1.0));

‎src/protocols/ToplevelExport.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ class CToplevelExportClient {
4040
class CToplevelExportFrame {
4141
public:
4242
CToplevelExportFrame(SP<CHyprlandToplevelExportFrameV1> resource_, int32_t overlayCursor, PHLWINDOW pWindow);
43-
~CToplevelExportFrame();
4443

4544
bool good();
4645

@@ -55,7 +54,7 @@ class CToplevelExportFrame {
5554
bool m_ignoreDamage = false;
5655
bool lockedSWCursors = false;
5756

58-
WP<IHLBuffer> buffer;
57+
CHLBufferReference buffer;
5958
bool bufferDMA = false;
6059
uint32_t shmFormat = 0;
6160
uint32_t dmabufFormat = 0;

‎src/protocols/Viewporter.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ CViewportResource::CViewportResource(SP<CWpViewport> resource_, SP<CWLSurfaceRes
1515
return;
1616
}
1717

18+
surface->pending.updated.viewport = true;
19+
1820
if (x == -1 && y == -1) {
1921
surface->pending.viewport.hasDestination = false;
2022
return;
@@ -35,6 +37,8 @@ CViewportResource::CViewportResource(SP<CWpViewport> resource_, SP<CWLSurfaceRes
3537
return;
3638
}
3739

40+
surface->pending.updated.viewport = true;
41+
3842
double x = wl_fixed_to_double(fx), y = wl_fixed_to_double(fy), w = wl_fixed_to_double(fw), h = wl_fixed_to_double(fh);
3943

4044
if (x == -1 && y == -1 && w == -1 && h == -1) {

0 commit comments

Comments
 (0)