Closed Bug 980568 Opened 11 years ago Closed 11 years ago

GonkDisplayJB::Post() does not set planeAlpha correctly on KK

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.0+, firefox31 wontfix, firefox32 wontfix, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
mozilla33
blocking-b2g 2.0+
Tracking Status
firefox31 --- wontfix
firefox32 --- wontfix
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(1 file)

In GonkDisplayJB::Post() we have this code: #if ANDROID_VERSION == 18 mList->hwLayers[1].planeAlpha = 0xFF; #endif It seems that would leave planeAlpha uninitialized on KK. Also, should mList->hwLayers[1].blending be set to HWC_BLENDING_NONE if we are treating it as an opaque layer?
Here's a patch changing those two things. I don't see a significant impact on my nexus-5, but Post() does not seem to get called too often.
Attachment #8387121 - Flags: review?(sotaro.ikeda.g)
Comment on attachment 8387121 [details] [diff] [review] Set planeAlpha and blending for opaque layer in GonkDisplayJB::Post() on KK. I am not a correct person to review the patch forward to Sushil.
Attachment #8387121 - Flags: review?(sotaro.ikeda.g) → review?(sushilchauhan)
Comment on attachment 8387121 [details] [diff] [review] Set planeAlpha and blending for opaque layer in GonkDisplayJB::Post() on KK. Review of attachment 8387121 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/libdisplay/GonkDisplayJB.cpp @@ +247,5 @@ > mList->hwLayers[1].hints = 0; > mList->hwLayers[1].flags = 0; > mList->hwLayers[1].handle = buf; > mList->hwLayers[1].transform = 0; > + mList->hwLayers[1].blending = HWC_BLENDING_NONE; Is this change really helping ? I asked because GonkDisplayJB::Post() is only used during Boot Animation. After boot animation, FB layer of GPU Composition is always handled in HwcComposer2D::Render(). Is any other specific use-case uses GonkDisplayJB to post frames ? Can you please make this change in HwcComposer2D::Prepare() and see if it helps you. Here: mList->hwLayers[idx].blending = HWC_BLENDING_PREMULT; And yes, it makes sense to keep FB layer as BLENDING_NONE in GPU Composition since it is the only layer in frame.
You are correct the change to GonkDisplayJB did not help much. Just seemed wrong to me so thought I would point it out. I will try the HwcComposer2D::Prepare() change you request later this evening.
Flags: needinfo?(bkelly)
We can set HWC_BLENDING_NONE on FB layer only in case of GPU Composition. But in HWC composition, we need it to be HWC_BLENDING_PREMULT.
Changing HwcComposer2D::Prepare() to use HWC_BLENDING_NONE causes the settings app to fail to paint at all. I get the launch animation, but the app does not appear.
Flags: needinfo?(bkelly)
(In reply to Sushil from comment #3) > Comment on attachment 8387121 [details] [diff] [review] > Set planeAlpha and blending for opaque layer in GonkDisplayJB::Post() on KK. > > Review of attachment 8387121 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/gonk/libdisplay/GonkDisplayJB.cpp > @@ +247,5 @@ > > mList->hwLayers[1].hints = 0; > > mList->hwLayers[1].flags = 0; > > mList->hwLayers[1].handle = buf; > > mList->hwLayers[1].transform = 0; > > + mList->hwLayers[1].blending = HWC_BLENDING_NONE; > > Is this change really helping ? I asked because GonkDisplayJB::Post() is > only used during Boot Animation. After boot animation, FB layer of GPU > Composition is always handled in HwcComposer2D::Render(). GonkDisplayJB::Post() is used on nexus-4 and nexus-5 build. AOSP's HWC does not support RBswap, therefore default b2g build automatically disable HWC. Then GonkDisplayJB::Post() is used for composition.
(In reply to Sotaro Ikeda [:sotaro] from comment #7) > GonkDisplayJB::Post() is used on nexus-4 and nexus-5 build. AOSP's HWC does > not support RBswap, therefore default b2g build automatically disable HWC. > Then GonkDisplayJB::Post() is used for composition. Correction: nexus-4 and nexus-5 do not support RB swap and color layer. From the following code, No Color layer support disable HWC on them. http://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.js#558
The following is also related part to enable/disable HWC. http://dxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#167
(In reply to Ben Kelly [:bkelly] from comment #6) > Changing HwcComposer2D::Prepare() to use HWC_BLENDING_NONE causes the > settings app to fail to paint at all. I get the launch animation, but the > app does not appear. Then, let us not change it to HWC_BLENDING_NONE to keep switching between GPU and HWC compositions seem less. Android also keeps blending for FB layer as HWC_BLENDING_PREMULT always. BTW, did you test on Nexus 5 ? Because as per Comment 8, it should not use HwcComposer2D::Prepare() then.
(In reply to Sotaro Ikeda [:sotaro] from comment #8) > (In reply to Sotaro Ikeda [:sotaro] from comment #7) > > GonkDisplayJB::Post() is used on nexus-4 and nexus-5 build. AOSP's HWC does > > not support RBswap, therefore default b2g build automatically disable HWC. > > Then GonkDisplayJB::Post() is used for composition. > > Correction: > nexus-4 and nexus-5 do not support RB swap and color layer. From the > following code, No Color layer support disable HWC on them. > > http://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings. > js#558 I am OK with setting it to HWC_BLENDING_NONE in GonkDisplayJB since it always enforces GPU Composition path. Please make sure we do not see any issue with this change on Nexus-4, Nexus-5 & devices using HWC.
Attachment #8387121 - Flags: review?(sushilchauhan) → review+
Oops, I lost track of this since I didn't have it assigned to myself. I'll do the testing requested in comment 11 this week.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Whiteboard: [c= p=1 s= u=]
Priority: -- → P3
We need this fix, in particular for the planeAlpha fix.
Keywords: checkin-needed
hey Ben, could you provide a try link for this patches, thanks!
Keywords: checkin-needed
Here is a try build: https://tbpl.mozilla.org/?tree=Try&rev=8df049ab07f7 I have not built or tested this patch in many months, so I am unsure if it will be green. Michael, can you follow-up on this to see it landed? I'm sorry, but I need to investigate some other bustage and won't have time for this today.
Assignee: bkelly → mwu
This patch can't help but go green. We don't have any tests running on JB/KK, so as long as the build succeeds, it'll work.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Whiteboard: [c= p=1 s= u=]
Hi Ben, Is it possible you patch can uplift to 2.0 branch? Bug 1054168 needs this patch to be solved. Thanks.
Flags: needinfo?(bkelly)
(In reply to Vincent Liu[:vliu] from comment #20) > Is it possible you patch can uplift to 2.0 branch? Bug 1054168 needs this > patch to be solved. Thanks. That should work. I think you can just set the appropriate approval flag on the patch.
Flags: needinfo?(bkelly) → needinfo?(vliu)
[Blocking Requested - why for this release]: Any KK devices like Flame-KK need this patch on v2.0 branch.
blocking-b2g: --- → 2.0?
Flags: needinfo?(vliu)
We need the boot splash screen to work on KK => 2.0+
blocking-b2g: 2.0? → 2.0+
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: