Bug 182903 - [Web Animations] Store all parsed keyframe input information in a single structure
Summary: [Web Animations] Store all parsed keyframe input information in a single stru...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-17 05:54 PST by Antoine Quint
Modified: 2018-02-19 11:15 PST (History)
2 users (show)

See Also:


Attachments
Patch (59.52 KB, patch)
2018-02-17 06:13 PST, Antoine Quint
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2018-02-17 05:54:10 PST
[Web Animations] Store all parsed keyframe input information in a single structure
Comment 1 Antoine Quint 2018-02-17 06:13:21 PST
Created attachment 334107 [details]
Patch
Comment 2 Dean Jackson 2018-02-19 09:50:15 PST
Comment on attachment 334107 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334107&action=review

> Source/WebCore/animation/KeyframeEffectReadOnly.cpp:85
> +        keyframe.computedOffset = keyframe.offset ? keyframe.offset.value() : 0;

keyframe.offset.value_or(0)

> Source/WebCore/animation/KeyframeEffectReadOnly.cpp:442
> +        ParsedKeyframe parsedKeyframe;
> +        parsedKeyframe.easing = sourceParsedKeyframe.easing;
> +        parsedKeyframe.offset = sourceParsedKeyframe.offset;
> +        parsedKeyframe.composite = sourceParsedKeyframe.composite;
> +        parsedKeyframe.unparsedStyle = sourceParsedKeyframe.unparsedStyle;
> +        parsedKeyframe.computedOffset = sourceParsedKeyframe.computedOffset;
> +        parsedKeyframe.timingFunction = sourceParsedKeyframe.timingFunction;
> +        parsedKeyframe.style = sourceParsedKeyframe.style->mutableCopy();

You should make a operator= for this.
Comment 3 Antoine Quint 2018-02-19 10:04:15 PST
(In reply to Dean Jackson from comment #2)
> Comment on attachment 334107 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334107&action=review
> 
> > Source/WebCore/animation/KeyframeEffectReadOnly.cpp:85
> > +        keyframe.computedOffset = keyframe.offset ? keyframe.offset.value() : 0;
> 
> keyframe.offset.value_or(0)

Very cool.

> > Source/WebCore/animation/KeyframeEffectReadOnly.cpp:442
> > +        ParsedKeyframe parsedKeyframe;
> > +        parsedKeyframe.easing = sourceParsedKeyframe.easing;
> > +        parsedKeyframe.offset = sourceParsedKeyframe.offset;
> > +        parsedKeyframe.composite = sourceParsedKeyframe.composite;
> > +        parsedKeyframe.unparsedStyle = sourceParsedKeyframe.unparsedStyle;
> > +        parsedKeyframe.computedOffset = sourceParsedKeyframe.computedOffset;
> > +        parsedKeyframe.timingFunction = sourceParsedKeyframe.timingFunction;
> > +        parsedKeyframe.style = sourceParsedKeyframe.style->mutableCopy();
> 
> You should make a operator= for this.

I intentionally did not provide one because I would like to avoid unexpected copies of ParsedKeyframe.
Comment 4 Antoine Quint 2018-02-19 11:06:30 PST
Committed r228702: <https://trac.webkit.org/changeset/228702>
Comment 5 Radar WebKit Bug Importer 2018-02-19 11:07:15 PST
<rdar://problem/37677856>