Merge lp:~vanvugt/unity8/more-smooth-less-lag into lp:unity8

Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel d'Andrada
Approved revision: 1989
Merged at revision: 2010
Proposed branch: lp:~vanvugt/unity8/more-smooth-less-lag
Merge into: lp:unity8
Diff against target: 20 lines (+10/-0)
1 file modified
data/unity8.conf (+10/-0)
To merge this branch: bzr merge lp:~vanvugt/unity8/more-smooth-less-lag
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Gerry Boland (community) Needs Information
kevin gunn (community) Needs Information
Daniel d'Andrada (community) Approve
Michał Sawicz conceptual Approve
Michael Zanetti (community) Needs Information
Review via email: mp+272888@code.launchpad.net

Commit message

Disable Qt's stuttering 'touch compression' to fix scrolling smoothness

Fixes (LP: #1486341). As a bonus, this eliminates most of the
lag seen in the indicator panel pull-down (LP: #1488327) and also
reduces lag seen in apps.

Description of the change

Note that the variable needs to be set from outside of the unity8 process like this, because Qt reads it very early during global static construction. So setting it from within unity8/qtmir/libmir* would not work.

To post a comment you must log in.
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Would like to hear Daniel's opinion on this. If he's ok with it, it looks good to me.

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1989
http://jenkins.qa.ubuntu.com/job/unity8-ci/6387/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4429
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/765
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1099
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-wily/417
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/994
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/995
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/626
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/627
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3603
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4426
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4426/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23791
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/460
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/765
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/765/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23792

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

It seems weird to me that we'd do that. Is it just the Qt feature doesn't make sense in our scenario or that it's just implemented badly? If that's the case - shouldn't we fix it in upstream Qt instead of disabling it, or at least file a bug?

review: Needs Information
Revision history for this message
kevin gunn (kgunn72) wrote :

first, do we have numbers to back up in both cases of app vertical scroll & indicator panel pull down? we need those

second, have we looked at all scenarios - app spread, dash swipe, scope scroll, browser specifically? we should

also, i wouldn't be opposed to disabling this while we also fix upstream. _if_ the data can be provided for all scenarios of concern.

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Upstream it's possibly not broken because no one else uses nesting. It's more broken for the Mir nested architecture for three reasons:

1. QML touch compression doesn't support nesting (hence stuttering). If you remove it from either client layer the stuttering goes away (bug 1486341).

2. QML touch compression is laggy by design in that it withholds touch events for a full frame. And this lag seemingly happens twice (once in the app client, once in the unity8 client). So it's at least twice as laggy as the Mir input resampling algorithm when nested.

3. QML touch compression (when it does finally) emits events at too high a rate for Mir's long buffer queues. So it fills up the buffer queues making the nested buffer lag 4x16.6ms = 66ms. By comparison, Mir's touch resampling employs a trick to hide the long nested buffer queue lag. In Mir we emit events only at 59Hz so that the queue drains faster than it fills, and so you don't suffer from the full length of the buffer queue. It's more like half or less.

So Mir's built-in touch resampling (which is already enabled) works much better. It's designed to deal with nesting (without stuttering), and also has lower latency.

The QML touch compression is probably not broken for upstream. Upstream no one is trying to use nesting (although this might be evidence: https://bugreports.qt.io/browse/QTBUG-40167).

In the very least you can see some beneficial numbers where the stuttering was first described in bug 1486341.

You can also see the reduced lag easily with your own eyes:

Manual test case #1:
Drag the indicator panel down and up quickly. The gap between your finger and the edge of the panel is much shorter.

Manual test case #2:
Run Mir's latency test client (requires Mir 0.16) in Unity8:
  $ mir_demo_client_target -- --desktop_file_hint=unity8
And now draw consistent circles. The gap between your finger and the target is much shorter.

I've been talking up the benefits of Mir's input resampling algorithm for a while now (it landed on 2014-08-29). But I guess the message still hasn't sunk in everywhere yet.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Correction: Upstream the QML touch compression isn't "broken" but it is still inferior to Mir's touch resampling even when not nested (hence the indicator panel which is not nested), because it generates higher latency with triple buffers. QML touch compression fills the queue on each server and so has 2 frames latency at each level. Mir's touch resampling however is crafted to not fill the queues so has typically only 0-1 frame latency at each level.

If your architecture is not nested _and_ you only ever use double buffering instead of triple then the QML touch compression would theoretically be able to perform as well as Mir's touch resampling. So yeah it's great for other projects but not Ubuntu Touch any time soon.

Revision history for this message
Michał Sawicz (saviq) wrote :

Thanks for the clarification.

review: Approve (conceptual)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

I tried it on my Nexus 7 and honestly couldn't spot a difference. Also had my Nexus 4 next to it (without this patch) to compare.

Anyway, if Mir already does the touch resampling/compression, I agree there's no benefit in unity8 doing it as well at Qt level (in principle, at least).

review: Approve
Revision history for this message
kevin gunn (kgunn72) wrote :

i would still like numbers

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

dandrader - When looking for latency changes you have to be very careful and use the same device with the same software. No changes other than the change you are testing.

Also, it requires practice. If you use Ubuntu Touch a lot then unfortunately your brain is tuned to expect a lot of lag. So I guess the best suggestion I have is spend more time using other touch operating systems to get out of that mindset.

1990. By Daniel van Vugt

Merge latest trunk

Revision history for this message
Gerry Boland (gerboland) wrote :

I have to agree with Kevin, I'd like numbers to back up this assertion.
https://code.launchpad.net/~unity-team/qtmir/touch_tracing/+merge/271655
adds tooling to gather those numbers, you might find it useful.

I agree with you in principle that having 2 touch compression systems working in series is not ideal.

MIR_SERVER_NBUFFERS=2 forces double-buffering?

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

It makes me sad our core developers are not practised at seeing significant differences in lag. But OK...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Numbers!

This is actually the first time I've ever tried the slow motion feature on my Canon G16. Units are frames 'f' of lag captured at 240FPS (four camera frames per phone frame on mako):

Indicator bar pulldown: before=36f(150ms), after=22f(91ms) -> 59ms lag reduction
mir_demo_client_target: before=19f(79ms), after=11f(45ms) -> 34ms lag reduction

Weird the indicator bar is slower than the client. I guess bug 1488327 still requires some work after this. Also interesting the client gets exactly the improvement we would expect in theory: two triple-buffer queues each with two frames lag, reduced to one frame lag in each queue equals a reduction of ~2x16.6ms=33.2ms

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

And yes MIR_SERVER_NBUFFERS=2 forces double buffering which generally halves lag. However we don't use that because on Ubuntu Touch it tends to stall the rendering pipeline so much that droids clock down to lower power modes and suffer. However that only happens with slow rendering clients (like unity8-dash) when nested on an otherwise idle device. So given any of:
  (a) a desktop/laptop; or
  (b) a much faster client than unity8-dash; or
  (c) no nesting; or
  (d) a spinning background process keeping the CPU out of low power mode;
you can then use MIR_SERVER_NBUFFERS=2 and get smooth graphics with half the lag.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Attached a bunch of slow-mo GIFs for your viewing pleasure -> bug 1488327

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Actually the extra indicator pulldown lag might be explained by this 50ms timer:
   /usr/share/unity8/Panel/PanelVelocityCalculator.qml: interval: 50
which is missing a 'triggeredOnStart' property.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Oh, ignore the above comment. It appears to not need triggeredOnStart.

Revision history for this message
Gerry Boland (gerboland) wrote :

> It makes me sad our core developers are not practised at seeing significant
> differences in lag. But OK...
I do not like to rely exclusively on fault-prone instrumentation, like eyes and the visual cortex. Good numbers do not lie, and I want to encourage the practice of using them to back up performance improvement claims.

Revision history for this message
kevin gunn (kgunn72) wrote :

> It makes me sad our core developers are not practised at seeing significant
> differences in lag. But OK...

not only uncalled for, but it should be a matter of course we provide numbers for claims. Helps guard ourselves from "figures never lie, but liars always figure"
Thanks for the numbers

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I completely agree that more reliable measurements are required. Being able to see four-frames-per-frame (and even observe the LCD panel latency) is pretty good, so long as you can count and repeat your measurements. However something automated that we can trust would also be good.

Earlier this year kdub made some progress on that inroducing the ClientLatency acceptance test. However that acceptance test was actually buggy and returning incorrect results until very recently. Now it's fixed (in Mir 0.16.0) we have the ability to build on it further...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Gerry,

I would agree with your logic, however the last couple of years have taught me that the human brain can and does observe latencies at a much higher resolution than a 60Hz display. It takes practice, but eventually the differences become obvious.

So why am I sceptical of measurements from automation? Because we (the Mir team) have regularly messed up our automation of performance measurement. First was when racarr proposed changes with tests that reported lower latency but using your own eyes you could see the latency was actually higher. And second more recently was the ClientLatency acceptance test, which was taken as absolute truth at the time, only to later be shown as completely incorrect in the results it reports (fixed in Mir 0.16.0).

So our history so far has made me trust my eyes more than our automation.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1990
http://jenkins.qa.ubuntu.com/job/unity8-ci/6492/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4712
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/874
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1204
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-wily/520
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1099
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1100
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/731
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/732
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3805
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4709
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4709/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24365
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/518
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/874
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/874/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24367

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

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'data/unity8.conf'
--- data/unity8.conf 2015-09-01 12:41:57 +0000
+++ data/unity8.conf 2015-10-23 07:03:07 +0000
@@ -46,6 +46,16 @@
46 initctl set-env --global MIR_SERVER_PROMPT_FILE=146 initctl set-env --global MIR_SERVER_PROMPT_FILE=1
4747
48 initctl emit --no-wait indicator-services-start48 initctl emit --no-wait indicator-services-start
49
50 # Disable Qt's stuttering 'touch compression' to fix scrolling smoothness
51 # issues (LP: #1486341). As a bonus, this eliminates most of the
52 # lag seen in the indicator panel pull-down (LP: #1488327) and also
53 # reduces lag seen in apps:
54 initctl set-env --global QML_NO_TOUCH_COMPRESSION=1
55
56 # For twice the fun and half the latency, try this (Warning: not all
57 # devices are fast enough to keep up smoothly yet)...
58 # initctl set-env MIR_SERVER_NBUFFERS=2
49end script59end script
5060
51exec ${BINARY:-unity8} $ARGS61exec ${BINARY:-unity8} $ARGS

Subscribers

People subscribed via source and target branches