Mir

Merge lp://staging/~vanvugt/mir/mesa-double into lp://staging/mir

Proposed by Daniel van Vugt
Status: Merged
Approved by: Kevin DuBois
Approved revision: no longer in the source branch.
Merged at revision: 2218
Proposed branch: lp://staging/~vanvugt/mir/mesa-double
Merge into: lp://staging/mir
Diff against target: 126 lines (+57/-8)
3 files modified
src/platforms/mesa/display_buffer.cpp (+25/-0)
tests/unit-tests/graphics/mesa/test_display.cpp (+2/-1)
tests/unit-tests/graphics/mesa/test_display_buffer.cpp (+30/-7)
To merge this branch: bzr merge lp://staging/~vanvugt/mir/mesa-double
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Robert Carr (community) Approve
Alexandros Frantzis (community) Approve
Alan Griffiths Approve
Cemil Azizoglu Pending
Review via email: mp+245723@code.staging.launchpad.net

Commit message

mesa::DisplayBuffer: Schedule page flips in a double buffered pattern
instead of triple when it's safe to do so (not clone mode). This
noticeably reduces visible lag. (LP: #1350725)

Clone mode still needs triple buffering because you can't tell two or
more separate monitors to vblank at the same time. You just have to
accept that waiting for multiple vblanks will often take an extra
frame's time.

The more common cases of single or separate outputs however only need
to deal with one vsync signal per frame so can be optimized into
double buffering. And this feels significantly less laggy when dragging
windows around.

This is an improvement on the present behaviour introduced in r1380.

Description of the change

Q: What difference does one frame make?
A: A lot. It's the difference between the mouse pointer staying on your titlebar during window drags, and not.

Q: Is this really a priority?
A: Short term, no. But I've been sitting on this branch for almost two months and want to get it out the way. Longer term: Yes, less lag is important for Unity and everyone.

* Request for manual testing *

I'm satisfied performance is now good enough (since swap-then-flip) that we can afford to go back to a double buffered compositor. However beware of:
  - Intel bug 1377872
  - General performance bug 1395421
which could both be more noticeable as a result of this.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

That's a new one...

[ 85%] Build timed out (after 240 minutes). Marking the build as failed.
java.lang.InterruptedException
Build step 'Debian PBuilder NG' marked build as failure

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I'm not sure how we should be measuring performance around this area. But OK

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

There's an obvious visual difference when dragging things around with this branch. And also tests added for the theory being correct. Not sure what else we can do in terms of measurement.

You do see a very slight amount of lag remaining even with this branch (mouse pointer ahead of the shell rendering). But that's expected. If and when we get to a zero-lag prototype, the visual proof will then be conclusive if the mouse pointer sticks perfectly.

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

My perception is that dragging a window around is more snappy with this change, although without a proper blind test I may be just be unconsciously fooling myself.

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

LGTM.

review: Approve
Revision history for this message
Kevin DuBois (kdub) wrote :

top-approving!

Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches