Bug 48336 – Scaling of feTurbulence is miscalculated

• Status: RESOLVED
• Resolution: INVALID
• Component: SVG

[Renata Hodovan]

Scaling of feTurbulence is miscalculated

[Dirk Schulze]

Do you have an example that shows the problem? I couldn't see a problem right now.

[Renata Hodovan]

Created attachment 71906 [details]
Fix the calcluation of feTurbulence if it's scaled

[Renata Hodovan]

(In reply to comment #1)
> Do you have an example that shows the problem? I couldn't see a problem right now.
If you get any test of feTurbulence and resize the image, the turbulence filter doesn't scale but seems like you see the pic through a growing window.

[Nikolas Zimmermann]

Comment on attachment 71906 [details]
Fix the calcluation of feTurbulence if it's scaled

Hey Reni,

this needs a test. You can copy any existing feTurbulence patch to svg/zoom/page, add the "var zoomPageCount..." etc. logic. That should be sufficient.
Going to leave the real review of the code changes to Dirk.

Cheers,
Niko

[Dirk Schulze]

Comment on attachment 71906 [details]
Fix the calcluation of feTurbulence if it's scaled

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.

[Renata Hodovan]

Created attachment 72062 [details]
Example

The two images should be equivalent except of their size

[Dirk Schulze]

(In reply to comment #6)
> Created an attachment (id=72062) [details]
> Example
> 
> The two images should be equivalent except of their size

Checked it with a 4 days old build. The result looks to 100% identical to the results of Opera and Firefox.

[Dirk Schulze]

Reni, could you check it again please? Still don't see any problems.

[Renata Hodovan]

It looks good to me now. Closed.