Merge lp:~afrantzis/mir/fix-render-surfaces-vt-switch into lp:~mir-team/mir/trunk

[Alexandros Frantzis]

compositor: Manage the rendering target properly in the compositing threads

compositor: Manage the rendering target properly in the compositing threads

This fixes an issue with VT switching while running render_surfaces.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:783
http://jenkins.qa.ubuntu.com/job/mir-ci/845/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1068
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/953
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/83
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/83/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

What was the original issue? I can't seem to reproduce any problems switching VTs with render_surfaces. Other than the predictable issue that you need to be root.

review: Needs Information

[Alexandros Frantzis (afrantzis)]

When switching back from a VT switch, we create new compositing threads, which don't have a current GL context. render_surfaces assumes that a context is current and calls GL functions when rendering, causing Mesa to crash on my system. You could argue that Mesa shouldn't crash in the first place, so this is a workaround for this, but at the same time this is a better, more symmetrical way to handle the context, i.e., the compositing thread makes a context current when starting and releases that context when it finishes.

[Daniel van Vugt (vanvugt)]

Makes sense

review: Approve

[Thomas Voß (thomas-voss)]

LGTM

review: Approve

=== modified file 'src/server/compositor/multi_threaded_compositor.cpp'
--- src/server/compositor/multi_threaded_compositor.cpp	2013-04-24 05:22:20 +0000
+++ src/server/compositor/multi_threaded_compositor.cpp	2013-06-27 09:52:32 +0000
@@ -33,6 +33,24 @@
 namespace compositor
 {
 
+class CurrentRenderingTarget
+{
+public:
+    CurrentRenderingTarget(mg::DisplayBuffer& buffer)
+        : buffer(buffer)
+    {
+        buffer.make_current();
+    }
+
+    ~CurrentRenderingTarget()
+    {
+        buffer.release_current();
+    }
+
+private:
+    mg::DisplayBuffer& buffer;
+};
+
 class CompositingFunctor
 {
 public:
@@ -49,6 +67,12 @@
     {
         std::unique_lock<std::mutex> lock{run_mutex};
 
+        /*
+         * Make the buffer the current rendering target, and release
+         * it when the thread is finished.
+         */
+        CurrentRenderingTarget target{buffer};
+
         while (running)
         {
             /* Wait until compositing has been scheduled or we are stopped */
@@ -68,12 +92,6 @@
                 lock.lock();
             }
         }
-
-        /*
-         * Release the display buffer from this thread, so that it can be
-         * made current in another thread.
-         */
-        buffer.release_current();
     }
 
     void schedule_compositing()

=== modified file 'tests/unit-tests/compositor/test_multi_threaded_compositor.cpp'
--- tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2013-06-12 10:27:50 +0000
+++ tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2013-06-27 09:52:32 +0000
@@ -355,7 +355,7 @@
     compositor.stop();
 }
 
-TEST(MultiThreadedCompositor, releases_display_buffer_context_when_stopping)
+TEST(MultiThreadedCompositor, makes_and_releases_display_buffer_current_target)
 {
     using namespace testing;
 
@@ -368,6 +368,7 @@
 
     display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)
     {
+        EXPECT_CALL(mock_buf, make_current()).Times(1);
         EXPECT_CALL(mock_buf, release_current()).Times(1);
     });