Reland "Use retargeted start point when starting composited transitions."

This reverts commit 426a53b7.

Original review at
https://chromium-review.googlesource.com/c/chromium/src/+/1126519

TBR=majidvp@chromium.org

Bug: 853698
Change-Id: I8dce9db88ee4af68d5cf93e89ac15963f39f6da0
Reviewed-on: https://chromium-review.googlesource.com/1133125
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574170}

third_party/blink/renderer/core/BUILD.gn

@@ -1645,6 +1645,7 @@ jumbo_source_set("unit_tests") {
     "animation/animation_test_helper.cc",
     "animation/animation_test_helper.h",
     "animation/compositor_animations_test.cc",
+    "animation/css/css_animations_test.cc",
     "animation/css/css_transition_data_test.cc",
     "animation/document_timeline_test.cc",
     "animation/effect_input_test.cc",

third_party/blink/renderer/core/animation/css/css_animations.cc

@@ -199,6 +199,24 @@ StringKeyframeEffectModel* CreateKeyframeEffectModel(
   return model;
 }
 
+// Sample the given |animation| at the given |inherited_time|. Returns nullptr
+// if the |inherited_time| falls outside of the animation.
+std::unique_ptr<TypedInterpolationValue> SampleAnimation(
+    Animation* animation,
+    double inherited_time) {
+  KeyframeEffect* effect = ToKeyframeEffect(animation->effect());
+  InertEffect* inert_animation_for_sampling = InertEffect::Create(
+      effect->Model(), effect->SpecifiedTiming(), false, inherited_time);
+  Vector<scoped_refptr<Interpolation>> sample;
+  inert_animation_for_sampling->Sample(sample);
+  // Transition animation has only animated a single property or is not in
+  // effect.
+  DCHECK_LE(sample.size(), 1u);
+  if (sample.IsEmpty())
+    return nullptr;
+  return ToTransitionInterpolation(*sample.at(0)).GetInterpolatedValue();
+}
+
 }  // namespace
 
 CSSAnimations::CSSAnimations() = default;
@@ -525,8 +543,7 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
   // be when transitions are retargeted. Instead of triggering complete style
   // recalculation, we find these cases by searching for new transitions that
   // have matching cancelled animation property IDs on the compositor.
-  HeapHashMap<PropertyHandle, std::pair<Member<KeyframeEffect>, double>>
-      retargeted_compositor_transitions;
+  HashSet<PropertyHandle> retargeted_compositor_transitions;
   for (const PropertyHandle& property :
        pending_update_.CancelledTransitions()) {
     DCHECK(transitions_.Contains(property));
@@ -537,10 +554,7 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
         pending_update_.NewTransitions().find(property) !=
             pending_update_.NewTransitions().end() &&
         !animation->Limited()) {
-      retargeted_compositor_transitions.insert(
-          property,
-          std::pair<KeyframeEffect*, double>(
-              effect, animation->StartTimeInternal().value_or(NullValue())));
+      retargeted_compositor_transitions.insert(property);
     }
     animation->cancel();
     // after cancelation, transitions must be downgraded or they'll fail
@@ -579,44 +593,6 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
 
     KeyframeEffectModelBase* model = inert_animation->Model();
 
-    if (retargeted_compositor_transitions.Contains(property)) {
-      const std::pair<Member<KeyframeEffect>, double>& old_transition =
-          retargeted_compositor_transitions.at(property);
-      KeyframeEffect* old_animation = old_transition.first;
-      double old_start_time = old_transition.second;
-      double inherited_time =
-          IsNull(old_start_time)
-              ? 0
-              : element->GetDocument().Timeline().CurrentTimeInternal() -
-                    old_start_time;
-
-      TransitionKeyframeEffectModel* old_effect =
-          ToTransitionKeyframeEffectModel(inert_animation->Model());
-      const KeyframeVector& frames = old_effect->GetFrames();
-
-      TransitionKeyframeVector new_frames;
-      new_frames.push_back(ToTransitionKeyframe(frames[0]->Clone().get()));
-      new_frames.push_back(ToTransitionKeyframe(frames[1]->Clone().get()));
-      new_frames.push_back(ToTransitionKeyframe(frames[2]->Clone().get()));
-
-      InertEffect* inert_animation_for_sampling = InertEffect::Create(
-          old_animation->Model(), old_animation->SpecifiedTiming(), false,
-          inherited_time);
-      Vector<scoped_refptr<Interpolation>> sample;
-      inert_animation_for_sampling->Sample(sample);
-      if (sample.size() == 1) {
-        const TransitionInterpolation& interpolation =
-            ToTransitionInterpolation(*sample.at(0));
-        new_frames[0]->SetValue(interpolation.GetInterpolatedValue());
-        new_frames[0]->SetCompositorValue(
-            interpolation.GetInterpolatedCompositorValue());
-        new_frames[1]->SetValue(interpolation.GetInterpolatedValue());
-        new_frames[1]->SetCompositorValue(
-            interpolation.GetInterpolatedCompositorValue());
-        model = TransitionKeyframeEffectModel::Create(new_frames);
-      }
-    }
-
     KeyframeEffect* transition = KeyframeEffect::Create(
         element, model, inert_animation->SpecifiedTiming(),
         KeyframeEffect::kTransitionPriority, event_delegate);
@@ -678,6 +654,7 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
   }
 
   const RunningTransition* interrupted_transition = nullptr;
+  const RunningTransition* retargeted_compositor_transition = nullptr;
   if (state.active_transitions) {
     TransitionMap::const_iterator active_transition_iter =
         state.active_transitions->find(property);
@@ -689,6 +666,10 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
         return;
       }
       state.update.CancelTransition(property);
+      KeyframeEffect* effect =
+          ToKeyframeEffect(running_transition->animation->effect());
+      if (effect->HasActiveAnimationsOnCompositor())
+        retargeted_compositor_transition = running_transition;
       DCHECK(!state.animating_element->GetElementAnimations() ||
              !state.animating_element->GetElementAnimations()
                   ->IsAnimationStyleChange());
@@ -718,22 +699,52 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
                                state.animating_element->GetDocument());
   CSSInterpolationEnvironment old_environment(map, state.old_style);
   CSSInterpolationEnvironment new_environment(map, state.style);
+  const InterpolationType* transition_type = nullptr;
   InterpolationValue start = nullptr;
   InterpolationValue end = nullptr;
-  const InterpolationType* transition_type = nullptr;
-  for (const auto& interpolation_type : map.Get(property)) {
-    start = interpolation_type->MaybeConvertUnderlyingValue(old_environment);
-    if (!start) {
-      continue;
-    }
-    end = interpolation_type->MaybeConvertUnderlyingValue(new_environment);
-    if (!end) {
-      continue;
+  if (retargeted_compositor_transition) {
+    double old_start_time =
+        retargeted_compositor_transition->animation->StartTimeInternal()
+            .value_or(NullValue());
+    // TODO(flackr): This should be able to just use
+    // animation->currentTime() / 1000 rather than trying to calculate current
+    // time.
+    double inherited_time = IsNull(old_start_time)
+                                ? 0
+                                : state.animating_element->GetDocument()
+                                          .Timeline()
+                                          .CurrentTimeInternal() -
+                                      old_start_time;
+    std::unique_ptr<TypedInterpolationValue> retargeted_start = SampleAnimation(
+        retargeted_compositor_transition->animation, inherited_time);
+    if (retargeted_start) {
+      const InterpolationType& interpolation_type = retargeted_start->GetType();
+      start = retargeted_start->Value().Clone();
+      end = interpolation_type.MaybeConvertUnderlyingValue(new_environment);
+      if (end &&
+          interpolation_type.MaybeMergeSingles(start.Clone(), end.Clone()))
+        transition_type = &interpolation_type;
+    } else {
+      // If the previous transition was not in effect it is not used for
+      // retargeting.
+      retargeted_compositor_transition = nullptr;
     }
-    // Merge will only succeed if the two values are considered interpolable.
-    if (interpolation_type->MaybeMergeSingles(start.Clone(), end.Clone())) {
-      transition_type = interpolation_type.get();
-      break;
+  }
+  if (!retargeted_compositor_transition) {
+    for (const auto& interpolation_type : map.Get(property)) {
+      start = interpolation_type->MaybeConvertUnderlyingValue(old_environment);
+      if (!start) {
+        continue;
+      }
+      end = interpolation_type->MaybeConvertUnderlyingValue(new_environment);
+      if (!end) {
+        continue;
+      }
+      // Merge will only succeed if the two values are considered interpolable.
+      if (interpolation_type->MaybeMergeSingles(start.Clone(), end.Clone())) {
+        transition_type = interpolation_type.get();
+        break;
+      }
     }
   }
 

third_party/blink/renderer/core/animation/css/css_animations_test.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/animation/css/css_animations.h"
+#include "third_party/blink/renderer/core/animation/animation.h"
+#include "third_party/blink/renderer/core/animation/element_animations.h"
+#include "third_party/blink/renderer/core/dom/node_computed_style.h"
+#include "third_party/blink/renderer/core/layout/layout_object.h"
+#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
+#include "third_party/blink/renderer/platform/animation/compositor_animation_delegate.h"
+#include "third_party/blink/renderer/platform/testing/testing_platform_support_with_mock_scheduler.h"
+
+namespace blink {
+
+namespace {
+
+class TestingPlatformSupportWithMockSchedulerAndThreadedAnimations
+    : public TestingPlatformSupportWithMockScheduler {
+ public:
+  bool IsThreadedAnimationEnabled() override { return true; }
+};
+
+}  // namespace
+
+class CSSAnimationsTest : public RenderingTest {
+ public:
+  void SetUp() override {
+    platform_->SetAutoAdvanceNowToPendingTasks(false);
+    // Advance timer manually as RenderingTest expects the time to be non-zero.
+    platform_->AdvanceClockSeconds(1.);
+    RenderingTest::SetUp();
+    EnableCompositing();
+  }
+
+  void TearDown() override {
+    platform_->SetAutoAdvanceNowToPendingTasks(true);
+    platform_->RunUntilIdle();
+  }
+
+  void StartAnimationOnCompositor(Animation* animation) {
+    static_cast<CompositorAnimationDelegate*>(animation)
+        ->NotifyAnimationStarted(
+            (CurrentTimeTicks() - base::TimeTicks()).InSecondsF(),
+            animation->CompositorGroup());
+  }
+
+  void AdvanceClockSeconds(double seconds) {
+    platform_->AdvanceClockSeconds(seconds);
+    platform_->RunUntilIdle();
+  }
+
+  double GetContrastFilterAmount(Element* element) {
+    EXPECT_EQ(1u, element->GetComputedStyle()->Filter().size());
+    const FilterOperation* filter =
+        element->GetComputedStyle()->Filter().Operations()[0];
+    EXPECT_EQ(FilterOperation::OperationType::CONTRAST, filter->GetType());
+    return static_cast<const BasicComponentTransferFilterOperation*>(filter)
+        ->Amount();
+  }
+
+ private:
+  ScopedTestingPlatformSupport<
+      TestingPlatformSupportWithMockSchedulerAndThreadedAnimations>
+      platform_;
+};
+
+// Verify that a composited animation is retargeted according to its composited
+// time.
+TEST_F(CSSAnimationsTest, RetargetedTransition) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #test { transition: filter linear 1s; }
+      .contrast1 { filter: contrast(50%); }
+      .contrast2 { filter: contrast(0%); }
+    </style>
+    <div id='test'></div>
+  )HTML");
+  Element* element = GetDocument().getElementById("test");
+  element->setAttribute(HTMLNames::classAttr, "contrast1");
+  GetDocument().View()->UpdateAllLifecyclePhases();
+  ElementAnimations* animations = element->GetElementAnimations();
+  EXPECT_EQ(1u, animations->Animations().size());
+  Animation* animation = (*animations->Animations().begin()).key;
+  // Start animation on compositor and advance .8 seconds.
+  StartAnimationOnCompositor(animation);
+  EXPECT_TRUE(animation->HasActiveAnimationsOnCompositor());
+  AdvanceClockSeconds(0.8);
+
+  // Starting the second transition should retarget the active transition.
+  element->setAttribute(HTMLNames::classAttr, "contrast2");
+  GetPage().Animator().ServiceScriptedAnimations(CurrentTimeTicks());
+  GetDocument().View()->UpdateAllLifecyclePhases();
+  EXPECT_DOUBLE_EQ(0.6, GetContrastFilterAmount(element));
+
+  // As it has been retargeted, advancing halfway should go to 0.3.
+  AdvanceClockSeconds(0.5);
+  GetPage().Animator().ServiceScriptedAnimations(CurrentTimeTicks());
+  GetDocument().View()->UpdateAllLifecyclePhases();
+  EXPECT_DOUBLE_EQ(0.3, GetContrastFilterAmount(element));
+}
+
+// Test that when an incompatible in progress compositor transition
+// would be retargeted it does not incorrectly combine with a new
+// transition target.
+TEST_F(CSSAnimationsTest, IncompatibleRetargetedTransition) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #test { transition: filter 1s; }
+      .saturate { filter: saturate(20%); }
+      .contrast { filter: contrast(20%); }
+    </style>
+    <div id='test'></div>
+  )HTML");
+  Element* element = GetDocument().getElementById("test");
+  element->setAttribute(HTMLNames::classAttr, "saturate");
+  GetDocument().View()->UpdateAllLifecyclePhases();
+  ElementAnimations* animations = element->GetElementAnimations();
+  EXPECT_EQ(1u, animations->Animations().size());
+  Animation* animation = (*animations->Animations().begin()).key;
+
+  // Start animation on compositor and advance partially.
+  StartAnimationOnCompositor(animation);
+  EXPECT_TRUE(animation->HasActiveAnimationsOnCompositor());
+  AdvanceClockSeconds(0.003);
+
+  // The computed style still contains no filter until the next frame.
+  EXPECT_TRUE(element->GetComputedStyle()->Filter().IsEmpty());
+
+  // 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(HTMLNames::classAttr, "contrast");
+  EXPECT_TRUE(element->GetComputedStyle()->Filter().IsEmpty());
+  GetDocument().View()->UpdateAllLifecyclePhases();
+  EXPECT_EQ(0.2, GetContrastFilterAmount(element));
+}
+
+}  // namespace blink