Closed Bug 1160877 Opened 9 years ago Closed 9 years ago

[Shinano][Aries] Framebuffer flickering mixed with Gecko display

Categories

(Core Graveyard :: Widget: Gonk, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.5+, firefox40 fixed, b2g-master fixed)

RESOLVED FIXED
mozilla40
blocking-b2g 2.5+
Tracking Status
firefox40 --- fixed
b2g-master --- fixed

People

(Reporter: gerard-majax, Assigned: jerry)

References

Details

(Keywords: regression, Whiteboard: [spark])

Attachments

(2 files, 2 obsolete files)

This is a recent regression, a build from friday was good.

STR:
 0. ./repo sync && ./build.sh
 1. Flash resulting build on aries device
 2. Boot

Expected:
 Display is good

Actual:
 Gecko display flickers and gets interleaved with framebuffer bootanimation
Bisected:
8c1db328cb78470d395736f6cac543a1b8c4e754 is the first bad commit
commit 8c1db328cb78470d395736f6cac543a1b8c4e754
Author: chiajung hung <chung@mozilla.com>
Date:   Sun Apr 26 22:39:00 2015 -0400

    Bug 1156140 - Choose different code path for boot animation based on device capability. r=mwu

:040000 040000 e658d48ca743480548c1cd15db4f316f04148faf f1c216a8972ec0d010fb9fc34621ec5f494936b9 M	widget
Blocks: 1156140
Flags: needinfo?(vliu)
Flags: needinfo?(chung)
Blocking spark+. Really bad bug. Everything is unusable do to this bug. Please help investigate regression
blocking-b2g: --- → spark+
@Vincent,

Can you help on this? I can not reproduce this on my Flame, so it is impossible for me to debug this.
Flags: needinfo?(chung)
(In reply to Chiajung Hung [:chiajung] from comment #3)
> @Vincent,
> 
> Can you help on this? I can not reproduce this on my Flame, so it is
> impossible for me to debug this.

Ok. I will work on this issue
I know what's going on for this issue. I will have a patch soon.
Assignee: nobody → hshih
Status: NEW → ASSIGNED
Flags: needinfo?(vliu)
Blocks: 1155797
There is a wrong condition for stopping bootanim. The flickering is because of this.
Attachment #8601601 - Flags: review?(mwu)
fix wrong usage of android sp.
Attachment #8601601 - Attachment is obsolete: true
Attachment #8601601 - Flags: review?(mwu)
Attachment #8601604 - Flags: review?(mwu)
Comment on attachment 8601604 [details] [diff] [review]
fix gonkDisplay stop anim condition. v2

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

::: widget/gonk/libdisplay/GonkDisplayJB.cpp
@@ +355,5 @@
> +{
> +    if (mBootAnimBuffer.get()) {
> +        mBootAnimBuffer = nullptr;
> +    }
> +    StopBootAnimation();

Here is the original patch.
I don't know why we put the stopAnim() in if block.
B2G might have bootAnim without hwc(mBootAnimBuffer is empty in this case), but we still need to stop anim.

-    if (mBootAnimBuffer.get()) {
-        StopBootAnimation();
-        mBootAnimBuffer = nullptr;
-    }
Component: GonkIntegration → Widget: Gonk
Product: Firefox OS → Core
Comment on attachment 8601604 [details] [diff] [review]
fix gonkDisplay stop anim condition. v2

Looks good, thanks
Attachment #8601604 - Flags: review?(mwu) → review+
Just a simple fix. Please land the attachment 8601604 [details] [diff] [review] to m-c.
Keywords: checkin-needed
Blocks: spark-device
No longer blocks: spark
Priority: -- → P1
Whiteboard: [spark]
https://hg.mozilla.org/mozilla-central/rev/c92c76073e9c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8602719 [details] [diff] [review]
fix the wrong stop bootAnim sequence. r=mwu

It's just a mistake for the use-after-free problem. We can't free the buffer before we stop anim.
Attachment #8602719 - Attachment description: fix the wrong stop bootAnim sequence. → fix the wrong stop bootAnim sequence. r=mwu
Attachment #8602719 - Flags: review+
(In reply to Jerry Shih[:jerry] (UTC+8) from comment #15)
> Comment on attachment 8602719 [details] [diff] [review]
> fix the wrong stop bootAnim sequence. r=mwu
> 
> It's just a mistake for the use-after-free problem. We can't free the buffer
> before we stop anim.
carry r+ from :mwu in attachment 8601604 [details] [diff] [review]
Backed out for causing B2G phone startup crashes.
https://hg.mozilla.org/mozilla-central/rev/5e13fa41d549

Re-landed as a folded patch.
https://hg.mozilla.org/mozilla-central/rev/0df9e0c692a3

Will trigger new device image nightlies in a bit.
The patches I provided before fix the flickering problem, but they broke the Bug 1156140 patch which want to extend the boot animation time.
Bug 1152135 changes some flow that make Bug 1156140 not work. This patch provided a clear interface to control bootAnimation stop time for both fbdev and hwc usage.
Attachment #8603296 - Flags: feedback?(chung)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8603296 [details] [diff] [review]
modify bootAnimation stop timing.

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

::: gfx/gl/GLContextProviderEGL.cpp
@@ +191,5 @@
>  CreateSurfaceForWindow(nsIWidget* widget, const EGLConfig& config) {
>      EGLSurface newSurface = EGL_NO_SURFACE;
>  
> +    #ifdef MOZ_WIDGET_GONK
> +        GetGonkDisplay()->RequestGLComposition();

If we use fbdev, we use framebuffer to play bootAnimation. Before we create gl context, we should stop bootAnimation to disconnect nativeWindow link.

::: widget/gonk/HwcComposer2D.cpp
@@ +673,5 @@
>  #if ANDROID_VERSION >= 17
>  bool
>  HwcComposer2D::TryHwComposition()
>  {
> +    GetGonkDisplay()->RequestHwcComposition();

If we use hwc, stop the bootAnim to prevent the racing problem for hwc::prepare() and hwc::set().

@@ +768,5 @@
>  
>  bool
>  HwcComposer2D::Render()
>  {
> +    GetGonkDisplay()->RequestHwcComposition();

If we don't use hwc, we will not call TryHwComposition(). Instead, we will use this function. Stop the bootAnim for the same racing problem.
Comment on attachment 8603296 [details] [diff] [review]
modify bootAnimation stop timing.

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

Looks good to me.
Attachment #8603296 - Flags: feedback?(chung) → feedback+
Attachment #8603296 - Flags: review?(mwu)
Do you mind filing a new bug for this? It's a bit confusing - your patch did fix this bug as stated.
(In reply to Michael Wu [:mwu] from comment #21)
> Do you mind filing a new bug for this? It's a bit confusing - your patch did
> fix this bug as stated.

I will create a new one and describe the reason again. THX~
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Attachment #8603296 - Attachment is obsolete: true
Attachment #8603296 - Flags: review?(mwu)
blocking-b2g: spark+ → 2.5+
Blocks: 1165161
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.