Merge lp:~alan-griffiths/mir/workaround-1464690 into lp:mir

[Alan Griffiths]

examples: don't block the window manager to paint titlebars

examples: don't block the window manager to paint titlebars

This avoids the deadlock noted in lp:1464690.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2662
http://jenkins.qa.ubuntu.com/job/mir-ci/4098/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2845
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/358
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/254
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/254/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2793
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2793/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5625
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21191

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

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

LGTM

review: Approve

[Kevin DuBois (kdub)]

There's still a bit of a structural problem (lp: #1395421 ), that oftentimes the compositor will drive the ipc, and now, it will sometimes paint on behalf of the window manager. I suppose this is better than a deadlock though, so lgtm too

review: Approve

=== modified file 'examples/server_example_canonical_window_manager.cpp'
--- examples/server_example_canonical_window_manager.cpp	2015-06-08 14:41:13 +0000
+++ examples/server_example_canonical_window_manager.cpp	2015-06-15 13:53:13 +0000
@@ -72,7 +72,8 @@
     width_inc{params.width_inc},
     height_inc{params.height_inc},
     min_aspect{params.min_aspect},
-    max_aspect{params.max_aspect}
+    max_aspect{params.max_aspect},
+    buffer_stream(surface->primary_buffer_stream())
 {
 }
 
@@ -253,49 +254,27 @@
 
 //TODO: provide an easier way for the server to write to a surface!
 //TODO: this is painful to use mg::Buffer::write()
-namespace
-{
-void swap_buffers(
-    std::shared_ptr<ms::Surface> const& surface,
-    mir::graphics::Buffer*& surface_buffer)
-{
-    std::mutex mut;
-    std::condition_variable cv;
-
-    auto const callback = [&](mir::graphics::Buffer* buffer)
+void mir::examples::CanonicalSurfaceInfoCopy::paint_titlebar(int intensity)
+{
+    if (auto const buf = this->buffer.load())
+    {
+        auto const format = buffer_stream->pixel_format();
+        auto const sz = buf->size().height.as_int() *
+            buf->size().width.as_int() * MIR_BYTES_PER_PIXEL(format);
+
+        std::vector<unsigned char> pixels(sz, intensity);
+        buf->write(pixels.data(), sz);
+    }
+
+    auto const callback = [this, intensity](mir::graphics::Buffer* new_buffer)
         {
-            std::unique_lock<decltype(mut)> lk(mut);
-            surface_buffer = buffer;
-            cv.notify_one();
+            bool const first_time = !buffer;
+            buffer.store(new_buffer);
+            if (first_time)
+                paint_titlebar(intensity);
         };
 
-    auto const old_buffer = surface_buffer;
-
-    surface->primary_buffer_stream()->swap_buffers(surface_buffer, callback);
-
-    std::unique_lock<decltype(mut)> lk(mut);
-    cv.wait(lk, [&]{return old_buffer != surface_buffer;});
-}
-
-void paint_titlebar(
-    std::shared_ptr<ms::Surface> const& titlebar,
-    mir::examples::CanonicalSurfaceInfoCopy& titlebar_info,
-    int intensity)
-{
-    auto stream = titlebar->primary_buffer_stream();
-    auto const format = stream->pixel_format();
-
-    if (!titlebar_info.buffer)
-        swap_buffers(titlebar, titlebar_info.buffer);
-
-    auto const sz = titlebar_info.buffer->size().height.as_int() *
-                    titlebar_info.buffer->size().width.as_int() * MIR_BYTES_PER_PIXEL(format);
-
-    std::vector<unsigned char> pixels(sz, intensity);
-    titlebar_info.buffer->write(pixels.data(), sz);
-
-    swap_buffers(titlebar, titlebar_info.buffer);
-}
+    buffer_stream->swap_buffers(buffer, callback);
 }
 
 void me::CanonicalWindowManagerPolicyCopy::generate_decorations_for(
@@ -708,7 +687,7 @@
         {
             if (auto const titlebar = tools->info_for(active_surface).titlebar)
             {
-                paint_titlebar(titlebar, tools->info_for(titlebar), 0x3F);
+                tools->info_for(titlebar).paint_titlebar(0x3F);
             }
         }
 
@@ -734,12 +713,12 @@
         {
             if (auto const titlebar = tools->info_for(active_surface).titlebar)
             {
-                paint_titlebar(titlebar, tools->info_for(titlebar), 0x3F);
+                tools->info_for(titlebar).paint_titlebar(0x3F);
             }
         }
         if (auto const titlebar = tools->info_for(surface).titlebar)
         {
-            paint_titlebar(titlebar, tools->info_for(titlebar), 0xFF);
+            tools->info_for(titlebar).paint_titlebar(0xFF);
         }
         tools->set_focus_to(info_for.session.lock(), surface);
         raise_tree(surface);

=== modified file 'examples/server_example_canonical_window_manager.h'
--- examples/server_example_canonical_window_manager.h	2015-05-27 11:37:38 +0000
+++ examples/server_example_canonical_window_manager.h	2015-06-15 13:53:13 +0000
@@ -23,6 +23,8 @@
 
 #include "mir/geometry/displacement.h"
 
+#include <atomic>
+
 ///\example server_example_canonical_window_manager.h
 // Based on "Mir and Unity: Surfaces, input, and displays (v0.3)"
 
@@ -59,7 +61,20 @@
     mir::optional_value<shell::SurfaceAspectRatio> min_aspect;
     mir::optional_value<shell::SurfaceAspectRatio> max_aspect;
 
-    graphics::Buffer* buffer{nullptr};
+    void paint_titlebar(int intensity);
+
+private:
+    std::shared_ptr<frontend::BufferStream> const buffer_stream;
+
+    // Add copy-ctor to std::atomic
+    struct AtomicBufferPtr : std::atomic<graphics::Buffer*>
+    {
+        using std::atomic<graphics::Buffer*>::atomic;
+
+        AtomicBufferPtr(AtomicBufferPtr const& that) :
+            std::atomic<graphics::Buffer*>{that.load()} {}
+    };
+    AtomicBufferPtr buffer{nullptr};
 };
 
 // standard window management algorithm: