Made top controls work with virtual viewport pinch-to-zoom. (Chromium)

cc/trees/layer_tree_host_impl.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/trees/layer_tree_host_impl.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/basictypes.h"
@@ skipping 57 matching lines @@
 #include "cc/trees/occlusion_tracker.h"
 #include "cc/trees/single_thread_proxy.h"
 #include "cc/trees/tree_synchronizer.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "ui/gfx/frame_time.h"
 #include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/gfx/vector2d_conversions.h"
 
+namespace {
+
+// Small helper class that saves the current viewport location as the user sees
+// it and resets to the same location on destruction.
+class ViewportAnchor {
+ public:
+  ViewportAnchor(cc::LayerImpl* inner_scroll, cc::LayerImpl* outer_scroll)
+  : inner_(inner_scroll)
+  , outer_(outer_scroll) {
+    viewportInContentCoordinates_ = inner_->TotalScrollOffset();
+
+    if (outer_)
+      viewportInContentCoordinates_ += outer_->TotalScrollOffset();
+  }
+
+  ~ViewportAnchor() {

[aelias_OOO_until_Jul13, 2014/10/09 20:56:16]

I'd prefer this to be an explicit method call rather than a destructor since
this doesn't free resources in the usual pattern, so it would be more readable
at the calling site.

[bokan, 2014/10/09 21:51:37]

Done.

+    // No-op scroll to force clamp to maxScrollOffset.
+    inner_->ScrollBy(gfx::Vector2dF());

[aelias_OOO_until_Jul13, 2014/10/09 20:55:13]

Please call LayerImpl::ClampScrollToMaxScrollOffset here instead.

[bokan, 2014/10/09 21:51:37]

Done.

+    gfx::ScrollOffset viewport_location = inner_->TotalScrollOffset();
+
+    if (outer_) {

[aelias_OOO_until_Jul13, 2014/10/09 20:55:13]

These if (outer_) statements are mainly what makes this look complicated.  Are
those for the old path without pinch viewport?  Can we just not call this method
in that case according to the setting (which we can do if this is no longer a
destructor).

[bokan, 2014/10/09 21:51:37]

Done. Doesn't even need a guard since it's below the if(!outer_container)
early-out.

+      outer_->ScrollBy(gfx::Vector2dF());
+      viewport_location += outer_->TotalScrollOffset();
+    }
+
+    gfx::Vector2dF delta =
+        viewportInContentCoordinates_.DeltaFrom(viewport_location);
+
+    if (outer_)
+      delta = outer_->ScrollBy(delta);
+
+    inner_->ScrollBy(delta);
+  }
+
+ private:
+  cc::LayerImpl* inner_;
+  cc::LayerImpl* outer_;
+  gfx::ScrollOffset viewportInContentCoordinates_;

[aelias_OOO_until_Jul13, 2014/10/09 20:55:13]

nit: camel-case not Chromium style

[bokan, 2014/10/09 21:51:37]

Done.

+};
+
+}  // namespace
+
 namespace cc {
 namespace {
 
 void DidVisibilityChange(LayerTreeHostImpl* id, bool visible) {
   if (visible) {
     TRACE_EVENT_ASYNC_BEGIN1("webkit",
                              "LayerTreeHostImpl::SetVisible",
                              id,
                              "LayerTreeHostImpl",
                              id);
@@ skipping 1558 matching lines @@
 }
 
 void LayerTreeHostImpl::WillBeginImplFrame(const BeginFrameArgs& args) {
   // Sample the frame time now. This time will be used for updating animations
   // when we draw.
   UpdateCurrentBeginFrameArgs(args);
   // Cache the begin impl frame interval
   begin_impl_frame_interval_ = args.interval;
 }
 
-void LayerTreeHostImpl::UpdateInnerViewportContainerSize() {
-  LayerImpl* container_layer = active_tree_->InnerViewportContainerLayer();
-  if (!container_layer)
+void LayerTreeHostImpl::UpdateViewportContainerSizes() {
+  LayerImpl* inner_container = active_tree_->InnerViewportContainerLayer();
+  LayerImpl* outer_container = active_tree_->OuterViewportContainerLayer();
+
+  if (!inner_container || !top_controls_manager_)
     return;
 
-  if (top_controls_manager_) {
-    container_layer->SetBoundsDelta(
+  {
+    ViewportAnchor anchor(InnerViewportScrollLayer(),

[aelias_OOO_until_Jul13, 2014/10/09 03:51:36]

Hmm, could you describe the clamping problem that led you to create this
anchoring machinery?  It seems complicated and I'm wondering if we could find a
better way.  But I'd need to understand the problem space better first.

[bokan, 2014/10/09 13:30:13]

These issues occur when the viewports are fully scrolled, changing their bounds
may place them outside of MaxScrollOffset. There's two cases to consider:

The top controls hide - both viewports increase in size. The inner viewport
should remain bounded by the outer, but the outer viewport getting larger means
that it'll be scrolled up to stay within the document bounds. The anchor moves
the inner viewport down so we stay at the bottom of the document.

Top controls show - both viewports decrease in size. When you're zoomed in, the
outer viewport will shrink by more than the inner viewport so it'll push the
inner viewport up. In this case, the anchor will move the outer viewport down to
compensate.

I was previously applying the counter scrolls explicitly in this method but it
ended up looking much messier. IMO, the anchor is actually quite simple, if a
little verbose.

[aelias_OOO_until_Jul13, 2014/10/09 20:55:13]

OK, seems alright.  Looks like an unavoidable consequence of making the outer
viewport aspect ratio always match the inner.  This anchoring could be avoided
by changing the sizing policy when pinched, but upon further thought that would
just lead to even more messy problems particularly with the synchronization with
Blink, so this is probably the best approach.

+                          OuterViewportScrollLayer());
+
+    // Adjust the inner viewport by shrinking/expanding the container to account
+    // for the change in top controls height since the last Resize from Blink.
+    inner_container->SetBoundsDelta(
         gfx::Vector2dF(0, active_tree_->top_controls_layout_height() -
             active_tree_->total_top_controls_content_offset()));
+
+    if (!outer_container || outer_container->bounds().IsEmpty())
+      return;
+
+    // Adjust the outer viewport container as well, since adjusting only the
+    // inner may cause its bounds to exceed those of the outer, causing scroll
+    // clamping. We adjust it so it maintains the same aspect ratio as the
+    // inner viewport.
+    float aspect_ratio = inner_container->bounds().width() /
+        inner_container->bounds().height();
+    float target_height = outer_container->bounds().width() / aspect_ratio;
+    float current_outer_height = outer_container->bounds().height() -
+        outer_container->bounds_delta().y();
+    gfx::Vector2dF delta(0, target_height - current_outer_height);
+
+    outer_container->SetBoundsDelta(delta);
+    active_tree_->InnerViewportScrollLayer()->SetBoundsDelta(delta);
   }
 }
 
 void LayerTreeHostImpl::SetTopControlsLayoutHeight(float height) {
   if (active_tree_->top_controls_layout_height() == height)
     return;
 
   active_tree_->set_top_controls_layout_height(height);
-  UpdateInnerViewportContainerSize();
+  UpdateViewportContainerSizes();
   SetFullRootLayerDamage();
 }
 
 void LayerTreeHostImpl::DidLoseOutputSurface() {
   if (resource_provider_)
     resource_provider_->DidLoseOutputSurface();
   client_->DidLoseOutputSurfaceOnImplThread();
 }
 
 bool LayerTreeHostImpl::HaveRootScrollLayer() const {
@@ skipping 100 matching lines @@
 
     active_tree_->SetRootLayerScrollOffsetDelegate(
         root_layer_scroll_offset_delegate_);
 
     if (top_controls_manager_) {
       top_controls_manager_->SetControlsTopOffset(
           active_tree_->total_top_controls_content_offset() -
           top_controls_manager_->top_controls_height());
     }
 
-    UpdateInnerViewportContainerSize();
+    UpdateViewportContainerSizes();
   } else {
     active_tree_->ProcessUIResourceRequestQueue();
   }
 
   active_tree_->DidBecomeActive();
   ActivateAnimations();
   if (settings_.impl_side_painting)
     client_->RenewTreePriority();
 
   client_->OnCanDrawStateChanged(CanDraw());
@@ skipping 342 matching lines @@
 
 void LayerTreeHostImpl::SetViewportSize(const gfx::Size& device_viewport_size) {
   if (device_viewport_size == device_viewport_size_)
     return;
 
   if (pending_tree_)
     active_tree_->SetViewportSizeInvalid();
 
   device_viewport_size_ = device_viewport_size;
 
-  UpdateInnerViewportContainerSize();
+  UpdateViewportContainerSizes();
   client_->OnCanDrawStateChanged(CanDraw());
   SetFullRootLayerDamage();
   active_tree_->set_needs_update_draw_properties();
 }
 
 void LayerTreeHostImpl::SetOverhangUIResource(
     UIResourceId overhang_ui_resource_id,
     const gfx::Size& overhang_ui_resource_size) {
   overhang_ui_resource_id_ = overhang_ui_resource_id;
   overhang_ui_resource_size_ = overhang_ui_resource_size;
@@ skipping 30 matching lines @@
     return DeviceViewport();
 
   return external_clip_;
 }
 
 const gfx::Transform& LayerTreeHostImpl::DrawTransform() const {
   return external_transform_;
 }
 
 void LayerTreeHostImpl::DidChangeTopControlsPosition() {
-  UpdateInnerViewportContainerSize();
+  UpdateViewportContainerSizes();
   SetNeedsRedraw();
   SetNeedsAnimate();
   active_tree_->set_needs_update_draw_properties();
   SetFullRootLayerDamage();
 }
 
 void LayerTreeHostImpl::SetControlsTopOffset(float offset) {
   float current_top_offset = active_tree_->top_controls_content_offset() -
       top_controls_manager_->top_controls_height();
   active_tree_->set_top_controls_delta(offset - current_top_offset);
@@ skipping 275 matching lines @@
   return actual_viewport_end_point - viewport_point;
 }
 
 static gfx::Vector2dF ScrollLayerWithLocalDelta(LayerImpl* layer_impl,
     const gfx::Vector2dF& local_delta) {
   gfx::Vector2dF previous_delta(layer_impl->ScrollDelta());
   layer_impl->ScrollBy(local_delta);
   return layer_impl->ScrollDelta() - previous_delta;
 }
 
+bool LayerTreeHostImpl::ShouldTopControlsConsumeScroll(
+    const gfx::Vector2dF& scroll_delta) const {
+  if (!top_controls_manager_ || !CurrentlyScrollingLayer())

[aelias_OOO_until_Jul13, 2014/10/09 20:55:13]

!CurrentlyScrollingLayer() condition is unnecessary, please omit or DCHECK it.

[bokan, 2014/10/09 21:51:37]

Done.

+    return false;
+
+  // Always consume if it's in the direction to show the top controls.
+  if (scroll_delta.y() < 0)
+    return true;
+
+  if (CurrentlyScrollingLayer() != InnerViewportScrollLayer() &&
+      CurrentlyScrollingLayer() != OuterViewportScrollLayer())
+    return false;
+
+  if (InnerViewportScrollLayer()->MaxScrollOffset().y() > 0)
+    return true;
+
+  if (OuterViewportScrollLayer() &&
+      OuterViewportScrollLayer()->MaxScrollOffset().y() > 0)
+    return true;
+
+  return false;
+}
+
 bool LayerTreeHostImpl::ScrollBy(const gfx::Point& viewport_point,
                                  const gfx::Vector2dF& scroll_delta) {
   TRACE_EVENT0("cc", "LayerTreeHostImpl::ScrollBy");
   if (!CurrentlyScrollingLayer())
     return false;
 
   gfx::Vector2dF pending_delta = scroll_delta;
   gfx::Vector2dF unused_root_delta;
   bool did_scroll_x = false;
   bool did_scroll_y = false;
   bool did_scroll_top_controls = false;
-  // TODO(wjmaclean) Should we guard against CurrentlyScrollingLayer() == 0
-  // here?
-  bool consume_by_top_controls =
-      top_controls_manager_ &&
-      (((CurrentlyScrollingLayer() == InnerViewportScrollLayer() ||
-         CurrentlyScrollingLayer() == OuterViewportScrollLayer()) &&
-        InnerViewportScrollLayer()->MaxScrollOffset().y() > 0) ||
-       scroll_delta.y() < 0);
+
+  bool consume_by_top_controls = ShouldTopControlsConsumeScroll(scroll_delta);
 
   for (LayerImpl* layer_impl = CurrentlyScrollingLayer();
        layer_impl;
        layer_impl = layer_impl->parent()) {
     if (!layer_impl->scrollable())
       continue;
 
-    if (layer_impl == InnerViewportScrollLayer()) {
-      // Only allow bubble scrolling when the scroll is in the direction to make
-      // the top controls visible.
-      gfx::Vector2dF applied_delta;
-      gfx::Vector2dF excess_delta;
+    if (layer_impl == InnerViewportScrollLayer() ||
+        layer_impl == OuterViewportScrollLayer()) {
       if (consume_by_top_controls) {
-        excess_delta = top_controls_manager_->ScrollBy(pending_delta);
-        applied_delta = pending_delta - excess_delta;
+        gfx::Vector2dF excess_delta =
+            top_controls_manager_->ScrollBy(pending_delta);
+        gfx::Vector2dF applied_delta = pending_delta - excess_delta;
         pending_delta = excess_delta;
         // Force updating of vertical adjust values if needed.
-        if (applied_delta.y() != 0) {
+        if (applied_delta.y() != 0)
           did_scroll_top_controls = true;
-          layer_impl->ScrollbarParametersDidChange(false);

[bokan, 2014/10/08 23:03:55]

I think this is unneeded, it gets called when the layer bounds are changed.

-        }
       }
       // Track root layer deltas for reporting overscroll.
-      unused_root_delta = pending_delta;
+      if (layer_impl == InnerViewportScrollLayer())
+        unused_root_delta = pending_delta;
     }
 
     gfx::Vector2dF applied_delta;
     // Gesture events need to be transformed from viewport coordinates to local
     // layer coordinates so that the scrolling contents exactly follow the
     // user's finger. In contrast, wheel events represent a fixed amount of
     // scrolling so we can just apply them directly.
     if (!wheel_scrolling_) {
       float scale_from_viewport_to_screen_space = device_scale_factor_;
       applied_delta =
@@ skipping 823 matching lines @@
 }
 
 void LayerTreeHostImpl::UnregisterPictureLayerImpl(PictureLayerImpl* layer) {
   std::vector<PictureLayerImpl*>::iterator it =
       std::find(picture_layers_.begin(), picture_layers_.end(), layer);
   DCHECK(it != picture_layers_.end());
   picture_layers_.erase(it);
 }
 
 }  // namespace cc