Subpixel rendering: Wrong cliprect on absolute positioned elements.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Mar 2014 18:11:50 +0000 (18:11 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Mar 2014 18:11:50 +0000 (18:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=129656

Reviewed by Simon Fraser.

outlineBoundsForRepaint() is expected to return the outline repaint rect. Using enclosingIntRect()
to calculate the outline boundaries breaks repaint logic in RenderElement::repaintAfterLayoutIfNeeded().
Since enclosingIntRect() can return bigger rect than repaint rect, the old/new bounds' dimensions could end up
being different which triggers the size change repaint code path.

Source/WebCore:

Test: fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::outlineBoundsForRepaint):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::repaintAfterLayoutIfNeeded):
* rendering/svg/RenderSVGModelObject.cpp:
(WebCore::RenderSVGModelObject::outlineBoundsForRepaint):

LayoutTests:

* fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt: Added.
* fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@165113 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt [new file with mode: 0644]
LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/svg/RenderSVGModelObject.cpp

index 1004970..f9a13f0 100644 (file)
@@ -1,3 +1,18 @@
+2014-03-05  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: Wrong cliprect on absolute positioned elements.
+        https://bugs.webkit.org/show_bug.cgi?id=129656
+
+        Reviewed by Simon Fraser.
+
+        outlineBoundsForRepaint() is expected to return the outline repaint rect. Using enclosingIntRect()
+        to calculate the outline boundaries breaks repaint logic in RenderElement::repaintAfterLayoutIfNeeded().
+        Since enclosingIntRect() can return bigger rect than repaint rect, the old/new bounds' dimensions could end up
+        being different which triggers the size change repaint code path.
+
+        * fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt: Added.
+        * fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html: Added.
+
 2014-03-05  Chang Shu  <cshu@webkit.org>
 
         Copying wrapping text results in multiple spaces between wrapped lines stripped.
diff --git a/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt b/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt
new file mode 100644 (file)
index 0000000..cb73eee
--- /dev/null
@@ -0,0 +1,23 @@
+(repaint rects
+  (rect 0 0 22 22)
+  (rect 0.50 0 22 22)
+  (rect 25 0 22 22)
+  (rect 25.50 0 22 22)
+  (rect 50 0 22 22)
+  (rect 50.50 0 22 22)
+  (rect 75 0 22 22)
+  (rect 75.50 0 22 22)
+  (rect 100 0 22 22)
+  (rect 100.50 0 22 22)
+  (rect 125 0 22 22)
+  (rect 125.50 0 22 22)
+  (rect 150 0 22 22)
+  (rect 150.50 0 22 22)
+  (rect 175 0 22 22)
+  (rect 175.50 0 22 22)
+  (rect 200 0 22 22)
+  (rect 200.50 0 22 22)
+  (rect 225 0 22 22)
+  (rect 225.50 0 22 22)
+)
+
diff --git a/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html b/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html
new file mode 100644 (file)
index 0000000..1b39049
--- /dev/null
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests absolute positioned elements' repaint clipping after move. Pass if all borders are painted properly.</title>
+<head>
+<style>
+  div {
+    position: absolute;
+    border: green solid 1px;
+    width: 20px;
+    height: 20px;
+    left: 0px;
+    top: 0px;
+  }
+</style>
+</head>
+<body>
+<p id="container"></p>
+<script>
+  var container = document.getElementById("container");
+  for (j = 0; j < 10; ++j) {
+    var e = document.createElement("div");
+    e.style.left = (25 * j) + "px";
+    container.appendChild(e);
+  }
+  
+  function move() {
+    divs = document.getElementsByTagName("div");
+    for (i = 0; i < divs.length; ++i)
+      divs[i].style.left = (parseFloat(divs[i].style.left) + 0.3) + "px";
+
+    if (window.testRunner && window.internals) {
+        var dummy = document.body.offsetTop;
+        var repaintRects = window.internals.repaintRectsAsText();
+        window.internals.stopTrackingRepaints();
+
+        var pre = document.createElement('pre');
+        document.body.appendChild(pre);
+        pre.innerHTML = repaintRects;
+        testRunner.notifyDone();
+      }
+  }
+  
+  if (window.testRunner && window.internals) {
+    window.testRunner.dumpAsText(false);
+    window.internals.startTrackingRepaints();
+    testRunner.waitUntilDone();
+  }
+
+  setTimeout(move, 0);
+</script>
+</body>
+</html>
index 9084de4..6f1b3a5 100644 (file)
@@ -1,3 +1,24 @@
+2014-03-05  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: Wrong cliprect on absolute positioned elements.
+        https://bugs.webkit.org/show_bug.cgi?id=129656
+
+        Reviewed by Simon Fraser.
+
+        outlineBoundsForRepaint() is expected to return the outline repaint rect. Using enclosingIntRect()
+        to calculate the outline boundaries breaks repaint logic in RenderElement::repaintAfterLayoutIfNeeded().
+        Since enclosingIntRect() can return bigger rect than repaint rect, the old/new bounds' dimensions could end up
+        being different which triggers the size change repaint code path.
+
+        Test: fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::outlineBoundsForRepaint):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::repaintAfterLayoutIfNeeded):
+        * rendering/svg/RenderSVGModelObject.cpp:
+        (WebCore::RenderSVGModelObject::outlineBoundsForRepaint):
+
 2014-03-05  Krzysztof Czech  <k.czech@samsung.com>
 
         [ATK] Expose missing functionalities of AtkTableCell to AT.
index 2ada0ff..af45bb6 100644 (file)
@@ -639,14 +639,14 @@ LayoutRect RenderBox::outlineBoundsForRepaint(const RenderLayerModelObject* repa
         else
             containerRelativeQuad = localToContainerQuad(FloatRect(box), repaintContainer);
 
-        box = containerRelativeQuad.enclosingBoundingBox();
+        box = LayoutRect(containerRelativeQuad.boundingBox());
     }
     
     // FIXME: layoutDelta needs to be applied in parts before/after transforms and
     // repaint containers. https://bugs.webkit.org/show_bug.cgi?id=23308
     box.move(view().layoutDelta());
 
-    return box;
+    return LayoutRect(pixelSnappedForPainting(box, document().deviceScaleFactor()));
 }
 
 void RenderBox::addFocusRingRects(Vector<IntRect>& rects, const LayoutPoint& additionalOffset, const RenderLayerModelObject*)
index abececc..77586e9 100644 (file)
@@ -1254,7 +1254,7 @@ bool RenderElement::repaintAfterLayoutIfNeeded(const RenderLayerModelObject* rep
         LayoutUnit shadowLeft;
         LayoutUnit shadowRight;
         style().getBoxShadowHorizontalExtent(shadowLeft, shadowRight);
-        int borderRight = isBox() ? toRenderBox(this)->borderRight() : LayoutUnit::fromPixel(0);
+        LayoutUnit borderRight = isBox() ? toRenderBox(this)->borderRight() : LayoutUnit::fromPixel(0);
         LayoutUnit boxWidth = isBox() ? toRenderBox(this)->width() : LayoutUnit();
         LayoutUnit minInsetRightShadowExtent = std::min<LayoutUnit>(-insetShadowExtent.right(), std::min<LayoutUnit>(newBounds.width(), oldBounds.width()));
         LayoutUnit borderWidth = std::max<LayoutUnit>(borderRight, std::max<LayoutUnit>(valueForLength(style().borderTopRightRadius().width(), boxWidth, &view()), valueForLength(style().borderBottomRightRadius().width(), boxWidth)));
@@ -1274,7 +1274,7 @@ bool RenderElement::repaintAfterLayoutIfNeeded(const RenderLayerModelObject* rep
         LayoutUnit shadowTop;
         LayoutUnit shadowBottom;
         style().getBoxShadowVerticalExtent(shadowTop, shadowBottom);
-        int borderBottom = isBox() ? toRenderBox(this)->borderBottom() : LayoutUnit::fromPixel(0);
+        LayoutUnit borderBottom = isBox() ? toRenderBox(this)->borderBottom() : LayoutUnit::fromPixel(0);
         LayoutUnit boxHeight = isBox() ? toRenderBox(this)->height() : LayoutUnit();
         LayoutUnit minInsetBottomShadowExtent = std::min<LayoutUnit>(-insetShadowExtent.bottom(), std::min<LayoutUnit>(newBounds.height(), oldBounds.height()));
         LayoutUnit borderHeight = std::max<LayoutUnit>(borderBottom, std::max<LayoutUnit>(valueForLength(style().borderBottomLeftRadius().height(), boxHeight), valueForLength(style().borderBottomRightRadius().height(), boxHeight, &view())));
index a9db9d0..c9d0526 100644 (file)
@@ -74,7 +74,7 @@ LayoutRect RenderSVGModelObject::outlineBoundsForRepaint(const RenderLayerModelO
     adjustRectForOutlineAndShadow(box);
 
     FloatQuad containerRelativeQuad = localToContainerQuad(FloatRect(box), repaintContainer);
-    return containerRelativeQuad.enclosingBoundingBox();
+    return LayoutRect(pixelSnappedForPainting(LayoutRect(containerRelativeQuad.boundingBox()), document().deviceScaleFactor()));
 }
 
 void RenderSVGModelObject::absoluteRects(Vector<IntRect>& rects, const LayoutPoint& accumulatedOffset) const