Closed Bug 1171201 Opened 9 years ago Closed 9 years ago

[Aries][FTU] When the Date is changed in FTU, the carousel will overlay the tour

Categories

(Core :: Graphics, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

VERIFIED FIXED
blocking-b2g 2.5+
Tracking Status
b2g-master --- verified

People

(Reporter: dharris, Assigned: sotaro)

References

()

Details

(Keywords: smoketest, Whiteboard: [3.0-Daily-Testing][spark][systemsfe])

Attachments

(5 files, 2 obsolete files)

Attached file FTU Tour Logcat
Description:
Part of the date carousel can be seen overlaying the FTU tour on every page fo the tour. This issue will dissappear after waiting in the tour for a while, but is very noticable. The overlay also seems to flicker and linger for a while

Prerequisite: Do NOT have SIM in the device

Repro Steps:
1) Update a Xperia Z3 Compact (B2G) to 20150603164854
2) Tap next through the FTU until reaching the "Date&Time" page
3) Change the date (Month, Day, Year)
4) Continue through the FTU and then begin the tour


Actual:
Part of the date carousel can be seen overlaying the FTU tour


Expected:
THe tour displays normally without any overlay obstruction


Environmental Variables:
Device: Xperia Z3 Compact (B2G) 3.0
Build ID: 20150603164854
Gaia: ff80db87926a5c2769e158801090465b4ed117fa
Gecko: 196d99aabc27
Version: 41.0a1 (3.0)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0


Repro frequency: 5/5
See attached: Logcat, Video - https://youtu.be/smGek7siz7o
This issue does NOT occur on Flame 3.0

THe tour displays normally without any overlay obstruction

Environmental Variables:
Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
Build ID: 20150603010205
Gaia: 9b7ed13e0dee26b9f16ae5fbc076fa8bd588b256
Gecko: b0a507af2b4a
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 41.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
[Blocking Requested - why for this release]:
Regression from Flame behavior that fails smoke tests.
blocking-b2g: --- → spark?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
Keywords: smoketest
blocking-b2g: spark? → -
Flags: needinfo?(milan)
Don't have any Aries devices available for the graphics team; send some to Toronto.  Assuming we think this is graphics.
Flags: needinfo?(milan)
We did just get this device.
I think we'll find this is a dupe of 1171761, but I'll let you make that call.
Milan, can we get an official assignment from someone on your team?  Want to make sure we have a dev on point for this.  Thanks!
Flags: needinfo?(milan)
We currently only have Sotaro working on FxOS, and he is handling media as well, and all other devices.  I can assign, but it may take a while if it doesn't turn out to be a duplicate of bug 1171761.  Moving to Core->Graphics because of comment 5.
Assignee: nobody → sotaro.ikeda.g
Component: Gaia::First Time Experience → Graphics
Flags: needinfo?(milan)
Product: Firefox OS → Core
I confirmed to reproduce the problem on z3c.
Attachment #8621199 - Attachment description: Layer Tree dump - Showed Date Time → Layer Tree dump - During Showing Date Time
During Showing Date Time, "date time ui" is rendered by system app's layer. In attachment 8621199 [details], the layer overs almost all screen.

When the problem happens, attachment 8621201 [details] says that FTU's layer covers the almost all screen. But the system app's layer have a bit strange region. It has 2 regions. One is Top status bar. Another is a transparent region in the middle of the screen.

This strange region might cause the problem by lower level code.
When the problem happens, Composition was done by full hwc composition. When hwc is disabled, the problem did not happen.
There are 2 types of hwcs.
- copybit hwc : Use copybit hw for composition. All most recent Firefox OS device uses this hwc.
                Flame device also uses this.
- MDP hwc: Z3C has this type of hwc. Madai also have it.

Current almost all Firefox OS devices uses copybit hwc, therefore, code of MDP hwc is not tested well until now. Therefore, there might be a bug.
When the problem happens, system app's layer has the following 2 regions. It seems not correctly rendered by z3c's hwc. Instead, bounded rectangle seems to be rendered. The bounded rectangle is calculated by HwcComposer2D in gecko.
- (x=0, y=0, w=720, h=61); (x=83, y=264, w=436, h=402)

In copybit hwc's case, the region is handled by CopyBit::drawLayerUsingCopybit()
https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/tree/libhwcomposer/hwc_copybit.cpp?h=b2g_kk_3.5#n559

But mdp hwc's code seems not handle the region.
The patch fixed the problem on Z3C.
Comment on attachment 8621274 [details] [diff] [review]
patch - Do not compose by HWC when layer has multiple rectangle

mwu, can you feedback to the patch?
Attachment #8621274 - Flags: feedback?(mwu)
Sushil, diego, do you know why hwc_mdpcomp.cpp does not handle hwc_layer_1::visibleRegionScreen? Because of it, mdp hwc device can not render correctly when a layer has multiple rectangles.

As in Comment 16, hwc_copybit.cpp handle hwc_layer_1::visibleRegionScreen correctly.
Flags: needinfo?(sushilchauhan)
Flags: needinfo?(dwilson)
gerard-majax, to fix this bug, we need to apply change to /hardware/qcom/display. Aries uses caf repository for the code. What is a good way to apply the fix?

I am thinking the following is a way to go. How do you think about it?

- Change /hardware/qcom/display's repository from caf to https://github.com/mozilla-b2g/hardware_qcom_display
- Create "shinano" branch that have a fix of this bug.
Flags: needinfo?(lissyx+mozillians)
Comment on attachment 8621274 [details] [diff] [review]
patch - Do not compose by HWC when layer has multiple rectangle

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

::: libhwcomposer/hwc_mdpcomp.cpp
@@ +1230,5 @@
>      memset(&mCurrentFrame.drop, 0, sizeof(mCurrentFrame.drop));
>      mCurrentFrame.dropCount = 0;
>  
> +    for (size_t i = 0; i < list->numHwLayers; i++) {
> +        if (list->hwLayers[i].visibleRegionScreen.numRects > 1) {

Multiple rectangle in a layer could happen only when gecko is trying full hwc composition.
(In reply to Sotaro Ikeda [:sotaro] from comment #20)
> gerard-majax, to fix this bug, we need to apply change to
> /hardware/qcom/display. Aries uses caf repository for the code. What is a
> good way to apply the fix?
> 
> I am thinking the following is a way to go. How do you think about it?
> 
> - Change /hardware/qcom/display's repository from caf to
> https://github.com/mozilla-b2g/hardware_qcom_display
> - Create "shinano" branch that have a fix of this bug.

That would be the proper way to deal with it I think, but do we really have no other option? Also, could this be related to the multiple bugs where we have parts of the UI somehow leaking? Those are referenced in bug 1173213

My experience regarding the bugs referenced in bug 1173213 is that those started somewhere around mid/end of may, but otherwise the system was good before.
Flags: needinfo?(lissyx+mozillians) → needinfo?(sotaro.ikeda.g)
(In reply to Alexandre LISSY :gerard-majax from comment #22)
> 
> That would be the proper way to deal with it I think, but do we really have
> no other option?

off the top of my head, it is only solution.

> Also, could this be related to the multiple bugs where we
> have parts of the UI somehow leaking? Those are referenced in bug 1173213

I just confirmed that bug 1171761 is dup of this bug. hwc hal's bug should exist since its start. Therefore, I assume that layout module change caused multiple rectangles in a layer more often.
Flags: needinfo?(sotaro.ikeda.g)
See Also: 1169728
(In reply to Sotaro Ikeda [:sotaro] from comment #19)
> Sushil, diego, do you know why hwc_mdpcomp.cpp does not handle
> hwc_layer_1::visibleRegionScreen? Because of it, mdp hwc device can not
> render correctly when a layer has multiple rectangles.
> 
> As in Comment 16, hwc_copybit.cpp handle hwc_layer_1::visibleRegionScreen
> correctly.

Sushil tells me the HWC does not support multiple visible regions. I vaguely remember at some point we fell back to GPU if there were multiple visible rects.
Flags: needinfo?(dwilson)
Flags: needinfo?(sushilchauhan)
Thanks for the info!
Comment on attachment 8621274 [details] [diff] [review]
patch - Do not compose by HWC when layer has multiple rectangle

Diego, can you review the patch?
Attachment #8621274 - Flags: feedback?(mwu) → review?(dwilson)
Sotaro, this patch (Attachment #8621274 [details] [diff]) will cause GPU Composition fallback on Camera preview and recording use-cases because I remember those frames were also using multiple visible rectangles. Can you please confirm it with your patch ? GPU Composition will impact power numbers in Camera use cases.

Diego, can you please provide CAF link of the patch which we mainlined last time to address this issue ? I am not aware of current branch details.
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(dwilson)
(In reply to Sushil from comment #30)
> Sotaro, this patch (Attachment #8621274 [details] [diff] [diff]) will cause GPU
> Composition fallback on Camera preview and recording use-cases because I
> remember those frames were also using multiple visible rectangles. Can you
> please confirm it with your patch ? GPU Composition will impact power
> numbers in Camera use cases.
> 
> Diego, can you please provide CAF link of the patch which we mainlined last
> time to address this issue ? I am not aware of current branch details.

Yes, I confirmed that. It is nice if we could fix the problem more correctly. Can we have the caf's fix link?
Flags: needinfo?(sotaro.ikeda.g)
Comment on attachment 8621274 [details] [diff] [review]
patch - Do not compose by HWC when layer has multiple rectangle

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

Sotaro,

Rather than patch gonk, can't we just check for multiple visible rects in HwcComposer2D? It'll be easier to maintain than a gonk patch.
Attachment #8621274 - Flags: review?(dwilson)
OK, it's coming back to me. I added multiple visible regions to HWC to support HWC Camera in bug 832383
Flags: needinfo?(dwilson)
Diego, by bug 832383, HwcComposer2D stores multiple region to visibleRects. But it is not used by MDP hwc hal.
Flags: needinfo?(dwilson)
(In reply to Sotaro Ikeda [:sotaro] from comment #34)
> Diego, by bug 832383, HwcComposer2D stores multiple region to visibleRects.
> But it is not used by MDP hwc hal.

Can you check if the camera/camcorder use case still renders with HWC with your patch?
Flags: needinfo?(dwilson)
(In reply to Diego Wilson [:diego] from comment #35)
> (In reply to Sotaro Ikeda [:sotaro] from comment #34)
> > Diego, by bug 832383, HwcComposer2D stores multiple region to visibleRects.
> > But it is not used by MDP hwc hal.
> 
> Can you check if the camera/camcorder use case still renders with HWC with
> your patch?

Sometimes uses hwc, but majority of time, camera app was rendered by OpenGL composition.
Diego, we need the shot term fix as soon as possible for aries. Is there other ways than attachment 8621274 [details] [diff] [review] ?
Flags: needinfo?(dwilson)
(In reply to Sotaro Ikeda [:sotaro] from comment #37)
> Diego, we need the shot term fix as soon as possible for aries. Is there
> other ways than attachment 8621274 [details] [diff] [review] ?

Sotaro,

It seems we had a fix for this on CAF [1]. What kind of target is Aries?

[1] https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/commit/?id=1590d99e756b146b3ce20b02915f5d4d5e0c8100
Flags: needinfo?(dwilson)
(In reply to Diego Wilson [:diego] from comment #38)
> 
> It seems we had a fix for this on CAF [1]. What kind of target is Aries?
> 
> [1]
> https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/
> commit/?id=1590d99e756b146b3ce20b02915f5d4d5e0c8100

The code is already exit on aries code. The following part seems not correct. When the problem happened, the problematic layer had HWC_BLENDING_PREMULT instead of HWC_BLENDING_NONE

+                if (layer->blending == HWC_BLENDING_NONE) {
+                    // MDP Overlay does not support multiple visible rects.
+                    // In Video use case, do not allow MDP Composition on a
+                    // layer, if framework confirms with Opaque content flag
+                    // hence blending mode has been set to HWC_BLENDING_NONE
+                    list->hwLayers[i].flags |= HWC_SKIP_LAYER;
+                }
The patch fixed the problem.
Diego, how about attachment 8622645 [details] [diff] [review] ?
Flags: needinfo?(dwilson)
Comment on attachment 8622645 [details] [diff] [review]
patch - Skip a layer that has multiple rects

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

::: libhwcomposer/hwc_utils.cpp
@@ +840,5 @@
>          if ((ctx->mMDP.version > qdutils::MDP_V4_0) &&
>              (layer->visibleRegionScreen.numRects > 1) &&
>              !(layer->flags & HWC_COLOR_FILL)) {
>              if (ctx->listStats[dpy].yuvCount) {
> +                list->hwLayers[i].flags |= HWC_SKIP_LAYER;

With this patch, is Camera preview/recording using partial HWC Composition ? I mean YUV layer is composed by HWC and this layer is composed by GPU.

You can remove "ctx->listStats[dpy].yuvCount" check.
Apply the comment.
Attachment #8622645 - Attachment is obsolete: true
> 
> ::: libhwcomposer/hwc_utils.cpp
> @@ +840,5 @@
> >          if ((ctx->mMDP.version > qdutils::MDP_V4_0) &&
> >              (layer->visibleRegionScreen.numRects > 1) &&
> >              !(layer->flags & HWC_COLOR_FILL)) {
> >              if (ctx->listStats[dpy].yuvCount) {
> > +                list->hwLayers[i].flags |= HWC_SKIP_LAYER;
> 
> With this patch, is Camera preview/recording using partial HWC Composition ?
> I mean YUV layer is composed by HWC and this layer is composed by GPU.
> 
> You can remove "ctx->listStats[dpy].yuvCount" check.

Thanks for the comment. Yes, preview used partial HWC Composition by applying the patch.
Attachment #8623115 - Flags: review?(dwilson)
Flags: needinfo?(dwilson)
Attachment #8623115 - Flags: review?(dwilson) → review?(sushilchauhan)
Attachment #8623115 - Flags: review?(sushilchauhan) → review+
Diego, is it possible to merge to caf git://codeaurora.org/platform/hardware/qcom/display and uplift to caf/b2g_kk_3.5 branch?
Flags: needinfo?(dwilson)
(In reply to Sotaro Ikeda [:sotaro] from comment #45)
> Diego, is it possible to merge to caf
> git://codeaurora.org/platform/hardware/qcom/display and uplift to
> caf/b2g_kk_3.5 branch?

That branch is not getting updates anymore.
Flags: needinfo?(dwilson)
(In reply to Diego Wilson [:diego] from comment #46)
> (In reply to Sotaro Ikeda [:sotaro] from comment #45)
> > Diego, is it possible to merge to caf
> > git://codeaurora.org/platform/hardware/qcom/display and uplift to
> > caf/b2g_kk_3.5 branch?
> 
> That branch is not getting updates anymore.

OK, I am going to do comment 20.
Attachment #8621274 - Attachment is obsolete: true
created shinano branch to mozilla-b2g/hardware_qcom_display.
  https://github.com/mozilla-b2g/hardware_qcom_display/tree/shinano
Attached file link to pull request
Attachment #8625640 - Attachment is obsolete: true
Attachment #8625640 - Attachment is obsolete: false
Attachment #8625640 - Flags: review?(lissyx+mozillians)
Attachment #8625640 - Flags: review?(lissyx+mozillians) → review+
(In reply to Sotaro Ikeda [:sotaro] from comment #51)
> https://github.com/mozilla-b2g/b2g-manifest/commit/
> d15f471e742d33b3e5ceace59138408c7b5526f5

Can we resolve this bug Sotaro, or is there more work to do?
Yes, this is resolved.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
This issue is verified fixed on the latest Spark Aries 2.5 build.
Date and Time carousels do not overlap other FTU screens.  FTU appears appropriately.

Environmental Variables:
Device: Aries 2.5
BuildID: 20150812231434
Gaia: 52f3ea58df38e5427f6afeb636bc6ad01d24022f
Gecko: 7649ffe28b67aa2dad0f67ea01500c0ff91b2bac
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Smoketest blocker
blocking-b2g: - → 2.5+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: