Summary: | [chromium] Remove surface damage client from occlusion tracker | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dana Jansens <danakj> | ||||||||
Component: | New Bugs | Assignee: | Dana Jansens <danakj> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | backer, cc-bugs, enne, jamesr, piman, shawnsingh, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 81222 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Dana Jansens
2012-03-15 11:22:08 PDT
I claim that the root surface damage rect will cull at least as well as the (bounding box of the) rect transformed into the current target surface. Proof by picture http://imgur.com/r5I1s Created attachment 132085 [details]
Patch
Comment on attachment 132085 [details] Patch Looks great... but I have to admit I'm not wrapping my brain fully around the proof of equivalence yet. But if someone else already feels confident about it, then that's good enough for me =) Otherwise I'll take a closer look later tonight. View in context: https://bugs.webkit.org/attachment.cgi?id=132085&action=review > Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:321 > { Maybe we should now rename this function and the variables inside it, since it is not really a "scissor" anymore. Its just the clipRect, right? > Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:324 > if (!layer->clipRect().isEmpty()) Since we're driving by, could we add a FIXME referring to the usesLayerClipping bug? https://bugs.webkit.org/show_bug.cgi?id=80622 (In reply to comment #3) > (From update of attachment 132085 [details]) > > Looks great... but I have to admit I'm not wrapping my brain fully around the proof of equivalence yet. But if someone else already feels confident about it, then that's good enough for me =) Otherwise I'll take a closer look later tonight. > > > View in context: https://bugs.webkit.org/attachment.cgi?id=132085&action=review > > > Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:321 > > { > > Maybe we should now rename this function and the variables inside it, since it is not really a "scissor" anymore. Its just the clipRect, right? Well, it's the clip rect + the target's content bounds. Which is sort of like the visibleLayerRect.. but in this case acts like a scissor. I think the name still makes sense under those terms? > > Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:324 > > if (!layer->clipRect().isEmpty()) > > Since we're driving by, could we add a FIXME referring to the usesLayerClipping bug? https://bugs.webkit.org/show_bug.cgi?id=80622 Sure! Created attachment 132097 [details]
Patch
Comment on attachment 132097 [details]
Patch
I think I'm convinced that scissoring in screen space is always going to cull just as much (if not more) than mapping the scissor onto a layer and taking the enclosing IntRect and scissoring with that. Thanks for the diagram.
It's nice not to have to need this client class. R=me.
Created attachment 132279 [details]
Patch
Comment on attachment 132279 [details] Patch Clearing flags on attachment: 132279 Committed r111021: <http://trac.webkit.org/changeset/111021> All reviewed patches have been landed. Closing bug. |