Support fifo-v1 and commit-timing-v1 protocol #1588
Conversation
cmeissl
force-pushed
the
feature/fifo_v1
branch
from
ee4acc9
to
9e6d071
Compare
| @@ -306,7 +306,8 @@ impl PrivateSurfaceData { | |||
| } | |||
| my_data | |||
| .pending_transaction | |||
| .insert_state(surface.clone(), my_data.current_txid); | |||
| .insert_state(surface.clone(), current_txid); | |||
| my_data.current_txid.0 = my_data.current_txid.0.wrapping_add(1); | |||
There was a problem hiding this comment.
@elinorbgr @Drakulix @ids1024 Not sure this is correct, but without incrementing the transaction id the blocked state will not be serialized but directly merged into any already blocked transaction. This means that for example the buffer will be overwritten and might get a release making fifo unusable with blockers (same for explicit sync, but I guess there it might not be an issue). It would also break my fifo barrier logic because it would allow to override an already pending barrier.
There was a problem hiding this comment.
Honestly not sure, if this is a correct "fix" for this issue, but yeah with explicit sync and fifo/commit-timing we shouldn't merge commit states, but just advance to the latest ready one.
There was a problem hiding this comment.
yeah, until now with mailbox alone this just wasn't important. but so far this seems to work correct and not regress existing stuff.
| } | ||
|
|
||
| if let Some(barrier) = into.barrier.replace(barrier) { | ||
| barrier.signal(); |
There was a problem hiding this comment.
I tried to come up with a reason why this might be problematic, but so far couldn't find one.
cmeissl
force-pushed
the
feature/fifo_v1
branch
2 times, most recently
from
781d84f
to
6f358d4
Compare
cmeissl
changed the title
cmeissl
force-pushed
the
feature/fifo_v1
branch
4 times, most recently
from
b899004
to
467a82f
Compare
| @@ -306,7 +306,8 @@ impl PrivateSurfaceData { | |||
| } | |||
| my_data | |||
| .pending_transaction | |||
| .insert_state(surface.clone(), my_data.current_txid); | |||
| .insert_state(surface.clone(), current_txid); | |||
| my_data.current_txid.0 = my_data.current_txid.0.wrapping_add(1); | |||
There was a problem hiding this comment.
Honestly not sure, if this is a correct "fix" for this issue, but yeah with explicit sync and fifo/commit-timing we shouldn't merge commit states, but just advance to the latest ready one.
At commit time we might not know and the protocol only says may. What I did in anvil is that a blocker will be cleared if the processed output matches the primary output or if the surface does not have a primary output. imo the main reason for having this written in the spec is to allow a client make forward progress instead of being blocked indefinitely with frame callbacks. |
cmeissl
force-pushed
the
feature/fifo_v1
branch
from
467a82f
to
6d3f291
Compare
cmeissl
force-pushed
the
feature/fifo_v1
branch
from
6d3f291
to
8f4034a
Compare
and while at it resolve the long standing issue that anvil does not send frame callbacks for cursor and dnd surfaces
cmeissl
force-pushed
the
feature/fifo_v1
branch
from
8f4034a
to
c870be3
Compare
Support for the new fifo and commit timing protocol and changes for wp-presentation version 2
Clearing barriers needs to be better abstracted/helpers are needed
The integration in anvil for clearing fifo barriers is just bare bones. We probably wantto provide something similar like we already do for
send_framesand so.Also it is incomplete atm, we also need to clear blockers for offscreen/non visible surfaces and so on.
Actually it might be a good start as is, we can clear some blockers directly while figuring out the primary scan-out output.
Can we do better than using an idle task for calling
blocker_cleared?This might be anvil specifc, but I am actually not sure. callingblocker_clearedneeds the client state and wealso have to provide the compositor state. this makes it hard to call from within a nested function inside the state
like the render function in anvil. For now I just worked around this by scheduling an idle task...
Yes, I refactored
post_commitand it can now use the state. In addition it also de-duplicatesblocker_clearedper client.Testing
I pushed a simple test client here.
closes #1581
closes #1582