926535 - Wait for the app to be fully repainted before starting the warm launch transition

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: warm.launch.wait.for.full.repaint.patch

Right now the layer tree is rebuilding in the user face when the application is warm and it is reopened.

the patch ensure that the layer tree has been reconstructed/repainted before starting the transition.

Comment 1

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 816686 [details] [diff] [review]
warm.launch.wait.for.full.repaint.patch

Review of attachment 816686 [details] [diff] [review]:
-----------------------------------------------------------------

The original appWindow._waitForNext has a timer to ensure the callback finally occurs.
I don't know what's the latency of |getScreenshot| but if it's fast enough I'm O.K.

BTW, |waitForFullRepaint| looks to me should be named after |ensureFullRepaint| or whatever. But I don't mind if you don't change the name.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/e8356415358cc1c9bce496427ff14c88d9d77ec3

I landed it with the name change. About the timeout I think one of the main reasons for having it on waitForNextPaint is because the app could be in  state where no repainting are occuring, which is different since we are kind of forcing a repaint with getScreenshot. But if we noticed that it cause issues I won't mind to add a timer :)

Thanks for the quick review.

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

This seems fix bug 922546(but I don't know why yet)

Comment 4

[Preeti Raghunath(:Preeti)]

Hi Alive,

First of all, what is the user impact of not taking this bug fix?

Also, can we be certain that this bug does fix 922546.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

I could only tell user impact is the unstable feeling about UI flicking for bug 922546

(In reply to Preeti Raghunath(:Preeti) from comment #4)
> Hi Alive,
> 
> First of all, what is the user impact of not taking this bug fix?
> 
> Also, can we be certain that this bug does fix 922546.

As Sotaro said
https://bugzilla.mozilla.org/show_bug.cgi?id=922546#c28

Comment 6

[bhavana bajaj [:bajaj]]

Blocking to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=922546

Comment 8

[bhavana bajaj [:bajaj]]

Setting the assignee to Vivien given he has authored the patch. Vivien is this good to land ?

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

This is already landed on master, need uplift.

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

The merged commit is e8356415358cc1c9bce496427ff14c88d9d77ec3

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 e8356415358cc1c9bce496427ff14c88d9d77ec3
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Sigh. This stuff probably need a completely custom patch to the window manager to land on 1.2. Do we really want this, this is pretty risky :(

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Asking koi? again since I had to introduce a 400ms delay to wait instead of ensuring that the app is fully repainted. The main reason for that is because some apps sometimes takes a few seconds to repaint and this waiting that long makes the interface feels unresponsive. And with the delay the application can still flicker when it comes back from the card view...

Comment 15

[Preeti Raghunath(:Preeti)]

Vivien,

What is the impact to fugu?

Comment 16

[James Zhang (Spreadtrum)]

Attachment: 247432.patch

patch for v1.2 or v1.2f, verified.

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8347019 [details] [diff] [review]
247432.patch

Review of attachment 8347019 [details] [diff] [review]:
-----------------------------------------------------------------

You want to backport bug 941024 on top of this one or it will create its own set of other issues.

Comment 18

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Preeti Raghunath(:Preeti) from comment #15)
> Vivien,
> 
> What is the impact to fugu?

The impact if the patch from bug 941024 is not backported on top of it, is when you click re-open the application by clicking the icon on the homescreen it may take a while, and make the device feels unresponsive.

With bug 941024 on top of it, the situation is better. If the app take a while to repaint then we still start to open it and it repaints in the face of user for the rest of the time.