Bug 203238 - [Web Animations] Only process CSS properties affected by a given CSS transition
Summary: [Web Animations] Only process CSS properties affected by a given CSS transition
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: 2019-10-22 01:17 PDT by Antoine Quint
Modified: 2019-10-24 09:35 PDT (History)
3 users (show)

See Also:

Attachments
Patch (4.84 KB, patch)
2019-10-22 01:21 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Patch (27.28 KB, patch)
2019-10-24 02:58 PDT, Antoine Quint 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2019-10-22 01:17:18 PDT 
[Web Animations] Only process CSS properties affected by a given CSS transition
Comment 1 Antoine Quint 2019-10-22 01:21:05 PDT 
Created attachment 381524 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2019-10-22 01:21:58 PDT 
<rdar://problem/56492517>
Comment 3 Simon Fraser (smfr) 2019-10-22 08:09:32 PDT 
Comment on attachment 381524 [details]
Patch

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

> Source/WebCore/animation/AnimationTimeline.cpp:497
> +    auto compileTransitionPropertiesInStyle = [&](const RenderStyle& style) {

This is already a long function. I think you should move this and processProperty into real functions.
Comment 4 Antoine Quint 2019-10-24 02:58:56 PDT 
Created attachment 381793 [details]
Patch
Comment 5 Simon Fraser (smfr) 2019-10-24 08:49:54 PDT 
Comment on attachment 381793 [details]
Patch

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

> Source/WebCore/animation/AnimationTimeline.cpp:379
> +void AnimationTimeline::updateCSSTransitionsForElementAndProperty(Element& element, CSSPropertyID property, const RenderStyle& currentStyle, const RenderStyle& afterChangeStyle, AnimationTimeline::PropertyToTransitionMap& runningTransitionsByProperty, PropertyToTransitionMap& completedTransitionsByProperty, const MonotonicTime& generationTime)

You can pass MonotonicTime by value (it has sizeof(double)).

> Source/WebCore/animation/AnimationTimeline.cpp:535
>          }

I would return here and remove the else.
Comment 6 Antoine Quint 2019-10-24 09:35:16 PDT 
Committed r251543: <https://trac.webkit.org/changeset/251543>