Mir

Wrong frame is seen on wake up/resume/unlock.

Bug #1233564 reported by Albert Astals Cid on 2013-10-01
44
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Mir
Fix Released
High
Alexandros Frantzis
Unity System Compositor
Undecided
Unassigned
mir (Ubuntu)
High
Unassigned
unity-system-compositor (Ubuntu)
Undecided
Unassigned
unity8 (Ubuntu)
Critical
Michael Terry

Bug Description

This is happening when using Mir.

How to reproduce:
 * Start the phone
 * Unlock the greeter
 * Wait until the phone screen turns black
 * Wait a bit more
 * Press the phone side button
 * See that the greeter slides in

I added some quick debugging and when the screen turns black we actually enter the
  if (status == Powerd.Off && (flags & Powerd.UseProximity) == 0) {
switch in onDisplayPowerStateChange so it seems it is doing that part correctly.

Related branches

Changed in unity8:
assignee: nobody → Michael Terry (mterry)
Revision history for this message
Michał Sawicz (saviq) wrote :

This is related to the "don't animate on locking" bug that we had some time ago... You can also see that under surfaceflinger when you press the power button twice quickly - you can see the greeter animate in.

Changed in unity8:
importance: Undecided → Critical
milestone: none → phone-v1-freeze
Revision history for this message
Michael Terry (mterry) wrote :

The "don't animate on locking" issue was bug 1192707.

This is odd, that we'd wait until screen is back on to engage the animation. Looking into it.

Revision history for this message
Michael Terry (mterry) wrote :

Animation starts on time but pauses inexplicably after a second. It doesn't seem to be actually had pause() called on it or anything. I'm guessing Mir itself (or unity-mir) is putting unity8 on ice when the screen is black?

Revision history for this message
Michael Terry (mterry) wrote :

OK, talking to Mir guys, the ideal fix is two-fold:
1) Make unity8 avoid animating the greeter when blanking the screen.
2) Make Mir flush its buffers when blanking the screen to avoid showing any old frames

racarr said he could work on Mir side this week, maybe Thursday. I've got a branch I'm about to propose for unity8 side.

Changed in mir:
assignee: nobody → Robert Carr (robertcarr)
kevin gunn (kgunn72) on 2013-10-04
Changed in mir:
status: New → Opinion
Changed in unity8:
status: New → In Progress
Changed in mir:
status: Opinion → Invalid
status: Invalid → Triaged
importance: Undecided → High
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:unity8 at revision None, scheduled for release in unity8, milestone ubuntu-13.09

Changed in unity8:
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package unity8 - 7.82+13.10.20131008-0ubuntu1

---------------
unity8 (7.82+13.10.20131008-0ubuntu1) saucy; urgency=low

  [ Michał Sawicz ]
  * update previews to match design. (LP: #1224555)
  * Add an InputFilterArea in Notifications. (LP: #1233411, #1235215)

  [ Ying-Chun Liu ]
  * update previews to match design. (LP: #1224555)

  [ Albert Astals ]
  * Unrevert 376 by reverting r395 and a small fix to fix the cpu
    hogging issue . (LP: #1124567)

  [ Michael Terry ]
  * Add Showable.showNow() method and use it in Shell to immediately
    show greeter when we blank the screen rather than animating it. (LP:
    #1233564)

  [ Michael Zanetti ]
  * update previews to match design. (LP: #1224555)

  [ Diego Sarmentero ]
  * update previews to match design. (LP: #1224555)

  [ Ubuntu daily release ]
  * Automatic snapshot from revision 404
 -- Ubuntu daily release <email address hidden> Tue, 08 Oct 2013 02:57:55 +0000

Changed in unity8 (Ubuntu):
status: New → Fix Released
Michał Sawicz (saviq) on 2013-10-08
Changed in unity8:
status: Fix Committed → Fix Released
Omer Akram (om26er) on 2013-10-10
Changed in mir (Ubuntu):
importance: Undecided → High
status: New → Triaged
tags: added: rls-t-incoming
Revision history for this message
kevin gunn (kgunn72) wrote :

since we are on the cusp of splitting out the greeter to use mir-on-mir, this bug will likely get more noticeable.
Raising priority to get a fix in place in time to avoid that

Changed in mir:
importance: High → Critical
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

It's been a month. That's long enough at artificially inflated importance :)

Changed in mir:
importance: Critical → Medium
Revision history for this message
kevin gunn (kgunn72) wrote :

"likely get more noticeable."
inflated huh ? ;)
https://bugs.launchpad.net/ubuntu/+source/mir/+bug/1279078

Changed in mir:
importance: Medium → High
Revision history for this message
kevin gunn (kgunn72) wrote :

and we really need to fix this along with split greeter, sort of defeats the purpose of the lock screen if you can (altho briefly) see the information behind it

Changed in mir:
assignee: Robert Carr (robertcarr) → nobody
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

An idea I raised quietly a couple of months ago was to bring together power management and compositor pause/resume as a single operation all done and coordinated by Mir itself:
    https://bugs.launchpad.net/unity-mir/+bug/1255045/comments/18
That would provide us also with the perfect opportunity to "flush" all the buffer streams/bundles on resume and resolve this bug.

The good news is that we already have the high-level interface:
   Display::pause()
   Display::resume()

The only problem as mentioned in the above link is that it would require some additional coupling that does not presently exist - the Display needs to know about Compositors.

summary: - Greeter is seen animating when pressing the side button to wake up
+ Wrong frame is seen on wake up/resume/unlock.
Changed in unity-system-compositor:
status: New → In Progress
assignee: nobody → Alberto Aguirre (albaguirre)
Changed in mir:
status: Triaged → Invalid
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I'm pretty sure we can fix this properly in Mir. So leave the option open.

Changed in mir:
status: Invalid → Triaged
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

This should be taken care of outside of Mir.

Currently the issue occurs due to the unity-system-compositor stopping the compositor before the shell/greeter has a change to show any shutdown animation.

On display power off:

powerd sends D-Bus signal setScreenPowerMode("off") to com.canonical.Unity.Screen
unity-system-compositor receives signal, stops mir compositor and turns off screen
powerd emits DisplayPowerStateChange signal
unity8 shell receives DisplayPowerStateChange - attempts to show greeter
rendering is blocked since USC stopped compositor.
There may some frames queued up with content before greeter animation hence the issue reported here.

On display power on:
powerd sends D-Bus signal setScreenPowerMode("on") to com.canonical.Unity.Screen
unity-system-compositor receives signal, starts compositor and turns on screen
mir compositor starts, unity8 shell rendering continues from where it left off.

For now a workaround is to add a configurable delay at power off in unity-system-compositor:
https://code.launchpad.net/~albaguirre/unity-system-compositor/add-power-off-delay-option/+merge/208281

The ideal solution is better coordination between powerd, the shell/greeter and USC; The shell should be given an opportunity to show a shutdown animation.

Maybe Mir could add a parameter to the compositor stop to specify how many frames to render before actually stopping though I don't see this as necessary.

Perhaps the shell should be in charge of notifying USC when to power off instead.

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

Well the problem is going to occur on desktop still. And it's basically the same problem for any shell/compositor so we don't want to repeat effort. Any logic likely to be reused in other shells should reside in Mir itself.

The problem is quite simple and definite inside Mir. It's just a bit tricky working out the best way to flush each surface's buffer queue on resume.

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

I wanted to point out something - we need to be able to delay powering _on_, too, for shell to be able to composite a frame with new time and such, even if we delay the greeter now, when we wake up we'll see the old time for a moment.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package unity-system-compositor - 0.0.2+14.04.20140311.1-0ubuntu1

---------------
unity-system-compositor (0.0.2+14.04.20140311.1-0ubuntu1) trusty; urgency=low

  [ Robert Ancell ]
  * Install LightDM configuration into /usr/share so it's not kept
    around by dpkg

  [ CI bot ]
  * Add an optional delay when powering off screen. Add an option
    (power-off-delay, defaulted to 0) to delay stopping the compositor
    and powering off the screen to allow the shell to present a shutdown
    animation; without delay, the shell and-or greeter will get blocked
    while trying to render such animation; this is due to the compositor
    stopping as USC receives the power state change before the shell.
    This is mostly a workaround as ideally, the greeter and-or shell
    should coordinate when to shutdown the screen after they are given a
    chance to possibly show a shutdown animation. fixes: lp: #1233564
    (LP: #1233564)

  [ Michael Terry ]
  * Make sure that the active session has focus when multiple sessions
    are opened.
 -- Ubuntu daily release <email address hidden> Tue, 11 Mar 2014 03:37:20 +0000

Changed in unity-system-compositor (Ubuntu):
status: New → Fix Released
Revision history for this message
Selene ToyKeeper (toykeeper) wrote :

Just a note; I'm still seeing this on image 263. It seems that the longer the phone is off, the longer it takes the display to catch up when it turns on again. If it has been idle for a whole day, the display can end up frozen for 5+ seconds before it responds to user input.

tags: added: flo r263
Revision history for this message
Michał Sawicz (saviq) wrote : Re: [Bug 1233564] Re: Wrong frame is seen on wake up/resume/unlock.

On 27.03.2014 08:39, Selene Scriven wrote:
> Just a note; I'm still seeing this on image 263. It seems that the
> longer the phone is off, the longer it takes the display to catch up
> when it turns on again. If it has been idle for a whole day, the
> display can end up frozen for 5+ seconds before it responds to user
> input.

That's a side-effect of bug #1292306.

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

I believe this branch lp:~afrantzis/mir/non-blocking-swap-buffers will indirectly fix this issue as the clients rendering won't be blocked.

Changed in unity-system-compositor:
status: In Progress → Triaged
assignee: Alberto Aguirre (albaguirre) → nobody
Changed in mir:
milestone: none → 0.1.9
assignee: nobody → Alexandros Frantzis (afrantzis)
status: Triaged → In Progress
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:mir/devel at revision None, scheduled for release in mir, milestone Unknown

Changed in mir:
status: In Progress → Fix Committed
Changed in mir:
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package mir - 0.1.9+14.10.20140430.1-0ubuntu1

---------------
mir (0.1.9+14.10.20140430.1-0ubuntu1) utopic; urgency=medium

  [ Daniel van Vugt ]
  * New upstream release 0.1.9 (https://launchpad.net/mir/+milestone/0.1.9)
    - mirclient ABI unchanged, still at 7. Clients do not need rebuilding.
    - mirserver ABI bumped to 19. Shells need rebuilding.
    - More libmirserver class changes and reorganization, including;
      . Moving things from shell:: to scene::
      . Rewriting/refactoring surface factories.
    - Added an id() to Renderable.
    - Scene/Renderer interfaces:
      . Scene is no longer responsible for its own iteration (no for_each
        any more). Instead you should iterate over the list returned by
        Scene::generate_renderable_list().
    - Bugs fixed:
      . Stale socket issue. (LP: #1285215)
      . Qt render gets blocked on EGLSwapBuffers. (LP: #1292306)
      . Lock order violated found in helgrind (potential deadlock).
        (LP: #1296544)
      . [regression] SwitchingBundle in framedropping mode can hang.
        (LP: #1306464)
      . [DPMS] Display backlight turns back on almost immediately after
        being turned off. (LP: #1231857)
      . Wrong frame is seen on wake up/resume/unlock. (LP: #1233564)
      . Nested platform is not testable (LP: #1299101)
      . [regression] mir_demo_server_shell crashes on display resume.
        (LP: #1308941)
      . Multi-threaded composition is actually mostly serialized by
        SurfaceStack::guard. (LP: #1234018)
      . Mirscreencast slows down compositing and makes it very jerky.
        (LP: #1280938)
      . Mirscreencast can cause clients to render faster than the screen
        refresh rate. (LP: #1294361)
      . Screen turns on when a new session/surface appears. (LP: #1297876)
      . mir-doc package is >56MB in size, expands to >100MB of files.
        (LP: #1304998)
      . [regression] Clang: 'mir::test::doubles::MockSurface::visible'
        hides overloaded virtual function [-Woverloaded-virtual].
        (LP: #1301135)
      . [regression] GLRenderer* unit tests have recently become noisy.
        (LP: #1308905)
      . FocusController::set_focus_to() no longer seems to raise a session
        to the top. (LP: #1302689)

  [ Ubuntu daily release ]
  * New rebuild forced
 -- Ubuntu daily release <email address hidden> Wed, 30 Apr 2014 13:26:58 +0000

Changed in mir (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Was fixed long ago and wasn't unity-system-compositor

Changed in unity-system-compositor:
status: Triaged → Fix Released
Michał Sawicz (saviq) on 2017-03-13
Changed in unity8 (Ubuntu):
assignee: nobody → Michael Terry (mterry)
importance: Undecided → Critical
no longer affects: unity8
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers