Mir

Merge lp://staging/~raof/mir/implement-preserve-framebuffers-for-android into lp://staging/mir

  1. implement-preserve-framebuffers-for-android
  2. Merge into development-branch
Proposed by Chris Halse Rogers
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: no longer in the source branch.
Merged at revision: 3806
Proposed branch: lp://staging/~raof/mir/implement-preserve-framebuffers-for-android
Merge into: lp://staging/mir
Diff against target: 385 lines (+140/-57)
16 files modified
include/platform/mir/graphics/display.h (+1/-1)
include/test/mir/test/doubles/null_display.h (+1/-1)
src/platform/graphics/CMakeLists.txt (+1/-0)
src/platforms/android/server/display.cpp (+68/-43)
src/platforms/android/server/display.h (+4/-1)
src/platforms/eglstream-kms/server/display.cpp (+1/-1)
src/platforms/eglstream-kms/server/display.h (+1/-1)
src/platforms/mesa/server/kms/display.cpp (+1/-1)
src/platforms/mesa/server/kms/display.h (+1/-1)
src/platforms/mesa/server/x11/graphics/display.cpp (+1/-1)
src/platforms/mesa/server/x11/graphics/display.h (+1/-1)
src/server/graphics/nested/display.cpp (+1/-1)
src/server/graphics/nested/display.h (+1/-1)
tests/include/mir/test/doubles/mock_display.h (+1/-2)
tests/mir_test_framework/stubbed_graphics_platform.cpp (+1/-1)
tests/unit-tests/platforms/android/server/test_display.cpp (+55/-0)
To merge this branch: bzr merge lp://staging/~raof/mir/implement-preserve-framebuffers-for-android
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Kevin DuBois (community) Approve
Andreas Pokorny (community) Approve
Review via email: mp+309902@code.staging.launchpad.net

Commit message

Implement Display::apply_if_configuration_preserves_display_buffers() for Android platform (part 2 of LP: #1556142).

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

PASSED: Continuous integration, rev:3802
https://mir-jenkins.ubuntu.com/job/mir-ci/2100/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2700
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2763
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2755
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2755
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2755
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2729
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2729/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/2729
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2729/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2729
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2729/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/2729
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2729/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/2729
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2729/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/2729
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2729/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

in bool mga::Display::apply_if_configuration_preserves_display_buffers:
+ /*
+ * We never invalidate display buffers based on the configuration to apply.
+ *
+ * The only way we invalidate a display buffer is if we detect that a previously-connected
+ * external display has been removed. In that case, regardless of whether or not it's enabled
+ * in the configuration, we destroy the display.
+ *
+ * Take the configuration lock to ensure consistency between our checking for external
+ * connections and configure's checking.
+ */
+ std::lock_guard<decltype(configuration_mutex)> lock{configuration_mutex};
+ if (!config.external().connected || displays.display_present(mga::DisplayName::external))
+ {
+ configure_locked(conf, lock);

I do not see how the comment matches with the test underneath .. shouldnt "conf" be involved in the test.

review: Needs Information
Revision history for this message
Chris Halse Rogers (raof) wrote :

Hah. I've clearly worded that badly, then.

The comment is trying to describe *why* conf doesn't appear in the if() 😀.

The reason is that the only time the Android platform invalidates a display buffer is when it detects that a previously-connected external display is disconnected, and it does this *even if* the requested configuration has the external display enabled.

Likewise, it never destroys the display buffer for a connected external display, even if the configuration has it disabled.

So the configuration that is going to be applied is irrelevant. Only the hardware state change matters.

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

regardless if it is enabled in the *new* configuration -- ah! .. the new part was missing

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

The changes look reasonable - if you happen to touch it again, maybe update the comment..

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

lgtm

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/755/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2722/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/798/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2785
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2777
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2777
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2777
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2751
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2751/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/2751
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2751/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2751
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2751/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/2751/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2751/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/2751
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2751/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/2751
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2751/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Looks like temporary network failure in CI; retriggering.

Revision history for this message
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

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