Merge lp:~andreas-pokorny/mir/preserve-framebuffer-on-compatible-configurations-in-stub-graphics into lp:mir

[Andreas Pokorny]

Make FakeDisplay preserve display buffers when the displayed region stays unchanged

This is a split out of the input configuration work. This change avoids a compositor thread shut down in tests that use only a single simulated display. The stopping of the compositor threads marks the nested surfaces occluded - hence cannot receive input for some time. That time frame is hard to track from the outside.

The rules for preserving the display buffers are based on what kms currently does.

[Mir CI Bot (mir-ci-bot)]

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

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

review: Approve (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3812
https://mir-jenkins.ubuntu.com/job/mir-ci/2138/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2769/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2832
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2824
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2824
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2824
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2798
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2798/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/2798/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2798
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2798/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/2798
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2798/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/2798
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2798/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/2798
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2798/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

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

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Again, please avoid the long branch names. It messes with Launchpad webpage formatting.

[Chris Halse Rogers (raof)]

Huh. I was sure I'd already reviewed this.

It's conservative - compatible() will return false in lots of cases where DisplayBuffers would not need to be invalidated - but so are all the real hardware implementations.

review: Approve

[Andreas Pokorny (andreas-pokorny)]

> Huh. I was sure I'd already reviewed this.
>
> It's conservative - compatible() will return false in lots of cases where
> DisplayBuffers would not need to be invalidated - but so are all the real
> hardware implementations.

yes thats the motivation.. I think we could reduce the invalidating cases in the hardware implementations, i.e. we could make the position of the db not be part of the db - but just an information the compositors get from the configuration. Then we can come back here and maybe reiterate on the behavior of fake display.

[Andreas Pokorny (andreas-pokorny)]

> Huh. I was sure I'd already reviewed this.
>
> It's conservative - compatible() will return false in lots of cases where
> DisplayBuffers would not need to be invalidated - but so are all the real
> hardware implementations.

yes thats the motivation.. I think we could reduce the invalidating cases in the hardware implementations, i.e. we could make the position of the db not be part of the db - but just an information the compositors get from the configuration. Then we can come back here and maybe reiterate on the behavior of fake display.

[Alan Griffiths (alan-griffiths)]

+ auto new_configuration = std::make_shared<StubDisplayConfig>(new_config);

We could probably arrange to defer creating this object until we've tested compatibility, but that's no blocker.

review: Approve

=== modified file 'include/test/mir/test/doubles/fake_display.h'
--- include/test/mir/test/doubles/fake_display.h	2016-09-06 06:14:17 +0000
+++ include/test/mir/test/doubles/fake_display.h	2016-11-09 15:08:34 +0000
@@ -34,6 +34,7 @@
 {
 namespace doubles
 {
+class StubDisplayConfig;
 class FakeDisplay : public NullDisplay
 {
 public:
@@ -49,6 +50,7 @@
         mir::graphics::EventHandlerRegister& handlers,
         mir::graphics::DisplayConfigurationChangeHandler const& handler) override;
 
+    bool apply_if_configuration_preserves_display_buffers(graphics::DisplayConfiguration const&) override;
     void configure(mir::graphics::DisplayConfiguration const&) override;
 
     void emit_configuration_change_event(
@@ -57,7 +59,7 @@
     void wait_for_configuration_change_handler();
 
 private:
-    std::shared_ptr<mir::graphics::DisplayConfiguration> config;
+    std::shared_ptr<StubDisplayConfig> config;
     std::vector<std::unique_ptr<StubDisplaySyncGroup>> groups;
     Fd const wakeup_trigger;
     std::atomic<bool> handler_called;

=== modified file 'tests/mir_test_doubles/fake_display.cpp'
--- tests/mir_test_doubles/fake_display.cpp	2016-09-06 06:14:17 +0000
+++ tests/mir_test_doubles/fake_display.cpp	2016-11-09 15:08:34 +0000
@@ -29,7 +29,30 @@
 #include <sys/eventfd.h>
 
 namespace mtd = mir::test::doubles;
+namespace mg = mir::graphics;
 
+namespace
+{
+bool compatible(mtd::StubDisplayConfig const& conf1, mtd::StubDisplayConfig const& conf2)
+{
+    auto compatible =
+        conf1.cards == conf2.cards &&
+        conf1.outputs.size() == conf2.outputs.size() &&
+        std::equal(begin(conf1.outputs), end(conf1.outputs),
+                             begin(conf2.outputs),
+                             [] (mg::DisplayConfigurationOutput const& output1, mg::DisplayConfigurationOutput const& output2)
+                             {
+                                 // ignore difference in orientation, scale factor, form factor, subpixel arrangement
+                                 auto clone = output2;
+                                 clone.orientation = output1.orientation;
+                                 clone.subpixel_arrangement = output1.subpixel_arrangement;
+                                 clone.scale = output1.scale;
+                                 clone.form_factor = output1.form_factor;
+                                 return clone == output1;
+                             });
+    return compatible;
+}
+}
 
 mtd::FakeDisplay::FakeDisplay()
     : config{std::make_shared<StubDisplayConfig>()},
@@ -90,6 +113,20 @@
             });
 }
 
+bool mtd::FakeDisplay::apply_if_configuration_preserves_display_buffers(graphics::DisplayConfiguration const& new_config)
+{
+    auto new_configuration = std::make_shared<StubDisplayConfig>(new_config);
+
+    std::lock_guard<decltype(configuration_mutex)> lock{configuration_mutex};
+    if (compatible(*config, *new_configuration))
+    {
+        std::swap(config, new_configuration);
+        return true;
+    }
+
+    return false;
+}
+
 void mtd::FakeDisplay::configure(mir::graphics::DisplayConfiguration const& new_config)
 {
     std::lock_guard<decltype(configuration_mutex)> lock{configuration_mutex};

=== modified file 'tests/mir_test_framework/stubbed_graphics_platform.cpp'
--- tests/mir_test_framework/stubbed_graphics_platform.cpp	2016-11-08 08:15:29 +0000
+++ tests/mir_test_framework/stubbed_graphics_platform.cpp	2016-11-09 15:08:34 +0000
@@ -26,7 +26,7 @@
 
 #include "mir_toolkit/common.h"
 #include "mir/test/doubles/stub_buffer_allocator.h"
-#include "mir/test/doubles/stub_display.h"
+#include "mir/test/doubles/fake_display.h"
 #include "mir/fd.h"
 #include "mir/assert_module_entry_point.h"
 #include "mir/test/pipe.h"
@@ -258,7 +258,7 @@
         return mir::make_module_ptr<WrappingDisplay>(temp);
     }
 
-    return mir::make_module_ptr<mtd::StubDisplay>(display_rects);
+    return mir::make_module_ptr<mtd::FakeDisplay>(display_rects);
 }
 
 namespace