Fix subpixel accumulation for composited content layers

third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp

Index: third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
diff --git a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
index 7891a098a51c8ec7d871f18cdaeb75ade2d33849..16a42c2e170d02369819df66620fe63764770ab6 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
@@ -79,22 +79,31 @@ using namespace HTMLNames;
 
 static IntRect clipBox(LayoutBox* layoutObject);
 
-static IntRect contentsRect(const LayoutObject* layoutObject)
+static LayoutRect contentsRect(const LayoutObject* layoutObject)
 {
     if (!layoutObject->isBox())
-        return IntRect();
+        return LayoutRect();
     if (layoutObject->isCanvas())
-        return pixelSnappedIntRect(toLayoutHTMLCanvas(layoutObject)->replacedContentRect());
+        return toLayoutHTMLCanvas(layoutObject)->replacedContentRect();
     if (layoutObject->isVideo())
         return toLayoutVideo(layoutObject)->videoBox();
 
-    return pixelSnappedIntRect(toLayoutBox(layoutObject)->contentBoxRect());
+    if (layoutObject->isLayoutPart()) {
+        const LayoutPart* layoutPart = toLayoutPart(layoutObject);
+        // FrameView are somehow pre-snapped. This is to match behavior of LayoutPart::updateWidgetGeometry().

[chrishtr, 2016/07/21 18:02:31]

PartPainter::paintContents does rounded snapping that looks the same as this.
This is now the third place that this code appears.
Please refactor to have it centralized in a method on LayoutPart.

+        if (layoutPart->widget() && layoutPart->widget()->isFrameView()) {
+            LayoutRect contentBoxRect = layoutPart->contentBoxRect();
+            return LayoutRect(contentBoxRect.location(), LayoutSize(roundedIntSize(contentBoxRect.size())));
+        }
+    }
+
+    return toLayoutBox(layoutObject)->contentBoxRect();
 }
 
-static IntRect backgroundRect(const LayoutObject* layoutObject)
+static LayoutRect backgroundRect(const LayoutObject* layoutObject)
 {
     if (!layoutObject->isBox())
-        return IntRect();
+        return LayoutRect();
 
     LayoutRect rect;
     const LayoutBox* box = toLayoutBox(layoutObject);
@@ -113,7 +122,7 @@ static IntRect backgroundRect(const LayoutObject* layoutObject)
         break;
     }
 
-    return pixelSnappedIntRect(rect);
+    return rect;
 }
 
 static inline bool isAcceleratedCanvas(const LayoutObject* layoutObject)
@@ -183,7 +192,6 @@ static ScrollingCoordinator* scrollingCoordinatorFromLayer(PaintLayer& layer)
 
 CompositedLayerMapping::CompositedLayerMapping(PaintLayer& layer)
     : m_owningLayer(layer)
-    , m_contentOffsetInCompositingLayerDirty(false)
     , m_pendingUpdateScope(GraphicsLayerUpdateNone)
     , m_isMainFrameLayoutViewLayer(false)
     , m_backgroundLayerPaintsFixedRootBackground(false)
@@ -334,7 +342,6 @@ void CompositedLayerMapping::updateCompositedBounds()
     ASSERT(m_owningLayer.compositor()->lifecycle().state() == DocumentLifecycle::InCompositingUpdate);
     // FIXME: if this is really needed for performance, it would be better to store it on Layer.
     m_compositedBounds = m_owningLayer.boundingBoxForCompositing();
-    m_contentOffsetInCompositingLayerDirty = true;
 }
 
 void CompositedLayerMapping::updateAfterPartResize()
@@ -344,7 +351,7 @@ void CompositedLayerMapping::updateAfterPartResize()
             innerCompositor->frameViewDidChangeSize();
             // We can floor this point because our frameviews are always aligned to pixel boundaries.
             ASSERT(m_compositedBounds.location() == flooredIntPoint(m_compositedBounds.location()));
-            innerCompositor->frameViewDidChangeLocation(flooredIntPoint(contentsBox().location()));
+            innerCompositor->frameViewDidChangeLocation(roundedIntPoint(contentsBox().location()));

[chrishtr, 2016/07/21 18:02:31]

Yet another instance of frame rounding to be refactored?

         }
     }
 }
@@ -747,7 +754,6 @@ void CompositedLayerMapping::updateGraphicsLayerGeometry(const PaintLayer* compo
 
     updateMainGraphicsLayerGeometry(relativeCompositingBounds, localCompositingBounds, graphicsLayerParentLocation);
     updateOverflowControlsHostLayerGeometry(compositingStackingContext, compositingContainer);
-    updateContentsOffsetInCompositingLayer(snappedOffsetFromCompositedAncestor, graphicsLayerParentLocation);
     updateSquashingLayerGeometry(graphicsLayerParentLocation, compositingContainer, m_squashedLayers, m_squashingLayer.get(), &m_squashingLayerOffsetFromTransformedAncestor, layersNeedingPaintInvalidation);
 
     // If we have a layer that clips children, position it.
@@ -923,9 +929,12 @@ void CompositedLayerMapping::updateChildTransformLayerGeometry()
 {
     if (!m_childTransformLayer)
         return;
-    const IntRect borderBox = toLayoutBox(m_owningLayer.layoutObject())->pixelSnappedBorderBoxRect();
-    m_childTransformLayer->setSize(FloatSize(borderBox.size()));
-    m_childTransformLayer->setPosition(FloatPoint(contentOffsetInCompositingLayer()));
+    LayoutRect borderBox = toLayoutBox(m_owningLayer.layoutObject())->borderBoxRect();
+    borderBox.move(contentOffsetInCompositingLayer());
+    IntRect pixelSnappedBorderBox = pixelSnappedIntRect(borderBox);
+
+    m_childTransformLayer->setSize(FloatSize(pixelSnappedBorderBox.size()));
+    m_childTransformLayer->setPosition(FloatPoint(pixelSnappedBorderBox.location()));
 }
 
 void CompositedLayerMapping::updateMaskLayerGeometry()
@@ -1191,44 +1200,6 @@ void CompositedLayerMapping::updateContentsRect()
     m_graphicsLayer->setContentsRect(pixelSnappedIntRect(contentsBox()));
 }
 
-void CompositedLayerMapping::updateContentsOffsetInCompositingLayer(const IntPoint& snappedOffsetFromCompositedAncestor, const IntPoint& graphicsLayerParentLocation)
-{
-    // m_graphicsLayer is positioned relative to our compositing ancestor
-    // PaintLayer, but it's not positioned at the origin of m_owningLayer, it's
-    // offset by m_contentBounds.location(). This is what
-    // contentOffsetInCompositingLayer is meant to capture, roughly speaking
-    // (ignoring rounding and subpixel accumulation).
-    //
-    // Our ancestor graphics layers in this CLM (m_graphicsLayer and potentially
-    // m_ancestorClippingLayer) have pixel snapped, so if we don't adjust this
-    // offset, we'll see accumulated rounding errors due to that snapping.
-    //
-    // In order to ensure that we account for this rounding, we compute
-    // contentsOffsetInCompositingLayer in a somewhat roundabout way.
-    //
-    // our position = (desired position) - (inherited graphics layer offset).
-    //
-    // Precisely,
-    // Offset = snappedOffsetFromCompositedAncestor - offsetDueToAncestorGraphicsLayers (See code below)
-    //      = snappedOffsetFromCompositedAncestor - (m_graphicsLayer->position() + graphicsLayerParentLocation)
-    //      = snappedOffsetFromCompositedAncestor - (relativeCompositingBounds.location() - graphicsLayerParentLocation + graphicsLayerParentLocation) (See updateMainGraphicsLayerGeometry)
-    //      = snappedOffsetFromCompositedAncestor - relativeCompositingBounds.location()
-    //      = snappedOffsetFromCompositedAncestor - (pixelSnappedIntRect(contentBounds.location()) + snappedOffsetFromCompositedAncestor) (See computeBoundsOfOwningLayer)
-    //      = -pixelSnappedIntRect(contentBounds.location())
-    //
-    // As you can see, we've ended up at the same spot (-contentBounds.location()),
-    // but by subtracting off our ancestor graphics layers positions, we can be
-    // sure we've accounted correctly for any pixel snapping due to ancestor
-    // graphics layers.
-    //
-    // And drawing of composited children takes into account the subpixel
-    // accumulation of this CLM already (through its own
-    // graphicsLayerParentLocation it appears).
-    FloatPoint offsetDueToAncestorGraphicsLayers = m_graphicsLayer->position() + graphicsLayerParentLocation;
-    m_contentOffsetInCompositingLayer = LayoutSize(snappedOffsetFromCompositedAncestor - offsetDueToAncestorGraphicsLayers);
-    m_contentOffsetInCompositingLayerDirty = false;
-}
-
 void CompositedLayerMapping::updateDrawsContent()
 {
     bool inOverlayFullscreenVideo = false;
@@ -2014,13 +1985,12 @@ FloatPoint3D CompositedLayerMapping::computeTransformOrigin(const IntRect& borde
 // LayoutObject's contents are painted.
 LayoutSize CompositedLayerMapping::contentOffsetInCompositingLayer() const
 {
-    ASSERT(!m_contentOffsetInCompositingLayerDirty);
-    return m_contentOffsetInCompositingLayer;
+    return m_owningLayer.subpixelAccumulation() - LayoutSize(m_graphicsLayer->offsetFromLayoutObject());

[chrishtr, 2016/07/21 18:02:31]

why subpixelAccumulation - offsetFromLayoutObject? Why not:

-(offsetFromLayoutObject + subpixelAccumulation)

 }
 
 LayoutRect CompositedLayerMapping::contentsBox() const
 {
-    LayoutRect contentsBox = LayoutRect(contentsRect(layoutObject()));
+    LayoutRect contentsBox = contentsRect(layoutObject());
     contentsBox.move(contentOffsetInCompositingLayer());
     return contentsBox;
 }