Bug 20995

Summary: Rewrite keyframe resolution to be like styleForElement()
Product: WebKit Reporter: Chris Marrin <cmarrin>
Component: CSSAssignee: Chris Marrin <cmarrin>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, mitz, simon.fraser
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 21043    
Attachments:
Description Flags
Patch, including LayoutTest file hyatt: review+

Chris Marrin
Reported 2008-09-22 09:45:31 PDT
Today, I resolve style when the style sheet is read. This has several problems, most notably that it can't resolve physical units, like ems. So it needs to change. Hyatt suggests: 1) In addKeyframeStyle, just store the CSSStyleDeclarations for each keyframe 2) Create a new method (keyframeStyleForElement?) which resolves keyframes given an element and animation name 3) When starting an animation, use this method to get the list of RenderStyles for each keyframe, caching them for the duration of the animation, and using them as today to run the animation.
Attachments
Patch, including LayoutTest file (36.31 KB, patch)
2008-09-25 14:15 PDT, Chris Marrin
hyatt: review+
Chris Marrin
Comment 1 2008-09-22 09:47:53 PDT
That's actually keyframeStylesForElement() since it collects style for all keyframes at once
mitz
Comment 2 2008-09-24 13:08:33 PDT
Raising to P1 because of dependent bug 21043.
Chris Marrin
Comment 3 2008-09-25 14:15:57 PDT
Created attachment 23821 [details] Patch, including LayoutTest file
Chris Marrin
Comment 4 2008-09-25 14:22:25 PDT
The fix is as outlines above. I ended up duplicating the element's RenderStyle into each keyframe RenderStyle, so they had full information for doing things like lineheight and other font metrics (and possibly others). This makes the RenderStyles a bit more heavyweight in some cases, but I think the model is right. The submitted testcase tests the line-height property, which was borked before this change.
Dave Hyatt
Comment 5 2008-09-29 12:38:56 PDT
Comment on attachment 23821 [details] Patch, including LayoutTest file r=me
Simon Fraser (smfr)
Comment 6 2008-09-29 14:31:37 PDT
Committed r37077 M WebCore/rendering/style/KeyframeList.cpp M WebCore/rendering/style/StyleRareNonInheritedData.h M WebCore/rendering/style/StyleRareNonInheritedData.cpp M WebCore/rendering/style/Animation.h M WebCore/rendering/style/Animation.cpp M WebCore/rendering/style/KeyframeList.h M WebCore/rendering/style/RenderStyle.h M WebCore/ChangeLog M WebCore/css/CSSStyleSelector.cpp M WebCore/css/CSSStyleSelector.h M WebCore/page/animation/CompositeAnimation.cpp M WebCore/page/animation/KeyframeAnimation.cpp M WebCore/page/animation/KeyframeAnimation.h M LayoutTests/ChangeLog A LayoutTests/animations/lineheight-animation.html A LayoutTests/animations/lineheight-animation-expected.txt r37077 = e979de02d997b41646bbf7c704ca6bc0c6c3bfdb (trunk)
Chris Marrin
Comment 7 2008-10-09 16:27:36 PDT
*** Bug 20148 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.