Subpixel rendering: Animating HTML elements leaves trails when embedd…

…ed to a subpxiel positioned iframe. https://bugs.webkit.org/show_bug.cgi?id=139691 rdar://problem/19078958 Reviewed by Simon Fraser. This patch ensures that repaint rect and actual paint coordinate calculations are in sync. Source/WebCore: RenderWidget painting still snaps final coordinates to integral positions. We need to mimic the same snapping behaviour when the repaint rects are being calculated so that they are in sync with the final repaint rects. This is a workaround until after widgets get pushed to device pixel positions. Test: fast/repaint/hidpi-content-inside-iframe-leaves-trails.html * rendering/RenderBox.cpp: (WebCore::RenderBox::computeRectForRepaint): * rendering/RenderView.cpp: (WebCore::RenderView::repaintViewRectangle): * rendering/RenderWidget.cpp: (WebCore::RenderWidget::paintContents): LayoutTests: * fast/repaint/hidpi-content-inside-iframe-leaves-trails-expected.txt: Added. * fast/repaint/hidpi-content-inside-iframe-leaves-trails.html: Added. Canonical link: https://commits.webkit.org/157612@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@177412 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Dec 17, 2014 · 1e4996bf666e5ceea3a9b57fcef22bc5f79d445b · 1e4996b
1 parent 440506d
commit 1e4996bf666e5ceea3a9b57fcef22bc5f79d445b
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2014-12-16  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: Animating HTML elements leaves trails when embedded to a subpxiel positioned iframe.
+        https://bugs.webkit.org/show_bug.cgi?id=139691
+        rdar://problem/19078958
+
+        Reviewed by Simon Fraser.
+
+        This patch ensures that repaint rect and actual paint coordinate calculations are in sync.
+
+        * fast/repaint/hidpi-content-inside-iframe-leaves-trails-expected.txt: Added.
+        * fast/repaint/hidpi-content-inside-iframe-leaves-trails.html: Added.
+
 2014-12-16  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Rebaseline Mavericks test result after r177398

diff --git a/LayoutTests/fast/repaint/hidpi-content-inside-iframe-leaves-trails-expected.txt b/LayoutTests/fast/repaint/hidpi-content-inside-iframe-leaves-trails-expected.txt
@@ -0,0 +1,5 @@
+PASS repaintRects.indexOf('rect 0 0 101 100') is not -1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/repaint/hidpi-content-inside-iframe-leaves-trails.html b/LayoutTests/fast/repaint/hidpi-content-inside-iframe-leaves-trails.html
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that subpixel positioned iframe content's repaint rect is properly calculated.</title>
+<script>jsTestIsAsync = true;</script>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+  if (window.testRunner && window.internals) {
+    testRunner.dumpAsText(false);
+    internals.startTrackingRepaints();
+  }
+
+var repaintRects;
+function test() {
+  setTimeout(function() {
+    var iframeDocument = window.frames["embeddedFrame"].document;
+    iframeDocument.getElementById("foo").style.left = "50px";
+    iframeDocument.body.offsetWidth;
+    if (window.testRunner && window.internals) {
+        repaintRects = internals.repaintRectsAsText();
+        shouldNotBe("repaintRects.indexOf('rect 0 0 101 100')", "-1");
+        internals.stopTrackingRepaints();
+        finishJSTest();
+    }
+  }, 10);
+  }
+</script>
+</head>
+<body>
+  <iframe onload="test()" name="embeddedFrame" style="position:absolute; top: 0px; left: 0.4px; width: 200px; height: 200px" frameBorder=0; srcdoc='
+    <!DOCTYPE html>
+    <html>	
+    <head>
+    <style>
+      div {
+        position: absolute;
+        background: red;
+        height: 100px;
+        width: 100px;
+        top: 0px;
+        left: 0.7px;
+      }
+    </style>
+    </head>
+    <body>
+      <div id=foo></div>
+    </body>
+    </html>'>
+  </iframe>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
+
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2014-12-16  Zalan Bujtas  <zalan@apple.com>
+
+        Subpixel rendering: Animating HTML elements leaves trails when embedded to a subpxiel positioned iframe.
+        https://bugs.webkit.org/show_bug.cgi?id=139691
+        rdar://problem/19078958
+
+        Reviewed by Simon Fraser.
+
+        This patch ensures that repaint rect and actual paint coordinate calculations are in sync.
+
+        RenderWidget painting still snaps final coordinates to integral positions. We need to
+        mimic the same snapping behaviour when the repaint rects are being calculated so that
+        they are in sync with the final repaint rects. This is a workaround until after
+        widgets get pushed to device pixel positions.
+
+        Test: fast/repaint/hidpi-content-inside-iframe-leaves-trails.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::computeRectForRepaint):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::repaintViewRectangle):
+        * rendering/RenderWidget.cpp:
+        (WebCore::RenderWidget::paintContents):
+
 2014-12-16  Beth Dakin  <bdakin@apple.com>
 
         REGRESSION: Preview popovers obscure the link, look wrong

diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
@@ -2185,16 +2185,20 @@ void RenderBox::computeRectForRepaint(const RenderLayerModelObject* repaintConta
     if (isWritingModeRoot() && !isOutOfFlowPositioned())
         flipForWritingMode(rect);
 
+    LayoutSize locationOffset = this->locationOffset();
+    // FIXME: This is needed as long as RenderWidget snaps to integral size/position.
+    if (isRenderReplaced() && isWidget())
+        locationOffset = toIntSize(flooredIntPoint(locationOffset));
     LayoutPoint topLeft = rect.location();
-    topLeft.move(locationOffset());
+    topLeft.move(locationOffset);
 
     // We are now in our parent container's coordinate space.  Apply our transform to obtain a bounding box
     // in the parent's coordinate space that encloses us.
     if (hasLayer() && layer()->transform()) {
         fixed = position == FixedPosition;
         rect = LayoutRect(encloseRectToDevicePixels(layer()->transform()->mapRect(rect), document().deviceScaleFactor()));
         topLeft = rect.location();
-        topLeft.move(locationOffset());
+        topLeft.move(locationOffset);
     } else if (position == FixedPosition)
         fixed = true;
 

diff --git a/Source/WebCore/rendering/RenderView.cpp b/Source/WebCore/rendering/RenderView.cpp
@@ -619,16 +619,18 @@ void RenderView::repaintViewRectangle(const LayoutRect& repaintRect) const
     if (!shouldRepaint(repaintRect))
         return;
 
+    // FIXME: enclosingRect is needed as long as we integral snap ScrollView/FrameView/RenderWidget size/position.
+    IntRect enclosingRect = enclosingIntRect(repaintRect);
     if (auto ownerElement = document().ownerElement()) {
         RenderBox* ownerBox = ownerElement->renderBox();
         if (!ownerBox)
             return;
         LayoutRect viewRect = this->viewRect();
 #if PLATFORM(IOS)
         // Don't clip using the visible rect since clipping is handled at a higher level on iPhone.
-        LayoutRect adjustedRect = repaintRect;
+        LayoutRect adjustedRect = enclosingRect;
 #else
-        LayoutRect adjustedRect = intersection(repaintRect, viewRect);
+        LayoutRect adjustedRect = intersection(enclosingRect, viewRect);
 #endif
         adjustedRect.moveBy(-viewRect.location());
         adjustedRect.moveBy(ownerBox->contentBoxRect().location());
@@ -637,9 +639,6 @@ void RenderView::repaintViewRectangle(const LayoutRect& repaintRect) const
     }
 
     frameView().addTrackedRepaintRect(snapRectToDevicePixels(repaintRect, document().deviceScaleFactor()));
-
-    // FIXME: convert all repaint rect dependencies to FloatRect.
-    IntRect enclosingRect = enclosingIntRect(repaintRect);
     if (!m_accumulatedRepaintRegion) {
         frameView().repaintContentRectangle(enclosingRect);
         return;

diff --git a/Source/WebCore/rendering/RenderWidget.cpp b/Source/WebCore/rendering/RenderWidget.cpp
@@ -217,22 +217,20 @@ void RenderWidget::styleDidChange(StyleDifference diff, const RenderStyle* oldSt
 
 void RenderWidget::paintContents(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
-    LayoutPoint adjustedPaintOffset = paintOffset + location();
-
+    IntPoint contentPaintOffset = roundedIntPoint(paintOffset + location() + contentBoxRect().location());
     // Tell the widget to paint now. This is the only time the widget is allowed
     // to paint itself. That way it will composite properly with z-indexed layers.
-    IntPoint widgetLocation = m_widget->frameRect().location();
-    IntPoint paintLocation(roundToInt(adjustedPaintOffset.x() + borderLeft() + paddingLeft()),
-        roundToInt(adjustedPaintOffset.y() + borderTop() + paddingTop()));
     LayoutRect paintRect = paintInfo.rect;
 
-    LayoutSize widgetPaintOffset = paintLocation - widgetLocation;
+    IntPoint widgetLocation = m_widget->frameRect().location();
+    IntSize widgetPaintOffset = contentPaintOffset - widgetLocation;
     // When painting widgets into compositing layers, tx and ty are relative to the enclosing compositing layer,
     // not the root. In this case, shift the CTM and adjust the paintRect to be root-relative to fix plug-in drawing.
     if (!widgetPaintOffset.isZero()) {
         paintInfo.context->translate(widgetPaintOffset);
         paintRect.move(-widgetPaintOffset);
     }
+    // FIXME: Remove repaintrect encolsing/integral snapping when RenderWidget becomes device pixel snapped.
     m_widget->paint(paintInfo.context, snappedIntRect(paintRect));
 
     if (!widgetPaintOffset.isZero())