From 0ff2e3844ea2b4334e1c6fc979ed78cbad1b29e9 Mon Sep 17 00:00:00 2001 From: "zalan@apple.com" Date: Tue, 21 Oct 2014 17:32:09 +0000 Subject: [PATCH] REGRESSION: Google Search (mobile) video thumbnails are too large. https://bugs.webkit.org/show_bug.cgi?id=137895 Reviewed by Simon Fraser. This patch fixes layer clipping when an ancestor layer has border-radius clipping. In cases, where the current layer has non-radius cliprect, while an ancestor layer has border-radius clipping, we only use the border-radius rect to clip. Source/WebCore: Test: fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html * rendering/RenderLayer.cpp: (WebCore::RenderLayer::clipToRect): (WebCore::RenderLayer::restoreClip): (WebCore::RenderLayer::collectFragments): (WebCore::RenderLayer::calculateClipRects): * rendering/RenderLayer.h: (WebCore::ClipRect::ClipRect): (WebCore::ClipRect::effectedByRadius): (WebCore::ClipRect::setEffectedByRadius): (WebCore::ClipRect::operator==): (WebCore::ClipRect::operator!=): (WebCore::ClipRect::intersect): (WebCore::ClipRect::hasRadius): Deleted. (WebCore::ClipRect::setHasRadius): Deleted. LayoutTests: * fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent-expected.html: Added. * fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html: Added. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174986 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 15 ++++++++++ ...r-radius-overflow-clipping-parent-expected.html | 25 ++++++++++++++++ ...ith-border-radius-overflow-clipping-parent.html | 32 ++++++++++++++++++++ Source/WebCore/ChangeLog | 29 ++++++++++++++++++ Source/WebCore/rendering/RenderLayer.cpp | 34 ++++++++++++---------- Source/WebCore/rendering/RenderLayer.h | 26 +++++++++-------- 6 files changed, 134 insertions(+), 27 deletions(-) create mode 100644 LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent-expected.html create mode 100644 LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 7d1fe66..db5f865 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,18 @@ +2014-10-21 Zalan Bujtas + + REGRESSION: Google Search (mobile) video thumbnails are too large. + https://bugs.webkit.org/show_bug.cgi?id=137895 + + Reviewed by Simon Fraser. + + This patch fixes layer clipping when an ancestor layer has border-radius clipping. + + In cases, where the current layer has non-radius cliprect, while an ancestor layer + has border-radius clipping, we only use the border-radius rect to clip. + + * fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent-expected.html: Added. + * fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html: Added. + 2014-10-21 Jer Noble REGRESSION (r170808): Volume slider in built-in media controls only changes volume when thumb is released, not while dragging diff --git a/LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent-expected.html b/LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent-expected.html new file mode 100644 index 0000000..83963ce --- /dev/null +++ b/LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent-expected.html @@ -0,0 +1,25 @@ + + + +This tests that parent clipping is applied properly when ancestor border-radius is present. + + + +
+
+
+ + diff --git a/LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html b/LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html new file mode 100644 index 0000000..1291631 --- /dev/null +++ b/LayoutTests/fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html @@ -0,0 +1,32 @@ + + + +This tests that parent clipping is applied properly when ancestor border-radius is present. + + + +
+
+
+
+
+ + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index f832c66..836bb77 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,32 @@ +2014-10-21 Zalan Bujtas + + REGRESSION: Google Search (mobile) video thumbnails are too large. + https://bugs.webkit.org/show_bug.cgi?id=137895 + + Reviewed by Simon Fraser. + + This patch fixes layer clipping when an ancestor layer has border-radius clipping. + + In cases, where the current layer has non-radius cliprect, while an ancestor layer + has border-radius clipping, we only use the border-radius rect to clip. + + Test: fast/clip/overflow-hidden-with-border-radius-overflow-clipping-parent.html + + * rendering/RenderLayer.cpp: + (WebCore::RenderLayer::clipToRect): + (WebCore::RenderLayer::restoreClip): + (WebCore::RenderLayer::collectFragments): + (WebCore::RenderLayer::calculateClipRects): + * rendering/RenderLayer.h: + (WebCore::ClipRect::ClipRect): + (WebCore::ClipRect::effectedByRadius): + (WebCore::ClipRect::setEffectedByRadius): + (WebCore::ClipRect::operator==): + (WebCore::ClipRect::operator!=): + (WebCore::ClipRect::intersect): + (WebCore::ClipRect::hasRadius): Deleted. + (WebCore::ClipRect::setHasRadius): Deleted. + 2014-10-20 Michael Saboff Don't create cached functions for HTMLDocument.write*() diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp index 2a06a51..a569392 100644 --- a/Source/WebCore/rendering/RenderLayer.cpp +++ b/Source/WebCore/rendering/RenderLayer.cpp @@ -3664,10 +3664,19 @@ static bool inContainingBlockChain(RenderLayer* startLayer, RenderLayer* endLaye void RenderLayer::clipToRect(const LayerPaintingInfo& paintingInfo, GraphicsContext* context, const ClipRect& clipRect, BorderRadiusClippingRule rule) { - float deviceScaleFactor = renderer().document().deviceScaleFactor(); - if (clipRect.hasRadius()) { + bool needsClipping = clipRect.rect() != paintingInfo.paintDirtyRect; + if (needsClipping || clipRect.affectedByRadius()) context->save(); - + + float deviceScaleFactor = renderer().document().deviceScaleFactor(); + bool layerHasBorderRadius = renderer().style().hasBorderRadius(); + if (needsClipping && !layerHasBorderRadius) { + LayoutRect adjustedClipRect = clipRect.rect(); + adjustedClipRect.move(paintingInfo.subpixelAccumulation); + context->clip(snapRectToDevicePixels(adjustedClipRect, deviceScaleFactor)); + } + + if (clipRect.affectedByRadius()) { // If the clip rect has been tainted by a border radius, then we have to walk up our layer chain applying the clips from // any layers with overflow. The condition for being able to apply these clips is that the overflow object be in our // containing block chain so we check that also. @@ -3681,17 +3690,12 @@ void RenderLayer::clipToRect(const LayerPaintingInfo& paintingInfo, GraphicsCont if (layer == paintingInfo.rootLayer) break; } - } else if (clipRect.rect() != paintingInfo.paintDirtyRect) { - context->save(); - LayoutRect adjustedClipRect = clipRect.rect(); - adjustedClipRect.move(paintingInfo.subpixelAccumulation); - context->clip(snapRectToDevicePixels(adjustedClipRect, deviceScaleFactor)); } } void RenderLayer::restoreClip(GraphicsContext* context, const LayoutRect& paintDirtyRect, const ClipRect& clipRect) { - if (clipRect.rect() != paintDirtyRect || clipRect.hasRadius()) + if (clipRect.rect() != paintDirtyRect || clipRect.affectedByRadius()) context->restore(); } @@ -4397,10 +4401,10 @@ void RenderLayer::collectFragments(LayerFragments& fragments, const RenderLayer* fragment.intersect(ancestorClipRect.rect()); // If the ancestor clip rect has border-radius, make sure to apply it to the fragments. - if (ancestorClipRect.hasRadius()) { - fragment.foregroundRect.setHasRadius(true); - fragment.backgroundRect.setHasRadius(true); - fragment.outlineRect.setHasRadius(true); + if (ancestorClipRect.affectedByRadius()) { + fragment.foregroundRect.setAffectedByRadius(true); + fragment.backgroundRect.setAffectedByRadius(true); + fragment.outlineRect.setAffectedByRadius(true); } // Now intersect with our pagination clip. This will typically mean we're just intersecting the dirty rect with the column @@ -5304,7 +5308,7 @@ void RenderLayer::calculateClipRects(const ClipRectsContext& clipRectsContext, C if (renderer().hasOverflowClip()) { ClipRect newOverflowClip = downcast(renderer()).overflowClipRectForChildLayers(offset, currentRenderNamedFlowFragment(), clipRectsContext.overlayScrollbarSizeRelevancy); - newOverflowClip.setHasRadius(renderer().style().hasBorderRadius()); + newOverflowClip.setAffectedByRadius(renderer().style().hasBorderRadius()); clipRects.setOverflowClipRect(intersection(newOverflowClip, clipRects.overflowClipRect())); if (renderer().isPositioned()) clipRects.setPosClipRect(intersection(newOverflowClip, clipRects.posClipRect())); @@ -5429,7 +5433,7 @@ void RenderLayer::calculateRects(const ClipRectsContext& clipRectsContext, const if (renderer().hasOverflowClip() && (this != clipRectsContext.rootLayer || clipRectsContext.respectOverflowClip == RespectOverflowClip)) { foregroundRect.intersect(downcast(renderer()).overflowClipRect(toLayoutPoint(offsetFromRootLocal), namedFlowFragment, clipRectsContext.overlayScrollbarSizeRelevancy)); if (renderer().style().hasBorderRadius()) - foregroundRect.setHasRadius(true); + foregroundRect.setAffectedByRadius(true); } if (renderer().hasClip()) { diff --git a/Source/WebCore/rendering/RenderLayer.h b/Source/WebCore/rendering/RenderLayer.h index 9642d84..9e71d2d 100644 --- a/Source/WebCore/rendering/RenderLayer.h +++ b/Source/WebCore/rendering/RenderLayer.h @@ -84,30 +84,32 @@ enum RepaintStatus { class ClipRect { public: ClipRect() - : m_hasRadius(false) - { } + : m_affectedByRadius(false) + { + } ClipRect(const LayoutRect& rect) - : m_rect(rect) - , m_hasRadius(false) - { } + : m_rect(rect) + , m_affectedByRadius(false) + { + } const LayoutRect& rect() const { return m_rect; } void setRect(const LayoutRect& rect) { m_rect = rect; } - bool hasRadius() const { return m_hasRadius; } - void setHasRadius(bool hasRadius) { m_hasRadius = hasRadius; } + bool affectedByRadius() const { return m_affectedByRadius; } + void setAffectedByRadius(bool affectedByRadius) { m_affectedByRadius = affectedByRadius; } - bool operator==(const ClipRect& other) const { return rect() == other.rect() && hasRadius() == other.hasRadius(); } - bool operator!=(const ClipRect& other) const { return rect() != other.rect() || hasRadius() != other.hasRadius(); } + bool operator==(const ClipRect& other) const { return rect() == other.rect() && affectedByRadius() == other.affectedByRadius(); } + bool operator!=(const ClipRect& other) const { return rect() != other.rect() || affectedByRadius() != other.affectedByRadius(); } bool operator!=(const LayoutRect& otherRect) const { return rect() != otherRect; } void intersect(const LayoutRect& other) { m_rect.intersect(other); } void intersect(const ClipRect& other) { m_rect.intersect(other.rect()); - if (other.hasRadius()) - m_hasRadius = true; + if (other.affectedByRadius()) + m_affectedByRadius = true; } void move(LayoutUnit x, LayoutUnit y) { m_rect.move(x, y); } void move(const LayoutSize& size) { m_rect.move(size); } @@ -123,7 +125,7 @@ public: private: LayoutRect m_rect; - bool m_hasRadius; + bool m_affectedByRadius; }; inline ClipRect intersection(const ClipRect& a, const ClipRect& b) -- 1.8.3.1