Calculate correct clip foregroundBound and backgroundBound

Source/core/paint/DeprecatedPaintLayerClipper.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
  * Other contributors:
  *   Robert O'Callahan <roc+@cs.cmu.edu>
  *   David Baron <dbaron@fas.harvard.edu>
  *   Christian Biesinger <cbiesinger@web.de>
  *   Randall Jesup <rjesup@wgate.com>
@@ skipping 199 matching lines @@
     return clipRect;
 }
 
 void DeprecatedPaintLayerClipper::calculateRects(const ClipRectsContext& context, const LayoutRect& paintDirtyRect, LayoutRect& layerBounds,
     ClipRect& backgroundRect, ClipRect& foregroundRect, ClipRect& outlineRect, const LayoutPoint* offsetFromRoot) const
 {
     bool isClippingRoot = m_layoutObject.layer() == context.rootLayer;
 
     if (!isClippingRoot && m_layoutObject.layer()->parent()) {
         backgroundRect = backgroundClipRect(context);
-        backgroundRect.move(roundedIntSize(context.subPixelAccumulation));

[pdr., 2015/08/17 21:17:53]

We used roundedIntSize before, but don't now. Was that intentional or tested?

[qiankun, 2015/08/19 09:14:19]

It's intentional. The layout test which was added for this line passed. 

         backgroundRect.intersect(paintDirtyRect);

[pdr., 2015/08/17 21:17:53]

This seems wrong... we're intersecting with the paintDirtyRect and then moving
by the subPixelAccumulation below. I think we need to do dirty rect intersection
before moving.

[qiankun, 2015/08/19 09:14:19]

Both backgroundRect and paintDirtyRect should move by subPixelAccumulation here.
But backgroundRect and paintDirtyRect didn't consider subPixelAccumulation, so
moving subPixelAccumulation after intersection.

[pdr., 2015/08/20 05:55:13]

I'm just worried about the order these are being done.

For example (x, y, w, h):
paintDirtyRect: (0, 0, 100, 100)
backgroundRect: (-1, 0, 1, 100)
subpixelAccumulation: (0.5, 0)

Without your patch:
backgroundRect.move(subpixelAccumulation) = (-0.5, 0, 1, 100)
backgroundRect.intersect(paintDirtyRect) = (0, 0, 0.5, 100)
This will paint a line at (0, 0) with width 0.5px and height 100px.

With your patch:
backgroundRect.intersect(paintDirtyRect) = (0, 0, 0, 0)
backgroundRect.move(subpixelAccumulation) = (0.5, 0, 0, 0)
This will paint nothing because it's empty.

[qiankun, 2015/08/20 06:08:43]

Current implementation is wrong here. I think we should also do
paintDirtyRect.move(subpixelAccumulation) before intersection. 

     } else {
         backgroundRect = paintDirtyRect;
     }
 
+    LayoutPoint offset;
+    if (offsetFromRoot) {
+        offset = *offsetFromRoot;
+        backgroundRect.move(context.subPixelAccumulation);
+    } else {
+        m_layoutObject.layer()->convertToLayerCoords(context.rootLayer, offset);
+    }
+    layerBounds = LayoutRect(offset, LayoutSize(m_layoutObject.layer()->size()));
+
     foregroundRect = backgroundRect;
     outlineRect = backgroundRect;
 
-    LayoutPoint offset;
-    if (offsetFromRoot)
-        offset = *offsetFromRoot;
-    else
-        m_layoutObject.layer()->convertToLayerCoords(context.rootLayer, offset);
-    layerBounds = LayoutRect(offset, LayoutSize(m_layoutObject.layer()->size()));
-
     // Update the clip rects that will be passed to child layers.
     if (m_layoutObject.hasOverflowClip()) {
         // This layer establishes a clip of some kind.
         if (shouldRespectOverflowClip(context)) {
             foregroundRect.intersect(toLayoutBox(m_layoutObject).overflowClipRect(offset, context.scrollbarRelevancy));
             if (m_layoutObject.style()->hasBorderRadius())
                 foregroundRect.setHasRadius(true);
         }
 
         // If we establish an overflow clip at all, then go ahead and make sure our background
@@ skipping 137 matching lines @@
     if (context.respectOverflowClip == IgnoreOverflowClip)
         return false;
 
     if (layer->isRootLayer() && context.respectOverflowClipForViewport == IgnoreOverflowClip)
         return false;
 
     return true;
 }
 
 } // namespace blink