New issue
Advanced search Search tips

Issue 785526 link

Starred by 1 user
Status: Fixed
Owner:
smcgruer@chromium.org
Closed: Nov 2017
Cc:
flackr@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Web Animations: a Keyframe should only have a composite operation if it differs from the KeyframeEffect one

Project Member Reported by smcgruer@chromium.org, Nov 15 2017 
Currently the individual Keyframe objects each track a composite order, even if the user doesn't specify one on that keyframe. But the spec says:

"Each keyframe may also have a keyframe-specific composite operation that is applied to all values specified in that keyframe. The possible operations and their meanings are identical to those defined for the composite operation associated with the keyframe effect as a whole in ยง4.5.4 Effect composition. If no keyframe-specific composite operation is specified for a keyframe, the composite operation specified for the keyframe effect as a whole is used for values specified in that keyframe."

We need to add the concept of 'composite' to KeyframeEffectReadOnly and no longer always assign them in Keyframe creation.

Comment 1 by nainar@chromium.org, Nov 16 2017

Labels: Hotlist-Interop Update-Quarterly
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/41658d028f33d2ce5992165844e4dad9ff2015f5

commit 41658d028f33d2ce5992165844e4dad9ff2015f5
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Wed Nov 29 20:26:21 2017

Support the KeyframeEffectReadOnly composite property

Previously we associated the 'composite' property with the Keyframe,
including the default 'replace' value. However the spec differentiates
between the KeyframeEffectReadOnly composite property and additional
keyframe-specific composite properties. This CL updates the code to
properly track these two concepts. Keyframe now has an optional
keyframe-specific composite operation, and the final composite operation
is determined when creating the property-specific keyframes.

Unfortunately due to the way the code is structured this change was
non-trivial. Major changes:

  * KeyframeEffectReadOnly and InertEffect now store a
    KeyframeEffectModelBase rather than an EffectModel, due to the
    need to store and retrieve the 'composite' value.
  * The KeyframeEffectModelBase on KeyframeEffectReadOnly is now
    non-nullable. Previously a null model indicated that a
    KeyframeEffectReadOnly had no keyframes; this is now tracked
    explicitly in the model.

Bug:  785526 
Change-Id: I9b4967bf11c010bbd1f61f2d5ced1159fc0dd0c4
Reviewed-on: https://chromium-review.googlesource.com/786292
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520224}
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/LayoutTests/external/wpt/web-animations/animation-model/combining-effects/effect-composition-expected.txt
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/LayoutTests/external/wpt/web-animations/interfaces/Animatable/animate-expected.txt
[delete] https://crrev.com/3a959a80c53000f0d3e1fb86d48dc2ec38e61336/third_party/WebKit/LayoutTests/external/wpt/web-animations/interfaces/KeyframeEffect/composite-expected.txt
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/LayoutTests/external/wpt/web-animations/interfaces/KeyframeEffect/constructor-expected.txt
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/LayoutTests/external/wpt/web-animations/interfaces/KeyframeEffect/idlharness-expected.txt
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/AnimationTest.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/BUILD.gn
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/DocumentTimelineTest.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/EffectInput.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/EffectInput.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/EffectInputTest.cpp
[add] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/EffectModel.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/EffectModel.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/EffectStackTest.cpp
[add] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/ElementAnimation.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/ElementAnimation.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/InertEffect.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/InertEffect.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/Keyframe.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/Keyframe.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffect.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffect.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffect.idl
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffectModel.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffectModel.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffectOptions.idl
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffectReadOnly.idl
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/KeyframeEffectTest.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/StringKeyframe.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/StringKeyframe.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/TransitionKeyframe.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/TransitionKeyframe.h
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/core/inspector/InspectorAnimationAgent.cpp
[modify] https://crrev.com/41658d028f33d2ce5992165844e4dad9ff2015f5/third_party/WebKit/Source/modules/animationworklet/WorkletAnimation.cpp

Comment 3 by smcgruer@chromium.org, Nov 29 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment