Changeset 128131 in webkit


Ignore:
Timestamp:
Sep 10, 2012 5:07:26 PM (12 years ago)
Author:
pdr@google.com
Message:

Source/WebCore: Remove unnecessary codepaths in SMILTimeContainer::updateAnimations
​https://bugs.webkit.org/show_bug.cgi?id=96224

Reviewed by Nikolas Zimmermann.

This change removes two sources of unnecessary code in
SMILTimeContainer::updateAnimations:

1) After r117711 we now accumulate the result of multiple

animations into the first _contributing_ animation
element. As a result it is no longer necessary to
track both which elements are contributing AND which elements
we are storing results into. Both cases are now handled
together with resultsElements.

2) r32044 added a second sort of the animation elements

in order to process animateTransform last. This change
was added 4 years ago, before we correctly handled <use>
and the instance tree, and I think the extra sort is no
longer necessary. A test has been added to ensure this
is the case.

This change also does a minor cleanup of resultsElements. Previously,
we added animation elements to resultsElements and then removed them
if the animation element did not contribute. After this change, we
only add to resultsElements (no more add-then-remove).

Test: svg/animations/use-animate-transform-and-position.html

  • svg/animation/SMILTimeContainer.cpp:

(WebCore::SMILTimeContainer::sortByPriority):
(WebCore::SMILTimeContainer::updateAnimations):

LayoutTests: Remove unnecessary work in SMILTimeContainer::updateAnimations
​https://bugs.webkit.org/show_bug.cgi?id=96224

Reviewed by Nikolas Zimmermann.

This change should have no functional differences but a test
is being added to show that. SMILTimeContainer::updateAnimations
contained a comment explaining why a animateTransform needed to be
processed last but that is no longer the case (as the test shows).

  • svg/animations/script-tests/use-animate-transform-and-position.js: Added.

(sample1):
(sample2):
(sample3):
(sample4):
(sample5):
(executeTest):

  • svg/animations/use-animate-transform-and-position-expected.txt: Added.
  • svg/animations/use-animate-transform-and-position.html: Added.
Location:
trunk
Files:
3 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r128129 r128131  
     12012-09-10  Philip Rogers  <pdr@google.com>
     2
     3        Remove unnecessary work in SMILTimeContainer::updateAnimations
     4        https://bugs.webkit.org/show_bug.cgi?id=96224
     5
     6        Reviewed by Nikolas Zimmermann.
     7
     8        This change should have no functional differences but a test
     9        is being added to show that. SMILTimeContainer::updateAnimations
     10        contained a comment explaining why a animateTransform needed to be
     11        processed last but that is no longer the case (as the test shows).
     12
     13        * svg/animations/script-tests/use-animate-transform-and-position.js: Added.
     14        (sample1):
     15        (sample2):
     16        (sample3):
     17        (sample4):
     18        (sample5):
     19        (executeTest):
     20        * svg/animations/use-animate-transform-and-position-expected.txt: Added.
     21        * svg/animations/use-animate-transform-and-position.html: Added.
     22
    1232012-09-10  Jer Noble  <jer.noble@apple.com>
    224

  • trunk/Source/WebCore/ChangeLog

    r128130 r128131  
     12012-09-10  Philip Rogers  <pdr@google.com>
     2
     3        Remove unnecessary codepaths in SMILTimeContainer::updateAnimations
     4        https://bugs.webkit.org/show_bug.cgi?id=96224
     5
     6        Reviewed by Nikolas Zimmermann.
     7
     8        This change removes two sources of unnecessary code in
     9        SMILTimeContainer::updateAnimations:
     10            1) After r117711 we now accumulate the result of multiple
     11               animations into the first _contributing_ animation
     12               element. As a result it is no longer necessary to
     13               track both which elements are contributing AND which elements
     14               we are storing results into. Both cases are now handled
     15               together with resultsElements.
     16
     17            2) r32044 added a second sort of the animation elements
     18               in order to process animateTransform last. This change
     19               was added 4 years ago, before we correctly handled <use>
     20               and the instance tree, and I think the extra sort is no
     21               longer necessary. A test has been added to ensure this
     22               is the case.
     23
     24        This change also does a minor cleanup of resultsElements. Previously,
     25        we added animation elements to resultsElements and then removed them
     26        if the animation element did not contribute. After this change, we
     27        only add to resultsElements (no more add-then-remove).
     28
     29        Test: svg/animations/use-animate-transform-and-position.html
     30
     31        * svg/animation/SMILTimeContainer.cpp:
     32        (WebCore::SMILTimeContainer::sortByPriority):
     33        (WebCore::SMILTimeContainer::updateAnimations):
     34
    1352012-09-10  Ojan Vafai  <ojan@chromium.org>
    236

  • trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp

    r127474 r128131  
    205205    std::sort(smilElements.begin(), smilElements.end(), PriorityCompare(elapsed));
    206206}
    207    
    208 static bool applyOrderSortFunction(SVGSMILElement* a, SVGSMILElement* b)
    209 {
    210     if (!a->hasTagName(SVGNames::animateTransformTag) && b->hasTagName(SVGNames::animateTransformTag))
    211         return true;
    212     return false;
    213 }
    214    
    215 static void sortByApplyOrder(Vector<SVGSMILElement*>& smilElements)
    216 {
    217     std::sort(smilElements.begin(), smilElements.end(), applyOrderSortFunction);
    218 }
    219207
    220208void SMILTimeContainer::updateAnimations(SMILTime elapsed, bool seekToTime)
    … …  
    230218    // have higher priority.
    231219    sortByPriority(toAnimate, elapsed);
    232    
     220
    233221    // Calculate animation contributions.
    234222    typedef pair<SVGElement*, QualifiedName> ElementAttributePair;
    235     typedef HashMap<ElementAttributePair, RefPtr<SVGSMILElement> > ResultElementMap;
     223    typedef HashMap<ElementAttributePair, SVGSMILElement*> ResultElementMap;
    236224    ResultElementMap resultsElements;
    237     HashSet<SVGSMILElement*> contributingElements;
    238225    for (unsigned n = 0; n < toAnimate.size(); ++n) {
    239226        SVGSMILElement* animation = toAnimate[n];
    … …  
    243230        if (!targetElement)
    244231            continue;
    245        
     232
    246233        QualifiedName attributeName = animation->attributeName();
    247234        if (attributeName == anyQName()) {
    … …  
    251238                continue;
    252239        }
    253        
     240
    254241        // Results are accumulated to the first animation that animates and contributes to a particular element/attribute pair.
    255242        ElementAttributePair key(targetElement, attributeName);
    256         SVGSMILElement* resultElement = resultsElements.get(key).get();
    257         bool accumulatedResultElement = false;
     243        SVGSMILElement* resultElement = resultsElements.get(key);
    258244        if (!resultElement) {
    259245            if (!animation->hasValidAttributeType())
    260246                continue;
    261247            resultElement = animation;
     248        } else
     249            ASSERT(resultElement != animation);
     250
     251        // This will calculate the contribution from the animation and add it to the resultsElement.
     252        if (animation->progress(elapsed, resultElement, seekToTime) && resultElement == animation)
    262253            resultsElements.add(key, resultElement);
    263             accumulatedResultElement = true;
    264         }
    265 
    266         // This will calculate the contribution from the animation and add it to the resultsElement.
    267         if (animation->progress(elapsed, resultElement, seekToTime))
    268             contributingElements.add(resultElement);
    269         else if (accumulatedResultElement)
    270             resultsElements.remove(key);
    271254
    272255        SMILTime nextFireTime = animation->nextProgressTime();
    … …  
    274257            earliersFireTime = min(nextFireTime, earliersFireTime);
    275258    }
    276    
    277     Vector<SVGSMILElement*> animationsToApply;
     259
     260    unsigned resultsToApplySize = resultsElements.size();
     261    if (!resultsToApplySize) {
     262        startTimer(earliersFireTime, animationFrameDelay);
     263        return;
     264    }
     265
     266    // Apply results to target elements.
    278267    ResultElementMap::iterator end = resultsElements.end();
    279     for (ResultElementMap::iterator it = resultsElements.begin(); it != end; ++it) {
    280         SVGSMILElement* animation = it->second.get();
    281         if (contributingElements.contains(animation))
    282             animationsToApply.append(animation);
    283     }
    284 
    285     unsigned animationsToApplySize = animationsToApply.size();
    286     if (!animationsToApplySize) {
    287         startTimer(earliersFireTime, animationFrameDelay);
    288         return;
    289     }
    290 
    291     // Sort <animateTranform> to be the last one to be applied. <animate> may change transform attribute as
    292     // well (directly or indirectly by modifying <use> x/y) and this way transforms combine properly.
    293     sortByApplyOrder(animationsToApply);
    294 
    295     // Apply results to target elements.
    296     for (unsigned i = 0; i < animationsToApplySize; ++i)
    297         animationsToApply[i]->applyResultsToTarget();
     268    for (ResultElementMap::iterator it = resultsElements.begin(); it != end; ++it)
     269        it->second->applyResultsToTarget();
    298270
    299271    startTimer(earliersFireTime, animationFrameDelay);
Note: See TracChangeset for help on using the changeset viewer.