824454 - Window to window activity transition is broken.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Al Tsai [:atsai]]

Animation is broken, the mini frame is lower than usual.

STR:
1. Launch camera.
2. Click "gallery"

expect:
*. see the animation for changing app

actual:
*. the animation is lower and bigger than expected

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

It's not a Gaia bug, but a regression of async animation, possibility bug 780692.

Turning off async animation will prevent this bug from happening:

==
diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js
index 50f5a70..e781ca7 100644
--- a/b2g/app/b2g.js
+++ b/b2g/app/b2g.js
@@ -250,12 +250,12 @@ pref("layers.offmainthreadcomposition.animate-opacity", fa
 pref("layers.offmainthreadcomposition.animate-transform", false);
 pref("layers.async-video.enabled", false);
 #else
-pref("dom.ipc.tabs.disabled", false);
-pref("layers.offmainthreadcomposition.enabled", true);
-pref("layers.acceleration.disabled", false);
-pref("layers.offmainthreadcomposition.animate-opacity", true);
-pref("layers.offmainthreadcomposition.animate-transform", true);
-pref("layers.offmainthreadcomposition.throttle-animations", true);
+pref("dom.ipc.tabs.disabled", true);
+pref("layers.offmainthreadcomposition.enabled", false);
+pref("layers.acceleration.disabled", true);
+pref("layers.offmainthreadcomposition.animate-opacity", false);
+pref("layers.offmainthreadcomposition.animate-transform", false);
+pref("layers.offmainthreadcomposition.throttle-animations", false);
 pref("layers.async-video.enabled", true);
 pref("layers.async-pan-zoom.enabled", true);
 #endif

Comment 2

[Nick Cameron [:nrc]]

Can I check that this is still broken after bug 822231 landed?

Also, to confirm that 780692 broke this, just setting layers.offmainthreadcomposition.throttle-animations to false and leaving the other prefs true should fix it.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Nick Cameron [:nrc] from comment #2)
> Can I check that this is still broken after bug 822231 landed?
> 

I am using latest gecko-b2g18, with the bug 822231 patch.

> Also, to confirm that 780692 broke this, just setting
> layers.offmainthreadcomposition.throttle-animations to false and leaving the
> other prefs true should fix it.

Yeah, checking that right now.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #3)
> (In reply to Nick Cameron [:nrc] from comment #2)
> 
> > Also, to confirm that 780692 broke this, just setting
> > layers.offmainthreadcomposition.throttle-animations to false and leaving the
> > other prefs true should fix it.
> 
> Yeah, checking that right now.

OK. setting only layers.offmainthreadcomposition.throttle-animations to false does NOT stop this bug from happening. Somewhere else in async animation is causing the regression.

Comment 5

[Nick Cameron [:nrc]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #3)
> (In reply to Nick Cameron [:nrc] from comment #2)
> > Can I check that this is still broken after bug 822231 landed?
> > 
> 
> I am using latest gecko-b2g18, with the bug 822231 patch.

:-(

> 
> > Also, to confirm that 780692 broke this, just setting
> > layers.offmainthreadcomposition.throttle-animations to false and leaving the
> > other prefs true should fix it.
> 
> Yeah, checking that right now.

Thanks for checking, I'll look into this tomorrow. If it is not too much trouble, it would be great to have an html test case that is confirmed to have the same behaviour on the phone (I don't have a b2g testing setup right now due to a broken laptop). If not I'll try to make a test case from the Gaia source.

Comment 6

[Nick Cameron [:nrc]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #4)
> OK. setting only layers.offmainthreadcomposition.throttle-animations to
> false does NOT stop this bug from happening. Somewhere else in async
> animation is causing the regression.

It is still possible, but much less likely, but this is useful info, thanks!

Comment 7

[Johnny Stenback (:jst)]

Blocking, as this is a UX regression.

Comment 8

[Nick Cameron [:nrc]]

Attachment: Possible testcase

Could someone check if this mimics the bug when used in the b2g browser? I see the green box transition smoothly except for the bottom ~150pixels which jumps at the end of the animation. That is not really what is described by the reporter, but it does disappear if you turn off the full set of prefs (note: which includes all HWA), but not if you just turn off OMTA throttling. Also, seems to be a recent-ish regression. And the transition is the same as a window opening, I think.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: View meta viewport supplication

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: With meta viewport supplication

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(Temporarily beset by bug 825023, but) this test seems to work as intended when run as both a web page and an app.

Comment 12

[Nick Cameron [:nrc]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #11)
> (Temporarily beset by bug 825023, but) this test seems to work as intended
> when run as both a web page and an app.

Sorry cjones, but "work as intended" means displays the broken behaviour or renders correctly?

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Heh, sorry.  I don't see anything janky.

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

More precisely, it looks about the same as in my desktop FF (except without the frame skipping and tearing ;) ).

Comment 15

[Nick Cameron [:nrc]]

Is there somewhere other than camera/gallery that this transition gets used?

I have found an issue with colour layers and OMTA, I'm working on a fix for that, but I don't think that is causing the problem here (because then my test case should display the same broken behaviour).

The camera app seems to be a different window size from the gallery. I've observed that on my phone which has a build from around the time that OMTA throttling landed (i.e., with OMTA everything, but without the bug fixes since). I can also observe that on Firefox OS, which seems to be an older build since the pref for throttling OMTA is not there. Also the size difference appears whether HWA is on or off (as in comment 1, but in the simulator), so that makes me think it is not the same issue as reported here. But perhaps it contributes. I tried looking through the Gaia source but could not find a way to check it. Is this a known issue? Could someone who knows the html/css look into that?

To be precise about what I am observing: I see that the gallery app is consistently smaller (just in height, by apx 0.5cm) and lower down (by a few mm) than the camera app when it is in the card state (i.e., shrunk down). When the camera expands to fill the window the extra height janks into place at the end of the transition. Is this exactly what is observed in the bug description, or is there something else too.

Comment 16

[Andrew Overholt [:overholt]]

Are you still looking for information here, Nick?

Comment 17

[Nick Cameron [:nrc]]

(In reply to Andrew Overholt [:overholt] from comment #16)
> Are you still looking for information here, Nick?

Yes please!

Comment 18

[Nick Cameron [:nrc]]

So I thought I had a possible cause for this bug (bug 825995), but the bug is a regression and is too recent to be in the b2g18 repo :-( So back to the start on this.

Getting the info requested in comment 15 would be useful, thanks!

Comment 19

[Andrew Overholt [:overholt]]

CCing some people who may be able to provide the info requested in comment 15.

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Nick Cameron [:nrc] from comment #15)
> Is there somewhere other than camera/gallery that this transition gets used?
> 

Open any app to the foreground, pull down the utility tray and press the rightmost button to launch the settings app would trigger the transition.

> I have found an issue with colour layers and OMTA, I'm working on a fix for
> that, but I don't think that is causing the problem here (because then my
> test case should display the same broken behaviour).
> 
> The camera app seems to be a different window size from the gallery. I've
> observed that on my phone which has a build from around the time that OMTA
> throttling landed (i.e., with OMTA everything, but without the bug fixes
> since). I can also observe that on Firefox OS, which seems to be an older
> build since the pref for throttling OMTA is not there. Also the size
> difference appears whether HWA is on or off (as in comment 1, but in the
> simulator), so that makes me think it is not the same issue as reported
> here. But perhaps it contributes. I tried looking through the Gaia source
> but could not find a way to check it. Is this a known issue? Could someone
> who knows the html/css look into that?
> 

Yes, camera app is a fullscreen app and is given a class |fullscreen-app|. All of the CSS related to app iframes is located in apps/system/style/system/system.css

> To be precise about what I am observing: I see that the gallery app is
> consistently smaller (just in height, by apx 0.5cm) and lower down (by a few
> mm) than the camera app when it is in the card state (i.e., shrunk down).
> When the camera expands to fill the window the extra height janks into place
> at the end of the transition. Is this exactly what is observed in the bug
> description, or is there something else too.

That's a glitch and a Gaia polish bug. This bug is about something else.

App-to-app transition happens in three stages:

1) the foreground app is shrunk down into a card, 
2) two app frames, both shrunk down, moves horizontally until the opening app is centered
3) the opening app is enlarged

What happened is at stage (2), the |transform: scale(0.6)| is being ignored.

If you happen to have two phones, disable async animation and run the same transition, you will see the difference.

Feel free to needinfo? me if have more questions :)

Comment 21

[Andrew Overholt [:overholt]]

Did you mean to take off the bb+ on this, Tim?

Comment 22

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Andrew Overholt [:overholt] from comment #21)
> Did you mean to take off the bb+ on this, Tim?

No. Please set it back, thanks.

Comment 24

[Joe Cheng [:jcheng] (please needinfo)]

comment 20 changed BB+ to BB? by accident. change back to BB+

Comment 25

[Nick Cameron [:nrc]]

Could we get help getting a regression window for this? It must be a fairly recent regression.

Comment 26

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: screenshot of the erroneously scaled cards

screenshot of the erroneously scaled cards

Comment 27

[Al Tsai [:atsai]]

it works good at BUILD ID: 20121222230202

and it was broken after BUILD ID: 20121223070202

Comment 28

[Nick Cameron [:nrc]]

(In reply to Al Tsai [:atsai] from comment #27)
> it works good at BUILD ID: 20121222230202
> 
> and it was broken after BUILD ID: 20121223070202

These look to me to be built off the same changeset (368d0c521410), I assume I've made a mistake looking up the changeset id. Could you add the changeset ids for the working and broken builds please?

Comment 29

[Nick Cameron [:nrc]]

The second patch in bug 822231 is what regressed this.

Comment 30

[Julien Wajsberg [:julienw]]

Wondering if this is the same than Bug 827263.

Comment 31

[Nick Cameron [:nrc]]

(In reply to Julien Wajsberg [:julienw] from comment #30)
> Wondering if this is the same than Bug 827263.

I don't think it is, the test case is still broken for me without patch which causes this bug.

Comment 33

[Nick Cameron [:nrc]]

Attachment: patch

Comment 34

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 699616 [details] [diff] [review]
patch

r=me after IRL discussion.

landitlanditlandit

Comment 35

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c20accd6774

Comment 36

[Nick Cameron [:nrc]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/6975c38ff5f6

Comment 37

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4c20accd6774

Comment 38

[Tim Guan-tin Chien [:timdream] (please needinfo)]

woot!

Comment 39

[Al Tsai [:atsai]]

Close the bug because it works fine now.