Summary: | adding animation property support to RenderStyle | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||||||
Component: | CSS | Assignee: | 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
Dean Jackson
2008-07-16 17:52:02 PDT
Created attachment 22327 [details]
adds animation properties into RenderStyle and Transition
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.
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.
Filed bug 20148 on fixing keyframes to not live in RenderStyle. Comment on attachment 22327 [details]
adds animation properties into RenderStyle and Transition
Ok, r=me assuming cleanup is done.
Created attachment 22489 [details]
updated patch
Updated patch with review fixes. It won't be committed until a blocking patch lands.
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.
|