Hi Thomas. Thomas DeWeese: > I was able to track this down yesterday. The problem is both > a deep one and a simple one ;) You have switched updating almost entirely > from registering DOM listeners to being notified of animated attributes > be changed (Bridge.handleAnimatedAttributeChanged). I actually quite > like this it is a move that could make a real dependency tracker more > feasible. The problem is that currently the sending of the attribute > change > notifications only happens if an animatable attribute has a > LiveAttributeValue > (such as SVGOMAnimatedTransformList), if it's live attribute value is > cleared then the attribute ceases to be treated as animated any more. > > So the bug is that the live attribute values are only held by the > SoftDoublyIndexedTable which as it's name implies only holds the value > with a soft reference. So the bug occurs if the AnimatedTransformList > (the transform attributes live attribute type) get's GCed before the code > starts manipulating the transform on the group.
Oh, good find! So I wonder why it was the transform attribute that got collected while others didn’t? > This is the deep part. The bug applies to every animatable attribute! > The shallow bit is that there are I think two fairly simple solutions to > the problem: > > 1) Change the SoftDoublyIndexedTable into a > HardDoublyIndexedTable. > > 2) Have the Bridges hold a hard reference to the animatable values > that they use. > > Of the two the second is my preference, this could potentially > speed things up as the get's would no longer need to go through the > HashTable nonsense. But it raises the issue that essentially all > attributes would almost always have their live attribute present. > This makes me think of potentially abolishing the separate live attribute > values table and adding a LAV member to the AttrNodes (either all > or just the one's with potential live attributes). > > What do people think? I agree, if the unified animated-or-not code in the bridges need to look up all of the LAVs anyway, they may as well be kept as hard references all the time. The problem with keeping the LAVs in the Attr objects (although it would reduce the number of objects, and be in line with having the different SVG Element objects descending from AbstractElement) is that sometimes animations will target attributes that haven’t been specified in the DOM, and thus won’t have an Attr object. But we could have a combination of the two: have the bridge object keep the primary reference to the LAV, and the Attr object keep a (weak?) reference so that name-based lookups (e.g. those used by most of the methods in AnimationTarget) can be fast and reuse AbstractElement’s NamedNodeHashMap. Since I’d like to get rid of all of the string comparisons that are currently done in the implementation of the AnimationTarget methods on the SVG DOM classes, the LAV objects would have to know the type of value they represent, whether it is animatable, whether it is additive, etc. To avoid storing that sort of information in each instance, would having different classes for the different attributes be ok, or would that be overkill? Cameron -- Cameron McCormack, http://mcc.id.au/ xmpp:[EMAIL PROTECTED] ▪ ICQ 26955922 ▪ MSN [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
