wayland: Surface Transactions

[Drakulix]

Best reviewed commit-by-commit:

compositor: Use the TransactionQueue

Changes the internal handling of the compositor module to use the TransactionQueue. Should result in identical behavior, doesn't expose anything new to downstream.

compositor: Make it possible to add Blockers

Adds new public api to the compositor module to allow downstream to add Blockers to a surface, which will be considered on the next commit.

Couple of open questions for this commit:

• The CompositorClientState::blocker_cleared-method takes a lot of really generic arguments. It works for the upcoming dmabuf-commit, but might be problematic for other use-cases. Could we require less? Should we possibly move the responsibility to re-run the commit-handler to downstream and just take a DisplayHandle? Necessary to call commit and less of an issue with the changes below.
• If we keep the commit-callback here, do we maybe want to skip the original commit-callback in the first place? We can easily bubble up the return value of apply_ready to conditionally skip the call. commit call has been moved into Transaction::apply
• If there are pending blockers, post-commit-hooks will not observe any changed state. Luckily smithay doesn't use them for anything, does any compositor? Do we consider this expected behaviour? Do we want to run them once the transaction is done instead? Can we maybe just drop them? post_commit-hooks as well

buffer: Add buffer_attached callback

Adds a new buffer_attached-callback to the BufferHandler. Prerequisite for the next commit

WIP: anvil: Wait for dmabufs to become ready

Very rudimentary test of the above functionality.

Possible todos:

• The code should be moved into a different module
• Maybe we can add some implementations/abstractions in smithay to make this easier, specifically for the dmabuf case?
• We should poll new buffers before adding a blocker for them. We can skip a complete event loop cycle, if the buffers are already ready, which is not unlikely.

[elinorbgr]

We'll need documentation here before this PR is merged.

[elinorbgr]

Ok so, thinking more about it, regarding the commit callback and your questions on the second commit, a general answer:

Given how the code is right now, it's entirely possible that the invocation of apply_ready in blocker_cleared might apply transactions that are unrelated to the surface passed as argument, and in a future-proof view, applying a transaction can involve multiple surfaces, and ideally all of them should have their commit() callback invoked.

So, I think maybe we should check if it'd be possible to have those callbacks (and the post_commit hooks) invoked directly in Transaction::apply ? I believe it would solve the whole problem, if it is possible.

[Drakulix]

So, I think maybe we should check if it'd be possible to have those callbacks (and the post_commit hooks) invoked directly in Transaction::apply ? I believe it would solve the whole problem, if it is possible.

Seems like a good solution.
This is ready for another review round. :)

[codecov-commenter]

Codecov Report

Patch coverage: 18.61% and project coverage change: +0.08 🎉

Comparison is base (3bec067) 23.56% compared to head (7239191) 23.65%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
+ Coverage   23.56%   23.65%   +0.08%     
==========================================
  Files         132      134       +2     
  Lines       21089    21284     +195     
==========================================
+ Hits         4969     5034      +65     
- Misses      16120    16250     +130     

Flag	Coverage Δ
wlcs-core	20.79% <18.61%> (+0.08%)	⬆️
wlcs-output	8.69% <0.00%> (-0.07%)	⬇️
wlcs-pointer-input	22.91% <18.61%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
anvil/src/state.rs	34.97% <0.00%> (ø)
src/backend/allocator/dmabuf.rs	0.00% <0.00%> (ø)
src/wayland/buffer/mod.rs	0.00% <0.00%> (ø)
src/wayland/compositor/mod.rs	66.07% <0.00%> (-6.48%)	⬇️
anvil/src/shell/buffer.rs	20.00% <20.00%> (ø)
anvil/src/shell/mod.rs	40.96% <66.66%> (+0.79%)	⬆️
src/wayland/compositor/tree.rs	56.21% <68.75%> (-0.23%)	⬇️
src/wayland/compositor/transaction.rs	53.67% <80.00%> (+28.27%)	⬆️
src/wayland/compositor/handlers.rs	38.74% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[cmeissl]

Overall I really like the blocker API, I just have a few questions that I left as comments.

[cmeissl]

Could that be directly added to XWaylandClientData? launch initializes XWaylandClientData where it would be possible to directly add CompositorClientState as a field instead of using user_data.

[Drakulix]

Now based on #1014 to make use of a pre-commit hook instead of buffer_attached for adding the blocker.

[cmeissl]

The whole anvil/src/shell/buffer.rs module looks like a left over from moving the block to the pre-commit handler.

[Drakulix]

The whole anvil/src/shell/buffer.rs module looks like a left over from moving the block to the pre-commit handler.

Removed.

[Drakulix]

Rebased after #1014 got merged without any other changes.

[cmeissl]

LGTM