Merge lp:~vanvugt/mir/fix-motion-bugs-0.7 into lp:mir/0.7

[Daniel van Vugt]

Backport improvements to touch (reduced latency and increased smoothness).

Tested with krillin image 67. Seems to work well for Unity8 and apps.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1919
http://jenkins.qa.ubuntu.com/job/mir-0.7-ci/3/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-0.7-utopic-amd64-ci/3/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1924
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1931
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1884
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/821
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/821/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2902
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13791

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

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Oh, we just killed the build host with the compiler. Retry...
virtual memory exhausted: Cannot allocate memory

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1920
http://jenkins.qa.ubuntu.com/job/mir-0.7-ci/4/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-0.7-utopic-amd64-ci/4
        deb: http://jenkins.qa.ubuntu.com/job/mir-0.7-utopic-amd64-ci/4/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1928
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1935
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1887
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/824
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/824/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2904
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13797

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Kevin DuBois (kdub)]

lgtm

review: Approve

[Cemil Azizoglu (cemil-azizoglu)]

We'll be releasing 0.8 soon to rtm and ubuntu. So this is really unnecessary. It doesn't hurt, though, if you want it on the 0.7 branch. (Then again, we won't be landing 0.7 on rtm anymore, unless you wanna do that yourself).

review: Abstain

[Daniel van Vugt (vanvugt)]

I think this is still important. 0.8 has no chance of getting into RTM updates, whereas 0.7 (included this proposal) does.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-09-16 08:33:51 +0000
+++ CMakeLists.txt	2014-09-26 09:05:45 +0000
@@ -28,7 +28,7 @@
 
 set(MIR_VERSION_MAJOR 0)
 set(MIR_VERSION_MINOR 7) # This should change at least with every MIRSERVER_ABI
-set(MIR_VERSION_PATCH 3)
+set(MIR_VERSION_PATCH 4)
 
 set(MIR_VERSION ${MIR_VERSION_MAJOR}.${MIR_VERSION_MINOR}.${MIR_VERSION_PATCH})
 
 
=== modified file 'debian/changelog'
--- debian/changelog	2014-09-18 14:54:51 +0000
+++ debian/changelog	2014-09-26 09:05:45 +0000
@@ -1,3 +1,14 @@
+mir (0.7.4-0ubuntu1) UNRELEASED; urgency=medium
+
+  * New upstream release 0.7.4 (https://launchpad.net/mir/+milestone/0.7.4)
+    - Bug fixes
+      . Fix jerky/stuttering event delivery that could happen inside
+        nested servers sometimes (LP: #1372300)
+    - Enhancement
+      . Reduced touch input lag slightly for 60Hz displays.
+
+ -- Daniel van Vugt <[e-mail]>  Thu, 25 Sep 2014 16:18:10 +0800
+
 mir (0.7.3+14.10.20140918.1-0ubuntu1) utopic; urgency=medium
 
   [ Andreas Pokorny ]
 
=== modified file 'src/shared/input/android/android_input_receiver.cpp'
--- src/shared/input/android/android_input_receiver.cpp	2014-08-29 13:25:18 +0000
+++ src/shared/input/android/android_input_receiver.cpp	2014-09-26 09:05:45 +0000
@@ -90,18 +90,26 @@
      *   consumeBatches = true, so as to ensure the "cooked" event rate that
      *      clients experience is at least the minimum of event_rate_hz
      *      and the raw device event rate.
-     *   frame_time = A regular interval of 60Hz. This provides a virtual frame
+     *   frame_time = A regular interval. This provides a virtual frame
      *      interval during which InputConsumer will collect raw events,
-     *      resample them and emit a "cooked" event back to us at least every
+     *      resample them and emit a "cooked" event back to us at roughly every
      *      60th of a second. "cooked" events are both smoothed and
      *      extrapolated/predicted into the future (for tool=finger) giving the
      *      appearance of lower latency. Getting a real frame time from the
      *      graphics logic (which is messy) does not appear to be necessary to
      *      gain significant benefit.
+     *
+     * Note event_rate_hz is only 55Hz. This allows rendering to catch up and
+     * overtake the event rate every ~12th frame (200ms) on a 60Hz display.
+     * Thus on every 12th+1 frame, there will be zero buffer lag in responding
+     * to the cooked input event we have given the client.
+     * This phase control is useful as it eliminates the one frame of lag you
+     * would otherwise never catch up to if the event rate was exactly the same
+     * as the display refresh rate.
      */
 
     nsecs_t const now = android_clock(SYSTEM_TIME_MONOTONIC);
-    int const event_rate_hz = 60;
+    int const event_rate_hz = 55;
     nsecs_t const one_frame = 1000000000ULL / event_rate_hz;
     nsecs_t frame_time = (now / one_frame) * one_frame;
 
@@ -141,15 +149,11 @@
     }
     else if (input_consumer->hasPendingBatch())
     {
-        /*
-         * A batch is pending and must be completed or else the client could
-         * be starved of events. But don't hurry. A continuous motion gesture
-         * will wake us up much sooner than 50ms. This timeout is only reached
-         * in the case that motion has ended (fingers lifted).
-         */
-        std::chrono::milliseconds const motion_idle_timeout(50);
-        if (timeout.count() < 0 || timeout > motion_idle_timeout)
-            reduced_timeout = motion_idle_timeout;
+        // When in constant motion we will usually "hasPendingBatch".
+        // But the batch won't get flushed until the next frame interval,
+        // so be sure to use a non-zero sleep time to avoid spinning the CPU
+        // for the whole interval...
+        reduced_timeout = std::chrono::milliseconds(1);
     }
 
     auto result = looper->pollOnce(reduced_timeout.count());
 
=== modified file 'tests/unit-tests/client/input/test_android_input_receiver.cpp'
--- tests/unit-tests/client/input/test_android_input_receiver.cpp	2014-08-29 13:25:18 +0000
+++ tests/unit-tests/client/input/test_android_input_receiver.cpp	2014-09-26 09:05:45 +0000
@@ -204,6 +204,54 @@
     EXPECT_FALSE(receiver.next_event(std::chrono::milliseconds(1), ev)); // Minimal timeout needed for valgrind
 }
 
+TEST_F(AndroidInputReceiverSetup, slow_raw_input_doesnt_cause_frameskipping)
+{   // Regression test for LP: #1372300
+    using namespace testing;
+    using namespace std::chrono;
+
+    nsecs_t t = 0;
+
+    mircva::InputReceiver receiver(
+        client_fd, std::make_shared<mircv::NullInputReceiverReport>(),
+        [&t](int) { return t; }
+        );
+    TestingInputProducer producer(server_fd);
+
+    nsecs_t const one_millisecond = 1000000ULL;
+    nsecs_t const one_second = 1000 * one_millisecond;
+    nsecs_t const one_frame = one_second / 60;
+
+    MirEvent ev;
+
+    producer.produce_a_motion_event(123, 456, t);
+    producer.produce_a_key_event();
+    flush_channels();
+
+    std::chrono::milliseconds const max_timeout(1000);
+
+    // Key events don't get resampled. Will be reported first.
+    ASSERT_TRUE(receiver.next_event(max_timeout, ev));
+    ASSERT_EQ(mir_event_type_key, ev.type);
+
+    // The motion is still too new. Won't be reported yet, but is batched.
+    auto start = high_resolution_clock::now();
+    ASSERT_FALSE(receiver.next_event(max_timeout, ev));
+    auto end = high_resolution_clock::now();
+    auto duration = end - start;
+
+    // Verify we timed out in under a frame (LP: #1372300)
+    // Sadly using real time as droidinput::Looper doesn't use a mocked clock.
+    ASSERT_LT(duration_cast<nanoseconds>(duration).count(), one_frame);
+
+    // Verify we don't use all the CPU by not sleeping (LP: #1373809)
+    EXPECT_GT(duration_cast<nanoseconds>(duration).count(), one_millisecond);
+
+    // But later in a frame or so, the motion will be reported:
+    t += 2 * one_frame;  // Account for the new slower 55Hz event rate
+    ASSERT_TRUE(receiver.next_event(max_timeout, ev));
+    ASSERT_EQ(mir_event_type_motion, ev.type);
+}
+
 TEST_F(AndroidInputReceiverSetup, rendering_does_not_lag_behind_input)
 {
     using namespace testing;
@@ -252,3 +300,62 @@
     EXPECT_THAT(average_lag_milliseconds, Le(1));
 }
 
+TEST_F(AndroidInputReceiverSetup, input_comes_in_phase_with_rendering)
+{
+    using namespace testing;
+
+    nsecs_t t = 0;
+
+    mircva::InputReceiver receiver(
+        client_fd, std::make_shared<mircv::NullInputReceiverReport>(),
+        [&t](int) { return t; }
+        );
+    TestingInputProducer producer(server_fd);
+
+    nsecs_t const one_millisecond = 1000000ULL;
+    nsecs_t const one_second = 1000 * one_millisecond;
+    nsecs_t const device_sample_interval = one_second / 125;
+    nsecs_t const frame_interval = one_second / 60;
+    nsecs_t const gesture_duration = 10 * one_second;
+
+    nsecs_t last_produced = 0, last_consumed = 0, last_rendered = 0;
+    nsecs_t last_in_phase = 0;
+
+    for (t = 0; t < gesture_duration; t += one_millisecond)
+    {
+        if (!t || t >= (last_produced + device_sample_interval))
+        {
+            last_produced = t;
+            float a = t * M_PI / 1000000.0f;
+            float x = 500.0f * sinf(a);
+            float y = 1000.0f * cosf(a);
+            producer.produce_a_motion_event(x, y, t);
+            flush_channels();
+        }
+
+        MirEvent ev;
+        if (receiver.next_event(std::chrono::milliseconds(0), ev))
+            last_consumed = t;
+
+        if (t >= (last_rendered + frame_interval))
+        {
+            last_rendered = t;
+
+            if (last_consumed)
+            {
+                auto lag = last_rendered - last_consumed;
+
+                // How often does vsync drift in phase (very close) with the
+                // time that we emitted/consumed input events?
+                if (lag < 4*one_millisecond)
+                    last_in_phase = t;
+                last_consumed = 0;
+            }
+        }
+
+        // Verify input and vsync come into phase at least a few times every
+        // second (if not always). This ensure visible lag is minimized.
+        ASSERT_GE(250*one_millisecond, (t - last_in_phase));
+    }
+}
+