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

cc/picture_layer_tiling_set.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/picture_layer_tiling_set.h"
 
 namespace cc {
 
+namespace {
+
+class LargestToSmallestScaleFunctor {
+ public:
+  bool operator() (PictureLayerTiling* left, PictureLayerTiling* right) {
+    return left->contents_scale() > right->contents_scale();
+  }
+};
+
+}  // namespace
+
+
 PictureLayerTilingSet::PictureLayerTilingSet(
     PictureLayerTilingClient * client)
     : client_(client) {
 }
 
 PictureLayerTilingSet::~PictureLayerTilingSet() {
 }
 
 void PictureLayerTilingSet::CloneAll(
     const PictureLayerTilingSet& other,
@@ skipping 11 matching lines @@
 void PictureLayerTilingSet::Clone(
     const PictureLayerTiling* tiling,
     const Region& invalidation) {
 
   for (size_t i = 0; i < tilings_.size(); ++i)
     DCHECK_NE(tilings_[i]->contents_scale(), tiling->contents_scale());
 
   tilings_.append(tiling->Clone());
   tilings_.last()->SetClient(client_);
   tilings_.last()->Invalidate(invalidation);
+
+  sort(tilings_.begin(), tilings_.end(), LargestToSmallestScaleFunctor());
 }
 
 void PictureLayerTilingSet::SetLayerBounds(gfx::Size layer_bounds) {
   if (layer_bounds_ == layer_bounds)
     return;
   layer_bounds_ = layer_bounds;
   for (size_t i = 0; i < tilings_.size(); ++i)
     tilings_[i]->SetLayerBounds(layer_bounds);
 }
 
 gfx::Size PictureLayerTilingSet::LayerBounds() const {
   return layer_bounds_;
 }
 
-const PictureLayerTiling* PictureLayerTilingSet::AddTiling(
+PictureLayerTiling* PictureLayerTilingSet::AddTiling(
     float contents_scale,
     gfx::Size tile_size) {
   tilings_.append(PictureLayerTiling::Create(contents_scale, tile_size));
-  tilings_.last()->SetClient(client_);
-  tilings_.last()->SetLayerBounds(layer_bounds_);
-  return tilings_.last();
+  PictureLayerTiling* appended = tilings_.last();
+  appended->SetClient(client_);
+  appended->SetLayerBounds(layer_bounds_);
+
+  sort(tilings_.begin(), tilings_.end(), LargestToSmallestScaleFunctor());
+  return appended;
+}
+
+void PictureLayerTilingSet::RemoveAll() {
+  tilings_.clear();
+}
+
+void PictureLayerTilingSet::Remove(PictureLayerTiling* tiling) {
+  for (size_t i = 0; i < tilings_.size(); ++i) {

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

did you consider the erase(std::remove_if()) paradigm?

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

I feel like remove_if only makes sense if you have more than one thing you're
potentially removing.  Writing a function here would be more lines than this for
loop.

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

Ah, ok good point.

+    if (tilings_[i] == tiling) {
+      tilings_.remove(i);
+      return;
+    }
+  }
 }
 
 void PictureLayerTilingSet::Reset() {
   for (size_t i = 0; i < tilings_.size(); ++i)
     tilings_[i]->Reset();
 }
 
-PictureLayerTilingSet::Iterator::Iterator(const PictureLayerTilingSet* set,
-                                          float contents_scale,
-                                          gfx::Rect content_rect)
+PictureLayerTilingSet::Iterator::Iterator(
+    const PictureLayerTilingSet* set,
+    float contents_scale,
+    gfx::Rect content_rect,
+    float ideal_contents_scale)
     : set_(set),
       contents_scale_(contents_scale),
+      ideal_contents_scale_(ideal_contents_scale),
       current_tiling_(-1) {
   missing_region_.Union(content_rect);
+
+  for (ideal_tiling_ = 0;

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

can the set ever have 0 tilings? Is it okay for ideal_tiling_ to be 0 in that
case?

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

Yeah, it can, but this works out.  If ideal_tiling is zero, then NextTiling
initializes current_tiling to zero, and since that's greater than the
tilings_.size(), it's done.

+       ideal_tiling_ < set_->tilings_.size();
+       ++ideal_tiling_) {
+    PictureLayerTiling* tiling = set_->tilings_[ideal_tiling_];
+    if (tiling->contents_scale() < ideal_contents_scale_) {
+      if (ideal_tiling_ > 0)
+        ideal_tiling_--;

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

this seems less "ideal" and more "best" ? should we distinguish between them?

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

The ideal tiling here is just the tiling that is closest to the ideal contents
scale.  Is your discomfort here that ideal maybe implies some "absolute ideal
under any circumstances" and this is really "best given what we have"?

I feel like "ideal" and "best" are pretty much synonymous here, but could rename
variables if you think that'd make it clearer.

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

I felt like ideal is used elsewhere to refer to the thing that exactly matches
the contents scale 1:1. whereas here, it's not, so best seemed more appropriate?

i may be misattributing accuracy on the other "ideal" cases though?

+      break;
+    }
+  }
+
   ++(*this);
 }
 
 PictureLayerTilingSet::Iterator::~Iterator() {
 }
 
 gfx::Rect PictureLayerTilingSet::Iterator::geometry_rect() const {
   if (!tiling_iter_) {
     if (!region_iter_.has_rect())
       return gfx::Rect();
@@ skipping 19 matching lines @@
     return NULL;
   return *tiling_iter_;
 }
 
 Tile* PictureLayerTilingSet::Iterator::operator*() const {
   if (!tiling_iter_)
     return NULL;
   return *tiling_iter_;
 }
 
+PictureLayerTiling* PictureLayerTilingSet::Iterator::CurrentTiling() {
+  if (current_tiling_ < 0 || current_tiling_ >= set_->tilings_.size())
+    return NULL;
+  return set_->tilings_[current_tiling_];
+}
+
+int PictureLayerTilingSet::Iterator::NextTiling() const {
+  // Prefer hi-res tiles, but prefer the least hi-res if bigger than ideal,

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

I'm still not sure what "prefer the least hi-res if bigger than ideal" means.
You could spell out the ordering like:

// Order returned by this method is:
// 1. Ideal
// 2. Tilings < Ideal in decreasing order.
// 3. Tilings > Ideal in increasing order.
// 4. tilings.size()

Or, maybe it's not needed at all with the else changes. At the moment I don't
think this comment is adding much cuz I don't get it, even though I understand
the ordering.

[enne (OOO), 2013/01/08 23:54:38]

Done.

+  // since it won't add any additional fidelity on screen.
+  if (current_tiling_ < 0)
+    return ideal_tiling_;
+  else if (current_tiling_ > ideal_tiling_)
+    return current_tiling_ + 1;
+  else

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

would "else if (current_tiling_) ... else ..." make this more clear than
switching into ?: mode suddenly in the else? 

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

Done.

+    return current_tiling_ ? current_tiling_ - 1 : ideal_tiling_ + 1;
+}
+
 PictureLayerTilingSet::Iterator& PictureLayerTilingSet::Iterator::operator++() {
   bool first_time = current_tiling_ < 0;
 
   if (!*this && !first_time)
     return *this;
 
   if (tiling_iter_)
     ++tiling_iter_;
 
   // Loop until we find a valid place to stop.
   while (true) {
     while (tiling_iter_ && (!*tiling_iter_ || !tiling_iter_->GetResourceId())) {
       missing_region_.Union(tiling_iter_.geometry_rect());
       ++tiling_iter_;
     }
     if (tiling_iter_)
       return *this;
 
     // If the set of current rects for this tiling is done, go to the next
     // tiling and set up to iterate through all of the remaining holes.
     // This will also happen the first time through the loop.
     if (!region_iter_.has_rect()) {
-      current_tiling_++;
+      current_tiling_ = NextTiling();
       current_region_.Swap(missing_region_);
       missing_region_.Clear();
       region_iter_ = Region::Iterator(current_region_);
 
       // All done and all filled.
       if (!region_iter_.has_rect()) {
         current_tiling_ = set_->tilings_.size();
         return *this;
       }
 
       // No more valid tiles, return this checkerboard rect.
       if (current_tiling_ >= static_cast<int>(set_->tilings_.size()))

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

Does this still happen now? Seems like NextTiling() would loop back around to
ideal_tiling_ in the end, and you'd get stuck in here forever?

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

Yes, this case is definitely still hit.  This will always get hit for the first
checkerboard rect.  After that, region_iter.has_rect(), so you won't hit this if
statement.

If you have 5 tilings and index 3 is ideal, then NextTiling() returns: 3, 2, 1,
0, 4, 5.

I'm sorry this class is undertested; I think that would help explain how it's
supposed to work.

         return *this;
     }
 
     // Pop a rect off.  If there are no more tilings, then these will be
     // treated as geometry with null tiles that the caller can checkerboard.
     gfx::Rect last_rect = region_iter_.rect();
     region_iter_.next();
 
     // Done, found next checkerboard rect to return.
     if (current_tiling_ >= static_cast<int>(set_->tilings_.size()))
@@ skipping 29 matching lines @@
         device_viewport,
         layer_content_scale_x,
         layer_content_scale_y,
         last_screen_transform,
         current_screen_transform,
         time_delta);
   }
 }
 
 }  // namespace cc