Web Animations CSS: Support animation of fill and stroke

Web Animations CSS: Support animation of fill and stroke

Note that we step paints other than rbg colors at 50%. The existing implementation
did not attempt to set these values at all.

BUG=257591
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158904

[Steve Block, 2013-10-03 02:37:53]

https://codereview.chromium.org/25456002/diff/1/LayoutTests/animations/interp...
File LayoutTests/animations/interpolation/svg-fill-interpolation.html (right):

https://codereview.chromium.org/25456002/diff/1/LayoutTests/animations/interp...
LayoutTests/animations/interpolation/svg-fill-interpolation.html:32: {at: -0.4,
is: '#ffe700'},
Can you add a test at a more negative fraction, such that the green channel gets
clamped too?

https://codereview.chromium.org/25456002/diff/1/LayoutTests/animations/interp...
LayoutTests/animations/interpolation/svg-fill-interpolation.html:45: {at: 0.6,
is: 'url(#gradient)'}
Can you test at 0 and 1 too to make clear this is a step

https://codereview.chromium.org/25456002/diff/1/Source/core/animation/Animata...
File Source/core/animation/AnimatableSVGPaint.cpp (right):

https://codereview.chromium.org/25456002/diff/1/Source/core/animation/Animata...
Source/core/animation/AnimatableSVGPaint.cpp:41: return
AnimatableSVGPaint::create(this->paintType(),
this->m_color.interpolateTo(svgPaint->m_color, fraction), this->uri());
If you test for (paintType() == SVG_PAINTTYPE_RGBCOLOR && svgPaint->paintType()
== SVG_PAINTTYPE_RGBCOLOR) it would be more clear why it's OK to pass
this->paintType() to create(). You could even pass SVG_PAINTTYPE_RGBCOLOR
directly.

Also, no need for this->foo().

Also, can you assert that the uri is unused/empty in this case?

https://codereview.chromium.org/25456002/diff/1/Source/core/animation/Animata...
Source/core/animation/AnimatableSVGPaint.cpp:49: return
AnimatableSVGPaint::create(this->paintType(),
this->m_color.addWith(svgPaint->m_color), this->uri());
Similar comments to above.

[dstockwell, 2013-10-03 05:01:07]

https://codereview.chromium.org/25456002/diff/1/LayoutTests/animations/interp...
File LayoutTests/animations/interpolation/svg-fill-interpolation.html (right):

https://codereview.chromium.org/25456002/diff/1/LayoutTests/animations/interp...
LayoutTests/animations/interpolation/svg-fill-interpolation.html:32: {at: -0.4,
is: '#ffe700'},
On 2013/10/03 02:37:53, Steve Block wrote:
> Can you add a test at a more negative fraction, such that the green channel
gets
> clamped too?

Done.

https://codereview.chromium.org/25456002/diff/1/LayoutTests/animations/interp...
LayoutTests/animations/interpolation/svg-fill-interpolation.html:45: {at: 0.6,
is: 'url(#gradient)'}
On 2013/10/03 02:37:53, Steve Block wrote:
> Can you test at 0 and 1 too to make clear this is a step

Done.

https://codereview.chromium.org/25456002/diff/1/Source/core/animation/Animata...
File Source/core/animation/AnimatableSVGPaint.cpp (right):

https://codereview.chromium.org/25456002/diff/1/Source/core/animation/Animata...
Source/core/animation/AnimatableSVGPaint.cpp:41: return
AnimatableSVGPaint::create(this->paintType(),
this->m_color.interpolateTo(svgPaint->m_color, fraction), this->uri());
On 2013/10/03 02:37:53, Steve Block wrote:
> If you test for (paintType() == SVG_PAINTTYPE_RGBCOLOR &&
svgPaint->paintType()
> == SVG_PAINTTYPE_RGBCOLOR) it would be more clear why it's OK to pass
> this->paintType() to create(). You could even pass SVG_PAINTTYPE_RGBCOLOR
> directly.
> 
> Also, no need for this->foo().
> 
> Also, can you assert that the uri is unused/empty in this case?

Done.

https://codereview.chromium.org/25456002/diff/1/Source/core/animation/Animata...
Source/core/animation/AnimatableSVGPaint.cpp:49: return
AnimatableSVGPaint::create(this->paintType(),
this->m_color.addWith(svgPaint->m_color), this->uri());
On 2013/10/03 02:37:53, Steve Block wrote:
> Similar comments to above.

Done.

[Steve Block, 2013-10-04 00:20:44]

lgtm

[commit-bot: I haz the power, 2013-10-04 00:43:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dstockwell@chromium.org/25456002/7001

[commit-bot: I haz the power, 2013-10-04 07:27:47]

Change committed as 158904