[webkit-reviews] review granted: [Bug 20068] adding animation property support to RenderStyle : [Attachment 22327] adds animation properties into RenderStyle and Transition

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 23 13:53:12 PDT 2008


Dave Hyatt <hyatt at apple.com> has granted Dean Jackson <dino at apple.com>'s
request for review:
Bug 20068: adding animation property support to RenderStyle
https://bugs.webkit.org/show_bug.cgi?id=20068

Attachment 22327: adds animation properties into RenderStyle and Transition
https://bugs.webkit.org/attachment.cgi?id=22327&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
-    setStyle(style);
+    if (style)
+	 setStyle(style);

setAnimatableStyle should never be called with a null style.   This smacks of
working around some other problem.  If you have a test case where a null style
was set, then we should be debugging how that happened and stop it rather than
throwing in an unnecessary null check.

I think "Transition" could ultimately be split up into multiple classes (it
seems gross to have all the animation and transition properties together in a
single class that is still called "Transition").  I think this could be done
later though.

Keyframes are all wrong, but I have promised to look the other way for this
initial landing.

EAnimPlayState is only 2 bits so could really be part of the "isSet" bitfield
properties if you moved it to after the keyframe list and made it into:

unsigned m_playState : 2;

(unsigned for Windows compiler's benefit)

That would save a bit of memory.

r=me without making these changes, but I'd like to see the null check removed.


More information about the webkit-reviews mailing list