Mir

Merge lp://qastaging/~vanvugt/mir/timerless-multimonitor-frame-sync-without-bypass into lp://qastaging/~mir-team/mir/trunk

  1. timerless-multimonitor-frame-sync-without-bypass
  2. Merge into trunk
Proposed by Daniel van Vugt
Status: Rejected
Rejected by: Daniel van Vugt
Proposed branch: lp://qastaging/~vanvugt/mir/timerless-multimonitor-frame-sync-without-bypass
Merge into: lp://qastaging/~mir-team/mir/trunk
Diff against target: 1100 lines (+162/-165)
33 files modified
include/server/mir/compositor/buffer_stream_surfaces.h (+2/-1)
include/server/mir/compositor/renderer.h (+1/-1)
include/server/mir/surfaces/buffer_stream.h (+2/-1)
include/test/mir_test_doubles/mock_buffer_bundle.h (+1/-1)
include/test/mir_test_doubles/mock_buffer_stream.h (+2/-1)
include/test/mir_test_doubles/mock_surface_renderer.h (+1/-1)
include/test/mir_test_doubles/stub_buffer_stream.h (+1/-1)
src/server/compositor/buffer_bundle.h (+2/-1)
src/server/compositor/buffer_stream_surfaces.cpp (+4/-2)
src/server/compositor/default_display_buffer_compositor.cpp (+21/-2)
src/server/compositor/default_display_buffer_compositor.h (+2/-0)
src/server/compositor/gl_renderer.cpp (+4/-2)
src/server/compositor/gl_renderer.h (+3/-1)
src/server/compositor/switching_bundle.cpp (+5/-7)
src/server/compositor/switching_bundle.h (+3/-6)
src/server/compositor/temporary_buffers.cpp (+2/-2)
src/server/compositor/temporary_buffers.h (+1/-1)
src/server/surfaces/surface.cpp (+1/-1)
tests/acceptance-tests/test_server_shutdown.cpp (+1/-1)
tests/integration-tests/compositor/test_buffer_stream.cpp (+15/-21)
tests/integration-tests/compositor/test_swapping_swappers.cpp (+4/-2)
tests/integration-tests/shell/test_session.cpp (+2/-2)
tests/integration-tests/test_surface_first_frame_sync.cpp (+1/-1)
tests/integration-tests/test_swapinterval.cpp (+1/-1)
tests/mir_test_framework/testing_server_options.cpp (+2/-2)
tests/unit-tests/compositor/test_buffer_stream.cpp (+4/-4)
tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp (+1/-1)
tests/unit-tests/compositor/test_gl_renderer.cpp (+1/-1)
tests/unit-tests/compositor/test_rendering_operator.cpp (+1/-1)
tests/unit-tests/compositor/test_switching_bundle.cpp (+65/-91)
tests/unit-tests/compositor/test_temporary_buffers.cpp (+3/-3)
tests/unit-tests/surfaces/test_surface.cpp (+1/-1)
tests/unit-tests/surfaces/test_surface_stack.cpp (+2/-1)
To merge this branch: bzr merge lp://qastaging/~vanvugt/mir/timerless-multimonitor-frame-sync-without-bypass
Reviewer Review Type Date Requested Status
Alan Griffiths Needs Fixing
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+182540@code.qastaging.launchpad.net

Commit message

Dramatically improved multi-monitor frame synchronization, using a global
frame count equivalent to the highest refresh rate of all monitors. This is
much more reliable than the old logic which was based on timers.

This fixes LP: #1210478

Description of the change

This version has no dependencies on bypass, unlike the previous version. However it will conflict with bypass slightly. So whatever comes last will need fixing.

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

PASSED: Continuous integration, rev:1028
http://jenkins.qa.ubuntu.com/job/mir-ci/1407/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1892
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1777
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/645
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/645/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

171 + if (global_frame_count < local_frame_count || local_frame_count <= 0)
172 + global_frame_count = local_frame_count;
173 + else
174 + local_frame_count = global_frame_count;

Start with:

thread 1: local_frame_count = MAX_UNIT
thread 2: local_frame_count = MAX_UNIT-1

thread 1 runs through this code leaving:

global_frame_count == (t1)local_frame_count == 0

Then thread 2 runs through it leaving:

global_frame_count == (t2)local_frame_count == MAX_UNIT

When the result needed by the algorithm is:

global_frame_count == (t2)local_frame_count == 0

Things do sort themselves out and the error is mostly harmless but it could be improved upon. Vis:

171 + if (global_frame_count < local_frame_count || local_frame_count <= 0)
          if (global_frame_count - local_frame_count > UNIT_MAX/2)

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I'm still not entirely happy with the naming - frameno, global_frame_count and local_frame_count are not really what their names suggest. They are neither identifiers nor counts but actually sequence numbers.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Admittedly you'd have to be running for 2.2 years before the wrapping occurs. :)

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> Admittedly you'd have to be running for 2.2 years before the wrapping occurs.
> :)

Or with a frame-rate beyond current technology. ;)

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Discussion moved back to:
https://code.launchpad.net/~vanvugt/mir/timerless-multimonitor-frame-sync/+merge/182085

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