Fix scroll position adjustment for fixed-position layers

cc/trees/property_tree_builder.cc

 // Copyright 2014 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/trees/property_tree_builder.h"
 
 #include <map>
 #include <set>
 
 #include "cc/base/math_util.h"
 #include "cc/layers/layer.h"
 #include "cc/trees/layer_tree_host.h"
 #include "ui/gfx/geometry/point_f.h"
+#include "ui/gfx/geometry/vector2d_conversions.h"
 
 namespace cc {
 
 class LayerTreeHost;
 
 namespace {
 
 struct DataForRecursion {
   TransformTree* transform_tree;
   ClipTree* clip_tree;
   OpacityTree* opacity_tree;
   Layer* transform_tree_parent;
   Layer* transform_fixed_parent;
   Layer* render_target;
   int clip_tree_parent;
   int opacity_tree_parent;
   const Layer* page_scale_layer;
   float page_scale_factor;
   float device_scale_factor;
   bool in_subtree_of_page_scale_application_layer;
   bool should_flatten;
   const gfx::Transform* device_transform;
+  gfx::Vector2d scroll_compensation_adjustment;
 };
 
 static Layer* GetTransformParent(const DataForRecursion& data, Layer* layer) {
   return layer->position_constraint().is_fixed_position()
              ? data.transform_fixed_parent
              : data.transform_tree_parent;
 }
 
 static ClipNode* GetClipParent(const DataForRecursion& data, Layer* layer) {
   const bool inherits_clip = !layer->parent() || !layer->clip_parent();
@@ skipping 88 matching lines @@
     if (layer->scroll_parent()) {
       gfx::Transform to_parent;
       Layer* source = layer->parent();
       parent_offset += source->offset_to_transform_parent();
       data_from_ancestor.transform_tree->ComputeTransform(
           source->transform_tree_index(),
           transform_parent->transform_tree_index(), &to_parent);
       parent_offset += to_parent.To2dTranslation();
     } else if (!is_fixed) {
       parent_offset = transform_parent->offset_to_transform_parent();
-    } else if (data_from_ancestor.transform_tree_parent !=
-               data_from_ancestor.transform_fixed_parent) {
-      gfx::Vector2dF fixed_offset = data_from_ancestor.transform_tree_parent
-                                        ->offset_to_transform_parent();
-      gfx::Transform parent_to_parent;
-      data_from_ancestor.transform_tree->ComputeTransform(
-          data_from_ancestor.transform_tree_parent->transform_tree_index(),
-          data_from_ancestor.transform_fixed_parent->transform_tree_index(),
-          &parent_to_parent);
+    } else {
+      if (data_from_ancestor.transform_tree_parent !=
+          data_from_ancestor.transform_fixed_parent) {
+        gfx::Vector2dF fixed_offset = data_from_ancestor.transform_tree_parent
+                                          ->offset_to_transform_parent();
+        gfx::Transform parent_to_parent;
+        data_from_ancestor.transform_tree->ComputeTransform(
+            data_from_ancestor.transform_tree_parent->transform_tree_index(),
+            data_from_ancestor.transform_fixed_parent->transform_tree_index(),
+            &parent_to_parent);
 
-      fixed_offset += parent_to_parent.To2dTranslation();
-      parent_offset += fixed_offset;
+        fixed_offset += parent_to_parent.To2dTranslation();
+        parent_offset += fixed_offset;
+      }
+      parent_offset += data_from_ancestor.scroll_compensation_adjustment;
     }
   }
 
   if (layer->IsContainerForFixedPositionLayers() || is_root)
     data_for_children->transform_fixed_parent = layer;
   data_for_children->transform_tree_parent = layer;
 
+  if (layer->IsContainerForFixedPositionLayers() || is_fixed)
+    data_for_children->scroll_compensation_adjustment = gfx::Vector2d();

[ajuma, 2015/03/26 18:03:25]

Does clearing here when is_fixed is true work when a fixed position layer has a
fixed position descendant (which has the same container)?

[Ian Vollick, 2015/03/30 17:51:25]

It does, yeah. The current GraphicsLayer tree is constructed in such a way that
fixed pos layers will descend from one another (as it's a stacking context) and
would therefore "descendant" fixed pos layers would get compensated by virtue of
their ancestor's compensation. This matches that behavior. For the CDP analog to
this, see the computation of
current_layer_resets_scroll_compensation_for_descendants in LTHC's
ComputeScrollCompensationMatrixForChildren.

+  data_for_children->scroll_compensation_adjustment +=
+      gfx::ToRoundedVector2d(layer->ScrollCompensationAdjustment());

[ajuma, 2015/03/26 18:03:25]

It's not obvious that rounding is the right thing to do here. Does this work
when the compensation is smaller than 0.5? For example, suppose we have two
scrollers, each with scroll compensation adjustment 0.1. Rounding at each step
will mean data_for_children has an adjustment value of 2. Is this correct?

[Ian Vollick, 2015/03/30 17:51:25]

You're right -- this isn't the right place for it. What's happening here was an
attempt to mimic the tweaking of the inherited scroll componsation adjustment to
account for scroll snapping. It turns out that this is much easier to do by
simply saving the scroll snap and applying it directly which I've done in the
latest patch.

+
   if (!requires_node) {
     data_for_children->should_flatten |= layer->should_flatten_transform();
     gfx::Vector2dF local_offset = layer->position().OffsetFromOrigin() +
                                   layer->transform().To2dTranslation();
     layer->set_offset_to_transform_parent(parent_offset + local_offset);
     layer->set_should_flatten_transform_from_property_tree(
         data_from_ancestor.should_flatten);
     layer->set_transform_tree_index(transform_parent->transform_tree_index());
     return;
   }
@@ skipping 146 matching lines @@
   data_for_recursion.device_transform = &device_transform;
 
   ClipNode root_clip;
   root_clip.data.clip = viewport;
   root_clip.data.transform_id = 0;
   data_for_recursion.clip_tree_parent = clip_tree->Insert(root_clip, 0);
   BuildPropertyTreesInternal(root_layer, data_for_recursion);
 }
 
 }  // namespace cc