Use blending for premultiplied alpha buffers

RESOLVED FIXED in Firefox 21

Status

()

Core
Graphics: Layers
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: cjones, Assigned: diego)

Tracking

Trunk
mozilla21
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

(Whiteboard: [hwc-blocker])

Attachments

(3 attachments, 1 obsolete attachment)

This is a complaint we've gotten from a partner.  I'm just now hooked up with a colorfill-enabled kernel so can see the bug too.

STR
 (1) Go to homescreen
 (2) Pan to first page of app icons
 (3) Release pan

There's a noticeable visual change when we switch from GPU to hwc compositing.  Upcoming screenshots will illustrate.

The change is in parts of the scene that should have alpha channels.  We're currently assuming hwc doesn't support OVER blending, so we have to fall back on the GPU whenever we see a buffer with an RGBA channel (in addition to layers with opacity).

I took a quick peek through the code and didn't see a de-optimize path for RGBA buffers (or solid colors with non-identity alpha).  Should be an easy fix.
Created attachment 706302 [details]
GPU rendered
Created attachment 706303 [details]
hwc rendered
Created attachment 706328 [details] [diff] [review]
Don't use hwc when OVER compositing may be required

Fixes bad renderings in homescreen and dialer.  Throws us off of hwc for camera which is likely a problem.  Let's chat tomorrow.
Attachment #706328 - Flags: feedback?(dwilson)
Summary: Buggy OVER compositing (?) with hwc → Use blending for premultiplied alpha buffers
Created attachment 706331 [details] [diff] [review]
Use PREMULT blending

This fixes the text rendering issue on the homescreen.  There's still a difference in rendering that seems to be caused by misrendering a color layer or mask.  On the right track though.
Attachment #706328 - Attachment is obsolete: true
Attachment #706328 - Flags: feedback?(dwilson)
Attachment #706331 - Flags: feedback?(dwilson)
Whiteboard: [hwc-blocker]
(Assignee)

Comment 5

5 years ago
Comment on attachment 706331 [details] [diff] [review]
Use PREMULT blending

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

I saw this in action with mwu. This seems to work fine on a 32 bit framebuffer but still looks off on a 16 bit framebuffer. I'll dive into the the HWC and below to see how it actually makes the calculation
(Assignee)

Updated

5 years ago
Assignee: nobody → dwilson
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Attachment #706331 - Flags: feedback?(dwilson)
Blocks: 836891
(Assignee)

Comment 6

4 years ago
Comment on attachment 706331 [details] [diff] [review]
Use PREMULT blending

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

@cjones,

Did a bit more research. Turns out this patch is correct. The reason it didn't completely solve the problem for RGB 565 is because HWC always tries to dither for a 16 bit frame buffer

I'll add patch libhwcomposer in CAF to avoid dithering so HWC frames are identical to GPU frames. In the meantime can this patch land on gecko?
Attachment #706331 - Flags: review+
Comment on attachment 706331 [details] [diff] [review]
Use PREMULT blending

This patch will be in the shipping product code.  We should have it in the upstream gecko repository.
Attachment #706331 - Flags: approval-mozilla-b2g18?
https://hg.mozilla.org/integration/mozilla-inbound/rev/756df77be193
https://hg.mozilla.org/mozilla-central/rev/756df77be193
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Would be great to know a risk assessment here - has this been tested?  Is there anything QA can do to verify this fix on nightlies before we uplift?  What do they need to do to confirm this maintains expectations on both 16/32 bit framebuffers?
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → affected
tracking-b2g18: --- → +
This is my own patch, so I'm trying to follow the guideline of not getting high on my own supply and approving it, but I will probably start doing that from now on.

As the approval request said, this patch will make it into shipping builds whether or not it's upstream in gecko.  It's better for everyone if it's upstream.

I'm aware the hwc setup is not widely understood, unfortunately I know it best on the Moz side.  So we need to either

 1. Drop the "high on your supply" guideline and I'll handle these approvals
 2. Note which bug this one blocks and have someone else set blocking+ based on that
 3. Spend a long time catching up on hwc background

I much prefer (1).  If no one specifically posts here that their jimmies will be rustled by that, along with an explanation why, that's the approach I'm going to take.
Comment on attachment 706331 [details] [diff] [review]
Use PREMULT blending

Required to keep gecko upstream in sync with shipping code.
Attachment #706331 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
(Assignee)

Comment 13

4 years ago
FYI this is the hwc patch to disable dithering https://www.codeaurora.org/gitweb/quic/lf/?p=b2g/build.git;a=commit;h=f0192386d9f19c465120865588295a65f6ca8f1b
https://hg.mozilla.org/releases/mozilla-b2g18/rev/e1c70fe5b142
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/799296a7ad4b
status-b2g18: affected → fixed
status-b2g18-v1.0.1: affected → fixed
status-firefox19: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → fixed
You need to log in before you can comment on or make changes to this bug.