Bug 90490 - [chromium] Use SkRegion clip for compositor painting
Summary: [chromium] Use SkRegion clip for compositor painting
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-03 12:38 PDT by Adrienne Walker
Modified: 2012-07-09 11:27 PDT (History)
5 users (show)

See Also:

Attachments
Patch (21.79 KB, patch)
2012-07-03 12:44 PDT, Adrienne Walker 		no flags Details | Formatted Diff | Diff
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 2012-07-03 12:38:06 PDT 
[chromium] Use SkRegion clip for compositor painting
Comment 1 Adrienne Walker 2012-07-03 12:44:14 PDT 
Created attachment 150663 [details]
Patch
Comment 2 Adrienne Walker 2012-07-03 12:49:29 PDT 
This still needs some investigation into how opaque region tracking is affected by this, but I just wanted to put up an initial patch.
Comment 3 Dana Jansens 2012-07-04 06:17:36 PDT 
Right now it would prevent any opaque tracking.

In OpaqueRegionSkia::didDraw:


    // Apply the current clip.
    SkRect deviceClipRect;
    if (!getDeviceClipAsRect(context, deviceClipRect))
        fillsBounds = false;
    else if (!targetRect.intersect(deviceClipRect))
        return;

This would cause fillsBounds = false. When that's the case it doesn't try to mark anything as opaque as it's not sure what parts to mark.

If the clip is a non-rect, but the bounds of the didDraw rect intersected with the clip is still a rect, then we could use that as "targetRect" and keep fillsBounds = true.

How to do this in an efficient way with the Skia API is something to explore.
Comment 4 Adrienne Walker 2012-07-09 11:26:57 PDT 
(In reply to comment #3)
> Right now it would prevent any opaque tracking.

Ugh.  Thanks for pointing that out.  I was hoping this would be a small change for some performance benefit, but if it breaks opaque tracking then maybe it's not worth it.

I could always just turn this on for already opaque layers (i.e. NCCH) which would handle the case I was trying to improve (http://crbug.com/135004), but that seems like a bit of a hack too.

I don't think this approach is right, so I'll just mark this as invalid.