Raster PictureLayerTiling with fractional translation

cc/layers/picture_layer_impl.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/layers/picture_layer_impl.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 833 matching lines @@
 }
 
 void PictureLayerImpl::SetNearestNeighbor(bool nearest_neighbor) {
   if (nearest_neighbor_ == nearest_neighbor)
     return;
 
   nearest_neighbor_ = 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() {
   tilings_->RemoveAllTilings();
   // If there are no tilings, then raster scales are no longer meaningful.
   ResetRasterScale();
 }
 
 void PictureLayerImpl::AddTilingsForRasterScale() {
   // Reset all resolution enums on tilings, we'll be setting new values in this
   // function.
   tilings_->MarkAllTilingsNonIdeal();
 
   PictureLayerTiling* high_res =
       tilings_->FindTilingWithScale(raster_contents_scale_);

[enne (OOO), 2016/08/22 22:57:39]

What if the layer moves and the contents translation for the high res version
becomes not ideal? Shouldn't this code not consider that high res and make
another?

Your patch also does not adjust PictureLayerImpl::GetPendingOrActiveTwinTiling
and so two tilings that have different content translations will be considered
twins, and so "required for activation" will be incorrect.

I think you need to write a unit test for this activation case to make sure that
if the main thread adjusts the layer offset that activation will not occur until
a tiling with the correct translation is ready.

[trchen, 2016/08/29 10:14:48]

It was what was done in patchset 1, but later I changed to make content
translation recalc lazier. The policy implemented in this patchset is to never
change content translation of existing tile sets.
You are right. It'd be a best if we can allow an ideal tile set and a non-ideal
tile set that has different translation.

In the newly uploaded patchset I made GetPendingOrActiveTwinTiling to return
nullptr if the transform doesn't match. During activation the old tile set will
be purged.

   if (!high_res) {
+    gfx::Vector2dF raster_translation = CalculateRasterTranslation();
     // 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_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.
     high_res->Reset();
     // Reset the flag now that we'll make it high res, it will have fully
     // rastered content.
     high_res->reset_may_contain_low_resolution_tiles();
   }
   high_res->set_resolution(HIGH_RESOLUTION);
 
@@ skipping 81 matching lines @@
       tilings_->FindTilingWithScale(low_res_raster_contents_scale_);
   DCHECK(!low_res || low_res->resolution() != HIGH_RESOLUTION);
 
   // Only create new low res tilings when the transform is static.  This
   // prevents wastefully creating a paired low res tiling for every new high
   // res tiling during a pinch or a CSS animation.
   bool is_pinching = layer_tree_impl()->PinchGestureActive();
   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);
   }
 }
 
+gfx::Vector2dF PictureLayerImpl::CalculateRasterTranslation() {

[enne (OOO), 2016/08/22 22:57:39]

This function depending on raster_contents_scale_ is a little weird, and means
that it can only be used for high res.  Can you make it take
raster_contents_scale_ as a parameter explicitly?

[trchen, 2016/08/29 10:14:48]

Done.

+  if (draw_properties().screen_space_transform_is_animating)
+    return gfx::Vector2dF();
+
+  gfx::Transform draw_transform = DrawTransform();

[trchen, 2016/08/12 22:11:30]

We can certainly use draw transform to align instead. The above check at line
989 becomes weird though, because we can have static draw transform while having
animating screen transform.

Personally I think it is okay to use either. Using the draw transform makes more
sense in today's world. In the future world I believe draw transform will be
equal to screen transform in most of the cases (i.e. render surfaces align with
device pixels).

[enne (OOO), 2016/08/16 23:02:52]

If you have a static draw transform, then you are already snapped to pixel
coordinates and its your target that needs to adjust.  Adjusting to screen
coordinates would be incorrect.

+  if (!draw_transform.IsScaleOrTranslation())
+    return gfx::Vector2dF();
+
+  // Good match if the maximum alignment error on a layer of size 10000px
+  // does not exceed 0.001px.
+  static constexpr float kErrorThreshold = 0.0000001f;

[enne (OOO), 2016/08/22 22:57:39]

I think this function needs some more comments.  This block of code would be
good to say something like "if the draw transform scale and raster content scale
are not equal, then the tiling will not be aligned to the target and so a
content scale would not help with making the tiling appear crisp".

[trchen, 2016/08/29 10:14:48]

Done.

+  if (std::abs(draw_transform.matrix().getFloat(0, 0) -
+               raster_contents_scale_) > kErrorThreshold ||
+      std::abs(draw_transform.matrix().getFloat(1, 1) -
+               raster_contents_scale_) > kErrorThreshold)
+    return gfx::Vector2dF();
+
+  float origin_x = draw_transform.matrix().getFloat(0, 3);
+  float origin_y = draw_transform.matrix().getFloat(1, 3);
+  return gfx::Vector2dF(origin_x - floorf(origin_x),
+                        origin_y - floorf(origin_y));
+}
+
 void PictureLayerImpl::RecalculateRasterScales() {
   if (is_directly_composited_image_) {
     if (!raster_source_scale_)
       raster_source_scale_ = 1.f;
 
     float min_scale = MinimumContentsScale();
     float max_scale = std::max(1.f, MinimumContentsScale());
     float clamped_ideal_source_scale_ =
         std::max(min_scale, std::min(ideal_source_scale_, max_scale));
 
@@ skipping 333 matching lines @@
 bool PictureLayerImpl::IsOnActiveOrPendingTree() const {
   return !layer_tree_impl()->IsRecycleTree();
 }
 
 bool PictureLayerImpl::HasValidTilePriorities() const {
   return IsOnActiveOrPendingTree() &&
          is_drawn_render_surface_layer_list_member();
 }
 
 }  // namespace cc