Don't pixel-snap overflow clips in clip paint property nodes.

third_party/WebKit/Source/core/paint/PaintLayerClipper.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>
@@ skipping 231 matching lines @@
 
   LayoutPoint clippingRootOffset;
   m_layer.convertToLayerCoords(&clippingRootLayer, clippingRootOffset);
   clipRect.moveBy(-clippingRootOffset);
 
   return clipRect;
 }
 
 #ifdef CHECK_CLIP_RECTS
 #define CHECK_RECTS_EQ(expected, actual)                                \
-  CHECK((expected.isEmpty() && actual.isEmpty()) || expected == actual) \
-      << "expected=" << expected.toString() << " actual=" << actual.toString()
+  do {                                                                  \

[chrishtr, 2017/02/16 21:44:28]

I adjusted this to not assert, in order to get better debugging output.
Regardless, the checks are not on in debug or any other compile mode
by default.

+    bool matches =                                                      \
+        (expected.isEmpty() && actual.isEmpty()) || expected == actual; \
+    if (!matches) {                                                     \
+      LOG(ERROR) << "Rects don't match for m_layer="                    \
+                 << m_layer.layoutObject()->debugName()                 \
+                 << " expected=" << expected.toString()                 \
+                 << " actual=" << actual.toString();                    \
+    }                                                                   \
+  } while (false);
 #endif
 
 void PaintLayerClipper::mapLocalToRootWithGeometryMapper(
     const ClipRectsContext& context,
     LayoutRect& rectToMap) const {
   DCHECK(m_geometryMapper);
 
   const auto* layerTransform = m_layer.layoutObject()
                                    ->paintProperties()
                                    ->localBorderBoxProperties()
                                    ->transform();
   const auto* rootTransform = context.rootLayer->layoutObject()
                                   ->paintProperties()
                                   ->localBorderBoxProperties()
                                   ->transform();
 
   FloatRect localRect(rectToMap);
   localRect.moveBy(FloatPoint(m_layer.layoutObject()->paintOffset()));
   rectToMap = LayoutRect(m_geometryMapper->sourceToDestinationRect(
       localRect, layerTransform, rootTransform));
   rectToMap.moveBy(-context.rootLayer->layoutObject()->paintOffset());
+  rectToMap.move(context.subPixelAccumulation);

[chrishtr, 2017/02/16 21:44:28]

This is exercised by the layout test
fast/sub-pixel/fractional-border-location-and-size.html in addition to the
new unittest.

 }
 
 void PaintLayerClipper::calculateRectsWithGeometryMapper(
     const ClipRectsContext& context,
     const LayoutRect& paintDirtyRect,
     LayoutRect& layerBounds,
     ClipRect& backgroundRect,
     ClipRect& foregroundRect,
     const LayoutPoint* offsetFromRoot) const {
   const auto* properties = m_layer.layoutObject()->paintProperties();
   // TODO(chrishtr): fix the underlying bug that causes this situation.
   if (!properties) {
     backgroundRect = ClipRect(LayoutRect(LayoutRect::infiniteIntRect()));
     foregroundRect = ClipRect(LayoutRect(LayoutRect::infiniteIntRect()));
   } else {
     backgroundRect = clipRectWithGeometryMapper(context, false);
+
+    backgroundRect.move(context.subPixelAccumulation);
+    backgroundRect.intersect(paintDirtyRect);
+
     applyOverflowClipToBackgroundRectWithGeometryMapper(context,
                                                         backgroundRect);
 
-    backgroundRect.move(
-        context.subPixelAccumulation);  // TODO(chrishtr): is this needed?

[chrishtr, 2017/02/16 21:44:28]

Confirmed that it is needed.

-    backgroundRect.intersect(paintDirtyRect);
-
-    foregroundRect.move(
-        context.subPixelAccumulation);  // TODO(chrishtr): is this needed?
     foregroundRect = clipRectWithGeometryMapper(context, true);
+    foregroundRect.move(context.subPixelAccumulation);

[chrishtr, 2017/02/16 21:44:28]

I moved this above new line 309 for consistency with the old code.

     foregroundRect.intersect(paintDirtyRect);
   }
   LayoutPoint offset;
   if (offsetFromRoot)
     offset = *offsetFromRoot;
   else
     m_layer.convertToLayerCoords(context.rootLayer, offset);
   layerBounds = LayoutRect(offset, LayoutSize(m_layer.size()));
 
 #ifdef CHECK_CLIP_RECTS
   ClipRect testBackgroundRect, testForegroundRect;
   LayoutRect testLayerBounds;
-  PaintLayerClipper(m_layer, false)
+  PaintLayerClipper(m_layer, nullptr)
       .calculateRects(context, paintDirtyRect, testLayerBounds,
-                      testBackgroundRect, testForegroundRect);
+                      testBackgroundRect, testForegroundRect, offsetFromRoot);
   CHECK_RECTS_EQ(testBackgroundRect, backgroundRect);
   CHECK_RECTS_EQ(testForegroundRect, foregroundRect);
   CHECK_RECTS_EQ(testLayerBounds, layerBounds);
 #endif
 }
 
 void PaintLayerClipper::calculateRects(
     const ClipRectsContext& context,
     const LayoutRect& paintDirtyRect,
     LayoutRect& layerBounds,
@@ skipping 177 matching lines @@
   mapLocalToRootWithGeometryMapper(context, layerBoundsWithVisualOverflow);
   clip.intersect(layerBoundsWithVisualOverflow);
 }
 
 ClipRect PaintLayerClipper::backgroundClipRect(
     const ClipRectsContext& context) const {
   if (m_geometryMapper) {
     ClipRect backgroundClipRect = clipRectWithGeometryMapper(context, false);
 #ifdef CHECK_CLIP_RECTS
     ClipRect testBackgroundClipRect =
-        PaintLayerClipper(m_layer, false).backgroundClipRect(context);
+        PaintLayerClipper(m_layer, nullptr).backgroundClipRect(context);
     CHECK_RECTS_EQ(testBackgroundClipRect, backgroundClipRect);
 #endif
     return backgroundClipRect;
   }
   DCHECK(m_layer.parent());
   LayoutView* layoutView = m_layer.layoutObject()->view();
   DCHECK(layoutView);
 
   RefPtr<ClipRects> parentClipRects = ClipRects::create();
   if (&m_layer == context.rootLayer) {
@@ skipping 59 matching lines @@
     const LayoutSize& subpixelAccumulation) const {
   DCHECK(!m_geometryMapper);
   ClipRectsContext context(rootLayer, PaintingClipRects,
                            IgnoreOverlayScrollbarSize, subpixelAccumulation);
   if (respectOverflowClip == IgnoreOverflowClip)
     context.setIgnoreOverflowClip();
   return getClipRects(context);
 }
 
 }  // namespace blink