cc: Generate tilings at other scales for impl-side painting

cc/picture_layer_impl.cc

Index: cc/picture_layer_impl.cc
diff --git a/cc/picture_layer_impl.cc b/cc/picture_layer_impl.cc
index e4c113a5aac5e9ce34d91998d77664cbe8e5b43d..537791664625eb2f60e040451bd8ec44cd024659 100644
--- a/cc/picture_layer_impl.cc
+++ b/cc/picture_layer_impl.cc
@@ -17,6 +17,10 @@
 #include "ui/gfx/quad_f.h"
 #include "ui/gfx/size_conversions.h"
 
+namespace {
+const float kMaxScaleRatioDuringPinch = 2.0f;
+}
+
 namespace cc {
 
 PictureLayerImpl::PictureLayerImpl(LayerTreeImpl* treeImpl, int id)
@@ -24,6 +28,8 @@ PictureLayerImpl::PictureLayerImpl(LayerTreeImpl* treeImpl, int id)
       tilings_(this),
       pile_(PicturePileImpl::Create()),
       last_update_time_(0),
+      last_content_scale_(0),
+      ideal_contents_scale_(0),
       is_mask_(false) {
 }
 
@@ -53,7 +59,8 @@ void PictureLayerImpl::appendQuads(QuadSink& quadSink,
   if (showDebugBorders()) {
     for (PictureLayerTilingSet::Iterator iter(&tilings_,
                                               contentsScaleX(),
-                                              rect);
+                                              rect,
+                                              ideal_contents_scale_);
          iter;
          ++iter) {
       SkColor color;
@@ -74,7 +81,14 @@ void PictureLayerImpl::appendQuads(QuadSink& quadSink,
     }
   }
 
-  for (PictureLayerTilingSet::Iterator iter(&tilings_, contentsScaleX(), rect);
+  // Keep track of the tilings that were used so that tilings that are
+  // unused can be considered for removal.
+  std::vector<PictureLayerTiling*> seen_tilings;
+
+  for (PictureLayerTilingSet::Iterator iter(&tilings_,
+                                            contentsScaleX(),
+                                            rect,
+                                            ideal_contents_scale_);
        iter;
        ++iter) {
     ResourceProvider::ResourceId resource = 0;
@@ -122,7 +136,15 @@ void PictureLayerImpl::appendQuads(QuadSink& quadSink,
                  outside_right_edge && useAA,
                  outside_bottom_edge && useAA);
     quadSink.append(quad.PassAs<DrawQuad>(), appendQuadsData);
+
+    if (!seen_tilings.size() || seen_tilings.back() != iter.CurrentTiling())
+      seen_tilings.push_back(iter.CurrentTiling());
   }
+
+  // During a pinch, a user could zoom in and out, so throwing away a tiling may
+  // be premature.
+  if (!layerTreeImpl()->PinchGestureActive())
+    CleanUpUnusedTilings(seen_tilings);

[danakj, 2013/01/07 22:35:56]

any reason why this isn't a didDraw() thing? it's going to delay getting pixels
on the screen to do work that's not needed to get pixels on the screen.

[enne (OOO), 2013/01/08 01:08:00]

didDraw isn't called before swap, just FYI.

[danakj, 2013/01/08 23:27:16]

I guess you mean "is". Doh..

 }
 
 void PictureLayerImpl::dumpLayerProperties(std::string*, int indent) const {
@@ -137,16 +159,15 @@ void PictureLayerImpl::didUpdateTransforms() {
   double time_delta = 0;
   if (last_update_time_ != 0 && last_bounds_ == bounds() &&
       last_content_bounds_ == contentBounds() &&
-      last_content_scale_x_ == contentsScaleX() &&
-      last_content_scale_y_ == contentsScaleY()) {
+      last_content_scale_ == contentsScaleX()) {
     time_delta = current_time - last_update_time_;
   }
   WhichTree tree = layerTreeImpl()->IsActiveTree() ? ACTIVE_TREE : PENDING_TREE;
   tilings_.UpdateTilePriorities(
       tree,
       layerTreeImpl()->device_viewport_size(),
+      last_content_scale_,
       contentsScaleX(),
-      contentsScaleY(),
       last_screen_space_transform_,
       current_screen_space_transform,
       time_delta);
@@ -155,8 +176,7 @@ void PictureLayerImpl::didUpdateTransforms() {
   last_update_time_ = current_time;
   last_bounds_ = bounds();
   last_content_bounds_ = contentBounds();
-  last_content_scale_x_ = contentsScaleX();
-  last_content_scale_y_ = contentsScaleY();
+  last_content_scale_ = contentsScaleX();
 }
 
 void PictureLayerImpl::calculateContentsScale(
@@ -170,9 +190,9 @@ void PictureLayerImpl::calculateContentsScale(
   }
 
   float min_contents_scale = layerTreeImpl()->settings().minimumContentsScale;
-  ideal_contents_scale = std::max(ideal_contents_scale, min_contents_scale);
+  ideal_contents_scale_ = std::max(ideal_contents_scale, min_contents_scale);
 
-  ManageTilings(ideal_contents_scale);
+  ManageTilings(ideal_contents_scale_);
 
   // The content scale and bounds for a PictureLayerImpl is somewhat fictitious.
   // There are (usually) several tilings at different scales.  However, the
@@ -239,7 +259,10 @@ void PictureLayerImpl::SetIsMask(bool is_mask) {
 ResourceProvider::ResourceId PictureLayerImpl::contentsResourceId() const {
   gfx::Rect content_rect(gfx::Point(), contentBounds());
   float scale = contentsScaleX();
-  for (PictureLayerTilingSet::Iterator iter(&tilings_, scale, content_rect);
+  for (PictureLayerTilingSet::Iterator iter(&tilings_,
+                                            scale,
+                                            content_rect,
+                                            ideal_contents_scale_);
        iter;
        ++iter) {
     // Mask resource not ready yet.
@@ -253,25 +276,26 @@ ResourceProvider::ResourceId PictureLayerImpl::contentsResourceId() const {
   return 0;
 }
 
-void PictureLayerImpl::AddTiling(float contents_scale, gfx::Size tile_size) {
+PictureLayerTiling* PictureLayerImpl::AddTiling(float contents_scale) {
   if (contents_scale < layerTreeImpl()->settings().minimumContentsScale)
-    return;
+    return NULL;
 
-  const PictureLayerTiling* tiling = tilings_.AddTiling(
+  PictureLayerTiling* tiling = tilings_.AddTiling(
       contents_scale,
-      tile_size);
+      TileSize());
 
   // If a new tiling is created on the active tree, sync it to the pending tree
   // so that it can share the same tiles.
   if (layerTreeImpl()->IsPendingTree())
-    return;
+    return tiling;
 
   PictureLayerImpl* pending_twin = static_cast<PictureLayerImpl*>(
       layerTreeImpl()->FindPendingTreeLayerById(id()));
   if (!pending_twin)
-    return;
+    return tiling;
   DCHECK_EQ(id(), pending_twin->id());
   pending_twin->SyncTiling(tiling);
+  return tiling;
 }
 
 gfx::Size PictureLayerImpl::TileSize() const {
@@ -285,18 +309,130 @@ gfx::Size PictureLayerImpl::TileSize() const {
   return layerTreeImpl()->settings().defaultTileSize;
 }
 
+namespace {
+
+inline float PositiveRatio(float float1, float float2) {
+  DCHECK(float1 > 0);
+  DCHECK(float2 > 0);
+  return float1 > float2 ? float1 / float2 : float2 / float1;
+}
+
+inline bool IsCloserToThan(
+    PictureLayerTiling* layer1,
+    PictureLayerTiling* layer2,
+    float contents_scale) {
+  // Absolute value for ratios.
+  float ratio1 = PositiveRatio(layer1->contents_scale(), contents_scale);
+  float ratio2 = PositiveRatio(layer2->contents_scale(), contents_scale);
+  return ratio1 < ratio2;
+}
+
+}  // namespace
+
 void PictureLayerImpl::ManageTilings(float ideal_contents_scale) {
-  if (drawsContent()) {
-    // TODO(enne): Add tilings during pinch zoom
-    // TODO(enne): Consider culling old tilings after pinch finishes.
-    if (!tilings_.num_tilings()) {
-      AddTiling(ideal_contents_scale, TileSize());
-      // TODO(enne): Add a low-res tiling as well.
+  DCHECK(ideal_contents_scale);
+  float low_res_factor = layerTreeImpl()->settings().lowResContentsScaleFactor;
+  float low_res_contents_scale = ideal_contents_scale * low_res_factor;
+
+  // Find existing tilings closest to ideal high / low res
+  PictureLayerTiling* high_res = NULL;
+  PictureLayerTiling* low_res = NULL;
+
+  if (layerTreeImpl()->IsPendingTree() &&

[danakj, 2013/01/07 22:35:56]

maybe a temp bool var here for this if()? then the comment inside could probably
be obsolete.

[enne (OOO), 2013/01/08 01:08:00]

Why the temp bool? It doesn't get reused anywhere.  I'll clarify the comment, if
that's the confusion.

[danakj, 2013/01/08 23:27:16]

I thought maybe giving a name to the combined state would be nice (comments by
variables), like

bool remove_non_ideal_high_and_low_res_tilings = IsPendingTree() &&
!PinchActive;
if (remove_non_ideal_high_and_low_res_tilings) {
}

Maybe not though..

+      !layerTreeImpl()->PinchGestureActive()) {
+    // Remove any tilings that don't exactly match the contents scale.
+    std::vector<PictureLayerTiling*> remove_list;
+    for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
+      PictureLayerTiling* tiling = tilings_.tiling_at(i);
+      if (tiling->contents_scale() == ideal_contents_scale)
+        high_res = tiling;
+      else if (tiling->contents_scale() == low_res_contents_scale)
+        low_res = tiling;
+      else
+        remove_list.push_back(tiling);
     }
+
+    for (size_t i = 0; i < remove_list.size(); ++i)
+      tilings_.Remove(remove_list[i]);
   } else {
-    // TODO(enne): This should be unnecessary once there are two trees.
-    tilings_.Reset();
+    for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
+      PictureLayerTiling* tiling = tilings_.tiling_at(i);
+      if (!high_res || IsCloserToThan(tiling, high_res, ideal_contents_scale))
+        high_res = tiling;
+      if (!low_res || IsCloserToThan(tiling, low_res, low_res_contents_scale))
+        low_res = tiling;
+
+      // Set everything to non-ideal to start.
+      tiling->set_resolution(NON_IDEAL_RESOLUTION);

[danakj, 2013/01/07 22:35:56]

should this be outside the else{}?

[enne (OOO), 2013/01/08 01:08:00]

Technically, everything gets removed but low_res and high_res above, and
high_res and low_res get set to appropriate values below, so I think it's a
wash.  I suppose it'd be cleaner to do so and would make the code less
confusing.  Done.

+    }
+  }
+
+  // Active tree creates tilings first, and the pending tree will get them

[danakj, 2013/01/07 22:35:56]

s/ first//

I misunderstood this comment the first couple times.

[enne (OOO), 2013/01/08 01:08:00]

The active tree gets calcDrawProperties called on it before the pending tree
does.  I'll try to clarify this comment.

+  // synced to it.
+  if (layerTreeImpl()->IsActiveTree()) {
+    if (layerTreeImpl()->PinchGestureActive()) {
+      if (high_res) {
+        // If zooming out, if only available high-res tiling is very high
+        // resolution, create additional tilings closer to the ideal.
+        float high_ratio = high_res->contents_scale() / ideal_contents_scale;
+        if (high_ratio >= kMaxScaleRatioDuringPinch)

[danakj, 2013/01/07 22:35:56]

can you use PositiveRatio() to simplify this area a bunch and only need one
if()?

[enne (OOO), 2013/01/08 01:08:00]

Oh, nice thought, done.

+          high_res = AddTiling(ideal_contents_scale);
+
+        // When zooming in, add some additional tilings so that content
+        // "crisps up" prior to releasing pinch.
+        float low_ratio = ideal_contents_scale / high_res->contents_scale();
+        if (low_ratio >= kMaxScaleRatioDuringPinch)
+          high_res = AddTiling(ideal_contents_scale);
+      } else if (!low_res)

[danakj, 2013/01/07 22:35:56]

why do you only do this if !high_res? What if you are missing tiles that have
become visible now, would you not want a low res version of them even though
your high res tiles are okay still?

[enne (OOO), 2013/01/08 01:08:00]

Fixed to always add.

+        // If no tilings at all, add a low res during pinch for sanity.
+        low_res = AddTiling(low_res_contents_scale);
+    } else {
+      // When not pinching, ensure exact contents scales

[danakj, 2013/01/07 22:35:56]

nit: period

[enne (OOO), 2013/01/08 01:08:00]

Done.

+      if (!high_res || high_res->contents_scale() != ideal_contents_scale)
+        high_res = AddTiling(ideal_contents_scale);
+      if (!low_res || low_res->contents_scale() != low_res_contents_scale)
+        low_res = AddTiling(low_res_contents_scale);
+    }
   }
+
+  if (high_res)
+    high_res->set_resolution(HIGH_RESOLUTION);
+  if (low_res) {
+    if (low_res == high_res)
+      low_res->set_resolution(NON_IDEAL_RESOLUTION);

[danakj, 2013/01/07 22:35:56]

i don't get this. if low == high, then you set the one thing to not be either?
Why wouldn't it still be HIGH?

[enne (OOO), 2013/01/08 01:08:00]

I don't know what I was thinking.  Fixed.

+    else
+      low_res->set_resolution(LOW_RESOLUTION);
+  }
+}
+
+void PictureLayerImpl::CleanUpUnusedTilings(
+    std::vector<PictureLayerTiling*> used) {

[danakj, 2013/01/07 22:35:56]

i don't understand the reason for this parameter, can you explain?

it seems to me that if you have a set of tilings_, then you could iterate thru
the whole thing and remove anything that's not your current HIGH or LOW res
tiling.

What tilings would be in tilings_ but not in used, and why would you not want to
remove things that were not in |used| (and therefore unused)?

[enne (OOO), 2013/01/08 01:08:00]

Yeah, this is the important part and why I couldn't just remove tilings from the
active tree as a part of ManageTilings.

Let's say you zoom your ideal contents scale from 2 to 1. Right before you let
go of the pinch, you'll have tilings of scales 0.25 and 2.  When you release the
pinch and manage tilings, you'll have the following scales: 0.125, 0.250, 1, 2.

However, the 1 was *just created* and so likely has no content.  It's wasteful
to keep 2 around forever, so you want to get rid of 2 as soon as you don't need
it anymore.  When do you not need it anymore? When you don't use it for any
visible quads.

This is intimately tied in to the iterator change, where it tries to get tiles
from the ideal tiling first and then only non-ideal tilings second.  In this
case, your iteration would go through scales (in order) 1, 2, 0.25, 0.125.  If 1
can handle the entire content rect on its own, then you don't get any tiles from
2, and then it's not used, and therefore can be removed.

[danakj, 2013/01/08 23:27:16]

Oh, I see. |used| is a blacklist to not delete those. Somehow I read this as
deleting stuff from the used list.

+  if (ideal_contents_scale_ <= 0)

[danakj, 2013/01/07 22:35:56]

how is this possible?

[enne (OOO), 2013/01/08 01:08:00]

This is vestigial.  I added it previously to work around user error, but is not
actually needed.  BALETED.

+    return;
+
+  std::vector<PictureLayerTiling*> to_remove;
+
+  PictureLayerTiling* low_res = NULL;
+  float low_res_factor = layerTreeImpl()->settings().lowResContentsScaleFactor;
+  float low_res_contents_scale = ideal_contents_scale_ * low_res_factor;
+  for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
+    PictureLayerTiling* tiling = tilings_.tiling_at(i);
+    if (!low_res || IsCloserToThan(tiling, low_res, low_res_contents_scale))
+      low_res = tiling;
+  }
+
+  for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
+    PictureLayerTiling* tiling = tilings_.tiling_at(i);
+    if (tiling->contents_scale() == ideal_contents_scale_)
+      continue;
+    if (tiling == low_res)

[danakj, 2013/01/07 22:35:56]

can you not use the HIGH_RESOLUTION and LOW_RESOLUTION bits here instead of
doing a walk to find the low_res?

[enne (OOO), 2013/01/08 01:08:00]

Oh, hey.  That's a really good idea.  It's almost as if I've already gone
through the trouble of marking the tilings I care about.  Done.

+      continue;
+    if (std::find(used.begin(), used.end(), tiling) == used.end())
+      to_remove.push_back(tiling);
+  }
+
+  for (size_t i = 0; i < to_remove.size(); ++i)
+    tilings_.Remove(to_remove[i]);
 }
 
 }  // namespace cc