[webkit-reviews] review denied: [Bug 82251] [chromium] Layers should should know their visibility outside their content bounds : [Attachment 152661] Patch

[bugzilla-daemon at webkit.org]

Adrienne Walker <enne at google.com> has denied Eric Penner
<epenner at chromium.org>'s request for review:
Bug 82251: [chromium] Layers should should know their visibility outside their
content bounds
https://bugs.webkit.org/show_bug.cgi?id=82251

Attachment 152661: Patch
https://bugs.webkit.org/attachment.cgi?id=152661&action=review

------- Additional Comments from Adrienne Walker <enne at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=152661&action=review


I think unclippedVisibleContentRect is a reasonable name if there's a comment
with it in LayerChromium.h that says something like it's the projection of the
visible rect into content space, but that it's not clipped to the layer's
content bounds.

This seriously needs some tests.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:57
> +    if (clip == ClipVisibleRect)
> +	   minimalSurfaceRect.intersect(layerInSurfaceSpace);

Can you convince me that this does anything? I would have figured that the
shift to projectClippedRect would make this particular intersection a no-op
and, if anything, this only increases the accuracy of the resulting visible
rect, unclipped or not.

Do you also not need to avoid the other intersection with the layerBoundRect
below on line 65? Wouldn't that make the unclipped visible rect empty for
anything that didn't intersect with the content rect? I thought that's what the
whole point of this was.