Bug 178674

Summary:

[Web Animations] Add basic timing and target properties

Product:

WebKit

Reporter:

Antoine Quint <graouts>

Component:

Animations

Assignee:

Antoine Quint <graouts>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dino, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=122912

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch for landing	none

Antoine Quint

Reported 2017-10-23 11:04:26 PDT

[Web Animations] Add basic timing and target properties

Attachments
Patch (58.44 KB, patch) 2017-10-23 11:07 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (59.41 KB, patch) 2017-10-23 14:25 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (59.12 KB, patch) 2017-10-23 14:47 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (59.16 KB, patch) 2017-10-23 14:54 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (59.16 KB, patch) 2017-10-23 15:01 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (59.66 KB, patch) 2017-10-23 15:07 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (59.68 KB, patch) 2017-10-23 22:22 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (56.11 KB, patch) 2017-10-23 22:44 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (56.50 KB, patch) 2017-10-23 23:13 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (56.49 KB, patch) 2017-10-23 23:23 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch for landing (56.49 KB, patch) 2017-10-24 00:16 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Antoine Quint

Comment 1 2017-10-23 11:07:59 PDT

Created attachment 324567 [details] Patch

Simon Fraser (smfr)

Comment 2 2017-10-23 11:12:39 PDT

Comment on attachment 324567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324567&action=review > Source/WebCore/animation/AnimationEffectTiming.h:45 > + double m_duration; Can we use Seconds here? > Source/WebCore/animation/WebAnimation.h:55 > + std::optional<double> m_startTime; MonotonicTime

Dean Jackson

Comment 3 2017-10-23 11:20:41 PDT

Comment on attachment 324567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324567&action=review > Source/WebCore/animation/AnimationEffect.cpp:41 > +AnimationEffect::~AnimationEffect() > +{ > +} Will this eventually have code? If not, put it in the header. > Source/WebCore/animation/WebAnimation.h:55 > + std::optional<double> m_startTime; As smfr probably said, use Seconds or something. > Source/WebCore/animation/WebAnimation.idl:35 > ] interface WebAnimation { > + attribute AnimationEffect? effect; > readonly attribute AnimationTimeline? timeline; > + attribute double? startTime; > }; Is this weird spacing standard? > Source/WebCore/bindings/js/JSAnimationEffectCustom.cpp:42 > +JSValue toJSNewlyCreated(ExecState*, JSDOMGlobalObject* globalObject, Ref<AnimationEffect>&& value) > +{ > + if (value->isKeyframeEffect()) > + return createWrapper<KeyframeEffect>(globalObject, WTFMove(value)); > + return createWrapper<AnimationEffect>(globalObject, WTFMove(value)); > +} Wish this code was automatic. Paging Rabbi Weinig. > LayoutTests/ChangeLog:23 > + * webanimations/animation-effect-expected.txt: Added. > + * webanimations/animation-effect-timing-expected.txt: Added. > + * webanimations/animation-effect-timing.html: Added. > + * webanimations/animation-effect.html: Added. > + * webanimations/animation-interface-effect-property-expected.txt: Added. > + * webanimations/animation-interface-effect-property.html: Added. > + * webanimations/animation-interface-start-time-property-expected.txt: Added. > + * webanimations/animation-interface-start-time-property.html: Added. > + * webanimations/keyframe-effect-expected.txt: Added. > + * webanimations/keyframe-effect-interface-timing-duration-expected.txt: Added. > + * webanimations/keyframe-effect-interface-timing-duration.html: Added. > + * webanimations/keyframe-effect.html: Added. > + All these tests should be written for Web Platform Tests. Put them in http/wpt/ for now, with the goal of submitting them to WPT. Use the WPT API for testing.

Dean Jackson

Comment 4 2017-10-23 12:16:30 PDT

Comment on attachment 324567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324567&action=review > Source/WebCore/animation/WebAnimation.h:49 > + void setStartTime(std::optional<double> startTime) { m_startTime = startTime; } I'm sure you're changing this, but no need for the optional in the parameter. If you're setting the value, it clearly exists.

Antoine Quint

Comment 5 2017-10-23 14:25:56 PDT

Created attachment 324588 [details] Patch

Dean Jackson

Comment 6 2017-10-23 14:33:03 PDT

Comment on attachment 324588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324588&action=review > Source/WebCore/ChangeLog:18 > + Tests: webanimations/animation-effect-timing.html > + webanimations/animation-effect.html > + webanimations/animation-interface-effect-property.html > + webanimations/animation-interface-start-time-property.html > + webanimations/keyframe-effect-interface-timing-duration.html > + webanimations/keyframe-effect.html Please file a bug to move these to WPT. > Source/WebCore/animation/AnimationEffect.h:44 > + enum ClassType { Use enum class. Then you'll probably need AnimationEffect::Foo elsewhere. > Source/WebCore/animation/WebAnimation.h:50 > + std::optional<double> bindingsStartTime() const; > + void setBindingsStartTime(std::optional<double>); > + std::optional<MonotonicTime> startTime() const { return m_startTime; } > + void setStartTime(MonotonicTime& startTime) { m_startTime = startTime; } You're fixing my bad advice here.

Antoine Quint

Comment 7 2017-10-23 14:47:18 PDT

Created attachment 324590 [details] Patch

Antoine Quint

Comment 8 2017-10-23 14:54:20 PDT

Created attachment 324591 [details] Patch

Antoine Quint

Comment 9 2017-10-23 15:01:27 PDT

Created attachment 324595 [details] Patch

Antoine Quint

Comment 10 2017-10-23 15:07:57 PDT

Created attachment 324596 [details] Patch