Mir

Merge lp://qastaging/~afrantzis/mir/swapper-multiple-compositor-acquire into lp://qastaging/~mir-team/mir/trunk

  1. swapper-multiple-compositor-acquire
  2. Merge into trunk
Proposed by Alexandros Frantzis
Status: Rejected
Rejected by: Alexandros Frantzis
Proposed branch: lp://qastaging/~afrantzis/mir/swapper-multiple-compositor-acquire
Merge into: lp://qastaging/~mir-team/mir/trunk
Diff against target: 430 lines (+238/-26)
7 files modified
include/server/mir/compositor/buffer_swapper.h (+18/-3)
include/server/mir/compositor/buffer_swapper_multi.h (+26/-1)
src/server/compositor/buffer_swapper_multi.cpp (+109/-10)
src/server/compositor/buffer_swapper_spin.cpp (+3/-0)
tests/integration-tests/compositor/test_stress_buffer_swapper.cpp (+15/-3)
tests/integration-tests/compositor/test_swapping_swappers.cpp (+8/-9)
tests/unit-tests/compositor/test_buffer_swapper_double.cpp (+59/-0)
To merge this branch: bzr merge lp://qastaging/~afrantzis/mir/swapper-multiple-compositor-acquire
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Mir development team Pending
Review via email: mp+169473@code.qastaging.launchpad.net

Commit message

compositor: Support multiple compositor_acquire() calls for BufferSwapperMulti

Description of the change

compositor: Support multiple compositor_acquire() calls for BufferSwapperMulti

This is the next step in the session snapshots saga: being able to hand out compositor buffers to multiple consumers. Note that this deals with normal acquires, not buffer peeking. Buffer peeking complicates the swapper even more, and after a lot of experimentation I decided not to pursue it further until there is demonstrated need for it (the current code can be modified to support peeking).

Acquiring (instead of peeking) a buffer when taking a snapshot may lead to the compositor losing a buffer (= glitch). However, this may not actually be a problem in practice since in the current use cases snapshotting happens when a surface is about to be hidden. In any case, consider this to be first step which we can refine in the future if there is a need.

Also, note that this work will benefit multiple monitor support, for which the compositor may need to acquire the buffer multiple times (one for each screen), for example, if a surface is visible on more than one screens (and we are not in clone mode).

The semantics of multiple compositor acquires for BufferSwapperMulti are:

1. All calls in a series of acquires without any intervening releases will return the same buffer, even if newer buffers have been posted.
2. The acquired buffer will return to the client only after all acquires have been released.
3. The next acquire after any release, even if it is not a final release, will get the last posted buffer (which may be the already acquired buffer if there is no newer buffer).

(1) Reduces the missing of buffers:

Although this scenario may still cause the compositors to miss buffers:

Comp1: AR__AR
Comp2: __AR__AR

...in the following scenarios buffers are not missed:

Comp1: A__R
Comp2: _AR_

Comp1: A_R_
Comp2: _A_R_

(3) This ensures we don't get stuck with the same buffer forever, e.g., in the following scenario:

Comp1: A_RA__RA__RA ...
Comp2: _A__RA__RA__ ...

In the scenario above the buffer stays acquired forever. If we didn't have (3) or a similar escape route, the two compositors would get the same buffer forever (because of (1))!

These rules are not set in stone, they were chosen as a decent starting poing that seems to work well, but we are free to change them as needed. For example we can get rid of (1)+(3), and decide that acquire() will always return the last posted buffer.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:743
http://jenkins.qa.ubuntu.com/job/mir-ci/734/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/913
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/795
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/219/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/421

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/734/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Spurious jenkins failure... resubmitted.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:743
http://jenkins.qa.ubuntu.com/job/mir-ci/738/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/917
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/799
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/223
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/223/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/425

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/738/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

the new tests look like what we want to do. I'm not sure if the approach with AcquiredBuffers is the simplest though. We could do something like have SwitchingSwapper take care of 'cloning' and refcounting the outgoing buffers on the compositor side (and keeping the mc::BufferSwapper requirements the same).

I also don't mind having a strong reference to the Buffer in both the TemporaryBuffer the compositor owns and the BufferBundle, if that makes it simpler.

Revision history for this message
Kevin DuBois (kdub) wrote :

I guess with all the new swapper functionality we're finding that we need (like, resizing a bundle, switching swapping algorithms mid-flight, multimonitor, and peeking for individual snapshots (maybe even screencasting?)) I'd rather solve the problems in the layer between the BufferStream and the BufferSwappers than by making BufferSwapper have more responsibilities. Hopefully this will keep our BufferSwappers steady state algorithms for swapping buffers, and the 'corner cases' (mentioned above) are taken care of mostly by diverting the buffers once we've ensured sync in the BufferSwappers.

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> I'd rather solve the problems in the layer between the
> BufferStream and the BufferSwappers than by making BufferSwapper have more
> responsibilities. Hopefully this will keep our BufferSwappers steady state
> algorithms for swapping buffers, and the 'corner cases' (mentioned above)
> are taken care of mostly by diverting the buffers once we've ensured sync
> in the BufferSwappers.

Good idea. I am unhappy with adding complexity to BufferSwapper myself.

I will move the multiple acquire support to SwitchingBundle. Note that we still need to use the AcquiredBuffers class (or something similar) in SwitchingBundle, to enforce the behaviour mentioned in the description.

Unmerged revisions

743. By Alexandros Frantzis

compositor: Support multiple compositor_acquire() calls for BufferSwapperMulti

742. By Daniel van Vugt

setup-android-dependencies.sh: Fix typo causing libxkbcommon0 to not be
installed and the script never quite finish. (LP: #1190884). Fixes: https://bugs.launchpad.net/bugs/1190884.

Approved by Alexandros Frantzis, PS Jenkins bot.

741. By Daniel van Vugt

Introducing fingerpaint. It's not just a multi-touch demo that responds to
pressure and size, but more importantly it's a future testbed for localized
input coordinates.

Pretty boring on desktop. Less so on Nexus 4.
.

Approved by Kevin DuBois, Alan Griffiths, PS Jenkins bot.

740. By Alan Griffiths

compositor: Restore force_requests_to_complete() logic to what we had before -r 728. Fixes: https://bugs.launchpad.net/bugs/1189443.

Approved by PS Jenkins bot, Kevin DuBois, Alexandros Frantzis.

739. By Daniel van Vugt

Fix debuild failures on saucy / gcc-4.8.

Approved by Alexandros Frantzis, Alan Griffiths, PS Jenkins bot, Chris Halse Rogers.

738. By Daniel van Vugt

Minor correction to documentation building_source_for_pc so that the
instructions will actually work :)

Remember sudo.

Approved by Alexandros Frantzis, PS Jenkins bot.

737. By Daniel van Vugt

Resolve the two differing interpretations of "stride" which were causing
software surfaces to be distorted on Android. (LP: #1116452)

Just make sure that "stride" in any Android structure is a number of pixels
and in Mir types it is always a number of bytes. Fixes: https://bugs.launchpad.net/bugs/1116452.

Approved by Kevin DuBois, Alexandros Frantzis, Alan Griffiths, PS Jenkins bot.

736. By Kevin DuBois

Iterate on the SwapperFactory so it can meet the more complex demands of different allocations. This is the cleanup of the the rough edges of the SwapperSwitcher algorithm branch.

Approved by PS Jenkins bot, Alexandros Frantzis, Alan Griffiths.

735. By Robert Carr

Depthify the surface stack to support the launcher use case.

Approved by Alan Griffiths, PS Jenkins bot, Alexandros Frantzis.

734. By Daniel van Vugt

Fix backward error message and improve error details (mention the errno).
(LP: #1189726). Fixes: https://bugs.launchpad.net/bugs/1189726.

Approved by Alexandros Frantzis, Alan Griffiths, PS Jenkins bot.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches