[Mako] Session screen seen upon quick power key strike

btw, saw this on both virgin image & with qtcomp during qtcomp testing

I think I noticed this in mir_demo_server_shell too. USC shines through briefly on resume... Although in my case I'm trying to run two system compositors so expect them to fight for the screen.

Actually, not confirmed. My problem was indeed that I had two system compositors fighting for the screen on wakeup.

Re-confirmed. It's rather obvious in official images.

Inverstigation indicates that the main cause of the issue is a delay in unity8 when showing the greeter. There may be a stale frame problem in Mir hiding in here, but I can neither confirm nor disprove this at the moment (so setting this bug as incomplete for Mir), since the effect of unity8's rendering delay is overwhelming.

What I see happening is:

1. User strikes power key
2. USC catches event, blanks screen, notifies unity dbus service of change in display state
3. unity8 gets display state change event, tells greeter to show
4. user strikes power key again
5. USC catches event, unblanks screen, notifies unity dbus service of change in display state
6. If (4) happens very soon after (1) (and note that 3,4 can occur concurrently), unity8 has not yet had the time to show the greeter on screen, so the user sees a "flash" of the dash first, and soon after the greeter is displayed.

Experiments supporting the unity8 delay hypothesis:

1. The closer the two power key strikes are, the longer the "flash" of the dash is.
2. In USC, moving the unity dbus service display change notification sooner (e.g. before blanking the screen), makes the dash "flash" shorter (since we give unity8 more time to render).
3. Simplifying the Greeter (e.g. hacking its qml to display a red rectangle instead of all the user info and background), reduces the dash "flash" time, since it reduces the time unity8 needs to render the greeter

Note that unless unity8 can get the display state change event and render the greeter in less than the time USC needs to react to the two power key strikes (to blank and unblank the screen, respectively), we are always going to have this issue (it's an inherent "race").

Further investigation has confirmed that a stale frame issue exists *in addition* to the greeter rendering delay problem described in the previous comment. As noted before, the stale frame issue is overshadowed by the greeter rendering delay in most cases.

For all the following the notation is : d = dash frame arrival, g = greeter frame arrival, {...}=screen off period

Basic stale frame:
d {g1} g2 ... => d, g1 are rendered after screen is turned on (d seen as a "flash")

Greeter delay overshadowing stale frame:
d {} g1 g2 ... => d is rendered when screen is turned on, since we have no newer frame at that time

The stale frame issue is not seen for slow power button strikes with the greeter, because the greeter is animating (producing frames) and if the strikes are over 100ms apart the oldest frame (which is the stale frame) gets dropped by our frame dropping policy since we are not consuming it. This is:

d {g1 g2} => Time between strikes > 100ms: g1, g2 are rendered (d = stale dash frame is evicted)

Experiments supporting the stale frame issue:

1. If we simplify the greeter to be a filled rectangle, without any animations, we occasionally see just the dash when the screen is turned off and on again. We need to force another rendering (e.g. by touching the screen) to see the greeter. This happens because the restarted compositor only renders one frame (the stale one), since it has no knowledge that other frames exist (this is a bug in Mir):

d {g} => Only d rendered when compositor restarted, need to trigger to render g

2. In the previous case (1), if we force the compositor to render 2 frames on start instead of 1, we *always* get a dash "flash" after turning the screen on, even if the time between the button strikes is not small. Note that this is different from the normal greeter case because in this case there is no animation going on, no extra frames are sent, and therefore our frame dropping policy doesn't kick in to evict old frames.

d {g} => Both d,g rendered when compositor restarted, regardless of time screen is off

To fix the stale frame issue, Mir should drop all old frames for a surface if a new frame for that surface arrives while the screen is turned off:

d {g1} g2 ... => g1 arrived while screen was off, d is dropped, so in the end g1, g2... are rendered

Note how this fix doesn't resolve the greeter delay problem since in that case no frames arrive while the screen is off.

A solution for the stale frame aspect of this bug is proposed in https://code.launchpad.net/~afrantzis/mir/drop-stale-frames/+merge/227961 .

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

Still happens on Utopic 161, but it doesn't seem like a blocker. As far as I can tell, the screen must be off for less than a second in order to trigger this bug, which is very unlikely to happen unless the person who turned it off is the same as the person who turned it back on. So, not really a privacy risk. It looks like the welcome screen program simply hasn't loaded/rendered yet.

I'm not sure if anyone mentioned this yet, but it appears Mir's default behaviour is to immediately draw a frame on compositor (re)start (for non-nested servers only, like USC).

It's the last parameter here:
           return std::make_shared<mc::MultiThreadedCompositor>(
                the_display(),
                the_scene(),
                the_display_buffer_compositor_factory(),
                the_compositor_report(),
                !the_options()->is_set(options::host_socket_opt)); <=== bool compose_on_start

I'm wondering if we've got that backwards. What we want to see on resume is a new frame from the nested server propagate through. So that statement should probably just be inverted:

    the_options()->is_set(options::host_socket_opt));

Thus on resume, the first frame comes from the nested server. Although we'd also want the same in the case of a single-server system.

Let me rephrase that...

The problem seems to be that MultiThreadedCompositor (unity-system-compositor) unconditionally recomposes on restart:
        compose_on_start = true;

This means the user is guaranteed to see an old frame on restart because we've allowed no time for clients (included nested servers) to provide any new frames.

We should probably just sleep for a second or so in MultiThreadedCompositor on restart. That will allow clients time to provide a new frame (as soon as they can). And then only if no clients have provided a new frame after that delay should MultiThreadedCompositor force a recompose itself.

Alright, I'm preparing to propose a fix/workaround in Mir. Although sadly that won't do us any good if Unity8 still takes a second or so to respond to wakeup. My Mir solution depends on all clients (and nested servers like Unity8) responding to wakeup and providing a new frame (the greeter) in less than half a second (preferably faster than that). If Unity8 doesn't do that (or prepare a greeter screen earlier when the display turns off) then no improvement to Mir will help.

I have verified the proposed Mir fix does not help right now. The threshold of 500ms it uses is too low to account for the Unity8/dbus delay. And increasing the Mir delay to seconds to account for that delay I think would not feel as nice as the screen coming on immediately like it does now.

please update with a reason if you disagree & change is back.
But with the information above, i'm marking this as "opinion" for mir...but leaving open for unity8.

I'm curious why it takes u8 so long to respond to the power key press. It should be almost instantly receiving the dbus message and drawing a frame with the greeter shown. DBus isn't /that/ slow.

Do we show greeter as soon as power-off is hit? I hope we don't draw Greeter on power-on only.

The greeter is shown when it receives the DisplayPowerStateChange with an off status signal which is only emitted after the compositor thread, display and back-light have been shut off.

My plan was to expand the DisplayPowerStateChange signal with a "will power off soon" status and have some sort of ack system, (with a suitable timeout) which shells could register for to allow for short screen lock animations - which I think the greeter lock screen was supposed to be.

OK, alberto says this doesn't happen on krillin & he's right I just gave it several attempts.
I'm suggesting we consider this a mako only issue now, and remove rtm14.

Actually, I just figured out bug 1379610. That will affect unity-system-compositor and so could be related to this.

@alf - since you're in the midst of u-s-c refactor, this is probably a good one for you to track

Please keep related branches linked. They're handy for future reference. Even if 'related' doesn't mean a fix.

@duflu - then why not link even more branches? to me the whole point of attaching a branch is to indicate resolution. I do not see any point in attaching a branch that's not destined to provide a solution for the bug.
If the attached branch _is_ a solution for the bug, then i wholeheartedly agree.
So is lp:~vanvugt/mir/flush a solution in part or in whole ?

Yes, the flush branch is a partial solution. Depending on how the other projects evolve.

New information, this bug had been resolve for some time.
And still only on Nexus4
An additional caveat is that this is only occuring when the device is on cellular roaming (Pat with same phone/sw saw this happen in Barcelona, went away when he returned home, and occurs again on biz travel to london)

Hmm, I just tried on a freshly flashed mako with or without SIM (not roaming), and can reproduce this reliably.

Krillin and Arale seem to behave fine, but I'm not sure if it's by accident - do we actually have code that waits for something (a new frame? feedback from shell?) before powering the screen on?

No, the fix was proposed last year but never landed due to excessive delays outside of Mir that meant it couldn't help yet. See above branch. Might be worth revisiting and landing because if everything other than Mir is now behaving then we need it.

> Krillin and Arale seem to behave fine, but I'm not sure if it's by accident - do we actually have code that waits for something
> (a new frame? feedback from shell?) before powering the screen on?

Before turning the screen on/enabling the compositor, after the power button is pressed, USC currently waits for powerd to notify us that we have come out of suspend. On some devices, this notification takes some time to arrive (about ~0.5 second on Krillin). I haven't tried myself recently but Alberto mentioned that N4 wakes up faster.

The different delays (depending on HW) introduced by this mechanism offer a plausible explanation of why we see the frame on some devices and not on others.

Also see my previous explanation in comment 5 [1]. It's important to note that although we can eventually get a "good enough" solution (e.g. unity8 signals USC that it has rendered the greeter), a 100% perfect solution is difficult to achieve since we can't really know when the frame with the greeter has actually been displayed on screen (vs only rendered and swapped).

[1] https://bugs.launchpad.net/unity-system-compositor/+bug/1340510/comments/5