Bug 81242 – [chromium] Remove surface damage client from occlusion tracker

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dana Jansens]

[chromium] Remove surface damage client from occlusion tracker

[Dana Jansens]

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

[Dana Jansens]

Created attachment 132085 [details]
Patch

[Shawn Singh]

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

[Dana Jansens]

(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!

[Dana Jansens]

Created attachment 132097 [details]
Patch

[Adrienne Walker]

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.

[Dana Jansens]

Created attachment 132279 [details]
Patch

[WebKit Review Bot]

Comment on attachment 132279 [details]
Patch

Clearing flags on attachment: 132279

Committed r111021: <http://trac.webkit.org/changeset/111021>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.