Bug 46124 - AnimationEngine.removeAnimation doesn't clean up the "animation sandwich" correctly.
AnimationEngine.removeAnimation doesn't clean up the "animation sandwich" cor...
Status: RESOLVED FIXED
Product: Batik - Now in Jira
Classification: Unclassified
Component: SVG Viewer
1.7
PC Windows XP
: P2 major
: ---
Assigned To: Batik Developer's Mailing list
:
: 44439 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2008-10-29 15:47 UTC by Steve Chui
Modified: 2009-01-10 04:26 UTC (History)
1 user (show)



Attachments
Test case created from proposed solution (2.01 KB, image/svg+xml)
2008-10-31 04:35 UTC, Helder Magalhães
Details
Patch created from proposed solution (1.40 KB, patch)
2008-10-31 04:36 UTC, Helder Magalhães
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Chui 2008-10-29 15:47:41 UTC
When a user tries to remove an animation node, the removeAnimation function fails to remove the associated "animation sandwich" object that left in the TargetInfo.otherAnimations internal storage. The sandwich is referring to a 'null' animation which cause null pointer exception when the viewer tries to execute all animations against the target element.

========= Here is the exception ===========
java.lang.NullPointerException
	at org.apache.batik.anim.AnimationEngine.tick(AnimationEngine.java:389)
	at org.apache.batik.bridge.SVGAnimationEngine.access$601(SVGAnimationEngine.java:99)
	at org.apache.batik.bridge.SVGAnimationEngine$AnimationTickRunnable.run(SVGAnimationEngine.java:859)
	at org.apache.batik.util.RunnableQueue.run(RunnableQueue.java:237)
	at java.lang.Thread.run(Unknown Source)

========== Here is a test case ============
	var activeElement = svgDoc.getElementById("theElement");
        var anim = svgDoc.createElementNS("http://www.w3.org/2000/svg", "animateMotion");
        anim.setAttributeNS(null, "id", "1");
        anim.setAttributeNS(null, "dur", "1");
        anim.setAttributeNS(null, "path", "m 0, 0 L 0, 5");
       	anim.setAttributeNS(null, "begin", "indefinite");
       	anim.setAttributeNS(null, "fill", "freeze");
       	activeElement.appendChild(anim);
       	
       	var anim2 = svgDoc.createElementNS("http://www.w3.org/2000/svg", "animate");
        anim2.setAttributeNS(null, "id", "2");
        anim2.setAttributeNS(null, "dur", "1");
        anim2.setAttributeNS(null, "values", "0;26;51;76;102");
        anim2.setAttributeNS(null, "attributeName", "x");
       	anim2.setAttributeNS(null, "begin", "indefinite");
       	anim2.setAttributeNS(null, "fill", "freeze");
       	activeElement.appendChild(anim2);
       	
       	activeElement.removeChild(anim);
       	
       	anim2.beginElement();

=========== here is the purposed solution ==============
    protected Sandwich removeSandwich(AnimationTarget target, short type,
			String ns, String an) {
		TargetInfo info = getTargetInfo(target);
		Sandwich sandwich;
		if (type == ANIM_TYPE_XML) {
			sandwich = (Sandwich) info.xmlAnimations.remove(ns, an);
		} else if (type == ANIM_TYPE_CSS) {
			sandwich = (Sandwich) info.cssAnimations.remove(an);
		} else {
			sandwich = (Sandwich) info.otherAnimations.remove(an);
		}
		return sandwich;
	}

    public void removeAnimation(AbstractAnimation anim) {
         timedDocumentRoot.removeChild(anim.getTimedElement());
        AbstractAnimation nextHigher = anim.higherAnimation;
        if (nextHigher != null) {
            nextHigher.markDirty();
        }
        moveToBottom(anim);
        if (anim.higherAnimation != null) {
            anim.higherAnimation.lowerAnimation = null;
        }
        AnimationInfo animInfo = getAnimationInfo(anim);
        Sandwich sandwich = getSandwich(animInfo.target, animInfo.type,
                                        animInfo.attributeNamespaceURI,
                                        animInfo.attributeLocalName);
        if (sandwich.animation == anim) {
            sandwich.animation = null;
            sandwich.lowestAnimation = null;
            sandwich.shouldUpdate = true;
            removeSandwich(animInfo.target, animInfo.type,
                                        animInfo.attributeNamespaceURI,
                                        animInfo.attributeLocalName);
        }
    }
Comment 1 Helder Magalhães 2008-10-31 04:35:14 UTC
Created attachment 22812 [details]
Test case created from proposed solution

Test case created from proposed solution which, unfortunately, was incomplete. ;-)

Tried to guess the missing information and fixed the test case (reviewing it would be helpful).
Comment 2 Helder Magalhães 2008-10-31 04:36:05 UTC
Created attachment 22813 [details]
Patch created from proposed solution

Patch created from proposed solution which, unfortunately, didn't refer to which file the changes referred to. ;-)

Minor improvements such as indent corrections and add missing documentation.

Confirmed that the patch fixes the exception compared to Batik 1.7 release. :-)
Comment 3 Cameron McCormack 2008-11-05 19:55:18 UTC
Hi Steve and Helder.

Thanks for the bug report, and the attached patch and test.  The proposed solution doesn't completely solve the bug, however.  When an animation is removed, its effect needs to be removed also.  This is meant to be achieved by leaving the Sandwich object in there, with no animations in it but with shouldUpdate = true.

The ANIM_TYPE_OTHER case is just missing a check in tick() to ensure sandwich.animation is not null (as is checked for XML/CSS animations earlier in the method).

This is now fixed in r711765.
Comment 4 Helder Magalhães 2009-01-10 04:26:59 UTC
*** Bug 44439 has been marked as a duplicate of this bug. ***