protocols: Add fifo-v1 and commit-timing-v1#11703
Conversation
|
Also @ikalco @gulafaran I touched your surface commit things. Might wanna verify I didn't exactly fuk something up |
|
@fufexan wp bump needed tooooo.... idk which version, fifo and ct |
|
Games using SDL3 (SDL2 as well, not sure?) should be taking advantage of these two protos. As indicated, SDL should automatically pick Wayland as its default backend now that these are available to it. CS2 will still default to XWL for now as they've manually added an env var to their launch script to use x11 SDL backend. |
|
I tried PPSSPP with wayland backend and it didnt bind to either fifo or ct |
not needed, they've been available since 1.38. We have 1.45. |
|
why is ci failing then huh oh I forgor mesonnn |
|
@ikalco check now |
|
crash to tty when trying to launch CS2 with |
|
oops fixer |
mpv has implemented fifo-v1 (mpv-player/mpv@39b53dd) |
No, it has to be |
|
check flicker now |
|
@Zebra2711 I can't get mpv to bind to fifo. |
naw, still cooked. The flicker looks exactly like the flicker from pre-explicit-sync days. Discord flickering, steam flickering, cs2 flickering, etc etc, in the same ways they used to. |
mpv can use fifo, |
|
how are you launching it to get it to use fifo? |
i run with |
|
it doesnt bind for me :( |
| void CWLSurfaceResource::lockState() { | ||
| m_stateLocks++; | ||
| } | ||
|
|
||
| void CWLSurfaceResource::unlockState(std::optional<size_t> seq) { | ||
| RASSERT(!!m_stateLocks, "Tried to unlock an unlocked wl_surface state"); | ||
| m_stateLocks--; | ||
|
|
||
| if (m_stateLocks) | ||
| return; | ||
|
|
||
| while (!m_pendingStates.empty() && (!seq || m_pendingStates.front()->seq <= *seq)) { | ||
| commitState(*m_pendingStates.front()); | ||
| m_pendingStates.pop(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This should be in SSurfaceState
currently we can skip a frame/buffer:
- we get commit1 with buffer1, locks = 1
- we get commit2 with buffer2, locks = 2
- buffer1 is readable, locks = 1 .... this should've applied commit1
- buffer2 is readable, locks = 0 .... this applies commit1 followed by commit2
- commit1 buffer was never applied and scanned out, so a frame is skipped
I wrote this before you added the seq mechanism, but I still think this applies cause buffer unlock isn't the only thing locking/unlocking every queued surface state
There was a problem hiding this comment.
right... I'll rethink this tomorrow.
There was a problem hiding this comment.
@ikalco what about having a queue with fences?
Say we have a queue with states:
A B C D
commit comes, we insert a fence on the buffer F1
A B C D F1
more commits come:
A B C D F1 E
fifo and a new buffer is used, F2 for dma and F3 for fifo:
A B C D F1 E G F2 F3
when F1 signals, all commits before it get applied:
E G F2 F3
Now the only problem is that for E and G, as far as I understand, both F2 and F3 need to signal. This is fine though and easy code-wise.
There was a problem hiding this comment.
yeah I think that can work
just make sure that commits without constraints are applied immediately
| SState m_current, m_pending; | ||
|
|
||
| struct { | ||
| CHyprSignalListener surfacePrecommit; | ||
| } m_listeners; |
There was a problem hiding this comment.
this should be in SSurfaceState, cause this bypasses the m_pendingStates queue
also as a clarifying note, surface commit's aren't really "double buffered"
the first "buffer" is m_pending, and then the second "buffer" is actually that m_pending is added into the m_pendingStates queue until it can be applied depending on constraints (buffer readability, commit timing timeout, fifo_barrier is unset, more stuff?)
| // Signal all surfaces | ||
| // TODO: should we wait for the correct monitor(s)? | ||
|
|
||
| fifo->presented(); |
There was a problem hiding this comment.
I think the monitor probably would matter because fifo proto guarantees that a surface buffer will be active for at least one refresh cycle, which depends on each monitors refresh rate
There was a problem hiding this comment.
what if its on two?
?
it should only unlock for the corresponding monitor (PHLMONITOR m), otherwise when refresh cycles are desynced bad stuff happens (mon1 has 60hz, mon2 144hz)
|
@ikalco check now, I've done locks per-state, should be better |
|
mpv is able to open now, but it immediately closes when playing a video full log log.txt |
|
check now @fxzzi |
rip, still bugged |
|
well I dunno then, and it's really annoying to dev without being able to test or debug |
|
Let him remote innnn, dooo ittt |
|
I tried the weston fifo test and it seems to have passed? idk how to test this really mpv doesn't fucking want to use fifo |
I built mpv with the flag |
|
ok I'll try that |
|
still no |
|
|
|
I tried it and still hangs. When I run with My mpv is the default 0.40.0 (manjaro package) with default settings. Nvidia 960. |
|
mpv-fifo.log |
yup. Nothing makes it use fifo. AMD, fwiw. |
|
Shot in the dark here but what about: |
|
nope |
|
Damn. Maybe you have the following option from the manual set or, more likely, it defaults to yes behavior for this option and does not work because the protocol isn't detected by mpv when running it?
If the latter, try setting --wayland-interval-sync=no or something. Might force it to use your implementation of the wayland protocol? Again another shot in the dark, but it seems like it defaults to yes behavior if the protocol is not detected in wayland. |
|
I tried that too, I think I tried fucking everything at this point. I can see fifo v1 in WAYLAND_DEBUG so it is exported. |
|
Maybe |
|
fifo-v1 already in mesa from 24.3, lol |
|
nope, doesnt bind fifo |
vkcube doesn't bind? binds for me on nvidia... hangs immediately |
Yeah I just meant recompiling locally cause maybe it's just a packaging issue or something like that |
|
It keeps going for some time with |
|
tom is working on his own cuz I cant for the life of me test this #12052 |
|
think i got fifo working, mpv, vkcube seems happy. anyone knows what i can test commit-timing-v1 with? |
vulkan fifo is implemented using both protocols in mesa (https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26150) so mpv and vkcube should be ok to test it too ig |
It was later relaxed to be able to use FIFO without commit-timing in https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/32556. |
|
ok feel free to test #12052 , im not sure i got all the protocol things correct but vkcube,cs2,mpv seems happy now. and it kinda changed all sorts of buffer commit/release things so it might just end up regressing vrr or other things. |

Fixes #11386
Needs some testing, I didn't test this because I dont know what apps use it.
If anyone has a test app, do lmk.
@Justsnoopy30 @fxzzi