[webkit-reviews] review denied: [Bug 5864] feTurbulence is not implemented : [Attachment 62876] SVG feTurbulence imeplementation

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 29 02:47:12 PDT 2010


Nikolas Zimmermann <zimmermann at kde.org> has denied Renata Hodovan
<hodovan at inf.u-szeged.hu>'s request for review:
Bug 5864: feTurbulence is not implemented
https://bugs.webkit.org/show_bug.cgi?id=5864

Attachment 62876:  SVG feTurbulence imeplementation
https://bugs.webkit.org/attachment.cgi?id=62876&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
WebCore/svg/graphics/filters/SVGFETurbulence.cpp:34
 +  #include <math.h>
Can you use <wtf/MathExtras.h> here? math.h shouldn't be used.

WebCore/svg/graphics/filters/SVGFETurbulence.cpp:212
 +		float tempPosition = component + s_perlinNoise;
Maybe rename to just 'position', looks slightly better.

WebCore/svg/graphics/filters/SVGFETurbulence.cpp:327
 +	FloatPoint point;
Can you move this declaration after the "FloatRect filterRegion = .." line.

WebCore/svg/graphics/filters/SVGFETurbulence.cpp:326
 +	int indexOfPixelChannel = 0;
This declaration could be moved right before the for loop.

WebCore/svg/graphics/filters/SVGFETurbulence.cpp:329
 +  
Newline can be removed as well.

WebCore/svg/graphics/filters/SVGFETurbulence.h:41
 +	static PassRefPtr<FETurbulence> create(TurbulanceType, float, float,
int, float, bool);
Header indention is still wrong :-)

Almost there, r- for the issues above.


More information about the webkit-reviews mailing list