Bug 71930

<rdar://problem/10420021>

Created attachment 119471 [details] Patch

Comment on attachment 119471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119471&action=review > Source/WebCore/rendering/FilterEffectRenderer.h:74 > + IntRect outputRect() { return lastEffect()->hasResult() ? lastEffect()->requestedRegionOfInputImageData(IntRect(m_filterRegion)) : IntRect(); } Should be const. > Source/WebCore/rendering/RenderBox.cpp:3516 > void RenderBox::addBoxShadowAndBorderOverflow() The method name is wrong after this change. Maybe it should be renamed to addBoxDecorationOverflow, addVisualEffectOverflow or something? > Source/WebCore/rendering/RenderLayerBacking.cpp:741 > static bool hasBorderOutlineOrShadow(const RenderStyle* style) Method name is wrong now. > Source/WebCore/rendering/style/FilterOperations.cpp:35 > +static inline void outsetSizeForBlur(unsigned& outsetX, unsigned& outsetY, float stdX, float stdY) Can't this return an IntSize? > Source/WebCore/rendering/style/FilterOperations.cpp:68 > + if (operationType == FilterOperation::BLUR || operationType == FilterOperation::DROP_SHADOW) Is it worth checking for non-zero blur radius/shadow offset? > Source/WebCore/rendering/style/RenderStyle.h:406 > + if (hasFilter()) > + filter().getOutsets(top, right, bottom, left, borderBoxSize); I think you should set the parameters to zero if there is no filter. > LayoutTests/ChangeLog:4 > + Filters need to affect visual overflow > + https://bugs.webkit.org/show_bug.cgi?id=71930 You should probably add some margins between the elements in these filter tests to avoid adjacent blurs overlapping.

(In reply to comment #3) > (From update of attachment 119471 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=119471&action=review > > > Source/WebCore/rendering/FilterEffectRenderer.h:74 > > + IntRect outputRect() { return lastEffect()->hasResult() ? lastEffect()->requestedRegionOfInputImageData(IntRect(m_filterRegion)) : IntRect(); } > > Should be const. fixed > > > Source/WebCore/rendering/RenderBox.cpp:3516 > > void RenderBox::addBoxShadowAndBorderOverflow() > > The method name is wrong after this change. Maybe it should be renamed to addBoxDecorationOverflow, addVisualEffectOverflow or something? addVisualEffectOverflow it is > > > Source/WebCore/rendering/RenderLayerBacking.cpp:741 > > static bool hasBorderOutlineOrShadow(const RenderStyle* style) > > Method name is wrong now. hasBoxDecorations (matches existing functions nicely) > > > Source/WebCore/rendering/style/FilterOperations.cpp:35 > > +static inline void outsetSizeForBlur(unsigned& outsetX, unsigned& outsetY, float stdX, float stdY) > > Can't this return an IntSize? fixed > > > Source/WebCore/rendering/style/FilterOperations.cpp:68 > > + if (operationType == FilterOperation::BLUR || operationType == FilterOperation::DROP_SHADOW) > > Is it worth checking for non-zero blur radius/shadow offset? I don't think so. That would require casting to a real operation, etc. Although now that I've committed, I do see that people will use a blur(0) as a transition starting point to be a common case. I'll file a followup. > > > Source/WebCore/rendering/style/RenderStyle.h:406 > > + if (hasFilter()) > > + filter().getOutsets(top, right, bottom, left, borderBoxSize); > > I think you should set the parameters to zero if there is no filter. fixed > > > LayoutTests/ChangeLog:4 > > + Filters need to affect visual overflow > > + https://bugs.webkit.org/show_bug.cgi?id=71930 > > You should probably add some margins between the elements in these filter tests to avoid adjacent blurs overlapping. fixed