Bug 30055

Most of the time ist spent in: RenderLayer::paintLayer RenderSVGRoot::paint It seems that the complete SVG will be rendered every time. I tested it with the Qt port where the dirtyRect is always the size of the whole frame.

The same page as html (HTML:div insted of SVG:rect): http://paroga.com/webkit/div.php?c=4 http://paroga.com/webkit/div.php?c=10000 When only setting a single rect (click on rect 3 or 4) the HTML version is much faster. When changing the color of all rects the SVG version is faster.

I'm not sure I see the difference. WebKit only seems to be drawing the SVG rects as their added. Using Quartz Debug, I do not see it repainting the whole SVG every time. Bug 14015 is sorta related (drawing too much). I think we need to see a better example to see the performance difference, if there is one. Your example does not convince me that we're drawing too much. However having written the code, I believe you that we're drawing too much. :)

(In reply to comment #3) > I'm not sure I see the difference. WebKit only seems to be drawing the SVG > rects as their added. Using Quartz Debug, I do not see it repainting the whole > SVG every time. Have you tried it wit rect.php?c=10000&s=3? Maybay it there to many rects out of the frame, you can't see a difference. It is not the inital rendering, you can see the difference only when click on rect 1, 2, 3 or 4. I think that the dirtyRegion is calculatet correctly, but nothing cares about it (at least in the Qt port)

OK, so clicking is important, yes. I didn't realize I had to click on them. I tried these just now in Safari 4.0.3 and SVG was correctly redrawing only one svg rect when toggling the color on one rect. If Qt is re-drawing the whole page, that seems like a qt bug. I did not try on tip of tree, but I kinda doubt this regressed... but maybe it did.

(In reply to comment #5) 1) open http://paroga.com/webkit/rect.php?c=4&s=3 2) click on the 1st rect and stop the time until the first rect changes its color 3) click on the 3rd rect and stop the time until the first rect changes its color 4) open http://paroga.com/webkit/rect.php?c=10000&s=3 5) click on the 1st rect and stop the time until the first rect changes its color 6) click on the 3rd rect and stop the time until the first rect changes its color 7) time in step 6 is much bigger than in step 3 Both times should be the same, because only one rect changes it's color. When time in step 2 is much bigger than in step 5 this is ok, because in the second page are much more items to be redrawn. You can see the expected result when you open the page with Firefox.

Created attachment 41379 [details] Paint RenderPath only when necessary With this path clicking on the 3rd or 4th rect in http://paroga.com/webkit/rect.php?c=10000&s=3 is about 4 times faster on my computer. (time from click event until the first rect changes it's color) I'm not sure if this is the right place for my changes.

This looks great! I'd like to test the patch myself first before I r+ this though.

Comment on attachment 41379 [details] Paint RenderPath only when necessary Why are you checking for intersection prior to applying the local transform to the paintInfo region?

(In reply to comment #9) > Why are you checking for intersection prior to applying the local transform to > the paintInfo region? I only tried to find an easy way to demonstate the performance problem. The paintInfo.context->save(); stuff takes a significant amount of time on my machine. In my optinion there must be a better way to handle "DOM repaints" in general. Always iterateing over _all_ items isn't very cool. Maybe you can confirm this issue and tell me the prefered solution. If you take a look at http://paroga.com/webkit/livedemo.php you can see a (not so good) example what people do with SVG.

This patch is exactly the right idea. I just need to confirm that it's implemented correctly. I need to make space in my schedule to test your patch locally and check to see if other if statements like this one should be added to SVG renderers.

Ok, I'm not able to reproduce the slowness you mention about clicking on the 3rd or 4th rect in http://paroga.com/webkit/rect.php?c=10000&s=3. That example is actually really poor, because it requires someone to know that they're supposed to click on a special rect to demonstrate the bug. Much better would be a self-documenting page with buttons one could click on which say what they do. I agree we should probably make a change like this. I just don't think we've justified it very well up until now. The pathalogical case that you're trying to demonstrate is a single group containing a large number of path objects (<rect>, etc) where only one of them changes. In current SVG code, all of them will be asked to paint and then the results will be clipped out. It appears that the clipping is rather cheap in CoreGraphics, but whatever platform you are using is slower.

(In reply to comment #12) > That example is actually really poor, because it requires someone to know that > they're supposed to click on a special rect to demonstrate the bug. Much > better would be a self-documenting page with buttons one could click on which > say what they do. I've improved the example.

(In reply to comment #12) > Ok, I'm not able to reproduce the slowness you mention about clicking on the > 3rd or 4th rect in http://paroga.com/webkit/rect.php?c=10000&s=3. I can't believe you! ;-) I now compiled r50334 with the Qt-port (4.6 beta) on windows and tested the following patch (with g_checkIntersect true and false): > if (paintInfo.context->paintingDisabled() || style()->visibility() ...) > return; > > + FloatRect boundingBox = repaintRectInLocalCoordinates(); > + if (g_checkIntersect && !boundingBox.intersects(paintInfo.rect)) > + return; > + > paintInfo.context->save(); > paintInfo.context->concatCTM(localToParentTransform()); > > SVGResourceFilter* filter = 0; > > - FloatRect boundingBox = repaintRectInLocalCoordinates(); > if (paintInfo.phase == PaintPhaseForeground) { > PaintInfo savedInfo(paintInfo); When I open http://paroga.com/webkit/rect.php?c=80000&s=3 with g_checkIntersect = true it takes about 2 seconds to change the color of the first element. With g_checkIntersect = false the same action takes about 10 seconds on my machine. Maybe the rendering/clipping isn't as time-consuming with your port? Is it possible that you try it with the Qt-port? When i apply this patch to WebCore it can't be a Qt-specific rendering problem?!?

> Maybe the rendering/clipping isn't as time-consuming with your port? Is it > possible that you try it with the Qt-port? When i apply this patch to WebCore > it can't be a Qt-specific rendering problem?!? WebCore uses the platform's graphics library, if that graphics library has bad perf in some cases it can easily cause all sorts of badness. I'm unsure whether the solution should be to get Qt to fix this issue (presumably it's a problem in their graphics library) or fix it in WebCore itself (which may negatively impact performance of other platforms) Maybe this logic should be in GrpahicsContextQt?

(In reply to comment #15) > WebCore uses the platform's graphics library, if that graphics library has bad > perf in some cases it can easily cause all sorts of badness. I'm unsure > whether the solution should be to get Qt to fix this issue (presumably it's a > problem in their graphics library) or fix it in WebCore itself (which may > negatively impact performance of other platforms) > > Maybe this logic should be in GrpahicsContextQt? Do you think rendering anything ist better than checking if it is necessary? I think WebCore should render as least as possible. In GrpahicsContextQt there won't be any information to check if painting is required. Rely on a "perfect performing" graphics library isn't the right way in opinion. There might be some other ports with slow graphics librarys too.

(In reply to comment #16) > (In reply to comment #15) > > WebCore uses the platform's graphics library, if that graphics library has bad > > perf in some cases it can easily cause all sorts of badness. I'm unsure > > whether the solution should be to get Qt to fix this issue (presumably it's a > > problem in their graphics library) or fix it in WebCore itself (which may > > negatively impact performance of other platforms) > > > > Maybe this logic should be in GrpahicsContextQt? > Do you think rendering anything ist better than checking if it is necessary? I > think WebCore should render as least as possible. In GrpahicsContextQt there > won't be any information to check if painting is required. Rely on a "perfect > performing" graphics library isn't the right way in opinion. There might be > some other ports with slow graphics librarys too. The problem is that if the underlying graphics library is already doing the tests it may have a more efficient way of doing so, add to that the fact that the underlying graphics libraries that do perform sensible clipping optimisations will still be doing their own clip tests this seems like a patch that has bad perf impact on good libraries to deal with poor performance in others. I suspect if we really do want webcore to work around what is arguably a perf bug in Qts graphics subsystem the fix should be placed in GraphicsContextQt directly

I think we can use both approaches, and I think that this approach has some usefulness. It needs to be well tested though. I also agree with Oliver, that if Qt is not properly ignoring drawing commands which are issued outside of the clip region, than that seems like a major bug in Qt.

(In reply to comment #17) > The problem is that if the underlying graphics library is already doing the > tests it may have a more efficient way of doing so, add to that the fact that > the underlying graphics libraries that do perform sensible clipping > optimisations will still be doing their own clip tests this seems like a patch > that has bad perf impact on good libraries to deal with poor performance in > others. So this patch has to move to a higher level where "more intelligent" test can be done and the clipping should be fixed in GraphicsContextQt directly. However the following test is much slower(!) in Safari/Chrome than in FF: Click on "Set first lime/red" at http://paroga.com/webkit/rect.php?c=80000&s=2 On my machine: FF ~150ms, Opera ~300ms, Chrome ~2s, Safari ~10s How to get the same performance of FF/Opera in WebCore? Any suggestions?

Attachment 41379 [details] passed the style-queue

(In reply to comment #19) > (In reply to comment #17) > > The problem is that if the underlying graphics library is already doing the > > tests it may have a more efficient way of doing so, add to that the fact that > > the underlying graphics libraries that do perform sensible clipping > > optimisations will still be doing their own clip tests this seems like a patch > > that has bad perf impact on good libraries to deal with poor performance in > > others. > So this patch has to move to a higher level where "more intelligent" test can > be done and the clipping should be fixed in GraphicsContextQt directly. > > However the following test is much slower(!) in Safari/Chrome than in FF: > Click on "Set first lime/red" at http://paroga.com/webkit/rect.php?c=80000&s=2 > On my machine: FF ~150ms, Opera ~300ms, Chrome ~2s, Safari ~10s > > How to get the same performance of FF/Opera in WebCore? Any suggestions? I don't see this 10s delay you're referring to what platform are you testing on?

(In reply to comment #21) > I don't see this 10s delay you're referring to what platform are you testing > on? Safari 4.0.4, Windows XP SP3, IBM T60 2GHz, 3GB

Comment on attachment 41379 [details] Paint RenderPath only when necessary I found another testcase where we get pathologically bad perf due to this issue which would imply that there's just too much cost in not clipping prior to calling the graphics library -- my test case just created many many many (~50000 odd) nodes mostly off the screen and this patch represents a huge improvement in that case. Still not sure why i don't see the perf problems you do in your example, but with a testcase that is too slow this shows up as a clear problem in shark. r=me. The patch applies cleanly lets see how the commit bot deals with the changelog.

Comment on attachment 41379 [details] Paint RenderPath only when necessary Rejecting patch 41379 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11771 test cases. svg/batik/filters/filterRegions.svg -> failed Exiting early after 1 failures. 10019 tests run. 313.92s total testing time 10018 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 6 test cases (<1%) had stderr output

(In reply to comment #23) > (From update of attachment 41379 [details]) > I found another testcase where we get pathologically bad perf due to this issue > which would imply that there's just too much cost in not clipping prior to > calling the graphics library -- my test case just created many many many > (~50000 odd) nodes mostly off the screen and this patch represents a huge > improvement in that case. offscreen: It depends. If the SVG size is also bigger than the screensize, we shouldn't clip to the currently visible area. smfr remind me, that we can include a SVG into a HTML-file and make some 3D transformations (via CSS3) with it. That might not work correctly, if the SVG is clipped.

(In reply to comment #25) > (In reply to comment #23) > > (From update of attachment 41379 [details] [details]) > > I found another testcase where we get pathologically bad perf due to this issue > > which would imply that there's just too much cost in not clipping prior to > > calling the graphics library -- my test case just created many many many > > (~50000 odd) nodes mostly off the screen and this patch represents a huge > > improvement in that case. > > offscreen: > It depends. If the SVG size is also bigger than the screensize, we shouldn't > clip to the currently visible area. smfr remind me, that we can include a SVG > into a HTML-file and make some 3D transformations (via CSS3) with it. That > might not work correctly, if the SVG is clipped. We should simply be clipping to the region that is going to actually be drawn to, assuming everything is working correctly that is correct -- if i can apply a 3d transform and that will result in incorrect clipping area then thats a serious bug in the 3d transform paths

Curious what the status of this patch is? Should this be r-'d now that we know that this patch will at least cause one layout test to fail?

Comment on attachment 41379 [details] Paint RenderPath only when necessary Marking r- since this causes a test to fail.

Also the second OOPS in the ChangeLog would have caused the commit to fail. ChangeLogs should mention tests affected by a change.

(In reply to comment #28) > (From update of attachment 41379 [details]) > Marking r- since this causes a test to fail. The problem is that an other area of Webkit does something wrong. This patch is ok, but i depends on an "not created bug". Can we create a new bug, make the patch depend on in r+ it again? Who is responsible for the code tested with filterRegions.svg? Maybe he can make a change there to get this patch in?

Yes, if we're waiting to commit this because a problem exists that we've never actually created a bug for, then definitely we should create a bug. :) Getting an r+ is relatively easy. Especially since this patch already had one. But if we can't land it due to another bug, we should file the other bug and mark this one blocked.

Can we move this fix to SVGRenderBase::prepareToRenderSVGContent?? This would stop rendering earlier and can improve the rendering time on other parts like images too.

(In reply to comment #31) > Yes, if we're waiting to commit this because a problem exists that we've never > actually created a bug for, then definitely we should create a bug. :) Can we r+ and cq+ it again since bug 32815 is fixed now? (In reply to comment #32) > Can we move this fix to SVGRenderBase::prepareToRenderSVGContent?? > > This would stop rendering earlier and can improve the rendering time on other > parts like images too. I think we should commit this change and maybe another too. This patch is not in the SVG part so it might improve also some other parts.

> (In reply to comment #32) > (In reply to comment #32) > > Can we move this fix to SVGRenderBase::prepareToRenderSVGContent?? > > > > This would stop rendering earlier and can improve the rendering time on other > > parts like images too. > I think we should commit this change and maybe another too. This patch is not > in the SVG part so it might improve also some other parts. RenderPath is SVG only.

(In reply to comment #34) > RenderPath is SVG only. I meant that maybe another componet will use RenderPath in the future, but thats not a real reason. Moving it to prepareToRenderSVGContent won't prevent _unnecessary_ paintInfo.context->save(), concatCTM(...), restore() and so on. It should be done as early as possible! Iterating over _all_ items, even if only one pixel changed, is already not very performant. (checking which items need a repaint might be even more cpu consuming)

(In reply to comment #35) > (In reply to comment #34) > > RenderPath is SVG only. > I meant that maybe another componet will use RenderPath in the future, but > thats not a real reason. > > Moving it to prepareToRenderSVGContent won't prevent _unnecessary_ > paintInfo.context->save(), concatCTM(...), restore() and so on. > It should be done as early as possible! Iterating over _all_ items, even if > only one pixel changed, is already not very performant. (checking which items > need a repaint might be even more cpu consuming) Can you please upload a new patch with a link to the bugreport Changelog and the title of the bug? There shouldn't be Oops'es in a Changelog. Please write why it is not possible to test the speed improvement and I think we can commit it.

Created attachment 45658 [details] Paint RenderPath only when necessary

style-queue ran check-webkit-style on attachment 45658 [details] without any errors.

Comment on attachment 45658 [details] Paint RenderPath only when necessary LGTM. Thanks for the patch.

Comment on attachment 45658 [details] Paint RenderPath only when necessary Rejecting patch 45658 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueueSVN/LayoutTests Testing 11854 test cases. svg/custom/marker-overflow-clip.svg -> failed Exiting early after 1 failures. 10306 tests run. 313.90s total testing time 10305 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 7 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/152351

(In reply to comment #40) > (From update of attachment 45658 [details]) > Rejecting patch 45658 from commit-queue. > > Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', > '--exit-after-n-failures=1', '--quiet']" exit_code: 1 > Running build-dumprendertree > Running tests from /Users/eseidel/Projects/CommitQueueSVN/LayoutTests > Testing 11854 test cases. > svg/custom/marker-overflow-clip.svg -> failed > > Exiting early after 1 failures. 10306 tests run. > 313.90s total testing time > > 10305 test cases (99%) succeeded > 1 test case (<1%) had incorrect layout > 7 test cases (<1%) had stderr output > > Full output: http://webkit-commit-queue.appspot.com/results/152351 Ahhh sorry. There is another bug about markers and boundingBoxes: bug 33012 . Without knowing the actual result with your patch, I guess that this fixes the problem.

Comment on attachment 45658 [details] Paint RenderPath only when necessary Marking r- since this seems to cause a test to fail? Correct me if I'm wrong Dirk.

(In reply to comment #42) > (From update of attachment 45658 [details]) > Marking r- since this seems to cause a test to fail? Correct me if I'm wrong > Dirk. No, the patch is ok. It just depends on anotherone. I'll commot-queue+ the patch again, once Niko fixed the marker bug.

Comment on attachment 45658 [details] Paint RenderPath only when necessary Markers and repaintRects are fixed. Lets try it again.

Comment on attachment 45658 [details] Paint RenderPath only when necessary Clearing flags on attachment: 45658 Committed r52900: <http://trac.webkit.org/changeset/52900>

All reviewed patches have been landed. Closing bug.

Comment on attachment 45658 [details] Paint RenderPath only when necessary This type of fix would be more interesting to apply to the hit-testing code path. But this fix should be fine. Thanks for the patch Patrick and the review Dirk!

This change broke a bunch of SVG tests, all pixel results. For illustration, visit this at ToT: http://trac.webkit.org/export/52929/trunk/LayoutTests/fast/backgrounds/size/contain-and-cover.html I think we should roll this change out.

Reverted as http://trac.webkit.org/changeset/52930.

(In reply to comment #48) > This change broke a bunch of SVG tests, all pixel results. For illustration, > visit this at ToT: > http://trac.webkit.org/export/52929/trunk/LayoutTests/fast/backgrounds/size/contain-and-cover.html > > I think we should roll this change out. A problem of transparent and half transparent parts of some objects?

Created attachment 46243 [details] Bad DOM performance in large SVG files

Created attachment 46244 [details] Bad DOM performance in large SVG files

Comment on attachment 46243 [details] Bad DOM performance in large SVG files LGTM, I assume there are no pixel test regressions, r+.

Comment on attachment 46244 [details] Bad DOM performance in large SVG files Interessting that CGPathIsEmpty is that slow... I assume you see a good benefit, so r=me.

First patch landed in r53057. I'm actually slightly uneasy about landing the isempty caching so i'm going to see if there's something that can be done to get a bigger win. That said i think that this patch should resolve all of the perf issues in the main testcase attached to this patch.

Sorry guys, still no luck on pixel tests. Running "run-webkit-tests --pixel-tests svg" produces: Enabling pixel tests with a tolerance of 0.1% WARNING: Temporarily changing the main display color profile: The colors on your screen will change for the duration of the testing. This allows the pixel tests to have consistent color values across all machines. Testing 945 test cases. svg/W3C-SVG-1.1 ....................................................................... svg/W3C-SVG-1.1/coords-units-01-b.svg -> pixel test failed ......... svg/W3C-SVG-1.1/filters-composite-02-b.svg -> pixel test failed .......................................... svg/W3C-SVG-1.1/masking-intro-01-f.svg -> pixel test failed . svg/W3C-SVG-1.1/masking-mask-01-b.svg -> pixel test failed ...................................... svg/W3C-SVG-1.1/pservers-grad-03-b.svg -> pixel test failed ... svg/W3C-SVG-1.1/pservers-grad-06-b.svg -> pixel test failed ....... svg/W3C-SVG-1.1/pservers-grad-13-b.svg -> pixel hash failed (but pixel test still passes) .... svg/W3C-SVG-1.1/pservers-grad-17-b.svg -> pixel hash failed (but pixel test still passes) ... svg/W3C-SVG-1.1/pservers-pattern-01-b.svg -> pixel test failed ....................................................................... svg/W3C-SVG-1.1/text-deco-01-b.svg -> pixel hash failed (but pixel test still passes) .... svg/W3C-SVG-1.1/text-path-01-b.svg -> pixel hash failed (but pixel test still passes) ... svg/W3C-SVG-1.1/text-text-03-b.svg -> pixel hash failed (but pixel test still passes) .... svg/W3C-SVG-1.1/text-text-07-t.svg -> pixel hash failed (but pixel test still passes) ........ svg/animations . svg/batik/filters .. svg/batik/masking . svg/batik/masking/maskRegions.svg -> pixel test failed svg/batik/paints .. svg/batik/paints/patternPreserveAspectRatioA.svg -> pixel test failed . svg/batik/paints/patternRegionA.svg -> pixel test failed . svg/batik/paints/patternRegions.svg -> pixel test failed svg/batik/text ..... svg/batik/text/textDecoration2.svg -> pixel hash failed (but pixel test still passes) .. svg/batik/text/textEffect3.svg -> pixel hash failed (but pixel test still passes) .. svg/batik/text/textFeatures.svg -> pixel hash failed (but pixel test still passes) . svg/batik/text/textGlyphOrientationHorizontal.svg -> pixel hash failed (but pixel test still passes) ....... svg/batik/text/textOnPathSpaces.svg -> pixel hash failed (but pixel test still passes) ..... svg/batik/text/textProperties.svg -> pixel hash failed (but pixel test still passes) . svg/batik/text/textStyles.svg -> pixel hash failed (but pixel test still passes) . svg/batik/text/verticalText.svg -> pixel hash failed (but pixel test still passes) . svg/batik/text/verticalTextOnPath.svg -> pixel hash failed (but pixel test still passes) . svg/carto.net .......... svg/css . svg/css/arrow-with-shadow.svg -> pixel hash failed (but pixel test still passes) . svg/css/circle-in-mask-with-shadow.svg -> pixel test failed .. svg/css/composite-shadow-example.html -> pixel hash failed (but pixel test still passes) . svg/css/composite-shadow-with-opacity.html -> pixel hash failed (but pixel test still passes) .... svg/css/group-with-shadow.svg -> pixel hash failed (but pixel test still passes) . svg/css/mask-with-shadow.svg -> pixel test failed . svg/css/path-with-shadow.svg -> pixel hash failed (but pixel test still passes) . svg/css/stars-with-shadow.html -> pixel hash failed (but pixel test still passes) svg/custom ....................................... svg/custom/deep-dynamic-updates.svg -> pixel test failed ................................................... svg/custom/grayscale-gradient-mask.svg -> pixel test failed ............ svg/custom/image-with-transform-clip-filter.svg -> pixel hash failed (but pixel test still passes) ................. svg/custom/js-late-mask-and-object-creation.svg -> pixel test failed . svg/custom/js-late-mask-creation.svg -> pixel test failed . svg/custom/js-late-pattern-and-object-creation.svg -> pixel test failed . svg/custom/js-late-pattern-creation.svg -> pixel test failed ............ svg/custom/js-update-pattern-child.svg -> pixel test failed . svg/custom/js-update-pattern.svg -> pixel test failed ....................... svg/custom/marker-changes.svg -> pixel hash failed (but pixel test still passes) . svg/custom/marker-child-changes.svg -> pixel hash failed (but pixel test still passes) ...... svg/custom/marker-strokeWidth-changes.svg -> pixel hash failed (but pixel test still passes) .. svg/custom/mask-changes.svg -> pixel test failed . svg/custom/mask-child-changes.svg -> pixel test failed . svg/custom/mask-excessive-malloc.svg -> pixel test failed . svg/custom/mask-inside-defs.svg -> pixel test failed . svg/custom/mask-on-multiple-objects.svg -> pixel test failed . svg/custom/mask-with-all-units.svg -> pixel test failed .............. svg/custom/pattern-cycle-detection.svg -> pixel test failed . svg/custom/pattern-deep-referencing.svg -> pixel test failed . svg/custom/pattern-in-defs.svg -> pixel test failed . svg/custom/pattern-rotate.svg -> pixel test failed . svg/custom/pattern-with-transformation.svg -> pixel test failed . svg/custom/pattern-y-offset.svg -> pixel test failed ............... svg/custom/radial-gradient-with-outstanding-focalPoint.svg -> pixel test failed .... svg/custom/resource-invalidate-on-target-update.svg -> pixel test failed ................ svg/custom/stroked-pattern.svg -> pixel test failed ........................................................................... svg/custom/use-on-symbol-inside-pattern.svg -> pixel test failed ......................... svg/custom/visibility-override-mask.svg -> pixel test failed .... svg/dom ......................... svg/dom/SVGRectElement . svg/dom/SVGScriptElement .... svg/dom/SVGStyleElement . svg/dynamic-updates ................................................................................................................................ svg/filters .......... svg/filters/filter-clip.svg -> pixel test failed ........ svg/hittest .. svg/hixie/cascade .. svg/hixie/data-types .. svg/hixie/dynamic ...... svg/hixie/error .................. svg/hixie/links ... svg/hixie/mixed ......... svg/hixie/painting . svg/hixie/perf ..... svg/hixie/processing-model ... svg/hixie/rendering-model ..... svg/hixie/shapes/path . svg/hixie/text ..... svg/hixie/transform . svg/hixie/use ... svg/hixie/viewbox .... svg/hixie/viewbox/preserveAspectRatio .. svg/text .... svg/text/selection-background-color.xhtml -> pixel hash failed (but pixel test still passes) .... svg/text/text-align-04-b.svg -> pixel hash failed (but pixel test still passes) .... svg/text/text-deco-01-b.svg -> pixel hash failed (but pixel test still passes) ....... svg/text/text-path-01-b.svg -> pixel hash failed (but pixel test still passes) .... svg/text/text-text-01-b.svg -> pixel hash failed (but pixel test still passes) . svg/text/text-text-03-b.svg -> pixel hash failed (but pixel test still passes) . svg/text/text-text-04-t.svg -> pixel hash failed (but pixel test still passes) ... svg/text/text-text-07-t.svg -> pixel hash failed (but pixel test still passes) . svg/text/text-text-08-b.svg -> pixel hash failed (but pixel test still passes) ....... svg/transforms .. svg/transforms/text-with-pattern-inside-transformed-html.xhtml -> pixel test failed . svg/transforms/text-with-pattern-with-svg-transform.svg -> pixel test failed svg/webarchive ... 72.68s total testing time 904 test cases (95%) succeeded 41 test cases (4%) had incorrect layout 1 test case (<1%) had stderr output

You can see the failures clearly here: http://build.chromium.org/buildbot/waterfall.fyi/waterfall?branch=&builder=Webkit+(webkit.org)&builder=Webkit+Linux+(webkit.org)&builder=Webkit+Mac+(webkit.org)&builder=XP+Perf+(webkit.org)&builder=Linux+Perf+(webkit.org)&builder=Webkit+(purify+webkit.org)

Just to make sure, with the patch rolled out locally, same command produces: 944 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 1 test case (<1%) had stderr output

Comment on attachment 46243 [details] Bad DOM performance in large SVG files Clearing review flag, this patch needs to be revisited. Anyone who works on this has to be aware that regressions can only be spotted when running _PIXEL TESTS_.

Comment on attachment 46244 [details] Bad DOM performance in large SVG files Taking back review, as discussed with Oliver.

Created attachment 46667 [details] Patch

Created attachment 46668 [details] Patch

Comment on attachment 46668 [details] Patch Okay, let's hope it fixes it this time... Can you file a follow-up bug regarding PaintInfo fixes is renderSubtreeToImage?

Committed r53331: <http://trac.webkit.org/changeset/53331>

Darnit! One test still broke: http://trac.webkit.org/browser/trunk/LayoutTests/fast/borders/svg-as-border-image-2.html Can you take a look and see what's happening there?

Rolled out Oliver's patch in http://trac.webkit.org/changeset/53334, per IRC discussion w/Nikolas. But you're really, really close now! Don't give up!

Created attachment 46701 [details] Patch

Committed r53343: <http://trac.webkit.org/changeset/53343>

Created attachment 46715 [details] Patch

Just opening for a improvement to culling

Comment on attachment 46715 [details] Patch Please add more complete explanation to the ChangeLog. r=me

Committed r53349: <http://trac.webkit.org/changeset/53349>