Do not clip inside OcclusionTracker: Contributing Surfaces Edition.

Do not clip inside OcclusionTracker: Contributing Surfaces Edition.

This is a follow-up for https://codereview.chromium.org/23708021 where
we stopped clipping layers in the occlusion tracker. In this CL we stop
clipping contributing RenderSurfaces as well. The contents inside the
render surface are already clipped, and this is applied to the content
rect.

While we might be able to clip further in target space, we have to
blow the result back up into the content space again, so this should
produce about the same results.

While we're at it, fix a bug in QuadCuller where it always used the
surface draw transform to cull RenderPassDrawQuads even if the quad
was for the replica, which could lead to us culling the replica of
a surface when the surface itself is occluded. This was covered in
occlusion tracker tests, but not in QuadCuller tests.

R=enne
BUG=344962, 276725
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258431

[enne (OOO), 2014-03-19 22:53:17]

lgtm

https://codereview.chromium.org/205443002/diff/20001/cc/trees/quad_culler.cc
File cc/trees/quad_culler.cc (right):

https://codereview.chromium.org/205443002/diff/20001/cc/trees/quad_culler.cc#...
cc/trees/quad_culler.cc:71: gfx::Transform draw_transform = rpdq->is_replica
Replicas.  :(

[danakj, 2014-03-19 23:14:34]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-03-19 23:14:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/205443002/20001

[commit-bot: I haz the power, 2014-03-19 23:37:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-19 23:37:48]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[danakj, 2014-03-20 15:21:54]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-03-20 15:22:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/205443002/20001

[commit-bot: I haz the power, 2014-03-20 16:54:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 16:54:21]

Retried try job too often on win_rel for step(s) net_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[danakj, 2014-03-20 16:54:52]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-03-20 16:55:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/205443002/20001

[alokp, 2014-03-20 17:22:55]

You may want to tag crbug.com/276725 as well.

Last time I looked, it seemed to require more work than just removing the clips
from OcclusionTracker. Copying some stuff from my notes:

==============
LTH::CalculateDrawProperties does not clip RenderSurface::content_rect to the
viewport and target surface. For example:

root (bounds: 100 x 200)
child (bounds: 100 x 300, force_render_surface: true)

At the end of CalculateDrawProperties, I was expecting the child
RenderSurface::content_rect to be of size 100 x 200 - the same size as the root
render-surface. But apparently this clipping only happens in
OcclusionTracker::UnoccludedContributingSurfaceContentRect, not in
CalculateDrawProperties.

LTH only clips if a clip-rect or clip-to-bounds is explicitly set on the parent.
It does not clip contributing surfaces with the target surface bounds.
============

So if we are still not clipping the render surfaces during
CalculateDrawProperties, wouldn't we end with huge intermediate surfaces/FBOs?

[danakj, 2014-03-20 17:30:09]

On 2014/03/20 17:22:55, Alok Priyadarshi wrote:
> You may want to tag crbug.com/276725 as well.
> 
> Last time I looked, it seemed to require more work than just removing the
clips
> from OcclusionTracker. Copying some stuff from my notes:
> 
> ==============
> LTH::CalculateDrawProperties does not clip RenderSurface::content_rect to the
> viewport and target surface. For example:
> 
> root (bounds: 100 x 200)
> child (bounds: 100 x 300, force_render_surface: true)
> 
> At the end of CalculateDrawProperties, I was expecting the child
> RenderSurface::content_rect to be of size 100 x 200 - the same size as the
root
> render-surface. But apparently this clipping only happens in
> OcclusionTracker::UnoccludedContributingSurfaceContentRect, not in
> CalculateDrawProperties.
> 
> LTH only clips if a clip-rect or clip-to-bounds is explicitly set on the
parent.
> It does not clip contributing surfaces with the target surface bounds.
> ============
> 
> So if we are still not clipping the render surfaces during
> CalculateDrawProperties, wouldn't we end with huge intermediate surfaces/FBOs?

OcclusionTracker doesn't change the size of the surface textures with this
clipping. It changes the size of the DrawQuads that read from the surface
textures. If they aren't clipped by the FBO that they draw into, I don't think
that causes harm. Having the rect for these textures that we give to GL extend
outside the viewport's / target texture's bounds should not have a big perf
impact, GL can't draw out there.

(Actually today we don't partially cull RenderPassDrawQuads, so unless the the
surface is entirely clipped to 0 by its target, the code in OcclusionTracker
would have no effect. In the case a surface's content is entirely outside its
target FBO, I'd expect CalcDrawProps to not generate a surface at all.)

[danakj, 2014-03-20 17:30:54]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2014-03-20 17:31:15]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-03-20 17:35:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/205443002/20001

[commit-bot: I haz the power, 2014-03-20 22:11:58]

Change committed as 258431