[Webkit-unassigned] [Bug 48336] Scaling of feTurbulence is miscalculated

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Dirk Schulze <krit at webkit.org>  2010-10-26 23:24:00 PST ---
(From update of attachment 71906)
View in context: https://bugs.webkit.org/attachment.cgi?id=71906&action=review

In general I don't see the great difference to the current implementation. It's definitely better not to map the point on every loop call, thats why I'm fine with the changes anyway (but please take a look at the subregion first), but you mentioned that you saw failures. I didn't so far. Can you please add a test that fails at the moment but doesn't with your patch? Please attache it on this br as well.

> WebCore/platform/graphics/filters/FETurbulence.cpp:31
> +#include "SVGFilter.h"

Why did you inclue SVGFilter? It's definitely wrong to use SVG code in WebCore/platform. This will also break some builds.

> WebCore/platform/graphics/filters/FETurbulence.cpp:332
> +    FloatRect subregion = filterPrimitiveSubregion();
> +    subregion.intersect(filter->filterRegionInUserSpace());

I'm not sure if this is correct, since I understand the algorihtm that it needs the subregion, unclipped. Can you please add more tests? I'd like to see a subregion that is bigger than filterRegion and x,y should positioned outside of the filterRegion (top left).

> WebCore/platform/graphics/filters/FETurbulence.cpp:338
> +    FloatRect absoluteScale = reinterpret_cast<SVGFilter*>(filter)->mapLocalRectToAbsoluteRect(subregion);

I'd use FloatSize for scales, you could just add a size() at the end.

> WebCore/platform/graphics/filters/FETurbulence.cpp:340
> +    float stepY = subregion.height() / absoluteScale.height();
> +    float stepX = subregion.width() / absoluteScale.width();

Please use FloatSize again here.

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