cc: Avoid redraw for missing tile outside visible rect

cc/layers/picture_layer_impl.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/layers/picture_layer_impl.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/debug/trace_event_argument.h"
@@ skipping 246 matching lines @@
                                 visible_geometry_rect,
                                 color,
                                 width);
     }
   }
 
   // Keep track of the tilings that were used so that tilings that are
   // unused can be considered for removal.
   std::vector<PictureLayerTiling*> seen_tilings;
 
+  gfx::Rect tile_rect = GetVisibleRectForTilePriorityInContentSpace();

[danakj, 2014/08/15 22:09:08]

scaled_tile_priority_rect?

[enne (OOO), 2014/08/15 22:33:50]

"ForTilePriority" also seems not true anymore.  Is there a better name?

[boliu, 2014/08/15 23:07:26]

How about something with activation, external_activation_rect? Implies tile
priority etc.

+  tile_rect = gfx::ScaleToEnclosingRect(tile_rect, max_contents_scale);
+
   size_t missing_tile_count = 0u;
   size_t on_demand_missing_tile_count = 0u;
   for (PictureLayerTilingSet::CoverageIterator iter(tilings_.get(),
                                                     max_contents_scale,
                                                     scaled_visible_content_rect,
                                                     ideal_contents_scale_);
        iter;
        ++iter) {
     gfx::Rect geometry_rect = iter.geometry_rect();
     gfx::Rect visible_geometry_rect = occlusion_tracker.UnoccludedContentRect(
         geometry_rect, scaled_draw_transform);
     if (visible_geometry_rect.IsEmpty())
       continue;
 
     append_quads_data->visible_content_area +=
         visible_geometry_rect.width() * visible_geometry_rect.height();
 
     if (*iter && iter->IsReadyToDraw()) {
       const ManagedTileState::TileVersion& tile_version =
           iter->GetTileVersionForDrawing();
       switch (tile_version.mode()) {
         case ManagedTileState::TileVersion::RESOURCE_MODE: {
           gfx::RectF texture_rect = iter.texture_rect();
           gfx::Rect opaque_rect = iter->opaque_rect();
           opaque_rect.Intersect(geometry_rect);
 
-          if (iter->contents_scale() != ideal_contents_scale_)
+          if (iter->contents_scale() != ideal_contents_scale_ &&
+              geometry_rect.Intersects(tile_rect)) {

[danakj, 2014/08/15 21:43:44]

We only iterate over the visible_content_rect in this method, so I don't get
this, won't everything intersect?

[boliu, 2014/08/15 21:56:26]

Yes in chrome. No in webview.

In chrome, draw viewport and "visible rect for tile priority" is the same thing.

In webview the draw viewport can be bigger than the visible rect.

[danakj, 2014/08/15 22:09:08]

Maybe we need separate counts (or a bool) in AppendQuadsData for tiles that are
inside vs outside the tile priority rect then. Just not counting them seems
awkward?

[hush (inactive), 2014/08/15 22:12:37]

I'm not exactly sure about the scales of the rects here:
tile_rect is scaled to the max contents scale, but I'm not sure what the scale
is for "geometry_rect". Can you explain?

[danakj, 2014/08/15 22:20:45]

geometry_rect is in the "geometry space" of the iterator, its the space we work
in to keep things in integers, and it is the space scaled by the max contents
scale. The scale given for that space was given to the iterator as the
max_contents_scale on L273, given to the PictureLayerTiling::CoverageIterator
here
https://code.google.com/p/chromium/codesearch#chromium/src/cc/resources/pictu...
and you can see how the geometry rect is computed in there as it's scaled by the
tiling this_geometry_scale/contents_scale when moving from the content space of
the tiling back to the "geometry space" input. 

[boliu, 2014/08/15 22:33:42]

I think we'll need a count then. One counts missing, one counts missing but
outside of the "tile priority rect". Only redraw if they are not equal.

Sounds ok?

[hush (inactive), 2014/08/15 22:35:36]

Thanks Dana!
So I suppose the dest_scale_ in PictureLayerTiling just means geometry scale.

On 2014/08/15 22:20:45, danakj wrote:

[danakj, 2014/08/15 22:36:06]

Yeh
 

             append_quads_data->num_incomplete_tiles++;
+          }
 
           TileDrawQuad* quad =
               render_pass->CreateAndAppendDrawQuad<TileDrawQuad>();
           quad->SetNew(shared_quad_state,
                        geometry_rect,
                        opaque_rect,
                        visible_geometry_rect,
                        tile_version.get_resource_id(),
                        texture_rect,
                        iter.texture_size(),
@@ skipping 52 matching lines @@
         SkColor color = SafeOpaqueBackgroundColor();
         SolidColorDrawQuad* quad =
             render_pass->CreateAndAppendDrawQuad<SolidColorDrawQuad>();
         quad->SetNew(shared_quad_state,
                      geometry_rect,
                      visible_geometry_rect,
                      color,
                      false);
       }
 
-      append_quads_data->num_missing_tiles++;
+      if (geometry_rect.Intersects(tile_rect)) {
+        append_quads_data->num_missing_tiles++;

[danakj, 2014/08/15 21:43:44]

num missing tiles is used for UMA metrics to tell how much checkerboarding was
seen, so we can't do this optionally.

[boliu, 2014/08/15 21:56:26]

In chrome, they will always intersect, so no change there.

In webview, I think the number of missing tiles in draw viewport but outside of
the visible rect isn't really a meaningful number. Also we don't collect uma
data, although I really wish we did.

[danakj, 2014/08/15 22:09:08]

This makes it hard to reason about when we're counting tiles in Chrome. "Always
but it looks like we're not"

+        ++missing_tile_count;
+      }
       append_quads_data->approximated_visible_content_area +=
           visible_geometry_rect.width() * visible_geometry_rect.height();
-      ++missing_tile_count;
       continue;
     }
 
     if (iter->priority(ACTIVE_TREE).resolution != HIGH_RESOLUTION) {
       append_quads_data->approximated_visible_content_area +=
           visible_geometry_rect.width() * visible_geometry_rect.height();
     }
 
     if (seen_tilings.empty() || seen_tilings.back() != iter.CurrentTiling())
       seen_tilings.push_back(iter.CurrentTiling());
@@ skipping 84 matching lines @@
   for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
     if (tilings_->tiling_at(i)->NeedsUpdateForFrameAtTime(
             current_frame_time_in_seconds)) {
       tiling_needs_update = true;
       break;
     }
   }
   if (!tiling_needs_update)
     return;
 
+  gfx::Rect visible_layer_rect = gfx::ScaleToEnclosingRect(
+      GetVisibleRectForTilePriorityInContentSpace(), 1.f / contents_scale_x());
+  WhichTree tree =
+      layer_tree_impl()->IsActiveTree() ? ACTIVE_TREE : PENDING_TREE;
+  for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
+    tilings_->tiling_at(i)->UpdateTilePriorities(tree,
+                                                 visible_layer_rect,
+                                                 ideal_contents_scale_,
+                                                 current_frame_time_in_seconds,
+                                                 occlusion_tracker,
+                                                 render_target(),
+                                                 draw_transform());
+  }
+
+  // Tile priorities were modified.
+  layer_tree_impl()->DidModifyTilePriorities();
+}
+
+gfx::Rect PictureLayerImpl::GetVisibleRectForTilePriorityInContentSpace()
+    const {
   // If visible_rect_for_tile_priority_ is empty or
   // viewport_rect_for_tile_priority_ is set to be different from the device
   // viewport, try to inverse project the viewport into layer space and use
   // that. Otherwise just use visible_rect_for_tile_priority_
   gfx::Rect visible_rect_in_content_space = visible_rect_for_tile_priority_;
 
   if (visible_rect_in_content_space.IsEmpty() ||
       layer_tree_impl()->DeviceViewport() != viewport_rect_for_tile_priority_) {
     gfx::Transform view_to_layer(gfx::Transform::kSkipInitialization);
 
     if (screen_space_transform_for_tile_priority_.GetInverse(&view_to_layer)) {
       // Transform from view space to content space.
       visible_rect_in_content_space =
           gfx::ToEnclosingRect(MathUtil::ProjectClippedRect(
               view_to_layer, viewport_rect_for_tile_priority_));
 
       visible_rect_in_content_space.Intersect(gfx::Rect(content_bounds()));
     }
   }
 
-  gfx::Rect visible_layer_rect = gfx::ScaleToEnclosingRect(
-      visible_rect_in_content_space, 1.f / contents_scale_x());
-  WhichTree tree =
-      layer_tree_impl()->IsActiveTree() ? ACTIVE_TREE : PENDING_TREE;
-  for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
-    tilings_->tiling_at(i)->UpdateTilePriorities(tree,
-                                                 visible_layer_rect,
-                                                 ideal_contents_scale_,
-                                                 current_frame_time_in_seconds,
-                                                 occlusion_tracker,
-                                                 render_target(),
-                                                 draw_transform());
-  }
-
-  // Tile priorities were modified.
-  layer_tree_impl()->DidModifyTilePriorities();
+  return visible_rect_in_content_space;
 }
 
 void PictureLayerImpl::NotifyTileStateChanged(const Tile* tile) {
   if (layer_tree_impl()->IsActiveTree()) {
     gfx::RectF layer_damage_rect =
         gfx::ScaleRect(tile->content_rect(), 1.f / tile->contents_scale());
     AddDamageRect(layer_damage_rect);
   }
 }
 
@@ skipping 236 matching lines @@
 
   // The goal of this function is to find the minimum set of tiles that need to
   // be ready to draw in order to activate without flashing content from a
   // higher res on the active tree to a lower res on the pending tree.
 
   // First, early out for layers with no visible content.
   if (visible_content_rect().IsEmpty())
     return;
 
   gfx::Rect rect(visible_content_rect());
+  rect.Intersect(GetVisibleRectForTilePriorityInContentSpace());

[hush (inactive), 2014/08/15 21:53:18]

I think you need to early out when intersection is empty

[danakj, 2014/08/15 22:09:08]

Why are we intersecting instead of just using the tile priority rect? For cases
where visible_content_rect is empty and we made something up? We early out above
in empty case already.

[boliu, 2014/08/15 22:33:42]

MarkRequiredOffscreenTiles test uses a
GetVisibleRectForTilePriorityInContentSpace() that's bigger than
visible_content_rect(), so using the first would cause more tile to be marked
required.

I'm trying to think if that can happen in practice though... It would mean the
draw viewport is smaller than the tile priority rect, which I think can actually
happen in webview *in theory*.

But I think we can guarantee tile priority rect is contained in the draw rect
outside of cc, then it should be ok, and would just need to fix that test.

Umm...thoughts?

[boliu, 2014/08/15 23:07:26]

This is actually kinda hard to do in webview code. Draw viewport and tile
priority rect are provided in difference spaces.

Also I think we should be intersecting with visible_content_rect() in the
UpdateTilePriorities too.

 
   float min_acceptable_scale =
       std::min(raster_contents_scale_, ideal_contents_scale_);
 
   if (PictureLayerImpl* twin = twin_layer_) {
     float twin_min_acceptable_scale =
         std::min(twin->ideal_contents_scale_, twin->raster_contents_scale_);
     // Ignore 0 scale in case CalculateContentsScale() has never been
     // called for active twin.
     if (twin_min_acceptable_scale != 0.0f) {
@@ skipping 912 matching lines @@
       PictureLayerTilingSet::TilingRange tiling_range = CurrentTilingRange();
       size_t current_tiling_range_offset = current_tiling_ - tiling_range.start;
       return tiling_range.end - 1 - current_tiling_range_offset;
     }
   }
   NOTREACHED();
   return 0;
 }
 
 }  // namespace cc