44374 2010-08-20 19:46:42 -0700 Changing cx/cy causes SVGCircleElement to regenerate path data when not needed 2012-06-23 13:39:33 -0700 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 44402 44375 65236 1 eric webkit-unassigned kbr krit pdr rwlbuis zimmermann oldest_to_newest 267512 0 eric 2010-08-20 19:46:42 -0700 Changing cx/cy causes SVGCircleElement to regenerate path data when not needed http://themaninblue.com/experiment/AnimationBenchmark/svg source: http://themaninblue.com/experiment/AnimationBenchmark/svg/js/main.js We're spending 7% of that benchmark regenerating path data for the circles which is unnecessary. (45% is spent in RenderPath::paint().) If instead we used a transform for the cx/cy, we could always draw the circle around 0 and not need to regen the path when cx/cy changes. 267520 1 eric 2010-08-20 19:59:14 -0700 Layout is close to 16% of that benchmark. We'd still need to do a layout, but the layout cost would be very small if we could avoid needing to regenerate the path. 267532 2 krit 2010-08-20 21:58:19 -0700 We could realy use the unite circle and just transform it according to x,y,cx,cy. But that causes some changes, first we would need to store the Path's in the elements (rect,circle,path,polygon,line,...). And we should also get rid of the current ellipse logic in Path::createEllipse and let the platforms draw the circle. 267566 3 eric 2010-08-21 07:33:49 -0700 As noted in bug 44375, path copying is really expensive. So all of our Path::create* functions which return a path, cause a copy. I think we should consider using the unit circle as you suggest. 267577 4 eric 2010-08-21 09:41:54 -0700 I downloaded the benchmark locally (http://themaninblue.com/experiment/AnimationBenchmark/benchmarks.zip) and modified the SVG benchmark to use transforms instead of setting cx/cy. We spend 6% time under layout() instead of 16%! (basically working around this bug). But our framerate still tops out at 30fps. Maybe we're hitting some sort of framerate cap? 267579 5 krit 2010-08-21 09:45:11 -0700 Commented on the wrong bug, see https://bugs.webkit.org/show_bug.cgi?id=44375#c6 267583 6 krit 2010-08-21 10:01:37 -0700 (In reply to comment #4) > I downloaded the benchmark locally (http://themaninblue.com/experiment/AnimationBenchmark/benchmarks.zip) and modified the SVG benchmark to use transforms instead of setting cx/cy. We spend 6% time under layout() instead of 16%! Well, if you modify the test in that way, we don't invalidate the path. Means, you avoid the copy process from SVG*Element to SVGRenderPath as well. That's what I mean with store the path in the element in comment #2. 443536 7 rwlbuis 2011-07-28 10:12:55 -0700 Implementing https://bugs.webkit.org/show_bug.cgi?id=65236 could make this bug unneeded. We should keep themaninblue benchmark in mind though for 65236. Cheers, Rob. 547639 8 krit 2012-02-01 21:49:46 -0800 Reni, are you continuing you work on RenderSVGShape, so that this bug get unnecessary? 656130 9 pdr 2012-06-23 13:39:33 -0700 Fixed in http://trac.webkit.org/changeset/112667