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)
Tracking
(blocking-b2g:2.0+, firefox31 wontfix, firefox32 wontfix, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: bkelly, Assigned: bkelly)
References
Details
Attachments
(1 file)
2.03 KB,
patch
|
sushilchauhan
:
review+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Comment 1•11 years ago
|
||
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 2•11 years ago
|
||
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.
Assignee | ||
Comment 4•11 years ago
|
||
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.
Assignee | ||
Comment 6•11 years ago
|
||
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)
Comment 7•11 years ago
|
||
(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.
Comment 8•11 years ago
|
||
(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
Comment 9•11 years ago
|
||
The following is also related part to enable/disable HWC.
http://dxr.mozilla.org/mozilla-central/source/widget/gonk/nsWindow.cpp#167
Comment 10•11 years ago
|
||
(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.
Comment 11•11 years ago
|
||
(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+
Assignee | ||
Comment 12•11 years ago
|
||
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
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c= p=1 s= u=]
Assignee | ||
Updated•11 years ago
|
Priority: -- → P3
Comment 13•11 years ago
|
||
We need this fix, in particular for the planeAlpha fix.
Keywords: checkin-needed
Comment 14•11 years ago
|
||
hey Ben, could you provide a try link for this patches, thanks!
Keywords: checkin-needed
Assignee | ||
Comment 15•11 years ago
|
||
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
Comment 16•11 years ago
|
||
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.
Comment 17•11 years ago
|
||
Assignee: mwu → bkelly
Comment 18•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Updated•11 years ago
|
Whiteboard: [c= p=1 s= u=]
Updated•10 years ago
|
status-b2g-v2.0:
--- → affected
Comment 20•10 years ago
|
||
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)
Assignee | ||
Comment 21•10 years ago
|
||
(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)
Comment 22•10 years ago
|
||
[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)
Comment 23•10 years ago
|
||
We need the boot splash screen to work on KK => 2.0+
blocking-b2g: 2.0? → 2.0+
Comment 24•10 years ago
|
||
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → wontfix
status-firefox32:
--- → wontfix
status-firefox33:
--- → fixed
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•