Last Comment Bug 834639 - Use blending for premultiplied alpha buffers
: Use blending for premultiplied alpha buffers
Status: RESOLVED FIXED
[hwc-blocker]
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla21
Assigned To: Diego Wilson [:diego]
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: 836891
  Show dependency treegraph
 
Reported: 2013-01-25 02:03 PST by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2013-02-15 10:37 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
wontfix
fixed
+
fixed
wontfix
fixed


Attachments
GPU rendered (116.80 KB, image/png)
2013-01-25 02:04 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details
hwc rendered (114.58 KB, image/png)
2013-01-25 02:04 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details
Don't use hwc when OVER compositing may be required (1.45 KB, patch)
2013-01-25 03:01 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Use PREMULT blending (1.28 KB, patch)
2013-01-25 03:19 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
dwilson: review+
cjones.bugs: approval‑mozilla‑b2g18+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-01-25 02:03:30 PST
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.
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-01-25 02:04:22 PST
Created attachment 706302 [details]
GPU rendered
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-01-25 02:04:45 PST
Created attachment 706303 [details]
hwc rendered
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-01-25 03:01:56 PST
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.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-01-25 03:19:51 PST
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.
Comment 5 Diego Wilson [:diego] 2013-01-25 18:08:45 PST
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
Comment 6 Diego Wilson [:diego] 2013-02-11 15:49:57 PST
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?
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-02-12 10:32:34 PST
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.
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-02-12 10:47:33 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/756df77be193
Comment 9 Ryan VanderMeulen [:RyanVM] 2013-02-12 18:33:08 PST
https://hg.mozilla.org/mozilla-central/rev/756df77be193
Comment 10 Lukas Blakk [:lsblakk] use ?needinfo 2013-02-13 11:41:20 PST
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?
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-02-13 12:31:03 PST
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 12 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2013-02-14 17:57:41 PST
Comment on attachment 706331 [details] [diff] [review]
Use PREMULT blending

Required to keep gecko upstream in sync with shipping code.
Comment 13 Diego Wilson [:diego] 2013-02-14 18:02:03 PST
FYI this is the hwc patch to disable dithering https://www.codeaurora.org/gitweb/quic/lf/?p=b2g/build.git;a=commit;h=f0192386d9f19c465120865588295a65f6ca8f1b

Note You need to log in before you can comment on or make changes to this bug.