Do not clip inside OcclusionTracker.

cc/trees/occlusion_tracker.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/occlusion_tracker.h"
 
 #include <algorithm>
 
 #include "cc/base/math_util.h"
 #include "cc/debug/overdraw_metrics.h"
 #include "cc/layers/layer.h"
 #include "cc/layers/layer_impl.h"
 #include "cc/layers/render_surface.h"
 #include "cc/layers/render_surface_impl.h"
 #include "ui/gfx/quad_f.h"
 #include "ui/gfx/rect_conversions.h"
 
 namespace cc {
 
 template <typename LayerType, typename RenderSurfaceType>
 OcclusionTrackerBase<LayerType, RenderSurfaceType>::OcclusionTrackerBase(
-    gfx::Rect screen_space_clip_rect, bool record_metrics_for_frame)
-    : screen_space_clip_rect_(screen_space_clip_rect),
-      overdraw_metrics_(OverdrawMetrics::Create(record_metrics_for_frame)),
+    bool record_metrics_for_frame)
+    : overdraw_metrics_(OverdrawMetrics::Create(record_metrics_for_frame)),
       prevent_occlusion_(false),
       occluding_screen_space_rects_(NULL),
       non_occluding_screen_space_rects_(NULL) {}
 
 template <typename LayerType, typename RenderSurfaceType>
 OcclusionTrackerBase<LayerType, RenderSurfaceType>::~OcclusionTrackerBase() {}
 
 template <typename LayerType, typename RenderSurfaceType>
 void OcclusionTrackerBase<LayerType, RenderSurfaceType>::EnterLayer(
     const LayerIteratorPosition<LayerType>& layer_iterator,
@@ skipping 17 matching lines @@
     MarkOccludedBehindLayer(layer_iterator.current_layer);
   // TODO(danakj): This should be done when entering the contributing surface,
   // but in a way that the surface's own occlusion won't occlude itself.
   else if (layer_iterator.represents_contributing_render_surface)
     LeaveToRenderTarget(render_target);
 
   prevent_occlusion_ = false;
 }
 
 template <typename RenderSurfaceType>
-static gfx::Rect ScreenSpaceClipRectInTargetSurface(
-    const RenderSurfaceType* target_surface, gfx::Rect screen_space_clip_rect) {
-  gfx::Transform inverse_screen_space_transform(
-      gfx::Transform::kSkipInitialization);
-  if (!target_surface->screen_space_transform().GetInverse(
-          &inverse_screen_space_transform))
-    return target_surface->content_rect();
-
-  return gfx::ToEnclosingRect(MathUtil::ProjectClippedRect(
-      inverse_screen_space_transform, screen_space_clip_rect));
-}
-
-template <typename RenderSurfaceType>
 static Region TransformSurfaceOpaqueRegion(const Region& region,
                                            bool have_clip_rect,
                                            gfx::Rect clip_rect_in_new_target,
                                            const gfx::Transform& transform) {
   if (region.IsEmpty())
     return Region();
 
   // Verify that rects within the |surface| will remain rects in its target
   // surface after applying |transform|. If this is true, then apply |transform|
   // to each rect within |region| in order to transform the entire Region.
@@ skipping 344 matching lines @@
   Region opaque_contents = layer->VisibleContentOpaqueRegion();
   if (opaque_contents.IsEmpty())
     return;
 
   DCHECK(layer->visible_content_rect().Contains(opaque_contents.bounds()));
 
   // TODO(danakj): Find a rect interior to each transformed quad.
   if (!layer->draw_transform().Preserves2dAxisAlignment())
     return;
 
-  gfx::Rect clip_rect_in_target = ScreenSpaceClipRectInTargetSurface(
-      layer->render_target()->render_surface(), screen_space_clip_rect_);
-  if (layer->is_clipped()) {
-    clip_rect_in_target.Intersect(layer->clip_rect());
-  } else {
-    clip_rect_in_target.Intersect(
-        layer->render_target()->render_surface()->content_rect());
-  }
-
   for (Region::Iterator opaque_content_rects(opaque_contents);
        opaque_content_rects.has_rect();
        opaque_content_rects.next()) {
     bool clipped;
     gfx::QuadF transformed_quad = MathUtil::MapQuad(
         layer->draw_transform(),
         gfx::QuadF(opaque_content_rects.rect()),
         &clipped);
     gfx::Rect transformed_rect =
         gfx::ToEnclosedRect(transformed_quad.BoundingBox());
     DCHECK(!clipped);  // We only map if the transform preserves axis alignment.
-    transformed_rect.Intersect(clip_rect_in_target);

[danakj, 2013/09/10 14:36:37]

Not intersecting with the clip rect means you'll mark things as occluding even
tho they are being clipped. I don't think you can change this function and not
get wrong results. Unless you're planning to do the clipping in
VisibleContentOpaqueRegion()? Or somewhere else?

[alokp, 2013/09/10 18:15:46]

VisibleContentOpaqueRegion is already clipped. It is computed using
layer::draw_properties::visible_content_rect, which is clipped in
CalculateDrawProps. No?

We are computing enclosed-rect in multiple places, which made me suspicious, but
since we only consider axis-aligned occluding layers, I think this is correct.

Having said that, leaving the clipping here is fine. This is not what slows us
down. Clipping query-rects do because we cannot early out.

[danakj, 2013/09/10 18:21:10]

The visible content rect is in content space. So it is clipped to a bounding box
around the actual clip rect, which lives in target space. You can't clip things
in content space and expect them to be tight.

[alokp, 2013/09/10 20:51:27]

I do not understand. I am looking at this:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...

The clipping happens in the target-space and projected back. If draw-transform
preserves axis alignment, why would it not be tight?

[danakj, 2013/09/10 20:54:24]

Yes it would, you're right! But if it is not axis-aligned.. then you need to do
clipping in the target space. The inputs to this function are in content space,
so you can't just use those, or you'll mark occlusion in the target where there
is actually none.

     if (transformed_rect.width() < minimum_tracking_size_.width() &&
         transformed_rect.height() < minimum_tracking_size_.height())
       continue;
     stack_.back().occlusion_from_inside_target.Union(transformed_rect);
 
     if (!occluding_screen_space_rects_)
       continue;
 
     // Save the occluding area in screen space for debug visualization.
     gfx::QuadF screen_space_quad = MathUtil::MapQuad(
@@ skipping 12 matching lines @@
   Region non_opaque_contents =
       SubtractRegions(gfx::Rect(layer->content_bounds()), opaque_contents);
   for (Region::Iterator non_opaque_content_rects(non_opaque_contents);
        non_opaque_content_rects.has_rect();
        non_opaque_content_rects.next()) {
     // We've already checked for clipping in the MapQuad call above, these calls
     // should not clip anything further.
     gfx::Rect transformed_rect = gfx::ToEnclosedRect(
         MathUtil::MapClippedRect(layer->draw_transform(),
                                  gfx::RectF(non_opaque_content_rects.rect())));
-    transformed_rect.Intersect(clip_rect_in_target);

[danakj, 2013/09/10 14:36:37]

same point here

     if (transformed_rect.IsEmpty())
       continue;
 
     bool clipped;
     gfx::QuadF screen_space_quad = MathUtil::MapQuad(
         layer->render_target()->render_surface()->screen_space_transform(),
         gfx::QuadF(transformed_rect),
         &clipped);
     // TODO(danakj): Store the quad in the debug info instead of the bounding
     // box.
     gfx::Rect screen_space_rect =
         gfx::ToEnclosedRect(screen_space_quad.BoundingBox());
     non_occluding_screen_space_rects_->push_back(screen_space_rect);
   }
 }
 
 template <typename LayerType, typename RenderSurfaceType>
 bool OcclusionTrackerBase<LayerType, RenderSurfaceType>::Occluded(
     const LayerType* render_target,
     gfx::Rect content_rect,
     const gfx::Transform& draw_transform,
     bool impl_draw_transform_is_unknown,
-    bool is_clipped,
-    gfx::Rect clip_rect_in_target,
     bool* has_occlusion_from_outside_target_surface) const {
   if (has_occlusion_from_outside_target_surface)
     *has_occlusion_from_outside_target_surface = false;
   if (prevent_occlusion_)
     return false;
 
   DCHECK(!stack_.empty());
   if (stack_.empty())
     return false;
   if (content_rect.IsEmpty())
     return true;
   if (impl_draw_transform_is_unknown)
     return false;
 
   // For tests with no render target.
   if (!render_target)
     return false;
 
   DCHECK_EQ(render_target->render_target(), render_target);
   DCHECK(render_target->render_surface());
   DCHECK_EQ(render_target, stack_.back().target);
 
+  if (stack_.back().occlusion_from_inside_target.IsEmpty() &&
+      stack_.back().occlusion_from_outside_target.IsEmpty()) {
+    return false;
+  }
+
   gfx::Transform inverse_draw_transform(gfx::Transform::kSkipInitialization);
   if (!draw_transform.GetInverse(&inverse_draw_transform))
     return false;
 
   // Take the ToEnclosingRect at each step, as we want to contain any unoccluded
   // partial pixels in the resulting Rect.
   Region unoccluded_region_in_target_surface = gfx::ToEnclosingRect(
       MathUtil::MapClippedRect(draw_transform, gfx::RectF(content_rect)));
-  // Layers can't clip across surfaces, so count this as internal occlusion.
-  if (is_clipped)
-    unoccluded_region_in_target_surface.Intersect(clip_rect_in_target);
   unoccluded_region_in_target_surface.Subtract(
       stack_.back().occlusion_from_inside_target);
   gfx::RectF unoccluded_rect_in_target_surface_without_outside_occlusion =
       unoccluded_region_in_target_surface.bounds();
   unoccluded_region_in_target_surface.Subtract(
       stack_.back().occlusion_from_outside_target);
 
-  // Treat other clipping as occlusion from outside the surface.
-  // TODO(danakj): Clip to visibleContentRect?
-  unoccluded_region_in_target_surface.Intersect(
-      render_target->render_surface()->content_rect());

[enne (OOO), 2013/09/09 23:37:39]

I think you shouldn't remove this line.

[alokp, 2013/09/10 13:31:44]

I think we should. This is also a form of clipping that has already happened
during CalculateDrawProps. This function should return the same result in both
scenarios:

1. Occluded region is empty. We early-out in this case and do not bother
clipping with anything - clip-rect, target-surface-rect, or screen-rect.

2. Occluded region is non-empty, but does not occlude the given rect. Here if we
clip against the target-surface-rect, you might get a different result.

This was the motivation for not doing any kind of clipping here.

-  unoccluded_region_in_target_surface.Intersect(
-      ScreenSpaceClipRectInTargetSurface(render_target->render_surface(),
-                                         screen_space_clip_rect_));
-
   gfx::RectF unoccluded_rect_in_target_surface =
       unoccluded_region_in_target_surface.bounds();
 
   if (has_occlusion_from_outside_target_surface) {
     // Check if the unoccluded rect shrank when applying outside occlusion.
     *has_occlusion_from_outside_target_surface = !gfx::SubtractRects(

[danakj, 2013/09/10 14:36:37]

Have you thought through how we can preserve this bit?

[alokp, 2013/09/10 18:15:46]

Sorry I have not looked at occlusion-from-outside closely. Is
occlusion-from-outside the same as clipping or something more?

[danakj, 2013/09/10 18:21:10]

It is different. This bit is false if nothing affected the drawing of the target
surface for this quad, such that the target surface texture is complete. In
other words, this is used for surface caching and should be true only if from
the perspective of this quad, the surface texture can be reused forever and
positioned anywhere on screen and remain correct.

If, instead we clipped the quad by something "outside the surface" ie not part
of the surface, then there are invalid pixels in the surface texture that are
not part of the current frame, but may be part of future frames.

This whole feature is disabled right now, but still covered by unit tests, with
some intention of turning it on again. As we continue to work around it, this
gets more awkward..

         unoccluded_rect_in_target_surface_without_outside_occlusion,
         unoccluded_rect_in_target_surface).IsEmpty();
   }
 
   return unoccluded_rect_in_target_surface.IsEmpty();
 }
 
 template <typename LayerType, typename RenderSurfaceType>
 gfx::Rect OcclusionTrackerBase<LayerType, RenderSurfaceType>::
     UnoccludedContentRect(
         const LayerType* render_target,
         gfx::Rect content_rect,
         const gfx::Transform& draw_transform,
         bool impl_draw_transform_is_unknown,
-        bool is_clipped,
-        gfx::Rect clip_rect_in_target,
         bool* has_occlusion_from_outside_target_surface) const {
   if (has_occlusion_from_outside_target_surface)
     *has_occlusion_from_outside_target_surface = false;
   if (prevent_occlusion_)
     return content_rect;
 
   DCHECK(!stack_.empty());
   if (stack_.empty())
     return content_rect;
   if (content_rect.IsEmpty())
     return content_rect;
   if (impl_draw_transform_is_unknown)
     return content_rect;
 
   // For tests with no render target.
   if (!render_target)
     return content_rect;
 
   DCHECK_EQ(render_target->render_target(), render_target);
   DCHECK(render_target->render_surface());
   DCHECK_EQ(render_target, stack_.back().target);
 
+  if (stack_.back().occlusion_from_inside_target.IsEmpty() &&
+      stack_.back().occlusion_from_outside_target.IsEmpty()) {
+    return content_rect;
+  }
+
   gfx::Transform inverse_draw_transform(gfx::Transform::kSkipInitialization);
   if (!draw_transform.GetInverse(&inverse_draw_transform))
     return content_rect;
 
   // Take the ToEnclosingRect at each step, as we want to contain any unoccluded
   // partial pixels in the resulting Rect.
   Region unoccluded_region_in_target_surface = gfx::ToEnclosingRect(
       MathUtil::MapClippedRect(draw_transform, gfx::RectF(content_rect)));
-  // Layers can't clip across surfaces, so count this as internal occlusion.
-  if (is_clipped)
-    unoccluded_region_in_target_surface.Intersect(clip_rect_in_target);
   unoccluded_region_in_target_surface.Subtract(
       stack_.back().occlusion_from_inside_target);
   gfx::RectF unoccluded_rect_in_target_surface_without_outside_occlusion =
       unoccluded_region_in_target_surface.bounds();
   unoccluded_region_in_target_surface.Subtract(
       stack_.back().occlusion_from_outside_target);
 
-  // Treat other clipping as occlusion from outside the surface.
-  // TODO(danakj): Clip to visibleContentRect?
-  unoccluded_region_in_target_surface.Intersect(
-      render_target->render_surface()->content_rect());

[enne (OOO), 2013/09/09 23:37:39]

Same here.

-  unoccluded_region_in_target_surface.Intersect(
-      ScreenSpaceClipRectInTargetSurface(render_target->render_surface(),
-                                         screen_space_clip_rect_));
-
   gfx::RectF unoccluded_rect_in_target_surface =
       unoccluded_region_in_target_surface.bounds();
   gfx::Rect unoccluded_rect = gfx::ToEnclosingRect(
       MathUtil::ProjectClippedRect(inverse_draw_transform,
                                    unoccluded_rect_in_target_surface));
   unoccluded_rect.Intersect(content_rect);
 
   if (has_occlusion_from_outside_target_surface) {
     // Check if the unoccluded rect shrank when applying outside occlusion.
     *has_occlusion_from_outside_target_surface = !gfx::SubtractRects(
@@ skipping 48 matching lines @@
   // surface, so only things outside the surface can occlude it. That occlusion
   // is found just below the top of the stack (if it exists).
   bool has_occlusion = stack_.size() > 1;
 
   // Take the ToEnclosingRect at each step, as we want to contain any unoccluded
   // partial pixels in the resulting Rect.
   Region unoccluded_region_in_target_surface = gfx::ToEnclosingRect(
       MathUtil::MapClippedRect(draw_transform, gfx::RectF(content_rect)));
   // Layers can't clip across surfaces, so count this as internal occlusion.
   if (surface->is_clipped())
     unoccluded_region_in_target_surface.Intersect(surface->clip_rect());

[danakj, 2013/09/10 14:36:37]

This too?

[alokp, 2013/09/10 18:15:46]

I have not looked at optimizing surface-rects yet. Focusing on layer-rects for
now. Can we do it in a subsequent patch?

[danakj, 2013/09/10 18:21:10]

I think you'll need to handle more than just tile layers anyways, so I don't
expect this should add much effort for you.. but if you like that might be
reasonable. But then you should not be removing the code at line 720, this
function should not be changed at all if you're not changing it fully.

[alokp, 2013/09/10 20:51:27]

OK. I will look at them once we resolve all the layer issues.

   if (has_occlusion) {
     const StackObject& second_last = stack_[stack_.size() - 2];
     unoccluded_region_in_target_surface.Subtract(
         second_last.occlusion_from_inside_target);
   }
   gfx::RectF unoccluded_rect_in_target_surface_without_outside_occlusion =
       unoccluded_region_in_target_surface.bounds();
   if (has_occlusion) {
     const StackObject& second_last = stack_[stack_.size() - 2];
     unoccluded_region_in_target_surface.Subtract(
         second_last.occlusion_from_outside_target);
   }
 
   // Treat other clipping as occlusion from outside the target surface.
   unoccluded_region_in_target_surface.Intersect(
       contributing_surface_render_target->render_surface()->content_rect());

[danakj, 2013/09/10 14:36:37]

Why not this?

[alokp, 2013/09/10 18:15:46]

I am planning to work on surface-rects in a separate patch.

-  unoccluded_region_in_target_surface.Intersect(
-      ScreenSpaceClipRectInTargetSurface(
-          contributing_surface_render_target->render_surface(),
-          screen_space_clip_rect_));
 
   gfx::RectF unoccluded_rect_in_target_surface =
       unoccluded_region_in_target_surface.bounds();
   gfx::Rect unoccluded_rect = gfx::ToEnclosingRect(
       MathUtil::ProjectClippedRect(inverse_draw_transform,
                                    unoccluded_rect_in_target_surface));
   unoccluded_rect.Intersect(content_rect);
 
   if (has_occlusion_from_outside_target_surface) {
     // Check if the unoccluded rect shrank when applying outside occlusion.
     *has_occlusion_from_outside_target_surface = !gfx::SubtractRects(
         unoccluded_rect_in_target_surface_without_outside_occlusion,
         unoccluded_rect_in_target_surface).IsEmpty();
   }
 
   return unoccluded_rect;
 }
 
 // Instantiate (and export) templates here for the linker.
 template class OcclusionTrackerBase<Layer, RenderSurface>;
 template class OcclusionTrackerBase<LayerImpl, RenderSurfaceImpl>;
 
 }  // namespace cc