Skip to content

Commit

Permalink
Use interpolation at the endpoints of animations.
Browse files Browse the repository at this point in the history
Use interpolation at the endpoints of animations (by removing various
optimizations to return the endpoint values at the endpoints), since
it's needed to get the correct axis for animation of the 'rotate'
property, the correct conversion away from a 'none' value at one
endpoint for the 'rotate', 'scale', and 'translate' properties, and
correct list lengths for list-valued properties that can interpolate
between lists of mismatched lengths, whether by repeating to the least
common multiple length (stroke-dasharray) or filling the shorter list
with no-op or zero values (filter, backdrop-filter, box-shadow,
text-shadow, some registered custom properties).

The changes to translate-composition.html and scale-composition.html
cause Firefox to pass the tests whose expectations are being modified,
and thus, like Chrome, pass the entire file (whereas they cause Safari
to fail additional tests).  The test changes are discussed further in
web-platform-tests/wpt#30377 .

The differences between the test expectations for background-image
(where the test expects discrete animation, per the spec) and for
-webkit-mask-image (where the test expects -webkit-cross-fade()) is
rather suspicious, but I've left the difference as-is for now.

Fixed: 1026169, 1180834
Change-Id: I6320b74b0aff29989a748fab1bff78b91426701e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3139948
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#923427}
  • Loading branch information
dbaron authored and Chromium LUCI CQ committed Sep 21, 2021
1 parent f94f2a6 commit 1962045
Show file tree
Hide file tree
Showing 30 changed files with 106 additions and 2,580 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ class CSSAnimationsTest : public RenderingTest, public PaintTestConfigurations {
->Amount();
}

double GetSaturateFilterAmount(Element* element) {
EXPECT_EQ(1u, element->GetComputedStyle()->Filter().size());
const FilterOperation* filter =
element->GetComputedStyle()->Filter().Operations()[0];
EXPECT_EQ(FilterOperation::OperationType::SATURATE, filter->GetType());
return static_cast<const BasicColorMatrixFilterOperation*>(filter)
->Amount();
}

void InvalidateCompositorKeyframesSnapshot(Animation* animation) {
auto* keyframe_effect = DynamicTo<KeyframeEffect>(animation->effect());
DCHECK(keyframe_effect);
Expand Down Expand Up @@ -134,7 +143,7 @@ TEST_P(CSSAnimationsTest, RetargetedTransition) {
TEST_P(CSSAnimationsTest, IncompatibleRetargetedTransition) {
SetBodyInnerHTML(R"HTML(
<style>
#test { transition: filter 1s; }
#test { transition: filter 1s linear; }
.saturate { filter: saturate(20%); }
.contrast { filter: contrast(20%); }
</style>
Expand All @@ -152,14 +161,13 @@ TEST_P(CSSAnimationsTest, IncompatibleRetargetedTransition) {
EXPECT_TRUE(animation->HasActiveAnimationsOnCompositor());
AdvanceClockSeconds(0.003);

// The computed style still contains no filter until the next frame.
EXPECT_TRUE(element->GetComputedStyle()->Filter().IsEmpty());
UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(1.0 * (1 - 0.003) + 0.2 * 0.003, GetSaturateFilterAmount(element));

// Now we start a contrast filter. Since it will try to combine with
// the in progress saturate filter, and be incompatible, there should
// be no transition and it should immediately apply on the next frame.
element->setAttribute(html_names::kClassAttr, "contrast");
EXPECT_TRUE(element->GetComputedStyle()->Filter().IsEmpty());
UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(0.2, GetContrastFilterAmount(element));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ void InvalidatableInterpolation::Interpolate(int, double fraction) {
if (fraction == current_fraction_)
return;

if (current_fraction_ == 0 || current_fraction_ == 1 || fraction == 0 ||
fraction == 1) {
ClearConversionCache();
}

current_fraction_ = fraction;
if (is_conversion_cached_ && cached_pair_conversion_)
cached_pair_conversion_->InterpolateValue(fraction, cached_value_);
Expand All @@ -32,7 +27,6 @@ std::unique_ptr<PairwisePrimitiveInterpolation>
InvalidatableInterpolation::MaybeConvertPairwise(
const InterpolationEnvironment& environment,
const UnderlyingValueOwner& underlying_value_owner) const {
DCHECK(current_fraction_ != 0 && current_fraction_ != 1);
for (const auto& interpolation_type : *interpolation_types_) {
if ((start_keyframe_->IsNeutral() || end_keyframe_->IsNeutral()) &&
(!underlying_value_owner ||
Expand Down Expand Up @@ -105,14 +99,12 @@ InvalidatableInterpolation::MaybeConvertUnderlyingValue(
}

bool InvalidatableInterpolation::DependsOnUnderlyingValue() const {
return (start_keyframe_->UnderlyingFraction() != 0 &&
current_fraction_ != 1) ||
(end_keyframe_->UnderlyingFraction() != 0 && current_fraction_ != 0);
return start_keyframe_->UnderlyingFraction() != 0 ||
end_keyframe_->UnderlyingFraction() != 0;
}

bool InvalidatableInterpolation::IsNeutralKeyframeActive() const {
return (start_keyframe_->IsNeutral() && current_fraction_ != 1) ||
(end_keyframe_->IsNeutral() && current_fraction_ != 0);
return start_keyframe_->IsNeutral() || end_keyframe_->IsNeutral();
}

void InvalidatableInterpolation::ClearConversionCache() const {
Expand Down Expand Up @@ -154,27 +146,21 @@ InvalidatableInterpolation::EnsureValidConversion(
if (IsConversionCacheValid(environment, underlying_value_owner))
return cached_value_.get();
ClearConversionCache();
if (current_fraction_ == 0) {
cached_value_ = ConvertSingleKeyframe(*start_keyframe_, environment,
underlying_value_owner);
} else if (current_fraction_ == 1) {
cached_value_ = ConvertSingleKeyframe(*end_keyframe_, environment,
underlying_value_owner);

std::unique_ptr<PairwisePrimitiveInterpolation> pairwise_conversion =
MaybeConvertPairwise(environment, underlying_value_owner);
if (pairwise_conversion) {
cached_value_ = pairwise_conversion->InitialValue();
cached_pair_conversion_ = std::move(pairwise_conversion);
} else {
std::unique_ptr<PairwisePrimitiveInterpolation> pairwise_conversion =
MaybeConvertPairwise(environment, underlying_value_owner);
if (pairwise_conversion) {
cached_value_ = pairwise_conversion->InitialValue();
cached_pair_conversion_ = std::move(pairwise_conversion);
} else {
cached_pair_conversion_ = std::make_unique<FlipPrimitiveInterpolation>(
ConvertSingleKeyframe(*start_keyframe_, environment,
underlying_value_owner),
ConvertSingleKeyframe(*end_keyframe_, environment,
underlying_value_owner));
}
cached_pair_conversion_->InterpolateValue(current_fraction_, cached_value_);
cached_pair_conversion_ = std::make_unique<FlipPrimitiveInterpolation>(
ConvertSingleKeyframe(*start_keyframe_, environment,
underlying_value_owner),
ConvertSingleKeyframe(*end_keyframe_, environment,
underlying_value_owner));
}
cached_pair_conversion_->InterpolateValue(current_fraction_, cached_value_);

is_conversion_cached_ = true;
return cached_value_.get();
}
Expand Down Expand Up @@ -269,14 +255,14 @@ void InvalidatableInterpolation::ApplyStack(
continue;
should_apply = true;
current_interpolation.SetFlagIfInheritUsed(environment);
double underlying_fraction = current_interpolation.UnderlyingFraction();
if (underlying_fraction == 0 || !underlying_value_owner ||
if (!current_interpolation.DependsOnUnderlyingValue() ||
!underlying_value_owner ||
underlying_value_owner.GetType() != current_value->GetType()) {
underlying_value_owner.Set(current_value);
} else {
current_value->GetType().Composite(
underlying_value_owner, underlying_fraction, current_value->Value(),
current_interpolation.current_fraction_);
underlying_value_owner, current_interpolation.UnderlyingFraction(),
current_value->Value(), current_interpolation.current_fraction_);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,20 @@ namespace blink {

void TransitionInterpolation::Interpolate(int iteration, double fraction) {
if (cached_fraction_ != fraction || cached_iteration_ != iteration) {
if (fraction != 0 && fraction != 1) {
merge_.start_interpolable_value->Interpolate(
*merge_.end_interpolable_value, fraction,
*cached_interpolable_value_);
}
merge_.start_interpolable_value->Interpolate(
*merge_.end_interpolable_value, fraction, *cached_interpolable_value_);
cached_iteration_ = iteration;
cached_fraction_ = fraction;
}
}

const InterpolableValue& TransitionInterpolation::CurrentInterpolableValue()
const {
if (cached_fraction_ == 0) {
return *start_.interpolable_value;
}
if (cached_fraction_ == 1) {
return *end_.interpolable_value;
}
return *cached_interpolable_value_;
}

const NonInterpolableValue*
TransitionInterpolation::CurrentNonInterpolableValue() const {
if (cached_fraction_ == 0) {
return start_.non_interpolable_value.get();
}
if (cached_fraction_ == 1) {
return end_.non_interpolable_value.get();
}
return merge_.non_interpolable_value.get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ PASS Compositing: property <backdrop-filter> underlying [sepia(0.5)] from add [s
PASS Compositing: property <backdrop-filter> underlying [sepia(0.5)] from add [sepia(0.5)] to replace [sepia(1)] at (0.25) is [sepia(0.625) sepia(0.375)]
PASS Compositing: property <backdrop-filter> underlying [sepia(0.5)] from add [sepia(0.5)] to replace [sepia(1)] at (0.5) is [sepia(0.75) sepia(0.25)]
PASS Compositing: property <backdrop-filter> underlying [sepia(0.5)] from add [sepia(0.5)] to replace [sepia(1)] at (0.75) is [sepia(0.875) sepia(0.125)]
FAIL Compositing: property <backdrop-filter> underlying [sepia(0.5)] from add [sepia(0.5)] to replace [sepia(1)] at (1) is [sepia(1)] assert_equals: expected "sepia ( 1 ) sepia ( 0 ) " but got "sepia ( 1 ) "
PASS Compositing: property <backdrop-filter> underlying [sepia(0.5)] from add [sepia(0.5)] to replace [sepia(1)] at (1) is [sepia(1) sepia(0)]
PASS Compositing: property <backdrop-filter> underlying [sepia(0.5)] from add [sepia(0.5)] to replace [sepia(1)] at (1.5) is [sepia(1) sepia(0)]
PASS Compositing: property <backdrop-filter> underlying [brightness(0)] from replace [brightness(0.5)] to add [brightness(1.5)] at (-0.5) is [brightness(0.75) brightness(0.75)]
FAIL Compositing: property <backdrop-filter> underlying [brightness(0)] from replace [brightness(0.5)] to add [brightness(1.5)] at (0) is [brightness(0.5)] assert_equals: expected "brightness ( 0.5 ) brightness ( 1 ) " but got "brightness ( 0.5 ) "
PASS Compositing: property <backdrop-filter> underlying [brightness(0)] from replace [brightness(0.5)] to add [brightness(1.5)] at (0) is [brightness(0.5) brightness(1)]
PASS Compositing: property <backdrop-filter> underlying [brightness(0)] from replace [brightness(0.5)] to add [brightness(1.5)] at (0.25) is [brightness(0.375) brightness(1.125)]
PASS Compositing: property <backdrop-filter> underlying [brightness(0)] from replace [brightness(0.5)] to add [brightness(1.5)] at (0.5) is [brightness(0.25) brightness(1.25)]
PASS Compositing: property <backdrop-filter> underlying [brightness(0)] from replace [brightness(0.5)] to add [brightness(1.5)] at (0.75) is [brightness(0.125) brightness(1.375)]
Expand All @@ -29,7 +29,7 @@ PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [
PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [saturate(2)] to add [saturate(3)] at (1) is [invert(0.5) saturate(3)]
PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [saturate(2)] to add [saturate(3)] at (1.5) is [invert(0.5) saturate(3.5)]
PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [none] to replace [invert(1) saturate(3)] at (-0.5) is [invert(0.25) saturate(0)]
FAIL Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [none] to replace [invert(1) saturate(3)] at (0) is [invert(0.5)] assert_equals: expected "invert ( 0.5 ) saturate ( 1 ) " but got "invert ( 0.5 ) "
PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [none] to replace [invert(1) saturate(3)] at (0) is [invert(0.5) saturate(1)]
PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [none] to replace [invert(1) saturate(3)] at (0.25) is [invert(0.625) saturate(1.5)]
PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [none] to replace [invert(1) saturate(3)] at (0.5) is [invert(0.75) saturate(2)]
PASS Compositing: property <backdrop-filter> underlying [invert(0.5)] from add [none] to replace [invert(1) saturate(3)] at (0.75) is [invert(0.875) saturate(2.5)]
Expand Down Expand Up @@ -176,7 +176,7 @@ PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumu
PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(30px)] to replace [blur(100px)] at (1) is [blur(100px)]
PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(30px)] to replace [blur(100px)] at (1.5) is [blur(130px)]
PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(40px)] to add [blur(100px)] at (-0.5) is [blur(70px) blur(0px)]
FAIL Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(40px)] to add [blur(100px)] at (0) is [blur(50px)] assert_equals: expected "blur ( 50px ) blur ( 0px ) " but got "blur ( 50px ) "
PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(40px)] to add [blur(100px)] at (0) is [blur(50px) blur(0px)]
PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(40px)] to add [blur(100px)] at (0.25) is [blur(40px) blur(25px)]
PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(40px)] to add [blur(100px)] at (0.5) is [blur(30px) blur(50px)]
PASS Compositing: property <backdrop-filter> underlying [blur(10px)] from accumulate [blur(40px)] to add [blur(100px)] at (0.75) is [blur(20px) blur(75px)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@
replaceTo: '15 20 25',
}, [
{at: -0.2, is: ' 3 8 1 9 2 7'},
{at: 0, is: ' 5 10'},
{at: 0, is: ' 5 10 5 10 5 10'},
{at: 0.2, is: ' 7 12 9 11 8 13'},
{at: 0.4, is: ' 9 14 13 12 11 16'},
{at: 0.6, is: '11 16 17 13 14 19'},
{at: 0.8, is: '13 18 21 14 17 22'},
{at: 1, is: '15 20 25'},
{at: 1, is: '15 20 25 15 20 25'},
{at: 1.2, is: '17 22 29 16 23 28'},
]);

Expand All @@ -95,12 +95,12 @@
addTo: '15 0 25 10 35 20',
}, [
{at: -0.2, is: ' 1 6 11 16 0 2 13 18 0 4 9 14'}, // Values must be non-negative.
{at: 0, is: ' 5 10 15 20'},
{at: 0, is: ' 5 10 15 20 5 10 15 20 5 10 15 20'},
{at: 0.2, is: ' 9 14 19 24 13 18 17 22 11 16 21 26'},
{at: 0.4, is: '13 18 23 28 21 26 19 24 17 22 27 32'},
{at: 0.6, is: '17 22 27 32 29 34 21 26 23 28 33 38'},
{at: 0.8, is: '21 26 31 36 37 42 23 28 29 34 39 44'},
{at: 1, is: '25 30 35 40 45 50'},
{at: 1, is: '25 30 35 40 45 50 25 30 35 40 45 50'},
{at: 1.2, is: '29 34 39 44 53 58 27 32 41 46 51 56'},
]);

Expand All @@ -112,12 +112,12 @@
addTo: '10 5 20 15',
}, [
{at: -0.2, is: ' 2 7 12 0 8 13 0 5 14 1 6 11'}, // Values must be non-negative.
{at: 0, is: ' 5 10 15'},
{at: 0, is: ' 5 10 15 5 10 15 5 10 15 5 10 15'},
{at: 0.2, is: ' 8 13 18 11 12 17 10 15 16 9 14 19'},
{at: 0.4, is: '11 16 21 17 14 19 15 20 17 13 18 23'},
{at: 0.6, is: '14 19 24 23 16 21 20 25 18 17 22 27'},
{at: 0.8, is: '17 22 27 29 18 23 25 30 19 21 26 31'},
{at: 1, is: '20 25 30 35'},
{at: 1, is: '20 25 30 35 20 25 30 35 20 25 30 35'},
{at: 1.2, is: '23 28 33 41 22 27 35 40 21 29 34 39'},
]);

Expand All @@ -129,12 +129,12 @@
addTo: '15 20 25 30 35',
}, [
{at: -0.2, is: ' 2 7 12 0 4 14 1 6 11 0 8 13 0 5 10'}, // Values must be non-negative.
{at: 0, is: ' 5 10 15'},
{at: 0, is: ' 5 10 15 5 10 15 5 10 15 5 10 15 5 10 15'},
{at: 0.2, is: ' 8 13 18 11 16 16 9 14 19 12 12 17 10 15 20'},
{at: 0.4, is: '11 16 21 17 22 17 13 18 23 19 14 19 15 20 25'},
{at: 0.6, is: '14 19 24 23 28 18 17 22 27 26 16 21 20 25 30'},
{at: 0.8, is: '17 22 27 29 34 19 21 26 31 33 18 23 25 30 35'},
{at: 1, is: '20 25 30 35 40'},
{at: 1, is: '20 25 30 35 40 20 25 30 35 40 20 25 30 35 40'},
{at: 1.2, is: '23 28 33 41 46 21 29 34 39 47 22 27 35 40 45'},
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
to: 'calc(100px + 100%)',
}, [
{at: -0.3, is: 'calc(-30% + -30px)'},
{at: 0, is: '0px'},
{at: 0, is: 'calc(0% + 0px)'},
{at: 0.5, is: 'calc(50% + 50px)'},
{at: 1, is: 'calc(100% + 100px)'},
{at: 1.5, is: 'calc(150% + 150px)'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,28 @@
});

animation.currentTime = 0;
assert_equals(getComputedStyle(target).getPropertyValue('--a'), '100', '--a at 0%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), '100', '--b at 0%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), '100', '--c at 0%');
assert_equals(getComputedStyle(target).getPropertyValue('--a'), 'initial-value', '--a at 0%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), 'initial-value', '--b at 0%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), 'initial-value', '--c at 0%');

animation.currentTime = 25;
assert_equals(getComputedStyle(target).getPropertyValue('--a'), 'initial-value', '--a at 25%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), 'initial-value', '--b at 25%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), 'initial-value', '--c at 25%');

animation.currentTime = 50;
assert_equals(getComputedStyle(target).getPropertyValue('--a'), '200', '--a at 50%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), '200', '--b at 50%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), '200', '--c at 50%');
assert_equals(getComputedStyle(target).getPropertyValue('--a'), 'initial-value', '--a at 50%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), 'initial-value', '--b at 50%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), 'initial-value', '--c at 50%');

animation.currentTime = 75;
assert_equals(getComputedStyle(target).getPropertyValue('--a'), 'initial-value', '--a at 75%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), 'initial-value', '--b at 75%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), 'initial-value', '--c at 75%');

animation.currentTime = 100;
assert_equals(getComputedStyle(target).getPropertyValue('--a'), '300', '--a at 100%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), '300', '--b at 100%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), '300', '--c at 100%');
assert_equals(getComputedStyle(target).getPropertyValue('--a'), 'initial-value', '--a at 100%');
assert_equals(getComputedStyle(target).getPropertyValue('--b'), 'initial-value', '--b at 100%');
assert_equals(getComputedStyle(target).getPropertyValue('--c'), 'initial-value', '--c at 100%');
}, 'Animated registered custom properties are invalid at computed-value time when there is a cyclic var() dependency between them.');
</script>
Loading

0 comments on commit 1962045

Please sign in to comment.