[xmir] [multimonitor] Frames eventually get slightly out of order, look like glitches or typing will feel slow

Sounds like bug 1216337

Confirmed here with Intel Arrandale. Typing in weechat is ridiculously slow (I get more than half a second of delay between pressing a key and actually seeing the letter), and mouse-scrolling in firefox causes a lot of flicker.

This bug has been reported on the Ubuntu Package testing tracker.

A list of all reports related to this bug can be found here:
http://packages.qa.ubuntu.com/qatracker/reports/bugs/1216472

I just noticed this seems to happen on systems where the monitor refresh rates are slightly different. No wonder I could never reproduce it on the desktop where they're identical...

I've now written a native Mir client that simulates the test case from the bug description...
lp:~vanvugt/mir/progressbar

Strangely, mir_demo_client_progressbar does not exhibit the bug at all even when connected to the same unity-system-compositor. XMir is still the only client that shows the bug.

Progress!

I've now discovered the bug only happens in X desktop environments that use page flipping. So that's Compiz/Unity and sometimes Gnome. If you use Xfce the bug does not occur, and if you disable page flipping in Compiz then the bug does not occur. Workaround added to the Bug Description.

Actually the above comment may not be entirely accurate. But at least disabling page flipping in your X compositor seems to almost entirely eliminate the problem.

It's starting to look like the issue might be in:
xserver-xorg-video-intel 2:2.21.14-4ubuntu2+xmirMM7
and how it schedules it's own double/triple buffers to Mir...

Status changed to 'Confirmed' because the bug affects multiple users.

This bug was fixed in the package mir - 0.0.10+13.10.20130827.1-0ubuntu1

---------------
mir (0.0.10+13.10.20130827.1-0ubuntu1) saucy; urgency=low

  [ Alan Griffiths ]
  * ipc: add a protocol version to the wire protocol so that we can bump
    it in future.
  * graphics::nested: Handling of output configuration changes.
  * graphics.nested: Hookup NestedDisplay to display change
    notifications.

  [ Daniel van Vugt ]
  * Introducing mir_demo_client_progressbar. It's pretty boring;
    designed to simulate key repeat scrolling in a terminal, as an aid
    for tracking down bug 1216472. . (LP: #1216472)

  [ Eleni Maria Stea ]
  * changed the GBMBufferAllocator constructor and class to use the
    gbm_device instead of the GBMPlatform to remove the dependency from
    the mg::Platform interface - this way we can use the
    GBMBufferAllocator with the NativeGBMPlatform (nested mir).

  [ Ubuntu daily release ]
  * Automatic snapshot from revision 1025
 -- Ubuntu daily release <email address hidden> Tue, 27 Aug 2013 18:04:47 +0000

Ignore the previous comment. Nothing is fixed. Silly robot.

Please be careful not to confuse this with bug 1218735. It will happen at the same time, unless you're using the workaround.

I can now debug this from within Mir. Almost disappointingly I can't see any incorrect frame ordering within Mir. The client (XMir) always gets frame 0,1,2,0,1,2,... and the server always composites frame 0,1,2,0,1,2... starting with the latest one released by the client. So everything looks correct from within the server.

Next week I will come at it with fresh eyes, and also try to devise some logical assertions to verify ordering.

OK, I've figured out a terrible hack that seems to solve the problem (although it generates constant damage and constant CPU usage):

_X_EXPORT RegionPtr
xmir_window_get_dirty(xmir_window *xmir_win)
{
    return &xmir_win->region; /* Terrible hack */

This seems to solve the bug, suggesting xmir_window_get_dirty is returning incorrect damage regions.

It's not definitive and not a fix. But looks like good progress.

Further notes... The issue seems to occur on frames where some but not all of the outputs are dirty. So one output makes it through to callback (sna_xmir_copy_to_mir), but the other does not:

_X_EXPORT void
xmir_screen_for_each_damaged_window(xmir_screen *xmir, xmir_window_proc callback)
{
    xmir_window *xmir_win, *tmp_win;
    xorg_list_for_each_entry_safe(xmir_win, tmp_win, &xmir->damage_list, link_damage) {
        if (xmir_window_has_free_buffer(xmir_win) &&
            xmir_window_is_dirty(xmir_win))
            (*callback)(xmir_win, xmir_window_get_dirty(xmir_win));
    }
}

XMir is intentionally designed to only redraw dirty outputs. So not all get swapped every frame. I wonder however if that is somehow incompatible with how the intel DDX works?

Also affects the intel DDX.

I keep finding myself tracing this bug into the Intel SNA code, which I don't yet understand. The issue seems to occur on frames where not-quite-all outputs are dirty, so sna_xmir_copy_to_mir() is only called for some of them (but not all).

The bug goes away if I ensure sna_xmir_copy_to_mir() is called for all outputs. So this seems to suggest that the Intel SNA code is keeping track of frames/buffers and expecting a precise number of kgem_submit's, which of course doesn't always happen with multimonitor.

Ok. I think I've convinced myself from a manual run-through that the XMir damage logic is sound. Next up: Intel DDX.

Status changed to 'Confirmed' because the bug affects multiple users.

I've just tried an experimental change from Alexandros which looks like a good solution. It needs some polish and test cases but should be fully ready next week....
http://paste.ubuntu.com/6132595/

Fix committed into lp:~mir-team/mir/development-branch at revision None, scheduled for release in mir, milestone Unknown

Definitely seeing this in Ubuntu.

I don't think this fix has hit Ubuntu yet. Our automated changelogs and multiple branches make it very unclear...

Confirmed the fix is not in Ubuntu or lp:mir yet. Only the development branch.

Fix committed to lp:mir revision 1088. Ubuntu packages coming soon I guess.

The changelog is inaccurate, due to our current (temporary) merging process. But I assure you the fix was just released, in:

mir (0.0.13+13.10.20131003-0ubuntu1) saucy; urgency=low

Saucy's now up to 0.0.14 so I think we can assume 0.0.13 good as released.