Merge lp:~vanvugt/mir/59Hz into lp:mir

[Daniel van Vugt]

Bump the default input resampling rate from 55Hz to 59Hz. This provides
a visibly smoother experience without any increase in latency.
(LP: #1436192)

An input rate even slightly over the display rate will be visibly laggy,
so given the vast majority of displays are ~60Hz and commonly 59.94Hz,
this seems like the best choice. Still not optimal for krillin's 67Hz
though.

Recent testing has shown input resampling to be dramatically more valuable than expected. So rather than making it go away completely, just make it work better.

I'd still like to add a client function so people can choose to turn it off (or adjust).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2423
http://jenkins.qa.ubuntu.com/job/mir-ci/3357/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1811
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1810
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1761
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1354
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1354/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1761
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1761/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4723
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19134

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

What's the best way to test this? (What scenarios show a convincing improvement?)

It seems like this ought to be configurable (so it can be set for a device). Or even selected according to the current display mode?

review: Needs Information

[Kevin DuBois (kdub)]

> It seems like this ought to be configurable (so it can be set for a device).
> Or even selected according to the current display mode?

The client does have access to the reported vsync rate... "Slightly less than vsync" seems like a more universal tuning than hard-coded to 59hz. I guess needs info

review: Needs Information

[Daniel van Vugt (vanvugt)]

Test scenarios:
1. Drag the square around in mir_demo_client_eglsquare.
2. Drag a window around in a _nested_ mir_proving_server, using Alt+drag.

It is semi-configurable already by environment: MIR_CLIENT_INPUT_RATE=Hz (or 0 to disable)
If we don't get around to making autodetection work well then that could be customized per-device.

It's worth noting however that if an app is capable (like mir_demo_client_fingerpaint) then it should have the option of disabling input resampling completely by a client function in future. Apps and toolkits that can decouple the input rate from the render rate themselves are visibly nicer to use. Like: env MIR_CLIENT_INPUT_RATE=0 mir_demo_client_fingerpaint

[Alan Griffiths (alan-griffiths)]

Okay but I still think there could be a friendlier way to configure this.

review: Approve

[Kevin DuBois (kdub)]

I believe that there's improvement in the above scenarios, but its tricky to prove whether or not this tuning is something that all the clients would appreciate without some data. I appreciate that we're lacking a quick way to gather that info, so it feels more like hopeful tuning, as opposed to tuning based on some data. I guess I buy that this would be better in theory, so I guess its an approve, with a side-point of "We'd be 'safer' to have data when tuning hardcoded numbers"

review: Approve

=== modified file 'src/client/input/android/android_input_receiver.cpp'
--- src/client/input/android/android_input_receiver.cpp	2015-03-24 16:02:48 +0000
+++ src/client/input/android/android_input_receiver.cpp	2015-03-25 07:30:38 +0000
@@ -52,7 +52,11 @@
     input_consumer(std::make_shared<droidinput::InputConsumer>(input_channel)),
     android_clock(clock)
 {
-    event_rate_hz = 55;
+    /*
+     * 59Hz by default. This ensures the input rate never gets ahead of the
+     * typical display rate, which would be seen as visible lag.
+     */
+    event_rate_hz = 59;
     auto env = getenv("MIR_CLIENT_INPUT_RATE");
     if (env != NULL)
         event_rate_hz = atoi(env);
@@ -160,14 +164,6 @@
      *      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.
      */
 
     auto frame_time = std::chrono::nanoseconds(-1);

=== modified file 'tests/unit-tests/client/input/test_android_input_receiver.cpp'
--- tests/unit-tests/client/input/test_android_input_receiver.cpp	2015-03-24 16:02:48 +0000
+++ tests/unit-tests/client/input/test_android_input_receiver.cpp	2015-03-25 07:30:38 +0000
@@ -277,7 +277,7 @@
     EXPECT_GT(duration, 1ms);
 
     // But later in a frame or so, the motion will be reported:
-    t += 2 * one_frame;  // Account for the new slower 55Hz event rate
+    t += 2 * one_frame;  // Account for the slower 59Hz event rate
     EXPECT_TRUE(mt::fd_becomes_readable(receiver.watch_fd(), next_event_timeout));
     receiver.dispatch(md::FdEvent::readable);