1161215 - [Gallery] Picture will flicker when changing orientation on the edit screen

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[KTucker [:KTucker][Inactive 3/4/2016]]

Description:
The user will notice that the picture on the crop screen will flicker when changing orientation from portrait to landscape and vise versa.

Repro Steps:
1) Update a Flame to 20150504010202.
2) Open Gallery.
3) Tap on a picture to open it.
4) Tap on the "Edit" icon.
5) Tap on the "Crop" icon.
6) Rotate the phone to landscape and back to portrait while on the crop screen.

Actual:
The picture flickers when changing orientation on the crop screen.

Expected:
The picture does not flicker when changing orientation.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150504010202
Gaia: e18cce173840d6ff07fb6f1f0e0ffb58b99aab3e
Gecko: dc5f85980a82
Gonk: a9f3f8fb8b0844724de32426b7bcc4e6dc4fa2ed
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Repro frequency: 5/5
See attached: video clip, logcat

Comment 1

[KTucker [:KTucker][Inactive 3/4/2016]]

Adding qawanted for a branch check.

Comment 2

[KTucker [:KTucker][Inactive 3/4/2016]]

Attachment: CropFlickeringVideo.mp4

Comment 3

[Brogan Zumwalt [Inactive]]

Issue does NOT reproduce on Flame 2.2, the picture does not flicker when changing orientation.

Device: Flame 2.2
Build ID: 20150504002502
Gaia: 8d14361337e608c8cdf165ea5034db5eda23b618
Gecko: cb7cb6597c91
Gonk: a9f3f8fb8b0844724de32426b7bcc4e6dc4fa2ed
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 4

[Brogan Zumwalt [Inactive]]

Attachment: Logcat

Comment 5

[KTucker [:KTucker][Inactive 3/4/2016]]

[Blocking Requested - why for this release]:

This looks bad and it is a regression so nominating 3.0?

Comment 6

[Brogan Zumwalt [Inactive]]

Mozilla-Central Regression window:

Unable to find inbound window before end of day, will provide inbound window tomorrow morning.

Last working Central build:
Device: Flame 3.0
Build ID: 20150401134635
Gaia: 4bb3a933bd805e8df1e11827cb247754c3565b0b
Gecko: e044f4d172e2
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

First broken Central build:
Device: Flame 3.0
Build ID: 20150401175537
Gaia: 4bb3a933bd805e8df1e11827cb247754c3565b0b
Gecko: 37ddc5e2eb72
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0


Working Gaia with Broken Gecko issue DOES reproduce:
Gaia: 4bb3a933bd805e8df1e11827cb247754c3565b0b
Gecko: 37ddc5e2eb72

Working Gecko with Broken Gaia issue does NOT reproduce:
Gaia: 4bb3a933bd805e8df1e11827cb247754c3565b0b
Gecko: e044f4d172e2


Central Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e044f4d172e2&tochange=37ddc5e2eb72

Comment 7

[Pi Wei Cheng [:piwei] (inactive)]

mozilla-inbound regression window:

Last Working
Device: Flame
BuildID: 20150401113734
Gaia: 4bb3a933bd805e8df1e11827cb247754c3565b0b
Gecko: 5864129e5d5f
Version: 40.0a1 (3.0 Master) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

First Broken
Device: Flame
BuildID: 20150401123737
Gaia: 4bb3a933bd805e8df1e11827cb247754c3565b0b
Gecko: 0160303649ae
Version: 40.0a1 (3.0 Master) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5864129e5d5f&tochange=0160303649ae

Caused by Bug 1075670.

Comment 8

[KTucker [:KTucker][Inactive 3/4/2016]]

David, can you take a look at this please? This might have been caused by the landing for bug 1075670.

Comment 9

[David Parks [:handyman]]

I'm not going to be around to look at this but kats might have some ideas...

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Step 5 in the STR is not needed; this happens on the edit screen as well. Note also that the "Edit" header bar doesn't flicker, it's just the picture area. So it's likely not a gfx issue but maybe we're dispatching too many resize events or something. I'll look into it a bit more.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Indeed, the resizeHandler() in apps/gallery/js/ImageEditor.js is getting hit 3 times on each rotation, which seems a little excessive. I can't easily back out bug 1075670 to test if that was the cause though.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

So yeah, it looks like we're sending 2 or 3 resize events from Gecko. RecvUpdateDimensions is getting called 3 times per rotation. The |rect| and |orientation| parameters don't change, but the |size| changes on the second call and the |chromeDisp| changes on the third call.

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

This fixes it for me.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8602198 [details] [diff] [review]
Patch

I can rebase this on top of your patch if you can figure out the issue there and want to get it landed first.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8602198 [details] [diff] [review]
Patch

At which point do we get RecvUpdateFrame(const FrameMetrics& aFrameMetrics)?
Since that ends up changing mLastRootMetrics, and HandlePossibleViewportChange
then uses that data.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8602198 [details] [diff] [review]
Patch

But that is actually a separate issue.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

(In reply to Olli Pettay [:smaug] from comment #15)
> At which point do we get RecvUpdateFrame(const FrameMetrics& aFrameMetrics)?

We get these periodically when APZ requests a content repaint.

> Since that ends up changing mLastRootMetrics, and
> HandlePossibleViewportChange
> then uses that data.

Hm, I suppose there might be possible issue here but we haven't run into it yet :) *fingers crossed*

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e76f9c955db8

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e76f9c955db8

Comment 20

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified fixed on Flame 3.0. Picture no longer flickers when rotating phone on crop screen.

Device: Flame (KK, full flashed, 319MB)
BuildID: 20150511010202
Gaia: 6089234ace8b294a8feef064387604bae16254e3
Gecko: d8420a541d1c
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 40.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0