Merge lp:~vanvugt/mir/workaround-1665802 into lp:mir

[Daniel van Vugt]

Provide an environment setting to work around LP: #1665802:

MIR_CLIENT_VSYNC_METHOD={client,server,both}

client = Reliably low latency, but suffers from the Freedreno driver bug.
server = High latency, but works around the bug.
both = Medium latency (varies randomly between client and server levels),
         and also works around the bug.

Unmerged revisions

4078. By Daniel van Vugt on 2017-03-09

Better comments

4077. By Daniel van Vugt on 2017-03-09

Merge latest trunk

4076. By Daniel van Vugt on 2017-03-08

Merge latest parent branch

4075. By Daniel van Vugt on 2017-03-08

Cleaner

4074. By Daniel van Vugt on 2017-03-08

Merge branch 'test-client-BufferStream-vsync-switching' which includes
the latest trunk

4073. By Daniel van Vugt on 2017-03-07

Works now

4072. By Daniel van Vugt on 2017-03-07

Prototyped, not working

4071. By Daniel van Vugt on 2017-03-07

Add a (failing) regression test for the new code to come

=== modified file 'src/client/buffer_stream.cpp'
--- src/client/buffer_stream.cpp	2017-02-15 07:38:33 +0000
+++ src/client/buffer_stream.cpp	2017-03-10 04:06:27 +0000
@@ -290,6 +290,15 @@
      */
     current_swap_interval = interval_config.swap_interval();
 
+    vsync_method = VSyncMethod::client;
+    if (char const *env = getenv("MIR_CLIENT_VSYNC_METHOD"))
+    {
+        if (!strcmp(env, "server"))
+            vsync_method = VSyncMethod::server;
+        else if (!strcmp(env, "both"))
+            vsync_method = VSyncMethod::both;
+    }
+
     if (!protobuf_bs->has_id())
     {
         if (!protobuf_bs->has_error())
@@ -476,9 +485,12 @@
 
 void mcl::BufferStream::swap_buffers_sync()
 {
+    VSyncMethod method;
+
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
-        using_client_side_vsync = true;
+        using_client_side_vsync = vsync_method != VSyncMethod::server;
+        method = vsync_method; // An unlocked copy
     }
 
     /*
@@ -492,7 +504,7 @@
      * interval 0 when using client-side vsync. This guarantees that random
      * scheduling imperfections won't create queuing lag.
      */
-    if (interval_config.swap_interval() != 0)
+    if (interval_config.swap_interval() != 0 && method == VSyncMethod::client)
         set_server_swap_interval(0);
 
     swap_buffers([](){})->wait_for_all();
@@ -502,9 +514,12 @@
         using_client_side_vsync = false;
     }
 
-    int interval = swap_interval();
-    for (int i = 0; i < interval; ++i)
-        wait_for_vsync();
+    if (method != VSyncMethod::server)
+    {
+        int const waits = swap_interval();
+        for (int i = 0; i < waits; ++i)
+            wait_for_vsync();
+    }
 }
 
 void mcl::BufferStream::request_and_wait_for_configure(MirWindowAttrib attrib, int interval)

=== modified file 'src/client/buffer_stream.h'
--- src/client/buffer_stream.h	2017-02-15 07:38:33 +0000
+++ src/client/buffer_stream.h	2017-03-10 04:06:27 +0000
@@ -181,6 +181,7 @@
     std::unordered_set<MirWindow*> users;
     std::shared_ptr<FrameClock> frame_clock;
     mir::time::PosixTimestamp last_vsync;
+    enum class VSyncMethod {client, server, both} vsync_method;
 };
 
 }

=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
--- tests/unit-tests/client/test_client_buffer_stream.cpp	2017-03-08 09:49:38 +0000
+++ tests/unit-tests/client/test_client_buffer_stream.cpp	2017-03-10 04:06:27 +0000
@@ -32,6 +32,7 @@
 #include "mir/test/doubles/mock_protobuf_server.h"
 #include "mir/test/doubles/mock_client_buffer.h"
 #include "mir/test/fake_shared.h"
+#include "mir_test_framework/temporary_environment_value.h"
 
 #include "mir_toolkit/mir_client_library.h"
 
@@ -709,10 +710,58 @@
     EXPECT_FALSE(conf.has_swapinterval());
     bs.set_swap_interval(1);
     bs.swap_buffers_sync();
-    EXPECT_TRUE(conf.has_swapinterval());
+    ASSERT_TRUE(conf.has_swapinterval());
     EXPECT_EQ(0, conf.swapinterval());
 }
 
+TEST_F(ClientBufferStream, can_force_vsync_method_both)
+{
+    mir_test_framework::TemporaryEnvironmentValue
+        method("MIR_CLIENT_VSYNC_METHOD", "both");
+    mcl::BufferStream bs{
+        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
+        map, factory,
+        response, perf_report, "", size, nbuffers};
+    service_requests_for(mock_protobuf_server.alloc_count);
+
+    EXPECT_CALL(mock_protobuf_server, configure_buffer_stream(_,_,_))
+        .Times(0);
+    bs.set_swap_interval(1);
+    bs.swap_buffers_sync();
+    /*
+     * TODO: Should also somehow test that the sleep happens here. Although
+     *       that's presently pointless because a non-adopted BufferStream
+     *       with no MirWindow won't have a frame clock and so won't sleep.
+     *       We'll need to either change adopted_by to take the frame clock
+     *       pointer (or some other interface) or mock MirWindow for it.
+     */
+}
+
+TEST_F(ClientBufferStream, can_force_vsync_method_server)
+{
+    mir_test_framework::TemporaryEnvironmentValue
+        method("MIR_CLIENT_VSYNC_METHOD", "server");
+    mcl::BufferStream bs{
+        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
+        map, factory,
+        response, perf_report, "", size, nbuffers};
+    service_requests_for(mock_protobuf_server.alloc_count);
+
+    EXPECT_CALL(mock_protobuf_server, configure_buffer_stream(_,_,_))
+        .Times(0);
+    bs.set_swap_interval(1);
+    bs.swap_buffers_sync();
+    /*
+     * TODO: Should also somehow test that NO sleep happens here. Although
+     *       that's presently pointless because a non-adopted BufferStream
+     *       with no MirWindow won't have a frame clock and so won't sleep.
+     *       We'll need to either change adopted_by to take the frame clock
+     *       pointer (or some other interface) or mock MirWindow for it.
+     */
+}
+
 TEST_F(ClientBufferStream, sets_swap_interval_requested)
 {
     mcl::BufferStream bs{