Mir

Merge lp:~vanvugt/mir/client-side-vsync into lp:mir

Proposed by Daniel van Vugt on 2017-01-09
Status: Merged
Approved by: Cemil Azizoglu on 2017-01-17
Approved revision: 3926
Merged at revision: 3952
Proposed branch: lp:~vanvugt/mir/client-side-vsync
Merge into: lp:mir
Prerequisite: lp:~vanvugt/mir/fix-StaleFrames-test-race
Diff against target: 1547 lines (+1049/-22)
24 files modified
include/client/mir_toolkit/mir_buffer_stream.h (+0/-1)
include/client/mir_toolkit/mir_surface.h (+0/-1)
include/core/mir_toolkit/common.h (+7/-4)
src/client/CMakeLists.txt (+1/-0)
src/client/buffer_stream.cpp (+111/-6)
src/client/buffer_stream.h (+12/-1)
src/client/error_stream.cpp (+6/-0)
src/client/error_stream.h (+2/-0)
src/client/frame_clock.cpp (+156/-0)
src/client/frame_clock.h (+73/-0)
src/client/mir_buffer_stream_api.cpp (+8/-1)
src/client/mir_surface.cpp (+113/-5)
src/client/mir_surface.h (+10/-0)
src/client/mir_surface_api.cpp (+1/-1)
src/client/screencast_stream.cpp (+8/-0)
src/client/screencast_stream.h (+2/-0)
src/include/client/mir/mir_buffer_stream.h (+2/-0)
tests/acceptance-tests/test_client_library.cpp (+5/-0)
tests/acceptance-tests/test_latency.cpp (+27/-2)
tests/include/mir/test/doubles/mock_mir_buffer_stream.h (+2/-0)
tests/unit-tests/client/CMakeLists.txt (+1/-0)
tests/unit-tests/client/test_client_mir_surface.cpp (+81/-0)
tests/unit-tests/client/test_frame_clock.cpp (+420/-0)
tests/unit-tests/platforms/android/server/test_hwc_device.cpp (+1/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/client-side-vsync
Reviewer Review Type Date Requested Status
Alan Griffiths Approve on 2017-01-17
Kevin DuBois (community) Approve on 2017-01-17
Mir CI Bot continuous-integration Approve on 2017-01-17
Cemil Azizoglu (community) 2017-01-09 Approve on 2017-01-16
Review via email: mp+314313@code.launchpad.net

Commit message

Introducing client-side vsync, part 1.
For dramatically lower latency and improved smoothness.
Fixes LP: #1240909, LP: #1388490, LP: #1591328, LP: #1624188

Part 1 covers all users of:
   eglSwapBuffers
   mir_buffer_stream_swap_buffers_sync

Part 2 will cover the rest (mir_buffer_stream_swap_buffers) but we're not
in a hurry for it as that still has working server-side vsync. Plus I
think Xmir is the only user of that function at present.

Part 3 will introduce IPC to communicate hardware vsync timestamps to the
client for phase correction and increased precision (although that's not
a prerequisite to this).

Immediate benefits:

 * Greatly reduced latency from the client to the screen. Theoretically
   three frames lower latency for windowed apps of a nested server like
   Unity8. And this has been confirmed with real measurements:
   https://docs.google.com/spreadsheets/d/1RbTVDbx04ohkF4-md3wAlgmxbSI1DttstnT6xdcXhZQ/pubchart?oid=1566479835&format=interactive
   You can also confirm using your own eyes: mir_demo_client_target

 * No client/toolkit changes required.

 * Latency and smoothness is no longer affected by process scheduling
   randomness or poor server performance. Although we presently use input
   resampling to work around some of the latency problem, that's not
   immune to latency increasing again when the server has insufficient
   time to keep up with the client. This approach is.

 * Lower latency even than double buffering (when nested), and also
   without the risk of decimation when a client takes slightly too long
   to render.

Description of the change

Future benefits:

 * We can remove Mir's input resampling now, since client-side vsync
   should make the QML touch compression algorithm safely nestable.

 * We get closer to not needing server-side queuing and multi-monitor
   frame sync logic. Although some people would like to keep it for other
   reasons, this approach (after mir_buffer_stream_swap_buffers gets
   ported) eliminates most of the need for it.

 * Overall this approach could deprecate many more lines of code than it
   adds (see above).

 * Toolkits can have greater flexibility in how they choose to implement
   their event loops without threading. If we shared timing information
   in the client API in future then a toolkit could just choose interval 0
   and do its own vsync baked into its event loop.

 * Mir is free to support dropping in the system compositor if it wants,
   and no longer requires additional fixing to throttle drivers (like
   software renderers) that don't support page flipping.

Risks:

 * We rely on the graphics driver to report accurate frame timing.
   Although it's only a serious problem if the driver is out by orders of
   magnitude (LP: #1639725). Otherwise only being slightly inaccurate is
   actually the same problem as what we experience presently with the
   59Hz input resampling when user input is driving animation.

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3911
https://mir-jenkins.ubuntu.com/job/mir-ci/2578/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3358
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3425
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3417
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3417
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3417
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3387
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3387/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3387
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3387/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3387
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3387/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3387
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3387/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3387
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3387/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3387
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3387/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2578/rebuild

review: Approve (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3917
https://mir-jenkins.ubuntu.com/job/mir-ci/2672/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3472/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3539
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3529
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3529
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3529
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3499/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3499
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3499/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3499/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3499
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3499/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3499
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3499/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3499/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2672/rebuild

review: Needs Fixing (continuous-integration)
lp:~vanvugt/mir/client-side-vsync updated on 2017-01-12
3918. By Daniel van Vugt on 2017-01-12

Try again please

Cemil Azizoglu (cemil-azizoglu) wrote :

Nice latency numbers.

Needs fixing:

MirSurface references should be MirWindow now.

review: Needs Fixing
Kevin DuBois (kdub) wrote :

LGTM overall (still haven't fully understood the logic in FrameClock, so more comments on that later, perhaps)

discussion:
135 + * TODO: Remove these functions in future, after
136 + * mir_buffer_stream_swap_buffers has been converted to use
137 + * client-side vsync, and the server has been told to always use
138 + * dropping for BufferStream. Then there will be no need to ever
139 + * change the server swap interval from zero.

Would prefer if this read "and the server has other ways to express buffer queuing other than swapinterval". Would leave room for some of the vulkan submission modes to work via server-side queuing (jury's still out on whether this is a good idea, but I don't know if we want to limit ourselves to client-side queuing only)

needs info:
1)
+void mcl::BufferStream::adopted_by(MirSurface* surface)

Other than BufferStream's currently-terrible constructor, why couldn't the frame clock be given as a build dependency via the constructor? One more parameter to the constructor seems better than a too-long constructor list + a 2step initialization

2)
Where the code currently is might be only useful to android, post-MirRenderSurface/MirSurface/mir-egl-platform. (which still uses MirBufferStream as part of its egl stack for now). From what I've heard from Cemil, mesa is using MirPresentationChains + MirBuffers, so might need some way via an extension or a public-api to get the timing info. (android might also might be better suited for that as well, for now its just easier to re-use MirBufferStream for that platform)

nit:
348 +typedef std::unique_lock<std::mutex> Lock;
IMHO, doesn't improve readability.

I guess that's mostly a 'discussion' vote.

review: Needs Information
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3918
https://mir-jenkins.ubuntu.com/job/mir-ci/2685/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3487/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3554
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3544
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3544
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3544
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3514
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3514/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3514
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3514/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3514
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3514/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3514
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3514/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3514
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3514/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3514/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2685/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :
lp:~vanvugt/mir/client-side-vsync updated on 2017-01-15
3919. By Daniel van Vugt on 2017-01-15

Merge latest trunk

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3919
https://mir-jenkins.ubuntu.com/job/mir-ci/2703/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3513/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3581
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3571
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3571
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3571
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3540
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3540/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3540
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3540/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3540
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3540/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3540/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3540/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3540
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3540/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3540/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2703/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :

^^^
Bug 1647244, and more random examples failures on krillin:

13:14:00 I: The following clients failed to execute successfully:
13:14:00 I: mir_demo_client_multistream
13:14:00 I: Smoke testing complete with returncode -1

lp:~vanvugt/mir/client-side-vsync updated on 2017-01-16
3920. By Daniel van Vugt on 2017-01-16

Try harder, Jenkins

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3920
https://mir-jenkins.ubuntu.com/job/mir-ci/2708/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3519/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3577
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3577
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3577
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3546
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3546/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3546/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3546
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3546/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3546
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3546/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3546
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3546/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3546
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3546/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2708/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :
lp:~vanvugt/mir/client-side-vsync updated on 2017-01-16
3921. By Daniel van Vugt on 2017-01-16

Merge latest trunk

Daniel van Vugt (vanvugt) wrote :

kdub:

(0) Sure you can keep server-side queueing if you want it. So long as this code still has the ability to turn it off. Because having it queueing on will reintroduce the bufferbloat-nested-queuing-lag problems that are fixed here. So you probably don't want that, but up to you :)

(1) No FrameClock is not set at construction. It's set (and changes) whenever the window moves to a new monitor (different FrameClock). We may also wish to change the FrameClock depending on occlusions in future (to replace the 10Hz dropping hack)...

(2) Yes, I expect the logic to move around with Cemil's future changes but where it is right now works for what we use right now. As for a public API, yes I agree that will be wanted; already mentioned it in the description above, but also a public API will be needed by Xmir to implement https://www.opengl.org/registry/specs/OML/glx_sync_control.txt

Daniel van Vugt (vanvugt) wrote :

Also a public API would let clients do the equivalent of client-side predictive bypass for even lower latency: https://www.opengl.org/registry/specs/NV/glx_delay_before_swap.txt

lp:~vanvugt/mir/client-side-vsync updated on 2017-01-16
3922. By Daniel van Vugt on 2017-01-16

Merge latest trunk

3923. By Daniel van Vugt on 2017-01-16

s/MirSurface/MirWindow/g

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3921
https://mir-jenkins.ubuntu.com/job/mir-ci/2711/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3524/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3592
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3582
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3582
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3582
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3551/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3551/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3551/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2711/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3923
https://mir-jenkins.ubuntu.com/job/mir-ci/2714/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3529/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3597
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3556
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3556/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3556
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3556/artifact/output/*zip*/output.zip
    ABORTED: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3556/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3556
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3556/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3556
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3556/artifact/output/*zip*/output.zip
    ABORTED: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3556/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2714/rebuild

review: Needs Fixing (continuous-integration)
Cemil Azizoglu (cemil-azizoglu) wrote :

Ok by me when CI is happy

review: Approve
Daniel van Vugt (vanvugt) wrote :

^^^
CI aborts are bug 1655929

lp:~vanvugt/mir/client-side-vsync updated on 2017-01-17
3924. By Daniel van Vugt on 2017-01-17

Merge latest trunk

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3924
https://mir-jenkins.ubuntu.com/job/mir-ci/2729/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3556/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3624
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3614
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3614
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3614
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3583
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3583
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3583
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3583/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3583/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3583
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3583/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3583
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3583/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2729/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :

^^^
Random krillin failures running mir-demos:

03:40:30 I: The following clients failed to execute successfully:
03:40:30 I: mir_demo_client_animated_cursor
03:40:30 I: mir_demo_client_multistream
03:40:30 I: Smoke testing complete with returncode -1

lp:~vanvugt/mir/client-side-vsync updated on 2017-01-17
3925. By Daniel van Vugt on 2017-01-17

Merge latest trunk

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3925
https://mir-jenkins.ubuntu.com/job/mir-ci/2733/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3564/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3632
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3622
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3622
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3622
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3591/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3591
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3591/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3591
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3591/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3591
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3591/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3591
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3591/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3591
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3591/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2733/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :
lp:~vanvugt/mir/client-side-vsync updated on 2017-01-17
3926. By Daniel van Vugt on 2017-01-17

Merge latest trunk

Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3926
https://mir-jenkins.ubuntu.com/job/mir-ci/2737/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3570
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3638
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3628
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3628
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3628
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3597
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3597/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3597
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3597/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3597
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3597/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3597
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3597/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3597
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3597/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3597
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3597/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2737/rebuild

review: Approve (continuous-integration)
Kevin DuBois (kdub) wrote :

> kdub:
>
> (0) Sure you can keep server-side queueing if you want it. So long as this
> code still has the ability to turn it off. Because having it queueing on will
> reintroduce the bufferbloat-nested-queuing-lag problems that are fixed here.
> So you probably don't want that, but up to you :)

Sure, we need an option to turn it off, it might just be useful to have it around.

> (2) Yes, I expect the logic to move around with Cemil's future changes but
> where it is right now works for what we use right now. As for a public API,
> yes I agree that will be wanted; already mentioned it in the description
> above, but also a public API will be needed by Xmir to implement
> https://www.opengl.org/registry/specs/OML/glx_sync_control.txt

Alright, so we might need some extensions for this to work in the "MirRenderSurface" world, and the driver would manage what BufferStream is managing now.

Kevin DuBois (kdub) wrote :

well, lgtm. The frame clock management still seems like 2-step initialization, but given the state of BufferStream, doesn't bother me as much.

review: Approve
Alan Griffiths (alan-griffiths) wrote :

+ * Copyright © 2016 Canonical Ltd.

Is the year really right?

Alan Griffiths (alan-griffiths) wrote :

> well, lgtm. The frame clock management still seems like 2-step initialization,
> but given the state of BufferStream, doesn't bother me as much.

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/client/mir_toolkit/mir_buffer_stream.h'
2--- include/client/mir_toolkit/mir_buffer_stream.h 2016-12-17 00:52:45 +0000
3+++ include/client/mir_toolkit/mir_buffer_stream.h 2017-01-17 06:23:26 +0000
4@@ -219,7 +219,6 @@
5 /**
6 * Set the swapinterval for the stream.
7 * \warning EGL users should use eglSwapInterval directly.
8- * \warning Only swapinterval of 0 or 1 is supported.
9 * \param [in] stream The buffer stream
10 * \param [in] interval The number of vblank signals that
11 * mir_buffer_stream_swap_buffers will wait for
12
13=== modified file 'include/client/mir_toolkit/mir_surface.h'
14--- include/client/mir_toolkit/mir_surface.h 2017-01-17 05:51:54 +0000
15+++ include/client/mir_toolkit/mir_surface.h 2017-01-17 06:23:26 +0000
16@@ -978,7 +978,6 @@
17 /**
18 * Set the swapinterval for the default stream.
19 * \warning EGL users should use eglSwapInterval directly.
20- * \warning Only swapinterval of 0 or 1 is supported.
21 * \warning If the surface was created with, or modified to have a
22 * MirSurfaceSpec containing streams added through
23 * mir_window_spec_set_streams(), the default stream will
24
25=== modified file 'include/core/mir_toolkit/common.h'
26--- include/core/mir_toolkit/common.h 2017-01-17 05:51:54 +0000
27+++ include/core/mir_toolkit/common.h 2017-01-17 06:23:26 +0000
28@@ -37,9 +37,9 @@
29 mir_surface_attrib_type,
30 mir_surface_attrib_state,
31 mir_surface_attrib_swapinterval, /**< \deprecated Do not listen for events
32- reporting this attribute. Use the
33- "mir_*_get_swapinterval()" functions
34- instead if you wish query its value */
35+ reporting this attribute. Use the
36+ "mir_*_get_swapinterval()" functions
37+ instead if you wish query its value */
38 mir_surface_attrib_focus,
39 mir_surface_attrib_dpi,
40 mir_surface_attrib_visibility,
41@@ -57,7 +57,10 @@
42 /* Do not specify values...code relies on 0...N ordering. */
43 mir_window_attrib_type,
44 mir_window_attrib_state,
45- mir_window_attrib_swapinterval,
46+ mir_window_attrib_swapinterval, /**< \deprecated Do not listen for events
47+ reporting this attribute. Use the
48+ "mir_*_get_swapinterval()" functions
49+ instead if you wish query its value */
50 mir_window_attrib_focus,
51 mir_window_attrib_dpi,
52 mir_window_attrib_visibility,
53
54=== modified file 'src/client/CMakeLists.txt'
55--- src/client/CMakeLists.txt 2016-12-13 06:00:32 +0000
56+++ src/client/CMakeLists.txt 2017-01-17 06:23:26 +0000
57@@ -62,6 +62,7 @@
58 default_connection_configuration.cpp
59 connection_surface_map.cpp
60 private.cpp
61+ frame_clock.cpp
62 mir_screencast.cpp
63 mir_screencast_api.cpp
64 mir_cursor_api.cpp
65
66=== modified file 'src/client/buffer_stream.cpp'
67--- src/client/buffer_stream.cpp 2017-01-17 05:51:54 +0000
68+++ src/client/buffer_stream.cpp 2017-01-17 06:23:26 +0000
69@@ -253,6 +253,7 @@
70 client_platform(client_platform),
71 protobuf_bs{mcl::make_protobuf_object<mir::protobuf::BufferStream>(a_protobuf_bs)},
72 user_swap_interval(parse_env_for_swap_interval()),
73+ using_client_side_vsync(false),
74 interval_config{server, frontend::BufferStreamId{a_protobuf_bs.id().value()}},
75 scale_(1.0f),
76 perf_report(perf_report),
77@@ -264,6 +265,16 @@
78 factory(factory),
79 render_surface_(render_surface)
80 {
81+ /*
82+ * Since we try to use client-side vsync where possible, a separate
83+ * copy of the current swap interval is required to represent that
84+ * the stream might have a non-zero interval while we want the server
85+ * to use a zero interval. This is not stored inside interval_config
86+ * because with some luck interval_config will eventually go away
87+ * leaving just int current_swap_interval.
88+ */
89+ current_swap_interval = interval_config.swap_interval();
90+
91 if (!protobuf_bs->has_id())
92 {
93 if (!protobuf_bs->has_error())
94@@ -354,6 +365,10 @@
95 std::unique_lock<decltype(mutex)> lock(mutex);
96 perf_report->end_frame(id);
97
98+ if (!using_client_side_vsync &&
99+ current_swap_interval != interval_config.swap_interval())
100+ set_server_swap_interval(current_swap_interval);
101+
102 secured_region.reset();
103
104 // TODO: We can fix the strange "ID casting" used below in the second phase
105@@ -406,9 +421,73 @@
106 mir_display_output_id_invalid};
107 }
108
109+void mcl::BufferStream::wait_for_vsync()
110+{
111+ mir::time::PosixTimestamp last, target;
112+ std::shared_ptr<FrameClock> clock;
113+
114+ {
115+ std::lock_guard<decltype(mutex)> lock(mutex);
116+ if (!frame_clock)
117+ return;
118+ last = last_vsync;
119+ clock = frame_clock;
120+ }
121+
122+ target = clock->next_frame_after(last);
123+ sleep_until(target);
124+
125+ {
126+ std::lock_guard<decltype(mutex)> lock(mutex);
127+ if (target > last_vsync)
128+ last_vsync = target;
129+ }
130+}
131+
132+MirWaitHandle* mcl::BufferStream::set_server_swap_interval(int i)
133+{
134+ /*
135+ * TODO: Remove these functions in future, after
136+ * mir_buffer_stream_swap_buffers has been converted to use
137+ * client-side vsync, and the server has been told to always use
138+ * dropping for BufferStream. Then there will be no need to ever
139+ * change the server swap interval from zero.
140+ */
141+ buffer_depository->set_interval(i);
142+ return interval_config.set_swap_interval(i);
143+}
144+
145 void mcl::BufferStream::swap_buffers_sync()
146 {
147+ {
148+ std::lock_guard<decltype(mutex)> lock(mutex);
149+ using_client_side_vsync = true;
150+ }
151+
152+ /*
153+ * Until recently it seemed like server-side vsync could co-exist with
154+ * client-side vsync. However my original theory that it was risky has
155+ * proven to be true. If we leave server-side vsync throttling to interval
156+ * one at the same time as using client-side, there's a risk the server
157+ * will not get scheduled sufficiently to drain the queue as fast as
158+ * we fill it, creating lag. The acceptance test `ClientLatency' has now
159+ * proven this is a real problem so we must be sure to put the server in
160+ * interval 0 when using client-side vsync. This guarantees that random
161+ * scheduling imperfections won't create queuing lag.
162+ */
163+ if (interval_config.swap_interval() != 0)
164+ set_server_swap_interval(0);
165+
166 swap_buffers([](){})->wait_for_all();
167+
168+ {
169+ std::lock_guard<decltype(mutex)> lock(mutex);
170+ using_client_side_vsync = false;
171+ }
172+
173+ int interval = swap_interval();
174+ for (int i = 0; i < interval; ++i)
175+ wait_for_vsync();
176 }
177
178 void mcl::BufferStream::request_and_wait_for_configure(MirWindowAttrib attrib, int interval)
179@@ -428,16 +507,42 @@
180
181 int mcl::BufferStream::swap_interval() const
182 {
183- return interval_config.swap_interval();
184+ std::lock_guard<decltype(mutex)> lock(mutex);
185+ return current_swap_interval;
186 }
187
188 MirWaitHandle* mcl::BufferStream::set_swap_interval(int interval)
189 {
190- if (user_swap_interval.is_set())
191- interval = user_swap_interval.value();
192-
193- buffer_depository->set_interval(interval);
194- return interval_config.set_swap_interval(interval);
195+ std::lock_guard<decltype(mutex)> lock(mutex);
196+ current_swap_interval = user_swap_interval.is_set() ?
197+ user_swap_interval.value() : interval;
198+
199+ return set_server_swap_interval(current_swap_interval);
200+}
201+
202+void mcl::BufferStream::adopted_by(MirWindow* win)
203+{
204+ std::lock_guard<decltype(mutex)> lock(mutex);
205+ /*
206+ * Yes, we're storing raw pointers here. That's safe so long as MirWindow
207+ * remembers to always call unadopted_by prior to its destruction.
208+ * The alternative of MirWindow passing in a shared_ptr to itself is
209+ * actually uglier than this...
210+ */
211+ users.insert(win);
212+ if (!frame_clock)
213+ frame_clock = win->get_frame_clock();
214+}
215+
216+void mcl::BufferStream::unadopted_by(MirWindow* win)
217+{
218+ std::lock_guard<decltype(mutex)> lock(mutex);
219+ users.erase(win);
220+ if (frame_clock == win->get_frame_clock())
221+ {
222+ frame_clock = users.empty() ? nullptr
223+ : (*users.begin())->get_frame_clock();
224+ }
225 }
226
227 MirNativeBuffer* mcl::BufferStream::get_current_buffer_package()
228
229=== modified file 'src/client/buffer_stream.h'
230--- src/client/buffer_stream.h 2017-01-17 05:51:54 +0000
231+++ src/client/buffer_stream.h 2017-01-17 06:23:26 +0000
232@@ -26,11 +26,13 @@
233 #include "mir/geometry/size.h"
234 #include "mir/optional_value.h"
235 #include "buffer_stream_configuration.h"
236+#include "frame_clock.h"
237
238 #include "mir_toolkit/client_types.h"
239
240 #include <EGL/eglplatform.h>
241
242+#include <unordered_set>
243 #include <queue>
244 #include <memory>
245 #include <mutex>
246@@ -97,6 +99,8 @@
247
248 int swap_interval() const override;
249 MirWaitHandle* set_swap_interval(int interval) override;
250+ void adopted_by(MirWindow*) override;
251+ void unadopted_by(MirWindow*) override;
252 void set_buffer_cache_size(unsigned int) override;
253
254 EGLNativeWindowType egl_native_window() override;
255@@ -133,8 +137,9 @@
256 void process_buffer(protobuf::Buffer const& buffer, std::unique_lock<std::mutex>&);
257 void on_scale_set(float scale);
258 void release_cpu_region();
259- MirWaitHandle* force_swap_interval(int interval);
260+ MirWaitHandle* set_server_swap_interval(int i);
261 void init_swap_interval();
262+ void wait_for_vsync();
263
264 mutable std::mutex mutex; // Protects all members of *this
265
266@@ -143,6 +148,8 @@
267 std::unique_ptr<mir::protobuf::BufferStream> protobuf_bs;
268
269 optional_value<int> user_swap_interval;
270+ int current_swap_interval;
271+ bool using_client_side_vsync;
272 BufferStreamConfiguration interval_config;
273 float scale_;
274
275@@ -161,6 +168,10 @@
276 std::weak_ptr<SurfaceMap> const map;
277 std::shared_ptr<AsyncBufferFactory> const factory;
278 MirRenderSurface* render_surface_;
279+
280+ std::unordered_set<MirWindow*> users;
281+ std::shared_ptr<FrameClock> frame_clock;
282+ mir::time::PosixTimestamp last_vsync;
283 };
284
285 }
286
287=== modified file 'src/client/error_stream.cpp'
288--- src/client/error_stream.cpp 2017-01-17 05:51:54 +0000
289+++ src/client/error_stream.cpp 2017-01-17 06:23:26 +0000
290@@ -94,6 +94,12 @@
291 {
292 BOOST_THROW_EXCEPTION(std::runtime_error(error));
293 }
294+void mcl::ErrorStream::adopted_by(MirWindow*)
295+{
296+}
297+void mcl::ErrorStream::unadopted_by(MirWindow*)
298+{
299+}
300 MirNativeBuffer* mcl::ErrorStream::get_current_buffer_package()
301 {
302 BOOST_THROW_EXCEPTION(std::runtime_error(error));
303
304=== modified file 'src/client/error_stream.h'
305--- src/client/error_stream.h 2017-01-17 05:51:54 +0000
306+++ src/client/error_stream.h 2017-01-17 06:23:26 +0000
307@@ -41,6 +41,8 @@
308 std::shared_ptr<MemoryRegion> secure_for_cpu_write() override;
309 int swap_interval() const override;
310 MirWaitHandle* set_swap_interval(int interval) override;
311+ void adopted_by(MirWindow*) override;
312+ void unadopted_by(MirWindow*) override;
313 MirNativeBuffer* get_current_buffer_package() override;
314 MirPlatformType platform_type() override;
315 frontend::BufferStreamId rpc_id() const override;
316
317=== added file 'src/client/frame_clock.cpp'
318--- src/client/frame_clock.cpp 1970-01-01 00:00:00 +0000
319+++ src/client/frame_clock.cpp 2017-01-17 06:23:26 +0000
320@@ -0,0 +1,156 @@
321+/*
322+ * Copyright © 2016 Canonical Ltd.
323+ *
324+ * This program is free software: you can redistribute it and/or modify it
325+ * under the terms of the GNU Lesser General Public License version 3,
326+ * as published by the Free Software Foundation.
327+ *
328+ * This program is distributed in the hope that it will be useful,
329+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
330+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
331+ * GNU Lesser General Public License for more details.
332+ *
333+ * You should have received a copy of the GNU Lesser General Public License
334+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
335+ *
336+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
337+ */
338+
339+#include "frame_clock.h"
340+#include <stdexcept>
341+#include <cassert>
342+
343+using mir::client::FrameClock;
344+using mir::time::PosixTimestamp;
345+
346+namespace
347+{
348+typedef std::unique_lock<std::mutex> Lock;
349+} // namespace
350+
351+FrameClock::FrameClock(FrameClock::GetCurrentTime gct)
352+ : get_current_time{gct}
353+ , config_changed{false}
354+ , period{0}
355+ , resync_callback{std::bind(&FrameClock::fallback_resync_callback, this)}
356+{
357+}
358+
359+void FrameClock::set_period(std::chrono::nanoseconds ns)
360+{
361+ Lock lock(mutex);
362+ period = ns;
363+ config_changed = true;
364+}
365+
366+void FrameClock::set_resync_callback(ResyncCallback cb)
367+{
368+ Lock lock(mutex);
369+ resync_callback = cb;
370+ config_changed = true;
371+}
372+
373+PosixTimestamp FrameClock::fallback_resync_callback() const
374+{
375+ auto const now = get_current_time(PosixTimestamp().clock_id);
376+ Lock lock(mutex);
377+ /*
378+ * The result here needs to be in phase for all processes that call it,
379+ * so that nesting servers does not add lag.
380+ */
381+ return period != period.zero() ? now - (now % period) : now;
382+}
383+
384+PosixTimestamp FrameClock::next_frame_after(PosixTimestamp prev) const
385+{
386+ Lock lock(mutex);
387+ /*
388+ * Unthrottled is an option too. But why?... Because a stream might exist
389+ * that's not bound to a surface. And if it's not bound to a surface then
390+ * it has no physical screen to sync to. Hence not throttled at all.
391+ */
392+ if (period == period.zero())
393+ return prev;
394+
395+ /*
396+ * Regardless of render times and scheduling delays, we should always
397+ * target a perfectly even interval. This results in the greatest
398+ * visual smoothness as well as providing a catch-up for cases where
399+ * the client's render time was a little too long.
400+ */
401+ auto target = prev + period;
402+
403+ /*
404+ * Count missed frames. Note how the target time would need to be more than
405+ * one frame in the past to count as a missed frame. If the target time
406+ * is less than one frame in the past then there's still a chance to catch
407+ * up by rendering the next frame without delay. This avoids decimating
408+ * to half frame rate.
409+ * Compared to triple buffering, this approach is reactive rather than
410+ * proactive. Both approaches have the same catch-up ability to avoid
411+ * half frame rate, but this approach has the added benefit of only
412+ * one frame of lag to the compositor compared to triple buffering's two
413+ * frames of lag. But we're also doing better than double buffering here
414+ * in that this approach avoids any additive lag when nested.
415+ */
416+ auto now = get_current_time(target.clock_id);
417+ long const missed_frames = now > target ? (now - target) / period : 0L;
418+
419+ /*
420+ * On the first frame and any resumption frame (after the client goes
421+ * from idle to busy) this will trigger a query to ask for the latest
422+ * hardware vsync timestamp. So we get in phase with the display.
423+ * Crucially this is not required on most frames, so that even if it is
424+ * implemented as a round trip to the server, that won't happen often.
425+ */
426+ if (missed_frames > 1 || config_changed)
427+ {
428+ lock.unlock();
429+ auto const server_frame = resync_callback();
430+ lock.lock();
431+
432+ /*
433+ * Avoid mismatches (which will throw) and ensure we're always
434+ * comparing timestamps of the same clock ID. This means our result
435+ * 'target' might have a different clock to that of 'prev'. And that's
436+ * OK... we want to use whatever clock ID the driver is using for
437+ * its hardware vsync timestamps. We support migrating between clocks.
438+ */
439+ now = get_current_time(server_frame.clock_id);
440+
441+ /*
442+ * It's important to target a future time and not allow 'now'. This
443+ * is because we will have some buffer queues for the time being that
444+ * have swapinterval 1, which if overfilled by being too agressive will
445+ * cause visual lag. After all buffer queues move to always dropping
446+ * (mailbox mode), this delay won't be required as a safety.
447+ */
448+ if (server_frame > now)
449+ {
450+ target = server_frame;
451+ }
452+ else
453+ {
454+ auto const age_ns = now - server_frame;
455+ /*
456+ * Ensure age_frames gets truncated if not already.
457+ * C++ just guarantees a "signed integer type of at least 64 bits"
458+ * for std::chrono::nanoseconds::rep
459+ */
460+ auto const age_frames = age_ns / period;
461+ target = server_frame + (age_frames + 1) * period;
462+ }
463+ assert(target > now);
464+ config_changed = false;
465+ }
466+ else if (missed_frames > 0)
467+ {
468+ /*
469+ * Low number of missed frames. Try catching up without missing more...
470+ */
471+ // FIXME? Missing += operator
472+ target = target + missed_frames * period;
473+ }
474+
475+ return target;
476+}
477
478=== added file 'src/client/frame_clock.h'
479--- src/client/frame_clock.h 1970-01-01 00:00:00 +0000
480+++ src/client/frame_clock.h 2017-01-17 06:23:26 +0000
481@@ -0,0 +1,73 @@
482+/*
483+ * Copyright © 2016 Canonical Ltd.
484+ *
485+ * This program is free software: you can redistribute it and/or modify it
486+ * under the terms of the GNU Lesser General Public License version 3,
487+ * as published by the Free Software Foundation.
488+ *
489+ * This program is distributed in the hope that it will be useful,
490+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
491+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
492+ * GNU Lesser General Public License for more details.
493+ *
494+ * You should have received a copy of the GNU Lesser General Public License
495+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
496+ *
497+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
498+ */
499+
500+#ifndef MIR_CLIENT_FRAME_CLOCK_H_
501+#define MIR_CLIENT_FRAME_CLOCK_H_
502+
503+#include "mir/time/posix_timestamp.h"
504+#include <chrono>
505+#include <functional>
506+#include <mutex>
507+
508+namespace mir { namespace client {
509+
510+class FrameClock
511+{
512+public:
513+ typedef std::function<time::PosixTimestamp()> ResyncCallback;
514+ typedef std::function<time::PosixTimestamp(clockid_t)> GetCurrentTime;
515+
516+ FrameClock(GetCurrentTime);
517+ FrameClock() : FrameClock(mir::time::PosixTimestamp::now) {}
518+
519+ /**
520+ * Set the precise frame period in nanoseconds (1000000000/Hz).
521+ */
522+ void set_period(std::chrono::nanoseconds);
523+
524+ /**
525+ * Optionally set a callback that obtains the latest hardware vsync
526+ * timestamp. This provides phase correction for increased precision but is
527+ * not strictly required.
528+ * Lowest precision: Don't provide a callback.
529+ * Medium precision: Provide a callback which returns a recent timestamp.
530+ * Highest precision: Provide a callback that queries the server.
531+ */
532+ void set_resync_callback(ResyncCallback);
533+
534+ /**
535+ * Return the next timestamp to sleep_until, which comes after the last one
536+ * that was slept till. On the first frame you can just provide an
537+ * uninitialized timestamp.
538+ */
539+ time::PosixTimestamp next_frame_after(time::PosixTimestamp prev) const;
540+
541+private:
542+ time::PosixTimestamp fallback_resync_callback() const;
543+
544+ GetCurrentTime const get_current_time;
545+
546+ mutable std::mutex mutex; // Protects below fields:
547+ mutable bool config_changed;
548+ std::chrono::nanoseconds period;
549+ ResyncCallback resync_callback;
550+};
551+
552+}} // namespace mir::client
553+
554+#endif // MIR_CLIENT_FRAME_CLOCK_H_
555
556=== modified file 'src/client/mir_buffer_stream_api.cpp'
557--- src/client/mir_buffer_stream_api.cpp 2017-01-17 05:51:54 +0000
558+++ src/client/mir_buffer_stream_api.cpp 2017-01-17 06:23:26 +0000
559@@ -113,6 +113,13 @@
560 void* context)
561 try
562 {
563+ /*
564+ * TODO: Add client-side vsync support for mir_buffer_stream_swap_buffers()
565+ * Not in a hurry though, because the old server-side vsync is still
566+ * present and AFAIK the only user of swap_buffers callbacks is Xmir.
567+ * There are many ways to approach the problem and some more
568+ * contentious than others, so do it later.
569+ */
570 return buffer_stream->swap_buffers([buffer_stream, callback, context]{
571 if (callback)
572 callback(buffer_stream, context);
573@@ -209,7 +216,7 @@
574 MirWaitHandle* mir_buffer_stream_set_swapinterval(MirBufferStream* buffer_stream, int interval)
575 try
576 {
577- if ((interval < 0) || (interval > 1))
578+ if (interval < 0)
579 return nullptr;
580
581 if (!buffer_stream)
582
583=== modified file 'src/client/mir_surface.cpp'
584--- src/client/mir_surface.cpp 2017-01-17 05:51:54 +0000
585+++ src/client/mir_surface.cpp 2017-01-17 06:23:26 +0000
586@@ -21,6 +21,7 @@
587 #include "cursor_configuration.h"
588 #include "make_protobuf_object.h"
589 #include "mir_protobuf.pb.h"
590+#include "connection_surface_map.h"
591
592 #include "mir_toolkit/mir_client_library.h"
593 #include "mir/frontend/client_constants.h"
594@@ -31,6 +32,7 @@
595 #include "mir/input/xkb_mapper.h"
596 #include "mir/cookie/cookie.h"
597 #include "mir_cookie.h"
598+#include "mir/time/posix_timestamp.h"
599
600 #include <cassert>
601 #include <unistd.h>
602@@ -46,6 +48,8 @@
603 namespace gp = google::protobuf;
604 namespace md = mir::dispatch;
605
606+using mir::client::FrameClock;
607+
608 namespace
609 {
610 std::mutex handle_mutex;
611@@ -96,6 +100,7 @@
612 std::shared_ptr<MirWaitHandle> const& handle) :
613 surface{mcl::make_protobuf_object<mir::protobuf::Surface>()},
614 connection_(conn),
615+ frame_clock(std::make_shared<FrameClock>()),
616 creation_handle(handle)
617 {
618 surface->set_error(error);
619@@ -103,6 +108,8 @@
620
621 std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);
622 valid_surfaces.insert(this);
623+
624+ configure_frame_clock();
625 }
626
627 MirSurface::MirSurface(
628@@ -126,12 +133,19 @@
629 input_platform(input_platform),
630 keymapper(std::make_shared<mircv::XKBMapper>()),
631 configure_result{mcl::make_protobuf_object<mir::protobuf::SurfaceSetting>()},
632+ frame_clock(std::make_shared<FrameClock>()),
633 creation_handle(handle),
634 size({surface_proto.width(), surface_proto.height()}),
635 format(static_cast<MirPixelFormat>(surface_proto.pixel_format())),
636 usage(static_cast<MirBufferUsage>(surface_proto.buffer_usage())),
637 output_id(spec.output_id.is_set() ? spec.output_id.value() : static_cast<uint32_t>(mir_display_output_id_invalid))
638 {
639+ if (default_stream)
640+ {
641+ streams.insert(default_stream);
642+ default_stream->adopted_by(this);
643+ }
644+
645 for(int i = 0; i < surface_proto.attributes_size(); i++)
646 {
647 auto const& attrib = surface_proto.attributes(i);
648@@ -155,10 +169,23 @@
649
650 std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);
651 valid_surfaces.insert(this);
652+
653+ configure_frame_clock();
654 }
655
656 MirSurface::~MirSurface()
657 {
658+ StreamSet old_streams;
659+
660+ {
661+ std::lock_guard<decltype(mutex)> lock(mutex);
662+ old_streams = std::move(streams);
663+ }
664+
665+ // Do callbacks without holding the lock
666+ for (auto const& s : old_streams)
667+ s->unadopted_by(this);
668+
669 {
670 std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);
671 valid_surfaces.erase(this);
672@@ -172,6 +199,18 @@
673 close(surface->fd(i));
674 }
675
676+void MirSurface::configure_frame_clock()
677+{
678+ /*
679+ * TODO: Implement frame_clock->set_resync_callback(...) when IPC to get
680+ * timestamps from the server exists.
681+ * Until we do, client-side vsync will perform suboptimally (it is
682+ * randomly up to one frame out of phase with the real display).
683+ * However even while it's suboptimal it's dramatically lower latency
684+ * than the old approach and still totally eliminates nesting lag.
685+ */
686+}
687+
688 MirWindowParameters MirSurface::get_parameters() const
689 {
690 std::lock_guard<decltype(mutex)> lock(mutex);
691@@ -458,6 +497,36 @@
692 default_stream->set_size(size);
693 break;
694 }
695+ case mir_event_type_surface_output:
696+ {
697+ /*
698+ * We set the frame clock according to the (maximum) refresh rate of
699+ * the display. Note that we don't do the obvious thing and measure
700+ * previous vsyncs mainly because that's a very unreliable predictor
701+ * of the desired maximum refresh rate in the case of GSync/FreeSync
702+ * and panel self-refresh. You can't assume the previous frame
703+ * represents the display running at full native speed and so should
704+ * not measure it. Instead we conveniently have
705+ * mir_surface_output_event_get_refresh_rate that tells us the full
706+ * native speed of the most relevant output...
707+ */
708+ auto soevent = mir_event_get_surface_output_event(&e);
709+ auto rate = mir_surface_output_event_get_refresh_rate(soevent);
710+ if (rate > 10.0) // should be >0, but 10 to workaround LP: #1639725
711+ {
712+ std::chrono::nanoseconds const ns(
713+ static_cast<long>(1000000000L / rate));
714+ frame_clock->set_period(ns);
715+ }
716+ /* else: The graphics driver has not provided valid timing so we will
717+ * default to swap interval 0 behaviour.
718+ * Or would people prefer some different fallback?
719+ * - Re-enable server-side vsync?
720+ * - Rate limit to 60Hz?
721+ * - Just fix LP: #1639725?
722+ */
723+ break;
724+ }
725 default:
726 break;
727 };
728@@ -609,17 +678,30 @@
729
730 if (spec.streams.is_set())
731 {
732+ std::shared_ptr<mir::client::ConnectionSurfaceMap> map;
733+ StreamSet old_streams, new_streams;
734+
735 /*
736- * Note that we don't check for errors from modify_surface. So in
737- * updating default_stream here, we're just assuming it will succeed.
738- * Seems to be a harmless assumption to have made so far and much
739- * simpler than communicating back suceessful mappings from the server,
740- * but there is a prototype started for that: lp:~vanvugt/mir/adoption
741+ * Note that we are updating our local copy of 'streams' before the
742+ * server has updated its copy. That's mainly because we have never yet
743+ * implemented any kind of client-side error handling for when
744+ * modify_surface fails and is rejected. There's a prototype started
745+ * along those lines in lp:~vanvugt/mir/adoption, but it does not seem
746+ * to be important to block on...
747+ * The worst case is still perfectly acceptable: That is if the server
748+ * rejects the surface spec our local copy of streams is wrong. That's
749+ * a harmless consequence which just means the stream is now
750+ * synchronized to the output the client intended even though the server
751+ * is refusing to display it.
752 */
753 {
754 std::lock_guard<decltype(mutex)> lock(mutex);
755 default_stream = nullptr;
756+ old_streams = std::move(streams);
757+ streams.clear(); // Leave the container valid before we unlock
758+ map = connection_->connection_surface_map();
759 }
760+
761 for(auto const& stream : spec.streams.value())
762 {
763 auto const new_stream = surface_specification->add_stream();
764@@ -631,6 +713,27 @@
765 new_stream->set_width(stream.size.value().width.as_int());
766 new_stream->set_height(stream.size.value().height.as_int());
767 }
768+
769+ mir::frontend::BufferStreamId id(stream.stream_id);
770+ if (auto bs = map->stream(id))
771+ new_streams.insert(bs);
772+ /* else: we could implement stream ID validation here, which we've
773+ * never had before.
774+ * The only problem with that plan is that we have some tests
775+ * that rely on the ability to pass in invalid stream IDs so those
776+ * need fixing.
777+ */
778+ }
779+
780+ // Worth optimizing and avoiding the intersection of both sets?....
781+ for (auto const& old_stream : old_streams)
782+ old_stream->unadopted_by(this);
783+ for (auto const& new_stream : new_streams)
784+ new_stream->adopted_by(this);
785+ // ... probably not, since we can std::move(new_streams) this way...
786+ {
787+ std::unique_lock<decltype(mutex)> lock(mutex);
788+ streams = std::move(new_streams);
789 }
790 }
791
792@@ -672,3 +775,8 @@
793 {
794 return connection_;
795 }
796+
797+std::shared_ptr<FrameClock> MirSurface::get_frame_clock() const
798+{
799+ return frame_clock;
800+}
801
802=== modified file 'src/client/mir_surface.h'
803--- src/client/mir_surface.h 2017-01-17 05:51:54 +0000
804+++ src/client/mir_surface.h 2017-01-17 06:23:26 +0000
805@@ -21,6 +21,7 @@
806 #include "cursor_configuration.h"
807 #include "mir/mir_buffer_stream.h"
808 #include "mir_wait_handle.h"
809+#include "frame_clock.h"
810 #include "rpc/mir_display_server.h"
811 #include "rpc/mir_display_server_debug.h"
812
813@@ -32,6 +33,7 @@
814 #include "mir_toolkit/common.h"
815 #include "mir_toolkit/mir_client_library.h"
816 #include "mir/graphics/native_buffer.h"
817+#include "mir/time/posix_timestamp.h"
818
819 #include <memory>
820 #include <functional>
821@@ -214,9 +216,13 @@
822
823 MirWaitHandle* request_persistent_id(mir_surface_id_callback callback, void* context);
824 MirConnection* connection() const;
825+
826+ std::shared_ptr<mir::client::FrameClock> get_frame_clock() const;
827+
828 private:
829 std::mutex mutable mutex; // Protects all members of *this
830
831+ void configure_frame_clock();
832 void on_configured();
833 void on_cursor_configured();
834 void acquired_persistent_id(mir_surface_id_callback callback, void* context);
835@@ -242,6 +248,8 @@
836
837 //Deprecated functions can cause MirSurfaces to be created with a default stream
838 std::shared_ptr<MirBufferStream> default_stream;
839+ typedef std::unordered_set<std::shared_ptr<MirBufferStream>> StreamSet;
840+ StreamSet streams;
841 std::shared_ptr<mir::input::receiver::InputPlatform> const input_platform;
842 std::shared_ptr<mir::input::receiver::XKBMapper> const keymapper;
843
844@@ -251,6 +259,8 @@
845 int attrib_cache[mir_window_attribs];
846 MirOrientation orientation = mir_orientation_normal;
847
848+ std::shared_ptr<mir::client::FrameClock> const frame_clock;
849+
850 std::function<void(MirEvent const*)> handle_event_callback;
851 std::shared_ptr<mir::dispatch::ThreadedDispatcher> input_thread;
852
853
854=== modified file 'src/client/mir_surface_api.cpp'
855--- src/client/mir_surface_api.cpp 2017-01-17 05:51:54 +0000
856+++ src/client/mir_surface_api.cpp 2017-01-17 06:23:26 +0000
857@@ -1204,7 +1204,7 @@
858
859 MirWaitHandle* mir_surface_set_swapinterval(MirSurface* surf, int interval)
860 {
861- if ((interval < 0) || (interval > 1))
862+ if (interval < 0)
863 return nullptr;
864
865 try
866
867=== modified file 'src/client/screencast_stream.cpp'
868--- src/client/screencast_stream.cpp 2017-01-17 05:51:54 +0000
869+++ src/client/screencast_stream.cpp 2017-01-17 06:23:26 +0000
870@@ -264,6 +264,14 @@
871 BOOST_THROW_EXCEPTION(std::logic_error("Attempt to set swap interval on screencast is invalid"));
872 }
873
874+void mcl::ScreencastStream::adopted_by(MirWindow*)
875+{
876+}
877+
878+void mcl::ScreencastStream::unadopted_by(MirWindow*)
879+{
880+}
881+
882 void mcl::ScreencastStream::set_size(geom::Size)
883 {
884 BOOST_THROW_EXCEPTION(std::logic_error("Attempt to set size on screencast is invalid"));
885
886=== modified file 'src/client/screencast_stream.h'
887--- src/client/screencast_stream.h 2017-01-17 05:51:54 +0000
888+++ src/client/screencast_stream.h 2017-01-17 06:23:26 +0000
889@@ -78,6 +78,8 @@
890 uint32_t get_current_buffer_id() override;
891 int swap_interval() const override;
892 MirWaitHandle* set_swap_interval(int interval) override;
893+ void adopted_by(MirWindow*) override;
894+ void unadopted_by(MirWindow*) override;
895 void set_buffer_cache_size(unsigned int) override;
896
897 EGLNativeWindowType egl_native_window() override;
898
899=== modified file 'src/include/client/mir/mir_buffer_stream.h'
900--- src/include/client/mir/mir_buffer_stream.h 2017-01-17 05:51:54 +0000
901+++ src/include/client/mir/mir_buffer_stream.h 2017-01-17 06:23:26 +0000
902@@ -88,6 +88,8 @@
903
904 virtual int swap_interval() const = 0;
905 virtual MirWaitHandle* set_swap_interval(int interval) = 0;
906+ virtual void adopted_by(MirWindow*) = 0;
907+ virtual void unadopted_by(MirWindow*) = 0;
908
909 virtual MirNativeBuffer* get_current_buffer_package() = 0;
910 virtual MirPlatformType platform_type() = 0;
911
912=== modified file 'tests/acceptance-tests/test_client_library.cpp'
913--- tests/acceptance-tests/test_client_library.cpp 2017-01-17 05:51:54 +0000
914+++ tests/acceptance-tests/test_client_library.cpp 2017-01-17 06:23:26 +0000
915@@ -809,6 +809,11 @@
916
917 buffers = 0;
918
919+ // StubDisplayConfiguration is set to 60Hz and we now actually honour
920+ // that. So to avoid 1024 frames taking 17 seconds or so...
921+ mir_buffer_stream_set_swapinterval(mir_surface_get_buffer_stream(surf_one), 0);
922+ mir_buffer_stream_set_swapinterval(mir_surface_get_buffer_stream(surf_two), 0);
923+
924 while (buffers < 1024)
925 {
926 mir_buffer_stream_swap_buffers_sync(
927
928=== modified file 'tests/acceptance-tests/test_latency.cpp'
929--- tests/acceptance-tests/test_latency.cpp 2017-01-17 05:51:54 +0000
930+++ tests/acceptance-tests/test_latency.cpp 2017-01-17 06:23:26 +0000
931@@ -24,6 +24,7 @@
932 #include "mir/test/doubles/null_display.h"
933 #include "mir/test/doubles/null_display_buffer.h"
934 #include "mir/test/doubles/null_display_sync_group.h"
935+#include "mir/test/doubles/stub_display_configuration.h"
936 #include "mir/test/fake_shared.h"
937 #include "mir_test_framework/visible_surface.h"
938 #include "mir/options/option.h"
939@@ -240,12 +241,36 @@
940 TimeTrackingDisplay(Stats& stats)
941 : group{stats}
942 {
943+ mg::DisplayConfigurationOutput output{
944+ mg::DisplayConfigurationOutputId{1},
945+ mg::DisplayConfigurationCardId{1},
946+ mg::DisplayConfigurationOutputType::hdmia,
947+ std::vector<MirPixelFormat>{mir_pixel_format_abgr_8888},
948+ {{{1920,1080}, refresh_rate}}, // <=== Refresh rate must be valid
949+ 0, mir::geometry::Size{}, true, true, mir::geometry::Point{}, 0,
950+ mir_pixel_format_abgr_8888, mir_power_mode_on,
951+ mir_orientation_normal,
952+ 1.0f,
953+ mir_form_factor_monitor,
954+ mir_subpixel_arrangement_unknown,
955+ {},
956+ mir_output_gamma_unsupported,
957+ {}
958+ };
959+ outputs.push_back(output);
960+ }
961+
962+ std::unique_ptr<mg::DisplayConfiguration> configuration() const override
963+ {
964+ return std::make_unique<mtd::StubDisplayConfig>(outputs);
965 }
966
967 void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f) override
968 {
969 f(group);
970 }
971+
972+ std::vector<mg::DisplayConfigurationOutput> outputs;
973 TimeTrackingGroup group;
974 };
975
976@@ -288,7 +313,7 @@
977 };
978 }
979
980-TEST_F(ClientLatency, average_latency_is_less_than_nbuffers)
981+TEST_F(ClientLatency, average_swap_buffers_sync_latency_is_one_frame)
982 {
983 auto stream = mir_window_get_buffer_stream(surface);
984 auto const deadline = steady_clock::now() + 60s;
985@@ -308,7 +333,7 @@
986
987 auto average_latency = display.group.average_latency();
988
989- EXPECT_THAT(average_latency, Lt(expected_client_buffers));
990+ EXPECT_NEAR(1.0f, average_latency, error_margin);
991 }
992
993 TEST_F(ClientLatency, max_latency_is_limited_to_nbuffers)
994
995=== modified file 'tests/include/mir/test/doubles/mock_mir_buffer_stream.h'
996--- tests/include/mir/test/doubles/mock_mir_buffer_stream.h 2017-01-17 05:51:54 +0000
997+++ tests/include/mir/test/doubles/mock_mir_buffer_stream.h 2017-01-17 06:23:26 +0000
998@@ -43,6 +43,8 @@
999 MOCK_METHOD0(secure_for_cpu_write, std::shared_ptr<client::MemoryRegion>());
1000 MOCK_CONST_METHOD0(swap_interval, int());
1001 MOCK_METHOD1(set_swap_interval, MirWaitHandle*(int));
1002+ MOCK_METHOD1(adopted_by, void(MirWindow*));
1003+ MOCK_METHOD1(unadopted_by, void(MirWindow*));
1004 MOCK_METHOD0(platform_type, MirPlatformType(void));
1005 MOCK_METHOD0(get_current_buffer_package, MirNativeBuffer*(void));
1006 MOCK_METHOD0(get_create_wait_handle, MirWaitHandle*(void));
1007
1008=== modified file 'tests/unit-tests/client/CMakeLists.txt'
1009--- tests/unit-tests/client/CMakeLists.txt 2017-01-17 05:51:54 +0000
1010+++ tests/unit-tests/client/CMakeLists.txt 2017-01-17 06:23:26 +0000
1011@@ -24,6 +24,7 @@
1012 ${CMAKE_CURRENT_SOURCE_DIR}/test_client_mir_error.cpp
1013 ${CMAKE_CURRENT_SOURCE_DIR}/test_no_tls_future.cpp
1014 ${CMAKE_CURRENT_SOURCE_DIR}/test_mir_render_surface.cpp
1015+ ${CMAKE_CURRENT_SOURCE_DIR}/test_frame_clock.cpp
1016 )
1017
1018 if (NOT MIR_DISABLE_INPUT)
1019
1020=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
1021--- tests/unit-tests/client/test_client_mir_surface.cpp 2017-01-17 05:51:54 +0000
1022+++ tests/unit-tests/client/test_client_mir_surface.cpp 2017-01-17 06:23:26 +0000
1023@@ -416,6 +416,87 @@
1024 EXPECT_TRUE(dispatched->wait_for(std::chrono::seconds{5}));
1025 }
1026
1027+TEST_F(MirClientSurfaceTest, adopts_the_default_stream)
1028+{
1029+ using namespace ::testing;
1030+
1031+ auto mock_input_platform = std::make_shared<MockClientInputPlatform>();
1032+ auto mock_stream = std::make_shared<mtd::MockMirBufferStream>();
1033+
1034+ MirWindow* adopted_by = nullptr;
1035+ MirWindow* unadopted_by = nullptr;
1036+ EXPECT_CALL(*mock_stream, adopted_by(_))
1037+ .WillOnce(SaveArg<0>(&adopted_by));
1038+ EXPECT_CALL(*mock_stream, unadopted_by(_))
1039+ .WillOnce(SaveArg<0>(&unadopted_by));
1040+
1041+ {
1042+ MirWindow win{connection.get(), *client_comm_channel, nullptr,
1043+ mock_stream, mock_input_platform, spec, surface_proto, wh};
1044+ EXPECT_EQ(&win, adopted_by);
1045+ EXPECT_EQ(nullptr, unadopted_by);
1046+ }
1047+
1048+ EXPECT_NE(nullptr, unadopted_by);
1049+ EXPECT_EQ(adopted_by, unadopted_by);
1050+}
1051+
1052+TEST_F(MirClientSurfaceTest, adopts_custom_streams_if_set)
1053+{
1054+ using namespace testing;
1055+
1056+ auto mock_input_platform = std::make_shared<MockClientInputPlatform>();
1057+
1058+ mir::frontend::BufferStreamId const mock_old_stream_id(11);
1059+ auto mock_old_stream = std::make_shared<mtd::MockMirBufferStream>();
1060+ MirWindow* old_adopted_by = nullptr;
1061+ MirWindow* old_unadopted_by = nullptr;
1062+ EXPECT_CALL(*mock_old_stream, adopted_by(_))
1063+ .WillOnce(SaveArg<0>(&old_adopted_by));
1064+ EXPECT_CALL(*mock_old_stream, unadopted_by(_))
1065+ .WillOnce(SaveArg<0>(&old_unadopted_by));
1066+ ON_CALL(*mock_old_stream, rpc_id())
1067+ .WillByDefault(Return(mock_old_stream_id));
1068+
1069+ mir::frontend::BufferStreamId const mock_new_stream_id(22);
1070+ auto mock_new_stream = std::make_shared<mtd::MockMirBufferStream>();
1071+ MirWindow* new_adopted_by = nullptr;
1072+ MirWindow* new_unadopted_by = nullptr;
1073+ EXPECT_CALL(*mock_new_stream, adopted_by(_))
1074+ .WillOnce(SaveArg<0>(&new_adopted_by));
1075+ EXPECT_CALL(*mock_new_stream, unadopted_by(_))
1076+ .WillOnce(SaveArg<0>(&new_unadopted_by));
1077+ ON_CALL(*mock_new_stream, rpc_id())
1078+ .WillByDefault(Return(mock_new_stream_id));
1079+
1080+ surface_map->insert(mock_old_stream_id, mock_old_stream);
1081+ surface_map->insert(mock_new_stream_id, mock_new_stream);
1082+ {
1083+ MirWindow win{connection.get(), *client_comm_channel, nullptr,
1084+ mock_old_stream, mock_input_platform, spec, surface_proto, wh};
1085+
1086+ EXPECT_EQ(&win, old_adopted_by);
1087+ EXPECT_EQ(nullptr, old_unadopted_by);
1088+ EXPECT_EQ(nullptr, new_adopted_by);
1089+ EXPECT_EQ(nullptr, new_unadopted_by);
1090+
1091+ MirWindowSpec spec;
1092+ std::vector<ContentInfo> replacements
1093+ {
1094+ {geom::Displacement{0,0}, mock_new_stream_id.as_value(), geom::Size{1,1}}
1095+ };
1096+ spec.streams = replacements;
1097+ win.modify(spec)->wait_for_all();
1098+
1099+ EXPECT_EQ(&win, old_unadopted_by);
1100+ EXPECT_EQ(&win, new_adopted_by);
1101+ EXPECT_EQ(nullptr, new_unadopted_by);
1102+ }
1103+ EXPECT_EQ(new_adopted_by, new_unadopted_by);
1104+ surface_map->erase(mock_old_stream_id);
1105+ surface_map->erase(mock_new_stream_id);
1106+}
1107+
1108 TEST_F(MirClientSurfaceTest, replacing_delegate_with_nullptr_prevents_further_dispatch)
1109 {
1110 using namespace ::testing;
1111
1112=== added file 'tests/unit-tests/client/test_frame_clock.cpp'
1113--- tests/unit-tests/client/test_frame_clock.cpp 1970-01-01 00:00:00 +0000
1114+++ tests/unit-tests/client/test_frame_clock.cpp 2017-01-17 06:23:26 +0000
1115@@ -0,0 +1,420 @@
1116+/*
1117+ * Copyright © 2016 Canonical Ltd.
1118+ *
1119+ * This program is free software: you can redistribute it and/or modify
1120+ * it under the terms of the GNU General Public License version 3 as
1121+ * published by the Free Software Foundation.
1122+ *
1123+ * This program is distributed in the hope that it will be useful,
1124+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
1125+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1126+ * GNU General Public License for more details.
1127+ *
1128+ * You should have received a copy of the GNU General Public License
1129+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
1130+ *
1131+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
1132+ */
1133+
1134+#include "src/client/frame_clock.h"
1135+#include <gtest/gtest.h>
1136+#include <unordered_map>
1137+
1138+using namespace ::testing;
1139+using namespace ::std::chrono_literals;
1140+using mir::time::PosixTimestamp;
1141+using mir::client::FrameClock;
1142+
1143+class FrameClockTest : public Test
1144+{
1145+public:
1146+ FrameClockTest()
1147+ {
1148+ with_fake_time = [this](clockid_t clk){ return fake_time[clk]; };
1149+ }
1150+
1151+ void SetUp()
1152+ {
1153+ for (auto& c : {CLOCK_MONOTONIC, CLOCK_REALTIME,
1154+ CLOCK_MONOTONIC_RAW, CLOCK_REALTIME_COARSE,
1155+ CLOCK_MONOTONIC_COARSE, CLOCK_BOOTTIME})
1156+ fake_time[c] = PosixTimestamp(c, c * c * 1234567ns);
1157+ int const hz = 60;
1158+ one_frame = std::chrono::nanoseconds(1000000000L/hz);
1159+ }
1160+
1161+ void fake_sleep_for(std::chrono::nanoseconds ns)
1162+ {
1163+ for (auto& f : fake_time)
1164+ f.second.nanoseconds += ns;
1165+ }
1166+
1167+ void fake_sleep_until(PosixTimestamp t)
1168+ {
1169+ auto& now = fake_time[t.clock_id];
1170+ if (t > now)
1171+ {
1172+ auto delta = t.nanoseconds - now.nanoseconds;
1173+ fake_sleep_for(delta);
1174+ }
1175+ }
1176+
1177+protected:
1178+ FrameClock::GetCurrentTime with_fake_time;
1179+ std::unordered_map<clockid_t,PosixTimestamp> fake_time;
1180+ std::chrono::nanoseconds one_frame;
1181+};
1182+
1183+TEST_F(FrameClockTest, unthrottled_without_a_period)
1184+{
1185+ FrameClock clock(with_fake_time);
1186+
1187+ PosixTimestamp a;
1188+ auto b = clock.next_frame_after(a);
1189+ EXPECT_EQ(a, b);
1190+ auto c = clock.next_frame_after(b);
1191+ EXPECT_EQ(b, c);
1192+}
1193+
1194+TEST_F(FrameClockTest, first_frame_is_within_one_frame)
1195+{
1196+ auto& now = fake_time[CLOCK_MONOTONIC];
1197+ auto a = now;
1198+ FrameClock clock(with_fake_time);
1199+ clock.set_period(one_frame);
1200+
1201+ PosixTimestamp b;
1202+ auto c = clock.next_frame_after(b);
1203+ EXPECT_GE(c, a);
1204+ EXPECT_LE(c-a, one_frame);
1205+}
1206+
1207+TEST_F(FrameClockTest, interval_is_perfectly_smooth)
1208+{
1209+ FrameClock clock(with_fake_time);
1210+ clock.set_period(one_frame);
1211+
1212+ PosixTimestamp a;
1213+ auto b = clock.next_frame_after(a);
1214+
1215+ fake_sleep_until(b);
1216+ fake_sleep_for(one_frame/13); // short render time
1217+
1218+ auto c = clock.next_frame_after(b);
1219+ EXPECT_EQ(one_frame, c - b);
1220+
1221+ fake_sleep_until(c);
1222+ fake_sleep_for(one_frame/7); // short render time
1223+
1224+ auto d = clock.next_frame_after(c);
1225+ EXPECT_EQ(one_frame, d - c);
1226+
1227+ fake_sleep_until(d);
1228+ fake_sleep_for(one_frame/5); // short render time
1229+
1230+ auto e = clock.next_frame_after(d);
1231+ EXPECT_EQ(one_frame, e - d);
1232+}
1233+
1234+TEST_F(FrameClockTest, long_render_time_is_recoverable_without_decimation)
1235+{
1236+ FrameClock clock(with_fake_time);
1237+ clock.set_period(one_frame);
1238+
1239+ auto& now = fake_time[CLOCK_MONOTONIC];
1240+ auto a = now;
1241+ auto b = clock.next_frame_after(a);
1242+
1243+ fake_sleep_until(b);
1244+ fake_sleep_for(one_frame * 5 / 4); // long render time; over a frame
1245+
1246+ auto c = clock.next_frame_after(b);
1247+ EXPECT_EQ(one_frame, c - b);
1248+
1249+ fake_sleep_until(c);
1250+ fake_sleep_for(one_frame * 7 / 6); // long render time; over a frame
1251+
1252+ auto d = clock.next_frame_after(c);
1253+ EXPECT_EQ(one_frame, d - c);
1254+
1255+ EXPECT_LT(d, now);
1256+
1257+ fake_sleep_until(d);
1258+ fake_sleep_for(one_frame/4); // short render time
1259+
1260+ // We can recover since we had a short render time...
1261+ auto e = clock.next_frame_after(d);
1262+ EXPECT_EQ(one_frame, e - d);
1263+}
1264+
1265+TEST_F(FrameClockTest, resuming_from_sleep_targets_the_future)
1266+{
1267+ FrameClock clock(with_fake_time);
1268+ clock.set_period(one_frame);
1269+
1270+ auto& now = fake_time[CLOCK_MONOTONIC];
1271+ PosixTimestamp a = now;
1272+ auto b = clock.next_frame_after(a);
1273+ fake_sleep_until(b);
1274+ auto c = clock.next_frame_after(b);
1275+ EXPECT_EQ(one_frame, c - b);
1276+ fake_sleep_until(c);
1277+
1278+ // Client idles for a while without producing new frames:
1279+ fake_sleep_for(567 * one_frame);
1280+
1281+ auto d = clock.next_frame_after(c);
1282+ EXPECT_GT(d, now); // Resumption must be in the future
1283+ EXPECT_LE(d, now+one_frame); // But not too far in the future
1284+}
1285+
1286+TEST_F(FrameClockTest, multiple_streams_in_sync)
1287+{
1288+ FrameClock clock(with_fake_time);
1289+ clock.set_period(one_frame);
1290+
1291+ PosixTimestamp left;
1292+ left = clock.next_frame_after(left);
1293+
1294+ fake_sleep_for(one_frame / 9);
1295+
1296+ PosixTimestamp right;
1297+ right = clock.next_frame_after(right);
1298+
1299+ ASSERT_EQ(left, right);
1300+ fake_sleep_until(left);
1301+ fake_sleep_until(right);
1302+
1303+ left = clock.next_frame_after(left);
1304+ fake_sleep_for(one_frame / 5);
1305+ right = clock.next_frame_after(right);
1306+
1307+ ASSERT_EQ(left, right);
1308+ fake_sleep_until(left);
1309+ fake_sleep_until(right);
1310+
1311+ left = clock.next_frame_after(left);
1312+ fake_sleep_for(one_frame / 7);
1313+ right = clock.next_frame_after(right);
1314+
1315+ ASSERT_EQ(left, right);
1316+}
1317+
1318+TEST_F(FrameClockTest, moving_between_displays_adapts_to_new_rate)
1319+{
1320+ FrameClock clock(with_fake_time);
1321+ clock.set_period(one_frame);
1322+
1323+ auto const one_tv_frame = std::chrono::nanoseconds(1000000000L / 25);
1324+ ASSERT_NE(one_frame, one_tv_frame);
1325+
1326+ PosixTimestamp a;
1327+ auto b = clock.next_frame_after(a);
1328+ fake_sleep_until(b);
1329+ auto c = clock.next_frame_after(b);
1330+ EXPECT_EQ(one_frame, c - b);
1331+
1332+ fake_sleep_until(c);
1333+
1334+ // Window moves to a new slower display:
1335+ clock.set_period(one_tv_frame);
1336+ auto d = clock.next_frame_after(c);
1337+ // Clock keeps ticking into the future for the new display:
1338+ EXPECT_GT(d, c);
1339+ // But not too far in the future:
1340+ EXPECT_LE(d, c + one_tv_frame);
1341+
1342+ // Vsync is now at the slower rate of the TV:
1343+ fake_sleep_until(d);
1344+ auto e = clock.next_frame_after(d);
1345+ EXPECT_EQ(one_tv_frame, e - d);
1346+ fake_sleep_until(e);
1347+ auto f = clock.next_frame_after(e);
1348+ EXPECT_EQ(one_tv_frame, f - e);
1349+
1350+ fake_sleep_until(f);
1351+
1352+ // Window moves back to the faster display:
1353+ clock.set_period(one_frame);
1354+ auto g = clock.next_frame_after(f);
1355+ // Clock keeps ticking into the future for the new display:
1356+ EXPECT_GT(g, f);
1357+ // But not too far in the future:
1358+ EXPECT_LE(g, f + one_tv_frame);
1359+
1360+ // Vsync is now at the faster rate again:
1361+ fake_sleep_until(g);
1362+ auto h = clock.next_frame_after(g);
1363+ EXPECT_EQ(one_frame, h - g);
1364+ fake_sleep_until(e);
1365+ auto i = clock.next_frame_after(h);
1366+ EXPECT_EQ(one_frame, i - h);
1367+}
1368+
1369+TEST_F(FrameClockTest, resuming_comes_in_phase_with_server_vsync)
1370+{
1371+ FrameClock clock(with_fake_time);
1372+ clock.set_period(one_frame);
1373+
1374+ auto& now = fake_time[CLOCK_MONOTONIC];
1375+ PosixTimestamp a = now;
1376+ auto b = clock.next_frame_after(a);
1377+ fake_sleep_until(b);
1378+ auto c = clock.next_frame_after(b);
1379+ EXPECT_EQ(one_frame, c - b);
1380+ fake_sleep_until(c);
1381+
1382+ // Client idles for a while without producing new frames:
1383+ fake_sleep_for(789 * one_frame);
1384+
1385+ auto last_server_frame = now - 556677ns;
1386+ clock.set_resync_callback([last_server_frame](){return last_server_frame;});
1387+
1388+ auto d = clock.next_frame_after(c);
1389+ EXPECT_GT(d, now); // Resumption must be in the future
1390+ EXPECT_LE(d, now+one_frame); // But not too far in the future
1391+
1392+ auto server_phase = last_server_frame % one_frame;
1393+ EXPECT_NE(server_phase, c % one_frame); // wasn't in phase before
1394+ EXPECT_EQ(server_phase, d % one_frame); // but is in phase now
1395+
1396+ // Not only did we come in phase but we're targeting the soonest frame
1397+ EXPECT_EQ(last_server_frame+one_frame, d);
1398+}
1399+
1400+TEST_F(FrameClockTest, switches_to_the_server_clock_on_startup)
1401+{
1402+ FrameClock clock(with_fake_time);
1403+ clock.set_period(one_frame);
1404+
1405+ PosixTimestamp a(CLOCK_MONOTONIC_COARSE, 0ns);
1406+
1407+ PosixTimestamp b;
1408+ EXPECT_NO_THROW({
1409+ b = clock.next_frame_after(a);
1410+ });
1411+
1412+ // The default resync callback uses CLOCK_MONOTONIC...
1413+ EXPECT_NE(a.clock_id, b.clock_id);
1414+}
1415+
1416+TEST_F(FrameClockTest, can_migrate_between_different_driver_clocks)
1417+{
1418+ FrameClock clock(with_fake_time);
1419+ clock.set_period(one_frame);
1420+
1421+ PosixTimestamp a = fake_time[CLOCK_MONOTONIC];
1422+ auto b = clock.next_frame_after(a);
1423+ fake_sleep_until(b);
1424+ auto c = clock.next_frame_after(b);
1425+ EXPECT_EQ(one_frame, c - b);
1426+
1427+ fake_sleep_until(c);
1428+
1429+ // Window moves between displays and in the glorious future that might even
1430+ // mean it switches cards, with different drivers, different clocks...
1431+ auto last_server_frame = fake_time[CLOCK_MONOTONIC_COARSE] - 556677ns;
1432+ clock.set_resync_callback([last_server_frame](){return last_server_frame;});
1433+
1434+ EXPECT_EQ(CLOCK_MONOTONIC, c.clock_id);
1435+ auto d = clock.next_frame_after(c);
1436+ EXPECT_EQ(CLOCK_MONOTONIC_COARSE, d.clock_id);
1437+
1438+ EXPECT_GT(d, fake_time[CLOCK_MONOTONIC_COARSE]);
1439+ EXPECT_LE(d, fake_time[CLOCK_MONOTONIC_COARSE]+one_frame);
1440+
1441+ auto server_phase = last_server_frame % one_frame;
1442+ EXPECT_NE(server_phase, c % one_frame); // wasn't in phase before
1443+ EXPECT_EQ(server_phase, d % one_frame); // but is in phase now
1444+
1445+ // Not only did we come in phase but we're targeting the soonest frame
1446+ EXPECT_EQ(last_server_frame+one_frame, d);
1447+}
1448+
1449+TEST_F(FrameClockTest, does_not_resync_on_most_frames)
1450+{
1451+ int callbacks = 0;
1452+
1453+ FrameClock clock(with_fake_time);
1454+ clock.set_period(one_frame);
1455+ clock.set_resync_callback([&callbacks]
1456+ {
1457+ ++callbacks;
1458+ return PosixTimestamp();
1459+ });
1460+
1461+ PosixTimestamp v;
1462+ v = clock.next_frame_after(v);
1463+ EXPECT_EQ(1, callbacks); // sync with server happens on first frame
1464+
1465+ fake_sleep_until(v);
1466+
1467+ v = clock.next_frame_after(v);
1468+ EXPECT_EQ(1, callbacks);
1469+
1470+ fake_sleep_until(v);
1471+
1472+ v = clock.next_frame_after(v);
1473+ EXPECT_EQ(1, callbacks);
1474+
1475+ fake_sleep_until(v);
1476+
1477+ v = clock.next_frame_after(v);
1478+ EXPECT_EQ(1, callbacks);
1479+
1480+ fake_sleep_for(one_frame * 10);
1481+
1482+ clock.next_frame_after(v);
1483+ EXPECT_EQ(2, callbacks); // resync because we went idle too long
1484+}
1485+
1486+TEST_F(FrameClockTest, one_frame_skipped_only_after_2_frames_take_3_periods)
1487+{
1488+ FrameClock clock(with_fake_time);
1489+ clock.set_period(one_frame);
1490+
1491+ PosixTimestamp a;
1492+ auto b = clock.next_frame_after(a);
1493+
1494+ fake_sleep_until(b);
1495+ fake_sleep_for(one_frame * 3 / 2); // Render time: 1.5 frames
1496+
1497+ auto c = clock.next_frame_after(b);
1498+ EXPECT_EQ(one_frame, c - b); // No frame skipped
1499+
1500+ fake_sleep_until(c);
1501+ fake_sleep_for(one_frame * 8 / 5); // Render time: 1.6 frames
1502+
1503+ auto d = clock.next_frame_after(c);
1504+ EXPECT_EQ(2*one_frame, d - c); // One frame skipped
1505+
1506+ fake_sleep_until(d);
1507+ fake_sleep_for(one_frame/4); // Short render time, immediate recovery
1508+
1509+ auto e = clock.next_frame_after(d);
1510+ EXPECT_EQ(one_frame, e - d); // No frame skipped. We have recovered.
1511+}
1512+
1513+TEST_F(FrameClockTest, nesting_adds_zero_lag)
1514+{
1515+ FrameClock inner(with_fake_time);
1516+ FrameClock outer(with_fake_time);
1517+ inner.set_period(one_frame);
1518+ outer.set_period(one_frame);
1519+
1520+ PosixTimestamp in0, out0;
1521+ auto in1 = inner.next_frame_after(in0);
1522+ fake_sleep_for(1234567ns); // scheduling and IPC delay
1523+ auto out1 = outer.next_frame_after(out0);
1524+ EXPECT_EQ(in1, out1);
1525+
1526+ fake_sleep_until(in1);
1527+ fake_sleep_until(out1);
1528+
1529+ auto in2 = inner.next_frame_after(in1);
1530+ fake_sleep_for(8765432ns); // scheduling and IPC delay
1531+ auto out2 = outer.next_frame_after(out1);
1532+ EXPECT_EQ(in2, out2);
1533+ EXPECT_EQ(one_frame, in2 - in1);
1534+ EXPECT_EQ(one_frame, out2 - out1);
1535+}
1536
1537=== modified file 'tests/unit-tests/platforms/android/server/test_hwc_device.cpp'
1538--- tests/unit-tests/platforms/android/server/test_hwc_device.cpp 2016-12-09 02:54:31 +0000
1539+++ tests/unit-tests/platforms/android/server/test_hwc_device.cpp 2017-01-17 06:23:26 +0000
1540@@ -601,6 +601,7 @@
1541
1542 //LP: #1369763. We could get swapinterval 0 to work with overlays, but we'd need
1543 //the vsync signal from hwc to reach the client, so the client can rate-limit its submissions.
1544+// Update: We have client-side vsync now so it should be possible to do this...
1545 TEST_F(HwcDevice, rejects_list_containing_interval_0)
1546 {
1547 mga::HwcDevice device(mock_device);

Subscribers

People subscribed via source and target branches