Bug 20068

Summary: adding animation property support to RenderStyle
Product: WebKit Reporter: Dean Jackson <dino>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on: 19938    
Bug Blocks: 19028    
Attachments:
Description Flags
adds animation properties into RenderStyle and Transition
hyatt: review+
updated patch
none
Second update to patch dino: review+

Dean Jackson
Reported 2008-07-16 17:52:02 PDT
RenderStyle and its Transition class needs to be extended to support the animation properties: - direction - iterationCount - playState - name This patch adds these properties, but doesn't actually set them (ie. does not implement the CSS parsing part).
Attachments
adds animation properties into RenderStyle and Transition (25.89 KB, patch)
2008-07-16 17:53 PDT, Dean Jackson
hyatt: review+
updated patch (38.16 KB, patch)
2008-07-25 18:08 PDT, Dean Jackson
no flags
Second update to patch (28.63 KB, patch)
2008-07-31 10:45 PDT, Dean Jackson
dino: review+
Dean Jackson
Comment 1 2008-07-16 17:53:15 PDT
Created attachment 22327 [details] adds animation properties into RenderStyle and Transition
Dave Hyatt
Comment 2 2008-07-23 13:53:11 PDT
Comment on attachment 22327 [details] adds animation properties into RenderStyle and Transition - 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.
Dave Hyatt
Comment 3 2008-07-23 13:58:51 PDT
Comment on attachment 22327 [details] adds animation properties into RenderStyle and Transition Oh I also forgot that KeyframeList needs to be updated to the new refcounting model (with a create function). That's probably enough to tip this over into needing a new patch.
Simon Fraser (smfr)
Comment 4 2008-07-23 14:02:08 PDT
Filed bug 20148 on fixing keyframes to not live in RenderStyle.
Dave Hyatt
Comment 5 2008-07-23 14:11:41 PDT
Comment on attachment 22327 [details] adds animation properties into RenderStyle and Transition Ok, r=me assuming cleanup is done.
Dean Jackson
Comment 6 2008-07-25 18:08:59 PDT
Created attachment 22489 [details] updated patch Updated patch with review fixes. It won't be committed until a blocking patch lands.
Dean Jackson
Comment 7 2008-07-31 10:45:32 PDT
Created attachment 22579 [details] Second update to patch Second update to patch. Hyatt already gave a review+. This is waiting on the blocking bug before it can be landed.
Dean Jackson
Comment 8 2008-08-05 12:09:11 PDT
Committed r35568 M WebCore/rendering/style/RenderStyle.cpp M WebCore/rendering/style/RenderStyle.h M WebCore/ChangeLog M WebCore/css/CSSStyleSelector.cpp M WebCore/css/CSSComputedStyleDeclaration.cpp M WebCore/page/AnimationController.cpp
Note You need to log in before you can comment on or make changes to this bug.