From cb51fa63aceecc8ebdc3163d5c208e50819f97c8 Mon Sep 17 00:00:00 2001 From: Said Abou-Hallawa Date: Sun, 1 Mar 2015 08:24:55 +0000 Subject: [PATCH] Merge r180643 - Horizontal and vertical lines are clipped completely if clip-path is included in the tag but the referenced element is defined later. https://bugs.webkit.org/show_bug.cgi?id=141776. Reviewed by Dean Jackson. Source/WebCore: Tests: svg/clip-path/clip-path-line-use-before-defined-expected.svg svg/clip-path/clip-path-line-use-before-defined.svg * rendering/svg/RenderSVGResourceClipper.cpp: (WebCore::RenderSVGResourceClipper::applyClippingToContext): Ensure the renderer is added to m_clipper if it does not exist. The same renderer might have been added to m_clipper in resourceBoundingBox(). (WebCore::RenderSVGResourceClipper::addRendererToClipper): Add the renderer to m_clipper if it does not exist. Return the associated ClipperData. (WebCore::RenderSVGResourceClipper::resourceBoundingBox): If the clipper is referenced before it is defined, add the renderer to m_clipper. While doing the layout() for the clipper, we can check if m_clipper has values or not. If it does have, we are going to mark the clipper for client invalidation which is done by the SVG root. * rendering/svg/RenderSVGResourceClipper.h: * rendering/svg/RenderSVGResourceContainer.h: (WebCore::RenderSVGResourceContainer::selfNeedsClientInvalidation): Define a new function selfNeedsClientInvalidation() which controls marking the clipper for client invalidation. In RenderSVGResourceClipper, override it so it checks m_clipper to force clients validation even if it the first time we do layout for this clipper. * rendering/svg/RenderSVGResourceContainer.cpp: (WebCore::RenderSVGResourceContainer::layout): Call the virtual function selfNeedsClientInvalidation() to check whether we need to mark the clipper for client invalidation. * svg/SVGElement.cpp: Delete unneeded header file. LayoutTests: New test cases for SVG lines which are clipped to a . The is referenced before it is defined. * svg/clip-path/clip-path-line-use-before-defined-expected.svg: Added. * svg/clip-path/clip-path-line-use-before-defined.svg: Added. --- LayoutTests/ChangeLog | 24 +++++++ ...-path-line-use-before-defined-expected.svg | 44 +++++++++++++ .../clip-path-line-use-before-defined.svg | 46 ++++++++++++++ Source/WebCore/ChangeLog | 62 +++++++++++++++++++ .../svg/RenderSVGResourceClipper.cpp | 49 ++++++++------- .../rendering/svg/RenderSVGResourceClipper.h | 14 ++--- .../svg/RenderSVGResourceContainer.cpp | 2 +- .../svg/RenderSVGResourceContainer.h | 2 + Source/WebCore/svg/SVGElement.cpp | 1 - 9 files changed, 211 insertions(+), 33 deletions(-) create mode 100644 LayoutTests/svg/clip-path/clip-path-line-use-before-defined-expected.svg create mode 100644 LayoutTests/svg/clip-path/clip-path-line-use-before-defined.svg diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 1169bd1384d5..73234e747fe8 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,27 @@ +2015-02-26 Said Abou-Hallawa + + Cleanup RenderSVGResourceClipper class. + https://bugs.webkit.org/show_bug.cgi?id=142032. + + Reviewed by Darin Adler. + + * svg/clip-path/clip-path-line-use-before-defined-expected.svg: + * svg/clip-path/clip-path-line-use-before-defined.svg: Simplify the test + and make separate drawings for different cases. + +2015-02-25 Said Abou-Hallawa + + Horizontal and vertical lines are clipped completely if clip-path is included in the tag but the referenced element is defined later. + https://bugs.webkit.org/show_bug.cgi?id=141776. + + Reviewed by Dean Jackson. + + New test cases for SVG lines which are clipped to a . The + is referenced before it is defined. + + * svg/clip-path/clip-path-line-use-before-defined-expected.svg: Added. + * svg/clip-path/clip-path-line-use-before-defined.svg: Added. + 2015-02-25 Joanmarie Diggs AX: Implement support for ARIA 1.1 'searchbox' role diff --git a/LayoutTests/svg/clip-path/clip-path-line-use-before-defined-expected.svg b/LayoutTests/svg/clip-path/clip-path-line-use-before-defined-expected.svg new file mode 100644 index 000000000000..84f6a5e33998 --- /dev/null +++ b/LayoutTests/svg/clip-path/clip-path-line-use-before-defined-expected.svg @@ -0,0 +1,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/LayoutTests/svg/clip-path/clip-path-line-use-before-defined.svg b/LayoutTests/svg/clip-path/clip-path-line-use-before-defined.svg new file mode 100644 index 000000000000..dd203c455a81 --- /dev/null +++ b/LayoutTests/svg/clip-path/clip-path-line-use-before-defined.svg @@ -0,0 +1,46 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 65cd240bdf44..470dd32a8589 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,65 @@ +2015-02-26 Said Abou-Hallawa + + Cleanup RenderSVGResourceClipper class. + https://bugs.webkit.org/show_bug.cgi?id=142032. + + Reviewed by Darin Adler. + + This is a follow up for r180643: . + It includes cleanup for RenderSVGResourceClipper class. + + * rendering/svg/RenderSVGResourceClipper.cpp: + (WebCore::RenderSVGResourceClipper::applyClippingToContext): + (WebCore::RenderSVGResourceClipper::drawContentIntoMaskImage): + * rendering/svg/RenderSVGResourceClipper.h: Change ClipperData to be a + typedef instead of a class and rename it to ClipperMaskImage. The purpose + of having it a class even though it includes only one member was because + we wanted it to be WTF_MAKE_FAST_ALLOCATED. We do not need to allocate it + as a separate object on the heap anymore. + + (WebCore::RenderSVGResourceClipper::addRendererToClipper): Instead of doing + double hash table lookups by calling HashMap::contains() and then HashMap::get(), + we can use HashMap::add() instead. + +2015-02-25 Said Abou-Hallawa + + Horizontal and vertical lines are clipped completely if clip-path is included in the tag but the referenced element is defined later. + https://bugs.webkit.org/show_bug.cgi?id=141776. + + Reviewed by Dean Jackson. + + Tests: svg/clip-path/clip-path-line-use-before-defined-expected.svg + svg/clip-path/clip-path-line-use-before-defined.svg + + * rendering/svg/RenderSVGResourceClipper.cpp: + (WebCore::RenderSVGResourceClipper::applyClippingToContext): Ensure the renderer + is added to m_clipper if it does not exist. The same renderer might have been + added to m_clipper in resourceBoundingBox(). + + (WebCore::RenderSVGResourceClipper::addRendererToClipper): Add the renderer to + m_clipper if it does not exist. Return the associated ClipperData. + + (WebCore::RenderSVGResourceClipper::resourceBoundingBox): If the clipper is + referenced before it is defined, add the renderer to m_clipper. While doing the + layout() for the clipper, we can check if m_clipper has values or not. If it does + have, we are going to mark the clipper for client invalidation which is done by + the SVG root. + + * rendering/svg/RenderSVGResourceClipper.h: + * rendering/svg/RenderSVGResourceContainer.h: + (WebCore::RenderSVGResourceContainer::selfNeedsClientInvalidation): Define a + new function selfNeedsClientInvalidation() which controls marking the clipper + for client invalidation. In RenderSVGResourceClipper, override it so it checks + m_clipper to force clients validation even if it the first time we do layout + for this clipper. + + * rendering/svg/RenderSVGResourceContainer.cpp: + (WebCore::RenderSVGResourceContainer::layout): Call the virtual function + selfNeedsClientInvalidation() to check whether we need to mark the clipper for + client invalidation. + + * svg/SVGElement.cpp: Delete unneeded header file. + 2015-02-25 Beth Dakin REGRESSION (r180018 ): Holding a rubber-band in place can get stuck diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp b/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp index 03dd5d0b1cea..d170f2889f75 100644 --- a/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp +++ b/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp @@ -130,27 +130,22 @@ bool RenderSVGResourceClipper::pathOnlyClipping(GraphicsContext* context, const bool RenderSVGResourceClipper::applyClippingToContext(RenderElement& renderer, const FloatRect& objectBoundingBox, const FloatRect& repaintRect, GraphicsContext* context) { - bool missingClipperData = !m_clipper.contains(&renderer); - if (missingClipperData) - m_clipper.set(&renderer, std::make_unique()); + ClipperMaskImage& clipperMaskImage = addRendererToClipper(renderer); + bool shouldCreateClipperMaskImage = !clipperMaskImage; - bool shouldCreateClipData = false; AffineTransform animatedLocalTransform = clipPathElement().animatedLocalTransform(); - ClipperData* clipperData = m_clipper.get(&renderer); - if (!clipperData->clipMaskImage) { - if (pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox)) - return true; - shouldCreateClipData = true; - } + + if (shouldCreateClipperMaskImage && pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox)) + return true; AffineTransform absoluteTransform; SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(renderer, absoluteTransform); - if (shouldCreateClipData && !repaintRect.isEmpty()) { - if (!SVGRenderingContext::createImageBuffer(repaintRect, absoluteTransform, clipperData->clipMaskImage, ColorSpaceDeviceRGB, Unaccelerated)) + if (shouldCreateClipperMaskImage && !repaintRect.isEmpty()) { + if (!SVGRenderingContext::createImageBuffer(repaintRect, absoluteTransform, clipperMaskImage, ColorSpaceDeviceRGB, Unaccelerated)) return false; - GraphicsContext* maskContext = clipperData->clipMaskImage->context(); + GraphicsContext* maskContext = clipperMaskImage->context(); ASSERT(maskContext); maskContext->concatCTM(animatedLocalTransform); @@ -165,28 +160,27 @@ bool RenderSVGResourceClipper::applyClippingToContext(RenderElement& renderer, c if (!clipper->applyClippingToContext(*this, objectBoundingBox, repaintRect, maskContext)) return false; - succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox); + succeeded = drawContentIntoMaskImage(clipperMaskImage, objectBoundingBox); // The context restore applies the clipping on non-CG platforms. } else - succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox); + succeeded = drawContentIntoMaskImage(clipperMaskImage, objectBoundingBox); if (!succeeded) - clipperData->clipMaskImage.reset(); + clipperMaskImage.reset(); } - if (!clipperData->clipMaskImage) + if (!clipperMaskImage) return false; - SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage, missingClipperData); + SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperMaskImage, shouldCreateClipperMaskImage); return true; } -bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData, const FloatRect& objectBoundingBox) +bool RenderSVGResourceClipper::drawContentIntoMaskImage(const ClipperMaskImage& clipperMaskImage, const FloatRect& objectBoundingBox) { - ASSERT(clipperData); - ASSERT(clipperData->clipMaskImage); + ASSERT(clipperMaskImage); - GraphicsContext* maskContext = clipperData->clipMaskImage->context(); + GraphicsContext* maskContext = clipperMaskImage->context(); ASSERT(maskContext); AffineTransform maskContentTransformation; @@ -237,7 +231,7 @@ bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData // In the case of a element, we obtained its renderere above, to retrieve its clipRule. // We have to pass the renderer itself to renderSubtreeToImageBuffer() to apply it's x/y/transform/etc. values when rendering. // So if isUseElement is true, refetch the childNode->renderer(), as renderer got overriden above. - SVGRenderingContext::renderSubtreeToImageBuffer(clipperData->clipMaskImage.get(), isUseElement ? *child.renderer() : *renderer, maskContentTransformation); + SVGRenderingContext::renderSubtreeToImageBuffer(clipperMaskImage.get(), isUseElement ? *child.renderer() : *renderer, maskContentTransformation); } view().frameView().setPaintBehavior(oldBehavior); @@ -261,6 +255,11 @@ void RenderSVGResourceClipper::calculateClipContentRepaintRect() m_clipBoundaries = clipPathElement().animatedLocalTransform().mapRect(m_clipBoundaries); } +ClipperMaskImage& RenderSVGResourceClipper::addRendererToClipper(const RenderObject& object) +{ + return m_clipper.add(&object, ClipperMaskImage()).iterator->value; +} + bool RenderSVGResourceClipper::hitTestClipContent(const FloatRect& objectBoundingBox, const FloatPoint& nodeAtPoint) { FloatPoint point = nodeAtPoint; @@ -294,8 +293,10 @@ bool RenderSVGResourceClipper::hitTestClipContent(const FloatRect& objectBoundin FloatRect RenderSVGResourceClipper::resourceBoundingBox(const RenderObject& object) { // Resource was not layouted yet. Give back the boundingBox of the object. - if (selfNeedsLayout()) + if (selfNeedsLayout()) { + addRendererToClipper(object); return object.objectBoundingBox(); + } if (m_clipBoundaries.isEmpty()) calculateClipContentRepaintRect(); diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h b/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h index c0fd3a918ac6..4415b94b13a6 100644 --- a/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h +++ b/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h @@ -31,11 +31,7 @@ namespace WebCore { -struct ClipperData { - WTF_MAKE_FAST_ALLOCATED; -public: - std::unique_ptr clipMaskImage; -}; +typedef std::unique_ptr ClipperMaskImage; class RenderSVGResourceClipper final : public RenderSVGResourceContainer { public: @@ -60,17 +56,21 @@ class RenderSVGResourceClipper final : public RenderSVGResourceContainer { SVGUnitTypes::SVGUnitType clipPathUnits() const { return clipPathElement().clipPathUnits(); } +protected: + virtual bool selfNeedsClientInvalidation() const override { return (everHadLayout() || m_clipper.size()) && selfNeedsLayout(); } + private: void element() const = delete; virtual const char* renderName() const override { return "RenderSVGResourceClipper"; } bool pathOnlyClipping(GraphicsContext*, const AffineTransform&, const FloatRect&); - bool drawContentIntoMaskImage(ClipperData*, const FloatRect& objectBoundingBox); + bool drawContentIntoMaskImage(const ClipperMaskImage&, const FloatRect& objectBoundingBox); void calculateClipContentRepaintRect(); + ClipperMaskImage& addRendererToClipper(const RenderObject&); FloatRect m_clipBoundaries; - HashMap> m_clipper; + HashMap m_clipper; }; } diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp b/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp index 2bf2836ceff0..eea2ae3f32bb 100644 --- a/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp +++ b/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp @@ -52,7 +52,7 @@ void RenderSVGResourceContainer::layout() { StackStats::LayoutCheckPoint layoutCheckPoint; // Invalidate all resources if our layout changed. - if (everHadLayout() && selfNeedsLayout()) + if (selfNeedsClientInvalidation()) RenderSVGRoot::addResourceForClientInvalidation(this); RenderSVGHiddenContainer::layout(); diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h b/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h index 0baf1ea7087b..a03706e36318 100644 --- a/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h +++ b/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h @@ -55,6 +55,8 @@ class RenderSVGResourceContainer : public RenderSVGHiddenContainer, }; // Used from the invalidateClient/invalidateClients methods from classes, inheriting from us. + virtual bool selfNeedsClientInvalidation() const { return everHadLayout() && selfNeedsLayout(); } + void markAllClientsForInvalidation(InvalidationMode); void markAllClientLayersForInvalidation(); void markClientForInvalidation(RenderObject&, InvalidationMode); diff --git a/Source/WebCore/svg/SVGElement.cpp b/Source/WebCore/svg/SVGElement.cpp index dcd2a6970e76..9fbb4d1e43fe 100644 --- a/Source/WebCore/svg/SVGElement.cpp +++ b/Source/WebCore/svg/SVGElement.cpp @@ -39,7 +39,6 @@ #include "HTMLParserIdioms.h" #include "RenderObject.h" #include "RenderSVGResource.h" -#include "RenderSVGResourceClipper.h" #include "RenderSVGResourceFilter.h" #include "RenderSVGResourceMasker.h" #include "SVGCursorElement.h"