Pinch/Zoom Infrastructure & Plumbing CL

cc/layers/layer_impl.cc

Index: cc/layers/layer_impl.cc
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index 74a9b8468d79c673288fb29cb86459972b99d2c9..676a800891ecf634c7b1bccb84b306316cf1550c 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -41,7 +41,7 @@ LayerImpl::LayerImpl(LayerTreeImpl* tree_impl, int id)
       anchor_point_(0.5f, 0.5f),
       anchor_point_z_(0.f),
       scroll_offset_delegate_(NULL),
-      scrollable_(false),
+      scroll_clip_layer_(NULL),
       should_scroll_on_main_thread_(false),
       have_wheel_event_handlers_(false),
       user_scrollable_horizontal_(true),
@@ -63,9 +63,7 @@ LayerImpl::LayerImpl(LayerTreeImpl* tree_impl, int id)
       opacity_(1.0),
       blend_mode_(SkXfermode::kSrcOver_Mode),
       draw_depth_(0.f),
-      current_draw_mode_(DRAW_MODE_NONE),
-      horizontal_scrollbar_layer_(NULL),
-      vertical_scrollbar_layer_(NULL) {
+      current_draw_mode_(DRAW_MODE_NONE) {
   DCHECK_GT(layer_id_, 0);
   DCHECK(layer_tree_impl_);
   layer_tree_impl_->RegisterLayer(this);
@@ -359,7 +357,7 @@ void LayerImpl::SetSentScrollDelta(gfx::Vector2d sent_scroll_delta) {
 gfx::Vector2dF LayerImpl::ScrollBy(gfx::Vector2dF scroll) {
   DCHECK(scrollable());
   gfx::Vector2dF min_delta = -scroll_offset_;
-  gfx::Vector2dF max_delta = max_scroll_offset_ - scroll_offset_;
+  gfx::Vector2dF max_delta = MaxScrollOffset() - scroll_offset_;
   // Clamp new_delta so that position + delta stays within scroll bounds.
   gfx::Vector2dF new_delta = (ScrollDelta() + scroll);
   new_delta.SetToMax(min_delta);
@@ -367,9 +365,14 @@ gfx::Vector2dF LayerImpl::ScrollBy(gfx::Vector2dF scroll) {
   gfx::Vector2dF unscrolled =
       ScrollDelta() + scroll - new_delta;
   SetScrollDelta(new_delta);
+
   return unscrolled;
 }
 
+void LayerImpl::SetScrollClipLayer(int scroll_clip_layer_id) {
+  scroll_clip_layer_ = layer_tree_impl()->LayerById(scroll_clip_layer_id);
+}
+
 void LayerImpl::ApplySentScrollDeltasFromAbortedCommit() {
   // Pending tree never has sent scroll deltas
   DCHECK(layer_tree_impl()->IsActiveTree());
@@ -459,7 +462,8 @@ InputHandler::ScrollStatus LayerImpl::TryScroll(
     return InputHandler::ScrollIgnored;
   }
 
-  if (max_scroll_offset_.x() <= 0 && max_scroll_offset_.y() <= 0) {
+  gfx::Vector2d max_scroll_offset = MaxScrollOffset();
+  if (max_scroll_offset.x() <= 0 && max_scroll_offset.y() <= 0) {
     TRACE_EVENT0("cc",
                  "LayerImpl::tryScroll: Ignored. Technically scrollable,"
                  " but has no affordance in either direction.");
@@ -531,15 +535,14 @@ void LayerImpl::PushPropertiesTo(LayerImpl* layer) {
   layer->SetSublayerTransform(sublayer_transform_);
   layer->SetTransform(transform_);
 
-  layer->SetScrollable(scrollable_);
+  layer->SetScrollClipLayer(scroll_clip_layer_ ? scroll_clip_layer_->id()
+                                        : Layer::INVALID_ID);
   layer->set_user_scrollable_horizontal(user_scrollable_horizontal_);
   layer->set_user_scrollable_vertical(user_scrollable_vertical_);
   layer->SetScrollOffsetAndDelta(
       scroll_offset_, layer->ScrollDelta() - layer->sent_scroll_delta());
   layer->SetSentScrollDelta(gfx::Vector2d());
 
-  layer->SetMaxScrollOffset(max_scroll_offset_);
-
   LayerImpl* scroll_parent = NULL;
   if (scroll_parent_)
     scroll_parent = layer->layer_tree_impl()->LayerById(scroll_parent_->id());
@@ -612,8 +615,8 @@ base::DictionaryValue* LayerImpl::LayerTreeAsJson() const {
   result->SetDouble("Opacity", opacity());
   result->SetBoolean("ContentsOpaque", contents_opaque_);
 
-  if (scrollable_)
-    result->SetBoolean("Scrollable", scrollable_);
+  if (scrollable())
+    result->SetBoolean("Scrollable", true);
 
   if (have_wheel_event_handlers_)
     result->SetBoolean("WheelHandler", have_wheel_event_handlers_);
@@ -721,6 +724,7 @@ void LayerImpl::SetBounds(gfx::Size bounds) {
 
   bounds_ = bounds;
 
+  ScrollbarParametersDidChange();
   if (masks_to_bounds())
     NoteLayerPropertyChangedForSubtree();
   else
@@ -988,44 +992,6 @@ void LayerImpl::CalculateContentsScale(
   *content_bounds = this->content_bounds();
 }
 
-void LayerImpl::UpdateScrollbarPositions() {
-  gfx::Vector2dF current_offset = scroll_offset_ + ScrollDelta();
-
-  gfx::RectF viewport(PointAtOffsetFromOrigin(current_offset), bounds_);
-  gfx::SizeF scrollable_size(max_scroll_offset_.x() + bounds_.width(),
-                             max_scroll_offset_.y() + bounds_.height());
-  if (horizontal_scrollbar_layer_) {
-    horizontal_scrollbar_layer_->SetCurrentPos(current_offset.x());
-    horizontal_scrollbar_layer_->SetMaximum(max_scroll_offset_.x());
-    horizontal_scrollbar_layer_->SetVisibleToTotalLengthRatio(
-        viewport.width() / scrollable_size.width());
-  }
-  if (vertical_scrollbar_layer_) {
-    vertical_scrollbar_layer_->SetCurrentPos(current_offset.y());
-    vertical_scrollbar_layer_->SetMaximum(max_scroll_offset_.y());
-    vertical_scrollbar_layer_->SetVisibleToTotalLengthRatio(
-        viewport.height() / scrollable_size.height());
-  }
-
-  if (current_offset == last_scroll_offset_)
-    return;
-  last_scroll_offset_ = current_offset;
-
-  if (scrollbar_animation_controller_) {
-    bool should_animate = scrollbar_animation_controller_->DidScrollUpdate(
-        layer_tree_impl_->CurrentPhysicalTimeTicks());
-    if (should_animate)
-      layer_tree_impl_->StartScrollbarAnimation();
-  }
-
-  // Get the current_offset_.y() value for a sanity-check on scrolling
-  // benchmark metrics. Specifically, we want to make sure
-  // BasicMouseWheelSmoothScrollGesture has proper scroll curves.
-  if (layer_tree_impl()->IsActiveTree()) {
-    TRACE_COUNTER_ID1("gpu", "scroll_offset_y", this->id(), current_offset.y());
-  }
-}
-
 void LayerImpl::SetScrollOffsetDelegate(
     LayerScrollOffsetDelegate* scroll_offset_delegate) {
   // Having both a scroll parent and a scroll offset delegate is unsupported.
@@ -1036,10 +1002,8 @@ void LayerImpl::SetScrollOffsetDelegate(
   }
   gfx::Vector2dF total_offset = TotalScrollOffset();
   scroll_offset_delegate_ = scroll_offset_delegate;
-  if (scroll_offset_delegate_) {
-    scroll_offset_delegate_->SetMaxScrollOffset(max_scroll_offset_);
+  if (scroll_offset_delegate_)
     scroll_offset_delegate_->SetTotalScrollOffset(total_offset);
-  }
 }
 
 bool LayerImpl::IsExternalFlingActive() const {
@@ -1055,6 +1019,8 @@ void LayerImpl::SetScrollOffsetAndDelta(gfx::Vector2d scroll_offset,
                                         gfx::Vector2dF scroll_delta) {
   bool changed = false;
 
+  last_scroll_offset_ = scroll_offset;
+
   if (scroll_offset_ != scroll_offset) {
     changed = true;
     scroll_offset_ = scroll_offset;
@@ -1089,7 +1055,7 @@ void LayerImpl::SetScrollOffsetAndDelta(gfx::Vector2d scroll_offset,
 
   if (changed) {
     NoteLayerPropertyChangedForSubtree();
-    UpdateScrollbarPositions();
+    ScrollbarParametersDidChange();
   }
 }
 
@@ -1125,16 +1091,161 @@ void LayerImpl::DidBeginTracing() {}
 
 void LayerImpl::DidLoseOutputSurface() {}
 
-void LayerImpl::SetMaxScrollOffset(gfx::Vector2d max_scroll_offset) {
-  if (max_scroll_offset_ == max_scroll_offset)
+gfx::Vector2d LayerImpl::MaxScrollOffset() const {

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

Just checking: have you verified this logic also works with scrollable
subregions, since you're now ignoring what Blink specifies for those?

[wjmaclean, 2014/01/16 15:07:32]

I'm not sure that I understand what you mean by "ignoring".

Yes, I believe this works with scrollable sub-regions (e.g. an embedded iFrame).

+  if (!scroll_clip_layer_)
+    return gfx::Vector2d();
+
+  LayerImpl const* page_scale_layer = layer_tree_impl()->page_scale_layer();
+  DCHECK(this != page_scale_layer);
+  DCHECK(scroll_clip_layer_);
+  DCHECK(this != layer_tree_impl()->InnerViewportScrollLayer()
+         || IsContainerForFixedPositionLayers());
+
+  // TODO(wjmaclean) Find out why Blink sometimes gives us empty bounds on the
+  //                 scroll layer.
+  //
+  // For now we will use the content layer bounds as a fallback.
+  // It's apparently a known issue that Blink sometimes gives us zero bounds on
+  // the scroll layer, but as the contents layer has no way to directly notify
+  // when its bounds have changed, and since the content layer bounds can be
+  // updated after the scroll layer bounds in a tree sync, it seems better to
+  // only use them as a fall-back.
+  gfx::Size scaled_scroll_bounds(bounds());
+  if (scaled_scroll_bounds.IsEmpty()) {
+    // TODO(wjmaclean) Should we add some metrics collection here to see how
+    // often we end up with empty scroll bounds, and on which platforms?
+    if (children().size())
+      scaled_scroll_bounds = children()[0]->bounds();
+  }
+
+  float scale_factor = 1.f;
+  LayerImpl const* last_layer = 0;
+  for (LayerImpl const* current_layer = this;
+       current_layer != scroll_clip_layer_->parent();

[aelias_OOO_until_Jul13, 2014/01/16 19:49:26]

As one thing that appears like a bug here, you're looking at the clip layer's
transform but that's above the clip layer, not in between the clip layer and
scroll layer.  I think the only thing you may need to consider on the clip layer
is its sublayer_transform (which probably should be looked at for the in-between
layers as well).

[wjmaclean, 2014/01/16 20:13:36]

I wondered about that ... we can exclude the clip layer's transform easily
enough in the processing (though some tests combine the clip layer and page
scale layers, just a nuisance case ...)

+       current_layer = current_layer->parent()) {
+    float current_layer_scale = 1.f;
+
+    const gfx::Transform& layer_transform = current_layer->transform();

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

We don't need to take the other transforms into consideration at all.  They
cannot change the max scroll offset.  Only page scale can, because it resizes
the viewport (CSS scale transforms cannot do that).  So you can delete the code
using layer_transform.

[wjmaclean, 2014/01/16 15:07:32]

You're assuming that this scrolling mechanism is only ever used for viewports,
but it is meant to be more general than that (sub-frames, etc.) In general we
have assumed that scale transformations (other than page scale) can exist
between the clip and the scroll layer.

[aelias_OOO_until_Jul13, 2014/01/16 19:49:26]

OK, I read this as trying to compensate for CSS scale transformations coming
from the web author and my point was that no such compensations are necessary. 
It sounds like you're just trying to make things cleaner and more general at a
CC level then?  If so I can see how this makes  sense and I think I'm ok with
it.

+    if (current_layer == page_scale_layer) {
+      DCHECK(layer_transform.IsIdentity());
+      current_layer_scale = layer_tree_impl()->total_page_scale_factor();
+    } else {
+      // TODO(wjmaclean) Should we allow for translation too?
+      if (layer_transform.IsScale()) {
+        gfx::Vector2dF layer_scale = layer_transform.Scale();
+        // TODO(wjmaclean) Allow for non-isotropic scales.
+        DCHECK(layer_scale.x() == layer_scale.y());
+        current_layer_scale = layer_scale.x();
+      }
+    }
+
+    scale_factor *= current_layer_scale;
+    last_layer = current_layer;
+  }
+  DCHECK(last_layer == scroll_clip_layer_);
+
+  scaled_scroll_bounds.SetSize(
+      scale_factor * scaled_scroll_bounds.width(),
+      scale_factor * scaled_scroll_bounds.height());
+
+  gfx::RectF clip_rect(gfx::PointF(), scroll_clip_layer_->bounds());
+  if (this == layer_tree_impl()->InnerViewportScrollLayer())

[aelias_OOO_until_Jul13, 2014/01/16 19:49:26]

If we are going to go down the path of generality then, we should start to think
about how to avoid this kind of if statement, though that can wait for a later
patch.

[wjmaclean, 2014/01/16 20:13:36]

Yes, we had identified this special case as a weakness, but that's why I'd like
to just do it base on the actual bounds of the layer, plus the transform via the
page scale layer, i.e. not calling ScrollableViewportSize(). In fact, I think we
can avoid making that call at all.

[aelias_OOO_until_Jul13, 2014/01/16 20:36:38]

If you try that, you're going to run into the problem of Android-specific
adjustments for top controls.  It would be nice to bake these into the bounds,
but it's a bit of a can of worms I suggest we do later.

[wjmaclean, 2014/01/16 20:39:41]

Agreed ... I believe that's why it's there for now. I had been hoping we would
bake it into the bonds in a (soon) future patch.

+    clip_rect =
+        gfx::RectF(gfx::PointF(), layer_tree_impl()->ScrollableViewportSize());
+  gfx::Vector2dF max_offset(
+      scaled_scroll_bounds.width() - scroll_clip_layer_->bounds().width(),
+      scaled_scroll_bounds.height() - scroll_clip_layer_->bounds().height());
+  // We need the final scroll offset to be in CSS coords.
+  max_offset.Scale(1 / scale_factor);

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

The old UpdateMaxScrollOffset() logic was simpler than this and didn't do any
explicit scaling at all.  I don't think you need to here either.  It seems to me
that:

inner max scroll offset = OuterViewportSize() - InnerViewportSize()
outer max scroll offset = ScrollableSize() - OuterViewportSize()

and assuming all the methods return values in CSS coordinates, you are done. 
Only the implementation of InnerViewportSize() needs to take page scale into
account.

(InnerViewportSize already exists in the form of the current implementation of
ScrollableViewportSize, and OuterViewportSize is currently called
UnscaledScrollableViewportSize, but I suggest renaming them.)

[wjmaclean, 2014/01/16 15:07:32]

Again, I think the more general model we want to support allows for scaling
between a clip layer and its scroll layer.

+  return gfx::Vector2d(max_offset.x(), max_offset.y());
+}
+
+gfx::Vector2dF LayerImpl::ClampScrollToMaxScrollOffset() {
+  gfx::Vector2dF max_offset = MaxScrollOffset();
+  gfx::Vector2dF old_offset = TotalScrollOffset();
+  gfx::Vector2dF clamped_offset = old_offset;
+
+  clamped_offset.SetToMin(max_offset);
+  clamped_offset.SetToMax(gfx::Vector2d());
+  gfx::Vector2dF delta = clamped_offset - old_offset;
+  if (!delta.IsZero())
+    ScrollBy(delta);
+
+  return delta;
+}
+
+void LayerImpl::SetScrollbarPosition(ScrollbarLayerImplBase* scrollbar_layer,
+                                     LayerImpl* scrollbar_clip_layer) const {
+  DCHECK(scrollbar_layer);
+  LayerImpl* page_scale_layer = layer_tree_impl()->page_scale_layer();
+
+  DCHECK(this != page_scale_layer);
+  DCHECK(scrollbar_clip_layer);
+  DCHECK(this != layer_tree_impl()->InnerViewportScrollLayer()
+         || IsContainerForFixedPositionLayers());
+  gfx::RectF clip_rect(gfx::PointF(), scrollbar_clip_layer->bounds());
+
+  // See comment in MaxScrollOffset() regarding the use of the content layer
+  // bounds here.
+  gfx::RectF scroll_rect(gfx::PointF(), bounds());
+  if (scroll_rect.IsEmpty()) {
+    if (children().size())
+      scroll_rect = gfx::RectF(children()[0]->bounds());
+  }
+
+  if (scroll_rect.size().IsEmpty())
     return;
-  max_scroll_offset_ = max_scroll_offset;
 
-  if (scroll_offset_delegate_)
-    scroll_offset_delegate_->SetMaxScrollOffset(max_scroll_offset_);
+  // TODO(wjmaclean) This computation is nearly identical to the one in
+  // MaxScrollOffset. Find some way to combine these.
+  gfx::Vector2dF current_offset;

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

Likewise, I don't think any scaling is needed here.  Just add together inner and
outer total scroll offsets to get an offset in CSS coordinates.

+  LayerImpl const* last_layer = 0;
+  for (LayerImpl const* current_layer = this;
+       current_layer != scrollbar_clip_layer->parent();
+       current_layer = current_layer->parent()) {
+    const gfx::Transform& layer_transform = current_layer->transform();
+    if (current_layer == page_scale_layer) {
+      DCHECK(layer_transform.IsIdentity());
+      float scale_factor = layer_tree_impl()->total_page_scale_factor();
+      current_offset.Scale(scale_factor);
+      scroll_rect.Scale(scale_factor);
+    } else {
+      gfx::Vector2dF new_offset =
+          current_layer->scroll_offset() + current_layer->ScrollDelta();
+      if (layer_transform.IsScale()) {
+        gfx::Vector2dF layer_scale = layer_transform.Scale();
+        DCHECK(layer_scale.x() == layer_scale.y());
+        new_offset.Scale(layer_scale.x(), layer_scale.y());
+      }
+      current_offset += new_offset;
+    }
+    last_layer = current_layer;
+  }
+  DCHECK(last_layer == scrollbar_clip_layer);
+
+  scrollbar_layer->SetVerticalAdjust(layer_tree_impl()->VerticalAdjust(this));
+  if (scrollbar_layer->orientation() == HORIZONTAL) {
+    float visible_ratio = clip_rect.width() / scroll_rect.width();
+    scrollbar_layer->SetCurrentPos(current_offset.x());
+    scrollbar_layer->SetMaximum(scroll_rect.width() - clip_rect.width());
+    scrollbar_layer->SetVisibleToTotalLengthRatio(visible_ratio);
+  } else {
+    float visible_ratio = clip_rect.height() / scroll_rect.height();
+    scrollbar_layer->SetCurrentPos(current_offset.y());
+    scrollbar_layer->SetMaximum(scroll_rect.height() - clip_rect.height());
+    scrollbar_layer->SetVisibleToTotalLengthRatio(visible_ratio);
+  }
 
-  layer_tree_impl()->set_needs_update_draw_properties();
-  UpdateScrollbarPositions();
+  // TODO(wjmaclean) The scrollbar animator for the pinch-zoom scrollbars should
+  // activate for every scroll on the main frame, not just the scrolls that move
+  // the pinch virtual viewport (i.e. trigger from either inner or outer
+  // viewport).
+  if (scrollbar_animation_controller_) {
+    bool should_animate = scrollbar_animation_controller_->DidScrollUpdate(
+        layer_tree_impl_->CurrentPhysicalTimeTicks());
+    if (should_animate)
+      layer_tree_impl_->StartScrollbarAnimation();
+  }
 }
 
 void LayerImpl::DidBecomeActive() {
@@ -1143,8 +1254,7 @@ void LayerImpl::DidBecomeActive() {
     return;
   }
 
-  bool need_scrollbar_animation_controller = horizontal_scrollbar_layer_ ||
-                                             vertical_scrollbar_layer_;
+  bool need_scrollbar_animation_controller = scrollable() && scrollbars_;
   if (!need_scrollbar_animation_controller) {
     scrollbar_animation_controller_.reset();
     return;
@@ -1177,18 +1287,50 @@ void LayerImpl::DidBecomeActive() {
     break;
   }
 }
-void LayerImpl::SetHorizontalScrollbarLayer(
-    ScrollbarLayerImplBase* scrollbar_layer) {
-  horizontal_scrollbar_layer_ = scrollbar_layer;
-  if (horizontal_scrollbar_layer_)
-    horizontal_scrollbar_layer_->set_scroll_layer_id(id());
+
+void LayerImpl::ClearScrollbars() {
+  if (!scrollbars_)
+    return;
+
+  scrollbars_.reset(NULL);
+}
+
+void LayerImpl::AddScrollbar(ScrollbarLayerImplBase* layer) {
+  DCHECK(layer);
+  if (!scrollbars_)
+    scrollbars_.reset(new ScrollbarSet());
+
+  scrollbars_->insert(layer);
+}
+
+void LayerImpl::RemoveScrollbar(ScrollbarLayerImplBase* layer) {
+  DCHECK(scrollbars_);
+  DCHECK(layer);
+
+  scrollbars_->erase(layer);
+  if (scrollbars_->empty())
+    scrollbars_.reset();
+}
+
+bool LayerImpl::HasScrollbar(ScrollbarOrientation orientation) const {
+  if (!scrollbars_)
+    return false;
+
+  for (ScrollbarSet::iterator it = scrollbars_->begin();
+       it != scrollbars_->end(); ++it)
+    if ((*it)->orientation() == orientation)
+      return true;
+
+  return false;
 }
 
-void LayerImpl::SetVerticalScrollbarLayer(
-    ScrollbarLayerImplBase* scrollbar_layer) {
-  vertical_scrollbar_layer_ = scrollbar_layer;
-  if (vertical_scrollbar_layer_)
-    vertical_scrollbar_layer_->set_scroll_layer_id(id());
+void LayerImpl::ScrollbarParametersDidChange() {
+  if (!scrollbars_)
+    return;
+
+  for (ScrollbarSet::iterator it = scrollbars_->begin();
+       it != scrollbars_->end(); ++it)
+    (*it)->ScrollbarParametersDidChange();
 }
 
 void LayerImpl::AsValueInto(base::DictionaryValue* state) const {