5x speedup on svg-filter-animation.svg (from 6.6 fps to 30 fps on a Mac mini).
Created attachment 80310 [details] patch I think LightSource.cpp should be split up into 3 files, but I think that should go to a separate patch (requires build file changes, and not the scope of this patch).
Attachment 80310 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/svg/SVGFELightElement.cpp:103: Declaration has space between type name and * in SVGFilterPrimitiveStandardAttributes *parentPrimitive [whitespace/declaration] [3] Total errors found: 1 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 80310 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=80310&action=review > Source/WebCore/platform/graphics/filters/LightSource.h:86 > + virtual bool setAzimuth(float) { return false; } > + virtual bool setElevation(float) { return false; } > + virtual bool setX(float) { return false; } > + virtual bool setY(float) { return false; } > + virtual bool setZ(float) { return false; } > + virtual bool setPointsAtX(float) { return false; } > + virtual bool setPointsAtY(float) { return false; } > + virtual bool setPointsAtZ(float) { return false; } > + virtual bool setSpecularExponent(float) { return false; } > + virtual bool setLimitingConeAngle(float) { return false; } Do we really need all those virtual methods? Is there no way to find out wheter a LightSource is eg. a PointLightSource, then cast to it? and call the setter there, instead of having to declare a lot of virtual functions here? The general concept of the patch seems fine, but I dislike the addition of that many virtual methods. Please look into another approach! Thanks in advance!
Created attachment 80328 [details] updated patch
Comment on attachment 80328 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=80328&action=review r=me > Source/WebCore/svg/SVGFEDiffuseLightingElement.cpp:123 > + ASSERT_NOT_REACHED(); Is this assertion correct here? Why can't/shouldn't we pass a wrong attribute here?
Thanks for the review. > > Source/WebCore/svg/SVGFEDiffuseLightingElement.cpp:123 > > + ASSERT_NOT_REACHED(); > > Is this assertion correct here? Why can't/shouldn't we pass a wrong attribute here? That is correct, since the function is (transitively) called from another function of the same Element, if it recognised the attribute. Thus wrong / unknown attributes should not be passed to this particular function.
Landed in http://trac.webkit.org/changeset/77240 Closing bug.