Web Animations CSS: Snapshot missing start/end Keyframes and remove duplicates

Source/core/css/resolver/StyleResolver.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2004-2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Nicholas Shanks (webkit@nickshanks.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007, 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
@@ skipping 745 matching lines @@
             hundredPercentKeyframe = StyleKeyframe::create().leakRef();
             hundredPercentKeyframe->setKeyText("100%");
         }
         KeyframeValue keyframeValue(1, 0);
         keyframeValue.setStyle(styleForKeyframe(e, elementStyle, hundredPercentKeyframe));
         keyframeValue.addProperties(hundredPercentKeyframe->properties());
         list.insert(keyframeValue);
     }
 }
 
+bool compareOffsets(const RefPtr<Keyframe>& a, const RefPtr<Keyframe>& b)
+{
+    return a->offset() < b->offset();
+}

[Steve Block, 2013/08/06 06:30:20]

We already have this helper in KeyframeAnimationEffect. Maybe we should make it
a static member of Keyframe so it can be shared?

[dstockwell, 2013/08/06 07:14:28]

Done.

+
 void StyleResolver::resolveKeyframes(Element* element, const RenderStyle* style, const StringImpl* name, KeyframeAnimationEffect::KeyframeVector& keyframes)
 {
     const StyleRuleKeyframes* keyframesRule = matchScopedKeyframesRule(element, name);
     if (!keyframesRule)
         return;
 
     // Construct and populate the style for each keyframe
     const Vector<RefPtr<StyleKeyframe> >& styleKeyframes = keyframesRule->keyframes();
     for (unsigned i = 0; i < styleKeyframes.size(); ++i) {
         const StyleKeyframe* styleKeyframe = styleKeyframes[i].get();
         RefPtr<RenderStyle> keyframeStyle = styleForKeyframe(element, style, styleKeyframe);
-
         Vector<float> offsets;
         styleKeyframe->getKeys(offsets);
         for (size_t j = 0; j < offsets.size(); ++j) {
             RefPtr<Keyframe> keyframe = Keyframe::create();
             keyframe->setOffset(offsets[j]);
             const StylePropertySet* properties = styleKeyframe->properties();
             // FIXME: AnimatableValues should be shared between the keyframes at different offsets.
             for (unsigned k = 0; k < properties->propertyCount(); k++) {
                 CSSPropertyID property = properties->propertyAt(k).id();
-                // FIXME: CSSValue needs to be resolved.
                 keyframe->setPropertyValue(property, CSSAnimatableValueFactory::create(property, keyframeStyle.get()).get());
             }
             keyframes.append(keyframe);
         }
     }
 
-    // FIXME: If the 0% keyframe is missing, create it (but only if there is at least one other keyframe)
-    // FIXME: If the 100% keyframe is missing, create it (but only if there is at least one other keyframe)
+    if (keyframes.isEmpty())
+        return;
+
+    // Remove duplicate keyframes. In CSS the last keyframe at a given offset takes priority.
+    std::stable_sort(keyframes.begin(), keyframes.end(), compareOffsets);
+    unsigned duplicates = 0;

[Timothy Loh, 2013/08/05 03:44:59]

I think this should work:
    keyframes.shrink(std::unique(keyframes.begin(), keyframes.end()) -
keyframes.begin());

[dstockwell, 2013/08/05 05:52:18]

Tim and I discussed off thread, and while something like this should work (keep
the last as opposed to the first duplicate):

keyframes.remove(0, keyframes.rend() - std::unique(keyframes.rbegin(),
keyframes.rend(), equalOffsets))

It's not clear if it's a win, and still appears to be the first use of
std::unique in blink??

[Steve Block, 2013/08/06 06:30:20]

Does unique() work with reverse iterators?

+    for (size_t i = 1; i < keyframes.size(); i++) {
+        if (keyframes[i]->offset() == keyframes[i - 1 - duplicates]->offset()) {
+            keyframes[i - 1 - duplicates] = keyframes[i];
+            ++duplicates;
+        } else if (duplicates) {
+            keyframes[i - duplicates] = keyframes[i];

[Steve Block, 2013/08/06 06:30:20]

I think this would be easier to follow if you used a local 'size_t targetIndex'
or 'iterator targetSlot' to represent the slot in the array that the current
keyframe will be moved to.

[dstockwell, 2013/08/06 07:14:28]

Done.

+        }
+    }
+    keyframes.shrink(keyframes.size() - duplicates);
+
+    bool missingStartKeyframe = !keyframes[0]->offset;

[Steve Block, 2013/08/06 06:30:20]

I think you have this inverted.

Also, this local isn't a keyframe, so should probably be isStartKeyframeMissing

[dstockwell, 2013/08/06 07:14:28]

Fixed. Be nice if we could write != 0 :)
Done.

+    bool missingEndKeyframe = keyframes[keyframes.size() - 1]->offset != 1;
+    if (!missingStartKeyframe && !missingEndKeyframe)
+        return;
+
+    HashSet<CSSPropertyID> allProperties;
+    for (unsigned i = 0; i < styleKeyframes.size(); ++i) {

[Steve Block, 2013/08/06 06:30:20]

I think you should use size_t in loops like this, as that what Vector::size()
and Vector::operator[] use.

[dstockwell, 2013/08/06 07:14:28]

Done.

+        const StyleKeyframe* styleKeyframe = styleKeyframes[i].get();
+        Vector<float> offsets;
+        styleKeyframe->getKeys(offsets);
+        const StylePropertySet* properties = styleKeyframe->properties();
+        for (unsigned j = 0; j < properties->propertyCount(); ++j) {
+            allProperties.add(properties->propertyAt(j).id());
+        }
+    }
+
+    if (missingStartKeyframe) {
+        RefPtr<Keyframe> keyframe = Keyframe::create();
+        keyframe->setOffset(0);
+        for (HashSet<CSSPropertyID>::const_iterator iter = allProperties.begin(); iter != allProperties.end(); ++iter)
+            keyframe->setPropertyValue(*iter, CSSAnimatableValueFactory::create(*iter, style).get());
+        keyframes.prepend(keyframe);
+    }
+
+    if (missingEndKeyframe) {
+        RefPtr<Keyframe> keyframe = Keyframe::create();
+        keyframe->setOffset(1);
+        for (HashSet<CSSPropertyID>::const_iterator iter = allProperties.begin(); iter != allProperties.end(); ++iter)
+            keyframe->setPropertyValue(*iter, CSSAnimatableValueFactory::create(*iter, style).get());
+        keyframes.append(keyframe);
+    }
 }
 
 const StylePropertySet* StyleResolver::firstKeyframeStyles(const Element* element, const StringImpl* animationName)
 {
     const StyleRuleKeyframes* keyframesRule = matchScopedKeyframesRule(element, animationName);
     if (!keyframesRule)
         return 0;
 
     // Find the last keyframe at offset 0
     const StyleKeyframe* firstKeyframe = 0;
@@ skipping 629 matching lines @@
         m_matchedPropertiesSearches, m_matchedPropertiesHit, m_matchedPropertiesSharedInheritedHit, m_matchedPropertiesToCache, m_matchedPropertiesEnteredIntoCache);
 
     fprintf(stderr, "Total:\n");
     printStyleStats(m_totalSearches, m_totalElementsEligibleForSharing, m_totalStylesShared, m_totalSearchFoundSiblingForSharing, m_totalSearchesMissedSharing,
         m_totalMatchedPropertiesSearches, m_totalMatchedPropertiesHit, m_totalMatchedPropertiesSharedInheritedHit, m_totalMatchedPropertiesToCache, m_totalMatchedPropertiesEnteredIntoCache);
     fprintf(stderr, "--------------------------------------------------------------------------------\n");
 }
 #endif
 
 } // namespace WebCore