Fix bad scaling in TiledLayer

cc/layers/tiled_layer_unittest.cc

 // Copyright 2011 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/tiled_layer.h"
 
 #include <limits>
 #include <vector>
 
 #include "base/run_loop.h"
@@ skipping 271 matching lines @@
   layer->InvalidateContentRect(gfx::Rect(0, 0, 100, 200));
   layer->draw_properties().visible_content_rect = gfx::Rect(0, 0, 100, 100);
   UpdateAndPush(layer, layer_impl);
 
   // We should only have the first tile since the other tile was invalidated but
   // not painted.
   EXPECT_TRUE(layer_impl->HasResourceIdForTileAt(0, 0));
   EXPECT_FALSE(layer_impl->HasResourceIdForTileAt(0, 1));
 }
 
+TEST_F(TiledLayerTest, Scale) {
+  layer_tree_host_->SetDeviceScaleFactor(1.5);
+
+  scoped_refptr<FakeTiledLayer> layer =
+      make_scoped_refptr(new FakeTiledLayer(resource_manager_.get()));
+  scoped_ptr<FakeTiledLayerImpl> layer_impl =
+      make_scoped_ptr(new FakeTiledLayerImpl(host_impl_->active_tree(), 1));
+  RenderSurfaceLayerList render_surface_layer_list;
+
+  layer_tree_host_->root_layer()->AddChild(layer);
+
+  layer->SetBounds(gfx::Size(100, 200));
+  CalcDrawProps(&render_surface_layer_list);
+
+  // Change the width so that it doesn't divide cleanly by the scale.
+  layer->SetBounds(gfx::Size(101, 200));
+  UpdateAndPush(layer, layer_impl);
+
+  EXPECT_EQ(1.5, layer->fake_layer_updater()->last_contents_width_scale());
+}
+
 TEST_F(TiledLayerTest, PushOccludedDirtyTiles) {
   scoped_refptr<FakeTiledLayer> layer =
       make_scoped_refptr(new FakeTiledLayer(resource_manager_.get()));
   scoped_ptr<FakeTiledLayerImpl> layer_impl =
       make_scoped_ptr(new FakeTiledLayerImpl(host_impl_->active_tree(), 1));
   TestOcclusionTracker occluded;
   occlusion_ = &occluded;
   layer_tree_host_->SetViewportSize(gfx::Size(1000, 1000));
 
   layer_tree_host_->root_layer()->AddChild(layer);
@@ skipping 554 matching lines @@
   layer->InvalidateContentRect(gfx::Rect(0, 0, 50, 50));
   UpdateAndPush(layer, layer_impl);
   EXPECT_EQ(1, layer->fake_layer_updater()->prepare_count());
   layer->fake_layer_updater()->ClearPrepareCount();
 
   // The layer should still be invalid as update invoked invalidate.
   UpdateAndPush(layer, layer_impl);  // visible
   EXPECT_EQ(1, layer->fake_layer_updater()->prepare_count());
 }
 
-TEST_F(TiledLayerTest, VerifyUpdateRectWhenContentBoundsAreScaled) {

[sky, 2014/09/22 18:00:00]

This test didn't seem worth keeping around.

[danakj, 2014/09/22 19:21:18]

This is testing the Layer::update_rect_ is in layer space, which should still be
true and is kind of orthogonal to this change. It's verifying
InvalidateContentRect gives a scaled-to-layer-space rect up to the base class.

Did this fail with your change? I think this should stick around.

[sky, 2014/09/22 21:23:55]

Yes, because it didn't update the scales explicitly. I changed it, and changed
the numbers to be easy to deal with (eg not 2/3).

-  // The update rect (that indicates what was actually painted) should be in
-  // layer space, not the content space.
-  scoped_refptr<FakeTiledLayerWithScaledBounds> layer = make_scoped_refptr(
-      new FakeTiledLayerWithScaledBounds(resource_manager_.get()));
-
-  layer_tree_host_->root_layer()->AddChild(layer);
-
-  gfx::Rect layer_bounds(0, 0, 300, 200);
-  gfx::Rect content_bounds(0, 0, 200, 250);
-
-  layer->SetBounds(layer_bounds.size());
-  layer->SetContentBounds(content_bounds.size());
-  layer->draw_properties().visible_content_rect = content_bounds;
-
-  // On first update, the update_rect includes all tiles, even beyond the
-  // boundaries of the layer.
-  // However, it should still be in layer space, not content space.
-  layer->InvalidateContentRect(content_bounds);
-
-  layer->SetTexturePriorities(priority_calculator_);
-  resource_manager_->PrioritizeTextures();
-  layer->SavePaintProperties();
-  layer->Update(queue_.get(), NULL);
-  EXPECT_FLOAT_RECT_EQ(gfx::RectF(0, 0, 300, 300 * 0.8), layer->update_rect());
-  UpdateTextures();
-
-  // After the tiles are updated once, another invalidate only needs to update
-  // the bounds of the layer.
-  layer->SetTexturePriorities(priority_calculator_);
-  resource_manager_->PrioritizeTextures();
-  layer->InvalidateContentRect(content_bounds);
-  layer->SavePaintProperties();
-  layer->Update(queue_.get(), NULL);
-  EXPECT_FLOAT_RECT_EQ(gfx::RectF(layer_bounds), layer->update_rect());
-  UpdateTextures();
-
-  // Partial re-paint should also be represented by the update rect in layer
-  // space, not content space.
-  gfx::Rect partial_damage(30, 100, 10, 10);
-  layer->InvalidateContentRect(partial_damage);
-  layer->SetTexturePriorities(priority_calculator_);
-  resource_manager_->PrioritizeTextures();
-  layer->SavePaintProperties();
-  layer->Update(queue_.get(), NULL);
-  EXPECT_FLOAT_RECT_EQ(gfx::RectF(45, 80, 15, 8), layer->update_rect());
-}
-
 TEST_F(TiledLayerTest, VerifyInvalidationWhenContentsScaleChanges) {
   scoped_refptr<FakeTiledLayer> layer =
       make_scoped_refptr(new FakeTiledLayer(resource_manager_.get()));
   scoped_ptr<FakeTiledLayerImpl> layer_impl =
       make_scoped_ptr(new FakeTiledLayerImpl(host_impl_->active_tree(), 1));
   RenderSurfaceLayerList render_surface_layer_list;
 
   layer_tree_host_->root_layer()->AddChild(layer);
 
   // Create a layer with one tile.
@@ skipping 761 matching lines @@
   layer->tracking_layer_painter()->ResetPaintedRect();
 
   EXPECT_RECT_EQ(gfx::Rect(), layer->tracking_layer_painter()->PaintedRect());
   UpdateTextures();
 
   // Invalidate the entire layer in content space. When painting, the rect given
   // to webkit should match the layer's bounds.
   layer->InvalidateContentRect(content_rect);
   layer->Update(queue_.get(), NULL);
 
-  EXPECT_RECT_EQ(layer_rect, layer->tracking_layer_painter()->PaintedRect());
+  // Rounding errors may lead to an extra dip.
+  EXPECT_RECT_NEAR(

[danakj, 2014/09/22 19:21:18]

Can you instead EXPECT_EQ and just explain in a comment the math for why the
result is what it is? NEAR could go either way and is less clear.

I don't actually see why the rects in layer space would be different. We don't
ask blink to paint anything larger due to this change. We just clear some pixels
in content space.

[sky, 2014/09/22 21:23:55]

It's the 1.5 scale. Previously we calculated the scale based on bitmaps which
gave a slightly different scale. Now that we honor the exact scale we get a
different number here.

+      layer_rect, layer->tracking_layer_painter()->PaintedRect(), 1);
 }
 
 TEST_F(TiledLayerTest,
        NonIntegerContentsScaleIsNotDistortedDuringInvalidation) {
   scoped_refptr<UpdateTrackingTiledLayer> layer =
       make_scoped_refptr(new UpdateTrackingTiledLayer(resource_manager_.get()));
 
   layer_tree_host_->root_layer()->AddChild(layer);
 
   gfx::Rect layer_rect(0, 0, 30, 31);
@@ skipping 14 matching lines @@
   layer->tracking_layer_painter()->ResetPaintedRect();
 
   EXPECT_RECT_EQ(gfx::Rect(), layer->tracking_layer_painter()->PaintedRect());
   UpdateTextures();
 
   // Invalidate the entire layer in layer space. When painting, the rect given
   // to webkit should match the layer's bounds.
   layer->SetNeedsDisplayRect(layer_rect);
   layer->Update(queue_.get(), NULL);
 
-  EXPECT_RECT_EQ(layer_rect, layer->tracking_layer_painter()->PaintedRect());
+  // Rounding errors may lead to an extra dip.
+  EXPECT_RECT_NEAR(

[danakj, 2014/09/22 19:21:18]

Same comment.

+      layer_rect, layer->tracking_layer_painter()->PaintedRect(), 1);
 }
 
 }  // namespace
 }  // namespace cc