Closed Bug 1161215 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.5+, firefox40 fixed, b2g-v2.2 unaffected, b2g-master verified)

VERIFIED FIXED
2.2 S12 (15may)
blocking-b2g 2.5+
Tracking Status
firefox40 --- fixed
b2g-v2.2 --- unaffected
b2g-master --- verified

People

(Reporter: KTucker, Assigned: kats)

References

Details

(Keywords: regression, Whiteboard: [3.0-Daily-Testing])

Attachments

(3 files)

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
Adding qawanted for a branch check.
Keywords: qawanted
Whiteboard: [3.0-Daily-Testing]
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: qawantedregression
QA Contact: bzumwalt
Attached file Logcat
Blocks: 1161233
No longer blocks: 1161233
Flags: needinfo?(ktucker) → needinfo?(pbylenga)
[Blocking Requested - why for this release]:

This looks bad and it is a regression so nominating 3.0?
blocking-b2g: --- → 3.0?
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
QA Whiteboard: [QAnalyst-Triage?]
QA Contact: bzumwalt → pcheng
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.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
David, can you take a look at this please? This might have been caused by the landing for bug 1075670.
Blocks: 1075670
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(davidp99)
I'm not going to be around to look at this but kats might have some ideas...
Flags: needinfo?(davidp99) → needinfo?(bugmail.mozilla)
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.
Summary: [Gallery] Picture will flicker when changing orientation on the crop screen → [Gallery] Picture will flicker when changing orientation on the edit screen
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.
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.
Attached patch PatchSplinter Review
This fixes it for me.
Assignee: nobody → bugmail.mozilla
Flags: needinfo?(bugmail.mozilla)
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.
Attachment #8602198 - Flags: review?(bugs)
Flags: needinfo?(pbylenga)
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 on attachment 8602198 [details] [diff] [review]
Patch

But that is actually a separate issue.
Attachment #8602198 - Flags: review?(bugs) → review+
(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*
https://hg.mozilla.org/mozilla-central/rev/e76f9c955db8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S12 (15may)
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
blocking-b2g: 2.5? → 2.5+
You need to log in before you can comment on or make changes to this bug.