Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(251)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 194733002: Web Animations: Use StringKeyframes for element.animate() (Closed)

Created:
6 years, 9 months ago by alancutter (OOO until 2018)
Modified:
6 years, 8 months ago
Reviewers:
shans, dstockwell, esprehn
CC:
blink-reviews, ed+blinkwatch_opera.com, shans, rjwright, Mike Lawther (Google), rwlbuis, rune+blink, dglazkov+blink, dstockwell, Timothy Loh, apavlov+blink_chromium.org, darktears, Steve Block, dino_apple.com, Eric Willigers
Visibility:
Public.

Description

Web Animations: Use StringKeyframes for element.animate() This patch refactors element.animate()'s use of KeyframeEffectModel to be based on String keyframes instead of AnimatableValue keyframes. element.animate() is no longer responsible for creating snapshots of the keyframe values, this logic is now pushed into StringPropertySpecificKeyframe::createInterpolation() where we can work on removing the use of AnimatableValues in favour of more primitive InterpolableValues. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170943

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rebase #

Patch Set 3 : shans suggestion #

Patch Set 4 : Rebase passing element.animate() interpolation tests #

Patch Set 5 : Rebased #

Total comments: 7

Patch Set 6 : Rebased #

Patch Set 7 : Review changes #

Total comments: 4

Patch Set 8 : More comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -218 lines) Patch
M LayoutTests/animations/interpolation/background-position-interpolation-expected.txt View 1 2 3 1 chunk +21 lines, -21 lines 0 comments Download
M LayoutTests/animations/interpolation/background-position-origin-interpolation-expected.txt View 1 2 3 1 chunk +45 lines, -45 lines 0 comments Download
M LayoutTests/animations/interpolation/border-color-interpolation-expected.txt View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/animations/interpolation/border-radius-interpolation-expected.txt View 1 2 3 1 chunk +11 lines, -11 lines 0 comments Download
M LayoutTests/animations/interpolation/border-spacing-interpolation-expected.txt View 1 2 3 1 chunk +6 lines, -6 lines 0 comments Download
M LayoutTests/animations/interpolation/border-width-interpolation-expected.txt View 1 2 3 1 chunk +6 lines, -6 lines 0 comments Download
M LayoutTests/animations/interpolation/box-shadow-interpolation-expected.txt View 1 2 3 1 chunk +5 lines, -5 lines 0 comments Download
M LayoutTests/animations/interpolation/margin-interpolation-expected.txt View 1 2 3 1 chunk +5 lines, -5 lines 0 comments Download
M LayoutTests/animations/interpolation/outline-offset-interpolation-expected.txt View 1 2 3 1 chunk +6 lines, -6 lines 0 comments Download
M LayoutTests/animations/interpolation/outline-width-interpolation-expected.txt View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/animations/interpolation/padding-interpolation-expected.txt View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/animations/interpolation/transform-interpolation-expected.txt View 1 2 3 1 chunk +18 lines, -18 lines 0 comments Download
M Source/core/animation/AnimatableValueKeyframe.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimatableValueKeyframe.cpp View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimationTest.cpp View 1 2 3 4 5 6 1 chunk +4 lines, -7 lines 0 comments Download
M Source/core/animation/EffectInput.cpp View 1 2 3 4 5 6 2 chunks +15 lines, -27 lines 0 comments Download
M Source/core/animation/Keyframe.h View 1 2 3 4 2 chunks +3 lines, -1 line 0 comments Download
M Source/core/animation/KeyframeEffectModel.h View 1 2 3 4 2 chunks +10 lines, -1 line 0 comments Download
M Source/core/animation/KeyframeEffectModel.cpp View 1 2 3 4 4 chunks +4 lines, -2 lines 0 comments Download
M Source/core/animation/StringKeyframe.h View 1 2 3 4 5 6 2 chunks +22 lines, -17 lines 0 comments Download
M Source/core/animation/StringKeyframe.cpp View 1 2 3 4 5 6 7 3 chunks +30 lines, -12 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 1 2 3 4 5 6 7 1 chunk +6 lines, -13 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
alancutter (OOO until 2018)
This patch is for review only as part of an AnimatableValue refactor and is not ...
6 years, 9 months ago (2014-03-11 09:42:55 UTC) #1
alancutter (OOO until 2018)
https://codereview.chromium.org/194733002/diff/1/Source/core/animation/KeyframeEffectModel.cpp File Source/core/animation/KeyframeEffectModel.cpp (right): https://codereview.chromium.org/194733002/diff/1/Source/core/animation/KeyframeEffectModel.cpp#newcode294 Source/core/animation/KeyframeEffectModel.cpp:294: return LegacyStyleInterpolation::create(from, to, property); from.release(), to.release()
6 years, 9 months ago (2014-03-11 09:45:33 UTC) #2
alancutter (OOO until 2018)
6 years, 8 months ago (2014-04-04 06:49:10 UTC) #3
alancutter (OOO until 2018)
+esprehn for StyleResolver changes.
6 years, 8 months ago (2014-04-04 06:50:39 UTC) #4
esprehn
lgtm https://codereview.chromium.org/194733002/diff/80001/Source/core/animation/EffectInput.cpp File Source/core/animation/EffectInput.cpp (right): https://codereview.chromium.org/194733002/diff/80001/Source/core/animation/EffectInput.cpp#newcode48 Source/core/animation/EffectInput.cpp:48: element.document().updateRenderTreeIfNeeded(); I assume you're going to remove this ...
6 years, 8 months ago (2014-04-04 22:37:11 UTC) #5
shans
lgtm I'm curious why all those tests are suddenly passing though? https://codereview.chromium.org/194733002/diff/80001/Source/core/animation/EffectInput.cpp File Source/core/animation/EffectInput.cpp (right): ...
6 years, 8 months ago (2014-04-06 20:25:29 UTC) #6
alancutter (OOO until 2018)
On 2014/04/06 20:25:29, shans wrote: > lgtm > > I'm curious why all those tests ...
6 years, 8 months ago (2014-04-07 00:02:25 UTC) #7
dstockwell
lgtm https://codereview.chromium.org/194733002/diff/120001/Source/core/animation/StringKeyframe.cpp File Source/core/animation/StringKeyframe.cpp (right): https://codereview.chromium.org/194733002/diff/120001/Source/core/animation/StringKeyframe.cpp#newcode72 Source/core/animation/StringKeyframe.cpp:72: // FIXME: Remove the use of AnimatableValues and ...
6 years, 8 months ago (2014-04-07 00:14:05 UTC) #8
alancutter (OOO until 2018)
https://codereview.chromium.org/194733002/diff/120001/Source/core/animation/StringKeyframe.cpp File Source/core/animation/StringKeyframe.cpp (right): https://codereview.chromium.org/194733002/diff/120001/Source/core/animation/StringKeyframe.cpp#newcode72 Source/core/animation/StringKeyframe.cpp:72: // FIXME: Remove the use of AnimatableValues and RenderStyles ...
6 years, 8 months ago (2014-04-07 01:06:02 UTC) #9
alancutter (OOO until 2018)
The CQ bit was checked by alancutter@chromium.org
6 years, 8 months ago (2014-04-07 01:06:14 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alancutter@chromium.org/194733002/140001
6 years, 8 months ago (2014-04-07 01:06:17 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-07 01:25:43 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on win_blink_compile_dbg
6 years, 8 months ago (2014-04-07 01:25:43 UTC) #13
alancutter (OOO until 2018)
The CQ bit was checked by alancutter@chromium.org
6 years, 8 months ago (2014-04-07 05:20:48 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alancutter@chromium.org/194733002/140001
6 years, 8 months ago (2014-04-07 05:20:58 UTC) #15
commit-bot: I haz the power
6 years, 8 months ago (2014-04-07 05:21:28 UTC) #16
Message was sent while issue was closed. 
Change committed as 170943

Powered by Google App Engine
This is Rietveld 408576698