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+

Description Chris Marrin 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.
Comment 1 Chris Marrin 2008-09-22 09:47:53 PDT
That's actually keyframeStylesForElement() since it collects style for all keyframes at once
Comment 2 mitz 2008-09-24 13:08:33 PDT
Raising to P1 because of dependent bug 21043.
Comment 3 Chris Marrin 2008-09-25 14:15:57 PDT
Created attachment 23821 [details]
Patch, including LayoutTest file
Comment 4 Chris Marrin 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.
Comment 5 Dave Hyatt 2008-09-29 12:38:56 PDT
Comment on attachment 23821 [details]
Patch, including LayoutTest file

r=me
Comment 6 Simon Fraser (smfr) 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)
Comment 7 Chris Marrin 2008-10-09 16:27:36 PDT
*** Bug 20148 has been marked as a duplicate of this bug. ***