[webkit-reviews] review denied: [Bug 26380] SVG Filters and big sized filterRegions : [Attachment 42876] clip filter to viewport

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 10 10:58:49 PST 2009


Simon Fraser (smfr) <simon.fraser at apple.com> has denied Dirk Schulze
<krit at webkit.org>'s request for review:
Bug 26380: SVG Filters and big sized filterRegions
https://bugs.webkit.org/show_bug.cgi?id=26380

Attachment 42876: clip filter to viewport
https://bugs.webkit.org/attachment.cgi?id=42876&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>

> Index: WebCore/svg/SVGFilterElement.cpp
> ===================================================================

> @@ -125,6 +127,7 @@ void SVGFilterElement::buildFilter(const
>				  filterRect.width() * targetRect.width(),
>				  filterRect.height() * targetRect.height());
>  
> +    m_filter->setViewPort(document()->view()->visibleContentRect());

But is this rect in the same coordinate system as the rest of the filter rects?
What if the filter is on a child of a transformed element?

> Index: WebCore/svg/graphics/SVGResourceFilter.cpp
> ===================================================================
> --- WebCore/svg/graphics/SVGResourceFilter.cpp	(revision 50732)
> +++ WebCore/svg/graphics/SVGResourceFilter.cpp	(working copy)
> @@ -63,6 +63,9 @@ void SVGResourceFilter::prepareFilter(Gr
>      FloatRect targetRect = object->objectBoundingBox();
>      m_ownerElement->buildFilter(targetRect);
>  
> +    // clip filterRect to viewPort
> +    m_filterBBox.intersect(m_viewPort);

This isn't going to do the right thing for filtered SVG inside a HTML with CSS
3D transforms; element content that is outside the viewport can be made visible
with a rotateY(), for example. And, yes the visible filtered content may be
arbitrarily large in that case.

> Index: WebCore/svg/graphics/SVGResourceFilter.h
> ===================================================================
> --- WebCore/svg/graphics/SVGResourceFilter.h	(revision 50732)
> +++ WebCore/svg/graphics/SVGResourceFilter.h	(working copy)
> @@ -65,6 +65,9 @@ public:
>      FloatRect filterBoundingBox() { return m_filterBBox; }
>      void setFilterBoundingBox(const FloatRect& rect) { m_filterBBox = rect;
}
>  
> +    IntRect viewPort() { return m_viewPort; }
> +    void setViewPort(const IntRect& rect) { m_viewPort = rect; }

I question the need for these methods, but if you do have them, then viewPort()
should be const.


More information about the webkit-reviews mailing list