Skip to content

Commit

Permalink
Encapsulate globals in CSSPropertyAnimation.cpp
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=121205

Reviewed by Antti Koivisto.

Encapsulated the globals inside a newly added CSSPropertyAnimationWrapperMap. Also removed the circular
dependency from ShorthandPropertyWrapper's constructor to CSSPropertyAnimationWrapperMap::instance().
The circular dependency still exists in ensurePropertyMap but I'm going to remove it in the bug 121199.

* page/animation/CSSPropertyAnimation.cpp:
(WebCore::ShorthandPropertyWrapper::ShorthandPropertyWrapper): Takes a Vector of longhand wrappers instead of
calling wrapperForProperty in the middle of constructing the very table. This circular dependency is now
encapsulated in CSSPropertyAnimationWrapperMap::ensurePropertyMap.
(WebCore::CSSPropertyAnimationWrapperMap::instance): Added.
(WebCore::CSSPropertyAnimationWrapperMap::wrapperForProperty): Renamed from WebCore::wrapperForProperty.
(WebCore::CSSPropertyAnimationWrapperMap::wrapperForIndex): Added.
(WebCore::CSSPropertyAnimationWrapperMap::size): Added.
(WebCore::CSSPropertyAnimationWrapperMap::addPropertyWrapper): Renamed from WebCore::addPropertyWrapper. Also
cleaned up boolean logics to use early exits instead of nested ifs.
(WebCore::CSSPropertyAnimationWrapperMap::addShorthandProperties): Renamed from WebCore::addShorthandProperties.
(WebCore::CSSPropertyAnimationWrapperMap::ensurePropertyMap): Renamed from WebCore::ensurePropertyMap.
Added an alias gPropertyWrappers for m_propertyWrappers; this aliasing will be removed in the bug 121199.
(WebCore::CSSPropertyAnimation::blendProperties):
(WebCore::CSSPropertyAnimation::animationOfPropertyIsAccelerated):
(WebCore::CSSPropertyAnimation::animatableShorthandsAffectingProperty):
(WebCore::CSSPropertyAnimation::propertiesEqual):
(WebCore::CSSPropertyAnimation::getPropertyAtIndex):
(WebCore::CSSPropertyAnimation::getNumProperties):
* page/animation/CSSPropertyAnimation.h:
* rendering/style/RenderStyle.h:


Canonical link: https://commits.webkit.org/139204@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@155647 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@rniwa
rniwa committed Sep 12, 2013
1 parent 3258ab0 commit 999ffd3
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 64 deletions.
33 changes: 33 additions & 0 deletions Source/WebCore/ChangeLog
View file
@@ -1,3 +1,36 @@
2013-09-11 Ryosuke Niwa <rniwa@webkit.org>

Encapsulate globals in CSSPropertyAnimation.cpp
https://bugs.webkit.org/show_bug.cgi?id=121205

Reviewed by Antti Koivisto.

Encapsulated the globals inside a newly added CSSPropertyAnimationWrapperMap. Also removed the circular
dependency from ShorthandPropertyWrapper's constructor to CSSPropertyAnimationWrapperMap::instance().
The circular dependency still exists in ensurePropertyMap but I'm going to remove it in the bug 121199.

* page/animation/CSSPropertyAnimation.cpp:
(WebCore::ShorthandPropertyWrapper::ShorthandPropertyWrapper): Takes a Vector of longhand wrappers instead of
calling wrapperForProperty in the middle of constructing the very table. This circular dependency is now
encapsulated in CSSPropertyAnimationWrapperMap::ensurePropertyMap.
(WebCore::CSSPropertyAnimationWrapperMap::instance): Added.
(WebCore::CSSPropertyAnimationWrapperMap::wrapperForProperty): Renamed from WebCore::wrapperForProperty.
(WebCore::CSSPropertyAnimationWrapperMap::wrapperForIndex): Added.
(WebCore::CSSPropertyAnimationWrapperMap::size): Added.
(WebCore::CSSPropertyAnimationWrapperMap::addPropertyWrapper): Renamed from WebCore::addPropertyWrapper. Also
cleaned up boolean logics to use early exits instead of nested ifs.
(WebCore::CSSPropertyAnimationWrapperMap::addShorthandProperties): Renamed from WebCore::addShorthandProperties.
(WebCore::CSSPropertyAnimationWrapperMap::ensurePropertyMap): Renamed from WebCore::ensurePropertyMap.
Added an alias gPropertyWrappers for m_propertyWrappers; this aliasing will be removed in the bug 121199.
(WebCore::CSSPropertyAnimation::blendProperties):
(WebCore::CSSPropertyAnimation::animationOfPropertyIsAccelerated):
(WebCore::CSSPropertyAnimation::animatableShorthandsAffectingProperty):
(WebCore::CSSPropertyAnimation::propertiesEqual):
(WebCore::CSSPropertyAnimation::getPropertyAtIndex):
(WebCore::CSSPropertyAnimation::getNumProperties):
* page/animation/CSSPropertyAnimation.h:
* rendering/style/RenderStyle.h:

2013-09-12 Anders Carlsson <andersca@apple.com>

SharedBuffer::createNSData should return a RetainPtr<NSData>
Expand Down
150 changes: 89 additions & 61 deletions Source/WebCore/page/animation/CSSPropertyAnimation.cpp
View file
Expand Up @@ -401,32 +401,6 @@ class AnimationPropertyWrapperBase {
CSSPropertyID m_prop;
};

static int gPropertyWrapperMap[numCSSProperties];
static const int cInvalidPropertyWrapperIndex = -1;
static Vector<AnimationPropertyWrapperBase*>* gPropertyWrappers = 0;

static void addPropertyWrapper(CSSPropertyID propertyID, AnimationPropertyWrapperBase* wrapper)
{
int propIndex = propertyID - firstCSSProperty;

ASSERT(gPropertyWrapperMap[propIndex] == cInvalidPropertyWrapperIndex);

unsigned wrapperIndex = gPropertyWrappers->size();
gPropertyWrappers->append(wrapper);
gPropertyWrapperMap[propIndex] = wrapperIndex;
}

static AnimationPropertyWrapperBase* wrapperForProperty(CSSPropertyID propertyID)
{
int propIndex = propertyID - firstCSSProperty;
if (propIndex >= 0 && propIndex < numCSSProperties) {
int wrapperIndex = gPropertyWrapperMap[propIndex];
if (wrapperIndex >= 0)
return (*gPropertyWrappers)[wrapperIndex];
}
return 0;
}

template <typename T>
class PropertyWrapperGetter : public AnimationPropertyWrapperBase {
public:
Expand Down Expand Up @@ -982,14 +956,10 @@ class FillLayersPropertyWrapper : public AnimationPropertyWrapperBase {

class ShorthandPropertyWrapper : public AnimationPropertyWrapperBase {
public:
ShorthandPropertyWrapper(CSSPropertyID property, const StylePropertyShorthand& shorthand)
ShorthandPropertyWrapper(CSSPropertyID property, Vector<AnimationPropertyWrapperBase*> longhandWrappers)
: AnimationPropertyWrapperBase(property)
{
for (unsigned i = 0; i < shorthand.length(); ++i) {
AnimationPropertyWrapperBase* wrapper = wrapperForProperty(shorthand.properties()[i]);
if (wrapper)
m_propertyWrappers.append(wrapper);
}
m_propertyWrappers.swap(longhandWrappers);
}

virtual bool isShorthandWrapper() const { return true; }
Expand Down Expand Up @@ -1105,7 +1075,66 @@ class PropertyWrapperSVGPaint : public AnimationPropertyWrapperBase {
};
#endif

static void addShorthandProperties()
class CSSPropertyAnimationWrapperMap {
public:
static CSSPropertyAnimationWrapperMap& instance()
{
// FIXME: This data is never destroyed. Maybe we should ref count it and toss it when the last AnimationController is destroyed?
DEFINE_STATIC_LOCAL(OwnPtr<CSSPropertyAnimationWrapperMap>, map, ());
if (!map) {
map = adoptPtr(new CSSPropertyAnimationWrapperMap);
map->ensurePropertyMap(); // FIXME: ensurePropertyMap() calls instance() inside addShorthandProperties().
}
return *map;
}

AnimationPropertyWrapperBase* wrapperForProperty(CSSPropertyID propertyID)
{
int propIndex = propertyID - firstCSSProperty;
if (propertyID < firstCSSProperty || propertyID > lastCSSProperty)
return 0;

unsigned wrapperIndex = m_propertyToIdMap[propIndex];
if (wrapperIndex == cInvalidPropertyWrapperIndex)
return 0;

return m_propertyWrappers[wrapperIndex];
}

AnimationPropertyWrapperBase* wrapperForIndex(unsigned index)
{
ASSERT(index < m_propertyWrappers.size());
return m_propertyWrappers[index];
}

unsigned size()
{
return m_propertyWrappers.size();
}


private:
void addShorthandProperties();
void ensurePropertyMap();

void addPropertyWrapper(CSSPropertyID propertyID, AnimationPropertyWrapperBase* wrapper)
{
int propIndex = propertyID - firstCSSProperty;

ASSERT(m_propertyToIdMap[propIndex] == cInvalidPropertyWrapperIndex);

unsigned wrapperIndex = m_propertyWrappers.size();
m_propertyWrappers.append(wrapper);
m_propertyToIdMap[propIndex] = wrapperIndex;
}

Vector<AnimationPropertyWrapperBase*> m_propertyWrappers;
unsigned m_propertyToIdMap[numCSSProperties];

const unsigned cInvalidPropertyWrapperIndex = UINT_MAX;
};

void CSSPropertyAnimationWrapperMap::addShorthandProperties()
{
static const CSSPropertyID animatableShorthandProperties[] = {
CSSPropertyBackground, // for background-color, background-position, background-image
Expand All @@ -1130,21 +1159,27 @@ static void addShorthandProperties()
CSSPropertyWebkitTransformOrigin
};

CSSPropertyAnimationWrapperMap& map = CSSPropertyAnimationWrapperMap::instance();

for (size_t i = 0; i < WTF_ARRAY_LENGTH(animatableShorthandProperties); ++i) {
CSSPropertyID propertyID = animatableShorthandProperties[i];
StylePropertyShorthand shorthand = shorthandForProperty(propertyID);
if (shorthand.length() > 0)
addPropertyWrapper(propertyID, new ShorthandPropertyWrapper(propertyID, shorthand));
if (!shorthand.length())
continue;

Vector<AnimationPropertyWrapperBase*> longhandWrappers;
for (unsigned i = 0; i < shorthand.length(); ++i) {
if (AnimationPropertyWrapperBase* wrapper = map.wrapperForProperty(shorthand.properties()[i]))
longhandWrappers.append(wrapper);
}

map.addPropertyWrapper(propertyID, new ShorthandPropertyWrapper(propertyID, longhandWrappers));
}
}

void CSSPropertyAnimation::ensurePropertyMap()
void CSSPropertyAnimationWrapperMap::ensurePropertyMap()
{
// FIXME: This data is never destroyed. Maybe we should ref count it and toss it when the last AnimationController is destroyed?
if (gPropertyWrappers)
return;

gPropertyWrappers = new Vector<AnimationPropertyWrapperBase*>();
Vector<AnimationPropertyWrapperBase*>* gPropertyWrappers = &m_propertyWrappers; // FIXME: Remove this aliasing.

// build the list of property wrappers to do the comparisons and blends
gPropertyWrappers->append(new PropertyWrapper<Length>(CSSPropertyLeft, &RenderStyle::left, &RenderStyle::setLeft));
Expand Down Expand Up @@ -1307,14 +1342,14 @@ void CSSPropertyAnimation::ensurePropertyMap()

// Make sure unused slots have a value
for (unsigned int i = 0; i < static_cast<unsigned int>(numCSSProperties); ++i)
gPropertyWrapperMap[i] = cInvalidPropertyWrapperIndex;
m_propertyToIdMap[i] = cInvalidPropertyWrapperIndex;

// First we put the non-shorthand property wrappers into the map, so the shorthand-building
// code can find them.
size_t n = gPropertyWrappers->size();
for (unsigned int i = 0; i < n; ++i) {
ASSERT((*gPropertyWrappers)[i]->property() - firstCSSProperty < numCSSProperties);
gPropertyWrapperMap[(*gPropertyWrappers)[i]->property() - firstCSSProperty] = i;
ASSERT(m_propertyWrappers[i]->property() - firstCSSProperty < numCSSProperties);
m_propertyToIdMap[m_propertyWrappers[i]->property() - firstCSSProperty] = i;
}

// Now add the shorthand wrappers.
Expand Down Expand Up @@ -1347,9 +1382,7 @@ bool CSSPropertyAnimation::blendProperties(const AnimationBase* anim, CSSPropert
{
ASSERT(prop != CSSPropertyInvalid);

ensurePropertyMap();

AnimationPropertyWrapperBase* wrapper = wrapperForProperty(prop);
AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::instance().wrapperForProperty(prop);
if (wrapper) {
wrapper->blend(anim, dst, a, b, progress);
#if USE(ACCELERATED_COMPOSITING)
Expand All @@ -1365,51 +1398,46 @@ bool CSSPropertyAnimation::blendProperties(const AnimationBase* anim, CSSPropert
#if USE(ACCELERATED_COMPOSITING)
bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(CSSPropertyID prop)
{
ensurePropertyMap();
AnimationPropertyWrapperBase* wrapper = wrapperForProperty(prop);
AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::instance().wrapperForProperty(prop);
return wrapper ? wrapper->animationIsAccelerated() : false;
}
#endif

// Note: this is inefficient. It's only called from pauseTransitionAtTime().
HashSet<CSSPropertyID> CSSPropertyAnimation::animatableShorthandsAffectingProperty(CSSPropertyID property)
{
ensurePropertyMap();
CSSPropertyAnimationWrapperMap& map = CSSPropertyAnimationWrapperMap::instance();

HashSet<CSSPropertyID> foundProperties;
for (int i = 0; i < getNumProperties(); ++i)
gatherEnclosingShorthandProperties(property, (*gPropertyWrappers)[i], foundProperties);
for (unsigned i = 0; i < map.size(); ++i)
gatherEnclosingShorthandProperties(property, map.wrapperForIndex(i), foundProperties);

return foundProperties;
}

bool CSSPropertyAnimation::propertiesEqual(CSSPropertyID prop, const RenderStyle* a, const RenderStyle* b)
{
ensurePropertyMap();

AnimationPropertyWrapperBase* wrapper = wrapperForProperty(prop);
AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::instance().wrapperForProperty(prop);
if (wrapper)
return wrapper->equals(a, b);
return true;
}

CSSPropertyID CSSPropertyAnimation::getPropertyAtIndex(int i, bool& isShorthand)
{
ensurePropertyMap();
CSSPropertyAnimationWrapperMap& map = CSSPropertyAnimationWrapperMap::instance();

if (i < 0 || i >= getNumProperties())
if (i < 0 || static_cast<unsigned>(i) >= map.size())
return CSSPropertyInvalid;

AnimationPropertyWrapperBase* wrapper = (*gPropertyWrappers)[i];
AnimationPropertyWrapperBase* wrapper = map.wrapperForIndex(i);
isShorthand = wrapper->isShorthandWrapper();
return wrapper->property();
}

int CSSPropertyAnimation::getNumProperties()
{
ensurePropertyMap();

return gPropertyWrappers->size();
return CSSPropertyAnimationWrapperMap::instance().size();
}

}
2 changes: 0 additions & 2 deletions Source/WebCore/page/animation/CSSPropertyAnimation.h
View file
Expand Up @@ -51,8 +51,6 @@ class CSSPropertyAnimation {

// Return true if we need to start software animation timers
static bool blendProperties(const AnimationBase*, CSSPropertyID, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress);
private:
static void ensurePropertyMap();
};

} // namespace WebCore
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/style/RenderStyle.h
View file
Expand Up @@ -127,7 +127,7 @@ class ContentData;
typedef Vector<RefPtr<RenderStyle>, 4> PseudoStyleCache;

class RenderStyle: public RefCounted<RenderStyle> {
friend class CSSPropertyAnimation; // Used by CSS animations. We can't allow them to animate based off visited colors.
friend class CSSPropertyAnimationWrapperMap; // Used by CSS animations. We can't allow them to animate based off visited colors.
friend class ApplyStyleCommand; // Editing has to only reveal unvisited info.
friend class DeprecatedStyleBuilder; // Sets members directly.
friend class EditingStyle; // Editing has to only reveal unvisited info.
Expand Down

0 comments on commit 999ffd3

Please sign in to comment.