<feImage> has problems referencing local elements

https://bugs.webkit.org/show_bug.cgi?id=76800

Reviewed by Zoltan Herczeg.

Source/WebCore:

<feImage> referencing local elements are currently rendered into an ImageBuffer
by SVGFEImageElement, using the local coordinates of the referenced renderer.

This approach is buggy and should be avoided, by moving the rendering fully
into SVGFEImage, which takes care of respecting the correct transformations.

This fixes <feImage> + local references, which currently breaks two tests in trunk.
Covered by existing tests.

* rendering/svg/RenderSVGResourceFilterPrimitive.cpp:
(WebCore::RenderSVGResourceFilterPrimitive::determineFilterPrimitiveSubregion):
* svg/SVGFEImageElement.cpp:
(WebCore::SVGFEImageElement::build):
* svg/graphics/filters/SVGFEImage.cpp:
(WebCore::FEImage::FEImage):
(WebCore::FEImage::createWithImage):
(WebCore::FEImage::createWithIRIReference):
(WebCore::FEImage::determineAbsolutePaintRect):
(WebCore::FEImage::referencedRenderer):
(WebCore::FEImage::platformApplySoftware):
(WebCore::FEImage::externalRepresentation):
* svg/graphics/filters/SVGFEImage.h:
(WebCore::FEImage::~FEImage):
* svg/graphics/filters/SVGFilter.h:
(WebCore::SVGFilter::absoluteTransform):

LayoutTests:

Update svg/filters/feImage-reference-* results, which are fixed now.

* platform/chromium/test_expectations.txt : Updated.
* platform/mac/svg/W3C-SVG-1.1/filters-composite-02-b-expected.png: Update for marginal change.
* svg/filters/feImage-reference-invalidation-expected.png:
* svg/filters/feImage-reference-svg-primitive-expected.png:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105612 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 23df13d..b4e3d88 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2012-01-23  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        <feImage> has problems referencing local elements
+        https://bugs.webkit.org/show_bug.cgi?id=76800
+
+        Reviewed by Zoltan Herczeg.
+
+        Update svg/filters/feImage-reference-* results, which are fixed now.
+
+        * platform/chromium/test_expectations.txt : Updated.
+        * platform/mac/svg/W3C-SVG-1.1/filters-composite-02-b-expected.png: Update for marginal change.
+        * svg/filters/feImage-reference-invalidation-expected.png:
+        * svg/filters/feImage-reference-svg-primitive-expected.png:
+
 2012-01-23  Csaba Osztrogonác  <ossy@webkit.org>
 
         WebKit fails IETC composition event types

diff --git a/LayoutTests/platform/chromium/test_expectations.txt b/LayoutTests/platform/chromium/test_expectations.txt
index 6875d1d..e737985 100644 (file)
--- a/LayoutTests/platform/chromium/test_expectations.txt
+++ b/LayoutTests/platform/chromium/test_expectations.txt
@@ -3923,6 +3923,11 @@ BUGWK76647 : svg/zoom/page/zoom-background-images.html = IMAGE
 // Just needs a rebaseline.
 BUGWK76780 : svg/filters/feImage-preserveAspectratio.svg = IMAGE+TEXT
 
+// Just needs a rebaseline.
+BUGWK76800 : svg/W3C-SVG-1.1/filters-composite-02-b.svg = IMAGE
+BUGWK76800 : svg/filters/feImage-reference-invalidation.svg = IMAGE
+BUGWK76800 : svg/filters/feImage-reference-svg-primitive.svg = IMAGE
+
 // Change error (misspelling) underlines from Windows look to Mac look.
 BUG_CARYCLARK MAC : editing/deleting/delete-3928305-fix.html = IMAGE
 BUG_CARYCLARK MAC : editing/deleting/delete-3959464-fix.html = IMAGE

diff --git a/LayoutTests/platform/mac/svg/W3C-SVG-1.1/filters-composite-02-b-expected.png b/LayoutTests/platform/mac/svg/W3C-SVG-1.1/filters-composite-02-b-expected.png
index e18f5bd..8048156 100644 (file)
Binary files a/LayoutTests/platform/mac/svg/W3C-SVG-1.1/filters-composite-02-b-expected.png and b/LayoutTests/platform/mac/svg/W3C-SVG-1.1/filters-composite-02-b-expected.png differ

diff --git a/LayoutTests/svg/filters/feImage-reference-invalidation-expected.png b/LayoutTests/svg/filters/feImage-reference-invalidation-expected.png
index bdb6ba2..c319b37 100644 (file)
Binary files a/LayoutTests/svg/filters/feImage-reference-invalidation-expected.png and b/LayoutTests/svg/filters/feImage-reference-invalidation-expected.png differ

diff --git a/LayoutTests/svg/filters/feImage-reference-svg-primitive-expected.png b/LayoutTests/svg/filters/feImage-reference-svg-primitive-expected.png
index b569597..7e19e76 100644 (file)
Binary files a/LayoutTests/svg/filters/feImage-reference-svg-primitive-expected.png and b/LayoutTests/svg/filters/feImage-reference-svg-primitive-expected.png differ

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index fc69bab..c840b38 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,38 @@
 2012-01-23  Nikolas Zimmermann  <nzimmermann@rim.com>
 
+        <feImage> has problems referencing local elements
+        https://bugs.webkit.org/show_bug.cgi?id=76800
+
+        Reviewed by Zoltan Herczeg.
+
+        <feImage> referencing local elements are currently rendered into an ImageBuffer
+        by SVGFEImageElement, using the local coordinates of the referenced renderer.
+
+        This approach is buggy and should be avoided, by moving the rendering fully
+        into SVGFEImage, which takes care of respecting the correct transformations.
+
+        This fixes <feImage> + local references, which currently breaks two tests in trunk.
+        Covered by existing tests.
+
+        * rendering/svg/RenderSVGResourceFilterPrimitive.cpp:
+        (WebCore::RenderSVGResourceFilterPrimitive::determineFilterPrimitiveSubregion):
+        * svg/SVGFEImageElement.cpp:
+        (WebCore::SVGFEImageElement::build):
+        * svg/graphics/filters/SVGFEImage.cpp:
+        (WebCore::FEImage::FEImage):
+        (WebCore::FEImage::createWithImage):
+        (WebCore::FEImage::createWithIRIReference):
+        (WebCore::FEImage::determineAbsolutePaintRect):
+        (WebCore::FEImage::referencedRenderer):
+        (WebCore::FEImage::platformApplySoftware):
+        (WebCore::FEImage::externalRepresentation):
+        * svg/graphics/filters/SVGFEImage.h:
+        (WebCore::FEImage::~FEImage):
+        * svg/graphics/filters/SVGFilter.h:
+        (WebCore::SVGFilter::absoluteTransform):
+
+2012-01-23  Nikolas Zimmermann  <nzimmermann@rim.com>
+
         Not reviewed. Fix Mac build, by exporting a new symbol.
 
         * WebCore.exp.in:

diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceFilterPrimitive.cpp b/Source/WebCore/rendering/svg/RenderSVGResourceFilterPrimitive.cpp
index 085e650..183140b 100644 (file)
--- a/Source/WebCore/rendering/svg/RenderSVGResourceFilterPrimitive.cpp
+++ b/Source/WebCore/rendering/svg/RenderSVGResourceFilterPrimitive.cpp
@@ -91,7 +91,7 @@ FloatRect RenderSVGResourceFilterPrimitive::determineFilterPrimitiveSubregion(Fi
         subregion.setHeight(uniteRect.height());
     effect->setFilterPrimitiveSubregion(subregion);
 
-    FloatRect absoluteSubregion = filter->mapLocalRectToAbsoluteRect(subregion);
+    FloatRect absoluteSubregion = filter->absoluteTransform().mapRect(subregion);
     FloatSize filterResolution = filter->filterResolution();
     absoluteSubregion.scale(filterResolution.width(), filterResolution.height());
 

diff --git a/Source/WebCore/svg/SVGFEImageElement.cpp b/Source/WebCore/svg/SVGFEImageElement.cpp
index 0c833c0..d64c232 100644 (file)
--- a/Source/WebCore/svg/SVGFEImageElement.cpp
+++ b/Source/WebCore/svg/SVGFEImageElement.cpp
@@ -33,7 +33,6 @@
 #include "RenderObject.h"
 #include "RenderSVGResource.h"
 #include "SVGElementInstance.h"
-#include "SVGImageBufferTools.h"
 #include "SVGNames.h"
 #include "SVGPreserveAspectRatio.h"
 
@@ -183,27 +182,9 @@ PassRefPtr<FilterEffect> SVGFEImageElement::build(SVGFilterBuilder*, Filter* fil
     if (!m_cachedImage && !m_targetImage)
         requestImageResource();
 
-    if (!m_cachedImage && !m_targetImage) {
-        Element* hrefElement = SVGURIReference::targetElementFromIRIString(href(), document());
-        if (!hrefElement || !hrefElement->isSVGElement())
-            return 0;
-
-        RenderObject* renderer = hrefElement->renderer();
-        if (!renderer)
-            return 0;
-
-        IntRect targetRect = enclosingIntRect(renderer->objectBoundingBox());
-
-        if (targetRect.isEmpty())
-            return 0;
-
-        m_targetImage = ImageBuffer::create(targetRect.size(), ColorSpaceLinearRGB);
-
-        AffineTransform contentTransformation;
-        SVGImageBufferTools::renderSubtreeToImageBuffer(m_targetImage.get(), renderer, contentTransformation);
-    }
-
-    return FEImage::create(filter, m_targetImage ? m_targetImage->copyImage(CopyBackingStore) : m_cachedImage->imageForRenderer(renderer()), preserveAspectRatio());
+    if (!m_cachedImage && !m_targetImage)
+        return FEImage::createWithIRIReference(filter, document(), href(), preserveAspectRatio());
+    return FEImage::createWithImage(filter, m_cachedImage->imageForRenderer(renderer()), preserveAspectRatio());
 }
 
 void SVGFEImageElement::addSubresourceAttributeURLs(ListHashSet<KURL>& urls) const

diff --git a/Source/WebCore/svg/graphics/filters/SVGFEImage.cpp b/Source/WebCore/svg/graphics/filters/SVGFEImage.cpp
index df68a6b..fa86cd0 100644 (file)
--- a/Source/WebCore/svg/graphics/filters/SVGFEImage.cpp
+++ b/Source/WebCore/svg/graphics/filters/SVGFEImage.cpp
@@ -28,8 +28,12 @@
 #include "AffineTransform.h"
 #include "Filter.h"
 #include "GraphicsContext.h"
+#include "RenderObject.h"
 #include "RenderTreeAsText.h"
+#include "SVGFilter.h"
+#include "SVGImageBufferTools.h"
 #include "SVGPreserveAspectRatio.h"
+#include "SVGURIReference.h"
 #include "TextStream.h"
 
 namespace WebCore {
@@ -37,19 +41,37 @@ namespace WebCore {
 FEImage::FEImage(Filter* filter, PassRefPtr<Image> image, const SVGPreserveAspectRatio& preserveAspectRatio)
     : FilterEffect(filter)
     , m_image(image)
+    , m_document(0)
     , m_preserveAspectRatio(preserveAspectRatio)
 {
 }
 
-PassRefPtr<FEImage> FEImage::create(Filter* filter, PassRefPtr<Image> image, const SVGPreserveAspectRatio& preserveAspectRatio)
+FEImage::FEImage(Filter* filter, Document* document, const String& href, const SVGPreserveAspectRatio& preserveAspectRatio)
+    : FilterEffect(filter)
+    , m_document(document)
+    , m_href(href)
+    , m_preserveAspectRatio(preserveAspectRatio)
+{
+}
+
+PassRefPtr<FEImage> FEImage::createWithImage(Filter* filter, PassRefPtr<Image> image, const SVGPreserveAspectRatio& preserveAspectRatio)
 {
     return adoptRef(new FEImage(filter, image, preserveAspectRatio));
 }
 
+PassRefPtr<FEImage> FEImage::createWithIRIReference(Filter* filter, Document* document, const String& href, const SVGPreserveAspectRatio& preserveAspectRatio)
+{
+    return adoptRef(new FEImage(filter, document, href, preserveAspectRatio));
+}
+
 void FEImage::determineAbsolutePaintRect()
 {
-    ASSERT(m_image);
-    FloatRect srcRect(FloatPoint(), m_image->size());
+    FloatRect srcRect;
+    if (m_image)
+        srcRect.setSize(m_image->size());
+    else if (RenderObject* renderer = referencedRenderer())
+        srcRect = static_cast<SVGFilter*>(filter())->absoluteTransform().mapRect(renderer->repaintRectInLocalCoordinates());
+
     FloatRect paintRect(m_absoluteSubregion);
     m_preserveAspectRatio.transformRect(paintRect, srcRect);
     if (clipsToBounds())
@@ -59,15 +81,37 @@ void FEImage::determineAbsolutePaintRect()
     setAbsolutePaintRect(enclosingIntRect(paintRect));
 }
 
+RenderObject* FEImage::referencedRenderer() const
+{
+    if (!m_document)
+        return 0;
+    Element* hrefElement = SVGURIReference::targetElementFromIRIString(m_href, m_document);
+    if (!hrefElement || !hrefElement->isSVGElement())
+        return 0;
+    return hrefElement->renderer();
+}
+
 void FEImage::platformApplySoftware()
 {
-    if (!m_image.get())
+    RenderObject* renderer = referencedRenderer();
+    if (!m_image && !renderer)
         return;
 
     ImageBuffer* resultImage = createImageBufferResult();
     if (!resultImage)
         return;
 
+    if (renderer) {
+        const AffineTransform& absoluteTransform = static_cast<SVGFilter*>(filter())->absoluteTransform();
+        resultImage->context()->concatCTM(absoluteTransform);
+
+        AffineTransform contentTransformation;
+        SVGImageBufferTools::renderSubtreeToImageBuffer(resultImage, renderer, contentTransformation);
+
+        resultImage->context()->concatCTM(absoluteTransform.inverse());
+        return;
+    }
+
     FloatRect srcRect(FloatPoint(), m_image->size());
     FloatRect destRect(m_absoluteSubregion);
     m_preserveAspectRatio.transformRect(destRect, srcRect);
@@ -84,8 +128,11 @@ void FEImage::dump()
 
 TextStream& FEImage::externalRepresentation(TextStream& ts, int indent) const
 {
-    ASSERT(m_image);
-    IntSize imageSize = m_image->size();
+    IntSize imageSize;
+    if (m_image)
+        imageSize = m_image->size();
+    else if (RenderObject* renderer = referencedRenderer())
+        imageSize = enclosingIntRect(renderer->repaintRectInLocalCoordinates()).size();
     writeIndent(ts, indent);
     ts << "[feImage";
     FilterEffect::externalRepresentation(ts);

diff --git a/Source/WebCore/svg/graphics/filters/SVGFEImage.h b/Source/WebCore/svg/graphics/filters/SVGFEImage.h
index 2bd1525..53ef0e1 100644 (file)
--- a/Source/WebCore/svg/graphics/filters/SVGFEImage.h
+++ b/Source/WebCore/svg/graphics/filters/SVGFEImage.h
@@ -29,11 +29,14 @@
 
 namespace WebCore {
 
+class Document;
 class Image;
+class RenderObject;
 
 class FEImage : public FilterEffect {
 public:
-    static PassRefPtr<FEImage> create(Filter*, PassRefPtr<Image>, const SVGPreserveAspectRatio&);
+    static PassRefPtr<FEImage> createWithImage(Filter*, PassRefPtr<Image>, const SVGPreserveAspectRatio&);
+    static PassRefPtr<FEImage> createWithIRIReference(Filter*, Document*, const String&, const SVGPreserveAspectRatio&);
 
     void setAbsoluteSubregion(const FloatRect& absoluteSubregion) { m_absoluteSubregion = absoluteSubregion; }
 
@@ -47,9 +50,14 @@ public:
     virtual TextStream& externalRepresentation(TextStream&, int indention) const;
     
 private:
+    virtual ~FEImage() { }
     FEImage(Filter*, PassRefPtr<Image>, const SVGPreserveAspectRatio&);
+    FEImage(Filter*, Document*, const String&, const SVGPreserveAspectRatio&);
+    RenderObject* referencedRenderer() const;
 
     RefPtr<Image> m_image;
+    Document* m_document;
+    String m_href;
     SVGPreserveAspectRatio m_preserveAspectRatio;
     FloatRect m_absoluteSubregion;
 };

diff --git a/Source/WebCore/svg/graphics/filters/SVGFilter.h b/Source/WebCore/svg/graphics/filters/SVGFilter.h
index 84f2843..c88079b 100644 (file)
--- a/Source/WebCore/svg/graphics/filters/SVGFilter.h
+++ b/Source/WebCore/svg/graphics/filters/SVGFilter.h
@@ -41,7 +41,7 @@ public:
     virtual FloatRect filterRegion() const { return m_absoluteFilterRegion; }
 
     virtual FloatPoint mapAbsolutePointToLocalPoint(const FloatPoint& point) const { return m_absoluteTransform.inverse().mapPoint(point); }
-    FloatRect mapLocalRectToAbsoluteRect(const FloatRect& rect) const { return m_absoluteTransform.mapRect(rect); }
+    const AffineTransform& absoluteTransform() const { return m_absoluteTransform; }
 
     virtual float applyHorizontalScale(float value) const;
     virtual float applyVerticalScale(float value) const;