Fix pixel snapping issues when layers become composited

Source/core/rendering/CompositedLayerMapping.cpp

Index: Source/core/rendering/CompositedLayerMapping.cpp
diff --git a/Source/core/rendering/CompositedLayerMapping.cpp b/Source/core/rendering/CompositedLayerMapping.cpp
index 1beb7cc164262f6339bd2d03b1cfc88dbb9bae0d..cf568acfb6868bfda65a54cc3f80b3c5eaf98a16 100644
--- a/Source/core/rendering/CompositedLayerMapping.cpp
+++ b/Source/core/rendering/CompositedLayerMapping.cpp
@@ -323,7 +323,7 @@ bool CompositedLayerMapping::shouldClipCompositedBounds() const
 
 void CompositedLayerMapping::updateCompositedBounds()
 {
-    IntRect layerBounds = compositor()->calculateCompositedBounds(m_owningLayer, m_owningLayer);
+    LayoutRect layerBounds = compositor()->calculateCompositedBounds(m_owningLayer, m_owningLayer);
 
     // Clip to the size of the document or enclosing overflow-scroll layer.
     // If this or an ancestor is transformed, we can't currently compute the correct rect to intersect with.
@@ -345,7 +345,7 @@ void CompositedLayerMapping::updateCompositedBounds()
         m_owningLayer->convertToLayerCoords(rootLayer, delta);
         clippingBounds.move(-delta.x(), -delta.y());
 
-        layerBounds.intersect(pixelSnappedIntRect(clippingBounds));
+        layerBounds.intersect(clippingBounds);
         m_boundsConstrainedByClipping = true;
     } else {
         m_boundsConstrainedByClipping = false;
@@ -370,7 +370,8 @@ void CompositedLayerMapping::updateAfterWidgetResize()
     if (renderer()->isRenderPart()) {
         if (RenderLayerCompositor* innerCompositor = RenderLayerCompositor::frameContentsCompositor(toRenderPart(renderer()))) {
             innerCompositor->frameViewDidChangeSize();
-            innerCompositor->frameViewDidChangeLocation(contentsBox().location());
+            // We can floor this point because our frameviews are always aligned to pixel boundaries.

[eae, 2013/11/13 00:29:01]

Could you add an ASSERT ensuring that this is the case?

[leviw_travelin_and_unemployed, 2013/11/13 00:31:52]

Very good point. Yes.

+            innerCompositor->frameViewDidChangeLocation(flooredIntPoint(contentsBox().location()));
         }
     }
 }
@@ -507,6 +508,22 @@ static IntRect clipBox(RenderBox* renderer)
     return pixelSnappedIntRect(result);
 }
 
+void CompositedLayerMapping::adjustBoundsForSubPixelAccumulation(const RenderLayer* compositedAncestor, IntRect& localBounds, IntRect& relativeBounds, IntPoint& delta)
+{
+    LayoutRect localRawCompositingBounds = compositedBounds();
+    LayoutPoint rawDelta;
+    m_owningLayer->convertToLayerCoords(compositedAncestor, rawDelta);
+    delta = flooredIntPoint(rawDelta);
+    m_subpixelAccumulation = toLayoutSize(rawDelta).fraction();
+
+    // Move the bounds by the subpixel accumulation so that it pixel-snaps relative to absolute pixels instead of local coordinates.
+    localRawCompositingBounds.move(m_subpixelAccumulation);
+    localBounds = pixelSnappedIntRect(localRawCompositingBounds);
+
+    relativeBounds = localBounds;
+    relativeBounds.moveBy(delta);
+}
+
 void CompositedLayerMapping::updateGraphicsLayerGeometry()
 {
     // If we haven't built z-order lists yet, wait until later.
@@ -555,12 +572,10 @@ void CompositedLayerMapping::updateGraphicsLayerGeometry()
         ancestorCompositingBounds = pixelSnappedIntRect(compAncestor->compositedLayerMapping()->compositedBounds());
     }
 
-    IntRect localCompositingBounds = pixelSnappedIntRect(compositedBounds());
-
-    IntRect relativeCompositingBounds(localCompositingBounds);
+    IntRect localCompositingBounds;
+    IntRect relativeCompositingBounds;
     IntPoint delta;
-    m_owningLayer->convertToPixelSnappedLayerCoords(compAncestor, delta);
-    relativeCompositingBounds.moveBy(delta);
+    adjustBoundsForSubPixelAccumulation(compAncestor, localCompositingBounds, relativeCompositingBounds, delta);
 
     IntPoint graphicsLayerParentLocation;
     if (compAncestor && compAncestor->compositedLayerMapping()->hasClippingLayer()) {
@@ -635,7 +650,7 @@ void CompositedLayerMapping::updateGraphicsLayerGeometry()
         const IntRect borderBox = toRenderBox(renderer())->pixelSnappedBorderBoxRect();
 
         // Get layout bounds in the coords of compAncestor to match relativeCompositingBounds.
-        IntRect layerBounds = IntRect(delta, borderBox.size());
+        IntRect layerBounds(delta, borderBox.size());
 
         // Update properties that depend on layer dimensions
         FloatPoint3D transformOrigin = computeTransformOrigin(borderBox);
@@ -720,7 +735,7 @@ void CompositedLayerMapping::updateGraphicsLayerGeometry()
             clientBox.move(renderBox->verticalScrollbarWidth(), 0);
 
         IntSize adjustedScrollOffset = m_owningLayer->scrollableArea()->adjustedScrollOffset();
-        m_scrollingLayer->setPosition(FloatPoint(clientBox.location() - localCompositingBounds.location()));
+        m_scrollingLayer->setPosition(FloatPoint(clientBox.location() - localCompositingBounds.location() + roundedIntSize(m_subpixelAccumulation)));

[eae, 2013/11/13 00:29:01]

"clientBox.location() - localCompositingBounds.location()" is int so that
rounding the m_subpixelAccumulatio has the same effect as pixel snapping the
entire value, right? (If so, all good. If not then please explain why you round
the value).

[leviw_travelin_and_unemployed, 2013/11/13 00:31:52]

Precisely.

         m_scrollingLayer->setSize(clientBox.size());
 
         IntSize oldScrollingLayerOffset = m_scrollingLayer->offsetFromRenderer();
@@ -831,13 +846,13 @@ void CompositedLayerMapping::updateInternalHierarchy()
 
 void CompositedLayerMapping::updateContentsRect(bool isSimpleContainer)
 {
-    IntRect contentsRect;
+    LayoutRect contentsRect;
     if (isSimpleContainer && renderer()->hasBackground())
         contentsRect = backgroundBox();
     else
         contentsRect = contentsBox();
 
-    m_graphicsLayer->setContentsRect(contentsRect);
+    m_graphicsLayer->setContentsRect(pixelSnappedIntRect(contentsRect));
 }
 
 void CompositedLayerMapping::updateDrawsContent(bool isSimpleContainer)
@@ -964,7 +979,7 @@ bool CompositedLayerMapping::updateOverflowControlsLayers(bool needsHorizontalSc
 
 void CompositedLayerMapping::positionOverflowControlsLayers(const IntSize& offsetFromRoot)
 {
-    IntSize offsetFromRenderer = m_graphicsLayer->offsetFromRenderer();
+    IntSize offsetFromRenderer = m_graphicsLayer->offsetFromRenderer() - roundedIntSize(m_subpixelAccumulation);
     if (GraphicsLayer* layer = layerForHorizontalScrollbar()) {
         Scrollbar* hBar = m_owningLayer->scrollableArea()->horizontalScrollbar();
         if (hBar) {
@@ -1509,23 +1524,23 @@ FloatPoint CompositedLayerMapping::computePerspectiveOrigin(const IntRect& borde
 }
 
 // Return the offset from the top-left of this compositing layer at which the renderer's contents are painted.
-IntSize CompositedLayerMapping::contentOffsetInCompostingLayer() const
+LayoutSize CompositedLayerMapping::contentOffsetInCompostingLayer() const
 {
-    return IntSize(-m_compositedBounds.x(), -m_compositedBounds.y());
+    return LayoutSize(-m_compositedBounds.x(), -m_compositedBounds.y());
 }
 
-IntRect CompositedLayerMapping::contentsBox() const
+LayoutRect CompositedLayerMapping::contentsBox() const
 {
-    IntRect contentsBox = contentsRect(renderer());
+    LayoutRect contentsBox = contentsRect(renderer());
     contentsBox.move(contentOffsetInCompostingLayer());
     return contentsBox;
 }
 
 IntRect CompositedLayerMapping::backgroundBox() const
 {
-    IntRect backgroundBox = backgroundRect(renderer());
+    LayoutRect backgroundBox = backgroundRect(renderer());
     backgroundBox.move(contentOffsetInCompostingLayer());
-    return backgroundBox;
+    return pixelSnappedIntRect(backgroundBox);
 }
 
 GraphicsLayer* CompositedLayerMapping::parentForSublayers() const
@@ -1693,22 +1708,27 @@ void CompositedLayerMapping::doPaintTask(GraphicsLayerPaintInfo& paintInfo, Grap
     // assuming that the context's space was not affected by the RenderLayer
     // painting code.
 
-    LayoutSize offset = paintInfo.offsetFromRenderer;
+    IntSize offset = paintInfo.offsetFromRenderer;
     context->translate(-offset);
-    LayoutRect relativeClip(clip);
-    relativeClip.move(offset);
 
     // The dirtyRect is in the coords of the painting root.
-    IntRect dirtyRect = pixelSnappedIntRect(relativeClip);
-    if (!(paintInfo.paintingPhase & GraphicsLayerPaintOverflowContents))
-        dirtyRect.intersect(paintInfo.compositedBounds);
+    IntRect dirtyRect(clip);
+    dirtyRect.move(offset);
+
+    if (!(paintInfo.paintingPhase & GraphicsLayerPaintOverflowContents)) {
+        LayoutRect bounds = paintInfo.compositedBounds;
+        bounds.move(m_subpixelAccumulation);
+        dirtyRect.intersect(pixelSnappedIntRect(bounds));
+    } else {
+        dirtyRect.move(roundedIntSize(m_subpixelAccumulation));
+    }
 
 #ifndef NDEBUG
     paintInfo.renderLayer->renderer()->assertSubtreeIsLaidOut();
 #endif
 
     // FIXME: GraphicsLayers need a way to split for RenderRegions.
-    LayerPaintingInfo paintingInfo(paintInfo.renderLayer, dirtyRect, PaintBehaviorNormal, LayoutSize());
+    LayerPaintingInfo paintingInfo(paintInfo.renderLayer, dirtyRect, PaintBehaviorNormal, m_subpixelAccumulation);
     paintInfo.renderLayer->paintLayerContents(context, paintingInfo, paintFlags);
 
     ASSERT(!paintInfo.isBackgroundLayer || paintFlags & PaintLayerPaintingRootBackgroundOnly);
@@ -1943,12 +1963,12 @@ void CompositedLayerMapping::notifyAnimationStarted(const GraphicsLayer*, double
     renderer()->animation().notifyAnimationStarted(renderer(), time);
 }
 
-IntRect CompositedLayerMapping::compositedBounds() const
+LayoutRect CompositedLayerMapping::compositedBounds() const
 {
     return m_compositedBounds;
 }
 
-void CompositedLayerMapping::setCompositedBounds(const IntRect& bounds)
+void CompositedLayerMapping::setCompositedBounds(const LayoutRect& bounds)
 {
     m_compositedBounds = bounds;
 }