Raster PictureLayerTiling with fractional translation

cc/layers/picture_layer_impl.cc

Index: cc/layers/picture_layer_impl.cc
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index 12fe4e679d06f0b50e5137284e2b2273f5e59f9c..eb4e3b6df5a8b8c1d12a4097ff19ef7e7015b5f3 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -830,12 +830,15 @@ void PictureLayerImpl::SetNearestNeighbor(bool nearest_neighbor) {
   NoteLayerPropertyChanged();
 }
 
-PictureLayerTiling* PictureLayerImpl::AddTiling(float contents_scale) {
+PictureLayerTiling* PictureLayerImpl::AddTiling(
+    float contents_scale,
+    const gfx::Vector2dF& contents_translation) {
   DCHECK(CanHaveTilings());
   DCHECK_GE(contents_scale, MinimumContentsScale());
   DCHECK_LE(contents_scale, MaximumContentsScale());
   DCHECK(raster_source_->HasRecordings());
-  return tilings_->AddTiling(contents_scale, raster_source_);
+  return tilings_->AddTiling(contents_scale, contents_translation,
+                             raster_source_);
 }
 
 void PictureLayerImpl::RemoveAllTilings() {
@@ -851,9 +854,25 @@ void PictureLayerImpl::AddTilingsForRasterScale() {
 
   PictureLayerTiling* high_res =
       tilings_->FindTilingWithScale(raster_contents_scale_);
+
+  if (high_res) {
+    gfx::Vector2dF translation_error =
+        high_res->contents_translation() - raster_contents_translation_;
+    float error_x = std::abs(translation_error.x());
+    float error_y = std::abs(translation_error.y());
+    // Handle wrap over.
+    error_x = error_x > 0.5f ? 1.f - error_x : error_x;
+    error_y = error_y > 0.5f ? 1.f - error_y : error_y;
+    static constexpr float kErrorThreshold = 0.001;

[enne (OOO), 2016/07/26 00:22:16]

Where does this number come from? Should you just remember the layer level
translation and throw everything out if it changes at all?

[trchen, 2016/08/03 06:06:06]

I'm worrying about tiny errors due to FP precision. The layer can have draw
transform mutated but having the same fractional offset.

The threshold is set to 0.001px because with bi-linear filtering the resulting
error in pixel value can't exceed 0.512, which sounds like a reasonable
threshold. A better number would be 2^-10, but I don't think it matters...

[trchen, 2016/08/03 22:41:19]

Note: This code path is removed in patch set 2, as the simpler policy just
blindly reuse the old tiling if raster scale is not changed.

+    if (error_x > kErrorThreshold || error_y > kErrorThreshold) {
+      tilings_->Remove(high_res);

[enne (OOO), 2016/07/26 00:22:16]

It'd be nice if RecalculateRasterTranslation did all of this work of cleaning up
instead of doing it inside of AddTilingsForRasterScale.  Shouldn't you throw out
all of the tilings and not just the high res one?

[trchen, 2016/08/03 06:06:06]

Not necessary. The low-res tiling is not pixel-exact and is subject to
resampling & filtering anyway. Only high-res tiling with 1:1 pixel mapping
matters.

+      high_res = nullptr;
+    }
+  }
+
   if (!high_res) {
     // We always need a high res tiling, so create one if it doesn't exist.
-    high_res = AddTiling(raster_contents_scale_);
+    high_res = AddTiling(raster_contents_scale_, raster_contents_translation_);
   } else if (high_res->may_contain_low_resolution_tiles()) {
     // If the tiling we find here was LOW_RESOLUTION previously, it may not be
     // fully rastered, so destroy the old tiles.
@@ -955,11 +974,35 @@ void PictureLayerImpl::AddLowResolutionTilingIfNeeded() {
   bool is_animating = draw_properties().screen_space_transform_is_animating;
   if (!is_pinching && !is_animating) {
     if (!low_res)
-      low_res = AddTiling(low_res_raster_contents_scale_);
+      low_res = AddTiling(low_res_raster_contents_scale_, gfx::Vector2dF());
     low_res->set_resolution(LOW_RESOLUTION);
   }
 }
 
+void PictureLayerImpl::RecalculateRasterTranslation() {
+  raster_contents_translation_ = gfx::Vector2dF();
+  if (draw_properties().screen_space_transform_is_animating)

[enne (OOO), 2016/07/26 00:22:16]

This is my biggest worry in the patch.  I get that you want to not do this when
something is animating, but that doesn't account for how often folks animate
translations with Javascript.  We had a small number of sites that had
performance problem when we rerastered after a scale change, which we decided to
handle with will-change: transform as the indicator for when to reraster.

It's not clear to me how many sites would run into performance problem while
translating, but intuitively I suspect there are probably a lot more translation
than scales.  I think it'd be good to have UMA stats for this in the same way
that we did for scales so that we can make sure we're doing this carefully and
will not regress some large number of sites.

[danakj, 2016/07/27 22:52:17]

Here's use counters that enne is referring to for scale (we since removed them):
https://codereview.chromium.org/1946403003

[trchen, 2016/08/03 06:06:06]

I chatted with chris and also thought through a number of alternatives last
week. I think the most practical solution is to recalculate raster translation
if and only if raster scale is mutated. This way we can guarantee no performance
loss. Most of time quality will improve, but can also degrade in some corner
cases.

+    return;
+
+  gfx::Transform screen_space_transform = ScreenSpaceTransform();
+  if (!screen_space_transform.IsScaleOrTranslation())

[enne (OOO), 2016/07/26 00:22:16]

Should this just be "is translation?"

[enne (OOO), 2016/07/26 00:22:16]

Should this just be "is translation?"

[trchen, 2016/08/03 06:06:06]

Many bug reports are due to layers with odd pixel location on a 150% DPI
display. I think we should cover that case.

+    return;
+
+  // Good match if the maximum alignment error on a layer of size 10000px
+  // does not exceed 0.001px.
+  static constexpr float kErrorThreshold = 0.0000001f;
+  if (std::abs(screen_space_transform.matrix().getFloat(0, 0) -
+               raster_contents_scale_) > kErrorThreshold ||
+      std::abs(screen_space_transform.matrix().getFloat(1, 1) -
+               raster_contents_scale_) > kErrorThreshold)
+    return;
+
+  float origin_x = screen_space_transform.matrix().getFloat(0, 3);
+  float origin_y = screen_space_transform.matrix().getFloat(1, 3);
+  raster_contents_translation_ =
+      gfx::Vector2dF(origin_x - floorf(origin_x), origin_y - floorf(origin_y));
+}
+
 void PictureLayerImpl::RecalculateRasterScales() {
   if (is_directly_composited_image_) {
     if (!raster_source_scale_)
@@ -1072,6 +1115,8 @@ void PictureLayerImpl::RecalculateRasterScales() {
   DCHECK_GE(raster_contents_scale_, MinimumContentsScale());
   DCHECK_LE(raster_contents_scale_, MaximumContentsScale());
 
+  RecalculateRasterTranslation();
+
   // If this layer would create zero or one tiles at this content scale,
   // don't create a low res tiling.
   gfx::Size raster_bounds =