[Webkit-unassigned] [Bug 6021] WebKit+SVG does not support filterRes attribute

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Dirk Schulze <krit at webkit.org>  2009-11-18 23:49:49 PST ---
(In reply to comment #3)
> (From update of attachment 43448 [details])
> Jeeezz, what a large patch, thank god I'm on a nightshift with nothing else to
> do ;-)
I'll split out the LayoutTests into another patch, to shrink the patch to ~38k

> > Index: WebCore/platform/graphics/filters/FEGaussianBlur.cpp
> ...
> > +    sdx = std::max(sdx, static_cast<unsigned>(1));
> > +    sdy = std::max(sdy, static_cast<unsigned>(1));
> 
> Why not just std::max(sdx/y, 1); ?
Do not understand what you mean? the static_cast was neccessary to get a
unsigned 1. What is y in your example?

> > Index: WebCore/svg/SVGFilterElement.cpp
> ...
> > +    else if (attr->name() == SVGNames::filterResAttr) {
> > +        float x, y;
> > +        if (parseNumberOptionalNumber(value, x, y)) {
> > +            setFilterResXBaseValue(x);
> > +            setFilterResYBaseValue(y);
> > +        }
> 
> An else case reporting errors through SVGDocumentExtensions seems to be missing
> here.
I took a look at other SVG*Elements, we never handle parsing errors. We just
ignore the attribute, if the arguments are not parseable.

> > Index: WebCore/svg/graphics/filters/SVGFilter.h
> ...
> > -        bool effectBoundingBoxMode() const { return m_effectBBoxMode; }
> > +        virtual bool effectBoundingBoxMode() const { return m_effectBBoxMode; }
> >  
> > -        FloatRect filterRegion() const { return m_filterRect; }
> > -        FloatRect sourceImageRect() const { return m_itemBox; }
> > -        void calculateEffectSubRegion(FilterEffect*) const;
> > +        virtual FloatRect filterRegion() const { return m_filterRect; }
> > +        virtual FloatRect sourceImageRect() const { return m_itemBox; }
> > +
> > +        virtual FloatSize maxImageSize() const { return m_maxImageSize; }
> > +        virtual void calculateEffectSubRegion(FilterEffect*);
> 
> I guess these functions have already been marked virtual in the base class, and
> this is just for clarification?
Yes. Simon suggested this on ImageBufferFilter. I thougt it's the best to have
a common style on all childs of Filter.

> Wouldn't hurt if Simon Fraser / Eric Seidel / Oliver Hunt may have another
> look. I'll cc them soon.
I already added all persons, that responsed to bug 26380.

I'll upload a fixed patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.