Closed Bug 510139 Opened 10 years ago Closed 10 years ago

SVG SMIL: Un-animated attributes continue to be updated

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: birtles, Assigned: birtles)

Details

Attachments

(1 file)

Attached patch patch v1aSplinter Review
The operation of nsSMILAnimationController is sub-optimal. For an attribute that is no longer animated it will continue to grab the base value and apply it to the anim value.

Now that we have a facility for clearing the animation of an attribute, we ought to use it. So, if an attribute is no longer targetted by any active animation (including a frozen animation) we ought to clear its animation value and leave it alone.

The attached patch addresses this issue.

Operation is as follows:
1) Inactive animation functions are not added to the compositor table.
2) Therefore, if there is an attribute with no active (or frozen) animations targetting it, no entry will be created for it in the compositor table.
3) In Step 3 of nsSMILAnimationController::DoSample, mLastCompositorTable will be compared with the new compositor table and the fact that one of the compositors has dropped out will be noted
4) Hence ClearAnimationEffects will be called on the old compositor, thus the attribute will be cleared.

Subsequent animation samples will leave the attribute untouched.

Also, this patch renames nsSMILAnimationFunction::IsActive() to ...::IsActiveOrFrozen() for clarity (as discussed with dholbert).
Attachment #394198 - Flags: superreview?(roc)
Attachment #394198 - Flags: review?(dholbert)
Assignee: nobody → birtles
Status: NEW → ASSIGNED
Comment on attachment 394198 [details] [diff] [review]
patch v1a

Your comment made this sound like a bigger change than it is :-)
Attachment #394198 - Flags: superreview?(roc) → superreview+
Comment on attachment 394198 [details] [diff] [review]
patch v1a

Cool -- this looks like the AnimationController chunk of the old "patch v1a" from bug 506856, plus the IsActiveOrFrozen rename.

r=dholbert
Attachment #394198 - Flags: review?(dholbert) → review+
Passes on TryServer. Please check in.
Keywords: checkin-needed
Whiteboard: [needs landing]
Landed by roc: http://hg.mozilla.org/mozilla-central/rev/4919c19abb16
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.