SMIL performance affected by excessive drawNode calls

[GoogleCodeExporter]

View the attached file, pinwheel.svg, to observe the (low) frame rate. On some 
systems it may appear that the browser has hung.  Even when the animations are 
(visually) complete, the CPU is hogged by the animation code.

Tested with Dracolisk, Flash 10, forceflash=true.

Pinwheel.svg contains rectangles and text (SVG fonts) with animations of 
visibility, opacity and transform(rotate and scale).  The main overhead is 
caused by calls to drawNode which are not required.  Animating visibility (and 
opacity) currently incurs a complete redraw of the targetNode and all children 
at every frame for which the animation is active.

For each glyph at each frame, this means cloning, parsing the path, generating 
graphics commands, drawing, removing the old glyph and adding the new glyph.

For animations involving visibility and opacity (and display) a complete redraw 
of the targetNode can be avoided in much the same way as transforms are 
handled.  All that is required is setting the drawSprite.alpha (in the case of 
visibility and opacity) and topSprite.visible (for display).

I'm working on a patch which implements this approach.  Included are a few 
other efficiency gains I've found in the process.

Original issue reported on code.google.com by k...@svgmaker.com on 14 Jul 2010 at 7:50

Attachments:

• pinwheel.svg

[GoogleCodeExporter]

Attached is a patch (smil.diff) and svg.swf.

Overview
At each frame, if the animation is active, determine if the attribute can be 
handled without calling invalidateDisplay and invalidateChildren (and forcing a 
full redraw).  Candidate attributes are transform, viewBox, x, y, opacity, 
visibility, and display.  Similar to invalidateDisplay, an event listener is 
added to the targetNode to handle these "simple" animations.  Only one event 
listener is added even if more than one attribute on the targetNode is 
animated.  If targetNode.invalidateDisplay is called, then the pending invalid 
attributes are cleared and left for drawNode to do.

Servicing visibility and display outside of drawNode meant that some of the 
code path through drawNode needed to change.  The main change is performing the 
drawing operations even if display="none" or visibility="hidden".  This is to 
ensure that the drawing state is always updated in drawNode so that subsequent 
animations of display and visibility outside of drawNode can reveal the correct 
drawing state.

Brief notes on the patch:

SVGNode
- drawNode
  - only set topSprite.visible = false on nodes which explicitly set display="none".
    all child sprites become invisible. Saves on setting topSprite.visible on children
  - complete drawing the node, even if display="none"
    downside is that drawing code is executed (but not drawn to screen)
  - similar in SVGUseNode, SVGImageNode.onImageLoaded
- setVisibility
  - bug: when a child node with a visibility attribute is encountered, 
    the function should stop processing children. The code continued to 
    process the children anyway
  - setting alpha=0 on a node turns off all the children anyway, so processing 
    the chidren doesn't really achieve anything ?
  - setting drawSprite.alpha on SVGTextNodes does appear to work with Flash 10
    so I removed setVisibility (which used GlowFilter instead) from SVGTextNode
- nodeBeginFill and nodeBeginStroke
  - draw the node even if visibility="hidden" to update the drawing state
  - possibly should be doing this for pointer-events="all" anyway

SVGTimedNode
- drawNode
  - implement drawNode similar to SVGDefs.drawNode
  - avoid unnecessary attribute checking and processing on animation elements 
    when they have been invalidated by invalidateChildren

SVGAnimateNode
  - doInvalidate used to skip invalidating attributes in onSVGDocTimeUpdate
  - primarily used by SVGSetNode which only requires invalidating 
    at timeIntervalStarted and timeIntervalEnded
  - this saves an eventListener and possible drawNode per frame

SVGGlyphNode
  - the glyph path can't be changed (not animatable, no DOM access) so the 
    graphics commands for any given glyph only need to be parsed and generated
    once
  - graphics commands are generated on demand when the glyph is cloned and 
    kept for later cloning
  - this saves the parsing and generating graphics commands for each repeated 
    instance of a glyph. (see attached quick.001.svg).
  - the same saving at each drawNode during animations for the glyph
  - it may be possible to do something similar for SVGPathNode

SVGPathNode
  - not SMIL related
  - the ' ' (space) glyph path can be omitted from SVGGlyphNode and causes an 
    exception when trying to generate graphics commands

Original comment by k...@svgmaker.com on 15 Jul 2010 at 7:47

Attachments:

• smil.diff
• svg.swf
• quick.001.svg

[GoogleCodeExporter]

Original comment by bradneub...@gmail.com on 16 Jul 2010 at 12:12

• Added labels: Patch, Priority-Medium
• Removed labels: Priority-Low

[GoogleCodeExporter]

> Tested with Dracolisk, Flash 10, forceflash=true.

Oops.  I forgot that Dracolisk doesn't include the SMIL work from Issue 476 
(r1134).  I tested again against r1205 and the result is better.  Still a low 
frame rate but at completion, the CPU isn't bogged down by the animation code.

Original comment by k...@svgmaker.com on 16 Jul 2010 at 4:32

[GoogleCodeExporter]

Patch incorporated in r1206:
Issue 531; Optimize SMIL animation by reducing drawNode calls
for certain attribute changes.
Also includes removal of hidden native text workaround for flash 9.
Thanks to Ken Stacey at svgmaker for the patch!
The pinwheel example did not work for me but I tested various other files.

Original comment by grick23@gmail.com on 26 Jul 2010 at 3:03

• Changed state: Fixed

[GoogleCodeExporter]

Thanks for incorporating the patch.  Sorry about the pinwheel example - broken 
by some last second hand editing before I uploaded it.  It does demonstrate the 
issue well,  so here it is again.

Original comment by k...@svgmaker.com on 26 Jul 2010 at 1:49

Attachments:

• pinwheel.svg