Attachment #163827 for bug #96625

View Details Raw Unified Return to Bug 96625

| Differences between



2012-09-13  Sami Kyostila  <skyostil@google.com>

        Refactor paint overflow clipping
        https://bugs.webkit.org/show_bug.cgi?id=96625

        Reviewed by NOBODY (OOPS!).

        The same logic for applying overflow clipping based on the cached size of a
        RenderBox has been duplicated to four different places. Consolidate them to a
        single RenderBox utility function.

        This is in preparation for https://bugs.webkit.org/show_bug.cgi?id=96087.

        No new functionality; covered by existing tests.

        * rendering/RenderBox.cpp:
        (WebCore::RenderBox::applyCachedClipAndScrollOffsetForRepaint):
        (WebCore):
        (WebCore::RenderBox::computeRectForRepaint):
        * rendering/RenderBox.h:
        (RenderBox):
        * rendering/RenderInline.cpp:
        (WebCore::RenderInline::clippedOverflowRectForRepaint):
        (WebCore::RenderInline::computeRectForRepaint):
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::computeRectForRepaint):

2012-09-13  Mikhail Pozdnyakov  <mikhail.pozdnyakov@intel.com>

        [EFL] REGRESSION (r128274): fast/overflow/overflow-height-float-not-removed-crash.html



    return layer()->size();
}

void RenderBox::applyCachedClipAndScrollOffsetForRepaint(LayoutRect& paintRect) const
{
    paintRect.move(-scrolledContentOffset()); // For overflow:auto/scroll/hidden.

    // height() is inaccurate if we're in the middle of a layout of this RenderBox, so use the
    // layer's size instead. Even if the layer's size is wrong, the layer itself will repaint
    // anyway if its size does change.
    LayoutRect clipRect(LayoutPoint(), cachedSizeForOverflowClip());
    paintRect = intersection(paintRect, clipRect);
}

LayoutUnit RenderBox::minPreferredLogicalWidth() const
{
    if (preferredLogicalWidthsDirty())


    // FIXME: We ignore the lightweight clipping rect that controls use, since if |o| is in mid-layout,
    // its controlClipRect will be wrong. For overflow clip we use the values cached by the layer.
    rect.setLocation(topLeft);
    if (o->hasOverflowClip()) {
        RenderBox* containerBox = toRenderBox(o);
        containerBox->applyCachedClipAndScrollOffsetForRepaint(rect);
        // o->height() is inaccurate if we're in the middle of a layout of |o|, so use the
        // layer's size instead.  Even if the layer's size is wrong, the layer itself will repaint
        // anyway if its size does change.
        topLeft -= containerBox->scrolledContentOffset(); // For overflow:auto/scroll/hidden.

        LayoutRect repaintRect(topLeft, rect.size());
        LayoutRect boxRect(LayoutPoint(), containerBox->cachedSizeForOverflowClip());
        rect = intersection(repaintRect, boxRect);
        if (rect.isEmpty())
            return;
    }
        rect.setLocation(topLeft);

    if (containerSkipped) {
        // If the repaintContainer is below o, then we need to map the rect into repaintContainer's coordinates.

Lines 509-514 public: a/Source/WebCore/rendering/RenderBox.h_sec1
- a/Source/WebCore/rendering/RenderBox.h +1 lines
509		509
510	IntSize scrolledContentOffset() const;	510	IntSize scrolledContentOffset() const;
511	LayoutSize cachedSizeForOverflowClip() const;	511	LayoutSize cachedSizeForOverflowClip() const;
		512	void applyCachedClipAndScrollOffsetForRepaint(LayoutRect& paintRect) const;
512		513
513	virtual bool hasRelativeDimensions() const;	514	virtual bool hasRelativeDimensions() const;
514	virtual bool hasRelativeLogicalHeight() const;	515	virtual bool hasRelativeLogicalHeight() const;



    if (cb->hasColumns())
        cb->adjustRectForColumns(repaintRect);

    if (cb->hasOverflowClip())
        cb->applyCachedClipAndScrollOffsetForRepaint(repaintRect);
        // layer's size instead.  Even if the layer's size is wrong, the layer itself will repaint
        // anyway if its size does change.
        repaintRect.move(-cb->scrolledContentOffset()); // For overflow:auto/scroll/hidden.

        LayoutRect boxRect(LayoutPoint(), cb->cachedSizeForOverflowClip());
        repaintRect.intersect(boxRect);
    }

    cb->computeRectForRepaint(repaintContainer, repaintRect);


    
    // FIXME: We ignore the lightweight clipping rect that controls use, since if |o| is in mid-layout,
    // its controlClipRect will be wrong. For overflow clip we use the values cached by the layer.
    rect.setLocation(topLeft);
    if (o->hasOverflowClip()) {
        RenderBox* containerBox = toRenderBox(o);
        containerBox->applyCachedClipAndScrollOffsetForRepaint(rect);
        // o->height() is inaccurate if we're in the middle of a layout of |o|, so use the
        // layer's size instead.  Even if the layer's size is wrong, the layer itself will repaint
        // anyway if its size does change.
        topLeft -= containerBox->scrolledContentOffset(); // For overflow:auto/scroll/hidden.

        LayoutRect repaintRect(topLeft, rect.size());
        LayoutRect boxRect(LayoutPoint(), containerBox->cachedSizeForOverflowClip());
        rect = intersection(repaintRect, boxRect);
        if (rect.isEmpty())
            return;
    }
        rect.setLocation(topLeft);

    if (containerSkipped) {
        // If the repaintContainer is below o, then we need to map the rect into repaintContainer's coordinates.



        }

        if (o->hasOverflowClip()) {
            // o->height() is inaccurate if we're in the middle of a layout of |o|, so use the
            // layer's size instead.  Even if the layer's size is wrong, the layer itself will repaint
            // anyway if its size does change.
            RenderBox* boxParent = toRenderBox(o);
            boxParent->applyCachedClipAndScrollOffsetForRepaint(rect);
            LayoutRect repaintRect(rect);
            repaintRect.move(-boxParent->scrolledContentOffset()); // For overflow:auto/scroll/hidden.

            LayoutRect boxRect(LayoutPoint(), boxParent->cachedSizeForOverflowClip());
            rect = intersection(repaintRect, boxRect);
            if (rect.isEmpty())
                return;
        }

Return to Bug 96625

Lines 647-652 LayoutSize RenderBox::cachedSizeForOverflowClip() const a/Source/WebCore/rendering/RenderBox.cpp_sec1
- a/Source/WebCore/rendering/RenderBox.cpp -11 / +14 lines
647	return layer()->size();	647	return layer()->size();
648	}	648	}
649		649
		650	void RenderBox::applyCachedClipAndScrollOffsetForRepaint(LayoutRect& paintRect) const
		651	{
		652	paintRect.move(-scrolledContentOffset()); // For overflow:auto/scroll/hidden.
		653
		654	// height() is inaccurate if we're in the middle of a layout of this RenderBox, so use the
		655	// layer's size instead. Even if the layer's size is wrong, the layer itself will repaint
		656	// anyway if its size does change.
		657	LayoutRect clipRect(LayoutPoint(), cachedSizeForOverflowClip());
		658	paintRect = intersection(paintRect, clipRect);
		659	}
		660
650	LayoutUnit RenderBox::minPreferredLogicalWidth() const	661	LayoutUnit RenderBox::minPreferredLogicalWidth() const
651	{	662	{
652	if (preferredLogicalWidthsDirty())	663	if (preferredLogicalWidthsDirty())
Lines 1574-1594 void RenderBox::computeRectForRepaint(RenderBoxModelObject* repaintContainer, La a/Source/WebCore/rendering/RenderBox.cpp_sec2
1574		1585
1575	// FIXME: We ignore the lightweight clipping rect that controls use, since if \|o\| is in mid-layout,	1586	// FIXME: We ignore the lightweight clipping rect that controls use, since if \|o\| is in mid-layout,
1576	// its controlClipRect will be wrong. For overflow clip we use the values cached by the layer.	1587	// its controlClipRect will be wrong. For overflow clip we use the values cached by the layer.
		1588	rect.setLocation(topLeft);
1577	if (o->hasOverflowClip()) {	1589	if (o->hasOverflowClip()) {
1578	RenderBox* containerBox = toRenderBox(o);	1590	RenderBox* containerBox = toRenderBox(o);
1579		1591	containerBox->applyCachedClipAndScrollOffsetForRepaint(rect);
1580	// o->height() is inaccurate if we're in the middle of a layout of \|o\|, so use the
1581	// layer's size instead. Even if the layer's size is wrong, the layer itself will repaint
1582	// anyway if its size does change.
1583	topLeft -= containerBox->scrolledContentOffset(); // For overflow:auto/scroll/hidden.
1584
1585	LayoutRect repaintRect(topLeft, rect.size());
1586	LayoutRect boxRect(LayoutPoint(), containerBox->cachedSizeForOverflowClip());
1587	rect = intersection(repaintRect, boxRect);
1588	if (rect.isEmpty())	1592	if (rect.isEmpty())
1589	return;	1593	return;
1590	} else	1594	}
1591	rect.setLocation(topLeft);
1592		1595
1593	if (containerSkipped) {	1596	if (containerSkipped) {
1594	// If the repaintContainer is below o, then we need to map the rect into repaintContainer's coordinates.	1597	// If the repaintContainer is below o, then we need to map the rect into repaintContainer's coordinates.

Lines 993-1007 LayoutRect RenderInline::clippedOverflowRectForRepaint(RenderBoxModelObject* rep a/Source/WebCore/rendering/RenderInline.cpp_sec1
- a/Source/WebCore/rendering/RenderInline.cpp -20 / +5 lines
993	if (cb->hasColumns())	993	if (cb->hasColumns())
994	cb->adjustRectForColumns(repaintRect);	994	cb->adjustRectForColumns(repaintRect);
995		995
996	if (cb->hasOverflowClip()) {	996	if (cb->hasOverflowClip())
997	// cb->height() is inaccurate if we're in the middle of a layout of \|cb\|, so use the	997	cb->applyCachedClipAndScrollOffsetForRepaint(repaintRect);
998	// layer's size instead. Even if the layer's size is wrong, the layer itself will repaint
999	// anyway if its size does change.
1000	repaintRect.move(-cb->scrolledContentOffset()); // For overflow:auto/scroll/hidden.
1001
1002	LayoutRect boxRect(LayoutPoint(), cb->cachedSizeForOverflowClip());
1003	repaintRect.intersect(boxRect);
1004	}
1005		998
1006	cb->computeRectForRepaint(repaintContainer, repaintRect);	999	cb->computeRectForRepaint(repaintContainer, repaintRect);
1007		1000
Lines 1073-1093 void RenderInline::computeRectForRepaint(RenderBoxModelObject* repaintContainer, a/Source/WebCore/rendering/RenderInline.cpp_sec2
1073		1066
1074	// FIXME: We ignore the lightweight clipping rect that controls use, since if \|o\| is in mid-layout,	1067	// FIXME: We ignore the lightweight clipping rect that controls use, since if \|o\| is in mid-layout,
1075	// its controlClipRect will be wrong. For overflow clip we use the values cached by the layer.	1068	// its controlClipRect will be wrong. For overflow clip we use the values cached by the layer.
		1069	rect.setLocation(topLeft);
1076	if (o->hasOverflowClip()) {	1070	if (o->hasOverflowClip()) {
1077	RenderBox* containerBox = toRenderBox(o);	1071	RenderBox* containerBox = toRenderBox(o);
1078		1072	containerBox->applyCachedClipAndScrollOffsetForRepaint(rect);
1079	// o->height() is inaccurate if we're in the middle of a layout of \|o\|, so use the
1080	// layer's size instead. Even if the layer's size is wrong, the layer itself will repaint
1081	// anyway if its size does change.
1082	topLeft -= containerBox->scrolledContentOffset(); // For overflow:auto/scroll/hidden.
1083
1084	LayoutRect repaintRect(topLeft, rect.size());
1085	LayoutRect boxRect(LayoutPoint(), containerBox->cachedSizeForOverflowClip());
1086	rect = intersection(repaintRect, boxRect);
1087	if (rect.isEmpty())	1073	if (rect.isEmpty())
1088	return;	1074	return;
1089	} else	1075	}
1090	rect.setLocation(topLeft);
1091		1076
1092	if (containerSkipped) {	1077	if (containerSkipped) {
1093	// If the repaintContainer is below o, then we need to map the rect into repaintContainer's coordinates.	1078	// If the repaintContainer is below o, then we need to map the rect into repaintContainer's coordinates.

Lines 1507-1522 void RenderObject::computeRectForRepaint(RenderBoxModelObject* repaintContainer, a/Source/WebCore/rendering/RenderObject.cpp_sec1
- a/Source/WebCore/rendering/RenderObject.cpp -9 / +1 lines
1507	}	1507	}
1508		1508
1509	if (o->hasOverflowClip()) {	1509	if (o->hasOverflowClip()) {
1510	// o->height() is inaccurate if we're in the middle of a layout of \|o\|, so use the
1511	// layer's size instead. Even if the layer's size is wrong, the layer itself will repaint
1512	// anyway if its size does change.
1513	RenderBox* boxParent = toRenderBox(o);	1510	RenderBox* boxParent = toRenderBox(o);
1514		1511	boxParent->applyCachedClipAndScrollOffsetForRepaint(rect);
1515	LayoutRect repaintRect(rect);
1516	repaintRect.move(-boxParent->scrolledContentOffset()); // For overflow:auto/scroll/hidden.
1517
1518	LayoutRect boxRect(LayoutPoint(), boxParent->cachedSizeForOverflowClip());
1519	rect = intersection(repaintRect, boxRect);
1520	if (rect.isEmpty())	1512	if (rect.isEmpty())
1521	return;	1513	return;
1522	}	1514	}

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +27 lines
		1	2012-09-13 Sami Kyostila <skyostil@google.com>
		2
		3	Refactor paint overflow clipping
		4	https://bugs.webkit.org/show_bug.cgi?id=96625
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	The same logic for applying overflow clipping based on the cached size of a
		9	RenderBox has been duplicated to four different places. Consolidate them to a
		10	single RenderBox utility function.
		11
		12	This is in preparation for https://bugs.webkit.org/show_bug.cgi?id=96087.
		13
		14	No new functionality; covered by existing tests.
		15
		16	* rendering/RenderBox.cpp:
		17	(WebCore::RenderBox::applyCachedClipAndScrollOffsetForRepaint):
		18	(WebCore):
		19	(WebCore::RenderBox::computeRectForRepaint):
		20	* rendering/RenderBox.h:
		21	(RenderBox):
		22	* rendering/RenderInline.cpp:
		23	(WebCore::RenderInline::clippedOverflowRectForRepaint):
		24	(WebCore::RenderInline::computeRectForRepaint):
		25	* rendering/RenderObject.cpp:
		26	(WebCore::RenderObject::computeRectForRepaint):
		27
1	2012-09-13 Mikhail Pozdnyakov <mikhail.pozdnyakov@intel.com>	28	2012-09-13 Mikhail Pozdnyakov <mikhail.pozdnyakov@intel.com>
2		29
3	[EFL] REGRESSION (r128274): fast/overflow/overflow-height-float-not-removed-crash.html	30	[EFL] REGRESSION (r128274): fast/overflow/overflow-height-float-not-removed-crash.html