render: properly release rendered buffers

[ikalco]

Describe your PR, what does it fix/add?

as title says, this pr intends to properly do buffer release when rendering by

• ensuring we keep all surface buffers used in rendering referenced, including non-drmsyncobj buffers since wl_events.release still applies for them
• ensuring we only release buffers when rendering is done, so either on EGLSync signal or after a glFinish
• as a bonus we can also make a renderingDoneCallback, which can be useful for some purposes like screencopy

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Is it ready for merging, or does it need work?

yea

[ikalco]

merge after #9806 (i"ll rebase this to main then)
also some final testing from me and @gulafaran

[gulafaran]

merge after #9806 (i"ll rebase this to main then) also some testing from me and @gulafaran

As long as this pr moves from syncrelease/fd signaling to wayland eventloop/waiter releasing its going to get a no from me, as mentioned earlier it means moving from as early as possible at as correct time release possible to cpu based wait in the crowded wayland eventloop which makes it always out of time. and since this isnt per se fixing anything im not settling for a potential slower approach.

[ikalco]

merge after #9806 (i"ll rebase this to main then) also some testing from me and @gulafaran

As long as this pr moves from syncrelease/fd signaling to wayland eventloop/waiter releasing its going to get a no from me, as mentioned earlier it means moving from as early as possible at as correct time release possible to cpu based wait in the crowded wayland eventloop which makes it always out of time. and since this isnt per se fixing anything im not settling for a potential slower approach.

we're still using fd signaling, just not syncrelease

Hyprland/src/render/Renderer.cpp

Lines 2269 to 2277 in e216468

	if (eglSync.isValid()) {
	if (getExplicitSyncSettings(PMONITOR->output).explicitKMSEnabled) {
	// let kernel signal release points rather than slow wayland event loop
	for (const auto& buf : usedAsyncBuffers) {
	for (auto& point : buf->releasePoints) {
	point.importSyncFD(eglSync.fd());
	}
	}
	}

also previously we didn't release non-drmsynobj dmabuf buffers when rendering finished,
now we do through an eventfd in wayland loop
but if there's a drmsyncobj point, we signal based on EGLSync, like before, not through wayland loop

[gulafaran]

merge after #9806 (i"ll rebase this to main then) also some testing from me and @gulafaran

As long as this pr moves from syncrelease/fd signaling to wayland eventloop/waiter releasing its going to get a no from me, as mentioned earlier it means moving from as early as possible at as correct time release possible to cpu based wait in the crowded wayland eventloop which makes it always out of time. and since this isnt per se fixing anything im not settling for a potential slower approach.

we're still using fd signaling, just not syncrelease

Hyprland/src/render/Renderer.cpp

Lines 2269 to 2277 in e216468

	if (eglSync.isValid()) {
	if (getExplicitSyncSettings(PMONITOR->output).explicitKMSEnabled) {
	// let kernel signal release points rather than slow wayland event loop
	for (const auto& buf : usedAsyncBuffers) {
	for (auto& point : buf->releasePoints) {
	point.importSyncFD(eglSync.fd());
	}
	}
	}

also previously we didn't release non-drmsynobj dmabuf buffers when rendering finished, now we do through an eventfd in wayland loop but if there's a drmsyncobj point, we signal based on EGLSync, like before, not through wayland loop

well previously it accounted for both the release point fd and eglsync fd, it merged those and imported it. now it only accounts for the eglsync fd and ignores if the releasepoint fd is signaled, it was also previously signaling on destruction if the buffer wasnt rendered.

[gulafaran]

merge after #9806 (i"ll rebase this to main then) also some testing from me and @gulafaran

As long as this pr moves from syncrelease/fd signaling to wayland eventloop/waiter releasing its going to get a no from me, as mentioned earlier it means moving from as early as possible at as correct time release possible to cpu based wait in the crowded wayland eventloop which makes it always out of time. and since this isnt per se fixing anything im not settling for a potential slower approach.

we're still using fd signaling, just not syncrelease

Hyprland/src/render/Renderer.cpp

Lines 2269 to 2277 in e216468

	if (eglSync.isValid()) {
	if (getExplicitSyncSettings(PMONITOR->output).explicitKMSEnabled) {
	// let kernel signal release points rather than slow wayland event loop
	for (const auto& buf : usedAsyncBuffers) {
	for (auto& point : buf->releasePoints) {
	point.importSyncFD(eglSync.fd());
	}
	}
	}

also previously we didn't release non-drmsynobj dmabuf buffers when rendering finished, now we do through an eventfd in wayland loop but if there's a drmsyncobj point, we signal based on EGLSync, like before, not through wayland loop

well previously it accounted for both the release point fd and eglsync fd, it merged those and imported it. now it only accounts for the eglsync fd and ignores if the releasepoint fd is signaled, it was also previously signaling on destruction if the buffer wasnt rendered.

hm let me rethink that, it seems we were merely merging the eglsync fd if a new appeared before it was released. can we render the same buffer multiple times before its actually dropped? what happends if the client is at 30fps on a 60hz monitor. is the same buffer supposed to be rendered multiple times and the eglsync just merged until finally dropped?

[ikalco]

rebased to main, should be ready (btw nix compilation issue is prob from wayland-protocols dep)
I daily use this branch on my nvidia desktop and it works fine
also just retested on/off explicit sync with minecraft, csgo, vkcube, and obs

@fufexan @gulafaran thoughts/confirmation? btw I brought back the syncreleaser

edit:
oops meant @gulafaran for above, somehow my subconscious made me type fufexan lol

[fufexan]

@ikalco updated w-p, a rebase on main should fix the build.

[vaxerski]

generally ok by me but I'll test run it a bit tomorrow

[vaxerski]

also can you rebase on main

[ikalco]

also can you rebase on main

should be rebased already lol
its on 72cb5d2 (permissions: disable automatic reloading of permissions from cfg)

ignore that lmao, was on wrong the local branch
should be good now

[vaxerski]

generally works alright on my end, has anyone else tested other configs?

[gulafaran]

generally works alright on my end, has anyone else tested other configs?

besides my above comment that im not sure is even correct in -git either, was wondering what happends if the buffers for a client is slower then the rendering of the monitor. eglsync is now a local variable to endRender(), if destructed is the fd still valid if so? will it artifact if so? , on -git it might just have hidden it more being a member variable to the monitor and not destructing until the next endRender() , otherwise i got no objections

[vaxerski]

I checked civ4 tearing and it was like 40fps and my screen is 240hz and it looked just fine? or do you mean with direct scanout

[gulafaran]

I checked civ4 tearing and it was like 40fps and my screen is 240hz and it looked just fine? or do you mean with direct scanout

More of a late night phone wondering, try both i guess.

[ikalco]

generally works alright on my end, has anyone else tested other configs?

besides my above comment that im not sure is even correct in -git either, was wondering what happends if the buffers for a client is slower then the rendering of the monitor. eglsync is now a local variable to endRender(), if destructed is the fd still valid if so? will it artifact if so? , on -git it might just have hidden it more being a member variable to the monitor and not destructing until the next endRender() , otherwise i got no objections

with eglsync you should dupe the fence fd sync->fd().duplicate()
not keep around the EGLSyncKHR and use its original fd multiple times

[ikalco]

I checked civ4 tearing and it was like 40fps and my screen is 240hz and it looked just fine? or do you mean with direct scanout

huh? do you mean steam fps counter shows 40fps but screen looks like 240fps during DS?

[gulafaran]

generally works alright on my end, has anyone else tested other configs?

besides my above comment that im not sure is even correct in -git either, was wondering what happends if the buffers for a client is slower then the rendering of the monitor. eglsync is now a local variable to endRender(), if destructed is the fd still valid if so? will it artifact if so? , on -git it might just have hidden it more being a member variable to the monitor and not destructing until the next endRender() , otherwise i got no objections

with eglsync you should dupe the fence fd sync->fd().duplicate() not keep around the EGLSyncKHR and use its original fd multiple times

Yeah but isnt eglsync duping the EGLSyncKHR fd, storing it, and then you are duping the duped fd, and when endrender goes out of scope the fds is dead i think, no matter if its signaled or not

[gulafaran]

generally works alright on my end, has anyone else tested other configs?

besides my above comment that im not sure is even correct in -git either, was wondering what happends if the buffers for a client is slower then the rendering of the monitor. eglsync is now a local variable to endRender(), if destructed is the fd still valid if so? will it artifact if so? , on -git it might just have hidden it more being a member variable to the monitor and not destructing until the next endRender() , otherwise i got no objections

with eglsync you should dupe the fence fd sync->fd().duplicate() not keep around the EGLSyncKHR and use its original fd multiple times

Yeah but isnt eglsync duping the EGLSyncKHR fd, storing it, and then you are duping the duped fd, and when endrender goes out of scope the fds is dead i think, no matter if its signaled or not

as discussed on discord, this should be fine. the fd acts as a ref count for the kernel dma fence. so in the end this shouldnt be wrong. im satisfied. 👍