Attachment #225855 for bug #129656

View Details Raw Unified Return to Bug 129656

| Differences between



2014-03-04  Zalan Bujtas  <zalan@apple.com>

        Subpixel rendering: Wrong cliprect on absolute positioned elements.
        https://bugs.webkit.org/show_bug.cgi?id=129656

        Reviewed by NOBODY (OOPS!).

        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-04  Zalan Bujtas  <zalan@apple.com>

        Enable device pixel repaint rect tracking.
        https://bugs.webkit.org/show_bug.cgi?id=129712


Line 0 a/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt_sec1
- a/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt +33 lines
	1	(repaint rects
	2	(rect 0 0 0.50 22)
	3	(rect 22 0 0.50 22)
	4	(rect 21 0 1 22)
	5	(rect 25 0 0.50 22)
	6	(rect 47 0 0.50 22)
	7	(rect 46 0 1 22)
	8	(rect 50 0 0.50 22)
	9	(rect 72 0 0.50 22)
	10	(rect 71 0 1 22)
	11	(rect 75 0 0.50 22)
	12	(rect 97 0 0.50 22)
	13	(rect 96 0 1 22)
	14	(rect 100 0 0.50 22)
	15	(rect 122 0 0.50 22)
	16	(rect 121 0 1 22)
	17	(rect 125 0 0.50 22)
	18	(rect 147 0 0.50 22)
	19	(rect 146 0 1 22)
	20	(rect 150 0 0.50 22)
	21	(rect 172 0 0.50 22)
	22	(rect 171 0 1 22)
	23	(rect 175 0 0.50 22)
	24	(rect 197 0 0.50 22)
	25	(rect 196 0 1 22)
	26	(rect 200 0 0.50 22)
	27	(rect 222 0 0.50 22)
	28	(rect 221 0 1 22)
	29	(rect 225 0 0.50 22)
	30	(rect 247 0 0.50 22)
	31	(rect 246 0 1 22)
	32	)
	33



<!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>



2014-03-04  Zalan Bujtas  <zalan@apple.com>

        Subpixel rendering: Wrong cliprect on absolute positioned elements.
        https://bugs.webkit.org/show_bug.cgi?id=129656

        Reviewed by NOBODY (OOPS!).

        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-04  Zalan Bujtas  <zalan@apple.com>

        Enable device pixel repaint rect tracking.
        https://bugs.webkit.org/show_bug.cgi?id=129712




        else
            containerRelativeQuad = localToContainerQuad(FloatRect(box), repaintContainer);

        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 LayoutRect(pixelSnappedForPainting(box, document().deviceScaleFactor()));
}

void RenderBox::addFocusRingRects(Vector<IntRect>& rects, const LayoutPoint& additionalOffset, const RenderLayerModelObject*)



        LayoutUnit shadowLeft;
        LayoutUnit shadowRight;
        style().getBoxShadowHorizontalExtent(shadowLeft, shadowRight);
        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)));

        LayoutUnit shadowTop;
        LayoutUnit shadowBottom;
        style().getBoxShadowVerticalExtent(shadowTop, shadowBottom);
        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())));

Lines 74-80 LayoutRect RenderSVGModelObject::outlineBoundsForRepaint(const RenderLayerModelO a/Source/WebCore/rendering/svg/RenderSVGModelObject.cpp_sec1
- a/Source/WebCore/rendering/svg/RenderSVGModelObject.cpp -1 / +1 lines
74	adjustRectForOutlineAndShadow(box);	74	adjustRectForOutlineAndShadow(box);
75		75
76	FloatQuad containerRelativeQuad = localToContainerQuad(FloatRect(box), repaintContainer);	76	FloatQuad containerRelativeQuad = localToContainerQuad(FloatRect(box), repaintContainer);
77	return containerRelativeQuad.enclosingBoundingBox();	77	return LayoutRect(pixelSnappedForPainting(LayoutRect(containerRelativeQuad.boundingBox()), document().deviceScaleFactor()));
78	}	78	}
79		79
80	void RenderSVGModelObject::absoluteRects(Vector<IntRect>& rects, const LayoutPoint& accumulatedOffset) const	80	void RenderSVGModelObject::absoluteRects(Vector<IntRect>& rects, const LayoutPoint& accumulatedOffset) const

Return to Bug 129656

Lines 639-652 LayoutRect RenderBox::outlineBoundsForRepaint(const RenderLayerModelObject* repa a/Source/WebCore/rendering/RenderBox.cpp_sec1
- a/Source/WebCore/rendering/RenderBox.cpp -2 / +2 lines
639	else	639	else
640	containerRelativeQuad = localToContainerQuad(FloatRect(box), repaintContainer);	640	containerRelativeQuad = localToContainerQuad(FloatRect(box), repaintContainer);
641		641
642	box = containerRelativeQuad.enclosingBoundingBox();	642	box = LayoutRect(containerRelativeQuad.boundingBox());
643	}	643	}
644		644
645	// FIXME: layoutDelta needs to be applied in parts before/after transforms and	645	// FIXME: layoutDelta needs to be applied in parts before/after transforms and
646	// repaint containers. https://bugs.webkit.org/show_bug.cgi?id=23308	646	// repaint containers. https://bugs.webkit.org/show_bug.cgi?id=23308
647	box.move(view().layoutDelta());	647	box.move(view().layoutDelta());
648		648
649	return box;	649	return LayoutRect(pixelSnappedForPainting(box, document().deviceScaleFactor()));
650	}	650	}
651		651
652	void RenderBox::addFocusRingRects(Vector<IntRect>& rects, const LayoutPoint& additionalOffset, const RenderLayerModelObject*)	652	void RenderBox::addFocusRingRects(Vector<IntRect>& rects, const LayoutPoint& additionalOffset, const RenderLayerModelObject*)

Lines 1-5 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +15 lines
1	2014-03-04 Zalan Bujtas <zalan@apple.com>	1	2014-03-04 Zalan Bujtas <zalan@apple.com>
2		2
		3	Subpixel rendering: Wrong cliprect on absolute positioned elements.
		4	https://bugs.webkit.org/show_bug.cgi?id=129656
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	outlineBoundsForRepaint() is expected to return the outline repaint rect. Using enclosingIntRect()
		9	to calculate the outline boundaries breaks repaint logic in RenderElement::repaintAfterLayoutIfNeeded().
		10	Since enclosingIntRect() can return bigger rect than repaint rect, the old/new bounds' dimensions could end up
		11	being different which triggers the size change repaint code path.
		12
		13	* fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move-expected.txt: Added.
		14	* fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html: Added.
		15
		16	2014-03-04 Zalan Bujtas <zalan@apple.com>
		17
3	Enable device pixel repaint rect tracking.	18	Enable device pixel repaint rect tracking.
4	https://bugs.webkit.org/show_bug.cgi?id=129712	19	https://bugs.webkit.org/show_bug.cgi?id=129712
5		20

Line 0 a/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html_sec1
- a/LayoutTests/fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html +53 lines
	1	<!DOCTYPE html>
	2	<html>
	3	<head>
	4	<title>This tests absolute positioned elements' repaint clipping after move. Pass if all borders are painted properly.</title>
	5	<head>
	6	<style>
	7	div {
	8	position: absolute;
	9	border: green solid 1px;
	10	width: 20px;
	11	height: 20px;
	12	left: 0px;
	13	top: 0px;
	14	}
	15	</style>
	16	</head>
	17	<body>
	18	<p id="container"></p>
	19	<script>
	20	var container = document.getElementById("container");
	21	for (j = 0; j < 10; ++j) {
	22	var e = document.createElement("div");
	23	e.style.left = (25 * j) + "px";
	24	container.appendChild(e);
	25	}
	26
	27	function move() {
	28	divs = document.getElementsByTagName("div");
	29	for (i = 0; i < divs.length; ++i)
	30	divs[i].style.left = (parseFloat(divs[i].style.left) + 0.3) + "px";
	31
	32	if (window.testRunner && window.internals) {
	33	var dummy = document.body.offsetTop;
	34	var repaintRects = window.internals.repaintRectsAsText();
	35	window.internals.stopTrackingRepaints();
	36
	37	var pre = document.createElement('pre');
	38	document.body.appendChild(pre);
	39	pre.innerHTML = repaintRects;
	40	testRunner.notifyDone();
	41	}
	42	}
	43
	44	if (window.testRunner && window.internals) {
	45	window.testRunner.dumpAsText(false);
	46	window.internals.startTrackingRepaints();
	47	testRunner.waitUntilDone();
	48	}
	49
	50	setTimeout(move, 0);
	51	</script>
	52	</body>
	53	</html>

Lines 1-5 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +21 lines
1	2014-03-04 Zalan Bujtas <zalan@apple.com>	1	2014-03-04 Zalan Bujtas <zalan@apple.com>
2		2
		3	Subpixel rendering: Wrong cliprect on absolute positioned elements.
		4	https://bugs.webkit.org/show_bug.cgi?id=129656
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	outlineBoundsForRepaint() is expected to return the outline repaint rect. Using enclosingIntRect()
		9	to calculate the outline boundaries breaks repaint logic in RenderElement::repaintAfterLayoutIfNeeded().
		10	Since enclosingIntRect() can return bigger rect than repaint rect, the old/new bounds' dimensions could end up
		11	being different which triggers the size change repaint code path.
		12
		13	Test: fast/repaint/hidpi-absolute-positioned-element-wrong-cliprect-after-move.html
		14
		15	* rendering/RenderBox.cpp:
		16	(WebCore::RenderBox::outlineBoundsForRepaint):
		17	* rendering/RenderElement.cpp:
		18	(WebCore::RenderElement::repaintAfterLayoutIfNeeded):
		19	* rendering/svg/RenderSVGModelObject.cpp:
		20	(WebCore::RenderSVGModelObject::outlineBoundsForRepaint):
		21
		22	2014-03-04 Zalan Bujtas <zalan@apple.com>
		23
3	Enable device pixel repaint rect tracking.	24	Enable device pixel repaint rect tracking.
4	https://bugs.webkit.org/show_bug.cgi?id=129712	25	https://bugs.webkit.org/show_bug.cgi?id=129712
5		26

Lines 1254-1260 bool RenderElement::repaintAfterLayoutIfNeeded(const RenderLayerModelObject* rep a/Source/WebCore/rendering/RenderElement.cpp_sec1
- a/Source/WebCore/rendering/RenderElement.cpp -2 / +2 lines
1254	LayoutUnit shadowLeft;	1254	LayoutUnit shadowLeft;
1255	LayoutUnit shadowRight;	1255	LayoutUnit shadowRight;
1256	style().getBoxShadowHorizontalExtent(shadowLeft, shadowRight);	1256	style().getBoxShadowHorizontalExtent(shadowLeft, shadowRight);
1257	int borderRight = isBox() ? toRenderBox(this)->borderRight() : LayoutUnit::fromPixel(0);	1257	LayoutUnit borderRight = isBox() ? toRenderBox(this)->borderRight() : LayoutUnit::fromPixel(0);
1258	LayoutUnit boxWidth = isBox() ? toRenderBox(this)->width() : LayoutUnit();	1258	LayoutUnit boxWidth = isBox() ? toRenderBox(this)->width() : LayoutUnit();
1259	LayoutUnit minInsetRightShadowExtent = std::min<LayoutUnit>(-insetShadowExtent.right(), std::min<LayoutUnit>(newBounds.width(), oldBounds.width()));	1259	LayoutUnit minInsetRightShadowExtent = std::min<LayoutUnit>(-insetShadowExtent.right(), std::min<LayoutUnit>(newBounds.width(), oldBounds.width()));
1260	LayoutUnit borderWidth = std::max<LayoutUnit>(borderRight, std::max<LayoutUnit>(valueForLength(style().borderTopRightRadius().width(), boxWidth, &view()), valueForLength(style().borderBottomRightRadius().width(), boxWidth)));	1260	LayoutUnit borderWidth = std::max<LayoutUnit>(borderRight, std::max<LayoutUnit>(valueForLength(style().borderTopRightRadius().width(), boxWidth, &view()), valueForLength(style().borderBottomRightRadius().width(), boxWidth)));
Lines 1274-1280 bool RenderElement::repaintAfterLayoutIfNeeded(const RenderLayerModelObject* rep a/Source/WebCore/rendering/RenderElement.cpp_sec2
1274	LayoutUnit shadowTop;	1274	LayoutUnit shadowTop;
1275	LayoutUnit shadowBottom;	1275	LayoutUnit shadowBottom;
1276	style().getBoxShadowVerticalExtent(shadowTop, shadowBottom);	1276	style().getBoxShadowVerticalExtent(shadowTop, shadowBottom);
1277	int borderBottom = isBox() ? toRenderBox(this)->borderBottom() : LayoutUnit::fromPixel(0);	1277	LayoutUnit borderBottom = isBox() ? toRenderBox(this)->borderBottom() : LayoutUnit::fromPixel(0);
1278	LayoutUnit boxHeight = isBox() ? toRenderBox(this)->height() : LayoutUnit();	1278	LayoutUnit boxHeight = isBox() ? toRenderBox(this)->height() : LayoutUnit();
1279	LayoutUnit minInsetBottomShadowExtent = std::min<LayoutUnit>(-insetShadowExtent.bottom(), std::min<LayoutUnit>(newBounds.height(), oldBounds.height()));	1279	LayoutUnit minInsetBottomShadowExtent = std::min<LayoutUnit>(-insetShadowExtent.bottom(), std::min<LayoutUnit>(newBounds.height(), oldBounds.height()));
1280	LayoutUnit borderHeight = std::max<LayoutUnit>(borderBottom, std::max<LayoutUnit>(valueForLength(style().borderBottomLeftRadius().height(), boxHeight), valueForLength(style().borderBottomRightRadius().height(), boxHeight, &view())));	1280	LayoutUnit borderHeight = std::max<LayoutUnit>(borderBottom, std::max<LayoutUnit>(valueForLength(style().borderBottomLeftRadius().height(), boxHeight), valueForLength(style().borderBottomRightRadius().height(), boxHeight, &view())));