Fix pixel snapping issues when layers become composited

Source/core/rendering/RenderLayerBacking.cpp

Index: Source/core/rendering/RenderLayerBacking.cpp
diff --git a/Source/core/rendering/RenderLayerBacking.cpp b/Source/core/rendering/RenderLayerBacking.cpp
index 3e23e44cb8e96e5a5e14be9d9b2543ec60d766b3..aff829c785bd7f41f406f520b6947b92e11bc9a3 100644
--- a/Source/core/rendering/RenderLayerBacking.cpp
+++ b/Source/core/rendering/RenderLayerBacking.cpp
@@ -321,7 +321,7 @@ bool RenderLayerBacking::shouldClipCompositedBounds() const
 
 void RenderLayerBacking::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.
@@ -343,7 +343,7 @@ void RenderLayerBacking::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;
@@ -366,7 +366,7 @@ void RenderLayerBacking::updateAfterWidgetResize()
     if (renderer()->isRenderPart()) {
         if (RenderLayerCompositor* innerCompositor = RenderLayerCompositor::frameContentsCompositor(toRenderPart(renderer()))) {
             innerCompositor->frameViewDidChangeSize();
-            innerCompositor->frameViewDidChangeLocation(contentsBox().location());
+            innerCompositor->frameViewDidChangeLocation(flooredIntPoint(contentsBox().location()));

[eae, 2013/07/26 20:30:51]

Don't you need to account for the subpixel accumulation here?

         }
     }
 }
@@ -529,11 +529,16 @@ void RenderLayerBacking::updateGraphicsLayerGeometry()
         ancestorCompositingBounds = pixelSnappedIntRect(compAncestor->backing()->compositedBounds());
     }
 
-    IntRect localCompositingBounds = pixelSnappedIntRect(compositedBounds());
+    LayoutRect localRawCompositingBounds = compositedBounds();
+    LayoutPoint rawDelta;
+    m_owningLayer->convertToLayerCoords(compAncestor, rawDelta);
+    IntPoint delta = flooredIntPoint(rawDelta);
+    m_subpixelAccumulation = toLayoutSize(rawDelta.fraction());

[eae, 2013/07/26 20:30:51]

Wouldn't it make more sense to have the fraction method on LayoutSize? A
fractional size makes more sense (to me at least) than a fractional point.

toLayoutSize(rawDelta).fraction();

[leviw_travelin_and_unemployed, 2013/07/26 20:36:21]

We certainly can. It makes sense to me to request the fraction of a LayoutPoint
or Size.

+    // Move the bounds by the subpixel accumulation so that it pixel-snaps relative to absolute pixels instead of local coordinates.
+    localRawCompositingBounds.move(m_subpixelAccumulation);
 
+    IntRect localCompositingBounds = pixelSnappedIntRect(localRawCompositingBounds);

[eae, 2013/07/26 20:30:51]

How about moving all of this snapping logic into a (static) helper method?

[leviw_travelin_and_unemployed, 2013/07/26 20:36:21]

Sure.

     IntRect relativeCompositingBounds(localCompositingBounds);
-    IntPoint delta;
-    m_owningLayer->convertToPixelSnappedLayerCoords(compAncestor, delta);
     relativeCompositingBounds.moveBy(delta);
 
     IntPoint graphicsLayerParentLocation;
@@ -608,7 +613,7 @@ void RenderLayerBacking::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);
@@ -693,7 +698,7 @@ void RenderLayerBacking::updateGraphicsLayerGeometry()
             clientBox.move(renderBox->verticalScrollbarWidth(), 0);
 
         IntSize adjustedScrollOffset = m_owningLayer->adjustedScrollOffset();
-        m_scrollingLayer->setPosition(FloatPoint(clientBox.location() - localCompositingBounds.location()));
+        m_scrollingLayer->setPosition(FloatPoint(clientBox.location() - localCompositingBounds.location() + roundedIntSize(m_subpixelAccumulation)));

[eae, 2013/07/26 20:30:51]

Why round here?

         m_scrollingLayer->setSize(clientBox.size());
 
         IntSize oldScrollingLayerOffset = m_scrollingLayer->offsetFromRenderer();
@@ -800,13 +805,13 @@ void RenderLayerBacking::updateInternalHierarchy()
 
 void RenderLayerBacking::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 RenderLayerBacking::updateDrawsContent(bool isSimpleContainer)
@@ -934,7 +939,7 @@ bool RenderLayerBacking::updateOverflowControlsLayers(bool needsHorizontalScroll
 
 void RenderLayerBacking::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->horizontalScrollbar();
         if (hBar) {
@@ -1432,23 +1437,23 @@ FloatPoint RenderLayerBacking::computePerspectiveOrigin(const IntRect& borderBox
 }
 
 // Return the offset from the top-left of this compositing layer at which the renderer's contents are painted.
-IntSize RenderLayerBacking::contentOffsetInCompostingLayer() const
+LayoutSize RenderLayerBacking::contentOffsetInCompostingLayer() const
 {
-    return IntSize(-m_compositedBounds.x(), -m_compositedBounds.y());
+    return LayoutSize(-m_compositedBounds.x(), -m_compositedBounds.y());
 }
 
-IntRect RenderLayerBacking::contentsBox() const
+LayoutRect RenderLayerBacking::contentsBox() const
 {
-    IntRect contentsBox = contentsRect(renderer());
+    LayoutRect contentsBox = contentsRect(renderer());
     contentsBox.move(contentOffsetInCompostingLayer());
     return contentsBox;
 }
 
 IntRect RenderLayerBacking::backgroundBox() const
 {
-    IntRect backgroundBox = backgroundRect(renderer());
+    LayoutRect backgroundBox = backgroundRect(renderer());
     backgroundBox.move(contentOffsetInCompostingLayer());
-    return backgroundBox;
+    return pixelSnappedIntRect(backgroundBox);
 }
 
 GraphicsLayer* RenderLayerBacking::parentForSublayers() const
@@ -1572,7 +1577,7 @@ void RenderLayerBacking::paintIntoLayer(const GraphicsLayer* graphicsLayer, Grap
         paintFlags |= RenderLayer::PaintLayerPaintingSkipRootBackground;
 
     // FIXME: GraphicsLayers need a way to split for RenderRegions.
-    RenderLayer::LayerPaintingInfo paintingInfo(m_owningLayer, paintDirtyRect, paintBehavior, LayoutSize());
+    RenderLayer::LayerPaintingInfo paintingInfo(m_owningLayer, paintDirtyRect, paintBehavior, m_subpixelAccumulation);
     m_owningLayer->paintLayerContents(context, paintingInfo, paintFlags);
 
     ASSERT(graphicsLayer != m_backgroundLayer || paintFlags & RenderLayer::PaintLayerPaintingRootBackgroundOnly);
@@ -1614,8 +1619,13 @@ void RenderLayerBacking::paintContents(const GraphicsLayer* graphicsLayer, Graph
 
         // The dirtyRect is in the coords of the painting root.
         IntRect dirtyRect = clip;
-        if (!(paintingPhase & GraphicsLayerPaintOverflowContents))
-            dirtyRect.intersect(compositedBounds());
+        if (!(paintingPhase & GraphicsLayerPaintOverflowContents)) {
+            LayoutRect bounds = compositedBounds();
+            bounds.move(m_subpixelAccumulation);
+            dirtyRect.intersect(pixelSnappedIntRect(bounds));
+        } else {
+            dirtyRect.move(roundedIntSize(m_subpixelAccumulation));
+        }
 
         // We have to use the same root as for hit testing, because both methods can compute and cache clipRects.
         paintIntoLayer(graphicsLayer, &context, dirtyRect, PaintBehaviorNormal, paintingPhase);
@@ -1812,12 +1822,12 @@ void RenderLayerBacking::resumeAnimations()
     m_graphicsLayer->resumeAnimations();
 }
 
-IntRect RenderLayerBacking::compositedBounds() const
+LayoutRect RenderLayerBacking::compositedBounds() const
 {
     return m_compositedBounds;
 }
 
-void RenderLayerBacking::setCompositedBounds(const IntRect& bounds)
+void RenderLayerBacking::setCompositedBounds(const LayoutRect& bounds)
 {
     m_compositedBounds = bounds;
 }