[Webkit-unassigned] [Bug 19938] Improve AnimationController
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jul 7 21:07:23 PDT 2008
https://bugs.webkit.org/show_bug.cgi?id=19938
------- Comment #5 from simon.fraser at apple.com 2008-07-07 21:07 PDT -------
+++ b/WebCore/css/CSSComputedStyleDeclaration.cpp
+ case CSSPropertyWebkitTransitionProperty: {
+ RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
+ const Transition* t = style->transitions();
+ if (t) {
+ for ( ; t; t = t->next()) {
+ int prop = t->transitionProperty();
Isn't this t->property()? If not, why not?
diff --git a/WebCore/css/CSSParser.cpp b/WebCore/css/CSSParser.cpp
-PassRefPtr<CSSValue> CSSParser::parseTransitionRepeatCount()
+PassRefPtr<CSSValue> CSSParser::parseDuration()
{
CSSParserValue* value = m_valueList->current();
- if (value->id == CSSValueInfinite)
- return CSSPrimitiveValue::createIdentifier(value->id);
- if (validUnit(value, FInteger|FNonNeg, m_strict))
+ if (validUnit(value, FTime, m_strict))
return CSSPrimitiveValue::create(value->fValue,
(CSSPrimitiveValue::UnitTypes)value->unit);
return 0;
}
Shouldn't duration have the FInteger|FNonNeg flags?
diff --git a/WebCore/css/CSSStyleSelector.cpp
b/WebCore/css/CSSStyleSelector.cpp
index 36c652d..3b9a456 100644
--- a/WebCore/css/CSSStyleSelector.cpp
+++ b/WebCore/css/CSSStyleSelector.cpp
@@ -182,15 +182,15 @@ HANDLE_FILL_LAYER_INHERIT_AND_INITIAL(mask, Mask, prop,
Prop)
#define HANDLE_MASK_VALUE(prop, Prop, value) \
HANDLE_FILL_LAYER_VALUE(mask, Mask, prop, Prop, value)
-#define HANDLE_TRANSITION_INHERIT_AND_INITIAL(prop, Prop) \
+#define HANDLE_MULTILAYER_INHERIT_AND_INITIAL(layerType, LayerType, prop,
Prop) \
if (isInherit) { \
- Transition* currChild = m_style->accessTransitions(); \
- Transition* prevChild = 0; \
- const Transition* currParent = m_parentStyle->transitions(); \
+ LayerType* currChild = m_style->access##LayerType##s(); \
+ LayerType* prevChild = 0; \
I don't think the Transtion -> LayerType changes are necessary. We'll only
ever make Transition objects.
diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
void Document::updateRendering()
{
- if (hasChangedChild())
+ if (hasChangedChild()) {
recalcStyle(NoChange);
+ }
WebKit style is no parens around single line block.
+ // tell the animation controller that the style is all setup and it can
start animations
+ if (m_frame)
+ m_frame->animation()->styleIsSetup();
}
styleIsSetup() is a crappy method name. We should think of something better.
diff --git a/WebCore/dom/Element.cpp b/WebCore/dom/Element.cpp
index c83a7d2..81bb90d 100644
--- a/WebCore/dom/Element.cpp
+++ b/WebCore/dom/Element.cpp
@@ -751,6 +751,10 @@ void Element::recalcStyle(StyleChange change)
if (hasParentStyle && (change >= Inherit || changed())) {
RenderStyle *newStyle =
document()->styleSelector()->styleForElement(this);
StyleChange ch = diff(currentStyle, newStyle);
+
+ if (styleChangeType() == AnimatableStyleChange)
+ ch = Force;
+
if (ch == Detach || !currentStyle) {
if (attached())
detach();
@@ -784,9 +788,11 @@ void Element::recalcStyle(StyleChange change)
newStyle->setChildrenAffectedByDirectAdjacentRules();
}
- if (ch != NoChange)
- setRenderStyle(newStyle);
- else if (changed() && (document()->usesSiblingRules() ||
document()->usesDescendantRules())) {
+ if (ch != NoChange) {
+ if (newStyle) {
+ setRenderStyle(newStyle);
+ }
+ } else if (changed() && (document()->usesSiblingRules() ||
document()->usesDescendantRules())) {
// Although no change occurred, we use the new style so that the
cousin style sharing code won't get
// fooled into believing this style is the same. This is only
necessary if the document actually uses
// sibling/descendant rules, since otherwise it isn't possible for
ancestor styles to affect sharing of
@@ -800,7 +806,7 @@ void Element::recalcStyle(StyleChange change)
newStyle->deref(document()->renderArena());
if (change != Force) {
- if ((document()->usesDescendantRules() || hasPositionalRules) &&
styleChangeType() == FullStyleChange)
+ if ((document()->usesDescendantRules() || hasPositionalRules) &&
styleChangeType() >= FullStyleChange)
change = Force;
else
change = ch;
These changes are different from those we reviewed internally. They need to be
updated.
diff --git a/WebCore/dom/EventNames.h b/WebCore/dom/EventNames.h
index 872c6cf..d289829 100644
--- a/WebCore/dom/EventNames.h
+++ b/WebCore/dom/EventNames.h
@@ -119,6 +119,12 @@ namespace WebCore { namespace EventNames {
macro(progress) \
macro(stalled) \
\
+ macro(webkitAnimationEnd) \
+ macro(webkitAnimationStart) \
+ macro(webkitAnimationIteration) \
Do we want animation events in this patch?
diff --git a/WebCore/dom/Node.cpp b/WebCore/dom/Node.cpp
index c291b60..435d029 100644
--- a/WebCore/dom/Node.cpp
+++ b/WebCore/dom/Node.cpp
@@ -380,7 +380,7 @@ void Node::setChanged(StyleChangeType changeType)
if ((changeType != NoStyleChange) && !attached()) // changed compared to
what?
return;
- if (!(changeType == InlineStyleChange && m_styleChange ==
FullStyleChange))
+ if (changeType > (StyleChangeType)m_styleChange)
Also needs syncing with internally reviewed changes.
diff --git a/WebCore/dom/Node.h b/WebCore/dom/Node.h
index 60f7a9c..20370d0 100644
--- a/WebCore/dom/Node.h
+++ b/WebCore/dom/Node.h
@@ -60,7 +60,7 @@ struct NodeListsNodeData;
typedef int ExceptionCode;
-enum StyleChangeType { NoStyleChange, InlineStyleChange, FullStyleChange };
+enum StyleChangeType { NoStyleChange, InlineStyleChange, FullStyleChange,
AnimatableStyleChange };
Ditto. We decided on AnimationStyleChange.
diff --git a/WebCore/page/AnimationController.cpp
b/WebCore/page/AnimationController.cpp
index 444e6b4..97d92da 100644
--- a/WebCore/page/AnimationController.cpp
+++ b/WebCore/page/AnimationController.cpp
+class CompositeAnimation : public Noncopyable {
Add a comment to say what a CompositeAnimation is for.
+public:
+ CompositeAnimation(bool suspended, AnimationControllerPrivate*
animationController)
+ : m_suspended(suspended)
+ , m_animationController(animationController)
+ , m_numStyleIsSetupWaiters(0)
+ { }
+
+ ~CompositeAnimation()
+ {
+ deleteAllValues(m_transitions);
+ }
+
+ RenderStyle* animate(RenderObject*, RenderStyle* currentStyle,
RenderStyle* targetStyle);
+
+ void setAnimating(bool inAnimating);
+ bool animating();
+
+ bool hasAnimationForProperty(int prop) const { return
m_transitions.contains(prop); }
+
+ void resetTransitions(RenderObject*);
+ void resetAnimations(RenderObject*);
+
+ void cleanupFinishedAnimations(RenderObject*);
+
+ void setAnimationStartTime(double t);
+ void setTransitionStartTime(int property, double t);
Should we use CSSPropertyID rather than ints these days?
+ // Animations and Transitions go through the states below. When entering
the STARTED state
+ // the animation is started. This may or may not require deferred response
from the animator.
+ // If so, we stay in this state until that response is received (and it
returns the start time).
+ // Otherwise, we use the current time as the start time and go immediately
to the LOOPING or
+ // ENDING state.
+ enum AnimState {
+ STATE_NEW, // animation just created, animation
not running yet
+ STATE_START_WAIT_TIMER, // start timer running, waiting for
fire
+ STATE_START_WAIT_STYLE_SETUP, // waiting for style setup so we can
start animations
+ STATE_START_WAIT_RESPONSE, // animation started, waiting for
response
+ STATE_LOOPING, // response received, animation
running, loop timer running, waiting for fire
+ STATE_ENDING, // response received, animation
running, end timer running, waiting for fire
+ STATE_PAUSED_WAIT_TIMER, // animation in pause mode when
animation started
+ STATE_PAUSED_WAIT_RESPONSE, // animation paused when in STARTING
state
+ STATE_PAUSED_RUN, // animation paused when in LOOPING or
ENDING state
+ STATE_DONE // end timer fired, animation finished
and removed
+ };
+
+ enum AnimStateInput {
+ STATE_INPUT_MAKE_NEW, // reset back to new from any state
+ STATE_INPUT_START_ANIMATION, // animation requests a start
+ STATE_INPUT_RESTART_ANIMATION, // force a restart from any state
+ STATE_INPUT_START_TIMER_FIRED, // start timer fired
+ STATE_INPUT_STYLE_SETUP, // style is setup, ready to start
animating
+ STATE_INPUT_START_TIME_SET, // m_startTime was set
+ STATE_INPUT_LOOP_TIMER_FIRED, // loop timer fired
+ STATE_INPUT_END_TIMER_FIRED, // end timer fired
+ STATE_INPUT_PAUSE_OVERRIDE, // pause an animation due to override
+ STATE_INPUT_RESUME_OVERRIDE, // resume an overridden animation
+ STATE_INPUT_PLAY_STATE_RUNNING, // play state paused -> running
+ STATE_INPUT_PLAY_STATE_PAUSED, // play state running -> paused
+ STATE_INPUT_END_ANIMATION // force an end from any state
+ };
I don't think we need this complexity of states for the existing transitions.
+class ImplicitAnimation : public AnimationBase {
+public:
+ ImplicitAnimation(const Transition* transition, RenderObject* renderer,
CompositeAnimation* compAnim)
+ : AnimationBase(transition, renderer, compAnim)
+ , m_property(transition->transitionProperty())
+ , m_overridden(false)
+ , m_fromStyle(0)
+ , m_toStyle(0)
+ {
+ }
+
+ virtual ~ImplicitAnimation()
+ {
+ ASSERT(!m_fromStyle && !m_toStyle);
+
+ // If we were waiting for an end event, we need to finish the
animation to make sure no old
+ // animations stick around in the lower levels
+ if (waitingForEndEvent() && m_object)
+ ASSERT(0);
+ /* spma
+ m_object->transitionFinished(m_property);
+ */
spma?
+ case STATE_START_WAIT_STYLE_SETUP:
+ ASSERT(input == STATE_INPUT_STYLE_SETUP || input ==
STATE_INPUT_PLAY_STATE_PAUSED);
+
+ m_compAnim->setWaitingForStyleIsSetup(false);
+
+ if (input == STATE_INPUT_STYLE_SETUP) {
+ DSM_PRINT("STATE_START_WAIT_STYLE_SETUP",
"STATE_INPUT_STYLE_SETUP", "STATE_START_WAIT_RESPONSE");
+ // Start timer has fired, tell the animation to start and wait
for it to respond with start time
+ m_animState = STATE_START_WAIT_RESPONSE;
+
+ overrideAnimations();
+
+ // Send start event, if needed
+ onAnimationStart(0.0f); // the elapsedTime is always 0 here
+
+ // Start the animation
+ if (overridden() || !startAnimation(0)) {
+#ifdef DEBUG_STATE_MACHINE
+ fprintf(stderr, "*** startAnimation returned
false, setting start time immediately\n");
+#endif
+ // We're not going to get a startTime callback, so fire
the start time here
+ m_animState = STATE_START_WAIT_RESPONSE;
+ updateStateMachine(STATE_INPUT_START_TIME_SET,
currentTime());
+ }
+ else
+ m_waitedForResponse = true;
I'm not sure we can ever get into this state with the transitions we have here.
Much of this logic could be simplified.
+ // |this| may be deleted here if we came out of STATE_ENDING when we've
been called from timerFired()
Ick. Maybe we should RefCount<> these guys?
+void AnimationBase::animationEventDispatcherFired(HTMLElement* element, const
AtomicString& name, int property,
+ bool reset, const
AtomicString& eventType, double elapsedTime)
{
- double elapsedTime = currentTime() - m_startTime;
+ m_waitingForEndEvent = false;
- if (m_finished || !m_duration || elapsedTime >= m_duration)
- return 1.0;
+ // Keep an atomic string on the stack to keep it alive until we exit this
method
+ // (since dispatching the event may cause |this| to be deleted, therefore
removing
+ // the last ref to the atomic string).
+ AtomicString animName(name);
+ AtomicString animEventType(eventType);
+ // Make sure the element sticks around too
+ RefPtr<HTMLElement> elementRetainer(element);
+
+ ASSERT(!element || (element->document() &&
!element->document()->inPageCache()));
+ if (!element)
+ return;
+
+ // We didn't make the endAnimation call if we dispatched an event. This is
so
+ // we can call the animation event handler then remove the animation.
+ // We can't call endAnimation() here because it is specialized depending
on whether
+ // we are doing Transitions or Animations. So we do the equivalent to it
here.
+ if (animEventType == EventNames::webkitAnimationEndEvent) {
+ if (element->renderer()) {
+ // restore the original (unanimated) style
+ setChanged(element->renderer()->element());
+ }
+ }
+}
There's some reference to explicit animations here.
static inline TransformOperations blendFunc(const TransformOperations& from,
const TransformOperations& to, double progress)
@@ -237,7 +925,8 @@ static inline TransformOperations blendFunc(const
TransformOperations& from, con
TransformOperation* fromOp = i < fromSize ? from[i].get() : 0;
TransformOperation* toOp = i < toSize ? to[i].get() : 0;
TransformOperation* blendedOp = toOp ? toOp->blend(fromOp, progress) :
fromOp->blend(0, progress, true);
- result.append(blendedOp);
+ if (blendedOp)
+ result.append(blendedOp);
}
There's a problem here in some cases: <rdar://problem/6058375>
+// "animating" means that something is running that requires the timer to keep
firing
+// (e.g. a software animation)
e.g. transition
+void CompositeAnimation::setAnimationStartTime(double t)
+{
+}
Remove?
+void CompositeAnimation::setTransitionStartTime(int property, double t)
+{
+ // Set start time for given property transition
+ CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
+ for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin();
it != end; ++it) {
+ ImplicitAnimation* anim = it->second;
+ if (anim && anim->waitingForStartTime() &&
+ (anim->transitionProperty() == property ||
anim->transitionProperty() == cAnimateAll))
+
anim->updateStateMachine(AnimationBase::STATE_INPUT_START_TIME_SET, t);
+ }
+}
I don't think this will ever get called.
+void CompositeAnimation::suspendAnimations()
+{
+ if (m_suspended)
+ return;
+
+ m_suspended = true;
+
+ CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
+ for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin();
it != end; ++it) {
+ ImplicitAnimation* anim = it->second;
+ if (anim && anim->hasStyle()) {
+ anim->updatePlayState(false);
+ }
+ }
+}
Hmm, does playState do anything for transitions?
+void CompositeAnimation::overrideImplicitAnimations(int property)
+{
+ CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
+ for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin();
it != end; ++it) {
+ ImplicitAnimation* anim = it->second;
+ if (anim && (anim->transitionProperty() == property ||
anim->transitionProperty() == cAnimateAll))
+ anim->setOverridden(true);
+ }
+}
+
+void CompositeAnimation::resumeOverriddenImplicitAnimations(int property)
+{
+ CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
+ for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin();
it != end; ++it) {
+ ImplicitAnimation* anim = it->second;
+ if (anim && (anim->transitionProperty() == property ||
anim->transitionProperty() == cAnimateAll))
+ anim->setOverridden(false);
+ }
+}
Would be nice to have some comments explaing what this "overriding" is all
about.
diff --git a/WebCore/rendering/style/RenderStyle.cpp
b/WebCore/rendering/style/RenderStyle.cpp
index 0213a29..402c5a2 100644
--- a/WebCore/rendering/style/RenderStyle.cpp
+++ b/WebCore/rendering/style/RenderStyle.cpp
@@ -41,6 +41,12 @@ static bool imagesEquivalent(StyleImage* image1, StyleImage*
image2)
return true;
}
+// compare floating points, taking nan into account
+static inline bool floatingPointValuesEqual(float a, float b)
+{
+ return (isnan(a) && isnan(b)) || (a == b);
+}
Don't need this.
@@ -1869,32 +1851,58 @@ const Vector<StyleDashboardRegion>&
RenderStyle::noneDashboardRegions()
void RenderStyle::adjustTransitions()
{
if (transitions()) {
- if (transitions()->isEmpty()) {
+ if (transitions()->isEmptyOrZeroDuration()) {
clearTransitions();
return;
}
Transition* next;
for (Transition* p = accessTransitions(); p; p = next) {
- next = p->m_next;
+ next = p->next();
if (next && next->isEmpty()) {
- delete next;
- p->m_next = 0;
+ p->setNext(0); // removes the ref, so next gets deleted
+ next = 0;
break;
}
}
-
+
+ if (!transitions())
+ return;
+
// Repeat patterns into layers that don't have some properties set.
accessTransitions()->fillUnsetProperties();
+
+ // make sure there are no duplicate properties. This is an O(n^2)
algorithm
+ // but the lists tend to be very short, so it is probably ok
Not when it could be used as a DOS attack. Would be nice to find something more
efficient.
diff --git a/WebCore/rendering/style/RenderStyle.h
b/WebCore/rendering/style/RenderStyle.h
index ebab8b6..2d21186 100644
--- a/WebCore/rendering/style/RenderStyle.h
+++ b/WebCore/rendering/style/RenderStyle.h
@@ -1257,58 +1257,68 @@ private:
- int m_duration;
- int m_repeatCount;
+private:
+ double m_duration;
TimingFunction m_timingFunction;
+ float m_delay;
Stupid question: why is duration a double, and delay a float?
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
More information about the webkit-unassigned
mailing list