Bug 52766 - [chromium] Composited render surfaces should allow writes to alpha channel.
Summary: [chromium] Composited render surfaces should allow writes to alpha channel.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-19 16:20 PST by Adrienne Walker
Modified: 2011-01-20 15:53 PST (History)
5 users (show)

See Also:

Attachments
Patch (1.57 KB, patch)
2011-01-19 16:21 PST, Adrienne Walker 		no flags Details | Formatted Diff | Diff
Patch (5.90 KB, patch)
2011-01-20 15:09 PST, Adrienne Walker 		jamesr: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adrienne Walker 2011-01-19 16:20:50 PST 
[chromium] Composited render surfaces should allow writes to alpha channel.
Comment 1 Adrienne Walker 2011-01-19 16:21:55 PST 
Created attachment 79518 [details]
Patch
Comment 2 Adrienne Walker 2011-01-19 16:22:40 PST 
See: http://crbug.com/70140
Comment 3 Adrienne Walker 2011-01-19 16:23:49 PST 
Because render surfaces weren't enabling writes to the alpha channel, it caused issues with blending the text in the search drop down here: http://bodybrowser.googlelabs.com
Comment 4 James Robinson 2011-01-19 16:28:15 PST 
Comment on attachment 79518 [details]
Patch

Why doesn't the comment in http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp#L260 apply to non-root layers?
Comment 5 Adrienne Walker 2011-01-19 16:44:12 PST 
(In reply to comment #4)
> (From update of attachment 79518 [details])
> Why doesn't the comment in http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp#L260 apply to non-root layers?

My hope would be that we are not turning on subpixel antialiasing when we're not rendering into the backbuffer.  Since layers can be repositioned, how would you know which subpixel the layer lines up with?

I don't know where I'd look to verify that this is the case.
Comment 6 Vangelis Kokkevis 2011-01-19 16:45:39 PST 
Comment on attachment 79518 [details]
Patch

LGMT.  Thanks for taking care of it.  Still needs a reviewer to bless it.
Comment 7 James Robinson 2011-01-19 16:47:02 PST 
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 79518 [details] [details])
> > Why doesn't the comment in http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp#L260 apply to non-root layers?
> 
> My hope would be that we are not turning on subpixel antialiasing when we're not rendering into the backbuffer.  Since layers can be repositioned, how would you know which subpixel the layer lines up with?
> 
> I don't know where I'd look to verify that this is the case.

Ah, of course!  The place to check is:

http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp#L261

and we do turn subpixel AA off for sublayers (currently for all sublayers, but we could revise this in the future for opaque layers or whatever).
Comment 8 James Robinson 2011-01-19 16:52:55 PST 
Comment on attachment 79518 [details]
Patch

You need to remove the (OOPS!) before landing this - preferably with an explanation of why there's no new test added.
Comment 9 James Robinson 2011-01-19 16:53:12 PST 
(In reply to comment #8)
> (From update of attachment 79518 [details])
> You need to remove the (OOPS!) before landing this - preferably with an explanation of why there's no new test added.

Or even more preferably with a list of tests that cover the change :)
Comment 10 W. James MacLean 2011-01-20 06:39:05 PST 
I tried this patch on a ToT Mac build, and it didn't seem to work for me. Is something also required on the Chromium side?
Comment 11 W. James MacLean 2011-01-20 10:16:59 PST 
(In reply to comment #10)
> I tried this patch on a ToT Mac build, and it didn't seem to work for me. Is something also required on the Chromium side?

False alarm - apparently I did not run the right image on the Mac, it works for me now. Sorry about that.
Comment 12 Adrienne Walker 2011-01-20 15:09:47 PST 
Created attachment 79657 [details]
Patch
Comment 13 James Robinson 2011-01-20 15:17:35 PST 
Comment on attachment 79657 [details]
Patch

Looks good!
Comment 14 Adrienne Walker 2011-01-20 15:53:54 PST 
Committed r76299: <http://trac.webkit.org/changeset/76299>