Fix fixed-background offset issue

Source/core/paint/BackgroundImageGeometry.cpp

 // 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 "config.h"
 #include "core/paint/BackgroundImageGeometry.h"
 
 #include "core/layout/LayoutBox.h"
 #include "core/layout/LayoutBoxModelObject.h"
 #include "core/layout/LayoutView.h"
@@ skipping 138 matching lines @@
     m_phase.move(std::max(alignedPoint.x() - m_destRect.x(), 0), std::max(alignedPoint.y() - m_destRect.y(), 0));
 }
 
 void BackgroundImageGeometry::clip(const IntRect& clipRect)
 {
     m_destRect.intersect(clipRect);
 }
 
 void BackgroundImageGeometry::calculate(const LayoutBoxModelObject& obj, const LayoutBoxModelObject* paintContainer,
     const GlobalPaintFlags globalPaintFlags, const FillLayer& fillLayer, const LayoutRect& paintRect,
-    LayoutObject* backgroundObject)
+    const IntPoint& accumulatedScrollOffsetSincePaintContainer, LayoutObject* backgroundObject)
 {
     LayoutUnit left = 0;
     LayoutUnit top = 0;
     IntSize positioningAreaSize;
     IntRect snappedPaintRect = pixelSnappedIntRect(paintRect);
     bool isLayoutView = obj.isLayoutView();
     const LayoutBox* rootBox = nullptr;
     if (isLayoutView) {
         // It is only possible reach here when root element has a box.
         Element* documentElement = obj.document().documentElement();
@@ skipping 46 matching lines @@
             // Since left/top is relative to the paint rect, we need to offset them back.
             left -= paintRect.x();
             top -= paintRect.y();
         } else {
             positioningAreaSize = pixelSnappedIntSize(paintRect.size() - LayoutSize(left + right, top + bottom), paintRect.location());
         }
     } else {
         setHasNonLocalGeometry();
 
         IntRect viewportRect = pixelSnappedIntRect(obj.viewRect());
-        if (fixedBackgroundPaintsInLocalCoordinates(obj, globalPaintFlags))
+        if (fixedBackgroundPaintsInLocalCoordinates(obj, globalPaintFlags)) {
             viewportRect.setLocation(IntPoint());
-        else if (FrameView* frameView = obj.view()->frameView())
-            viewportRect.setLocation(frameView->scrollPosition());
+        } else {
+            if (FrameView* frameView = obj.view()->frameView()) {
+                viewportRect.setLocation(frameView->scrollPosition());
+            viewportRect.moveBy(accumulatedScrollOffsetSincePaintContainer);
+        }
 
         if (paintContainer) {
             IntPoint absoluteContainerOffset = roundedIntPoint(paintContainer->localToAbsolute(FloatPoint()));

[chrishtr, 2015/09/09 17:05:41]

localToAbsolute includes scroll offsets...why isn't it happening here?

[Xianzhu, 2015/09/09 17:09:09]

It includes scroll offsets from root to the paint container, not including those
from the paint container to the current painting object.

[chrishtr, 2015/09/09 17:29:16]

Oh right.

How about replacing your code with mapLocalToContainer from the local object to
the paintContainer? Then this code can at least be localized to fixed-position
backgrounds.

[Xianzhu, 2015/09/09 20:40:03]

This seems not to work because we only need to compensate scroll offsets for
which we issued translations, but not other offsets in paint offset.

[chrishtr, 2015/09/09 20:48:15]

Maybe just make a one-off method that accumulates just scroll offsets then? SP
phase 2 should
allow us to fix this more cleanly by specifying the root transform space, once
trchen's
work is done. In the meantime I think this should be as targeted of a fix as
possible.

[Xianzhu, 2015/09/09 22:04:06]

Good idea! Done.

             viewportRect.moveBy(-absoluteContainerOffset);
         }
 
         setDestRect(viewportRect);
         positioningAreaSize = destRect().size();
     }
 
     const LayoutObject* clientForBackgroundImage = backgroundObject ? backgroundObject : &obj;
     IntSize fillTileSize = calculateFillTileSize(positioningBox, fillLayer, positioningAreaSize);
     fillLayer.image()->setContainerSizeForLayoutObject(clientForBackgroundImage, fillTileSize, obj.style()->effectiveZoom());
@@ skipping 82 matching lines @@
         setSpaceSize(IntSize(spaceSize().width(), 0));
     }
 
     if (fixedAttachment)
         useFixedAttachment(snappedPaintRect.location());
 
     clip(snappedPaintRect);
 }
 
 } // namespace blink