Switch RootLayerScrolls to use root transform and scroll properties

third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp

 // Copyright 2015 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 "core/paint/PaintPropertyTreeBuilder.h"
 
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "core/layout/LayoutInline.h"
@@ skipping 34 matching lines @@
         return rootScroll;
     }
 }
 
 PaintPropertyTreeBuilderContext PaintPropertyTreeBuilder::setupInitialContext()
 {
     PaintPropertyTreeBuilderContext context;
 
     context.current.clip = context.absolutePosition.clip = context.fixedPosition.clip = rootClipNode();
     context.currentEffect = rootEffectNode();
-
-    // TODO(pdr): Update the root layer scrolling paths to use the static root nodes for transform and scroll.
-    if (RuntimeEnabledFeatures::rootLayerScrollingEnabled())
-        return context;
-
     context.current.transform = context.absolutePosition.transform = context.fixedPosition.transform = rootTransformNode();
     context.current.scroll = context.absolutePosition.scroll = context.fixedPosition.scroll = rootScrollNode();
 
     // Ensure scroll tree properties are reset. They will be rebuilt during the tree walk.
     rootScrollNode()->clearMainThreadScrollingReasons();
 
     return context;
 }
 
 void createOrUpdateFrameViewPreTranslation(FrameView& frameView,
@@ skipping 52 matching lines @@
         LayoutView* layoutView = frameView.layoutView();
         if (!layoutView)
             return;
 
         TransformationMatrix frameTranslate;
         frameTranslate.translate(
             frameView.x() + layoutView->location().x() + context.current.paintOffset.x(),
             frameView.y() + layoutView->location().y() + context.current.paintOffset.y());
         context.current.transform = layoutView->getMutableForPainting().ensureObjectPaintProperties().createOrUpdatePaintOffsetTranslation(
             context.current.transform, frameTranslate, FloatPoint3D());
-        context.current.scroll = layoutView->getMutableForPainting().ensureObjectPaintProperties().createOrUpdateScroll(
-            context.current.scroll, context.current.transform, IntSize(), IntSize(), false, false);
         context.current.paintOffset = LayoutPoint();
         context.current.renderingContextID = 0;
         context.current.shouldFlattenInheritedTransform = true;
         context.absolutePosition = context.current;
         context.containerForAbsolutePosition = nullptr; // This will get set in updateOutOfFlowContext().
         context.fixedPosition = context.current;
         return;
     }
 
     TransformationMatrix frameTranslate;
     frameTranslate.translate(frameView.x() + context.current.paintOffset.x(), frameView.y() + context.current.paintOffset.y());
     createOrUpdateFrameViewPreTranslation(frameView, context.current.transform, frameTranslate, FloatPoint3D());
 
     FloatRoundedRect contentClip(IntRect(IntPoint(), frameView.visibleContentSize()));
     createOrUpdateFrameViewContentClip(frameView, context.current.clip, frameView.preTranslation(), contentClip);
 
     DoubleSize scrollOffset = frameView.scrollOffsetDouble();

[szager1, 2016/09/26 19:57:54]

DoubleSize scrollOffset =
frameView.layoutViewportScrollableArea()->scrollOffsetDouble();

[pdr., 2016/09/26 20:30:56]

Why change this? It matches the code before this change so I'd prefer to do it
later.

-    TransformationMatrix frameScroll;
-    frameScroll.translate(-scrollOffset.width(), -scrollOffset.height());
-    // TODO(pdr): A scroll translation should not be needed when frameView.isScrollable() is false.
-    createOrUpdateFrameViewScrollTranslation(frameView, frameView.preTranslation(), frameScroll, FloatPoint3D());
+    if (frameView.isScrollable() || !scrollOffset.isZero()) {
+        TransformationMatrix frameScroll;
+        frameScroll.translate(-scrollOffset.width(), -scrollOffset.height());
+        createOrUpdateFrameViewScrollTranslation(frameView, frameView.preTranslation(), frameScroll, FloatPoint3D());
 
-    if (frameView.isScrollable()) {
         IntSize scrollClip = frameView.visibleContentSize();
         IntSize scrollBounds = frameView.contentsSize();
         bool userScrollableHorizontal = frameView.userInputScrollable(HorizontalScrollbar);
         bool userScrollableVertical = frameView.userInputScrollable(VerticalScrollbar);
         createOrUpdateFrameViewScroll(frameView, context.current.scroll, frameView.scrollTranslation(), scrollClip, scrollBounds, userScrollableHorizontal, userScrollableVertical);
     } else {
+        frameView.setScrollTranslation(nullptr);
         frameView.setScroll(nullptr);
     }
 
     // Initialize the context for current, absolute and fixed position cases.
     // They are the same, except that scroll translation does not apply to
     // fixed position descendants.
     ScrollPaintPropertyNode* initialScroll = context.current.scroll;
-    context.current.transform = frameView.scrollTranslation();
+    context.current.transform = frameView.scrollTranslation() ? frameView.scrollTranslation() : frameView.preTranslation();
     context.current.paintOffset = LayoutPoint();
     context.current.clip = frameView.contentClip();
     context.current.scroll = frameView.scroll() ? frameView.scroll() : initialScroll;
     context.current.renderingContextID = 0;
     context.current.shouldFlattenInheritedTransform = true;
     context.absolutePosition = context.current;
     context.containerForAbsolutePosition = nullptr;
     context.fixedPosition = context.current;
     context.fixedPosition.transform = frameView.preTranslation();
     context.fixedPosition.scroll = initialScroll;
@@ skipping 189 matching lines @@
     if (box.hasControlClip()) {
         clipRect = box.controlClipRect(context.current.paintOffset);
     } else if (box.hasOverflowClip() || (box.isSVGRoot() && toLayoutSVGRoot(box).shouldApplyViewportClip())) {
         clipRect = box.overflowClipRect(context.current.paintOffset);
     } else {
         if (ObjectPaintProperties* properties = object.getMutableForPainting().objectPaintProperties())
             properties->clearOverflowClip();
         return;
     }
 
+    // TODO(pdr): We should create an entry in ObjectPaintProperties for border radius to ensure
+    // the owership model of properties is simple for incremental updates.
     // This need to be in top-level block to hold the reference until we finish creating the normal clip node.
     RefPtr<ClipPaintPropertyNode> borderRadiusClip;
     if (box.styleRef().hasBorderRadius()) {
         auto innerBorder = box.styleRef().getRoundedInnerBorderFor(
             LayoutRect(context.current.paintOffset, box.size()));
         borderRadiusClip = ClipPaintPropertyNode::create(context.current.clip, context.current.transform, innerBorder);
         context.current.clip = borderRadiusClip.get();
     }
 
     context.current.clip = object.getMutableForPainting().ensureObjectPaintProperties().createOrUpdateOverflowClip(
@@ skipping 50 matching lines @@
     context.current.shouldFlattenInheritedTransform = false;
     context.current.renderingContextID = 0;
 }
 
 void PaintPropertyTreeBuilder::updateScrollAndScrollTranslation(const LayoutObject& object, PaintPropertyTreeBuilderContext& context)
 {
     if (object.isBoxModelObject() && object.hasOverflowClip()) {
         PaintLayer* layer = toLayoutBoxModelObject(object).layer();
         DCHECK(layer);
         DoubleSize scrollOffset = layer->getScrollableArea()->scrollOffset();
-        bool forceScrollingForLayoutView = object.isLayoutView() && RuntimeEnabledFeatures::rootLayerScrollingEnabled();
-        if (forceScrollingForLayoutView || !scrollOffset.isZero() || layer->scrollsOverflow()) {
+        if (!scrollOffset.isZero() || layer->scrollsOverflow()) {

[szager1, 2016/09/26 19:57:54]

I think you still need to create the transform for LayoutView even if scrolls
don't overflow.  Have you tried running tests with RLS enabled to verify?

[pdr., 2016/09/26 20:30:56]

Why? With this patch I've switched FrameView to not do that, and all tests pass.

             TransformationMatrix matrix = TransformationMatrix().translate(-scrollOffset.width(), -scrollOffset.height());
             context.current.transform = object.getMutableForPainting().ensureObjectPaintProperties().createOrUpdateScrollTranslation(
                 context.current.transform, matrix, FloatPoint3D(), context.current.shouldFlattenInheritedTransform, context.current.renderingContextID);
 
             IntSize scrollClip = layer->getScrollableArea()->visibleContentRect().size();
             IntSize scrollBounds = layer->getScrollableArea()->contentsSize();
             bool userScrollableHorizontal = layer->getScrollableArea()->userInputScrollable(HorizontalScrollbar);
             bool userScrollableVertical = layer->getScrollableArea()->userInputScrollable(VerticalScrollbar);
-            ScrollPaintPropertyNode* parentScrollNode = forceScrollingForLayoutView ? nullptr : context.current.scroll;
             context.current.scroll = object.getMutableForPainting().ensureObjectPaintProperties().createOrUpdateScroll(
-                parentScrollNode, context.current.transform, scrollClip, scrollBounds, userScrollableHorizontal, userScrollableVertical);
+                context.current.scroll, context.current.transform, scrollClip, scrollBounds, userScrollableHorizontal, userScrollableVertical);
 
             context.current.shouldFlattenInheritedTransform = false;
             return;
         }
     }
 
     if (ObjectPaintProperties* properties = object.getMutableForPainting().objectPaintProperties()) {
         properties->clearScrollTranslation();
         properties->clearScroll();
     }
 }
 
 void PaintPropertyTreeBuilder::updateOutOfFlowContext(const LayoutObject& object, PaintPropertyTreeBuilderContext& context)
 {
     if (object.canContainAbsolutePositionObjects()) {
         context.absolutePosition = context.current;
         context.containerForAbsolutePosition = &object;
     }
 
     if (object.isLayoutView()) {
         if (RuntimeEnabledFeatures::rootLayerScrollingEnabled()) {
+            const auto* initialFixedTransform = context.fixedPosition.transform;
+            auto* initialFixedScroll = context.fixedPosition.scroll;
+
             context.fixedPosition = context.current;
-            const TransformPaintPropertyNode* transform = object.objectPaintProperties()->paintOffsetTranslation();
-            DCHECK(transform);
-            context.fixedPosition.transform = transform;
+
+            // Fixed position transform and scroll nodes should not be affected.

[chrishtr, 2016/09/26 18:02:29]

Why? Isn't a LayoutView a container for fixed position?

[pdr., 2016/09/26 19:42:50]

It is, but only specific property tree nodes should be used by fixed position
children.

The initial scroll/transform tree nodes are setup for the layoutview (or
FrameView, if !RLS) in PaintPropertyTreeBuilder::buildTreeNodes(FrameView&
frameView, ...). In that code we set the correct fixed position transform and
scroll nodes. In the subsequent calls in PaintPropertyTreeBuilder
(updateTransform, updateEffect, updateCssClip, etc...), we can append to the
transform/scroll trees but we don't want to use those nodes for the fixed pos
children, which explains the code here.

For example, for the scroll tree, we don't want the fixed position element to be
scrolled by the layoutview, so we need to make sure the scroll node introduced
by the layoutview is not used. Here's an example showing that effect nodes
should be used, unlike scroll: http://jsbin.com/rasewe

+            context.fixedPosition.transform = initialFixedTransform;
+            context.fixedPosition.scroll = initialFixedScroll;
         }
     } else if (object.canContainFixedPositionObjects()) {
         context.fixedPosition = context.current;
     } else if (object.getMutableForPainting().objectPaintProperties() && object.objectPaintProperties()->cssClip()) {
         // CSS clip applies to all descendants, even if this object is not a containing block
         // ancestor of the descendant. It is okay for absolute-position descendants because
         // having CSS clip implies being absolute position container. However for fixed-position
         // descendants we need to insert the clip here if we are not a containing block ancestor
         // of them.
         auto* cssClip = object.getMutableForPainting().objectPaintProperties()->cssClip();
@@ skipping 84 matching lines @@
         return;
 
     updateOverflowClip(object, context);
     updatePerspective(object, context);
     updateSvgLocalToBorderBoxTransform(object, context);
     updateScrollAndScrollTranslation(object, context);
     updateOutOfFlowContext(object, context);
 }
 
 } // namespace blink