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

cc/picture_layer_tiling_set.cc

Index: cc/picture_layer_tiling_set.cc
diff --git a/cc/picture_layer_tiling_set.cc b/cc/picture_layer_tiling_set.cc
index 9edf8479d7352583cc1effdc2e4139d884dcbb6d..f9bde25d4e1a9a465e402246a7beaa54eb24af6b 100644
--- a/cc/picture_layer_tiling_set.cc
+++ b/cc/picture_layer_tiling_set.cc
@@ -6,6 +6,18 @@
 
 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) {
@@ -37,6 +49,8 @@ void PictureLayerTilingSet::Clone(
   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) {
@@ -51,13 +65,29 @@ 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() {
@@ -65,13 +95,28 @@ void PictureLayerTilingSet::Reset() {
     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);
 }
 
@@ -111,6 +156,23 @@ Tile* PictureLayerTilingSet::Iterator::operator*() const {
   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;
 
@@ -133,7 +195,7 @@ PictureLayerTilingSet::Iterator& PictureLayerTilingSet::Iterator::operator++() {
     // 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_);